From e82c7b2853788df2f3dbfb41657412a1fbe02590 Mon Sep 17 00:00:00 2001 From: peter Date: Sun, 28 Oct 2012 23:20:14 +0100 Subject: [PATCH] IDEA-74934 "Constant conditions & exceptions": take type parameters into consideration, take 1 (methods) --- .../dataFlow/StandardInstructionVisitor.java | 156 +++++++++++------- .../instructions/MethodCallInstruction.java | 30 ---- .../instructions/PushInstruction.java | 3 +- .../codeInsight/NullableNotNullManager.java | 4 +- .../fixture/AnnotatedTypeParameters.java | 32 ++++ .../DataFlowInspectionFixtureTest.java | 16 ++ 6 files changed, 146 insertions(+), 95 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/AnnotatedTypeParameters.java diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index a4d033680f69..05ca2b540609 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -22,7 +22,6 @@ import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.TypeConversionUtil; -import com.intellij.util.ArrayUtil; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.FactoryMap; import gnu.trove.THashSet; @@ -30,6 +29,8 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.ArrayList; +import java.util.Collections; +import java.util.Map; import java.util.Set; /** @@ -40,44 +41,14 @@ public class StandardInstructionVisitor extends InstructionVisitor { private final Set myCanBeNullInInstanceof = new THashSet(); private final Set myNotToReportReachability = new THashSet(); private final Set myUsefulInstanceofs = new THashSet(); - private final FactoryMap myParametersNotNull = new FactoryMap() { + private final FactoryMap> myParametersNullability = new FactoryMap>() { + @Nullable @Override - protected boolean[] create(MethodCallInstruction key) { - final PsiCallExpression callExpression = key.getCallExpression(); - final PsiMethod callee = callExpression == null ? null : callExpression.resolveMethod(); - if (callee != null) { - final PsiParameter[] params = callee.getParameterList().getParameters(); - boolean[] result = new boolean[params.length]; - for (int i = 0; i < params.length; i++) { - result[i] = NullableNotNullManager.getInstance(params[i].getProject()).isNotNull(params[i], false); - } - return result; - } - else { - return ArrayUtil.EMPTY_BOOLEAN_ARRAY; - } + protected Map create(MethodCallInstruction key) { + return calcParameterNullability(key.getCallExpression()); } }; - private final FactoryMap myParametersNonAnnotated = new FactoryMap() { - @Override - protected boolean[] create(MethodCallInstruction key) { - final PsiCallExpression callExpression = key.getCallExpression(); - final PsiMethod callee = callExpression == null ? null : callExpression.resolveMethod(); - if (callee != null) { - final PsiParameter[] params = callee.getParameterList().getParameters(); - boolean[] result = new boolean[params.length]; - final NullableNotNullManager notNullManager = NullableNotNullManager.getInstance(callee.getProject()); - for (int i = 0; i < params.length; i++) { - result[i] = !notNullManager.isNotNull(params[i], false) && !notNullManager.isNullable(params[i], false); - } - return result; - } - else { - return ArrayUtil.EMPTY_BOOLEAN_ARRAY; - } - } - }; - private final FactoryMap myCalleeNullability = new FactoryMap() { + private final FactoryMap myReturnTypeNullability = new FactoryMap() { @Override protected Boolean create(MethodCallInstruction key) { final PsiCallExpression callExpression = key.getCallExpression(); @@ -85,21 +56,85 @@ public class StandardInstructionVisitor extends InstructionVisitor { return Boolean.FALSE; } - if (callExpression != null) { - final PsiMethod callee = callExpression.resolveMethod(); - if (callee != null) { - if (NullableNotNullManager.isNullable(callee)) { - return Boolean.TRUE; - } - if (NullableNotNullManager.isNotNull(callee)) { - return Boolean.FALSE; - } - } - } - return null; + return callExpression != null ? getElementNullability(key.getResultType(), callExpression.resolveMethod()) : null; } }; + private static Map calcParameterNullability(@Nullable PsiCallExpression callExpression) { + PsiExpressionList argumentList = callExpression == null ? null : callExpression.getArgumentList(); + if (argumentList != null) { + JavaResolveResult result = callExpression.resolveMethodGenerics(); + PsiMethod method = (PsiMethod)result.getElement(); + if (method != null) { + PsiSubstitutor substitutor = result.getSubstitutor(); + PsiExpression[] args = argumentList.getExpressions(); + PsiParameter[] parameters = method.getParameterList().getParameters(); + + boolean varArg = isVarArgCall(method, substitutor, args, parameters); + int checkedCount = Math.min(args.length, parameters.length) - (varArg ? 1 : 0); + + Map map = ContainerUtil.newHashMap(); + for (int i = 0; i < checkedCount; i++) { + map.put(args[i], getElementNullability(substitutor.substitute(parameters[i].getType()), parameters[i])); + } + return map; + } + } + return Collections.emptyMap(); + } + + private static boolean isVarArgCall(PsiMethod method, PsiSubstitutor substitutor, PsiExpression[] args, PsiParameter[] parameters) { + if (!method.isVarArgs()) { + return false; + } + + int argCount = args.length; + int paramCount = parameters.length; + if (argCount > paramCount) { + return true; + } + else if (paramCount > 0) { + if (argCount == paramCount) { + PsiType lastArgType = args[argCount - 1].getType(); + if (lastArgType != null && + !substitutor.substitute(parameters[paramCount - 1].getType()).isAssignableFrom(lastArgType)) { + return true; + } + } + } + return false; + } + + + @Nullable + private static Boolean getElementNullability(@Nullable PsiType resultType, @Nullable PsiModifierListOwner owner) { + if (owner == null) { + return null; + } + + if (NullableNotNullManager.isNullable(owner)) { + return Boolean.TRUE; + } + if (NullableNotNullManager.isNotNull(owner)) { + return Boolean.FALSE; + } + + if (resultType != null) { + NullableNotNullManager nnn = NullableNotNullManager.getInstance(owner.getProject()); + for (PsiAnnotation annotation : resultType.getAnnotations()) { + String qualifiedName = annotation.getQualifiedName(); + if (nnn.getNullables().contains(qualifiedName)) { + return Boolean.TRUE; + } + if (nnn.getNotNulls().contains(qualifiedName)) { + return Boolean.FALSE; + } + } + } + + return null; + } + @Override public DfaInstructionState[] visitAssign(AssignInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { DfaValue dfaSource = memState.pop(); @@ -181,24 +216,21 @@ public class StandardInstructionVisitor extends InstructionVisitor { @Override public DfaInstructionState[] visitMethodCall(MethodCallInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { final PsiExpression[] args = instruction.getArgs(); - final boolean[] parametersNotNull = myParametersNotNull.get(instruction); - final boolean[] nonAnnotated = myParametersNonAnnotated.get(instruction); + Map map = myParametersNullability.get(instruction); final DfaNotNullValue.Factory factory = runner.getFactory().getNotNullFactory(); for (int i = 0; i < args.length; i++) { final DfaValue arg = memState.pop(); - final int revIdx = args.length - i - 1; - if (args.length <= parametersNotNull.length && revIdx < parametersNotNull.length && !(i == args.length - 1 && instruction.isVarargCall())) { - if (parametersNotNull[revIdx]) { - if (!memState.applyNotNull(arg)) { - onPassingNullParameter(runner, args[revIdx]); - if (arg instanceof DfaVariableValue) { - memState.setVarValue((DfaVariableValue)arg, factory.create(((DfaVariableValue)arg).getVariableType())); - } + PsiExpression expr = args[(args.length - i - 1)]; + if (map.get(expr) == Boolean.FALSE) { + if (!memState.applyNotNull(arg)) { + onPassingNullParameter(runner, expr); + if (arg instanceof DfaVariableValue) { + memState.setVarValue((DfaVariableValue)arg, factory.create(((DfaVariableValue)arg).getVariableType())); } } - else if (nonAnnotated[revIdx] && !memState.checkNotNullable(arg)) { - onPassingNullParameterToNonAnnotated(runner, args[revIdx]); - } + } + else if (map.containsKey(expr) && map.get(expr) == null && !memState.checkNotNullable(arg)) { + onPassingNullParameterToNonAnnotated(runner, expr); } } @@ -236,7 +268,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { final PsiType type = instruction.getResultType(); final MethodCallInstruction.MethodType methodType = instruction.getMethodType(); if (type != null && (type instanceof PsiClassType || type.getArrayDimensions() > 0)) { - @Nullable final Boolean nullability = myCalleeNullability.get(instruction); + @Nullable final Boolean nullability = myReturnTypeNullability.get(instruction); return nullability == Boolean.FALSE ? factory.getNotNullFactory().create(type) : factory.getTypeFactory().create(type, nullability == Boolean.TRUE); } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java index 6e5ac5dd534c..22f98623447d 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java @@ -42,7 +42,6 @@ public class MethodCallInstruction extends Instruction { @NotNull private final PsiExpression myContext; private final MethodType myMethodType; @Nullable private DfaValue myPrecalculatedReturnValue; - private boolean myVarargCall; public enum MethodType { BOXING, UNBOXING, REGULAR_METHOD_CALL, CAST } @@ -50,7 +49,6 @@ public class MethodCallInstruction extends Instruction { public MethodCallInstruction(@NotNull PsiCallExpression callExpression, @Nullable DfaValue precalculatedReturnValue) { this(callExpression, MethodType.REGULAR_METHOD_CALL); myPrecalculatedReturnValue = precalculatedReturnValue; - myVarargCall = calcIsVarargCall(callExpression); } public MethodCallInstruction(@NotNull PsiExpression context, MethodType methodType, @Nullable PsiType resultType) { @@ -79,34 +77,6 @@ public class MethodCallInstruction extends Instruction { return myType; } - public boolean isVarargCall() { - return myVarargCall; - } - - private static boolean calcIsVarargCall(PsiCallExpression callExpression) { - PsiExpressionList argumentList = callExpression.getArgumentList(); - if (argumentList != null) { - JavaResolveResult result = callExpression.resolveMethodGenerics(); - PsiMethod method = (PsiMethod)result.getElement(); - if (method != null && method.isVarArgs()) { - PsiType[] argTypes = argumentList.getExpressionTypes(); - int argCount = argTypes.length; - - PsiParameter[] parameters = method.getParameterList().getParameters(); - int paramCount = parameters.length; - - if (argCount > paramCount) { - return true; - } - PsiType lastParam = parameters[paramCount - 1].getType(); - if (argCount == paramCount && !result.getSubstitutor().substitute(lastParam).isAssignableFrom(argTypes[argCount - 1])) { - return true; - } - } - } - return false; - } - @NotNull public PsiExpression[] getArgs() { return myArgs; diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/PushInstruction.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/PushInstruction.java index 9bf11de1cb54..1c214f7b5c7d 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/PushInstruction.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/PushInstruction.java @@ -34,12 +34,13 @@ import com.intellij.psi.PsiExpression; import com.intellij.psi.PsiField; import com.intellij.psi.PsiReferenceExpression; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; public class PushInstruction extends Instruction { private final DfaValue myValue; private final PsiExpression myPlace; - public PushInstruction(DfaValue value, PsiExpression place) { + public PushInstruction(@Nullable DfaValue value, PsiExpression place) { myValue = value != null ? value : DfaUnknownValue.getInstance(); myPlace = place; } 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 d6c5849c5034..13462810a12b 100644 --- a/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java +++ b/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java @@ -68,13 +68,13 @@ public class NullableNotNullManager implements PersistentStateComponent } } - public void setNotNulls(String[] annotations) { + public void setNotNulls(String... annotations) { myNotNulls.clear(); addAllIfNotPresent(myNotNulls, DEFAULT_NOT_NULLS); addAllIfNotPresent(myNotNulls, annotations); } - public void setNullables(String[] annotations) { + public void setNullables(String... annotations) { myNullables.clear(); addAllIfNotPresent(myNullables, DEFAULT_NULLABLES); addAllIfNotPresent(myNullables, annotations); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/AnnotatedTypeParameters.java b/java/java-tests/testData/inspection/dataFlow/fixture/AnnotatedTypeParameters.java new file mode 100644 index 000000000000..2d7a8fc3fd56 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/AnnotatedTypeParameters.java @@ -0,0 +1,32 @@ +import foo.NotNull; +import foo.Nullable; + +import java.util.List; + +class Test { + private static void test(List<@NotNull Object> list) { + if (list.get(0) == null) { + return; + } + list.add(null); + } + private static void test2(List<@Nullable Object> list) { + if (list.get(0) == null) { + return; + } + list.add(null); + } + + private static void test3(Ref<@NotNull Object> ref) { + if (ref.value == null) { + return; + } + ref.value = null; + } + + +} + +class Ref { + T value; +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java index 6406a6c323bc..282aae37c8f9 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java @@ -16,6 +16,7 @@ package com.intellij.codeInspection; import com.intellij.JavaTestUtil; +import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInspection.dataFlow.DataFlowInspection; import com.intellij.pom.java.LanguageLevel; import com.intellij.testFramework.builders.JavaModuleFixtureBuilder; @@ -103,4 +104,19 @@ public class DataFlowInspectionFixtureTest extends JavaCodeInsightFixtureTestCas public void testPassingNullableIntoVararg() throws Throwable { doTest(); } public void testEqualsImpliesNotNull() throws Throwable { doTest(); } + public void testAnnotatedTypeParameters() throws Throwable { + myFixture.addClass("package foo; public @interface Nullable {}"); + myFixture.addClass("package foo; public @interface NotNull {}"); + NullableNotNullManager nnnManager = NullableNotNullManager.getInstance(getProject()); + nnnManager.setNotNulls("foo.NotNull"); + nnnManager.setNullables("foo.Nullable"); + try { + doTest(); + } + finally { + nnnManager.setNotNulls(); + nnnManager.setNullables(); + } + } + }