[extract method] IDEA-313673: don't suggest to make method static if there is a non-static class usage

GitOrigin-RevId: 5b2c6e7a16b730c1fc563b295509e1439b03fc97
This commit is contained in:
Alexandr Suhinin
2023-03-01 18:45:36 +02:00
committed by intellij-monorepo-bot
parent 7a23dd52ba
commit 7a80c0d0ed
7 changed files with 101 additions and 14 deletions

View File

@@ -124,7 +124,7 @@ class CodeFragmentAnalyzer(val elements: List<PsiElement>) {
val usedFields = ArrayList<MemberUsage>()
val visitor = object : ClassMemberReferencesVisitor(targetClass) {
override fun visitClassMemberReferenceElement(classMember: PsiMember, classMemberReference: PsiJavaCodeReferenceElement) {
val expression = PsiTreeUtil.getParentOfType(classMemberReference, PsiReferenceExpression::class.java, false)
val expression = PsiTreeUtil.getParentOfType(classMemberReference, PsiExpression::class.java, false)
if (expression != null && !classMember.hasModifierProperty(PsiModifier.STATIC)) {
usedFields += MemberUsage(classMember, expression)
}
@@ -164,19 +164,15 @@ class CodeFragmentAnalyzer(val elements: List<PsiElement>) {
private fun findDefaultExits(): List<Int> {
val lastInstruction = flow.instructions[flowRange.last - 1]
if (isInstructionReachable(flowRange.last - 1)) {
val defaultExits = when (lastInstruction) {
is ThrowToInstruction -> emptyList()
is GoToInstruction -> listOf(lastInstruction.offset)
is ConditionalThrowToInstruction -> listOf(flowRange.last)
is BranchingInstruction -> listOf(lastInstruction.offset, flowRange.last)
else -> listOf(flowRange.last)
}
return defaultExits.filterNot { it in flowRange.first until flowRange.last }
}
else {
return emptyList()
if (!isInstructionReachable(flowRange.last - 1)) return emptyList()
val defaultExits = when (lastInstruction) {
is ThrowToInstruction -> emptyList()
is GoToInstruction -> listOf(lastInstruction.offset)
is ConditionalThrowToInstruction -> listOf(flowRange.last)
is BranchingInstruction -> listOf(lastInstruction.offset, flowRange.last)
else -> listOf(flowRange.last)
}
return defaultExits.filterNot { it in flowRange.first until flowRange.last }
}
private fun findExitPoints(): List<Int> {

View File

@@ -31,6 +31,7 @@ import com.intellij.refactoring.extractMethod.newImpl.structures.ExtractOptions
import com.intellij.refactoring.extractMethod.newImpl.structures.InputParameter
import com.intellij.refactoring.util.RefactoringUtil
import com.intellij.refactoring.util.VariableData
import com.siyeh.ig.psiutils.ClassUtils
import java.util.concurrent.CompletableFuture
object ExtractMethodPipeline {
@@ -223,7 +224,7 @@ object ExtractMethodPipeline {
val targetClass = PsiTreeUtil.getParentOfType(extractOptions.anchor, PsiClass::class.java)!!
if (PsiUtil.isLocalOrAnonymousClass(targetClass) || PsiUtil.isInnerClass(targetClass)) return null
val memberUsages = analyzer.findInstanceMemberUsages(targetClass, extractOptions.elements)
if (memberUsages.any { usage -> PsiUtil.isAccessedForWriting(usage.reference)}) return null
if (memberUsages.any(::isNotExtractableUsage)) return null
val addedParameters = memberUsages.groupBy(MemberUsage::member).entries
.map { (member: PsiMember, usages: List<MemberUsage>) ->
createInputParameter(member, usages.map(MemberUsage::reference)) ?: return null
@@ -231,6 +232,10 @@ object ExtractMethodPipeline {
return extractOptions.copy(inputParameters = extractOptions.inputParameters + addedParameters, isStatic = true)
}
private fun isNotExtractableUsage(usage: MemberUsage): Boolean {
return PsiUtil.isAccessedForWriting(usage.reference) || (usage.member is PsiClass && ClassUtils.isNonStaticClass(usage.member))
}
private fun createInputParameter(member: PsiMember, usages: List<PsiExpression>): InputParameter? {
val name = member.name ?: return null
val type = when (member) {

View File

@@ -0,0 +1,14 @@
class X {
void test() {
<selection>Runnable r = new Runnable() {
@Override
public void run() {
System.out.println(new Y());
}
};</selection>
r.run();
}
class Y {
}
}

View File

@@ -0,0 +1,22 @@
import org.jetbrains.annotations.NotNull;
class X {
void test() {
Runnable r = getRunnable();
r.run();
}
@NotNull
private Runnable getRunnable() {
Runnable r = new Runnable() {
@Override
public void run() {
System.out.println(new Y());
}
};
return r;
}
class Y {
}
}

View File

@@ -0,0 +1,14 @@
class X {
void test() {
<selection>Runnable r = new Runnable() {
@Override
public void run() {
System.out.println(new Y());
}
};</selection>
r.run();
}
static class Y {
}
}

View File

@@ -0,0 +1,22 @@
import org.jetbrains.annotations.NotNull;
class X {
void test() {
Runnable r = getRunnable();
r.run();
}
@NotNull
private static Runnable getRunnable() {
Runnable r = new Runnable() {
@Override
public void run() {
System.out.println(new Y());
}
};
return r;
}
static class Y {
}
}

View File

@@ -339,6 +339,20 @@ class ExtractMethodAndDuplicatesInplaceTest: LightJavaCodeInsightTestCase() {
doTest()
}
fun testMakeStaticFailsWithClassUsage(){
runAndRevertSettings {
JavaRefactoringSettings.getInstance().EXTRACT_STATIC_METHOD_AND_PASS_FIELDS = true
doTest()
}
}
fun testMakeStaticWithClassUsage(){
runAndRevertSettings {
JavaRefactoringSettings.getInstance().EXTRACT_STATIC_METHOD_AND_PASS_FIELDS = true
doTest()
}
}
fun testIntroduceObjectConflictInsideNestedClass(){
doTest {
renameTemplate("Result")