From 1163a8e71793c348ab3ad485a27e85b2343dde70 Mon Sep 17 00:00:00 2001 From: peter Date: Fri, 24 Feb 2017 15:26:35 +0100 Subject: [PATCH] support type inference with nullity annotations across different language levels --- .../NullityAnnotationModifier.java | 33 ++++---- .../codeInsight/NullableNotNullManager.java | 9 ++- .../psi/augment/TypeAnnotationModifier.java | 26 ++++-- .../graphInference/InferenceSession.java | 2 +- .../graphInference/InferenceVariable.java | 23 +++--- .../DataFlowInspection8Test.java | 14 +++- .../DataFlowInspectionHeavyTest.groovy | 81 +++++++++++++++++++ .../DataFlowInspectionTestSuite.java | 1 + 8 files changed, 154 insertions(+), 35 deletions(-) create mode 100644 java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionHeavyTest.groovy diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/NullityAnnotationModifier.java b/java/java-analysis-impl/src/com/intellij/codeInsight/NullityAnnotationModifier.java index f289e22a6c47..80029bfb02cd 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/NullityAnnotationModifier.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/NullityAnnotationModifier.java @@ -16,7 +16,6 @@ package com.intellij.codeInsight; import com.intellij.psi.PsiAnnotation; -import com.intellij.psi.PsiClassType; import com.intellij.psi.PsiType; import com.intellij.psi.TypeAnnotationProvider; import com.intellij.psi.augment.TypeAnnotationModifier; @@ -32,11 +31,12 @@ import java.util.List; public class NullityAnnotationModifier extends TypeAnnotationModifier { @Nullable @Override - public TypeAnnotationProvider modifyAnnotations(@NotNull PsiType inferenceVariableType, @NotNull PsiClassType boundType) { + public TypeAnnotationProvider boundAppeared(@NotNull PsiType inferenceVariableType, @NotNull PsiType boundType) { PsiAnnotation[] annotations = inferenceVariableType.getAnnotations(); for (PsiAnnotation annotation : annotations) { String qName = annotation.getQualifiedName(); - if (qName != null && isMatchingAnnotation(boundType, annotation, qName)) { + if (qName != null && (NullableNotNullManager.isNullableAnnotation(annotation) || NullableNotNullManager + .isNotNullAnnotation(annotation)) && boundType.findAnnotation(qName) != null) { return removeAnnotation(annotations, annotation); } } @@ -44,20 +44,25 @@ public class NullityAnnotationModifier extends TypeAnnotationModifier { return null; } + @Nullable + @Override + public TypeAnnotationProvider modifyLowerBoundAnnotations(@NotNull PsiType lowerBound, @NotNull PsiType upperBound) { + PsiAnnotation[] lowerAnnotations = lowerBound.getAnnotations(); + PsiAnnotation nullable = findNullable(lowerAnnotations); + if (nullable != null && findNullable(upperBound.getAnnotations()) == null) { + return removeAnnotation(lowerAnnotations, nullable); + } + return null; + } + + private static PsiAnnotation findNullable(PsiAnnotation[] annotations) { + return ContainerUtil.find(annotations, NullableNotNullManager::isNullableAnnotation); + } + @NotNull private static TypeAnnotationProvider removeAnnotation(PsiAnnotation[] annotations, PsiAnnotation annotation) { List list = ContainerUtil.newArrayList(annotations); list.remove(annotation); - if (list.isEmpty()) { - return TypeAnnotationProvider.EMPTY; - } - - PsiAnnotation[] array = list.toArray(PsiAnnotation.EMPTY_ARRAY); - return () -> array; - } - - private static boolean isMatchingAnnotation(@NotNull PsiClassType boundType, PsiAnnotation annotation, String qName) { - NullableNotNullManager manager = NullableNotNullManager.getInstance(annotation.getProject()); - return (manager.getNullables().contains(qName) || manager.getNotNulls().contains(qName)) && boundType.findAnnotation(qName) != null; + return TypeAnnotationProvider.Static.create(list.toArray(PsiAnnotation.EMPTY_ARRAY)); } } diff --git a/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java b/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java index a76f94a18aae..8079d6d8fe09 100644 --- a/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java +++ b/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java @@ -18,7 +18,6 @@ package com.intellij.codeInsight; import com.intellij.openapi.components.ServiceManager; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.Computable; import com.intellij.openapi.util.JDOMExternalizableStringList; import com.intellij.openapi.util.RecursionManager; import com.intellij.psi.*; @@ -357,4 +356,12 @@ public abstract class NullableNotNullManager { } public abstract List getPredefinedNotNulls(); + + public static boolean isNullableAnnotation(@NotNull PsiAnnotation annotation) { + return getInstance(annotation.getProject()).getNullables().contains(annotation.getQualifiedName()); + } + + public static boolean isNotNullAnnotation(@NotNull PsiAnnotation annotation) { + return getInstance(annotation.getProject()).getNotNulls().contains(annotation.getQualifiedName()); + } } \ No newline at end of file diff --git a/java/java-psi-api/src/com/intellij/psi/augment/TypeAnnotationModifier.java b/java/java-psi-api/src/com/intellij/psi/augment/TypeAnnotationModifier.java index 962d725c2daa..f9eb50e54189 100644 --- a/java/java-psi-api/src/com/intellij/psi/augment/TypeAnnotationModifier.java +++ b/java/java-psi-api/src/com/intellij/psi/augment/TypeAnnotationModifier.java @@ -16,20 +16,22 @@ package com.intellij.psi.augment; import com.intellij.openapi.extensions.ExtensionPointName; -import com.intellij.psi.PsiClassType; import com.intellij.psi.PsiType; import com.intellij.psi.TypeAnnotationProvider; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +/** + * Type annotations are ignored during inference process. When they are present on types which are bounds of the inference variables, + * then the corresponding instantiations of inference variables would contain those type annotations. + * If different bounds contain contradicting type annotations or type annotations on types repeat target type annotations, + * it could be useful to ignore such annotations in the resulted instantiation. + */ public abstract class TypeAnnotationModifier { public static final ExtensionPointName EP_NAME = ExtensionPointName.create("com.intellij.lang.psiTypeAnnotationModifier"); /** - * Type annotations are ignored during inference process. When they are present on types which are bounds of the inference variables, - * then the corresponding instantiations of inference variables would contain that type annotations. - * If different bounds contain contradicting type annotations or type annotations on types repeat target type annotations, - * it could be useful to ignore such annotations in the resulted instantiation. + * Called when a new bound is added to an inference variable. Implementations may adjust boundType's annotations based on the inference variable's type annotations. * @param inferenceVariableType target type * @param boundType bound which annotations should be changed according to present annotations @@ -37,6 +39,18 @@ public abstract class TypeAnnotationModifier { * @return provider based on modified annotations or null if no applicable annotations found */ @Nullable - public abstract TypeAnnotationProvider modifyAnnotations(@NotNull PsiType inferenceVariableType, @NotNull PsiClassType boundType); + public TypeAnnotationProvider boundAppeared(@NotNull PsiType inferenceVariableType, @NotNull PsiType boundType) { + return null; + } + + /** + * Called when the inference decides to use the lower bound of a type variable as its final result. + + * @return provider based on modified annotations or null if no applicable annotations found + */ + @Nullable + public TypeAnnotationProvider modifyLowerBoundAnnotations(@NotNull PsiType lowerBound, @NotNull PsiType upperBound) { + return null; + } } diff --git a/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/graphInference/InferenceSession.java b/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/graphInference/InferenceSession.java index 119ca09ffa72..2b2255f1ee22 100644 --- a/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/graphInference/InferenceSession.java +++ b/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/graphInference/InferenceSession.java @@ -1247,7 +1247,7 @@ public class InferenceSession { } } else { - type = lowerBound; + type = InferenceVariable.modifyAnnotations(lowerBound, (lb, modifier) -> modifier.modifyLowerBoundAnnotations(lb, upperBound)); } if (type == PsiType.NULL) { diff --git a/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/graphInference/InferenceVariable.java b/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/graphInference/InferenceVariable.java index 680690547f28..7c8ed4c30cef 100644 --- a/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/graphInference/InferenceVariable.java +++ b/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/graphInference/InferenceVariable.java @@ -24,6 +24,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.*; +import java.util.function.BiFunction; /** * User: anna @@ -71,19 +72,23 @@ public class InferenceVariable extends LightTypeParameter { public static void addBound(PsiType inferenceVariableType, PsiType boundType, InferenceBound inferenceBound, InferenceSession session) { final InferenceVariable variable = session.getInferenceVariable(inferenceVariableType); if (variable != null) { - for (TypeAnnotationModifier modifier : TypeAnnotationModifier.EP_NAME.getExtensions()) { - if (boundType instanceof PsiClassType) { - final TypeAnnotationProvider annotationProvider = modifier.modifyAnnotations(inferenceVariableType, (PsiClassType)boundType); - if (annotationProvider != null) { - boundType = boundType.annotate(annotationProvider); - } - } - } - + boundType = modifyAnnotations(boundType, (b, modifier) -> modifier.boundAppeared(inferenceVariableType, b)); variable.addBound(boundType, inferenceBound, session.myIncorporationPhase); } } + static PsiType modifyAnnotations(PsiType type, BiFunction executeModifier) { + for (TypeAnnotationModifier modifier : TypeAnnotationModifier.EP_NAME.getExtensions()) { + if (type instanceof PsiClassType) { + final TypeAnnotationProvider annotationProvider = executeModifier.apply(type, modifier); + if (annotationProvider != null) { + type = type.annotate(annotationProvider); + } + } + } + return type; + } + public boolean addBound(PsiType classType, InferenceBound inferenceBound, @Nullable InferenceIncorporationPhase incorporationPhase) { if (inferenceBound == InferenceBound.EQ && PsiUtil.resolveClassInClassTypeOnly(classType) == this) { diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java index f5583323e6db..934849a2e28d 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java @@ -18,6 +18,8 @@ package com.intellij.codeInspection; import com.intellij.JavaTestUtil; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInspection.dataFlow.DataFlowInspection; +import com.intellij.openapi.Disposable; +import com.intellij.openapi.project.Project; import com.intellij.openapi.projectRoots.Sdk; import com.intellij.openapi.util.Disposer; import com.intellij.testFramework.IdeaTestUtil; @@ -126,10 +128,14 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { private void setupCustomAnnotations() { myFixture.addClass("package foo;\n\nimport java.lang.annotation.*;\n\n@Target({ElementType.TYPE_USE}) public @interface Nullable { }"); myFixture.addClass("package foo;\n\nimport java.lang.annotation.*;\n\n@Target({ElementType.TYPE_USE}) public @interface NotNull { }"); - NullableNotNullManager nnnManager = NullableNotNullManager.getInstance(getProject()); - nnnManager.setNotNulls("foo.NotNull"); - nnnManager.setNullables("foo.Nullable"); - Disposer.register(getTestRootDisposable(), () -> { + setCustomAnnotations(getProject(), getTestRootDisposable(), "foo.NotNull", "foo.Nullable"); + } + + static void setCustomAnnotations(Project project, Disposable parentDisposable, String notNull, String nullable) { + NullableNotNullManager nnnManager = NullableNotNullManager.getInstance(project); + nnnManager.setNotNulls(notNull); + nnnManager.setNullables(nullable); + Disposer.register(parentDisposable, () -> { nnnManager.setNotNulls(); nnnManager.setNullables(); }); diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionHeavyTest.groovy b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionHeavyTest.groovy new file mode 100644 index 000000000000..59752722566b --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionHeavyTest.groovy @@ -0,0 +1,81 @@ +/* + * Copyright 2000-2017 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 com.intellij.codeInspection + +import com.intellij.codeInspection.dataFlow.DataFlowInspection +import com.intellij.openapi.module.StdModuleTypes +import com.intellij.openapi.roots.ModuleRootManager +import com.intellij.openapi.roots.ModuleRootModificationUtil +import com.intellij.pom.java.LanguageLevel +import com.intellij.testFramework.IdeaTestUtil +import com.intellij.testFramework.PsiTestUtil +import com.intellij.testFramework.fixtures.JavaCodeInsightFixtureTestCase +/** + * @author peter + */ +class DataFlowInspectionHeavyTest extends JavaCodeInsightFixtureTestCase { + + void testDifferentAnnotationsWithDifferentLanguageLevels() { + myFixture.allowTreeAccessForAllFiles() + + def module6 = PsiTestUtil.addModule(project, StdModuleTypes.JAVA, 'mod6', myFixture.tempDirFixture.findOrCreateDir('mod6')) + IdeaTestUtil.setModuleLanguageLevel(module6, LanguageLevel.JDK_1_6) + IdeaTestUtil.setModuleLanguageLevel(myModule, LanguageLevel.JDK_1_8) + ModuleRootModificationUtil.addDependency(myModule, module6) + ModuleRootModificationUtil.setModuleSdk(module6, ModuleRootManager.getInstance(myModule).sdk) + + myFixture.addFileToProject 'mod6/annos/annos.java', annotationsText("ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE") + myFixture.addFileToProject 'mod6/foo/ObjectUtils.java', ''' + package foo; + public class ObjectUtils { + @annos.NotNull + public static native T notNull(@annos.Nullable T value); + } + ''' + + myFixture.addFileToProject 'annos/annos.java', annotationsText("ElementType.TYPE_USE") + DataFlowInspection8Test.setCustomAnnotations(project, testRootDisposable, 'annos.NotNull', 'annos.Nullable') + + def testFile = myFixture.addFileToProject 'test.java', ''' + class Zoo { + @annos.Nullable String a = null; + @annos.NotNull String f = foo.ObjectUtils.notNull(a); + + void bar(@annos.NotNull String param) { } + void goo(@annos.Nullable String param) { + String p1 = foo.ObjectUtils.notNull(param); + bar(p1); + } + } + ''' + myFixture.configureFromExistingVirtualFile(testFile.virtualFile) + myFixture.enableInspections(new DataFlowInspection()) + myFixture.checkHighlighting() + } + + private static String annotationsText(String targets) { + """ + package annos; + import java.lang.annotation.*; + + @Target({$targets}) + public @interface NotNull {} + + @Target({$targets}) + public @interface Nullable {} + """ + } +} diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTestSuite.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTestSuite.java index 81a89f5a3576..63519db6474a 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTestSuite.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTestSuite.java @@ -29,6 +29,7 @@ public class DataFlowInspectionTestSuite { suite.addTestSuite(DataFlowInspectionTest.class); suite.addTestSuite(DataFlowInspection8Test.class); + suite.addTestSuite(DataFlowInspectionHeavyTest.class); suite.addTestSuite(DataFlowInspectionAncientTest.class); suite.addTestSuite(ContractCheckTest.class); suite.addTestSuite(HardcodedContractsTest.class);