From 02740595a9c14ef533b7662a51c4e568fe085e9a Mon Sep 17 00:00:00 2001 From: "Maxim.Medvedev" Date: Tue, 22 May 2012 10:58:23 +0400 Subject: [PATCH] 'cast parameter to type' fixes in Groovy --- .../codeInspection/assignment/GrCastFix.java | 9 +- .../GroovyAssignabilityCheckInspection.java | 55 +++++++++--- .../assignment/ParameterCastFix.java | 84 +++++++++++++++++ .../signatures/GrClosureSignatureUtil.java | 90 +++++++++++++++++++ .../groovy/intentions/ArgumentCastTest.groovy | 59 ++++++++++++ .../intentions/GrIntentionTestCase.groovy | 7 +- .../IncompatibleTypesAssignments.groovy | 2 +- .../highlighting/ReturnAssignability.groovy | 2 +- 8 files changed, 288 insertions(+), 20 deletions(-) create mode 100644 plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/ParameterCastFix.java create mode 100644 plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/ArgumentCastTest.groovy diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GrCastFix.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GrCastFix.java index fdb1c004ecd3..e4cd6be74c27 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GrCastFix.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GrCastFix.java @@ -41,16 +41,19 @@ public class GrCastFix extends GroovyFix implements LocalQuickFix { @Override protected void doFix(Project project, ProblemDescriptor descriptor) throws IncorrectOperationException { - if (!myExpectedType.isValid()) return; + doCast(project, myExpectedType, descriptor.getPsiElement()); + } + + static void doCast(Project project, PsiType type, PsiElement element) { + if (!type.isValid()) return; - final PsiElement element = descriptor.getPsiElement(); if (!(element instanceof GrExpression)) return; final GrExpression expr = (GrExpression)element; final GroovyPsiElementFactory factory = GroovyPsiElementFactory.getInstance(project); final GrSafeCastExpression cast = (GrSafeCastExpression)factory.createExpressionFromText("foo as String"); - final GrTypeElement typeElement = factory.createTypeElement(myExpectedType); + final GrTypeElement typeElement = factory.createTypeElement(type); cast.getOperand().replaceWithExpression(expr, true); cast.getCastTypeElement().replace(typeElement); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java index de6efc0b617b..ff68bf774825 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java @@ -19,9 +19,11 @@ package org.jetbrains.plugins.groovy.codeInspection.assignment; import com.intellij.codeInspection.LocalQuickFix; import com.intellij.codeInspection.ProblemHighlightType; import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.util.Pair; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.Function; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -41,6 +43,8 @@ import org.jetbrains.plugins.groovy.lang.psi.GroovyFile; import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement; import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult; import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.GrListOrMap; +import org.jetbrains.plugins.groovy.lang.psi.api.signatures.GrClosureSignature; +import org.jetbrains.plugins.groovy.lang.psi.api.signatures.GrSignature; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrConstructorInvocation; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable; import org.jetbrains.plugins.groovy.lang.psi.api.statements.arguments.GrArgumentList; @@ -67,6 +71,8 @@ import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; import org.jetbrains.plugins.groovy.lang.resolve.MixinMemberContributor; import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil; +import java.util.ArrayList; +import java.util.List; import java.util.Map; /** @@ -110,16 +116,16 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { } private static class MyVisitor extends BaseInspectionVisitor { - private void checkAssignability(@NotNull PsiType expectedType, @NotNull GrExpression expression, GroovyPsiElement element) { + private void checkAssignability(@NotNull PsiType expectedType, @NotNull GrExpression expression) { if (PsiUtil.isRawClassMemberAccess(expression)) return; //GRVY-2197 - if (checkForImplicitEnumAssigning(expectedType, expression, element)) return; + if (checkForImplicitEnumAssigning(expectedType, expression, expression)) return; final PsiType rType = expression.getType(); if (rType == null || rType == PsiType.VOID) return; - if (!TypesUtil.isAssignable(expectedType, rType, element)) { + if (!TypesUtil.isAssignable(expectedType, rType, expression)) { final LocalQuickFix[] fixes = {new GrCastFix(expectedType)}; final String message = GroovyBundle.message("cannot.assign", rType.getPresentableText(), expectedType.getPresentableText()); - registerError(element, message, fixes, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); + registerError(expression, message, fixes, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); } } @@ -166,7 +172,7 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { if (returnValue != null && !(returnValue.getParent() instanceof GrReturnStatement) && !isNewInstanceInitialingByTuple(returnValue)) { - checkAssignability(expectedType, returnValue, returnValue); + checkAssignability(expectedType, returnValue); } return true; } @@ -189,7 +195,7 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { //don't check if the return type is void. the check is done inside annotator, because it's a compilation error if (expectedType == PsiType.VOID) return; - checkAssignability(expectedType, value, returnStatement); + checkAssignability(expectedType, value); } @Override @@ -215,7 +221,7 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { if (clazz != null && CommonClassNames.JAVA_UTIL_LIST.equals(clazz.getQualifiedName())) { final PsiType[] types = pct.getParameters(); if (types.length == 1 && types[0] != null && rType != null) { - checkAssignability(types[0], rValue, rValue); + checkAssignability(types[0], rValue); } } return; @@ -231,7 +237,7 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { } if (lType != null && rType != null) { - checkAssignability(lType, rValue, rValue); + checkAssignability(lType, rValue); } } @@ -249,7 +255,7 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { return; } - checkAssignability(varType, initializer, initializer); + checkAssignability(varType, initializer); } private static boolean isNewInstanceInitialingByTuple(GrExpression initializer) { @@ -363,8 +369,8 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { final GrExpression exception = throwStatement.getException(); if (exception != null) { - checkAssignability(PsiType.getJavaLangThrowable(throwStatement.getManager(), throwStatement.getResolveScope()), exception, - exception); + checkAssignability(PsiType.getJavaLangThrowable(throwStatement.getManager(), throwStatement.getResolveScope()), exception + ); } } @@ -542,7 +548,7 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { } private void highlightInapplicableMethodUsage(GroovyResolveResult methodResolveResult, - PsiElement place, + GroovyPsiElement place, PsiMethod method, PsiType[] argumentTypes) { final PsiClass containingClass = @@ -563,9 +569,32 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { else { message = GroovyBundle.message("cannot.apply.method1", method.getName(), canonicalText, typesString); } - registerError(getElementToHighlight(place, PsiUtil.getArgumentsList(place)), message); + + registerError(getElementToHighlight(place, PsiUtil.getArgumentsList(place)), message, + genCastFixes(GrClosureSignatureUtil.createSignature(methodResolveResult), argumentTypes, place), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING); } + private static LocalQuickFix[] genCastFixes(GrSignature signature, PsiType[] argumentTypes, GroovyPsiElement context) { + + final List signatures = GrClosureSignatureUtil.generateSimpleSignature(signature); + + List> errors = new ArrayList>(); + for (GrClosureSignature closureSignature : signatures) { + final GrClosureSignatureUtil.MapResultWithError map = + GrClosureSignatureUtil.mapSimpleSignatureWithErrors(closureSignature, argumentTypes, Function.ID, context, 1); + if (map != null) { + errors.addAll(map.getErrors()); + } + } + + final ArrayList fixes = new ArrayList(); + for (Pair error : errors) { + fixes.add(new ParameterCastFix(error.first, error.second)); + } + + return fixes.toArray(new LocalQuickFix[fixes.size()]); + } private boolean checkCallApplicability(PsiType type, GroovyPsiElement invokedExpr, boolean checkUnknownArgs) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/ParameterCastFix.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/ParameterCastFix.java new file mode 100644 index 000000000000..7714be1f6756 --- /dev/null +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/ParameterCastFix.java @@ -0,0 +1,84 @@ +/* + * Copyright 2000-2012 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jetbrains.plugins.groovy.codeInspection.assignment; + +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.openapi.project.Project; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiType; +import com.intellij.util.IncorrectOperationException; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.plugins.groovy.codeInspection.GroovyFix; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.arguments.GrArgumentList; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression; +import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; + +/** + * @author Max Medvedev + */ +public class ParameterCastFix extends GroovyFix { + private final int param; + private final PsiType myType; + private String myName; + + public ParameterCastFix(int param, PsiType type) { + this.param = param; + myType = type; + + StringBuilder builder = new StringBuilder(); + builder.append("Cast "); + + builder.append(param + 1); + switch (param + 1) { + case 1: + builder.append("st"); + break; + case 2: + builder.append("nd"); + break; + case 3: + builder.append("rd"); + break; + default: + builder.append("th"); + break; + } + builder.append(" parameter to ").append(type.getCanonicalText()); + + + myName = builder.toString(); + } + + @Override + protected void doFix(Project project, ProblemDescriptor descriptor) throws IncorrectOperationException { + final PsiElement element = descriptor.getPsiElement(); + final GrArgumentList list = element instanceof GrArgumentList ? (GrArgumentList)element :PsiUtil.getArgumentsList(element); + if (list == null) return; + + final GrExpression[] arguments = list.getExpressionArguments(); + + final int p = list.getNamedArguments().length > 0 ? param - 1 : param; + if (arguments.length <= p) return; + + GrCastFix.doCast(project, myType, arguments[p]); + } + + @NotNull + @Override + public String getName() { + return myName; + } +} diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/signatures/GrClosureSignatureUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/signatures/GrClosureSignatureUtil.java index b2d1341f3b6c..3e22c2ebb6be 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/signatures/GrClosureSignatureUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/signatures/GrClosureSignatureUtil.java @@ -847,4 +847,94 @@ public class GrClosureSignatureUtil { return null; } + + + public static class MapResultWithError { + private final ArgInfo[] mapping; + private final List> errorsAndExpectedType; + + public MapResultWithError(ArgInfo[] mapping, List> errorsAndExpectedType) { + this.mapping = mapping; + this.errorsAndExpectedType = errorsAndExpectedType; + } + + public ArgInfo[] getMapping() { + return mapping; + } + + public List> getErrors() { + return errorsAndExpectedType; + } + } + + @Nullable + public static MapResultWithError mapSimpleSignatureWithErrors(@NotNull GrClosureSignature signature, + @NotNull Arg[] args, + @NotNull Function typeComputer, + @NotNull GroovyPsiElement context, + int maxErrorCount) { + final GrClosureParameter[] params = signature.getParameters(); + if (args.length < params.length) return null; + + if (args.length > params.length && !signature.isVarargs()) return null; + + int optional = getOptionalParamCount(params, false); + assert optional == 0; + + int errorCount = 0; + ArgInfo[] map = new ArgInfo[params.length]; + + List> errors = new ArrayList>(maxErrorCount); + + for (int i = 0; i < params.length; i++) { + if (isAssignableByConversion(params[i].getType(), typeComputer.fun(args[i]), context)) { + map[i] = new ArgInfo(args[i]); + } + else if (params[i].getType() instanceof PsiArrayType && i == params.length - 1) { + if (i + 1 == args.length) { + errors.add(new Pair(i, params[i].getType())); + } + final PsiType ellipsis = ((PsiArrayType)params[i].getType()).getComponentType(); + for (int j = i; j < args.length; j++) { + if (!isAssignableByConversion(ellipsis, typeComputer.fun(args[j]), context)) { + errorCount++; + if (errorCount > maxErrorCount) return null; + errors.add(new Pair(i, ellipsis)); + } + map[i] = new ArgInfo(args[i]); + } + } + else { + errorCount++; + if (errorCount > maxErrorCount) return null; + errors.add(new Pair(i, params[i].getType())); + } + } + return new MapResultWithError(map, errors); + } + + public static List generateSimpleSignature(GrSignature signature) { + final List result = new ArrayList(); + signature.accept(new GrRecursiveSignatureVisitor() { + @Override + public void visitClosureSignature(GrClosureSignature signature) { + final GrClosureParameter[] original = signature.getParameters(); + final ArrayList parameters = new ArrayList(original.length); + + for (GrClosureParameter parameter : original) { + parameters.add(new GrClosureParameterImpl(parameter.getType(), false, null)); + } + + final int pcount = signature.isVarargs() ? signature.getParameterCount() - 2 : signature.getParameterCount() - 1; + for (int i = pcount; i >= 0; i--) { + if (original[i].isOptional()) { + result.add(new GrClosureSignatureImpl(parameters.toArray(new GrClosureParameter[parameters.size()]), signature.getReturnType(), signature.isVarargs(), false)); + parameters.remove(i); + } + } + result.add(new GrClosureSignatureImpl(parameters.toArray(new GrClosureParameter[parameters.size()]), signature.getReturnType(), signature.isVarargs(), false)); + } + }); + return result; + } } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/ArgumentCastTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/ArgumentCastTest.groovy new file mode 100644 index 000000000000..df598c39c8c5 --- /dev/null +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/ArgumentCastTest.groovy @@ -0,0 +1,59 @@ +/* + * Copyright 2000-2012 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jetbrains.plugins.groovy.intentions + +import org.jetbrains.plugins.groovy.codeInspection.assignment.GroovyAssignabilityCheckInspection + +/** + * @author Max Medvedev + */ +class ArgumentCastTest extends GrIntentionTestCase { + void test0() { + doTextTest('''\ +def foo(char c) {} +foo('a') +''', "Cast 1st parameter to char", '''\ +def foo(char c) {} +foo('a' as char) +''', GroovyAssignabilityCheckInspection) + } + + void test1() { + doTextTest('''\ +def foo(char c, int x) {} +foo('a', 2) +''', "Cast 1st parameter to char", '''\ +def foo(char c, int x) {} +foo('a' as char, 2) +''', GroovyAssignabilityCheckInspection) + } + + void test2() { + doAntiTest('''\ +def foo(char c, int x) {} + +foo('a', 2, 3) +''', "Cast 1st parameter to char", GroovyAssignabilityCheckInspection) + } + + void test3() { + doAntiTest('''\ +def foo(char c, int x) {} + +foo('a', 'a') +''', "Cast 1st parameter to char", GroovyAssignabilityCheckInspection) + } +} diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/GrIntentionTestCase.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/GrIntentionTestCase.groovy index 1534b0179da6..becb31f9c61e 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/GrIntentionTestCase.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/GrIntentionTestCase.groovy @@ -18,6 +18,7 @@ package org.jetbrains.plugins.groovy.intentions; import com.intellij.codeInsight.intention.IntentionAction +import com.intellij.codeInspection.LocalInspectionTool import com.intellij.psi.impl.source.PostprocessReformattingAspect import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase @@ -44,16 +45,18 @@ public abstract class GrIntentionTestCase extends LightCodeInsightFixtureTestCas } } - protected void doTextTest(String before, String hint, String after) { + protected void doTextTest(String before, String hint, String after, Class... inspections) { myFixture.configureByText("a.groovy", before); + myFixture.enableInspections(inspections) final List list = myFixture.filterAvailableIntentions(hint); myFixture.launchAction(assertOneElement(list)); PostprocessReformattingAspect.getInstance(project).doPostponedFormatting(); myFixture.checkResult(after); } - protected void doAntiTest(String before, String hint) { + protected void doAntiTest(String before, String hint, Class... inspections) { myFixture.configureByText("a.groovy", before); + myFixture.enableInspections(inspections) assertEmpty(myFixture.filterAvailableIntentions(hint)); } } diff --git a/plugins/groovy/testdata/highlighting/IncompatibleTypesAssignments.groovy b/plugins/groovy/testdata/highlighting/IncompatibleTypesAssignments.groovy index ec3ab80efe0d..43eec020a3ac 100644 --- a/plugins/groovy/testdata/highlighting/IncompatibleTypesAssignments.groovy +++ b/plugins/groovy/testdata/highlighting/IncompatibleTypesAssignments.groovy @@ -1,6 +1,6 @@ class X{ int method1(Date date) { - return date; + return date; } int method2(Date date) { diff --git a/plugins/groovy/testdata/highlighting/ReturnAssignability.groovy b/plugins/groovy/testdata/highlighting/ReturnAssignability.groovy index 7a8991cc01ad..d4f2e02df3f6 100644 --- a/plugins/groovy/testdata/highlighting/ReturnAssignability.groovy +++ b/plugins/groovy/testdata/highlighting/ReturnAssignability.groovy @@ -3,7 +3,7 @@ File foo() { if (ints.empty) { print {return 42} for (x in ints) { - return 43 + return 43 } } 67