From ce6d733ecafb68b4491ecdb01a7d38355b4e16cf Mon Sep 17 00:00:00 2001 From: Maxim Medvedev Date: Mon, 22 Aug 2011 13:13:28 +0400 Subject: [PATCH] IDEA-69167 Groovy 1.8: support implicit (G)String to Enum coercion --- .../plugins/groovy/GroovyBundle.properties | 4 ++- .../codeInspection/BaseInspectionVisitor.java | 27 +++++++++------ .../GroovyAssignabilityCheckInspection.java | 33 +++++++++++++++++++ .../groovy/lang/psi/impl/PsiImplUtil.java | 33 +++++++++---------- .../groovy/lang/Groovy16HighlightingTest.java | 6 +++- .../groovy/lang/GroovyHighlightingTest.java | 8 +++-- .../groovy/lang/MissingReturnTest.java | 2 +- .../highlighting/ImplicitEnumCoercion.groovy | 9 +++++ .../ImplicitEnumCoercion1_6.groovy | 5 +++ 9 files changed, 95 insertions(+), 32 deletions(-) create mode 100644 plugins/groovy/testdata/highlighting/ImplicitEnumCoercion.groovy create mode 100644 plugins/groovy/testdata/highlighting/ImplicitEnumCoercion1_6.groovy diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties b/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties index bdf233a4f37e..8edeff8237d2 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties @@ -350,4 +350,6 @@ duplicated.named.parameter=Duplicated named parameter ''{0}'' found cannot.find.method.call=No signature of method: {0}.call() is applicable for {1} no.super.classes.found=No super classes found no.super.method.found=No super methods found -wrong.package.name=Package name ''{0}'' does not corresponding to the file path ''{1}'' \ No newline at end of file +wrong.package.name=Package name ''{0}'' does not corresponding to the file path ''{1}'' +cannot.assign.string.to.enum.0=Cannot assign string to enum ''{0}'' +cannot.find.enum.constant.0.in.enum.1=Cannot find enum constant ''{0}'' in enum ''{1}'' \ No newline at end of file diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspectionVisitor.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspectionVisitor.java index 5b86dd411584..1cc6d367769c 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspectionVisitor.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspectionVisitor.java @@ -90,7 +90,7 @@ public abstract class BaseInspectionVisitor extends GroovyRecursiveElementVisito } final LocalQuickFix[] fix = createFixes(location); final String description = inspection.buildErrorString(location); - registerError(location, description, fix); + registerError(location, description, fix, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); } protected void registerMethodError(GrMethod method, Object... args) { @@ -99,7 +99,7 @@ public abstract class BaseInspectionVisitor extends GroovyRecursiveElementVisito } final LocalQuickFix[] fix = createFixes(method); final String description = inspection.buildErrorString(args); - registerError(method.getNameIdentifierGroovy(), description, fix); + registerError(method.getNameIdentifierGroovy(), description, fix, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); } protected void registerVariableError(GrVariable variable, Object... args) { @@ -108,7 +108,7 @@ public abstract class BaseInspectionVisitor extends GroovyRecursiveElementVisito } final LocalQuickFix[] fix = createFixes(variable); final String description = inspection.buildErrorString(args); - registerError(variable.getNameIdentifierGroovy(), description, fix); + registerError(variable.getNameIdentifierGroovy(), description, fix, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); } protected void registerMethodCallError(GrMethodCallExpression method, Object... args) { @@ -117,20 +117,27 @@ public abstract class BaseInspectionVisitor extends GroovyRecursiveElementVisito } final LocalQuickFix[] fix = createFixes(method); final String description = inspection.buildErrorString(args); - registerError(((GrReferenceExpression) method.getInvokedExpression()).getReferenceNameElement(), description, fix); + registerError(((GrReferenceExpression) method.getInvokedExpression()).getReferenceNameElement(), description, fix, + ProblemHighlightType.GENERIC_ERROR_OR_WARNING); } - private void registerError(@NotNull PsiElement location, String description, - LocalQuickFix[] fixes) { - problemsHolder.registerProblem(location, - description, - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, fixes); + private void registerError(@NotNull PsiElement location, + String description, + LocalQuickFix[] fixes, + ProblemHighlightType highlightType) { + problemsHolder.registerProblem(location, description, highlightType, fixes); } protected void registerError(@NotNull PsiElement location, Object... args) { + registerError(location, ProblemHighlightType.GENERIC_ERROR_OR_WARNING, args); + } + + protected void registerError(@NotNull PsiElement location, + ProblemHighlightType highlightType, + Object... args) { final LocalQuickFix[] fix = createFixes(location); final String description = inspection.buildErrorString(args); - registerError(location, description, fix); + registerError(location, description, fix, highlightType); } @Nullable 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 c8d46b33ba89..b62bcc12e3f5 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 @@ -16,6 +16,7 @@ package org.jetbrains.plugins.groovy.codeInspection.assignment; +import com.intellij.codeInspection.ProblemHighlightType; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; @@ -27,6 +28,7 @@ import org.jetbrains.plugins.groovy.annotator.GroovyAnnotator; import org.jetbrains.plugins.groovy.codeInspection.BaseInspection; import org.jetbrains.plugins.groovy.codeInspection.BaseInspectionVisitor; import org.jetbrains.plugins.groovy.codeInspection.utils.ControlFlowUtils; +import org.jetbrains.plugins.groovy.config.GroovyConfigUtils; import org.jetbrains.plugins.groovy.extensions.GroovyNamedArgumentProvider; import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes; import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement; @@ -51,6 +53,7 @@ import org.jetbrains.plugins.groovy.lang.psi.controlFlow.Instruction; import org.jetbrains.plugins.groovy.lang.psi.impl.GrClosureType; import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.TypesUtil; import org.jetbrains.plugins.groovy.lang.psi.util.GroovyCommonClassNames; +import org.jetbrains.plugins.groovy.lang.psi.util.GroovyConstantExpressionEvaluator; import org.jetbrains.plugins.groovy.lang.psi.util.GroovyPropertyUtils; import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil; import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil; @@ -93,6 +96,7 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { private static class MyVisitor extends BaseInspectionVisitor { private void checkAssignability(@NotNull PsiType expectedType, @NotNull GrExpression expression, GroovyPsiElement element) { if (PsiUtil.isRawClassMemberAccess(expression)) return; //GRVY-2197 + if (checkForImplicitEnumAssigning(expectedType, expression, element)) return; final PsiType rType = expression.getType(); if (rType == null || rType == PsiType.VOID) return; if (!TypesUtil.isAssignable(expectedType, rType, element)) { @@ -100,6 +104,35 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { } } + private boolean checkForImplicitEnumAssigning(PsiType expectedType, GrExpression expression, GroovyPsiElement element) { + if (!(expectedType instanceof PsiClassType)) return false; + + if (!GroovyConfigUtils.getInstance().isVersionAtLeast(element, GroovyConfigUtils.GROOVY1_8)) return false; + + final PsiClass resolved = ((PsiClassType)expectedType).resolve(); + if (resolved == null || !resolved.isEnum()) return false; + + final PsiType type = expression.getType(); + if (type == null) return false; + + if (!type.equalsToText(GroovyCommonClassNames.GROOVY_LANG_GSTRING) && !type.equalsToText(CommonClassNames.JAVA_LANG_STRING)) { + return false; + } + + final Object result = GroovyConstantExpressionEvaluator.evaluate(expression); + if (result == null || !(result instanceof String)) { + registerError(element, ProblemHighlightType.WEAK_WARNING, + GroovyBundle.message("cannot.assign.string.to.enum.0", expectedType.getPresentableText())); + } + else { + final PsiField field = resolved.findFieldByName((String)result, true); + if (!(field instanceof PsiEnumConstant)) { + registerError(element, GroovyBundle.message("cannot.find.enum.constant.0.in.enum.1", result, expectedType.getPresentableText())); + } + } + return true; + } + //isApplicable last expression on method body @Override public void visitOpenBlock(GrOpenBlock block) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java index f4cb20a5791e..a0beaff9b89c 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/PsiImplUtil.java @@ -191,25 +191,24 @@ public class PsiImplUtil { @Nullable public static GrExpression getRuntimeQualifier(GrReferenceExpression refExpr) { GrExpression qualifier = refExpr.getQualifierExpression(); - if (qualifier == null) { - GrClosableBlock closure = PsiTreeUtil.getParentOfType(refExpr, GrClosableBlock.class); - while (closure != null) { - PsiElement parent = closure.getParent(); - if (parent instanceof GrArgumentList) parent = parent.getParent(); - if (parent instanceof GrMethodCall) { - GrExpression funExpr = ((GrMethodCall)parent).getInvokedExpression(); - if (funExpr instanceof GrReferenceExpression && ((GrReferenceExpression)funExpr).resolve() instanceof PsiMethod) { - qualifier = ((GrReferenceExpression) funExpr).getQualifierExpression(); - if (qualifier != null) { - return qualifier; - } - } - else { - return funExpr; + if (qualifier != null) return qualifier; + + for (GrClosableBlock closure = PsiTreeUtil.getParentOfType(refExpr, GrClosableBlock.class); + closure != null; + closure = PsiTreeUtil.getParentOfType(closure, GrClosableBlock.class)) { + PsiElement parent = closure.getParent(); + if (parent instanceof GrArgumentList) parent = parent.getParent(); + if (parent instanceof GrMethodCall) { + GrExpression funExpr = ((GrMethodCall)parent).getInvokedExpression(); + if (funExpr instanceof GrReferenceExpression && ((GrReferenceExpression)funExpr).resolve() instanceof PsiMethod) { + qualifier = ((GrReferenceExpression)funExpr).getQualifierExpression(); + if (qualifier != null) { + return qualifier; } } - - closure = PsiTreeUtil.getParentOfType(closure, GrClosableBlock.class); + else { + return funExpr; + } } } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/Groovy16HighlightingTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/Groovy16HighlightingTest.java index 497425b225bf..5ac9fa880e45 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/Groovy16HighlightingTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/Groovy16HighlightingTest.java @@ -17,6 +17,7 @@ package org.jetbrains.plugins.groovy.lang; import com.intellij.codeInspection.LocalInspectionTool; import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; +import org.jetbrains.plugins.groovy.codeInspection.assignment.GroovyAssignabilityCheckInspection; import org.jetbrains.plugins.groovy.util.TestUtils; /** @@ -29,7 +30,7 @@ public class Groovy16HighlightingTest extends LightCodeInsightFixtureTestCase { return TestUtils.getTestDataPath() + "highlighting/"; } - private void doTest(LocalInspectionTool... tools) throws Exception { + private void doTest(LocalInspectionTool... tools) { myFixture.enableInspections(tools); myFixture.testHighlighting(true, false, false, getTestName(false) + ".groovy"); } @@ -37,4 +38,7 @@ public class Groovy16HighlightingTest extends LightCodeInsightFixtureTestCase { public void testInnerEnum() throws Exception {doTest();} public void testSuperWithNotEnclosingClass() throws Throwable {doTest();} public void testThisWithWrongQualifier() throws Throwable {doTest();} + + public void testImplicitEnumCoercion1_6() { + doTest(new GroovyAssignabilityCheckInspection());} } \ No newline at end of file diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java index 47bd821f4379..bdf19f7f146f 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java @@ -40,7 +40,7 @@ import java.io.IOException; * @author peter */ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase { - public static final DefaultLightProjectDescriptor GROOVY_17_PROJECT_DESCRIPTOR = new DefaultLightProjectDescriptor() { + public static final DefaultLightProjectDescriptor GROOVY_18_PROJECT_DESCRIPTOR = new DefaultLightProjectDescriptor() { @Override public void configureModule(Module module, ModifiableRootModel model, ContentEntry contentEntry) { final Library.ModifiableModel modifiableModel = model.getModuleLibraryTable().createLibrary("GROOVY").getModifiableModel(); @@ -59,7 +59,7 @@ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase { @NotNull @Override protected LightProjectDescriptor getProjectDescriptor() { - return GROOVY_17_PROJECT_DESCRIPTOR; + return GROOVY_18_PROJECT_DESCRIPTOR; } public void testDuplicateClosurePrivateVariable() throws Throwable { @@ -444,4 +444,8 @@ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase { public void testUnknownVarInArgList() { doTest(new GroovyAssignabilityCheckInspection()); } + + public void testImplicitEnumCoercion() { + doTest(new GroovyAssignabilityCheckInspection()); + } } \ No newline at end of file diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/MissingReturnTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/MissingReturnTest.java index 03cf8a98b335..53edcf51542f 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/MissingReturnTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/MissingReturnTest.java @@ -19,7 +19,7 @@ public class MissingReturnTest extends LightCodeInsightFixtureTestCase { @NotNull @Override protected LightProjectDescriptor getProjectDescriptor() { - return GroovyHighlightingTest.GROOVY_17_PROJECT_DESCRIPTOR; + return GroovyHighlightingTest.GROOVY_18_PROJECT_DESCRIPTOR; } public void testMissingReturnWithLastLoop() throws Throwable { doTest(); } diff --git a/plugins/groovy/testdata/highlighting/ImplicitEnumCoercion.groovy b/plugins/groovy/testdata/highlighting/ImplicitEnumCoercion.groovy new file mode 100644 index 000000000000..a1bc879d0237 --- /dev/null +++ b/plugins/groovy/testdata/highlighting/ImplicitEnumCoercion.groovy @@ -0,0 +1,9 @@ +enum My { + foo, bar +} + +My var = 'foo' +var = 'fail' + +var = "fo"+"o" +var="fo${'o'}" diff --git a/plugins/groovy/testdata/highlighting/ImplicitEnumCoercion1_6.groovy b/plugins/groovy/testdata/highlighting/ImplicitEnumCoercion1_6.groovy new file mode 100644 index 000000000000..f4e340f02bb8 --- /dev/null +++ b/plugins/groovy/testdata/highlighting/ImplicitEnumCoercion1_6.groovy @@ -0,0 +1,5 @@ +enum My { + foo, bar +} + +My var = 'foo'