From 0ea2cc86c870a95c3888955ef32a9dd8943ecd71 Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Tue, 30 Jun 2020 14:27:44 +0300 Subject: [PATCH] IDEA-242718: dont pass static fields as parameters GitOrigin-RevId: 2b00ccc0466cc643ca6bd9c40fcc974b82d2fed8 --- .../extractMethod/newImpl/CodeFragmentAnalyzer.kt | 4 ++-- .../extractMethod/newImpl/ExtractMethodAnalyzer.kt | 4 ++-- .../extractMethod/newImpl/ExtractOptionsPipeline.kt | 8 +++----- .../extractMethodNew/NotPassedStaticField.java | 8 ++++++++ .../extractMethodNew/NotPassedStaticField_after.java | 12 ++++++++++++ .../java/refactoring/ExtractMethodNewTest.java | 4 ++++ 6 files changed, 31 insertions(+), 9 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethodNew/NotPassedStaticField.java create mode 100644 java/java-tests/testData/refactoring/extractMethodNew/NotPassedStaticField_after.java diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/CodeFragmentAnalyzer.kt b/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/CodeFragmentAnalyzer.kt index 81a3f8444b62..574fe144548a 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/CodeFragmentAnalyzer.kt +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/CodeFragmentAnalyzer.kt @@ -117,7 +117,7 @@ class CodeFragmentAnalyzer(val elements: List) { return declaredVariables.intersect(externallyWrittenVariables).toList() } - fun findFieldUsages(targetClass: PsiClass, elements: List): List { + fun findLocalFieldUsages(targetClass: PsiClass, elements: List): List { val usedFields = ArrayList() val visitor = object : ClassMemberReferencesVisitor(targetClass) { override fun visitClassMemberReferenceElement(classMember: PsiMember, classMemberReference: PsiJavaCodeReferenceElement) { @@ -128,7 +128,7 @@ class CodeFragmentAnalyzer(val elements: List) { } } elements.forEach { it.accept(visitor) } - return usedFields.distinct() + return usedFields.distinct().filterNot { usage -> usage.field.modifierList?.hasExplicitModifier(PsiModifier.STATIC) == true } } private fun lastGotoPointFrom(instructionOffset: Int): Int { diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractMethodAnalyzer.kt b/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractMethodAnalyzer.kt index 0e886fb6f54e..64fe8fa512c7 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractMethodAnalyzer.kt +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractMethodAnalyzer.kt @@ -90,8 +90,8 @@ fun findExtractOptions(elements: List): ExtractOptions { val targetClass = PsiTreeUtil.getParentOfType(ExtractMethodHelper.getValidParentOf(elements.first()), PsiClass::class.java)!! - val fieldUsages = analyzer.findFieldUsages(targetClass, elements) - val finalFieldsWrites = fieldUsages.filter { it.isWrite && it.field.hasExplicitModifier(PsiModifier.FINAL) } + val fieldUsages = analyzer.findLocalFieldUsages(targetClass, elements) + val finalFieldsWrites = fieldUsages.filter { fieldsUsage -> fieldsUsage.isWrite && fieldsUsage.field.hasExplicitModifier(PsiModifier.FINAL) } val finalFields = finalFieldsWrites.map { it.field }.distinct() val field = finalFields.singleOrNull() extractOptions = when { diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractOptionsPipeline.kt b/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractOptionsPipeline.kt index b9097cd18371..e6d611187538 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractOptionsPipeline.kt +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractOptionsPipeline.kt @@ -13,7 +13,6 @@ import com.intellij.psi.* import com.intellij.psi.search.PsiElementProcessor import com.intellij.psi.util.PsiTreeUtil import com.intellij.psi.util.PsiUtil -import com.intellij.refactoring.extractMethod.PrepareFailedException import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.findUsedTypeParameters import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.hasExplicitModifier import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.inputParameterOf @@ -211,7 +210,7 @@ object ExtractMethodPipeline { fun withForcedStatic(analyzer: CodeFragmentAnalyzer, extractOptions: ExtractOptions): ExtractOptions? { val targetClass = PsiTreeUtil.getParentOfType(ExtractMethodHelper.getValidParentOf(extractOptions.elements.first()), PsiClass::class.java)!! - val fieldUsages = analyzer.findFieldUsages(targetClass, extractOptions.elements) + val fieldUsages = analyzer.findLocalFieldUsages(targetClass, extractOptions.elements) if (fieldUsages.any { it.isWrite }) return null val fieldInputParameters = fieldUsages.groupBy { it.field }.entries.map { (field, fieldUsages) -> @@ -232,9 +231,8 @@ object ExtractMethodPipeline { val firstStatement = method.body?.statements?.firstOrNull() ?: return false val startsOnBegin = firstStatement.textRange in TextRange(elements.first().textRange.startOffset, elements.last().textRange.endOffset) val outStatements = method.body?.statements.orEmpty().dropWhile { it.textRange.endOffset <= elements.last().textRange.endOffset } - val hasOuterFinalFieldAssignments = analyzer - .findFieldUsages(holderClass, outStatements) - .any { it.isWrite && it.field.hasExplicitModifier(PsiModifier.FINAL) } + val hasOuterFinalFieldAssignments = analyzer.findLocalFieldUsages(holderClass, outStatements) + .any { fieldUsage -> fieldUsage.isWrite && fieldUsage.field.hasExplicitModifier(PsiModifier.FINAL) } return method.isConstructor && startsOnBegin && !hasOuterFinalFieldAssignments && analyzer.findOutputVariables().isEmpty() } diff --git a/java/java-tests/testData/refactoring/extractMethodNew/NotPassedStaticField.java b/java/java-tests/testData/refactoring/extractMethodNew/NotPassedStaticField.java new file mode 100644 index 000000000000..9c2dfb343c20 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodNew/NotPassedStaticField.java @@ -0,0 +1,8 @@ +class Test { + int local = 42; + static int global = 42; + + void test(){ + System.out.println(local + global); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodNew/NotPassedStaticField_after.java b/java/java-tests/testData/refactoring/extractMethodNew/NotPassedStaticField_after.java new file mode 100644 index 000000000000..263d27ec5b7e --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodNew/NotPassedStaticField_after.java @@ -0,0 +1,12 @@ +class Test { + int local = 42; + static int global = 42; + + void test(){ + newMethod(local); + } + + private static void newMethod(int local) { + System.out.println(local + global); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodNewTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodNewTest.java index 56aa138e3130..9897b27d9c26 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodNewTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodNewTest.java @@ -253,6 +253,10 @@ public class ExtractMethodNewTest extends LightJavaCodeInsightTestCase { doTest(); } + public void testNotPassedStaticField() throws Exception { + doTestPassFieldsAsParams(); + } + public void testExtractAssignmentExpression() throws Exception { try { doTest();