IDEA-58056 Groovy: "Convert to method" intention: closure usages check could be added.

2026-09-27 10:03:11 +07:00 · 2010-09-14 16:54:04 +04:00
parent 308dde667a
commit ddedbbedd3
7 changed files with 229 additions and 68 deletions
@@ -58,4 +58,3 @@ pointless.boolean.quickfix=Simplify

 Cannot.perform.undo.operation=Cannot perform undo operation
 Undo.disable=Undo disabled
-closure.is.used.as.variable.in.not.groovy=Closure is used as a variable
@@ -97,4 +97,9 @@ closure.used.as.variable=Closure is passed as argument. Refactoring can break se
 convert.map.to.class.intention.name = Convert to class
 convert.map.to.class.intention.family.name = Convert Groovy native map to class instances
 do.you.want.to.change.method.return.type=Do you want to change return type of ''{0}'' method
-do.you.want.to.change.variable.type=Do you want to change type of ''{0}''
+do.you.want.to.change.variable.type=Do you want to change type of ''{0}''
+
+closure.is.accessed.outside.of.groovy=Field <b>{0}</b> is accessed outside of Groovy
+write.access.to.closure.variable=Write access to field <b>{0}</b>
+field.is.used.in.argument.label=Field <b>{0}</b> is used in argument label
+method.with.signature.already.exists=Method with signature {0} already exists
@@ -16,36 +16,52 @@

 package org.jetbrains.plugins.groovy.intentions.conversions;

+import com.intellij.openapi.application.ApplicationManager;
+import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.editor.Editor;
 import com.intellij.openapi.project.Project;
-import com.intellij.psi.PsiElement;
-import com.intellij.psi.PsiReference;
+import com.intellij.openapi.ui.DialogWrapper;
+import com.intellij.psi.*;
+import com.intellij.psi.search.searches.MethodReferencesSearch;
 import com.intellij.psi.search.searches.ReferencesSearch;
+import com.intellij.psi.util.MethodSignature;
+import com.intellij.psi.util.MethodSignatureUtil;
 import com.intellij.refactoring.ui.ConflictsDialog;
 import com.intellij.util.IncorrectOperationException;
 import com.intellij.util.containers.HashSet;
 import com.intellij.util.containers.MultiMap;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.plugins.groovy.GroovyFileType;
-import org.jetbrains.plugins.groovy.codeInspection.GroovyInspectionBundle;
+import org.jetbrains.plugins.groovy.intentions.GroovyIntentionsBundle;
 import org.jetbrains.plugins.groovy.intentions.base.Intention;
 import org.jetbrains.plugins.groovy.intentions.base.PsiElementPredicate;
+import org.jetbrains.plugins.groovy.lang.documentation.GroovyPresentationUtil;
 import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElementFactory;
 import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.GrModifier;
 import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.GrModifierList;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrField;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariableDeclaration;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.arguments.GrArgumentLabel;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression;
+import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrAccessorMethod;
 import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod;
+import org.jetbrains.plugins.groovy.lang.psi.api.types.GrClosureSignature;
+import org.jetbrains.plugins.groovy.lang.psi.impl.GrClosureType;
+import org.jetbrains.plugins.groovy.lang.psi.impl.types.GrClosureSignatureUtil;
 import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil;

 import java.util.Collection;
