From 214599080e56a0f08dbaa60a36dc77ff67bc084a Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Wed, 23 Nov 2022 12:29:17 +0200 Subject: [PATCH] [extract duplicates] IDEA-298940: check top level expression nodes to be equal GitOrigin-RevId: 5f6d408132c2e835434f2b19572e8e1e74fd572d --- .../newImpl/JavaDuplicatesFinder.kt | 25 ++++++++++++------- .../LiteralDuplicates.java | 8 ++++++ .../LiteralDuplicates_after.java | 15 +++++++++++ .../ExtractMethodAndDuplicatesInplaceTest.kt | 4 +++ 4 files changed, 43 insertions(+), 9 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/LiteralDuplicates.java create mode 100644 java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/LiteralDuplicates_after.java diff --git a/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/JavaDuplicatesFinder.kt b/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/JavaDuplicatesFinder.kt index a1b125dd0dbc..c8b9ff313e2f 100644 --- a/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/JavaDuplicatesFinder.kt +++ b/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/JavaDuplicatesFinder.kt @@ -46,9 +46,13 @@ class JavaDuplicatesFinder(pattern: List, private val predefinedChan object : JavaRecursiveElementWalkingVisitor(){ override fun visitExpression(expression: PsiExpression) { if (expression in ignoredElements) return - val duplicate = createDuplicate(childrenOf(patternExpression), childrenOf(expression)) + val duplicate = if (areNodesEquivalent(patternExpression, expression)) { + createDuplicate(listOf(patternExpression), listOf(expression)) + } else { + null + } if (duplicate != null) { - duplicates += duplicate.copy(pattern = listOf(patternExpression), candidate = listOf(expression)) + duplicates += duplicate } else { super.visitExpression(expression) } @@ -88,11 +92,6 @@ class JavaDuplicatesFinder(pattern: List, private val predefinedChan return siblingsOf(element?.firstChild).toList() } - fun createExpressionDuplicate(pattern: PsiExpression, candidate: PsiExpression): Duplicate? { - return createDuplicate(childrenOf(pattern), childrenOf(candidate)) - ?.copy(pattern = listOf(pattern), candidate = listOf(candidate)) - } - fun createDuplicate(pattern: List, candidate: List): Duplicate? { val changedExpressions = ArrayList() if (!traverseAndCollectChanges(pattern, candidate, changedExpressions)) return null @@ -120,13 +119,18 @@ class JavaDuplicatesFinder(pattern: List, private val predefinedChan return duplicate.copy(changedExpressions = changedExpressions) } + /** + * Does recursive equivalence check for [pattern] and [candidate] nodes. + * Puts parametrized expressions into [changedExpressions] list. + * @return false if are [pattern] and [candidate] are not parametrized duplicates. + */ private fun traverseAndCollectChanges(pattern: List, candidate: List, changedExpressions: MutableList): Boolean { if (candidate.size != pattern.size) return false val notEqualElements = pattern.zip(candidate).filterNot { (pattern, candidate) -> pattern !in predefinedChanges && - areEquivalent(pattern, candidate) && + areNodesEquivalent(pattern, candidate) && traverseAndCollectChanges(childrenOf(pattern), childrenOf(candidate), changedExpressions) } if (notEqualElements.any { (pattern, candidate) -> ! canBeReplaced(pattern, candidate) }) return false @@ -134,7 +138,10 @@ class JavaDuplicatesFinder(pattern: List, private val predefinedChan return true } - private fun areEquivalent(pattern: PsiElement, candidate: PsiElement): Boolean { + /** + * Non-recursive equivalence check for [pattern] and [candidate] elements. + */ + private fun areNodesEquivalent(pattern: PsiElement, candidate: PsiElement): Boolean { return when { pattern is PsiTypeElement && candidate is PsiTypeElement -> canBeReplaced(pattern.type, candidate.type) pattern is PsiJavaCodeReferenceElement && candidate is PsiJavaCodeReferenceElement -> diff --git a/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/LiteralDuplicates.java b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/LiteralDuplicates.java new file mode 100644 index 000000000000..ab96bd1dc3fe --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/LiteralDuplicates.java @@ -0,0 +1,8 @@ +public class Test { + + void test() { + String first = "First"; + String second = "First"; + String third = "Third"; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/LiteralDuplicates_after.java b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/LiteralDuplicates_after.java new file mode 100644 index 000000000000..6931e42c6d69 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/LiteralDuplicates_after.java @@ -0,0 +1,15 @@ +import org.jetbrains.annotations.NotNull; + +public class Test { + + void test() { + String first = getFirst(); + String second = getFirst(); + String third = "Third"; + } + + @NotNull + private static String getFirst() { + return "First"; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt index 6f0452112dc3..69a740dec049 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt @@ -238,6 +238,10 @@ class ExtractMethodAndDuplicatesInplaceTest: LightJavaCodeInsightTestCase() { doTest() } + fun testLiteralDuplicates(){ + doTest() + } + fun testFoldedParametersInExactDuplicates(){ val default = DuplicatesMethodExtractor.changeSignatureDefault try {