From 31863df0e5faf26e85af652953bf7352eb883f91 Mon Sep 17 00:00:00 2001 From: "Denis.Zhdanov" Date: Mon, 17 Dec 2012 19:15:56 +0400 Subject: [PATCH] IDEA-97680 Java arrangement: Respect 'dependent methods' for calls from anonymous classes --- .../arrangement/JavaArrangementVisitor.java | 38 +++++++++++++---- .../JavaRearrangerGrouperTest.groovy | 41 ++++++++++++++++--- 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/java/java-impl/src/com/intellij/psi/codeStyle/arrangement/JavaArrangementVisitor.java b/java/java-impl/src/com/intellij/psi/codeStyle/arrangement/JavaArrangementVisitor.java index ece95fe752a8..6800b666f00a 100644 --- a/java/java-impl/src/com/intellij/psi/codeStyle/arrangement/JavaArrangementVisitor.java +++ b/java/java-impl/src/com/intellij/psi/codeStyle/arrangement/JavaArrangementVisitor.java @@ -203,8 +203,15 @@ public class JavaArrangementVisitor extends JavaElementVisitor { if (overridden != null) { myInfo.onOverriddenMethod(overridden.getMethod(), method); } - myMethodBodyProcessor.setBaseMethod(method); - method.accept(myMethodBodyProcessor); + boolean reset = myMethodBodyProcessor.setBaseMethod(method); + try { + method.accept(myMethodBodyProcessor); + } + finally { + if (reset) { + myMethodBodyProcessor.setBaseMethod(null); + } + } } private void parseProperties(PsiMethod method, JavaElementArrangementEntry entry) { @@ -310,7 +317,7 @@ public class JavaArrangementVisitor extends JavaElementVisitor { if (canArrange) { TextRange expandedRange = myDocument == null ? null : ArrangementUtil.expandToLine(range, myDocument); TextRange rangeToUse = expandedRange == null ? range : expandedRange; - entry = new JavaElementArrangementEntry(current, rangeToUse, type, name, myDocument == null || expandedRange != null); + entry = new JavaElementArrangementEntry(current, rangeToUse, type, name, true); } else { entry = new JavaElementArrangementEntry(current, range, type, name, false); @@ -361,7 +368,7 @@ public class JavaArrangementVisitor extends JavaElementVisitor { private static class MethodBodyProcessor extends JavaRecursiveElementVisitor { @NotNull private final JavaArrangementParseInfo myInfo; - @NotNull private PsiMethod myBaseMethod; + @Nullable private PsiMethod myBaseMethod; MethodBodyProcessor(@NotNull JavaArrangementParseInfo info) { myInfo = info; @@ -374,14 +381,27 @@ public class JavaArrangementVisitor extends JavaElementVisitor { } PsiElement e = reference.resolve(); if (e instanceof PsiMethod) { - myInfo.registerDependency(myBaseMethod, (PsiMethod)e); + assert myBaseMethod != null; + PsiMethod m = (PsiMethod)e; + if (m.getContainingClass() == myBaseMethod.getContainingClass()) { + myInfo.registerDependency(myBaseMethod, m); + } } - // Now parse the expression list, it also might contain method calls. - super.visitExpressionList(psiMethodCallExpression.getArgumentList()); + + // We process all method call expression children because there is a possible case like below: + // new Runnable() { + // void test(); + // }.run(); + // Here we want to process that 'Runnable.run()' implementation. + super.visitMethodCallExpression(psiMethodCallExpression); } - public void setBaseMethod(@NotNull PsiMethod baseMethod) { - myBaseMethod = baseMethod; + public boolean setBaseMethod(@Nullable PsiMethod baseMethod) { + if (baseMethod == null || myBaseMethod == null /* don't override a base method in case of method-local anonymous classes */) { + myBaseMethod = baseMethod; + return true; + } + return false; } } } diff --git a/java/java-tests/testSrc/com/intellij/psi/codeStyle/arrangement/JavaRearrangerGrouperTest.groovy b/java/java-tests/testSrc/com/intellij/psi/codeStyle/arrangement/JavaRearrangerGrouperTest.groovy index 7e2c72e250b2..12cdf530c17e 100644 --- a/java/java-tests/testSrc/com/intellij/psi/codeStyle/arrangement/JavaRearrangerGrouperTest.groovy +++ b/java/java-tests/testSrc/com/intellij/psi/codeStyle/arrangement/JavaRearrangerGrouperTest.groovy @@ -29,7 +29,12 @@ import static com.intellij.psi.codeStyle.arrangement.order.ArrangementEntryOrder * @since 9/18/12 11:19 AM */ class JavaRearrangerGrouperTest extends AbstractJavaRearrangerTest { - + + void setUp() { + super.setUp() + commonSettings.BLANK_LINES_AROUND_METHOD = 0 + } + void testGettersAndSetters() { commonSettings.BLANK_LINES_AROUND_METHOD = 1 @@ -55,7 +60,6 @@ class Test { @Test void testUtilityMethodsDepthFirst() { - commonSettings.BLANK_LINES_AROUND_METHOD = 0 doTest( initial: '''\ class Test { @@ -78,7 +82,6 @@ class Test { @Test void testUtilityMethodsBreadthFirst() { - commonSettings.BLANK_LINES_AROUND_METHOD = 0 doTest( initial: '''\ class Test { @@ -98,7 +101,6 @@ class Test { } void testOverriddenMethods() { - commonSettings.BLANK_LINES_AROUND_METHOD = 0 doTest( initial: '''\ class Base { @@ -127,7 +129,6 @@ class Sub extends Base { }''') } void testOverriddenAndUtilityMethods() { - commonSettings.BLANK_LINES_AROUND_METHOD = 0 doTest( initial: '''\ class Base { @@ -159,4 +160,34 @@ class Sub extends Base { void test4() {} }''') } + + void "test that calls from anonymous class create a dependency"() { + doTest( + initial: ''' +class Test { + void test2() {} + void test1() { test2(); } + void root() { + new Runnable() { + public void run() { + test1(); + } + }.run(); + } +}''', + groups: [group(DEPENDENT_METHODS, DEPTH_FIRST)], + expected: ''' +class Test { + void root() { + new Runnable() { + public void run() { + test1(); + } + }.run(); + } + void test1() { test2(); } + void test2() {} +}''' + ) + } }