+import java.util.List;

 /**
 * @author Maxim.Medvedev
 */
 public class ConvertClosureToMethodIntention extends Intention {
+  private static final Logger LOG =
+    Logger.getInstance("#org.jetbrains.plugins.groovy.intentions.conversions.ConvertClosureToMethodIntention");
+
  @NotNull
  @Override
  protected PsiElementPredicate getElementPredicate() {
@@ -58,37 +74,68 @@ public class ConvertClosureToMethodIntention extends Intention {

    final GrField field = (GrField)element;

-    final Collection<PsiReference> usages = ReferencesSearch.search(field).findAll();
+    final HashSet<PsiReference> usages = new HashSet<PsiReference>();
+    usages.addAll(ReferencesSearch.search(field).findAll());
+    final GrAccessorMethod[] getters = field.getGetters();
+    for (GrAccessorMethod getter : getters) {
+      usages.addAll(MethodReferencesSearch.search(getter).findAll());
+    }
+    final GrAccessorMethod setter = field.getSetter();
+    if (setter != null) {
+      usages.addAll(MethodReferencesSearch.search(setter).findAll());
+    }

+    final String fieldName = field.getName();
+    LOG.assertTrue(fieldName != null);
    final Collection<PsiElement> fieldUsages = new HashSet<PsiElement>();
    MultiMap<PsiElement, String> conflicts = new MultiMap<PsiElement, String>();
    for (PsiReference usage : usages) {
      final PsiElement psiElement = usage.getElement();
      if (PsiUtil.isMethodUsage(psiElement)) continue;
-      if (GroovyFileType.GROOVY_LANGUAGE.equals(psiElement.getLanguage())) {
-        fieldUsages.add(psiElement);
+      if (!GroovyFileType.GROOVY_LANGUAGE.equals(psiElement.getLanguage())) {
+        conflicts.putValue(psiElement, GroovyIntentionsBundle.message("closure.is.accessed.outside.of.groovy", fieldName));
      }
      else {
-        conflicts.putValue(psiElement, GroovyInspectionBundle.message("closure.is.used.as.variable.in.not.groovy"));
+        if (psiElement instanceof GrReferenceExpression) {
+          fieldUsages.add(psiElement);
+          if (PsiUtil.isAccessedForWriting((GrExpression)psiElement)) {
+            conflicts.putValue(psiElement, GroovyIntentionsBundle.message("write.access.to.closure.variable", fieldName));
+          }
+        }
+        else if (psiElement instanceof GrArgumentLabel) {
+          conflicts.putValue(psiElement, GroovyIntentionsBundle.message("field.is.used.in.argument.label", fieldName));
+        }
+      }
+    }
+    final PsiClass containingClass = field.getContainingClass();
+    final PsiType type = field.getTypeGroovy();
+    LOG.assertTrue(type instanceof GrClosureType);
+    final GrClosureSignature signature = ((GrClosureType)type).getSignature();
+    final List<MethodSignature> signatures = GrClosureSignatureUtil.generateAllMethodSignaturesByClosureSignature(fieldName, signature);
+    for (MethodSignature s : signatures) {
+      final PsiMethod method = MethodSignatureUtil.findMethodBySignature(containingClass, s, true);
+      if (method != null) {
+        conflicts.putValue(method, GroovyIntentionsBundle.message("method.with.signature.already.exists",
+                                                                  GroovyPresentationUtil.getSignaturePresentation(s)));
      }
    }
    if (conflicts.size() > 0) {
      final ConflictsDialog conflictsDialog = new ConflictsDialog(project, conflicts, new Runnable() {
+        @Override
        public void run() {
          execute(field, fieldUsages);
        }
      });
      conflictsDialog.show();
+      if (conflictsDialog.getExitCode() != DialogWrapper.OK_EXIT_CODE) return;
    }
-    else {
-      execute(field, fieldUsages);
-    }
+    execute(field, fieldUsages);
  }

-  private static void execute(GrField field, Collection<PsiElement> fieldUsages) {
+  private static void execute(final GrField field, final Collection<PsiElement> fieldUsages) {
    final GroovyPsiElementFactory factory = GroovyPsiElementFactory.getInstance(field.getProject());

-    StringBuilder builder = new StringBuilder(field.getTextLength());
+    final StringBuilder builder = new StringBuilder(field.getTextLength());
    final GrClosableBlock block = (GrClosableBlock)field.getInitializerGroovy();

    final GrModifierList modifierList = field.getModifierList();
@@ -108,18 +155,54 @@ public class ConvertClosureToMethodIntention extends Intention {
      builder.append("def it = null");
    }
    builder.append(") {");
-    block.getParameterList().delete();
-    block.getLBrace().delete();
-    final PsiElement psiElement = PsiUtil.skipWhitespaces(block.getFirstChild(), true);
-    if (psiElement != null && "->".equals(psiElement.getText())) {
-      psiElement.delete();
-    }
-    builder.append(block.getText());
-    final GrMethod method = GroovyPsiElementFactory.getInstance(field.getProject()).createMethodFromText(builder.toString());
-    field.getParent().replace(method);
-    for (PsiElement usage : fieldUsages) {
-      usage.replace(factory.createReferenceExpressionFromText("&" + usage.getText()));
-    }
+
+
+    ApplicationManager.getApplication().runWriteAction(new Runnable() {
+      public void run() {
+        block.getParameterList().delete();
+        block.getLBrace().delete();
+        final PsiElement psiElement = PsiUtil.skipWhitespaces(block.getFirstChild(), true);
+        if (psiElement != null && "->".equals(psiElement.getText())) {
+          psiElement.delete();
+        }
+        builder.append(block.getText());
+        final GrMethod method = GroovyPsiElementFactory.getInstance(field.getProject()).createMethodFromText(builder.toString());
+        field.getParent().replace(method);
+        for (PsiElement usage : fieldUsages) {
+          if (usage instanceof GrReferenceExpression) {
+            final PsiElement parent = usage.getParent();
+            StringBuilder newRefText = new StringBuilder();
+            if (parent instanceof GrReferenceExpression &&
+                usage == ((GrReferenceExpression)parent).getQualifier() &&
+                "call".equals(((GrReferenceExpression)parent).getName())) {
+              newRefText.append(usage.getText());
+              usage = parent;
+            }
+            else {
+              PsiElement qualifier = ((GrReferenceExpression)usage).getQualifier();
+              if (qualifier == null) {
+                if (parent instanceof GrReferenceExpression &&
+                    ((GrReferenceExpression)parent).getQualifier() != null &&
+                    usage != ((GrReferenceExpression)parent).getQualifier()) {
+                  qualifier = ((GrReferenceExpression)parent).getQualifier();
+                  usage = parent;
+                }
+              }
+
+              if (qualifier != null) {
+                newRefText.append(qualifier.getText()).append('.');
+                ((GrReferenceExpression)usage).setQualifierExpression(null);
+              }
+              else {
+                newRefText.append("this.");
+              }
+              newRefText.append('&').append(usage.getText());
+            }
+            usage.replace(factory.createReferenceExpressionFromText(newRefText.toString()));
+          }
+        }
+      }
+    });
  }

  private static class MyPredicate implements PsiElementPredicate {
@@ -125,7 +125,7 @@ public class GrClosureSignatureUtil {

    if (args.length == 1) {
      final GrClosureParameter[] parameters = signature.getParameters();
-      if (parameters.length == 1 && parameters[0].getType() instanceof PsiArrayType) return false; 
+      if (parameters.length == 1 && parameters[0].getType() instanceof PsiArrayType) return false;
      PsiType arg = args[0];
      if (arg instanceof GrTupleType) {
        args = ((GrTupleType)arg).getComponentTypes();
@@ -303,7 +303,7 @@ public class GrClosureSignatureUtil {

  public static class ArgInfo<ArgType> {
    public static final ArgInfo[] EMPTY_ARRAY = new ArgInfo[0];
-    
+
    public List<ArgType> args;
    public final boolean isMultiArg;

@@ -322,8 +322,11 @@ public class GrClosureSignatureUtil {
      this.isMultiArg = isMultiArg;
    }

-    public static <ArgType> ArgInfo<ArgType>[] empty_array() {return EMPTY_ARRAY;}
+    public static <ArgType> ArgInfo<ArgType>[] empty_array() {
+      return EMPTY_ARRAY;
+    }
  }
+
  /**
   * Returns array of lists which contain psiElements mapped to parameters
   *
@@ -338,14 +341,14 @@ public class GrClosureSignatureUtil {
  }

  private static class InnerArg {
-      List<PsiElement> list;
-      PsiType type;
+    List<PsiElement> list;
+    PsiType type;

-      InnerArg(PsiType type, PsiElement... elements) {
-        this.list = new ArrayList<PsiElement>(Arrays.asList(elements));
-        this.type = type;
-      }
+    InnerArg(PsiType type, PsiElement... elements) {
+      this.list = new ArrayList<PsiElement>(Arrays.asList(elements));
+      this.type = type;
    }
+  }

  @Nullable
  public static Map<GrExpression, Pair<PsiParameter, PsiType>> mapArgumentsToParameters(@NotNull GroovyResolveResult resolveResult,
@@ -361,14 +364,17 @@ public class GrClosureSignatureUtil {
    if (element instanceof PsiMethod) {
      signature = createSignature((PsiMethod)element, substitutor);
      parameters = ((PsiMethod)element).getParameterList().getParameters();
-    } else if (element instanceof GrClosableBlock) {
+    }
+    else if (element instanceof GrClosableBlock) {
      signature = createSignature((GrClosableBlock)element);
      parameters = ((GrClosableBlock)element).getAllParameters();
-    } else {
+    }
+    else {
      return null;
    }

-    final ArgInfo<PsiElement>[] argInfos = mapParametersToArguments(signature, namedArgs, expressionArgs, context, closureArguments, partial);
+    final ArgInfo<PsiElement>[] argInfos =
+      mapParametersToArguments(signature, namedArgs, expressionArgs, context, closureArguments, partial);
    if (argInfos == null) {
      return null;
    }
@@ -475,30 +481,9 @@ public class GrClosureSignatureUtil {
  public static List<MethodSignature> generateAllSignaturesForMethod(GrMethod method, PsiSubstitutor substitutor) {
    GrClosureSignature signature = createSignature(method, substitutor);
    String name = method.getName();
-    GrClosureParameter[] params = signature.getParameters();
    PsiTypeParameter[] typeParameters = method.getTypeParameters();

-    ArrayList<PsiType> newParams = new ArrayList<PsiType>(params.length);
-    ArrayList<GrClosureParameter> opts = new ArrayList<GrClosureParameter>(params.length);
-    ArrayList<Integer> optInds = new ArrayList<Integer>(params.length);
-
-    for (int i = 0; i < params.length; i++) {
-      if (params[i].isOptional()) {
-        opts.add(params[i]);
-        optInds.add(i);
-      }
-      else {
-        newParams.add(params[i].getType());
-      }
-    }
-
-    List<MethodSignature> result = new ArrayList<MethodSignature>(opts.size() + 1);
-    result.add(generateSignature(name, newParams, typeParameters, substitutor));
-    for (int i = 0; i < opts.size(); i++) {
-      newParams.add(optInds.get(i), opts.get(i).getType());
-      result.add(generateSignature(name, newParams, typeParameters, substitutor));
-    }
-    return result;
+    return generateAllMethodSignaturesByClosureSignature(name, signature, typeParameters, substitutor);
  }

  public static MultiMap<MethodSignature, PsiMethod> findMethodSignatures(PsiMethod[] methods) {
@@ -529,4 +514,38 @@ public class GrClosureSignatureUtil {
                                                   PsiSubstitutor substitutor) {
    return MethodSignatureUtil.createMethodSignature(name, paramTypes.toArray(new PsiType[paramTypes.size()]), typeParameters, substitutor);
  }
+
+  public static List<MethodSignature> generateAllMethodSignaturesByClosureSignature(@NotNull String name,
+                                                                                     @NotNull GrClosureSignature signature,
+                                                                                     @NotNull PsiTypeParameter[] typeParameters,
+                                                                                     @NotNull PsiSubstitutor substitutor) {
+    GrClosureParameter[] params = signature.getParameters();
+
+    ArrayList<PsiType> newParams = new ArrayList<PsiType>(params.length);
+    ArrayList<GrClosureParameter> opts = new ArrayList<GrClosureParameter>(params.length);
+    ArrayList<Integer> optInds = new ArrayList<Integer>(params.length);
+
+    for (int i = 0; i < params.length; i++) {
+      if (params[i].isOptional()) {
+        opts.add(params[i]);
+        optInds.add(i);
+      }
+      else {
+        newParams.add(params[i].getType());
+      }
+    }
+
+    List<MethodSignature> result = new ArrayList<MethodSignature>(opts.size() + 1);
+    result.add(generateSignature(name, newParams, typeParameters, substitutor));
+    for (int i = 0; i < opts.size(); i++) {
+      newParams.add(optInds.get(i), opts.get(i).getType());
+      result.add(generateSignature(name, newParams, typeParameters, substitutor));
+    }
+    return result;
+  }
+
+  public static List<MethodSignature> generateAllMethodSignaturesByClosureSignature(@NotNull String name,
+                                                                                     @NotNull GrClosureSignature signature) {
+    return generateAllMethodSignaturesByClosureSignature(name, signature, PsiTypeParameter.EMPTY_ARRAY, PsiSubstitutor.EMPTY);
+  }
 }
@@ -27,23 +27,36 @@ public class ConvertMethodToClosureTest extends GrIntentionTestCase {
    return TestUtils.getTestDataPath() + "intentions/convertMethodToClosure/";
  }

-  public void testMethodToClosure() throws Exception {
-    doTest(GroovyIntentionsBundle.message("convert.method.to.closure.intention.name"), true);
+  public void testMethodToClosure() {
+    doMethodToClosureTest();
  }

-  public void testStaticMethodToClosure() throws Exception {
-    doTest(GroovyIntentionsBundle.message("convert.method.to.closure.intention.name"), true);
+  public void testStaticMethodToClosure()  {
+    doMethodToClosureTest();
  }

-  public void testClosureToMethod() throws Exception {
-    doTest(GroovyIntentionsBundle.message("convert.closure.to.method.intention.name"), true);
+  public void testClosureToMethod()  {
+    doClosureToMethodTest();
  }

-  public void testClosureWithImplicitParameterToMethod() throws Exception {
-    doTest(GroovyIntentionsBundle.message("convert.closure.to.method.intention.name"), true);
+  public void testClosureWithImplicitParameterToMethod()  {
+    doClosureToMethodTest();
  }

  public void testClosureWithoutModifiersToMethod() {
+    doClosureToMethodTest();
+  }
+
+  public void testClosureToMethodWithFieldUsages() {
+    doClosureToMethodTest();
+  }
+
+  private void doClosureToMethodTest() {
    doTest(GroovyIntentionsBundle.message("convert.closure.to.method.intention.name"), true);
  }
+
+  private void doMethodToClosureTest() {
+    doTest(GroovyIntentionsBundle.message("convert.method.to.closure.intention.name"), true);
+  }
+
 }
@@ -0,0 +1,21 @@
+class X {
+  def fo<caret>o = {print it}
+
+  def bar() {
+    foo(2)
+
+    this.foo(2)
+
+    foo.call(2)
+    foo.call()
+
+    print this.&foo
+  }
+}
+
+final X x = new X()
+x.foo(2)
+print x.foo
+x.foo.call(2)
+x.foo.call()
+
@@ -0,0 +1,21 @@
+class X {
+  def foo(def it = null) {print it}
+
+  def bar() {
+    foo(2)
+
+    this.foo(2)
+
+    foo(2)
+    foo()
+
+    print this.&foo
+  }
+}
+
+final X x = new X()
+x.foo(2)
+print x.&foo
+x.foo(2)
+x.foo()
+