From cbab61f15bd1b452a59325b03fc4f30edc6fdace Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Mon, 21 Nov 2016 15:29:55 +0100 Subject: [PATCH] fix annotation matching in duplicates finder --- .../util/duplicates/DuplicatesFinder.java | 21 +--- .../intellij/codeInsight/AnnotationUtil.java | 96 +++++++++++++++++++ .../extractMethod/DifferentAnnotations.java | 13 +++ .../DifferentAnnotations_after.java | 17 ++++ .../extractMethod/SameAnnotations.java | 19 ++++ .../extractMethod/SameAnnotations_after.java | 21 ++++ .../refactoring/ExtractMethodTest.java | 10 +- 7 files changed, 177 insertions(+), 20 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/DifferentAnnotations.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/DifferentAnnotations_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SameAnnotations.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SameAnnotations_after.java diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java index ded7d252ae48..26b37eb61212 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java @@ -15,6 +15,7 @@ */ package com.intellij.refactoring.util.duplicates; +import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInsight.PsiEquivalenceUtil; import com.intellij.lang.ASTNode; import com.intellij.openapi.diagnostic.Logger; @@ -30,7 +31,6 @@ import com.intellij.refactoring.extractMethod.InputVariables; import com.intellij.refactoring.util.RefactoringChangeUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; -import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.IntArrayList; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -556,24 +556,7 @@ public class DuplicatesFinder { } } } - final List annotations1 = ContainerUtil.newArrayList(modifierList1.getAnnotations()); - final List annotations2 = ContainerUtil.newArrayList(modifierList2.getAnnotations()); - annotations1.removeIf(a -> CommonClassNames.JAVA_LANG_OVERRIDE.equals(a.getQualifiedName())); - annotations2.removeIf(a -> CommonClassNames.JAVA_LANG_OVERRIDE.equals(a.getQualifiedName())); - if (annotations1.size() != annotations2.size()) { - return false; - } - for (final Iterator iterator = annotations1.iterator(); iterator.hasNext(); ) { - final PsiAnnotation annotation1 = iterator.next(); - for (final Iterator iterator2 = annotations2.iterator(); iterator2.hasNext(); ) { - final PsiAnnotation annotation2 = iterator2.next(); - if (PsiEquivalenceUtil.areElementsEquivalent(annotation1, annotation2)) { - iterator.remove(); - iterator2.remove(); - } - } - } - return true; + return AnnotationUtil.equal(modifierList1.getAnnotations(), modifierList2.getAnnotations()); } private static boolean checkParameterModification(PsiExpression expression, diff --git a/java/java-psi-api/src/com/intellij/codeInsight/AnnotationUtil.java b/java/java-psi-api/src/com/intellij/codeInsight/AnnotationUtil.java index 5a2e0f4623d7..70242bbf31fb 100644 --- a/java/java-psi-api/src/com/intellij/codeInsight/AnnotationUtil.java +++ b/java/java-psi-api/src/com/intellij/codeInsight/AnnotationUtil.java @@ -25,6 +25,7 @@ import com.intellij.util.Processor; import com.intellij.util.Processors; import com.intellij.util.containers.ConcurrentFactoryMap; import com.intellij.util.containers.ContainerUtil; +import gnu.trove.THashMap; import gnu.trove.THashSet; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NonNls; @@ -573,4 +574,99 @@ public class AnnotationUtil { } return null; } + + public static boolean equal(@Nullable PsiAnnotation a, @Nullable PsiAnnotation b) { + if (a == null) { + return b == null; + } + else if (b == null) { + return false; + } + final String name = a.getQualifiedName(); + if (name == null || !name.equals(b.getQualifiedName())) { + return false; + } + final Map valueMap1 = new THashMap(2); + final Map valueMap2 = new THashMap(2); + if (!fillValueMap(a.getParameterList(), valueMap1) || !fillValueMap(b.getParameterList(), valueMap2) || + valueMap1.size() != valueMap2.size()) { + return false; + } + for (Map.Entry entry : valueMap1.entrySet()) { + if (!equal(entry.getValue(), valueMap2.get(entry.getKey()))) { + return false; + } + } + return true; + } + + private static boolean fillValueMap(PsiAnnotationParameterList parameterList, Map valueMap) { + final PsiNameValuePair[] attributes1 = parameterList.getAttributes(); + for (PsiNameValuePair attribute : attributes1) { + final PsiReference reference = attribute.getReference(); + if (reference == null) { + return false; + } + final PsiElement target = reference.resolve(); + if (!(target instanceof PsiAnnotationMethod)) { + return false; + } + final PsiAnnotationMethod annotationMethod = (PsiAnnotationMethod)target; + final PsiAnnotationMemberValue defaultValue = annotationMethod.getDefaultValue(); + final PsiAnnotationMemberValue value = attribute.getValue(); + if (equal(value, defaultValue)) { + continue; + } + final String name1 = attribute.getName(); + valueMap.put(name1 == null ? PsiAnnotation.DEFAULT_REFERENCED_METHOD_NAME : name1, value); + } + return true; + } + + public static boolean equal(PsiAnnotationMemberValue value1, PsiAnnotationMemberValue value2) { + if (value1 instanceof PsiArrayInitializerMemberValue && value2 instanceof PsiArrayInitializerMemberValue) { + final PsiAnnotationMemberValue[] initializers1 = ((PsiArrayInitializerMemberValue)value1).getInitializers(); + final PsiAnnotationMemberValue[] initializers2 = ((PsiArrayInitializerMemberValue)value2).getInitializers(); + if (initializers1.length != initializers2.length) { + return false; + } + for (int i = 0; i < initializers1.length; i++) { + if (!equal(initializers1[i], initializers2[i])) { + return false; + } + } + return true; + } + if (value1 != null && value2 != null) { + final PsiConstantEvaluationHelper constantEvaluationHelper = + JavaPsiFacade.getInstance(value1.getProject()).getConstantEvaluationHelper(); + final Object const1 = constantEvaluationHelper.computeConstantExpression(value1); + final Object const2 = constantEvaluationHelper.computeConstantExpression(value2); + return const1 != null && const1.equals(const2); + } + return false; + } + + public static boolean equal(PsiAnnotation[] annotations1, PsiAnnotation[] annotations2) { + final Map map1 = buildAnnotationMap(annotations1); + final Map map2 = buildAnnotationMap(annotations2); + if (map1.size() != map2.size()) { + return false; + } + for (Map.Entry entry : map1.entrySet()) { + if (!equal(entry.getValue(), map2.get(entry.getKey()))) { + return false; + } + } + return true; + } + + private static Map buildAnnotationMap(PsiAnnotation[] annotations) { + final Map map = new HashMap(); + for (PsiAnnotation annotation : annotations) { + map.put(annotation.getQualifiedName(), annotation); + } + map.remove(CommonClassNames.JAVA_LANG_OVERRIDE); + return map; + } } diff --git a/java/java-tests/testData/refactoring/extractMethod/DifferentAnnotations.java b/java/java-tests/testData/refactoring/extractMethod/DifferentAnnotations.java new file mode 100644 index 000000000000..31b93b3c3667 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/DifferentAnnotations.java @@ -0,0 +1,13 @@ +class C { + { + @A int i = 0; + System.out.println(i); + } + + void f() { + @B int j = 0; + System.out.println(j); + } +} +@interface A {} +@interface B {} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/DifferentAnnotations_after.java b/java/java-tests/testData/refactoring/extractMethod/DifferentAnnotations_after.java new file mode 100644 index 000000000000..41510f5c31bd --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/DifferentAnnotations_after.java @@ -0,0 +1,17 @@ +class C { + { + @A int i = 0; + System.out.println(i); + } + + void f() { + newMethod(); + } + + private void newMethod() { + @B int j = 0; + System.out.println(j); + } +} +@interface A {} +@interface B {} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/SameAnnotations.java b/java/java-tests/testData/refactoring/extractMethod/SameAnnotations.java new file mode 100644 index 000000000000..df38a0616816 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SameAnnotations.java @@ -0,0 +1,19 @@ +class C { + { + @A(value="asdf") int i = 0; + System.out.println(i); + } + + void f() { + @A int j = 0; + System.out.println(j); + } + + void g() { + @A("asdf") int k = 0; + System.out.println(k); + } +} +@interface A { + String value() default "asdf"; +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/SameAnnotations_after.java b/java/java-tests/testData/refactoring/extractMethod/SameAnnotations_after.java new file mode 100644 index 000000000000..adf49fceec02 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SameAnnotations_after.java @@ -0,0 +1,21 @@ +class C { + { + newMethod(); + } + + void f() { + newMethod(); + } + + private void newMethod() { + @A int j = 0; + System.out.println(j); + } + + void g() { + newMethod(); + } +} +@interface A { + String value() default "asdf"; +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java b/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java index 22a97f955182..12fb997047ad 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java @@ -864,7 +864,15 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doTest(); } - public void testLocalVariableAnnotationsOrder() throws Exception { + public void testLocalVariableAnnotationsOrder() throws Exception { + doTest(); + } + + public void testDifferentAnnotations() throws Exception { + doTest(); + } + + public void testSameAnnotations() throws Exception { doTest(); }