From 8a9227543c7a99b426cbf8aa24c4b910fc2c4be0 Mon Sep 17 00:00:00 2001 From: Dmitry Batkovich Date: Thu, 18 Aug 2016 12:20:54 +0300 Subject: [PATCH] an index to detect nulls passed as arguments of method parameters + DataFlowInspection and NullableStuffInspection use it to detect method call failings when one of parameters is null --- .../dataFlow/ContractChecker.java | 9 +- .../dataFlow/DataFlowInspectionBase.java | 29 ++- .../dataFlow/DataFlowRunner.java | 9 +- .../codeInspection/dataFlow/DfaPsiUtil.java | 2 +- .../NullParameterConstraintChecker.java | 138 ++++++++++ .../dataFlow/StandardDataFlowRunner.java | 6 +- .../nullable/NullableStuffInspectionBase.java | 67 +++-- .../dataFlow/DataFlowInspection.java | 19 ++ .../nullable/NullableStuffInspection.java | 69 +++++ .../codeInspection/nullable/OptionsPanel.form | 16 +- .../search/JavaNullMethodArgumentIndex.java | 238 ++++++++++++++++++ .../search/JavaNullMethodArgumentUtil.java | 82 ++++++ .../fixture/GenericParameterNullity.java | 2 +- .../NullArgumentButParameterIsReassigned.java | 17 ++ .../NullArgumentIsFailingMethodCall.java | 29 +++ .../NullArgumentIsNotFailingMethodCall.java | 26 ++ ...llPassedToNotNullConstructorParameter.java | 43 ++++ .../NullPassedToNotNullParameter.java | 24 ++ .../DataFlowInspection8Test.java | 12 + .../NullableStuffInspectionTest.java | 8 + .../search/JavaNullMethodArgumentIndexTest.kt | 103 ++++++++ .../src/messages/InspectionsBundle.properties | 4 + resources/src/META-INF/IdeaPlugin.xml | 2 + 23 files changed, 907 insertions(+), 47 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullParameterConstraintChecker.java create mode 100644 java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaNullMethodArgumentIndex.java create mode 100644 java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaNullMethodArgumentUtil.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentButParameterIsReassigned.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentIsFailingMethodCall.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentIsNotFailingMethodCall.java create mode 100644 java/java-tests/testData/inspection/nullableProblems/NullPassedToNotNullConstructorParameter.java create mode 100644 java/java-tests/testData/inspection/nullableProblems/NullPassedToNotNullParameter.java create mode 100644 java/java-tests/testSrc/com/intellij/psi/impl/search/JavaNullMethodArgumentIndexTest.kt diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java index ad1fc779e46b..98f5446a6f2d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java @@ -35,15 +35,14 @@ import java.util.*; class ContractChecker extends DataFlowRunner { private final PsiMethod myMethod; private final MethodContract myContract; - private final boolean myOnTheFly; private final Set myViolations = ContainerUtil.newHashSet(); private final Set myNonViolations = ContainerUtil.newHashSet(); private final Set myFailures = ContainerUtil.newHashSet(); private ContractChecker(PsiMethod method, MethodContract contract, final boolean onTheFly) { + super(false, true, onTheFly); myMethod = method; myContract = contract; - myOnTheFly = onTheFly; } static Map checkContractClause(PsiMethod method, @@ -72,12 +71,6 @@ class ContractChecker extends DataFlowRunner { return checker.getErrors(); } - @Override - protected boolean shouldCheckTimeLimit() { - if (!myOnTheFly) return false; - return super.shouldCheckTimeLimit(); - } - @NotNull @Override protected DfaInstructionState[] acceptInstruction(@NotNull InstructionVisitor visitor, @NotNull DfaInstructionState instructionState) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index a993ff1cc31d..f55098cd051a 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -70,6 +70,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { public boolean TREAT_UNKNOWN_MEMBERS_AS_NULLABLE; public boolean IGNORE_ASSERT_STATEMENTS; public boolean REPORT_CONSTANT_REFERENCE_VALUES = true; + public boolean REPORT_NULLS_PASSED_TO_NOT_NULL_PARAMETER = true; @Override public JComponent createOptionsPanel() { @@ -103,6 +104,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { @Override public void visitMethod(PsiMethod method) { analyzeCodeBlock(method.getBody(), holder, isOnTheFly); + analyzeNullLiteralMethodArguments(method, holder, isOnTheFly); } @Override @@ -137,20 +139,31 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { }; } + protected LocalQuickFix createNavigateToNullParameterUsagesFix(PsiParameter parameter) { + return null; + } + + private void analyzeNullLiteralMethodArguments(PsiMethod method, ProblemsHolder holder, boolean isOnTheFly) { + if (REPORT_NULLS_PASSED_TO_NOT_NULL_PARAMETER) { + for (PsiParameter parameter : NullParameterConstraintChecker.checkMethodParameters(method, isOnTheFly)) { + final String name = parameter.getName(); + holder.registerProblem(parameter.getNameIdentifier(), + InspectionsBundle.message("dataflow.method.fails.with.null.argument", name), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + NullableStuffInspectionBase.getWrappedUiDependentQuickFix(this::createNavigateToNullParameterUsagesFix, parameter, isOnTheFly)); + } + } + } + private void analyzeCodeBlock(@Nullable final PsiElement scope, ProblemsHolder holder, final boolean onTheFly) { if (scope == null) return; PsiClass containingClass = PsiTreeUtil.getParentOfType(scope, PsiClass.class); if (containingClass != null && PsiUtil.isLocalOrAnonymousClass(containingClass) && !(containingClass instanceof PsiEnumConstantInitializer)) return; - final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner(TREAT_UNKNOWN_MEMBERS_AS_NULLABLE, !isInsideConstructorOrInitializer( - scope)) { - @Override - protected boolean shouldCheckTimeLimit() { - if (!onTheFly) return false; - return super.shouldCheckTimeLimit(); - } - }; + final StandardDataFlowRunner dfaRunner = + new StandardDataFlowRunner(TREAT_UNKNOWN_MEMBERS_AS_NULLABLE, + !isInsideConstructorOrInitializer(scope), onTheFly); analyzeDfaWithNestedClosures(scope, holder, dfaRunner, Collections.singletonList(dfaRunner.createMemoryState()), onTheFly); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java index a0f2c84c8232..fa4e6fc4dece 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java @@ -53,16 +53,17 @@ public class DataFlowRunner { private final MultiMap myNestedClosures = new MultiMap<>(); @NotNull private final DfaValueFactory myValueFactory; - + private final boolean myShouldCheckLimitTime; // Maximum allowed attempts to process instruction. Fail as too complex to process if certain instruction // is executed more than this limit times. static final int MAX_STATES_PER_BRANCH = 300; protected DataFlowRunner() { - this(false, true); + this(false, true, false); } - protected DataFlowRunner(boolean unknownMembersAreNullable, boolean honorFieldInitializers) { + protected DataFlowRunner(boolean unknownMembersAreNullable, boolean honorFieldInitializers, boolean shouldCheckLimitTime) { + myShouldCheckLimitTime = shouldCheckLimitTime; myValueFactory = new DfaValueFactory(honorFieldInitializers, unknownMembersAreNullable); } @@ -270,7 +271,7 @@ public class DataFlowRunner { } protected boolean shouldCheckTimeLimit() { - return !ApplicationManager.getApplication().isUnitTestMode(); + return myShouldCheckLimitTime && !ApplicationManager.getApplication().isUnitTestMode(); } @NotNull diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaPsiUtil.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaPsiUtil.java index b50071ea864d..50db5a142e76 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaPsiUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaPsiUtil.java @@ -150,7 +150,7 @@ public class DfaPsiUtil { public Result> compute() { final PsiCodeBlock body = constructor.getBody(); final Map map = ContainerUtil.newHashMap(); - final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner(false, false) { + final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner(false, false, false) { private boolean isCallExposingNonInitializedFields(Instruction instruction) { if (!(instruction instanceof MethodCallInstruction) || diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullParameterConstraintChecker.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullParameterConstraintChecker.java new file mode 100644 index 000000000000..296e5e2b33a4 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullParameterConstraintChecker.java @@ -0,0 +1,138 @@ +/* + * Copyright 2000-2016 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.codeInspection.dataFlow.instructions.AssignInstruction; +import com.intellij.codeInspection.dataFlow.instructions.Instruction; +import com.intellij.codeInspection.dataFlow.instructions.ReturnInstruction; +import com.intellij.codeInspection.dataFlow.value.DfaValue; +import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; +import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; +import com.intellij.psi.PsiMethod; +import com.intellij.psi.PsiModifierListOwner; +import com.intellij.psi.PsiParameter; +import com.intellij.psi.PsiPrimitiveType; +import com.intellij.psi.impl.search.JavaNullMethodArgumentUtil; +import com.intellij.util.SmartList; +import gnu.trove.THashSet; +import org.jetbrains.annotations.NotNull; + +import java.util.Collection; +import java.util.Set; + +/** + * This checker uses following idea: + * On the checker's start mark all parameters with null-argument usages as violated (i.e. the method fails if parameter is null). + * A parameter can be amnestied (excluded from violated) when one of following statements is true: + * 1. If at least one successful method execution ({@link ReturnInstruction#isViaException()} == false) + * doesn't require a not-null value for the parameter ({@link DfaMemoryState#isNotNull(DfaValue) == false}); + * OR + * 2. If the parameter has a reassignment while one of any method execution. + * + * All remaining violated parameters is required to be not-null for successful method execution. + */ +class NullParameterConstraintChecker extends DataFlowRunner { + private final Set myPossiblyViolatedParameters; + + private NullParameterConstraintChecker(Collection parameters, boolean isOnTheFly) { + super(false, true, isOnTheFly); + myPossiblyViolatedParameters = new THashSet<>(parameters); + } + + @NotNull + static PsiParameter[] checkMethodParameters(PsiMethod method, boolean isOnTheFly) { + if (method.getBody() == null) return PsiParameter.EMPTY_ARRAY; + + final Collection nullableParameters = new SmartList<>(); + final PsiParameter[] parameters = method.getParameterList().getParameters(); + for (int index = 0; index < parameters.length; index++) { + PsiParameter parameter = parameters[index]; + if (!(parameter.getType() instanceof PsiPrimitiveType) && + !NullableNotNullManager.isNotNull(parameter) && + JavaNullMethodArgumentUtil.hasNullArgument(method, index)) { + nullableParameters.add(parameter); + } + } + if (nullableParameters.isEmpty()) return PsiParameter.EMPTY_ARRAY; + + final NullParameterConstraintChecker checker = new NullParameterConstraintChecker(nullableParameters, isOnTheFly); + checker.analyzeMethod(method.getBody(), new StandardInstructionVisitor()); + + return checker.myPossiblyViolatedParameters.toArray(new PsiParameter[checker.myPossiblyViolatedParameters.size()]); + } + + @NotNull + @Override + protected DfaInstructionState[] acceptInstruction(@NotNull InstructionVisitor visitor, @NotNull DfaInstructionState instructionState) { + DfaMemoryState memState = instructionState.getMemoryState(); + if (memState.isEphemeral()) { + return DfaInstructionState.EMPTY_ARRAY; + } + Instruction instruction = instructionState.getInstruction(); + + if (instruction instanceof AssignInstruction) { + final DfaValue value = ((AssignInstruction)instruction).getAssignedValue(); + if (value instanceof DfaVariableValue) { + final PsiModifierListOwner psiVariable = ((DfaVariableValue)value).getPsiVariable(); + if (psiVariable instanceof PsiParameter) { + myPossiblyViolatedParameters.remove(psiVariable); + } + } + } + + if (instruction instanceof ReturnInstruction && !((ReturnInstruction)instruction).isViaException()) { + for (PsiParameter parameter : myPossiblyViolatedParameters.toArray(new PsiParameter[myPossiblyViolatedParameters.size()])) { + final DfaVariableValue dfaVar = getFactory().getVarFactory().createVariableValue(parameter, false); + if (!memState.isNotNull(dfaVar)) { + myPossiblyViolatedParameters.remove(parameter); + } + } + } + + return super.acceptInstruction(visitor, instructionState); + } + + @NotNull + @Override + protected DfaMemoryState createMemoryState() { + return new MyDfaMemoryState(getFactory()); + } + + private class MyDfaMemoryState extends DfaMemoryStateImpl { + + protected MyDfaMemoryState(DfaValueFactory factory) { + super(factory); + } + + protected MyDfaMemoryState(MyDfaMemoryState toCopy) { + super(toCopy); + } + + @Override + public void flushVariable(@NotNull DfaVariableValue variable) { + final PsiModifierListOwner psi = variable.getPsiVariable(); + if (psi instanceof PsiParameter && myPossiblyViolatedParameters.contains(psi)) return; + super.flushVariable(variable); + } + + @NotNull + @Override + public DfaMemoryStateImpl createCopy() { + return new MyDfaMemoryState(this); + } + } +} 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 984858ebed50..6535ee6ce46c 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,9 +27,9 @@ package com.intellij.codeInspection.dataFlow; public class StandardDataFlowRunner extends DataFlowRunner { public StandardDataFlowRunner() { - this(false, true); + this(false, true, false); } - public StandardDataFlowRunner(boolean unknownMembersAreNullable, boolean honorFieldInitializers) { - super(unknownMembersAreNullable, honorFieldInitializers); + public StandardDataFlowRunner(boolean unknownMembersAreNullable, boolean honorFieldInitializers, boolean shouldCheckLimitTime) { + super(unknownMembersAreNullable, honorFieldInitializers, shouldCheckLimitTime); } } 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 0a87c0aa0833..a41cf4512a1c 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 @@ -25,17 +25,16 @@ import com.intellij.codeInspection.*; import com.intellij.codeInspection.dataFlow.DfaPsiUtil; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.Condition; import com.intellij.openapi.util.WriteExternalException; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.codeStyle.VariableKind; +import com.intellij.psi.impl.search.JavaNullMethodArgumentUtil; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.search.searches.OverridingMethodsSearch; import com.intellij.psi.util.*; import com.intellij.util.ArrayUtil; -import com.intellij.util.Function; import com.intellij.util.containers.ContainerUtil; import org.jdom.Element; import org.jetbrains.annotations.NotNull; @@ -44,6 +43,7 @@ import javax.swing.*; import java.util.ArrayList; import java.util.List; import java.util.Set; +import java.util.function.Function; public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionTool { // deprecated fields remain to minimize changes to users inspection profiles (which are often located in version control). @@ -58,6 +58,7 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo @Deprecated @SuppressWarnings({"WeakerAccess"}) public boolean REPORT_NOT_ANNOTATED_SETTER_PARAMETER = true; @Deprecated @SuppressWarnings({"WeakerAccess"}) public boolean REPORT_ANNOTATION_NOT_PROPAGATED_TO_OVERRIDERS = true; // remains for test @SuppressWarnings({"WeakerAccess"}) public boolean REPORT_NULLS_PASSED_TO_NON_ANNOTATED_METHOD = true; + public boolean REPORT_NULLS_PASSED_TO_NOT_NULL_PARAMETER = true; private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.nullable.NullableStuffInspectionBase"); @@ -85,7 +86,7 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo return new JavaElementVisitor() { @Override public void visitMethod(PsiMethod method) { - checkNullableStuffForMethod(method, holder); + checkNullableStuffForMethod(method, holder, isOnTheFly); } @Override @@ -148,6 +149,10 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo }; } + protected LocalQuickFix createNavigateToNullParameterUsagesFix(PsiParameter parameter) { + return null; + } + private static boolean nullabilityAnnotationsNotAvailable(final PsiFile file) { final Project project = file.getProject(); final GlobalSearchScope scope = GlobalSearchScope.allScope(project); @@ -378,7 +383,7 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo return "NullableProblems"; } - private void checkNullableStuffForMethod(PsiMethod method, final ProblemsHolder holder) { + private void checkNullableStuffForMethod(PsiMethod method, final ProblemsHolder holder, boolean isOnFly) { Annotated annotated = check(method, holder, method.getReturnType()); List superMethods = ContainerUtil.map( @@ -387,7 +392,7 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo final NullableNotNullManager nullableManager = NullableNotNullManager.getInstance(holder.getProject()); checkSupers(method, holder, annotated, superMethods, nullableManager); - checkParameters(method, holder, superMethods, nullableManager); + checkParameters(method, holder, superMethods, nullableManager, isOnFly); checkOverriders(method, holder, annotated, nullableManager); } @@ -419,14 +424,18 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo holder.registerProblem(method.getNameIdentifier(), InspectionsBundle.message("inspection.nullable.problems.method.overrides.NotNull", getPresentableAnnoName(superMethod)), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - wrapFix(fix)); + fix); break; } } } } - private void checkParameters(PsiMethod method, ProblemsHolder holder, List superMethods, NullableNotNullManager nullableManager) { + private void checkParameters(PsiMethod method, + ProblemsHolder holder, + List superMethods, + NullableNotNullManager nullableManager, + boolean isOnFly) { PsiParameter[] parameters = method.getParameterList().getParameters(); for (int i = 0; i < parameters.length; i++) { PsiParameter parameter = parameters[i]; @@ -462,7 +471,7 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo holder.registerProblem(parameter.getNameIdentifier(), InspectionsBundle.message("inspection.nullable.problems.parameter.overrides.NotNull", getPresentableAnnoName(superParameter)), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - wrapFix(fix)); + fix); break; } } @@ -477,11 +486,32 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo holder.registerProblem(physical ? notNullAnnotation : parameter.getNameIdentifier(), InspectionsBundle.message("inspection.nullable.problems.NotNull.parameter.overrides.not.annotated", getPresentableAnnoName(parameter)), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - wrapFix(fix)); + fix); break; } } } + + checkNullLiteralArgumentOfNotNullParameterUsages(method, holder, nullableManager, isOnFly, i, parameter); + } + } + + private void checkNullLiteralArgumentOfNotNullParameterUsages(PsiMethod method, + ProblemsHolder holder, + NullableNotNullManager nullableManager, + boolean isOnFly, + int parameterIdx, + PsiParameter parameter) { + if (REPORT_NULLS_PASSED_TO_NOT_NULL_PARAMETER && isNotNullNotInferred(parameter, false, false)) { + PsiAnnotation notNullAnnotation = nullableManager.getNotNullAnnotation(parameter, false); + if (JavaNullMethodArgumentUtil.hasNullArgument(method, parameterIdx)) { + LocalQuickFix[] fixes = getWrappedUiDependentQuickFix(this::createNavigateToNullParameterUsagesFix, parameter, isOnFly); + boolean physical = PsiTreeUtil.isAncestor(parameter, notNullAnnotation, true); + holder.registerProblem(physical ? notNullAnnotation : parameter.getNameIdentifier(), + InspectionsBundle.message("inspection.nullable.problems.NotNull.parameter.receives.null.literal", getPresentableAnnoName(parameter)), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + fixes); + } } } @@ -533,7 +563,7 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo } holder.registerProblem(psiElement, InspectionsBundle.message("nullable.stuff.problems.overridden.methods.are.not.annotated"), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - wrapFix(fix)); + fix); methodQuickFixSuggested = true; } if (hasAnnotatedParameter) { @@ -552,10 +582,10 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo holder.registerProblem(psiElement, InspectionsBundle.message("nullable.stuff.problems.overridden.method.parameters.are.not.annotated"), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - wrapFix(!applicable + !applicable ? createChangeDefaultNotNullFix(nullableManager, parameters[i]) : new AnnotateOverriddenMethodParameterFix(defaultNotNull, - nullableManager.getDefaultNullable()))); + nullableManager.getDefaultNullable())); parameterQuickFixSuggested[i] = true; } } @@ -565,6 +595,13 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo } } + @NotNull + public static LocalQuickFix[] getWrappedUiDependentQuickFix(Function fixSupplier, PsiParameter parameter, boolean isOnTheFly) { + if (!isOnTheFly) return LocalQuickFix.EMPTY_ARRAY; + final LocalQuickFix fix = fixSupplier.apply(parameter); + return fix == null ? LocalQuickFix.EMPTY_ARRAY : new LocalQuickFix[]{fix}; + } + private static boolean isNotNullNotInferred(@NotNull PsiModifierListOwner owner, boolean checkBases, boolean skipExternal) { Project project = owner.getProject(); NullableNotNullManager manager = NullableNotNullManager.getInstance(project); @@ -585,12 +622,6 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo return !(anno != null && AnnotationUtil.isInferredAnnotation(anno)); } - @NotNull - private static LocalQuickFix[] wrapFix(LocalQuickFix fix) { - if (fix == null) return LocalQuickFix.EMPTY_ARRAY; - return new LocalQuickFix[]{fix}; - } - private static LocalQuickFix createChangeDefaultNotNullFix(NullableNotNullManager nullableManager, PsiModifierListOwner modifierListOwner) { final PsiAnnotation annotation = AnnotationUtil.findAnnotation(modifierListOwner, nullableManager.getNotNulls()); if (annotation != null) { diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java index 0a097934ca48..df5e9c78e7b7 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java @@ -17,6 +17,7 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.NullableNotNullDialog; import com.intellij.codeInspection.*; +import com.intellij.codeInspection.nullable.NullableStuffInspection; import com.intellij.ide.DataManager; import com.intellij.openapi.actionSystem.CommonDataKeys; import com.intellij.openapi.project.Project; @@ -75,12 +76,18 @@ public class DataFlowInspection extends DataFlowInspectionBase { }; } + @Override + protected LocalQuickFix createNavigateToNullParameterUsagesFix(PsiParameter parameter) { + return new NullableStuffInspection.NavigateToNullLiteralArguments(parameter); + } + private class OptionsPanel extends JPanel { private final JCheckBox myIgnoreAssertions; private final JCheckBox myReportConstantReferences; private final JCheckBox mySuggestNullables; private final JCheckBox myDontReportTrueAsserts; private final JCheckBox myTreatUnknownMembersAsNullable; + private final JCheckBox myReportNullArguments; private OptionsPanel() { super(new GridBagLayout()); @@ -138,6 +145,15 @@ public class DataFlowInspection extends DataFlowInspectionBase { } }); + myReportNullArguments = new JCheckBox("Report 'null' literals passed to not-null required parameter"); + myTreatUnknownMembersAsNullable.setSelected(REPORT_NULLS_PASSED_TO_NOT_NULL_PARAMETER); + myTreatUnknownMembersAsNullable.getModel().addChangeListener(new ChangeListener() { + @Override + public void stateChanged(ChangeEvent e) { + REPORT_NULLS_PASSED_TO_NOT_NULL_PARAMETER = myTreatUnknownMembersAsNullable.isSelected(); + } + }); + gc.insets = JBUI.emptyInsets(); gc.gridy = 0; add(mySuggestNullables, gc); @@ -172,6 +188,9 @@ public class DataFlowInspection extends DataFlowInspectionBase { gc.gridy++; add(myTreatUnknownMembersAsNullable, gc); + + gc.gridy++; + add(myReportNullArguments, gc); } } diff --git a/java/java-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspection.java b/java/java-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspection.java index 125bb6a2c11b..d2a78919b3a3 100644 --- a/java/java-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspection.java @@ -16,10 +16,28 @@ package com.intellij.codeInspection.nullable; import com.intellij.codeInsight.NullableNotNullDialog; +import com.intellij.codeInspection.InspectionsBundle; +import com.intellij.codeInspection.LocalQuickFix; +import com.intellij.codeInspection.LocalQuickFixOnPsiElement; +import com.intellij.find.findUsages.PsiElement2UsageTargetAdapter; import com.intellij.ide.DataManager; import com.intellij.openapi.actionSystem.CommonDataKeys; +import com.intellij.openapi.application.ReadAction; import com.intellij.openapi.project.Project; import com.intellij.openapi.project.ProjectManager; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiFile; +import com.intellij.psi.PsiMethod; +import com.intellij.psi.PsiParameter; +import com.intellij.psi.impl.search.JavaNullMethodArgumentUtil; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.ui.components.JBCheckBox; +import com.intellij.usageView.UsageInfo; +import com.intellij.usages.*; +import com.intellij.util.ArrayUtil; +import com.intellij.util.Processor; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; import javax.swing.*; import java.awt.*; @@ -27,6 +45,11 @@ import java.awt.event.ActionEvent; import java.awt.event.ActionListener; public class NullableStuffInspection extends NullableStuffInspectionBase { + @Override + protected LocalQuickFix createNavigateToNullParameterUsagesFix(PsiParameter parameter) { + return new NavigateToNullLiteralArguments(parameter); + } + @Override public JComponent createOptionsPanel() { return new OptionsPanel(); @@ -41,6 +64,7 @@ public class NullableStuffInspection extends NullableStuffInspectionBase { private JCheckBox myIgnoreExternalSuperNotNull; private JCheckBox myNNParameterOverridesNA; private JCheckBox myRequireNNFieldsInitialized; + private JBCheckBox myReportNullLiteralsPassedNotNullParameter; private OptionsPanel() { super(new BorderLayout()); @@ -58,6 +82,7 @@ public class NullableStuffInspection extends NullableStuffInspectionBase { myReportNotAnnotatedGetter.addActionListener(actionListener); myIgnoreExternalSuperNotNull.addActionListener(actionListener); myRequireNNFieldsInitialized.addActionListener(actionListener); + myReportNullLiteralsPassedNotNullParameter.addActionListener(actionListener); myConfigureAnnotationsButton.addActionListener(new ActionListener() { @Override public void actionPerformed(ActionEvent e) { @@ -77,6 +102,7 @@ public class NullableStuffInspection extends NullableStuffInspectionBase { myIgnoreExternalSuperNotNull.setSelected(IGNORE_EXTERNAL_SUPER_NOTNULL); myNNParameterOverridesNA.setSelected(REPORT_NOTNULL_PARAMETERS_OVERRIDES_NOT_ANNOTATED); myRequireNNFieldsInitialized.setSelected(REQUIRE_NOTNULL_FIELDS_INITIALIZED); + myReportNullLiteralsPassedNotNullParameter.setSelected(REPORT_NULLS_PASSED_TO_NOT_NULL_PARAMETER); myIgnoreExternalSuperNotNull.setEnabled(myNAMethodOverridesNN.isSelected()); } @@ -88,9 +114,52 @@ public class NullableStuffInspection extends NullableStuffInspectionBase { IGNORE_EXTERNAL_SUPER_NOTNULL = myIgnoreExternalSuperNotNull.isSelected(); REPORT_NOTNULL_PARAMETERS_OVERRIDES_NOT_ANNOTATED = myNNParameterOverridesNA.isSelected(); REQUIRE_NOTNULL_FIELDS_INITIALIZED = myRequireNNFieldsInitialized.isSelected(); + REPORT_NULLS_PASSED_TO_NOT_NULL_PARAMETER = myReportNullLiteralsPassedNotNullParameter.isSelected(); REPORT_ANNOTATION_NOT_PROPAGATED_TO_OVERRIDERS = REPORT_NOT_ANNOTATED_METHOD_OVERRIDES_NOTNULL; myIgnoreExternalSuperNotNull.setEnabled(myNAMethodOverridesNN.isSelected()); } } + + public static class NavigateToNullLiteralArguments extends LocalQuickFixOnPsiElement { + public NavigateToNullLiteralArguments(@NotNull PsiParameter element) { + super(element); + } + + @NotNull + @Override + public String getText() { + return getFamilyName(); + } + + @Nls + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("nullable.stuff.inspection.navigate.null.argument.usages.fix.family.name"); + } + + @Override + public void invoke(@NotNull Project project, @NotNull PsiFile file, @NotNull PsiElement startElement, @NotNull PsiElement endElement) { + PsiParameter p = (PsiParameter)startElement; + final PsiMethod method = PsiTreeUtil.getParentOfType(p, PsiMethod.class); + if (method == null) return; + final int parameterIdx = ArrayUtil.find(method.getParameterList().getParameters(), p); + if (parameterIdx < 0) return; + + UsageViewPresentation presentation = new UsageViewPresentation(); + String title = InspectionsBundle.message("nullable.stuff.inspection.navigate.null.argument.usages.view.name", p.getName()); + presentation.setUsagesString(title); + presentation.setTabName(title); + presentation.setTabText(title); + UsageViewManager.getInstance(project).searchAndShowUsages( + new UsageTarget[]{new PsiElement2UsageTargetAdapter(method.getParameterList().getParameters()[parameterIdx])}, + () -> new UsageSearcher() { + @Override + public void generate(@NotNull final Processor processor) { + ReadAction.run(() -> JavaNullMethodArgumentUtil.searchNullArgument(method, parameterIdx, (arg) -> processor.process(new UsageInfo2UsageAdapter(new UsageInfo(arg))))); + } + }, false, false, presentation, null); + } + }; } diff --git a/java/java-impl/src/com/intellij/codeInspection/nullable/OptionsPanel.form b/java/java-impl/src/com/intellij/codeInspection/nullable/OptionsPanel.form index c9d38f447b36..c673bfa543b2 100644 --- a/java/java-impl/src/com/intellij/codeInspection/nullable/OptionsPanel.form +++ b/java/java-impl/src/com/intellij/codeInspection/nullable/OptionsPanel.form @@ -1,16 +1,16 @@
- + - + - + @@ -39,7 +39,7 @@ - + @@ -69,6 +69,14 @@ + + + + + + + + diff --git a/java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaNullMethodArgumentIndex.java b/java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaNullMethodArgumentIndex.java new file mode 100644 index 000000000000..6cec93e0fa9f --- /dev/null +++ b/java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaNullMethodArgumentIndex.java @@ -0,0 +1,238 @@ +/* + * Copyright 2000-2016 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.psi.impl.search; + +import com.intellij.ide.highlighter.JavaFileType; +import com.intellij.lang.LighterAST; +import com.intellij.lang.LighterASTNode; +import com.intellij.lang.LighterASTTokenNode; +import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.util.text.StringUtil; +import com.intellij.psi.JavaTokenType; +import com.intellij.psi.PsiKeyword; +import com.intellij.psi.impl.cache.RecordUtil; +import com.intellij.psi.impl.java.stubs.JavaStubElementTypes; +import com.intellij.psi.impl.source.tree.ElementType; +import com.intellij.psi.impl.source.tree.JavaElementType; +import com.intellij.psi.impl.source.tree.LightTreeUtil; +import com.intellij.psi.impl.source.tree.RecursiveLighterASTNodeWalkingVisitor; +import com.intellij.psi.tree.IElementType; +import com.intellij.util.containers.IntArrayList; +import com.intellij.util.indexing.*; +import com.intellij.util.io.DataInputOutputUtil; +import com.intellij.util.io.EnumeratorStringDescriptor; +import com.intellij.util.io.KeyDescriptor; +import gnu.trove.THashMap; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.io.DataInput; +import java.io.DataOutput; +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +public class JavaNullMethodArgumentIndex extends ScalarIndexExtension implements PsiDependentIndex { + private static final Logger LOG = Logger.getInstance(JavaNullMethodArgumentIndex.class); + + public static final ID INDEX_ID = ID.create("java.null.method.argument"); + + @NotNull + @Override + public ID getName() { + return INDEX_ID; + } + + @NotNull + @Override + public DataIndexer getIndexer() { + return inputData -> { + final CharSequence contentAsText = inputData.getContentAsText(); + if (!JavaStubElementTypes.JAVA_FILE.shouldBuildStubFor(inputData.getFile())) { + return Collections.emptyMap(); + } + if (!StringUtil.contains(contentAsText, PsiKeyword.NULL)) { + return Collections.emptyMap(); + } + + Map result = new THashMap<>(); + final LighterAST lighterAst = ((FileContentImpl)inputData).getLighterASTForPsiDependentIndex(); + + new RecursiveLighterASTNodeWalkingVisitor(lighterAst) { + @Override + public void visitNode(@NotNull LighterASTNode element) { + if (element.getTokenType() == JavaElementType.METHOD_CALL_EXPRESSION || + element.getTokenType() == JavaElementType.NEW_EXPRESSION || + element.getTokenType() == JavaElementType.ANONYMOUS_CLASS) { + final IntArrayList indices = getNullParameterIndices(element); + if (indices != null) { + final String name = getMethodName(element, element.getTokenType()); + if (name != null) { + for (int i = 0; i < indices.size(); i++) { + final int nullParameterIndex = indices.get(i); + result.put(new MethodCallData(name, nullParameterIndex), null); + } + } + } + } + super.visitNode(element); + } + + @Nullable + private IntArrayList getNullParameterIndices(@NotNull LighterASTNode methodCall) { + final LighterASTNode node = LightTreeUtil.firstChildOfType(lighterAst, methodCall, JavaElementType.EXPRESSION_LIST); + if (node == null) return null; + final List parameters = LightTreeUtil.getChildrenOfType(lighterAst, node, ElementType.EXPRESSION_BIT_SET); + IntArrayList indices = null; + for (int idx = 0; idx < parameters.size(); idx++) { + LighterASTNode parameter = parameters.get(idx); + if (parameter.getTokenType() == JavaElementType.LITERAL_EXPRESSION) { + final CharSequence literal = ((LighterASTTokenNode) lighterAst.getChildren(parameter).get(0)).getText(); + if (StringUtil.equals(literal, PsiKeyword.NULL)) { + if (indices == null) { + indices = new IntArrayList(1); + } + indices.add(idx); + } + } + } + return indices; + } + + @Nullable + private String getMethodName(@NotNull LighterASTNode call, IElementType elementType) { + if (elementType == JavaElementType.NEW_EXPRESSION || elementType == JavaElementType.ANONYMOUS_CLASS) { + final List refs = LightTreeUtil.getChildrenOfType(lighterAst, call, JavaElementType.JAVA_CODE_REFERENCE); + if (refs.isEmpty()) return null; + final LighterASTNode lastRef = refs.get(refs.size() - 1); + return getLastIdentifierText(lastRef); + } else { + LOG.assertTrue(elementType == JavaElementType.METHOD_CALL_EXPRESSION); + final LighterASTNode methodReference = lighterAst.getChildren(call).get(0); + if (methodReference.getTokenType() == JavaElementType.REFERENCE_EXPRESSION) { + return getLastIdentifierText(methodReference); + } + } + return null; + } + + @Nullable + private String getLastIdentifierText(LighterASTNode lastRef) { + final List identifiers = LightTreeUtil.getChildrenOfType(lighterAst, lastRef, JavaTokenType.IDENTIFIER); + if (identifiers.isEmpty()) return null; + final LighterASTNode methodNameIdentifier = identifiers.get(identifiers.size() - 1); + return RecordUtil.intern(lighterAst.getCharTable(), methodNameIdentifier); + } + }.visitNode(lighterAst.getRoot()); + + return result; + }; + } + + @NotNull + @Override + public KeyDescriptor getKeyDescriptor() { + return new KeyDescriptor() { + @Override + public int getHashCode(MethodCallData value) { + return value.hashCode(); + } + + @Override + public boolean isEqual(MethodCallData val1, MethodCallData val2) { + return val1.equals(val2); + } + + @Override + public void save(@NotNull DataOutput out, MethodCallData value) throws IOException { + EnumeratorStringDescriptor.INSTANCE.save(out, value.getMethodName()); + DataInputOutputUtil.writeINT(out, value.getNullParameterIndex()); + } + + @Override + public MethodCallData read(@NotNull DataInput in) throws IOException { + return new MethodCallData(EnumeratorStringDescriptor.INSTANCE.read(in), + DataInputOutputUtil.readINT(in)); + } + }; + } + + @Override + public int getVersion() { + return 0; + } + + @NotNull + @Override + public FileBasedIndex.InputFilter getInputFilter() { + return new DefaultFileTypeSpecificInputFilter(JavaFileType.INSTANCE); + } + + @Override + public boolean dependsOnFileContent() { + return true; + } + + public static final class MethodCallData { + @NotNull + private final String myMethodName; + private final int myNullParameterIndex; + + public MethodCallData(@NotNull String name, int index) { + myMethodName = name; + myNullParameterIndex = index; + } + + @NotNull + public String getMethodName() { + return myMethodName; + } + + public int getNullParameterIndex() { + return myNullParameterIndex; + } + + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + MethodCallData data = (MethodCallData)o; + + if (myNullParameterIndex != data.myNullParameterIndex) return false; + if (!myMethodName.equals(data.myMethodName)) return false; + + return true; + } + + @Override + public int hashCode() { + int result = myMethodName.hashCode(); + result = 31 * result + myNullParameterIndex; + return result; + } + + @Override + public String toString() { + return "MethodCallData{" + + "myMethodName='" + myMethodName + '\'' + + ", myNullParameterIndex=" + myNullParameterIndex + + '}'; + } + } +} diff --git a/java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaNullMethodArgumentUtil.java b/java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaNullMethodArgumentUtil.java new file mode 100644 index 000000000000..677fd5b375b4 --- /dev/null +++ b/java/java-indexing-impl/src/com/intellij/psi/impl/search/JavaNullMethodArgumentUtil.java @@ -0,0 +1,82 @@ +/* + * Copyright 2000-2016 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.psi.impl.search; + +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.*; +import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.psi.search.GlobalSearchScopeUtil; +import com.intellij.psi.search.searches.MethodReferencesSearch; +import com.intellij.util.CommonProcessors; +import com.intellij.util.Processor; +import com.intellij.util.indexing.FileBasedIndex; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; + +public class JavaNullMethodArgumentUtil { + + public static boolean hasNullArgument(@NotNull PsiMethod method, final int argumentIdx) { + final boolean[] result = {false}; + searchNullArgument(method, argumentIdx, expression -> { + result[0] = true; + return false; + }); + return result[0]; + } + + public static void searchNullArgument(@NotNull PsiMethod method, final int argumentIdx, @NotNull Processor nullArgumentProcessor) { + final GlobalSearchScope scope = findScopeWhereNullArgumentCanPass(method, argumentIdx); + if (scope == null) return; + MethodReferencesSearch.search(method, scope, true).forEach(ref -> { + final PsiElement psi = ref.getElement(); + if (psi != null) { + final PsiElement parent = psi.getParent(); + PsiExpressionList argumentList = null; + if (parent instanceof PsiCallExpression) { + argumentList = ((PsiCallExpression)parent).getArgumentList(); + } + else if (parent instanceof PsiAnonymousClass) { + argumentList = ((PsiAnonymousClass)parent).getArgumentList(); + } + if (argumentList != null) { + final PsiExpression[] arguments = argumentList.getExpressions(); + final PsiExpression argument = arguments[argumentIdx]; + if (argument instanceof PsiLiteralExpression && PsiKeyword.NULL.equals(argument.getText())) { + return nullArgumentProcessor.process(argument); + } + } + } + return true; + }); + } + + @Nullable + private static GlobalSearchScope findScopeWhereNullArgumentCanPass(@NotNull PsiMethod method, int parameterIndex) { + final FileBasedIndex fileBasedIndex = FileBasedIndex.getInstance(); + final CommonProcessors.CollectProcessor collector = new CommonProcessors.CollectProcessor<>(new ArrayList<>()); + fileBasedIndex.getFilesWithKey(JavaNullMethodArgumentIndex.INDEX_ID, + Collections.singleton(new JavaNullMethodArgumentIndex.MethodCallData(method.getName(), parameterIndex)), + collector, + GlobalSearchScopeUtil.toGlobalSearchScope(method.getUseScope(), method.getProject())); + final Collection candidateFiles = collector.getResults(); + return candidateFiles.isEmpty() ? null : GlobalSearchScope.filesScope(method.getProject(), candidateFiles); + } + +} diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/GenericParameterNullity.java b/java/java-tests/testData/inspection/dataFlow/fixture/GenericParameterNullity.java index 43c15c19eb70..3016898f93f9 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/GenericParameterNullity.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/GenericParameterNullity.java @@ -17,7 +17,7 @@ class Test { } @NotNull - private static T notNull(@Nullable T value) { + private static T notNull(@Nullable T value) { if (value == null) { throw new RuntimeException("null"); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentButParameterIsReassigned.java b/java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentButParameterIsReassigned.java new file mode 100644 index 000000000000..d4d8793d30e8 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentButParameterIsReassigned.java @@ -0,0 +1,17 @@ +import java.util.Objects; + +class Test { + + void dangerousMethod(String reassginedParameter) { + if (reassginedParameter == null) { + reassginedParameter = "default value"; + } + Objects.requireNonNull(reassginedParameter); + System.out.println(reassginedParameter); + } + + void usage() { + dangerousMethod(null); + } + +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentIsFailingMethodCall.java b/java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentIsFailingMethodCall.java new file mode 100644 index 000000000000..9fb4311e2185 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentIsFailingMethodCall.java @@ -0,0 +1,29 @@ +import java.util.Objects; + +class Test { + private static void testMethod(Object o, Object o2, Object o3, Object o4, int i) { + Objects.requireNonNull(o, "o is not null"); + if (o3 != null) { + System.out.println(o3.hashCode()); + } else { + throw new NullPointerException(); + } + + System.out.println(o); + for (int j = 0; j < i; j++) { + System.out.println(j); + Objects.requireNonNull(o4); + } + Objects.requireNonNull(o3); + + if (o4 == null) { + System.out.println("o4 is null"); + } else { + Objects.requireNonNull(o4); + } + } + + public static void main(String[] args) { + testMethod(null, null, null, null, 10); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentIsNotFailingMethodCall.java b/java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentIsNotFailingMethodCall.java new file mode 100644 index 000000000000..2a2146d80c39 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/NullArgumentIsNotFailingMethodCall.java @@ -0,0 +1,26 @@ +import java.util.Objects; + +class Test { + private static void testMethod(Object o, Object o2, Object o3, Object o4, int i) { + Objects.requireNonNull(o2, "no usages with null literal argument"); + if (i > 2016) { + Objects.requireNonNull(o, "o is not null"); + } + + System.out.println(o); + for (int j = 0; j < i; j++) { + System.out.println(j); + Objects.requireNonNull(o4); + } + + if (o4 == null) { + System.out.println("o4 is null"); + } else { + Objects.requireNonNull(o4); + } + } + + public static void main(String[] args) { + testMethod(null, "I am not null", null, null, 10); + } +} diff --git a/java/java-tests/testData/inspection/nullableProblems/NullPassedToNotNullConstructorParameter.java b/java/java-tests/testData/inspection/nullableProblems/NullPassedToNotNullConstructorParameter.java new file mode 100644 index 000000000000..a5f5800d2aab --- /dev/null +++ b/java/java-tests/testData/inspection/nullableProblems/NullPassedToNotNullConstructorParameter.java @@ -0,0 +1,43 @@ +import org.jetbrains.annotations.NotNull; + +class Main111 { + + Main111(@NotNull Object o) { + + } + + static class SubClass { + SubClass(@NotNull Object o) { + + } + } + + static class SubClass2 { + SubClass2(@NotNull Object o) { + + } + } + + static void main() { + new Main111(null); + new Main111.SubClass(null); + new SubClass2(null); + + new ParamerizedRunnable(null) { + @Override + void run() { + + } + }; + } + + abstract static class ParamerizedRunnable { + private Object parameter; + + public ParamerizedRunnable(@NotNull Object parameter) { + this.parameter = parameter; + } + + abstract void run(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/nullableProblems/NullPassedToNotNullParameter.java b/java/java-tests/testData/inspection/nullableProblems/NullPassedToNotNullParameter.java new file mode 100644 index 000000000000..2b0a29b4107b --- /dev/null +++ b/java/java-tests/testData/inspection/nullableProblems/NullPassedToNotNullParameter.java @@ -0,0 +1,24 @@ +import org.jetbrains.annotations.NotNull; + +class Test { + void someMethod(@NotNull Object warn, @NotNull Object o2, @NotNull Object warn1) { + + } + + static void someStaticMethod(Object notAnnotated, @NotNull Object o2, @NotNull Object warn2) { + + } + + public static void main(String[] args) { + someStaticMethod(null, "", ""); + + Test.someStaticMethod("", null, null); + + new Test().someMethod("", null, null); + + Test m = new Test(); + + m.someMethod(null, "", ""); + m.someMethod(null, "", ""); + } +} diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java index 653a90b39d81..0e766d54f1bd 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspection8Test.java @@ -50,6 +50,18 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { public void testPrimitiveInVoidLambda() { doTest(); } public void testNotNullLambdaParameter() { doTest(); } + public void testNullArgumentIsFailingMethodCall() { + doTest(); + } + + public void testNullArgumentIsNotFailingMethodCall() { + doTest(); + } + + public void testNullArgumentButParameterIsReassigned() { + doTest(); + } + public void testNullableArrayComponent() { setupCustomAnnotations(); DataFlowInspection inspection = new DataFlowInspection(); diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/NullableStuffInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/NullableStuffInspectionTest.java index 4747a86311e8..0ace266bdc75 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/NullableStuffInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/NullableStuffInspectionTest.java @@ -163,6 +163,14 @@ public class NullableStuffInspectionTest extends LightCodeInsightFixtureTestCase myFixture.checkHighlighting(true, false, true); } + public void testNullPassedToNotNullParameter() { + doTest(); + } + + public void testNullPassedToNotNullConstructorParameter() { + doTest(); + } + public void testHonorParameterDefaultInSetters() { DataFlowInspectionTest.addJavaxNullabilityAnnotations(myFixture); DataFlowInspectionTest.addJavaxDefaultNullabilityAnnotations(myFixture); diff --git a/java/java-tests/testSrc/com/intellij/psi/impl/search/JavaNullMethodArgumentIndexTest.kt b/java/java-tests/testSrc/com/intellij/psi/impl/search/JavaNullMethodArgumentIndexTest.kt new file mode 100644 index 000000000000..da95f505a91a --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/psi/impl/search/JavaNullMethodArgumentIndexTest.kt @@ -0,0 +1,103 @@ +/* + * Copyright 2000-2016 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.psi.impl.search + +import com.intellij.openapi.fileTypes.StdFileTypes +import com.intellij.testFramework.fixtures.LightPlatformCodeInsightFixtureTestCase +import com.intellij.util.indexing.FileContentImpl +import com.intellij.util.indexing.IndexingDataKeys +import org.intellij.lang.annotations.Language + +class JavaNullMethodArgumentIndexTest : LightPlatformCodeInsightFixtureTestCase() { + + fun testIndex() { + @Language("JAVA") + val file = myFixture.configureByText(StdFileTypes.JAVA, """ + package org.some; + + class Main111 { + + Main111(Object o) { + + } + + void someMethod(Object o, Object o2, Object o3) { + } + + static void staticMethod(Object o, Object o2, Object o3) { + } + + public static void main(String[] args) { + staticMethod(null, "", ""); + org.some.Main111.staticMethod("", "", null); + new Main111(null).someMethod("", "", null); + Main111 m = new Main111(null); + m.someMethod(null, "", ""); + } + + static class SubClass { + SubClass(Object o) { + + } + } + + static class SubClass2 { + SubClass2(Object o) { + + } + } + + static void main() { + new org.some.Main111(null); + new org.some.Main111.SubClass(null); + new SubClass2(null); + + + new ParametrizedRunnable(null) { + @Override + void run() { + + }}; + } + + abstract class ParametrizedRunnable { + Object parameter; + + ParametrizedRunnable(Object parameter){ + this.parameter = parameter; + } + + abstract void run(); + } + } + """).virtualFile + val content = FileContentImpl.createByFile(file) + content.putUserData(IndexingDataKeys.PROJECT, project) + val data = JavaNullMethodArgumentIndex().indexer.map(content).keys + + assertSize(8, data) + assertContainsElements(data, + JavaNullMethodArgumentIndex.MethodCallData("staticMethod", 0), + JavaNullMethodArgumentIndex.MethodCallData("staticMethod", 2), + JavaNullMethodArgumentIndex.MethodCallData("someMethod", 0), + JavaNullMethodArgumentIndex.MethodCallData("someMethod", 2), + JavaNullMethodArgumentIndex.MethodCallData("Main111", 0), + JavaNullMethodArgumentIndex.MethodCallData("SubClass", 0), + JavaNullMethodArgumentIndex.MethodCallData("SubClass2", 0), + JavaNullMethodArgumentIndex.MethodCallData("ParametrizedRunnable", 0)) + + } +} \ No newline at end of file diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index b73031a17390..9814b858462a 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -91,6 +91,7 @@ dataflow.message.return.nullable.from.notnullable=Expression #ref m dataflow.message.unboxing=Unboxing of #ref #loc may produce java.lang.NullPointerException dataflow.message.unboxing.method.reference=Use of #ref #loc would need unboxing which may produce java.lang.NullPointerException dataflow.too.complex=Method #ref is too complex to analyze by data flow algorithm +dataflow.method.fails.with.null.argument=Method fails when parameter ''{0}'' is null #deprecated inspection.deprecated.display.name=Deprecated API usage @@ -153,6 +154,7 @@ inspection.nullable.problems.NotNull.parameter.overrides.Nullable=Parameter anno inspection.nullable.problems.NotNull.parameter.overrides.not.annotated=Parameter annotated @{0} should not override non-annotated parameter inspection.nullable.problems.parameter.overrides.NotNull=Not annotated parameter overrides @{0} parameter inspection.nullable.problems.primitive.type.annotation=Primitive type members cannot be annotated +inspection.nullable.problems.NotNull.parameter.receives.null.literal=Parameter annotated @{0} should not receive 'null' as an argument inspection.test.only.problems.display.name=Test-only class or method call in production code inspection.test.only.problems.test.only.method.call=Test-only method is called in production code @@ -724,3 +726,5 @@ inspection.annotate.method.quickfix.family.name=Annotate method inspection.tool.window.dialog.title=Inspection Tool Window inspection.tool.window.dialog.no.options=Inspection ''{0}'' has no configurable options inspection.tool.window.inspection.dialog.title=Inspection ''{0}'' options +nullable.stuff.inspection.navigate.null.argument.usages.fix.family.name=Navigate to 'null' argument usages +nullable.stuff.inspection.navigate.null.argument.usages.view.name=''null'' argument usages for parameter {0} \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index 0a7955e720fa..c377e0b1aac9 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -1762,6 +1762,8 @@ implementationClass="com.intellij.refactoring.introduceparameterobject.JavaIntroduceParameterObjectDelegate"/> + +