From 3ab1e8e852aa9eafbd867ebdefef6a04e90f41c0 Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Thu, 2 Jul 2020 15:54:58 +0300 Subject: [PATCH] IDEA-244988: extract method: dont suggest static modifier inside local or inner classes GitOrigin-RevId: 2c496066816e484a45c71cb65e25e217c394e261 --- .../newImpl/ExtractOptionsPipeline.kt | 3 ++- .../extractMethod/newImpl/MethodExtractor.kt | 6 ++--- .../NoStaticForInnerClass.java | 7 ++++++ .../StaticForNestedClass.java | 7 ++++++ .../StaticForNestedClass_after.java | 11 +++++++++ .../extractMethodNew/StaticForOuterClass.java | 7 ++++++ .../StaticForOuterClass_after.java | 11 +++++++++ .../refactoring/ExtractMethodNewTest.java | 23 +++++++++++++++++++ 8 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethodNew/NoStaticForInnerClass.java create mode 100644 java/java-tests/testData/refactoring/extractMethodNew/StaticForNestedClass.java create mode 100644 java/java-tests/testData/refactoring/extractMethodNew/StaticForNestedClass_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethodNew/StaticForOuterClass.java create mode 100644 java/java-tests/testData/refactoring/extractMethodNew/StaticForOuterClass_after.java 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 e6d611187538..312fd7867d72 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 @@ -209,7 +209,8 @@ object ExtractMethodPipeline { } fun withForcedStatic(analyzer: CodeFragmentAnalyzer, extractOptions: ExtractOptions): ExtractOptions? { - val targetClass = PsiTreeUtil.getParentOfType(ExtractMethodHelper.getValidParentOf(extractOptions.elements.first()), PsiClass::class.java)!! + 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 fieldInputParameters = diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/MethodExtractor.kt b/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/MethodExtractor.kt index 8304626980c5..b4d9806cee45 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/MethodExtractor.kt +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/newImpl/MethodExtractor.kt @@ -126,7 +126,8 @@ class MethodExtractor { val candidates = ExtractMethodPipeline.findTargetCandidates(analyzer, options) val defaultTargetClass = candidates.firstOrNull { it !is PsiAnonymousClass } ?: candidates.first() - options = ExtractMethodPipeline.withTargetClass(analyzer, options, defaultTargetClass) ?: throw ExtractException("Fail", elements.first()) + options = ExtractMethodPipeline.withTargetClass(analyzer, options, targetClass ?: defaultTargetClass) + ?: throw ExtractException("Fail", elements.first()) options = options.copy(methodName = "newMethod") if (isConstructor != options.isConstructor){ options = ExtractMethodPipeline.asConstructor(analyzer, options) ?: throw ExtractException("Fail", elements.first()) @@ -142,9 +143,6 @@ class MethodExtractor { if (returnType != null) { options = options.copy(dataOutput = options.dataOutput.withType(returnType)) } - if (targetClass != null) { - options = ExtractMethodPipeline.withTargetClass(analyzer, options, targetClass) ?: options - } if (disabledParameters.isNotEmpty()) { options = options.copy( disabledParameters = options.inputParameters.filterIndexed { index, _ -> index in disabledParameters }, diff --git a/java/java-tests/testData/refactoring/extractMethodNew/NoStaticForInnerClass.java b/java/java-tests/testData/refactoring/extractMethodNew/NoStaticForInnerClass.java new file mode 100644 index 000000000000..686726b198fc --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodNew/NoStaticForInnerClass.java @@ -0,0 +1,7 @@ +class Outer { + class Inner { + { + int i = 0; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodNew/StaticForNestedClass.java b/java/java-tests/testData/refactoring/extractMethodNew/StaticForNestedClass.java new file mode 100644 index 000000000000..3a93547d9923 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodNew/StaticForNestedClass.java @@ -0,0 +1,7 @@ +class Outer { + static class Nested { + { + int i = 0; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodNew/StaticForNestedClass_after.java b/java/java-tests/testData/refactoring/extractMethodNew/StaticForNestedClass_after.java new file mode 100644 index 000000000000..f1ddb3e9dcb7 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodNew/StaticForNestedClass_after.java @@ -0,0 +1,11 @@ +class Outer { + static class Nested { + { + newMethod(); + } + + private static void newMethod() { + int i = 0; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodNew/StaticForOuterClass.java b/java/java-tests/testData/refactoring/extractMethodNew/StaticForOuterClass.java new file mode 100644 index 000000000000..686726b198fc --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodNew/StaticForOuterClass.java @@ -0,0 +1,7 @@ +class Outer { + class Inner { + { + int i = 0; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodNew/StaticForOuterClass_after.java b/java/java-tests/testData/refactoring/extractMethodNew/StaticForOuterClass_after.java new file mode 100644 index 000000000000..73d10ed11529 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodNew/StaticForOuterClass_after.java @@ -0,0 +1,11 @@ +class Outer { + class Inner { + { + newMethod(); + } + } + + private static void newMethod() { + int i = 0; + } +} \ 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 6600fc62d7c2..f6b53994792f 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodNewTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodNewTest.java @@ -1414,6 +1414,29 @@ public class ExtractMethodNewTest extends LightJavaCodeInsightTestCase { checkResultByFile(BASE_PATH + getTestName(false) + "_after.java"); } + public void testNoStaticForInnerClass() { + try { + configureByFile(BASE_PATH + getTestName(false) + ".java"); + performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, true, null, null, null); + fail("Static modifier is forbidden inside inner classes"); + } catch (PrepareFailedException e){ + } + } + + public void testStaticForNestedClass() throws Exception { + configureByFile(BASE_PATH + getTestName(false) + ".java"); + performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, true, null, null, null); + checkResultByFile(BASE_PATH + getTestName(false) + "_after.java"); + } + + public void testStaticForOuterClass() throws Exception { + configureByFile(BASE_PATH + getTestName(false) + ".java"); + final int caret = getEditor().getSelectionModel().getLeadSelectionOffset(); + final PsiClass outerClass = PsiTreeUtil.getParentOfType(getFile().findElementAt(caret), PsiClass.class).getContainingClass(); + performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, true, null, outerClass, null); + checkResultByFile(BASE_PATH + getTestName(false) + "_after.java"); + } + public void testDontMakeParametersFinalDueToUsagesInsideAnonymous() throws Exception { doTest(); }