diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties index 88386e7c6530..483d5cf26fa2 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties @@ -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 diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/GroovyIntentionsBundle.properties b/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/GroovyIntentionsBundle.properties index 913911be88ab..bb9ff1941211 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/GroovyIntentionsBundle.properties +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/GroovyIntentionsBundle.properties @@ -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}'' \ No newline at end of file +do.you.want.to.change.variable.type=Do you want to change type of ''{0}'' + +closure.is.accessed.outside.of.groovy=Field {0} is accessed outside of Groovy +write.access.to.closure.variable=Write access to field {0} +field.is.used.in.argument.label=Field {0} is used in argument label +method.with.signature.already.exists=Method with signature {0} already exists diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/conversions/ConvertClosureToMethodIntention.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/conversions/ConvertClosureToMethodIntention.java index 8b5b919298ba..9853df9e106c 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/conversions/ConvertClosureToMethodIntention.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/conversions/ConvertClosureToMethodIntention.java @@ -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 usages = ReferencesSearch.search(field).findAll(); + final HashSet usages = new HashSet(); + 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 fieldUsages = new HashSet(); MultiMap conflicts = new MultiMap(); 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 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 fieldUsages) { + private static void execute(final GrField field, final Collection 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 { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureSignatureUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureSignatureUtil.java index 9f881e66aac6..97dd190768f0 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureSignatureUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrClosureSignatureUtil.java @@ -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 { public static final ArgInfo[] EMPTY_ARRAY = new ArgInfo[0]; - + public List args; public final boolean isMultiArg; @@ -322,8 +322,11 @@ public class GrClosureSignatureUtil { this.isMultiArg = isMultiArg; } - public static ArgInfo[] empty_array() {return EMPTY_ARRAY;} + public static ArgInfo[] 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 list; - PsiType type; + List list; + PsiType type; - InnerArg(PsiType type, PsiElement... elements) { - this.list = new ArrayList(Arrays.asList(elements)); - this.type = type; - } + InnerArg(PsiType type, PsiElement... elements) { + this.list = new ArrayList(Arrays.asList(elements)); + this.type = type; } + } @Nullable public static Map> 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[] argInfos = mapParametersToArguments(signature, namedArgs, expressionArgs, context, closureArguments, partial); + final ArgInfo[] argInfos = + mapParametersToArguments(signature, namedArgs, expressionArgs, context, closureArguments, partial); if (argInfos == null) { return null; } @@ -475,30 +481,9 @@ public class GrClosureSignatureUtil { public static List generateAllSignaturesForMethod(GrMethod method, PsiSubstitutor substitutor) { GrClosureSignature signature = createSignature(method, substitutor); String name = method.getName(); - GrClosureParameter[] params = signature.getParameters(); PsiTypeParameter[] typeParameters = method.getTypeParameters(); - ArrayList newParams = new ArrayList(params.length); - ArrayList opts = new ArrayList(params.length); - ArrayList optInds = new ArrayList(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 result = new ArrayList(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 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 generateAllMethodSignaturesByClosureSignature(@NotNull String name, + @NotNull GrClosureSignature signature, + @NotNull PsiTypeParameter[] typeParameters, + @NotNull PsiSubstitutor substitutor) { + GrClosureParameter[] params = signature.getParameters(); + + ArrayList newParams = new ArrayList(params.length); + ArrayList opts = new ArrayList(params.length); + ArrayList optInds = new ArrayList(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 result = new ArrayList(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 generateAllMethodSignaturesByClosureSignature(@NotNull String name, + @NotNull GrClosureSignature signature) { + return generateAllMethodSignaturesByClosureSignature(name, signature, PsiTypeParameter.EMPTY_ARRAY, PsiSubstitutor.EMPTY); + } } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/ConvertMethodToClosureTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/ConvertMethodToClosureTest.java index c43028ad0df7..da8fd998e6e2 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/ConvertMethodToClosureTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/ConvertMethodToClosureTest.java @@ -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); + } + } diff --git a/plugins/groovy/testdata/intentions/convertMethodToClosure/ClosureToMethodWithFieldUsages.groovy b/plugins/groovy/testdata/intentions/convertMethodToClosure/ClosureToMethodWithFieldUsages.groovy new file mode 100644 index 000000000000..5b94e7a1b626 --- /dev/null +++ b/plugins/groovy/testdata/intentions/convertMethodToClosure/ClosureToMethodWithFieldUsages.groovy @@ -0,0 +1,21 @@ +class X { + def foo = {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() + diff --git a/plugins/groovy/testdata/intentions/convertMethodToClosure/ClosureToMethodWithFieldUsages_after.groovy b/plugins/groovy/testdata/intentions/convertMethodToClosure/ClosureToMethodWithFieldUsages_after.groovy new file mode 100644 index 000000000000..240ac376a782 --- /dev/null +++ b/plugins/groovy/testdata/intentions/convertMethodToClosure/ClosureToMethodWithFieldUsages_after.groovy @@ -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() +