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 4678af83243d..08d6c3f2a2de 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 @@ -13,13 +13,13 @@ import com.intellij.psi.controlFlow.ControlFlow import com.intellij.psi.controlFlow.ControlFlowUtil.DEFAULT_EXIT_STATEMENTS_CLASSES import com.intellij.psi.util.PsiTreeUtil import com.intellij.psi.util.PsiUtil -import com.intellij.refactoring.util.classMembers.ClassMemberReferencesVisitor +import com.intellij.refactoring.util.classMembers.ElementNeedsThis import com.siyeh.ig.psiutils.VariableAccessUtils import it.unimi.dsi.fastutil.ints.IntArrayList data class ExitDescription(val statements: List, val numberOfExits: Int, val hasSpecialExits: Boolean) data class ExternalReference(val variable: PsiVariable, val references: List) -data class FieldUsage(val field: PsiField, val classMemberReference: PsiReferenceExpression, val isWrite: Boolean) +data class LocalUsage(val member: PsiMember, val reference: PsiReferenceExpression) class CodeFragmentAnalyzer(val elements: List) { @@ -117,18 +117,19 @@ class CodeFragmentAnalyzer(val elements: List) { return declaredVariables.intersect(externallyWrittenVariables).toList() } - fun findLocalFieldUsages(targetClass: PsiClass, elements: List): List { - val usedFields = ArrayList() - val visitor = object : ClassMemberReferencesVisitor(targetClass) { + fun findLocalUsages(targetClass: PsiClass, elements: List): List { + val usedFields = ArrayList() + val visitor: ElementNeedsThis = object : ElementNeedsThis(targetClass) { override fun visitClassMemberReferenceElement(classMember: PsiMember, classMemberReference: PsiJavaCodeReferenceElement) { - val expression = PsiTreeUtil.getParentOfType(classMemberReference, PsiExpression::class.java, false) - if (classMember is PsiField && expression != null && classMemberReference is PsiReferenceExpression) { - usedFields += FieldUsage(classMember, classMemberReference, PsiUtil.isAccessedForWriting(expression)) + val expression = PsiTreeUtil.getParentOfType(classMemberReference, PsiReferenceExpression::class.java, false) + if (expression != null && !classMember.hasModifierProperty(PsiModifier.STATIC)) { + usedFields += LocalUsage(classMember, expression) } + super.visitClassMemberReferenceElement(classMember, classMemberReference) } } elements.forEach { it.accept(visitor) } - return usedFields.distinct().filterNot { usage -> usage.field.modifierList?.hasModifierProperty(PsiModifier.STATIC) == true } + return usedFields.distinct() } 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 87963d6cb530..534e9d5ab22b 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 @@ -8,7 +8,6 @@ import com.intellij.java.refactoring.JavaRefactoringBundle import com.intellij.openapi.util.TextRange import com.intellij.psi.* import com.intellij.psi.GenericsUtil -import com.intellij.psi.controlFlow.ControlFlowUtil import com.intellij.psi.search.GlobalSearchScope import com.intellij.psi.search.searches.ReferencesSearch import com.intellij.psi.util.PsiTreeUtil @@ -18,7 +17,6 @@ import com.intellij.psi.util.TypeConversionUtil import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.findUsedTypeParameters import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.getExpressionType import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.guessName -import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.hasExplicitModifier import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.haveReferenceToScope import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.inputParameterOf import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodHelper.normalizedAnchor @@ -90,15 +88,16 @@ fun findExtractOptions(elements: List): ExtractOptions { val targetClass = PsiTreeUtil.getParentOfType(ExtractMethodHelper.getValidParentOf(elements.first()), PsiClass::class.java)!! - 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() + val localWriteViolations = analyzer.findLocalUsages(targetClass, elements) + .filter { localUsage -> PsiUtil.isAccessedForWriting(localUsage.reference) && localUsage.member.hasModifierProperty(PsiModifier.FINAL) } + + val fieldViolations = localWriteViolations.mapNotNull { it.member as? PsiField }.distinct() + val field = fieldViolations.singleOrNull() extractOptions = when { - finalFields.isEmpty() -> extractOptions + fieldViolations.isEmpty() -> extractOptions field != null && extractOptions.dataOutput is EmptyOutput -> extractOptions.copy(dataOutput = VariableOutput(field.type, field, false), requiredVariablesInside = listOf(field)) - else -> throw ExtractException(JavaRefactoringBundle.message("extract.method.error.many.finals"), finalFieldsWrites.map { it.classMemberReference }) + else -> throw ExtractException(JavaRefactoringBundle.message("extract.method.error.many.finals"), localWriteViolations.map { it.reference }) } checkLocalClass(extractOptions) 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 312fd7867d72..fced0a0c46b8 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 @@ -211,12 +211,17 @@ object ExtractMethodPipeline { fun withForcedStatic(analyzer: CodeFragmentAnalyzer, extractOptions: ExtractOptions): ExtractOptions? { val targetClass = PsiTreeUtil.getParentOfType(extractOptions.anchor, PsiClass::class.java)!! if (PsiUtil.isLocalOrAnonymousClass(targetClass) || PsiUtil.isInnerClass(targetClass)) return null - val fieldUsages = analyzer.findLocalFieldUsages(targetClass, extractOptions.elements) - if (fieldUsages.any { it.isWrite }) return null + val localUsages = analyzer.findLocalUsages(targetClass, extractOptions.elements) + val (violatedUsages, fieldUsages) = localUsages + .partition { localUsage -> PsiUtil.isAccessedForWriting(localUsage.reference) || localUsage.member !is PsiField } + + if (violatedUsages.isNotEmpty()) return null + val fieldInputParameters = - fieldUsages.groupBy { it.field }.entries.map { (field, fieldUsages) -> + fieldUsages.groupBy { it.member }.entries.map { (field, fieldUsages) -> + field as PsiField InputParameter( - references = fieldUsages.map { it.classMemberReference }, + references = fieldUsages.map { it.reference }, name = field.name, type = field.type ) @@ -232,8 +237,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.findLocalFieldUsages(holderClass, outStatements) - .any { fieldUsage -> fieldUsage.isWrite && fieldUsage.field.hasExplicitModifier(PsiModifier.FINAL) } + val hasOuterFinalFieldAssignments = analyzer.findLocalUsages(holderClass, outStatements) + .any { localUsage -> localUsage.member.hasModifierProperty(PsiModifier.FINAL) && PsiUtil.isAccessedForWriting(localUsage.reference) } return method.isConstructor && startsOnBegin && !hasOuterFinalFieldAssignments && analyzer.findOutputVariables().isEmpty() } diff --git a/java/java-tests/testData/refactoring/extractMethodNew/CantMakeStatic.java b/java/java-tests/testData/refactoring/extractMethodNew/CantMakeStatic.java new file mode 100644 index 000000000000..ce69826d16a1 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodNew/CantMakeStatic.java @@ -0,0 +1,8 @@ +class Foo { + static int i; + void m() { + foo(i); + } + + void foo(int p) {} +} \ 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 7e21c852b75a..c221399723a6 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodNewTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodNewTest.java @@ -1247,7 +1247,16 @@ public class ExtractMethodNewTest extends LightJavaCodeInsightTestCase { public void testCantPassFieldAsParameter() { try { doTestPassFieldsAsParams(); - fail("Field was modified inside. Make static should be disabled"); + fail("Field was modified inside. Make static should be disabled."); + } + catch (PrepareFailedException ignore) { + } + } + + public void testCantMakeStatic() { + try { + doTestPassFieldsAsParams(); + fail("Local method is used. Make static should be disabled."); } catch (PrepareFailedException ignore) { }