From 730b800db7b309118263ffe4ce0f7f79c9c3003d Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Tue, 1 Nov 2011 11:07:24 +0100 Subject: [PATCH] IDEA-76146 (Inspection "Return of collection type 'xxx'" generates wrong code for EnumSets) --- .../ReturnOfCollectionFieldInspection.java | 148 ++++++++++-------- 1 file changed, 87 insertions(+), 61 deletions(-) diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/encapsulation/ReturnOfCollectionFieldInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/encapsulation/ReturnOfCollectionFieldInspection.java index 25b3131b40f5..4a15061dfea6 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/encapsulation/ReturnOfCollectionFieldInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/encapsulation/ReturnOfCollectionFieldInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2010 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2011 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,8 @@ import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; import com.intellij.psi.*; +import com.intellij.psi.codeStyle.JavaCodeStyleManager; +import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.IncorrectOperationException; import com.siyeh.InspectionGadgetsBundle; @@ -26,6 +28,7 @@ import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.psiutils.CollectionUtils; +import com.siyeh.ig.psiutils.HighlightUtils; import com.siyeh.ig.psiutils.TypeUtils; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -49,17 +52,14 @@ public class ReturnOfCollectionFieldInspection extends BaseInspection { @Override @NotNull public String getDisplayName() { - return InspectionGadgetsBundle.message( - "return.of.collection.array.field.display.name"); + return InspectionGadgetsBundle.message("return.of.collection.array.field.display.name"); } @Override @Nullable public JComponent createOptionsPanel() { - return new SingleCheckboxOptionsPanel( - InspectionGadgetsBundle.message( - "return.of.collection.array.field.option"), this, - "ignorePrivateMethods"); + return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("return.of.collection.array.field.option"), + this, "ignorePrivateMethods"); } @Override @@ -68,53 +68,35 @@ public class ReturnOfCollectionFieldInspection extends BaseInspection { final PsiField field = (PsiField)infos[0]; final PsiType type = field.getType(); if (type instanceof PsiArrayType) { - return InspectionGadgetsBundle.message( - "return.of.collection.array.field.problem.descriptor.array"); + return InspectionGadgetsBundle.message("return.of.collection.array.field.problem.descriptor.array"); } else { - return InspectionGadgetsBundle.message( - "return.of.collection.array.field.problem.descriptor.collection"); + return InspectionGadgetsBundle.message("return.of.collection.array.field.problem.descriptor.collection"); } } @Override @Nullable protected InspectionGadgetsFix buildFix(Object... infos) { - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)infos[1]; + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)infos[1]; final String text = referenceExpression.getText(); - if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, - CommonClassNames.JAVA_UTIL_MAP)) { - if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, - "java.util.SortedMap")) { - return new ReturnOfCollectionFieldFix( - "java.util.Collections.unmodifiableSortedMap(" + - text + ')'); + if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, CommonClassNames.JAVA_UTIL_MAP)) { + if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, "java.util.SortedMap")) { + return new ReturnOfCollectionFieldFix("java.util.Collections.unmodifiableSortedMap(" + text + ')', "java.util.SortedMap"); } - return new ReturnOfCollectionFieldFix( - "java.util.Collections.unmodifiableMap(" + text + ')'); + return new ReturnOfCollectionFieldFix("java.util.Collections.unmodifiableMap(" + text + ')', CommonClassNames.JAVA_UTIL_MAP); } - else if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, - CommonClassNames.JAVA_UTIL_COLLECTION)) { - if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, - CommonClassNames.JAVA_UTIL_SET)) { - if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, - "java.util.SortedSet")) { - return new ReturnOfCollectionFieldFix( - "java.util.Collections.unmodifiableSortedSet(" + - text + ')'); + else if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, CommonClassNames.JAVA_UTIL_COLLECTION)) { + if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, CommonClassNames.JAVA_UTIL_SET)) { + if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, "java.util.SortedSet")) { + return new ReturnOfCollectionFieldFix("java.util.Collections.unmodifiableSortedSet(" + text + ')', "java.util.SortedSet"); } - return new ReturnOfCollectionFieldFix( - "java.util.Collections.unmodifiableSet(" + text + ')'); + return new ReturnOfCollectionFieldFix("java.util.Collections.unmodifiableSet(" + text + ')', "java.util.Set"); } - else if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, - CommonClassNames.JAVA_UTIL_LIST)) { - return new ReturnOfCollectionFieldFix( - "java.util.Collections.unmodifiableList(" + text + ')'); + else if (TypeUtils.expressionHasTypeOrSubtype(referenceExpression, CommonClassNames.JAVA_UTIL_LIST)) { + return new ReturnOfCollectionFieldFix("java.util.Collections.unmodifiableList(" + text + ')', "java.util.List"); } - return new ReturnOfCollectionFieldFix( - "java.util.Collections.unmodifiableCollection(" + text + - ')'); + return new ReturnOfCollectionFieldFix("java.util.Collections.unmodifiableCollection(" + text + ')', "java.util.Collection"); } return null; } @@ -124,36 +106,84 @@ public class ReturnOfCollectionFieldInspection extends BaseInspection { return new ReturnOfCollectionFieldVisitor(); } - private static class ReturnOfCollectionFieldFix - extends InspectionGadgetsFix { + private static class ReturnOfCollectionFieldFix extends InspectionGadgetsFix { - private final String replacementText; + private final String myReplacementText; + private final String myQualifiedClassName; - ReturnOfCollectionFieldFix(@NonNls String replacementText) { - this.replacementText = replacementText; + ReturnOfCollectionFieldFix(@NonNls String replacementText, String qualifiedClassName) { + myReplacementText = replacementText; + myQualifiedClassName = qualifiedClassName; } @NotNull public String getName() { return InspectionGadgetsBundle.message( - "return.of.collection.field.quickfix", replacementText); + "return.of.collection.field.quickfix", myReplacementText); } @Override - protected void doFix(Project project, ProblemDescriptor descriptor) - throws IncorrectOperationException { + protected void doFix(Project project, ProblemDescriptor descriptor) throws IncorrectOperationException { final PsiElement element = descriptor.getPsiElement(); if (!(element instanceof PsiReferenceExpression)) { return; } - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)element; - replaceExpressionAndShorten(referenceExpression, replacementText); + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)element; + fixContainingMethodReturnType(referenceExpression); + replaceExpressionAndShorten(referenceExpression, myReplacementText); + } + + private void fixContainingMethodReturnType(PsiReferenceExpression referenceExpression) { + final PsiMethod method = PsiTreeUtil.getParentOfType(referenceExpression, PsiMethod.class, true); + if (method == null) { + return; + } + final PsiTypeElement returnTypeElement = method.getReturnTypeElement(); + if (returnTypeElement == null) { + return; + } + final PsiType type = returnTypeElement.getType(); + if (!InheritanceUtil.isInheritor(type, myQualifiedClassName)) { + return; + } + if (!(type instanceof PsiClassType)) { + return; + } + final Project project = referenceExpression.getProject(); + final PsiClassType classType = (PsiClassType)type; + final PsiClass aClass = classType.resolve(); + if (aClass == null || myQualifiedClassName.equals(aClass.getQualifiedName())) { + return; + } + final PsiType[] parameters = classType.getParameters(); + final PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); + final String typeText; + if (parameters.length > 0) { + final StringBuilder builder = new StringBuilder(myQualifiedClassName); + builder.append('<'); + boolean comma = false; + for (PsiType parameter : parameters) { + if (comma) { + builder.append(','); + } else { + comma = true; + } + builder.append(parameter.getCanonicalText()); + } + builder.append('>'); + typeText = builder.toString(); + } else { + typeText = myQualifiedClassName; + } + final PsiTypeElement newTypeElement = factory.createTypeElementFromText(typeText, referenceExpression); + final PsiElement replacement = returnTypeElement.replace(newTypeElement); + final JavaCodeStyleManager javaCodeStyleManager = JavaCodeStyleManager.getInstance(project); + javaCodeStyleManager.shortenClassReferences(replacement); + HighlightUtils.highlightElement(replacement); } } - private class ReturnOfCollectionFieldVisitor - extends BaseInspectionVisitor { + private class ReturnOfCollectionFieldVisitor extends BaseInspectionVisitor { @Override public void visitReturnStatement(@NotNull PsiReturnStatement statement) { @@ -162,25 +192,21 @@ public class ReturnOfCollectionFieldInspection extends BaseInspection { if (returnValue == null) { return; } - final PsiMethod containingMethod = - PsiTreeUtil.getParentOfType(statement, PsiMethod.class); + final PsiMethod containingMethod = PsiTreeUtil.getParentOfType(statement, PsiMethod.class); if (containingMethod == null) { return; } - if (ignorePrivateMethods && - containingMethod.hasModifierProperty(PsiModifier.PRIVATE)) { + if (ignorePrivateMethods && containingMethod.hasModifierProperty(PsiModifier.PRIVATE)) { return; } - final PsiClass returnStatementClass = - containingMethod.getContainingClass(); + final PsiClass returnStatementClass = containingMethod.getContainingClass(); if (returnStatementClass == null) { return; } if (!(returnValue instanceof PsiReferenceExpression)) { return; } - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)returnValue; + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)returnValue; final PsiElement referent = referenceExpression.resolve(); if (!(referent instanceof PsiField)) { return;