From 53f0b8116ae261f7163426877540f47fcb597ca5 Mon Sep 17 00:00:00 2001 From: Maxim Medvedev Date: Fri, 23 Apr 2010 10:32:41 +0400 Subject: [PATCH] improved searching of method duplicates --- .../plugins/groovy/GroovyBundle.properties | 1 + .../groovy/annotator/GroovyAnnotator.java | 47 ++++++------ .../GroovyParameterInfoHandler.java | 16 ++-- .../psi/api/types/GrClosureParameter.java | 3 - .../psi/api/types/GrClosureSignature.java | 3 + .../groovy/lang/psi/impl/GrClosureType.java | 1 + .../impl/types/GrClosureParameterImpl.java | 15 +--- .../impl/types/GrClosureSignatureImpl.java | 13 ++-- .../{ => types}/GrClosureSignatureUtil.java | 74 +++++++++++++++++-- .../plugins/groovy/lang/psi/util/PsiUtil.java | 2 +- .../groovy/lang/GroovyHighlightingTest.java | 2 + .../highlighting/MethodDuplicates.groovy | 9 +++ 12 files changed, 124 insertions(+), 62 deletions(-) rename plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/{ => types}/GrClosureSignatureUtil.java (80%) create mode 100644 plugins/groovy/testdata/highlighting/MethodDuplicates.groovy diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties b/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties index fb9defa85f29..e21c42b9f823 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties @@ -335,3 +335,4 @@ property.name.expected=Property name expected add.method.body=Add method body wildcards.are.not.allowed.in.extends.list=A super type may not specify a wildcard type method.doesnot.override.super=Method does not override method from its superclass +method.duplicate={0} is already defined diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java index 32ab9945900d..5e91f438a33a 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java @@ -33,13 +33,13 @@ import com.intellij.psi.*; import com.intellij.psi.infos.CandidateInfo; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.search.searches.SuperMethodsSearch; +import com.intellij.psi.util.MethodSignature; import com.intellij.psi.util.MethodSignatureBackedByPsiMethod; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.MultiMap; import gnu.trove.THashSet; -import gnu.trove.TObjectHashingStrategy; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.GroovyBundle; @@ -49,7 +49,6 @@ import org.jetbrains.plugins.groovy.annotator.intentions.dynamic.DynamicProperty import org.jetbrains.plugins.groovy.codeInspection.GroovyImportsTracker; import org.jetbrains.plugins.groovy.config.GroovyConfigUtils; import org.jetbrains.plugins.groovy.highlighter.DefaultHighlighter; -import org.jetbrains.plugins.groovy.intentions.utils.DuplicatesUtil; import org.jetbrains.plugins.groovy.lang.groovydoc.psi.api.*; import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes; import org.jetbrains.plugins.groovy.lang.lexer.TokenSets; @@ -83,6 +82,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.util.GrVariableDeclarationOwner import org.jetbrains.plugins.groovy.lang.psi.impl.GrClosureType; import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.TypesUtil; import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GroovyScriptClass; +import org.jetbrains.plugins.groovy.lang.psi.impl.types.GrClosureSignatureUtil; import org.jetbrains.plugins.groovy.lang.psi.util.GroovyPropertyUtils; import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil; @@ -953,34 +953,33 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator { } private static void checkDuplicateMethod(GrMethod[] methods, AnnotationHolder holder) { - final Map> map = DuplicatesUtil.factorDuplicates(methods, new TObjectHashingStrategy() { - public int computeHashCode(GrMethod method) { - return method.getSignature(PsiSubstitutor.EMPTY).hashCode(); - } - - public boolean equals(GrMethod method1, GrMethod method2) { - return method1.getSignature(PsiSubstitutor.EMPTY).equals(method2.getSignature(PsiSubstitutor.EMPTY)); - } - }); + Map> map = GrClosureSignatureUtil.findMethodSignatures(methods); processMethodDuplicates(map, holder); } - protected static void processMethodDuplicates(Map> map, AnnotationHolder holder) { - HashSet duplicateMethodsWarning = new HashSet(); - HashSet duplicateMethodsErrors = new HashSet(); - - DuplicatesUtil.collectMethodDuplicates(map, duplicateMethodsWarning, duplicateMethodsErrors); - - for (GrMethod duplicateMethod : duplicateMethodsErrors) { - holder.createErrorAnnotation(duplicateMethod.getNameIdentifierGroovy(), - GroovyBundle.message("repetitive.method.name.signature.and.return.type")); - } - - for (GrMethod duplicateMethod : duplicateMethodsWarning) { - holder.createWarningAnnotation(duplicateMethod.getNameIdentifierGroovy(), GroovyBundle.message("repetitive.method.name.signature")); + protected static void processMethodDuplicates(Map> map, AnnotationHolder holder) { + for (MethodSignature signature : map.keySet()) { + List methods = map.get(signature); + if (methods.size() > 1) { + String signaturePresentation = getSignaturePresentation(signature); + for (GrMethod method : methods) { + holder.createErrorAnnotation(method.getNameIdentifierGroovy(), GroovyBundle.message("method.duplicate", signaturePresentation)); + } + } } } + private static String getSignaturePresentation(MethodSignature signature) { + StringBuilder builder = new StringBuilder(); + builder.append(signature.getName()).append('('); + PsiType[] types = signature.getParameterTypes(); + for (PsiType type : types) { + builder.append(type.getPresentableText()).append(", "); + } + if (types.length > 0) builder.delete(builder.length() - 2, builder.length()); + builder.append(")"); + return builder.toString(); + } private static void checkTypeDefinition(AnnotationHolder holder, GrTypeDefinition typeDefinition) { final GroovyConfigUtils configUtils = GroovyConfigUtils.getInstance(); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/parameterInfo/GroovyParameterInfoHandler.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/parameterInfo/GroovyParameterInfoHandler.java index 32740d1cd782..0382bb84c211 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/parameterInfo/GroovyParameterInfoHandler.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/parameterInfo/GroovyParameterInfoHandler.java @@ -328,18 +328,16 @@ public class GroovyParameterInfoHandler implements ParameterInfoHandler 0) { for (int i = 0; i < parameters.length; i++) { if (i > 0) buffer.append(", "); - final String name = parameters[i].getName(); final PsiType psiType = parameters[i].getType(); - if (name == null) { - buffer.append(psiType == null ? "null" : psiType.getPresentableText()); + if (psiType == null) { + buffer.append("def"); } else { - String typeText = psiType == null ? "def" : psiType.getPresentableText(); - buffer.append(typeText).append(' ').append(name); - final GrExpression initializer = parameters[i].getDefaultInitializer(); - if (initializer != null) { - buffer.append(" = ").append(initializer.getText()); - } + buffer.append(psiType.getPresentableText()); + } + final GrExpression initializer = parameters[i].getDefaultInitializer(); + if (initializer != null) { + buffer.append(" = ").append(initializer.getText()); } } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/types/GrClosureParameter.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/types/GrClosureParameter.java index 70e763cfdc44..17c8a2d0b33c 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/types/GrClosureParameter.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/types/GrClosureParameter.java @@ -35,7 +35,4 @@ public interface GrClosureParameter { GrExpression getDefaultInitializer(); boolean isValid(); - - @Nullable - String getName(); } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/types/GrClosureSignature.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/types/GrClosureSignature.java index ada0d0f01584..38bc577b0b62 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/types/GrClosureSignature.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/types/GrClosureSignature.java @@ -15,6 +15,7 @@ */ package org.jetbrains.plugins.groovy.lang.psi.api.types; +import com.intellij.psi.PsiSubstitutor; import com.intellij.psi.PsiType; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -23,6 +24,8 @@ import org.jetbrains.annotations.Nullable; * @author Maxim.Medvedev */ public interface GrClosureSignature { + @NotNull PsiSubstitutor getSubstitutor(); + @NotNull GrClosureParameter[] getParameters(); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GrClosureType.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GrClosureType.java index d48a6502f629..09f696369190 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GrClosureType.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GrClosureType.java @@ -26,6 +26,7 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock; import org.jetbrains.plugins.groovy.lang.psi.api.types.GrClosureParameter; import org.jetbrains.plugins.groovy.lang.psi.api.types.GrClosureSignature; +import org.jetbrains.plugins.groovy.lang.psi.impl.types.GrClosureSignatureUtil; /** * @author ven diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureParameterImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureParameterImpl.java index 1f858715a3a2..39ee2d7315cf 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureParameterImpl.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureParameterImpl.java @@ -29,12 +29,10 @@ import org.jetbrains.plugins.groovy.lang.psi.api.types.GrClosureParameter; */ public class GrClosureParameterImpl implements GrClosureParameter { @Nullable final PsiType myType; - @Nullable final String myName=null; final boolean myOptional; @Nullable final GrExpression myDefaultInitializer; - public GrClosureParameterImpl(/*String name,*/ PsiType type, boolean optional, GrExpression defaultInitializer) { -// myName = name; + public GrClosureParameterImpl(PsiType type, boolean optional, GrExpression defaultInitializer) { myType = type; myOptional = optional; if (myOptional) { @@ -45,16 +43,12 @@ public class GrClosureParameterImpl implements GrClosureParameter { } } - public GrClosureParameterImpl(@Nullable PsiType type) { - this(/*null,*/ type, false, null); - } - public GrClosureParameterImpl(PsiParameter parameter) { this(parameter, PsiSubstitutor.EMPTY); } public GrClosureParameterImpl(PsiParameter parameter, PsiSubstitutor substitutor) { - this(/*parameter.getName(), */substitutor.substitute(parameter.getType()), + this(substitutor.substitute(parameter.getType()), parameter instanceof GrParameter ? ((GrParameter)parameter).isOptional() : false, parameter instanceof GrParameter ? ((GrParameter)parameter).getDefaultInitializer() : null); } @@ -78,11 +72,6 @@ public class GrClosureParameterImpl implements GrClosureParameter { return (myType == null || myType.isValid()) && (myDefaultInitializer == null || myDefaultInitializer.isValid()); } - @Nullable - public String getName() { - return myName; - } - @Override public boolean equals(Object obj) { if (obj instanceof GrClosureParameter) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureSignatureImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureSignatureImpl.java index 0bd43b1b2b8d..e90e1c37eeaa 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureSignatureImpl.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureSignatureImpl.java @@ -32,6 +32,7 @@ public class GrClosureSignatureImpl implements GrClosureSignature { private final boolean myIsVarargs; @Nullable private final PsiType myReturnType; @NotNull private final GrClosureParameter[] myParameters; + @NotNull private PsiSubstitutor mySubstitutor; public GrClosureSignatureImpl(@NotNull PsiParameter[] parameters, @Nullable PsiType returnType, @NotNull PsiSubstitutor substitutor) { myReturnType = substitutor.substitute(returnType); @@ -46,16 +47,13 @@ public class GrClosureSignatureImpl implements GrClosureSignature { else { myIsVarargs = false; } + mySubstitutor = substitutor; } public GrClosureSignatureImpl(PsiParameter[] parameters, PsiType returnType) { this(parameters, returnType, PsiSubstitutor.EMPTY); } - public GrClosureSignatureImpl(PsiParameter[] parameters) { - this(parameters, null); - } - public GrClosureSignatureImpl(@NotNull GrClosableBlock block) { this(block.getAllParameters(), block.getReturnType()); } @@ -68,7 +66,7 @@ public class GrClosureSignatureImpl implements GrClosureSignature { this(method.getParameterList().getParameters(), PsiUtil.getSmartReturnType(method), substitutor); } - private GrClosureSignatureImpl(@NotNull GrClosureParameter[] params, @Nullable PsiType returnType, boolean isVarArgs) { + GrClosureSignatureImpl(@NotNull GrClosureParameter[] params, @Nullable PsiType returnType, boolean isVarArgs) { myParameters = params; myReturnType = returnType; myIsVarargs = isVarArgs; @@ -85,6 +83,11 @@ public class GrClosureSignatureImpl implements GrClosureSignature { return myReturnType; } + @NotNull + public PsiSubstitutor getSubstitutor() { + return mySubstitutor; + } + @NotNull public GrClosureParameter[] getParameters() { GrClosureParameter[] result = new GrClosureParameter[myParameters.length]; diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GrClosureSignatureUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureSignatureUtil.java similarity index 80% rename from plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GrClosureSignatureUtil.java rename to plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureSignatureUtil.java index 79510b570cd9..f8788f325e39 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/GrClosureSignatureUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureSignatureUtil.java @@ -13,25 +13,27 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.jetbrains.plugins.groovy.lang.psi.impl; +package org.jetbrains.plugins.groovy.lang.psi.impl.types; +import com.intellij.openapi.util.Pair; import com.intellij.psi.*; import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.psi.util.MethodSignature; +import com.intellij.psi.util.MethodSignatureUtil; +import gnu.trove.THashMap; import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.lang.psi.api.statements.arguments.GrArgumentList; import org.jetbrains.plugins.groovy.lang.psi.api.statements.arguments.GrNamedArgument; import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod; import org.jetbrains.plugins.groovy.lang.psi.api.types.GrClosureParameter; import org.jetbrains.plugins.groovy.lang.psi.api.types.GrClosureSignature; +import org.jetbrains.plugins.groovy.lang.psi.impl.GrTupleType; import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.TypesUtil; -import org.jetbrains.plugins.groovy.lang.psi.impl.types.GrClosureSignatureImpl; import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; +import java.util.*; /** * @author Maxim.Medvedev @@ -70,7 +72,7 @@ public class GrClosureSignatureUtil { } private static boolean isApplicable(GrClosureSignature signature, PsiType[] args, PsiManager manager, GlobalSearchScope scope) { - GrClosureParameter[] params = signature.getParameters(); + GrClosureParameter[] params = signature.getParameters(); if (args.length > params.length && !signature.isVarargs()) return false; int optional = getOptionalParamCount(signature, false); int notOptional = params.length - optional; @@ -344,4 +346,62 @@ public class GrClosureSignatureUtil { return copy; } } + + + public static List generateAllSignaturesForMethod(GrMethod method, PsiSubstitutor substitutor) { + GrClosureSignature signature = createSignature(method, substitutor); + String name = method.getName(); + GrClosureParameter[] params = signature.getParameters(); + PsiTypeParameter[] typeParameters = method.getTypeParameters(); + + ArrayList newParams = new ArrayList(params.length); + ArrayList opts = new ArrayList(params.length); + ArrayList optInds = new ArrayList(params.length); + + for (int i = 0; i < params.length; i++) { + if (params[i].isOptional()) { + opts.add(params[i]); + optInds.add(i); + } + else { + newParams.add(params[i].getType()); + } + } + + List result = new ArrayList(opts.size() + 1); + result.add(generateSignature(name, newParams, typeParameters, substitutor)); + for (int i = 0; i < opts.size(); i++) { + newParams.add(optInds.get(i), opts.get(i).getType()); + result.add(generateSignature(name, newParams, typeParameters, substitutor)); + } + return result; + } + + public static Map> findMethodSignatures(GrMethod[] methods) { + List> signatures = new ArrayList>(); + for (GrMethod method : methods) { + List current = generateAllSignaturesForMethod(method, PsiSubstitutor.EMPTY); + for (MethodSignature signature : current) { + signatures.add(new Pair(signature, method)); + } + } + + THashMap> map = new THashMap>(); + for (Pair pair : signatures) { + List list = map.get(pair.first); + if (list == null) { + list = new ArrayList(); + map.put(pair.first, list); + } + list.add(pair.second); + } + return map; + } + + private static MethodSignature generateSignature(String name, + List paramTypes, + PsiTypeParameter[] typeParameters, + PsiSubstitutor substitutor) { + return MethodSignatureUtil.createMethodSignature(name, paramTypes.toArray(new PsiType[paramTypes.size()]), typeParameters, substitutor); + } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java index 3d8b97b7594e..7fda1a07e520 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java @@ -64,11 +64,11 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMe import org.jetbrains.plugins.groovy.lang.psi.api.toplevel.imports.GrImportStatement; import org.jetbrains.plugins.groovy.lang.psi.api.types.GrClosureSignature; import org.jetbrains.plugins.groovy.lang.psi.api.types.GrCodeReferenceElement; -import org.jetbrains.plugins.groovy.lang.psi.impl.GrClosureSignatureUtil; import org.jetbrains.plugins.groovy.lang.psi.impl.GrClosureType; import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.TypesUtil; import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GroovyScriptClass; import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.JavaIdentifier; +import org.jetbrains.plugins.groovy.lang.psi.impl.types.GrClosureSignatureUtil; import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil; import org.jetbrains.plugins.groovy.lang.resolve.processors.MethodResolverProcessor; diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java index bd357303a96b..b78fd9708100 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java @@ -238,4 +238,6 @@ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase { public void testOverrideAnnotation() throws Exception {doTest();} public void testClosureCallWithTupleTypeArgument() throws Exception {doTest();} + + public void testMethodDuplicates() throws Exception {doTest();} } \ No newline at end of file diff --git a/plugins/groovy/testdata/highlighting/MethodDuplicates.groovy b/plugins/groovy/testdata/highlighting/MethodDuplicates.groovy new file mode 100644 index 000000000000..039707c2629e --- /dev/null +++ b/plugins/groovy/testdata/highlighting/MethodDuplicates.groovy @@ -0,0 +1,9 @@ +def foo(String s = "a", int i, double y = 4) { + +} + +def foo(int i, double y) {} + +def foo(String s, int i) {} + +def foo(String s, double y){} \ No newline at end of file