From 68c017caef18576d41ca661ede994fa54d187876 Mon Sep 17 00:00:00 2001 From: "Maxim.Medvedev" Date: Mon, 2 Jul 2012 14:41:57 +0400 Subject: [PATCH] IDEA-87991 Static type checking false negative ignoring automatic type conversion --- .../internal/FileEqualsUsageInspection.java | 2 +- .../com/intellij/psi/CommonClassNames.java | 1 + plugins/groovy/src/META-INF/plugin.xml | 1 + .../groovy/lang/psi/GrTypeConverter.java | 18 +++++++ .../psi/impl/types/GrStringTypeConverter.java | 48 +++++++++++++++++++ .../ClosureParameterEnhancer.java | 17 +++---- .../lang/psi/util/GroovyCommonClassNames.java | 1 + .../groovy/lang/GppFunctionalTest.groovy | 4 +- 8 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrStringTypeConverter.java diff --git a/java/java-impl/src/com/intellij/codeInspection/internal/FileEqualsUsageInspection.java b/java/java-impl/src/com/intellij/codeInspection/internal/FileEqualsUsageInspection.java index f9f5756581ba..08325f5c32cc 100644 --- a/java/java-impl/src/com/intellij/codeInspection/internal/FileEqualsUsageInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/internal/FileEqualsUsageInspection.java @@ -55,7 +55,7 @@ public class FileEqualsUsageInspection extends InternalInspection { if (clazz == null) return; String methodName = method.getName(); - if ("java.io.File".equals(clazz.getQualifiedName()) + if (CommonClassNames.JAVA_IO_FILE.equals(clazz.getQualifiedName()) && ("equals".equals(methodName) || "compareTo".equals(methodName) || "hashCode".equals(methodName))) { holder.registerProblem(methodExpression, "Do not use File.equals/hashCode/compareTo as they don't honor case-sensitivity on MacOS. Use FileUtil.filesEquals/fileHashCode/compareFiles instead", diff --git a/java/java-psi-api/src/com/intellij/psi/CommonClassNames.java b/java/java-psi-api/src/com/intellij/psi/CommonClassNames.java index 95d78fafed51..9a7c93449210 100644 --- a/java/java-psi-api/src/com/intellij/psi/CommonClassNames.java +++ b/java/java-psi-api/src/com/intellij/psi/CommonClassNames.java @@ -87,4 +87,5 @@ public interface CommonClassNames { @NonNls String JAVA_LANG_INVOKE_MH_POLYMORPHIC = "java.lang.invoke.MethodHandle.PolymorphicSignature"; String TARGET_ANNOTATION_FQ_NAME = "java.lang.annotation.Target"; @NonNls String JAVA_LANG_RUNNABLE = "java.lang.Runnable"; + @NonNls String JAVA_IO_FILE = "java.io.File"; } diff --git a/plugins/groovy/src/META-INF/plugin.xml b/plugins/groovy/src/META-INF/plugin.xml index e956617d9901..d76c850c98e0 100644 --- a/plugins/groovy/src/META-INF/plugin.xml +++ b/plugins/groovy/src/META-INF/plugin.xml @@ -126,6 +126,7 @@ + diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/GrTypeConverter.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/GrTypeConverter.java index aad90873f1a1..07961dd39581 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/GrTypeConverter.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/GrTypeConverter.java @@ -1,6 +1,8 @@ package org.jetbrains.plugins.groovy.lang.psi; import com.intellij.openapi.extensions.ExtensionPointName; +import com.intellij.psi.PsiClass; +import com.intellij.psi.PsiClassType; import com.intellij.psi.PsiType; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -19,4 +21,20 @@ public abstract class GrTypeConverter { @Nullable public abstract Boolean isConvertible(@NotNull PsiType lType, @NotNull PsiType rType, @NotNull GroovyPsiElement context); + protected static boolean resolvesTo(PsiType type, String fqn) { + if (type instanceof PsiClassType) { + final PsiClass resolved = ((PsiClassType)type).resolve(); + return resolved != null && fqn.equals(resolved.getQualifiedName()); + } + return false; + } + + protected static boolean isEnum(PsiType type) { + if (type instanceof PsiClassType) { + final PsiClass resolved = ((PsiClassType)type).resolve(); + return resolved != null && resolved.isEnum(); + } + + return false; + } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrStringTypeConverter.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrStringTypeConverter.java new file mode 100644 index 000000000000..d5c9b30e8392 --- /dev/null +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/types/GrStringTypeConverter.java @@ -0,0 +1,48 @@ +/* + * 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.lang.psi.impl.types; + +import com.intellij.psi.PsiType; +import com.intellij.psi.util.InheritanceUtil; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.plugins.groovy.config.GroovyConfigUtils; +import org.jetbrains.plugins.groovy.lang.psi.GrTypeConverter; +import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement; + +import static com.intellij.psi.CommonClassNames.JAVA_LANG_BOOLEAN; +import static com.intellij.psi.CommonClassNames.JAVA_LANG_CLASS; +import static org.jetbrains.plugins.groovy.lang.psi.util.GroovyCommonClassNames.GROOVY_LANG_GSTRING; +import static org.jetbrains.plugins.groovy.lang.psi.util.GroovyCommonClassNames.JAVA_LANG_CHAR_SEQUENCE; + +/** + * @author Max Medvedev + */ +public class GrStringTypeConverter extends GrTypeConverter { + @Override + public Boolean isConvertible(@NotNull PsiType lType, @NotNull PsiType rType, @NotNull GroovyPsiElement context) { + if (isMethodCallConversion(context)) return null; + if (!GroovyConfigUtils.getInstance().isVersionAtLeast(context, GroovyConfigUtils.GROOVY1_8)) return null; + if (!(InheritanceUtil.isInheritor(rType, JAVA_LANG_CHAR_SEQUENCE) || InheritanceUtil.isInheritor(rType, GROOVY_LANG_GSTRING))) { + return null; + } + + if (lType == PsiType.BOOLEAN || resolvesTo(lType, JAVA_LANG_BOOLEAN)) return true; + if (resolvesTo(lType, JAVA_LANG_CLASS)) return true; + if (isEnum(lType)) return true; + + return null; + } +} diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/typeEnhancers/ClosureParameterEnhancer.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/typeEnhancers/ClosureParameterEnhancer.java index 2e4e919fecb8..8749f37ee5d2 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/typeEnhancers/ClosureParameterEnhancer.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/typeEnhancers/ClosureParameterEnhancer.java @@ -23,6 +23,7 @@ import org.jetbrains.plugins.groovy.lang.psi.util.GroovyCommonClassNames; import java.util.Map; import java.util.Set; +import static com.intellij.psi.CommonClassNames.JAVA_IO_FILE; import static org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil.skipParentheses; /** @@ -44,13 +45,13 @@ public class ClosureParameterEnhancer extends AbstractClosureParameterEnhancer { simpleTypes.put("withDataOutputStream", "java.io.DataOutputStream"); simpleTypes.put("withDataInputStream", "java.io.DataInputStream"); simpleTypes.put("eachLine", "java.lang.String"); - simpleTypes.put("eachFile", "java.io.File"); - simpleTypes.put("eachDir", "java.io.File"); - simpleTypes.put("eachFileRecurse", "java.io.File"); - simpleTypes.put("traverse", "java.io.File"); - simpleTypes.put("eachDirRecurse", "java.io.File"); - simpleTypes.put("eachFileMatch", "java.io.File"); - simpleTypes.put("eachDirMatch", "java.io.File"); + simpleTypes.put("eachFile", JAVA_IO_FILE); + simpleTypes.put("eachDir", JAVA_IO_FILE); + simpleTypes.put("eachFileRecurse", JAVA_IO_FILE); + simpleTypes.put("traverse", JAVA_IO_FILE); + simpleTypes.put("eachDirRecurse", JAVA_IO_FILE); + simpleTypes.put("eachFileMatch", JAVA_IO_FILE); + simpleTypes.put("eachDirMatch", JAVA_IO_FILE); simpleTypes.put("withReader", "java.io.Reader"); simpleTypes.put("withWriter", "java.io.Writer"); simpleTypes.put("withWriterAppend", "java.io.Writer"); @@ -259,7 +260,7 @@ public class ClosureParameterEnhancer extends AbstractClosureParameterEnhancer { return res; } - if (TypesUtil.isClassType(iterType, CommonClassNames.JAVA_LANG_STRING) || TypesUtil.isClassType(iterType, "java.io.File")) { + if (TypesUtil.isClassType(iterType, CommonClassNames.JAVA_LANG_STRING) || TypesUtil.isClassType(iterType, JAVA_IO_FILE)) { return TypesUtil.createTypeByFQClassName(CommonClassNames.JAVA_LANG_STRING, context); } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GroovyCommonClassNames.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GroovyCommonClassNames.java index d939e0f36379..3c481fe510da 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GroovyCommonClassNames.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GroovyCommonClassNames.java @@ -54,6 +54,7 @@ public final class GroovyCommonClassNames { @NonNls public static final String GROOVY_TRANSFORM_COMPILE_STATIC = "groovy.transform.CompileStatic"; @NonNls public static final String GROOVY_TRANSFORM_TYPE_CHECKED = "groovy.transform.TypeChecked"; @NonNls public static final String GROOVY_TRANSFORM_TYPE_CHECKING_MODE = "groovy.transform.TypeCheckingMode"; + @NonNls public static final String JAVA_LANG_CHAR_SEQUENCE = "java.lang.CharSequence"; private GroovyCommonClassNames() { diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GppFunctionalTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GppFunctionalTest.groovy index dc92c267363d..5b9058c1e562 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GppFunctionalTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GppFunctionalTest.groovy @@ -604,11 +604,11 @@ println new Bar().zzz } class GppProjectDescriptor extends DefaultLightProjectDescriptor { - static def instance = new GppProjectDescriptor() + public static final instance = new GppProjectDescriptor() @Override public void configureModule(Module module, ModifiableRootModel model, ContentEntry contentEntry) { - final Library.ModifiableModel modifiableModel = model.getModuleLibraryTable().createLibrary("GROOVY++").getModifiableModel(); + final Library.ModifiableModel modifiableModel = model.moduleLibraryTable.createLibrary("GROOVY++").modifiableModel; modifiableModel.addRoot(JarFileSystem.instance.refreshAndFindFileByPath(TestUtils.absoluteTestDataPath + "mockGroovypp/groovypp-0.9.0_1.8.2.jar!/"), OrderRootType.CLASSES) modifiableModel.addRoot(JarFileSystem.instance.refreshAndFindFileByPath(TestUtils.mockGroovy1_7LibraryName + "!/"), OrderRootType.CLASSES); modifiableModel.commit();