From 5568197815cefc16decf2422462cb1509fa9ca88 Mon Sep 17 00:00:00 2001 From: Mikhail Pyltsin Date: Thu, 4 Jan 2024 14:40:28 +0100 Subject: [PATCH] [java-inspections] IDEA-342216 Fix place for call inspection ExtractMethodRecommenderInspection GitOrigin-RevId: 507f2d2fabb98a46e7a519a9edf904182fe5b3fb --- .../ExtractMethodRecommenderInspection.java | 18 +++++- .../extractMethodRecommender/CallExtract.java | 55 ++++++++++++++++ .../CallExtractFirstNotDeclaration.java | 24 +++++++ .../afterCallExtract.java | 63 +++++++++++++++++++ .../afterCallExtractFirstNotDeclaration.java | 32 ++++++++++ ...xtractMethodRecommenderInspectionTest.java | 38 +++++++++-- 6 files changed, 223 insertions(+), 7 deletions(-) create mode 100644 java/java-tests/testData/inspection/extractMethodRecommender/CallExtract.java create mode 100644 java/java-tests/testData/inspection/extractMethodRecommender/CallExtractFirstNotDeclaration.java create mode 100644 java/java-tests/testData/inspection/extractMethodRecommender/afterCallExtract.java create mode 100644 java/java-tests/testData/inspection/extractMethodRecommender/afterCallExtractFirstNotDeclaration.java diff --git a/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/ExtractMethodRecommenderInspection.java b/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/ExtractMethodRecommenderInspection.java index 88e105ade6ec..6bd3f22200b5 100644 --- a/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/ExtractMethodRecommenderInspection.java +++ b/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/ExtractMethodRecommenderInspection.java @@ -110,7 +110,8 @@ public final class ExtractMethodRecommenderInspection extends AbstractBaseJavaLo } } List fixes = new ArrayList<>(); - fixes.add(new ExtractMethodFix(from, count, output, inputVariables)); + ExtractMethodFix extractFix = new ExtractMethodFix(from, count, output, inputVariables); + fixes.add(extractFix); if (inputVariables.size() > 1) { fixes.add(LocalQuickFix.from(new UpdateInspectionOptionFix( ExtractMethodRecommenderInspection.this, "maxParameters", @@ -128,9 +129,10 @@ public final class ExtractMethodRecommenderInspection extends AbstractBaseJavaLo if (firstLineBreak > -1) { textRange = TextRange.from(textRange.getStartOffset(), firstLineBreak); TextRange firstStatementRange = statements[from].getTextRangeInParent(); - if (firstStatementRange.getStartOffset() == textRange.getStartOffset() && + if (firstStatementRange.getStartOffset() == textRange.getStartOffset() && firstStatementRange.getEndOffset() >= textRange.getEndOffset()) { anchor = statements[from]; + extractFix.shouldUseParent(); textRange = textRange.shiftLeft(textRange.getStartOffset()); } } @@ -440,6 +442,8 @@ public final class ExtractMethodRecommenderInspection extends AbstractBaseJavaLo private final String myOutputName; private final String myInputNames; + private boolean shouldUseParent = false; + private ExtractMethodFix(int from, int length, PsiVariable variable, List inputVariables) { myFrom = from; myLength = length; @@ -454,7 +458,11 @@ public final class ExtractMethodRecommenderInspection extends AbstractBaseJavaLo @Override public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { - PsiCodeBlock block = ObjectUtils.tryCast(descriptor.getStartElement(), PsiCodeBlock.class); + PsiElement element = descriptor.getStartElement(); + if (shouldUseParent) { + element = element.getParent(); + } + PsiCodeBlock block = ObjectUtils.tryCast(element, PsiCodeBlock.class); TextRange range = getRange(block); if (range == null) return; new MethodExtractor().doExtract(block.getContainingFile(), range.shiftRight(block.getTextRange().getStartOffset())); @@ -488,5 +496,9 @@ public final class ExtractMethodRecommenderInspection extends AbstractBaseJavaLo return new IntentionPreviewInfo.Html( JavaAnalysisBundle.message("inspection.extract.method.preview.html", myLength,input,myOutputName)); } + + private void shouldUseParent() { + shouldUseParent = true; + } } } diff --git a/java/java-tests/testData/inspection/extractMethodRecommender/CallExtract.java b/java/java-tests/testData/inspection/extractMethodRecommender/CallExtract.java new file mode 100644 index 000000000000..0f2fc4cd3ef7 --- /dev/null +++ b/java/java-tests/testData/inspection/extractMethodRecommender/CallExtract.java @@ -0,0 +1,55 @@ +class Test { + + public static void main(String[] args) { + } + + private static void test(Object o) { + for (int i = 0; i < 10; i++) { + System.out.println(1); + System.out.println(2); + System.out.println(3); + System.out.println(4); + System.out.println(5); + System.out.println(6); + System.out.println(7); + System.out.println(8); + System.out.println(9); + System.out.println(10); + + String s; + if (o instanceof String s2 && s2.length() == 1) { + s = "1"; + } else if (o instanceof String s2 && s2.length() == 2) { + s = "2"; + } else if (o instanceof String s2 && s2.length() == 3) { + s = "3"; + } else if (o instanceof String s2 && s2.length() == 4) { + s = "4"; + } else if (o instanceof String s2 && s2.length() == 5) { + s = "5"; + } else if (o instanceof String s2 && s2.length() == 6) { + s = "6"; + } else if (o instanceof String s2 && s2.length() == 7) { + s = "7"; + } else if (o instanceof String s2 && s2.length() == 8) { + s = "8"; + } else { + s = "null"; + } + + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + } + } +} diff --git a/java/java-tests/testData/inspection/extractMethodRecommender/CallExtractFirstNotDeclaration.java b/java/java-tests/testData/inspection/extractMethodRecommender/CallExtractFirstNotDeclaration.java new file mode 100644 index 000000000000..0fc834332aaf --- /dev/null +++ b/java/java-tests/testData/inspection/extractMethodRecommender/CallExtractFirstNotDeclaration.java @@ -0,0 +1,24 @@ +import java.util.ArrayList; +import java.util.List; + +class Test { + + public static void main(String[] args) { + } + List> simpleWithPrecedingComment() { + // Create list + // Comment + List list = new ArrayList<>(); + list.add("one"); + list.add("two"); + list.add("three"); + list.add("four"); + + List list2 = new ArrayList<>(); + list2.add("v1"); + list2.add("v2"); + list2.add("v3"); + list2.add("v4"); + return List.of(list, list2); + } +} diff --git a/java/java-tests/testData/inspection/extractMethodRecommender/afterCallExtract.java b/java/java-tests/testData/inspection/extractMethodRecommender/afterCallExtract.java new file mode 100644 index 000000000000..5150c120500c --- /dev/null +++ b/java/java-tests/testData/inspection/extractMethodRecommender/afterCallExtract.java @@ -0,0 +1,63 @@ +import org.jetbrains.annotations.NotNull; + +class Test { + + public static void main(String[] args) { + } + + private static void test(Object o) { + for (int i = 0; i < 10; i++) { + System.out.println(1); + System.out.println(2); + System.out.println(3); + System.out.println(4); + System.out.println(5); + System.out.println(6); + System.out.println(7); + System.out.println(8); + System.out.println(9); + System.out.println(10); + + String s = getString(o); + + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + System.out.println(s); + } + } + + @NotNull + private static String getString(Object o) { + String s; + if (o instanceof String s2 && s2.length() == 1) { + s = "1"; + } else if (o instanceof String s2 && s2.length() == 2) { + s = "2"; + } else if (o instanceof String s2 && s2.length() == 3) { + s = "3"; + } else if (o instanceof String s2 && s2.length() == 4) { + s = "4"; + } else if (o instanceof String s2 && s2.length() == 5) { + s = "5"; + } else if (o instanceof String s2 && s2.length() == 6) { + s = "6"; + } else if (o instanceof String s2 && s2.length() == 7) { + s = "7"; + } else if (o instanceof String s2 && s2.length() == 8) { + s = "8"; + } else { + s = "null"; + } + return s; + } +} diff --git a/java/java-tests/testData/inspection/extractMethodRecommender/afterCallExtractFirstNotDeclaration.java b/java/java-tests/testData/inspection/extractMethodRecommender/afterCallExtractFirstNotDeclaration.java new file mode 100644 index 000000000000..7e95328bb1d4 --- /dev/null +++ b/java/java-tests/testData/inspection/extractMethodRecommender/afterCallExtractFirstNotDeclaration.java @@ -0,0 +1,32 @@ +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.List; + +class Test { + + public static void main(String[] args) { + } + List> simpleWithPrecedingComment() { + // Create list + // Comment + List list = getStrings(); + + List list2 = new ArrayList<>(); + list2.add("v1"); + list2.add("v2"); + list2.add("v3"); + list2.add("v4"); + return List.of(list, list2); + } + + @NotNull + private static List getStrings() { + List list = new ArrayList<>(); + list.add("one"); + list.add("two"); + list.add("three"); + list.add("four"); + return list; + } +} diff --git a/java/java-tests/testSrc/com/intellij/refactoring/extractMethod/ExtractMethodRecommenderInspectionTest.java b/java/java-tests/testSrc/com/intellij/refactoring/extractMethod/ExtractMethodRecommenderInspectionTest.java index 026b589f00fa..c15d252e74de 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/extractMethod/ExtractMethodRecommenderInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/extractMethod/ExtractMethodRecommenderInspectionTest.java @@ -2,15 +2,13 @@ package com.intellij.refactoring.extractMethod; import com.intellij.JavaTestUtil; +import com.intellij.codeInsight.intention.IntentionAction; import com.intellij.codeInspection.RedundantSuppressInspection; +import com.intellij.java.JavaBundle; import com.intellij.testFramework.LightProjectDescriptor; import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase; import org.jetbrains.annotations.NotNull; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - public class ExtractMethodRecommenderInspectionTest extends LightJavaCodeInsightFixtureTestCase { public void testExtractMethodRecommender() { ExtractMethodRecommenderInspection inspection = new ExtractMethodRecommenderInspection(); @@ -35,6 +33,38 @@ public class ExtractMethodRecommenderInspectionTest extends LightJavaCodeInsight myFixture.checkHighlighting(); } + /** + * Test that quickfix can extract if anchor is moved to the declaration + */ + public void testCallExtract() { + ExtractMethodRecommenderInspection inspection = new ExtractMethodRecommenderInspection(); + inspection.minLength = 20; + myFixture.enableInspections(inspection); + myFixture.configureByFile(getTestName(false) + ".java"); + myFixture.checkHighlighting(); + IntentionAction intention = myFixture.getAvailableIntention(JavaBundle.message("intention.extract.method.text")); + assertNotNull(intention); + intention.invoke(getProject(), getEditor(), getFile()); + myFixture.checkResultByFile("after" + getTestName(false) + ".java"); + } + + /** + * Based on {@link ExtractMethodRecommenderInspectionTest#testExtractMethodRecommender()} + * when the suggestion is placed on comments, even though this place is not really convenient. + * This method checks that quickfix works even for this place + */ + public void testCallExtractFirstNotDeclaration() { + ExtractMethodRecommenderInspection inspection = new ExtractMethodRecommenderInspection(); + inspection.minLength = 10; + myFixture.enableInspections(inspection); + myFixture.configureByFile(getTestName(false) + ".java"); + myFixture.checkHighlighting(); + IntentionAction intention = myFixture.getAvailableIntention(JavaBundle.message("intention.extract.method.text")); + assertNotNull(intention); + intention.invoke(getProject(), getEditor(), getFile()); + myFixture.checkResultByFile("after" + getTestName(false) + ".java"); + } + @Override protected @NotNull LightProjectDescriptor getProjectDescriptor() { return JAVA_21;