From 9d9063bbb4300e80f2ffb53ea337c9306fab168f Mon Sep 17 00:00:00 2001
From: Maxim Medvedev <maxim.medvedev@jetbrains.com>
Date: Sun, 6 Mar 2011 19:33:46 +0300
Subject: [PATCH] IDEA-66202: Groovy Extract Method Broken: Nested Closures
 drop 'it' reference

---
 .../GrReferenceExpressionImpl.java            | 17 +---
 .../plugins/groovy/lang/psi/util/PsiUtil.java | 80 +++++++++++++------
 .../extractMethod/ExtractMethodTest.java      |  1 +
 .../dontShortenRefsIncorrect.test             |  7 ++
 4 files changed, 66 insertions(+), 39 deletions(-)
 create mode 100644 plugins/groovy/testdata/groovy/refactoring/extractMethod/dontShortenRefsIncorrect.test

diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/GrReferenceExpressionImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/GrReferenceExpressionImpl.java
index 495b287497b4..e54159c83bb6 100644
--- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/GrReferenceExpressionImpl.java
+++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/GrReferenceExpressionImpl.java
@@ -48,7 +48,6 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrField;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.*;
-import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.literals.GrLiteral;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrMethodCallExpression;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrAccessorMethod;
@@ -437,8 +436,9 @@ public class GrReferenceExpressionImpl extends GrReferenceElementImpl<GrExpressi
         PsiClass containingClass = method.getContainingClass();
         if (containingClass != null && CommonClassNames.JAVA_LANG_OBJECT.equals(containingClass.getQualifiedName()) &&
                 "getClass".equals(method.getName())) {
-          if (seemsToBeQualifiedClassName(getQualifierExpression())) {
-            result = createJavaLangClassType(facade, facade.getElementFactory().createTypeFromText(getQualifier().getText(), this));
+          final GrExpression qualifier = getQualifier();
+          if (PsiUtil.seemsToBeQualifiedClassName(qualifier)) {
+            result = createJavaLangClassType(facade, facade.getElementFactory().createTypeFromText(qualifier.getText(), this));
           }
           else {
             result = getTypeForObjectGetClass(facade, method);
@@ -464,7 +464,7 @@ public class GrReferenceExpressionImpl extends GrReferenceElementImpl<GrExpressi
     else if (resolved == null) {
       GrExpression qualifier = getQualifierExpression();
       if ("class".equals(getReferenceName())) {
-        if (seemsToBeQualifiedClassName(qualifier)) {
+        if (PsiUtil.seemsToBeQualifiedClassName(qualifier)) {
           assert qualifier != null;
           result = createJavaLangClassType(facade, facade.getElementFactory().createTypeFromText(qualifier.getText(), this));
         } else {
@@ -502,15 +502,6 @@ public class GrReferenceExpressionImpl extends GrReferenceElementImpl<GrExpressi
     }
   }
 
-  private static boolean seemsToBeQualifiedClassName(@Nullable GrExpression qualifier) {
-    if (qualifier == null) return false;
-    while (qualifier instanceof GrReferenceExpression) {
-      if (((GrReferenceExpression)qualifier).getReferenceNameElement() instanceof GrLiteral) return false;
-      qualifier = ((GrReferenceExpression)qualifier).getQualifierExpression();
-    }
-    return qualifier == null;
-  }
-
   @Nullable
   private PsiType createJavaLangClassType(JavaPsiFacade facade, @Nullable PsiType type) {
     PsiType result = null;
diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java
index 269e2b67da4f..24b735fc2784 100644
--- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java
+++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/PsiUtil.java
@@ -59,6 +59,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.arguments.GrArgument
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.arguments.GrNamedArgument;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.*;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.literals.GrLiteral;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrIndexProperty;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrMethodCallExpression;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrPropertySelection;
@@ -304,41 +305,59 @@ public class PsiUtil {
 
   public static <Qualifier extends PsiElement> boolean shortenReference(GrQualifiedReference<Qualifier> ref) {
     final Qualifier qualifier = ref.getQualifier();
-    if (qualifier != null && !(qualifier instanceof GrSuperReferenceExpression) &&
-        (PsiTreeUtil.getParentOfType(ref, GrDocMemberReference.class) != null ||
-         PsiTreeUtil.getParentOfType(ref, GrDocComment.class) == null) &&
-        PsiTreeUtil.getParentOfType(ref, GrImportStatement.class) == null &&
-        PsiTreeUtil.getParentOfType(ref, GroovyCodeFragment.class) == null) {
-      final PsiElement resolved = ref.resolve();
-      if (resolved != null) {
-        ref.setQualifier(null);
-        if (ref.isReferenceTo(resolved)) return true;
+    if (qualifier == null || qualifier instanceof GrSuperReferenceExpression || cannotShortenInContext(ref)) {
+      return false;
+    }
+    if (!canShorten(qualifier)) return false;
 
-        if (resolved instanceof PsiClass) {
-          final GroovyFileBase file = (GroovyFileBase)ref.getContainingFile();
-          final PsiClass clazz = (PsiClass)resolved;
-          final String qName = clazz.getQualifiedName();
-          if (qName != null) {
-            if (mayInsertImport(ref)) {
-              final GrImportStatement added = file.addImportForClass(clazz);
-              if (!ref.isReferenceTo(resolved)) {
-                file.removeImport(added);
-              }
-            }
+    final PsiElement resolved = ref.resolve();
+    if (resolved == null) return false;
+
+    ref.setQualifier(null);
+    if (ref.isReferenceTo(resolved)) return true;
+
+    if (resolved instanceof PsiClass) {
+      final GroovyFileBase file = (GroovyFileBase)ref.getContainingFile();
+      final PsiClass clazz = (PsiClass)resolved;
+      final String qName = clazz.getQualifiedName();
+      if (qName != null) {
+        if (mayInsertImport(ref)) {
+          final GrImportStatement added = file.addImportForClass(clazz);
+          if (!ref.isReferenceTo(resolved)) {
+            file.removeImport(added);
           }
         }
+      }
+    }
 
-        if (!ref.isReferenceTo(resolved)) {
-          ref.setQualifier((Qualifier)qualifier.copy());
-          return false;
-        } else {
-          return true;
-        }
+    if (!ref.isReferenceTo(resolved)) {
+      ref.setQualifier((Qualifier)qualifier.copy());
+      return false;
+    }
+    else {
+      return true;
+    }
+  }
+
+  private static <Qualifier extends PsiElement> boolean canShorten(Qualifier qualifier) {
+    if (qualifier instanceof GrCodeReferenceElement) return true;
+    if (qualifier instanceof GrExpression) {
+      if (qualifier instanceof GrThisReferenceExpression) return true;
+      if (seemsToBeQualifiedClassName((GrExpression)qualifier)) {
+        final PsiElement resolved = ((GrReferenceExpression)qualifier).resolve();
+        if (resolved instanceof PsiClass || resolved instanceof PsiPackage) return true;
       }
     }
     return false;
   }
 
+  private static <Qualifier extends PsiElement> boolean cannotShortenInContext(GrQualifiedReference<Qualifier> ref) {
+    return (PsiTreeUtil.getParentOfType(ref, GrDocMemberReference.class) == null &&
+            PsiTreeUtil.getParentOfType(ref, GrDocComment.class) != null) ||
+           PsiTreeUtil.getParentOfType(ref, GrImportStatement.class) != null ||
+           PsiTreeUtil.getParentOfType(ref, GroovyCodeFragment.class) != null;
+  }
+
   private static <Qualifier extends PsiElement> boolean mayInsertImport(GrQualifiedReference<Qualifier> ref) {
     return PsiTreeUtil.getParentOfType(ref, GrDocComment.class) == null &&
            !(ref.getContainingFile() instanceof GroovyCodeFragment) &&
@@ -1069,4 +1088,13 @@ public class PsiUtil {
 
     return ((GrListOrMap)firstArg).getNamedArguments();
   }
+
+  public static boolean seemsToBeQualifiedClassName(@Nullable GrExpression qualifier) {
+    if (qualifier == null) return false;
+    while (qualifier instanceof GrReferenceExpression) {
+      if (((GrReferenceExpression)qualifier).getReferenceNameElement() instanceof GrLiteral) return false;
+      qualifier = ((GrReferenceExpression)qualifier).getQualifierExpression();
+    }
+    return qualifier == null;
+  }
 }
diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/extractMethod/ExtractMethodTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/extractMethod/ExtractMethodTest.java
index 6697a9d1f111..386a7d4063e8 100644
--- a/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/extractMethod/ExtractMethodTest.java
+++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/refactoring/extractMethod/ExtractMethodTest.java
@@ -96,4 +96,5 @@ public class ExtractMethodTest extends LightGroovyTestCase {
   public void testMultiOutput4() {doTest();}
   public void testMultiOutput5() {doTest();}
 
+  public void testDontShortenRefsIncorrect() {doTest();}
 }
\ No newline at end of file
diff --git a/plugins/groovy/testdata/groovy/refactoring/extractMethod/dontShortenRefsIncorrect.test b/plugins/groovy/testdata/groovy/refactoring/extractMethod/dontShortenRefsIncorrect.test
new file mode 100644
index 000000000000..f729cf2e6552
--- /dev/null
+++ b/plugins/groovy/testdata/groovy/refactoring/extractMethod/dontShortenRefsIncorrect.test
@@ -0,0 +1,7 @@
+<begin>print 1.getClass()<end>
+-----
+testMethod()
+
+private def testMethod() {
+    print 1.getClass()
+}