From 434c7cd790d8700f536043f97aea5a87aed9bdc1 Mon Sep 17 00:00:00 2001 From: peter Date: Tue, 2 Dec 2014 15:15:27 +0100 Subject: [PATCH] infer method @Nullable/@NotNull by source code --- .../InferredAnnotationsManagerImpl.java | 43 ++++-- .../ProjectBytecodeAnalysis.java | 4 +- .../dataFlow/NullityInference.java | 128 ++++++++++++++++++ .../dataFlow/StandardDataFlowRunner.java | 18 ++- .../nullable/NullableStuffInspectionBase.java | 2 +- .../ContractInferenceFromSourceTest.groovy | 8 +- .../DataFlowInspectionTestSuite.java | 3 +- ...NullityInferenceFromSourceTestCase.groovy} | 29 +++- 8 files changed, 208 insertions(+), 27 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullityInference.java rename java/java-tests/testSrc/com/intellij/codeInspection/{NullityInferenceFromDfaTest.groovy => NullityInferenceFromSourceTestCase.groovy} (55%) diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java index 327e00f1fe05..7ba14c2367bb 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java @@ -16,10 +16,7 @@ package com.intellij.codeInsight; import com.intellij.codeInspection.bytecodeAnalysis.ProjectBytecodeAnalysis; -import com.intellij.codeInspection.dataFlow.ContractInference; -import com.intellij.codeInspection.dataFlow.HardcodedContracts; -import com.intellij.codeInspection.dataFlow.MethodContract; -import com.intellij.codeInspection.dataFlow.PurityInference; +import com.intellij.codeInspection.dataFlow.*; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; @@ -58,8 +55,17 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { } } - if (ORG_JETBRAINS_ANNOTATIONS_CONTRACT.equals(annotationFQN) && canHaveContract(listOwner)) { - return getInferredContractAnnotation((PsiMethod)listOwner); + if (canInferFromSource(listOwner)) { + //noinspection ConstantConditions + PsiMethod method = (PsiMethod)listOwner; + if (ORG_JETBRAINS_ANNOTATIONS_CONTRACT.equals(annotationFQN)) { + return getInferredContractAnnotation(method); + } + + if ((AnnotationUtil.NOT_NULL.equals(annotationFQN) || AnnotationUtil.NULLABLE.equals(annotationFQN))) { + PsiAnnotation anno = getInferredNullityAnnotation(method); + return anno == null ? null : annotationFQN.equals(anno.getQualifiedName()) ? anno : null; + } } return null; @@ -100,6 +106,23 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { return createContractAnnotation(ContractInference.inferContracts(method), PurityInference.inferPurity(method)); } + @Nullable + private PsiAnnotation getInferredNullityAnnotation(PsiMethod method) { + NullableNotNullManager manager = NullableNotNullManager.getInstance(myProject); + if (AnnotationUtil.findAnnotation(method, manager.getNotNulls(), true) != null || AnnotationUtil.findAnnotation(method, manager.getNullables(), true) != null) { + return null; + } + + Nullness nullness = NullityInference.inferNullity(method); + if (nullness == Nullness.NOT_NULL) { + return ProjectBytecodeAnalysis.getInstance(myProject).getNotNullAnnotation(); + } + if (nullness == Nullness.NULLABLE) { + return ProjectBytecodeAnalysis.getInstance(myProject).getNullableAnnotation(); + } + return null; + } + @Nullable private PsiAnnotation createContractAnnotation(List contracts, boolean pure) { final String attrs; @@ -115,7 +138,7 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { return ProjectBytecodeAnalysis.getInstance(myProject).createContractAnnotation(attrs); } - private static boolean canHaveContract(PsiModifierListOwner listOwner) { + private static boolean canInferFromSource(PsiModifierListOwner listOwner) { return listOwner instanceof PsiMethod && !PsiUtil.canBeOverriden((PsiMethod)listOwner); } @@ -127,15 +150,17 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { PsiAnnotation[] fromBytecode = ProjectBytecodeAnalysis.getInstance(myProject).findInferredAnnotations(listOwner); for (PsiAnnotation annotation : fromBytecode) { if (!ignoreInference(listOwner, annotation.getQualifiedName())) { - if (!ORG_JETBRAINS_ANNOTATIONS_CONTRACT.equals(annotation.getQualifiedName()) || canHaveContract(listOwner)) { + if (!ORG_JETBRAINS_ANNOTATIONS_CONTRACT.equals(annotation.getQualifiedName()) || canInferFromSource(listOwner)) { result.add(annotation); } } } - if (canHaveContract(listOwner)) { + if (canInferFromSource(listOwner)) { PsiAnnotation hardcoded = getHardcodedContractAnnotation((PsiMethod)listOwner); ContainerUtil.addIfNotNull(result, hardcoded != null ? hardcoded : getInferredContractAnnotation((PsiMethod)listOwner)); + + ContainerUtil.addIfNotNull(result, getInferredNullityAnnotation((PsiMethod)listOwner)); } return result.isEmpty() ? PsiAnnotation.EMPTY_ARRAY : result.toArray(new PsiAnnotation[result.size()]); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java index 4899f8451ed2..1258db655d49 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java @@ -198,7 +198,7 @@ public class ProjectBytecodeAnalysis { return PsiAnnotation.EMPTY_ARRAY; } - private PsiAnnotation getNotNullAnnotation() { + public PsiAnnotation getNotNullAnnotation() { return CachedValuesManager.getManager(myProject).getCachedValue(myProject, new CachedValueProvider() { @Nullable @Override @@ -208,7 +208,7 @@ public class ProjectBytecodeAnalysis { }); } - private PsiAnnotation getNullableAnnotation() { + public PsiAnnotation getNullableAnnotation() { return CachedValuesManager.getManager(myProject).getCachedValue(myProject, new CachedValueProvider() { @Nullable @Override diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullityInference.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullityInference.java new file mode 100644 index 000000000000..64d123f6beac --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullityInference.java @@ -0,0 +1,128 @@ +/* + * Copyright 2000-2014 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.dataFlow; + +import com.intellij.codeInsight.NullableNotNullManager; +import com.intellij.openapi.util.Computable; +import com.intellij.openapi.util.RecursionManager; +import com.intellij.psi.*; +import com.intellij.psi.util.CachedValueProvider; +import com.intellij.psi.util.CachedValuesManager; +import com.intellij.psi.util.PsiModificationTracker; +import com.intellij.util.containers.ContainerUtil; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * @author peter + */ +public class NullityInference { + + public static Nullness inferNullity(final PsiMethod method) { + if (ContractInference.isLibraryCode(method)) { + return Nullness.UNKNOWN; + } + + PsiType type = method.getReturnType(); + if (type == null || type instanceof PsiPrimitiveType) { + return Nullness.UNKNOWN; + } + + return CachedValuesManager.getCachedValue(method, new CachedValueProvider() { + @Nullable + @Override + public Result compute() { + Nullness result = RecursionManager.doPreventingRecursion(method, true, new Computable() { + @Override + public Nullness compute() { + return doInferNullity(method); + } + }); + if (result == null) result = Nullness.UNKNOWN; + return Result.create(result, method, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT); + } + }); + } + + @NotNull + private static Nullness doInferNullity(PsiMethod method) { + PsiCodeBlock body = method.getBody(); + if (body != null) { + final AtomicBoolean hasErrors = new AtomicBoolean(); + final AtomicBoolean hasNotNulls = new AtomicBoolean(); + final AtomicBoolean hasNulls = new AtomicBoolean(); + final AtomicBoolean hasUnknowns = new AtomicBoolean(); + final List calls = ContainerUtil.newArrayList(); + body.accept(new JavaRecursiveElementWalkingVisitor() { + @Override + public void visitReturnStatement(PsiReturnStatement statement) { + PsiExpression value = statement.getReturnValue(); + if (value == null) { + hasErrors.set(true); + } else { + if (value instanceof PsiLiteralExpression) { + if (value.textMatches(PsiKeyword.NULL)) { + hasNulls.set(true); + } else { + hasNotNulls.set(true); + } + } else if (value instanceof PsiMethodCallExpression) { + calls.add((PsiMethodCallExpression)value); + } else { + hasUnknowns.set(true); + } + } + super.visitReturnStatement(statement); + } + + @Override + public void visitErrorElement(PsiErrorElement element) { + hasErrors.set(true); + super.visitErrorElement(element); + } + }); + + if (hasNulls.get()) { + return Nullness.NULLABLE; + } + + if (calls.size() > 1) { + return Nullness.UNKNOWN; + } + + if (calls.size() == 1) { + PsiMethod target = calls.get(0).resolveMethod(); + if (target != null && NullableNotNullManager.isNotNull(target)) { + return Nullness.NOT_NULL; + } + return Nullness.UNKNOWN; + } + + if (hasUnknowns.get()) { + return Nullness.UNKNOWN; + } + + if (hasNotNulls.get()) { + return Nullness.NOT_NULL; + } + + } + return Nullness.UNKNOWN; + } +} diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardDataFlowRunner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardDataFlowRunner.java index d2d91d2c638d..4f9108befb17 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardDataFlowRunner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardDataFlowRunner.java @@ -27,6 +27,7 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInspection.dataFlow.instructions.InstanceofInstruction; import com.intellij.codeInspection.dataFlow.instructions.Instruction; +import com.intellij.codeInspection.nullable.NullableStuffInspectionBase; import com.intellij.psi.*; import org.jetbrains.annotations.NotNull; @@ -53,17 +54,13 @@ public class StandardDataFlowRunner extends DataFlowRunner { myIsInMethod = parent instanceof PsiMethod; if (myIsInMethod) { PsiMethod method = (PsiMethod)parent; - PsiType returnType = method.getReturnType(); - myInNullableMethod = NullableNotNullManager.isNullable(method) || - returnType != null && returnType.equalsToText(CommonClassNames.JAVA_LANG_VOID); + myInNullableMethod = isTreatedAsNullable(method); myInNotNullMethod = NullableNotNullManager.isNotNull(method); } else if (parent instanceof PsiLambdaExpression) { PsiMethod method = LambdaUtil.getFunctionalInterfaceMethod(((PsiLambdaExpression)parent).getFunctionalInterfaceType()); if (method != null) { myIsInMethod = true; - PsiType returnType = method.getReturnType(); - myInNullableMethod = NullableNotNullManager.isNullable(method) || - returnType != null && returnType.equalsToText(CommonClassNames.JAVA_LANG_VOID); + myInNullableMethod = isTreatedAsNullable(method); myInNotNullMethod = NullableNotNullManager.isNotNull(method); } } @@ -71,6 +68,15 @@ public class StandardDataFlowRunner extends DataFlowRunner { myCCEInstructions.clear(); } + private static boolean isTreatedAsNullable(PsiMethod method) { + if (NullableStuffInspectionBase.isNullableNotInferred(method, true)) { + return true; + } + + PsiType returnType = method.getReturnType(); + return returnType != null && returnType.equalsToText(CommonClassNames.JAVA_LANG_VOID); + } + public void onInstructionProducesCCE(Instruction instruction) { myCCEInstructions.add(instruction); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java index 63539ed5ed33..72b8320038bc 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java @@ -528,7 +528,7 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo return true; } - private static boolean isNullableNotInferred(@NotNull PsiModifierListOwner owner, boolean checkBases) { + public static boolean isNullableNotInferred(@NotNull PsiModifierListOwner owner, boolean checkBases) { Project project = owner.getProject(); NullableNotNullManager manager = NullableNotNullManager.getInstance(project); if (!manager.isNullable(owner, checkBases)) return false; diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy b/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy index 3364b74671df..13ba25910061 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy +++ b/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy @@ -359,13 +359,13 @@ class ContractInferenceFromSourceTest extends LightCodeInsightFixtureTestCase { public void "test use delegated method notnull"() { def c = inferContracts(""" - final Object foo(Object bar) { - return doo(); + final Object foo(Object bar, boolean b) { + return b ? doo() : null; } @org.jetbrains.annotations.NotNull Object doo() {} """) - assert c == ['_ -> !null'] + assert c == ['_, true -> !null', '_, false -> null'] } public void "test use delegated method notnull with contracts"() { @@ -379,7 +379,7 @@ class ContractInferenceFromSourceTest extends LightCodeInsightFixtureTestCase { return smth(); } """) - assert c == ['_, null -> fail', '_, _ -> !null'] + assert c == ['_, null -> fail'] } public void "test dig into type cast"() { diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTestSuite.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTestSuite.java index 3dbf6eb87994..fd8147bc8355 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTestSuite.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTestSuite.java @@ -30,7 +30,8 @@ public class DataFlowInspectionTestSuite { suite.addTestSuite(DataFlowInspectionAncientTest.class); suite.addTestSuite(ContractCheckTest.class); suite.addTestSuite(ContractInferenceFromSourceTest.class); - suite.addTestSuite(NullityInferenceFromDfaTest.class); + suite.addTestSuite(NullityInferenceFromSourceTestCase.DfaInferenceTest.class); + suite.addTestSuite(NullityInferenceFromSourceTestCase.LightInferenceTest.class); suite.addTestSuite(PurityInferenceFromSourceTest.class); suite.addTestSuite(SliceTreeTest.class); suite.addTestSuite(SliceBackwardTest.class); diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/NullityInferenceFromDfaTest.groovy b/java/java-tests/testSrc/com/intellij/codeInspection/NullityInferenceFromSourceTestCase.groovy similarity index 55% rename from java/java-tests/testSrc/com/intellij/codeInspection/NullityInferenceFromDfaTest.groovy rename to java/java-tests/testSrc/com/intellij/codeInspection/NullityInferenceFromSourceTestCase.groovy index 74b1120e3c67..3b60d9549375 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/NullityInferenceFromDfaTest.groovy +++ b/java/java-tests/testSrc/com/intellij/codeInspection/NullityInferenceFromSourceTestCase.groovy @@ -14,17 +14,17 @@ * limitations under the License. */ package com.intellij.codeInspection +import com.intellij.codeInsight.NullableNotNullManager import com.intellij.codeInspection.dataFlow.DfaUtil import com.intellij.codeInspection.dataFlow.Nullness import com.intellij.psi.PsiMethod import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase import static com.intellij.codeInspection.dataFlow.Nullness.* - /** * @author peter */ -class NullityInferenceFromDfaTest extends LightCodeInsightFixtureTestCase { +abstract class NullityInferenceFromSourceTestCase extends LightCodeInsightFixtureTestCase { void "test return string literal"() { assert inferNullity(parse('String foo() { return "a"; }')) == NOT_NULL @@ -38,12 +38,33 @@ class NullityInferenceFromDfaTest extends LightCodeInsightFixtureTestCase { assert inferNullity(parse('int foo() { return "z"; }')) == UNKNOWN } - private static Nullness inferNullity(PsiMethod method) { - return DfaUtil.inferMethodNullity(method) + void "test delegation"() { + assert inferNullity(parse('String foo() { return bar(); }; String bar() { return "z"; }; ')) == NOT_NULL } + void "test if branch returns null"() { + assert inferNullity(parse('String bar() { if (equals(2)) return null; return "a"; }; ')) == NULLABLE + } + + void "test delegation to nullable means nothing"() { + assert inferNullity(parse('String foo() { return bar(); }; String bar() { if (equals(2)) return null; return "a"; }; ')) == UNKNOWN + } + + protected abstract Nullness inferNullity(PsiMethod method) + private PsiMethod parse(String method) { return myFixture.addClass("final class Foo { $method }").methods[0] } + static class LightInferenceTest extends NullityInferenceFromSourceTestCase { + Nullness inferNullity(PsiMethod method) { + return NullableNotNullManager.isNotNull(method) ? NOT_NULL : NullableNotNullManager.isNullable(method) ? NULLABLE : UNKNOWN + } + } + + static class DfaInferenceTest extends NullityInferenceFromSourceTestCase { + Nullness inferNullity(PsiMethod method) { + return DfaUtil.inferMethodNullity(method) + } + } }