MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

IDEA-244986: extract method: dont suggest static modifier if non-static method is used

Browse Source 
GitOrigin-RevId: 7e15c57a00cdc3f51e287a7d217e286b350dbd80
This commit is contained in:
Alexandr Suhinin
2020-07-06 08:49:57 +00:00
committed by intellij-monorepo-bot
parent 9a875c81c3
commit 78e71d1f0b
5 changed files with 46 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/CodeFragmentAnalyzer.kt
+10 -9
View File
@@ -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<PsiStatement>, val numberOfExits: Int, val hasSpecialExits: Boolean)
data class ExternalReference(val variable: PsiVariable, val references: List<PsiReferenceExpression>)
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<PsiElement>) {
@@ -117,18 +117,19 @@ class CodeFragmentAnalyzer(val elements: List<PsiElement>) {
return declaredVariables.intersect(externallyWrittenVariables).toList()
}
fun findLocalFieldUsages(targetClass: PsiClass, elements: List<PsiElement>): List<FieldUsage> {
val usedFields = ArrayList<FieldUsage>()
val visitor = object : ClassMemberReferencesVisitor(targetClass) {
fun findLocalUsages(targetClass: PsiClass, elements: List<PsiElement>): List<LocalUsage> {
val usedFields = ArrayList<LocalUsage>()
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 {
java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractMethodAnalyzer.kt
+7 -8
View File
@@ -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<PsiElement>): 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)
java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/ExtractOptionsPipeline.kt
+11 -6
View File
@@ -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()
}
java/java-tests/testData/refactoring/extractMethodNew/CantMakeStatic.java
+8
View File
@@ -0,0 +1,8 @@
class Foo {
static int i;
void m() {
<selection>foo(i);</selection>
}
void foo(int p) {}
}
java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodNewTest.java
+10 -1
View File
@@ -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) {
}