DataFlowInspection: highlight method references known to return constant true/false value with quick-fix changing them to trivial lambdas (partially fixes IDEA-170885).

This commit is contained in:
Tagir Valeev
2017-04-26 12:41:40 +07:00
parent e018876a4c
commit 9127146364
13 changed files with 347 additions and 37 deletions

View File

@@ -34,6 +34,7 @@ import com.intellij.codeInsight.intention.impl.AddNullableAnnotationFix;
import com.intellij.codeInspection.*;
import com.intellij.codeInspection.dataFlow.instructions.*;
import com.intellij.codeInspection.dataFlow.value.DfaConstValue;
import com.intellij.codeInspection.dataFlow.value.DfaUnknownValue;
import com.intellij.codeInspection.dataFlow.value.DfaValue;
import com.intellij.codeInspection.nullable.NullableStuffInspectionBase;
import com.intellij.lang.java.JavaLanguage;
@@ -292,6 +293,10 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
return null;
}
protected LocalQuickFix createReplaceWithTrivialLambdaFix(Object value) {
return null;
}
protected void addSurroundWithIfFix(PsiExpression qualifier, List<LocalQuickFix> fixes, boolean onTheFly) {
}
@@ -346,13 +351,22 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
reportUncheckedOptionalGet(holder, visitor.getOptionalCalls(), visitor.getOptionalQualifiers());
Map<PsiMethodCallExpression, ThreeState> calls = visitor.getBooleanCalls();
calls.forEach((call, state) -> {
visitor.getBooleanCalls().forEach((call, state) -> {
if (state != ThreeState.UNSURE && reportedAnchors.add(call)) {
reportConstantCondition(holder, visitor, call, state.toBoolean());
}
});
visitor.getMethodReferenceResults().forEach((methodRef, dfaValue) -> {
if (dfaValue instanceof DfaConstValue) {
Object value = ((DfaConstValue)dfaValue).getValue();
if(value instanceof Boolean) {
holder.registerProblem(methodRef, InspectionsBundle.message("dataflow.message.constant.method.reference", value),
createReplaceWithTrivialLambdaFix(value));
}
}
});
if (REPORT_CONSTANT_REFERENCE_VALUES) {
reportConstantReferenceValues(holder, visitor, reportedAnchors);
}
@@ -970,6 +984,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
private final Map<MethodCallInstruction, Boolean> myFailingCalls = new HashMap<>();
private final Map<PsiMethodCallExpression, ThreeState> myOptionalCalls = new HashMap<>();
private final Map<PsiMethodCallExpression, ThreeState> myBooleanCalls = new HashMap<>();
private final Map<PsiMethodReferenceExpression, DfaValue> myMethodReferenceResults = new HashMap<>();
private final List<PsiExpression> myOptionalQualifiers = new ArrayList<>();
private boolean myAlwaysReturnsNotNull = true;
@@ -996,6 +1011,10 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
return myBooleanCalls;
}
Map<PsiMethodReferenceExpression, DfaValue> getMethodReferenceResults() {
return myMethodReferenceResults;
}
List<PsiExpression> getOptionalQualifiers() {
return myOptionalQualifiers;
}
@@ -1061,6 +1080,12 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
}
}
@Override
protected void processMethodReferenceResult(PsiMethodReferenceExpression methodRef, DfaValue res) {
// Do not track if method reference may have different results
myMethodReferenceResults.merge(methodRef, res, (a, b) -> a == b ? a : DfaUnknownValue.getInstance());
}
private static boolean hasNonTrivialFailingContracts(MethodCallInstruction instruction) {
List<MethodContract> contracts = instruction.getContracts();
return !contracts.isEmpty() && contracts.stream().anyMatch(

View File

@@ -103,21 +103,34 @@ public class DfaPsiUtil {
if (PsiJavaPatterns.psiParameter().withParents(PsiParameterList.class, PsiLambdaExpression.class).accepts(owner)) {
PsiLambdaExpression lambda = (PsiLambdaExpression)owner.getParent().getParent();
int index = lambda.getParameterList().getParameterIndex((PsiParameter)owner);
Nullness nullness = inferLambdaParameterNullness(lambda, index);
if(nullness != Nullness.UNKNOWN) {
return nullness;
}
PsiMethod sam = LambdaUtil.getFunctionalInterfaceMethod(lambda.getFunctionalInterfaceType());
if (sam != null && index < sam.getParameterList().getParametersCount()) {
return getElementNullability(null, sam.getParameterList().getParameters()[index]);
}
return getFunctionalParameterNullability(lambda, index);
}
return Nullness.UNKNOWN;
}
/**
* Returns the nullness of functional expression parameter
*
* @param function functional expression
* @param index parameter index
* @return nullness, defined by SAM parameter annotations or known otherwise
*/
@NotNull
private static Nullness inferLambdaParameterNullness(PsiLambdaExpression lambda, int parameterIndex) {
public static Nullness getFunctionalParameterNullability(PsiFunctionalExpression function, int index) {
Nullness nullness = inferLambdaParameterNullness(function, index);
if(nullness != Nullness.UNKNOWN) {
return nullness;
}
PsiMethod sam = LambdaUtil.getFunctionalInterfaceMethod(function.getFunctionalInterfaceType());
if (sam != null && index < sam.getParameterList().getParametersCount()) {
return getElementNullability(null, sam.getParameterList().getParameters()[index]);
}
return Nullness.UNKNOWN;
}
@NotNull
private static Nullness inferLambdaParameterNullness(PsiFunctionalExpression lambda, int parameterIndex) {
PsiElement expression = lambda;
PsiElement expressionParent = lambda.getParent();
while(expressionParent instanceof PsiConditionalExpression || expressionParent instanceof PsiParenthesizedExpression) {
@@ -204,7 +217,7 @@ public class DfaPsiUtil {
if (body == null) return Collections.emptySet();
return CachedValuesManager.getCachedValue(constructor, new CachedValueProvider<Set<PsiField>>() {
@Nullable
@NotNull
@Override
public Result<Set<PsiField>> compute() {
final PsiCodeBlock body = constructor.getBody();
@@ -298,7 +311,7 @@ public class DfaPsiUtil {
}
return CachedValuesManager.getCachedValue(psiClass, new CachedValueProvider<MultiMap<PsiField, PsiExpression>>() {
@Nullable
@NotNull
@Override
public Result<MultiMap<PsiField, PsiExpression>> compute() {
final Set<String> fieldNames = ContainerUtil.newHashSet();

View File

@@ -19,6 +19,7 @@ import com.intellij.codeInspection.dataFlow.value.DfaConstValue;
import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType;
import com.intellij.codeInspection.dataFlow.value.DfaValue;
import com.intellij.codeInspection.dataFlow.value.DfaValueFactory;
import com.intellij.psi.PsiType;
import org.jetbrains.annotations.Nullable;
import java.util.List;
@@ -41,6 +42,25 @@ public abstract class MethodContract {
*/
public abstract ValueConstraint getReturnValue();
/**
* Returns DfaValue describing the return value of this contract or null if this contract assumes that any value could be returned.
*
* @param factory factory to create values
* @param resultType method result type
* @return a DfaValue describing the return value of this contract
*/
@Nullable
DfaValue getDfaReturnValue(DfaValueFactory factory, PsiType resultType) {
switch (getReturnValue()) {
case NULL_VALUE: return factory.getConstFactory().getNull();
case NOT_NULL_VALUE: return factory.createTypeValue(resultType, Nullness.NOT_NULL);
case TRUE_VALUE: return factory.getConstFactory().getTrue();
case FALSE_VALUE: return factory.getConstFactory().getFalse();
case THROW_EXCEPTION: return factory.getConstFactory().getContractFail();
default: return null;
}
}
/**
* @return true if this contract result does not depend on arguments
*/

View File

@@ -128,10 +128,70 @@ public class StandardInstructionVisitor extends InstructionVisitor {
if (!checkNotNullable(memState, qualifier, NullabilityProblem.fieldAccessNPE, instruction.getElementToAssert())) {
forceNotNull(runner, memState, qualifier);
}
PsiElement parent = instruction.getExpression().getParent();
if (parent instanceof PsiMethodReferenceExpression) {
handleMethodReference(qualifier, (PsiMethodReferenceExpression)parent, runner, memState);
}
return nextInstruction(instruction, runner, memState);
}
private void handleMethodReference(DfaValue qualifier,
PsiMethodReferenceExpression methodRef,
DataFlowRunner runner,
DfaMemoryState state) {
PsiType functionalInterfaceType = methodRef.getFunctionalInterfaceType();
if (functionalInterfaceType == null) return;
PsiMethod sam = LambdaUtil.getFunctionalInterfaceMethod(functionalInterfaceType);
if (sam == null || PsiType.VOID.equals(sam.getReturnType())) return;
JavaResolveResult resolveResult = methodRef.advancedResolve(false);
PsiMethod method = ObjectUtils.tryCast(resolveResult.getElement(), PsiMethod.class);
if (method == null || !ControlFlowAnalyzer.isPure(method)) return;
List<? extends MethodContract> contracts = HardcodedContracts.getHardcodedContracts(method, null);
if (contracts.isEmpty()) {
contracts = ControlFlowAnalyzer.getMethodContracts(method);
}
if (contracts.isEmpty()) return;
PsiSubstitutor substitutor = resolveResult.getSubstitutor();
PsiParameter[] samParameters = sam.getParameterList().getParameters();
boolean isStatic = method.hasModifierProperty(PsiModifier.STATIC);
boolean instanceBound = !isStatic && !PsiMethodReferenceUtil.isStaticallyReferenced(methodRef);
PsiParameter[] parameters = method.getParameterList().getParameters();
DfaValue[] arguments = new DfaValue[parameters.length];
Arrays.fill(arguments, DfaUnknownValue.getInstance());
for (int i = 0; i < samParameters.length; i++) {
DfaValue value = runner.getFactory()
.createTypeValue(substitutor.substitute(samParameters[i].getType()), DfaPsiUtil.getFunctionalParameterNullability(methodRef, i));
if (i == 0 && !isStatic && !instanceBound) {
qualifier = value;
}
else {
int idx = i - ((isStatic || instanceBound) ? 0 : 1);
if (idx >= arguments.length) break;
if (!(parameters[idx].getType() instanceof PsiEllipsisType)) {
arguments[idx] = value;
}
}
}
LinkedHashSet<DfaMemoryState> currentStates = ContainerUtil.newLinkedHashSet(state.createClosureState());
Set<DfaMemoryState> finalStates = ContainerUtil.newLinkedHashSet();
PsiType returnType = substitutor.substitute(method.getReturnType());
DfaValue defaultResult = runner.getFactory().createTypeValue(returnType, DfaPsiUtil.getElementNullability(returnType, method));
for (MethodContract contract : contracts) {
DfaValue result = contract.getDfaReturnValue(runner.getFactory(), returnType);
if (result == null) {
result = defaultResult;
}
currentStates = addContractResults(qualifier, arguments, contract, currentStates, runner.getFactory(), finalStates, result);
}
StreamEx.of(finalStates).map(DfaMemoryState::peek)
.append(currentStates.isEmpty() ? StreamEx.empty() : StreamEx.of(defaultResult))
.distinct().forEach(res -> processMethodReferenceResult(methodRef, res));
}
protected void processMethodReferenceResult(PsiMethodReferenceExpression methodRef, DfaValue res) {
}
@Override
public DfaInstructionState[] visitPush(PushInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
PsiExpression place = instruction.getPlace();
@@ -212,7 +272,11 @@ public class StandardInstructionVisitor extends InstructionVisitor {
LinkedHashSet<DfaMemoryState> currentStates = ContainerUtil.newLinkedHashSet(memState);
if (argValues != null) {
for (MethodContract contract : instruction.getContracts()) {
currentStates = addContractResults(qualifier, argValues, contract, currentStates, instruction, runner.getFactory(), finalStates);
DfaValue returnValue = contract.getDfaReturnValue(runner.getFactory(), instruction.getResultType());
if (returnValue == null) {
returnValue = getMethodResultValue(instruction, qualifier, runner.getFactory());
}
currentStates = addContractResults(qualifier, argValues, contract, currentStates, runner.getFactory(), finalStates, returnValue);
if (currentStates.size() + finalStates.size() > DataFlowRunner.MAX_STATES_PER_BRANCH) {
if (LOG.isDebugEnabled()) {
LOG.debug("Too complex contract on " + instruction.getContext() + ", skipping contract processing");
@@ -376,17 +440,17 @@ public class StandardInstructionVisitor extends InstructionVisitor {
return qualifier;
}
private LinkedHashSet<DfaMemoryState> addContractResults(DfaValue qualifier,
DfaValue[] argValues,
MethodContract contract,
LinkedHashSet<DfaMemoryState> states,
MethodCallInstruction instruction,
DfaValueFactory factory,
Set<DfaMemoryState> finalStates) {
private static LinkedHashSet<DfaMemoryState> addContractResults(DfaValue qualifier,
DfaValue[] argValues,
MethodContract contract,
LinkedHashSet<DfaMemoryState> states,
DfaValueFactory factory,
Set<DfaMemoryState> finalStates,
DfaValue returnValue) {
List<DfaValue> conditions = contract.getConditions(factory, qualifier, argValues);
if (StreamEx.of(conditions).allMatch(factory.getConstFactory().getTrue()::equals)) {
for (DfaMemoryState state : states) {
state.push(getDfaContractReturnValue(contract, instruction, factory));
state.push(returnValue);
finalStates.add(state);
}
return new LinkedHashSet<>();
@@ -418,26 +482,13 @@ public class StandardInstructionVisitor extends InstructionVisitor {
}
for (DfaMemoryState state : trueStates) {
state.push(getDfaContractReturnValue(contract, instruction, factory));
state.push(returnValue);
finalStates.add(state);
}
return falseStates;
}
private DfaValue getDfaContractReturnValue(MethodContract contract,
MethodCallInstruction instruction,
DfaValueFactory factory) {
switch (contract.getReturnValue()) {
case NULL_VALUE: return factory.getConstFactory().getNull();
case NOT_NULL_VALUE: return factory.createTypeValue(instruction.getResultType(), Nullness.NOT_NULL);
case TRUE_VALUE: return factory.getConstFactory().getTrue();
case FALSE_VALUE: return factory.getConstFactory().getFalse();
case THROW_EXCEPTION: return factory.getConstFactory().getContractFail();
default: return getMethodResultValue(instruction, null, factory);
}
}
private static void forceNotNull(DataFlowRunner runner, DfaMemoryState memState, DfaValue arg) {
if (arg instanceof DfaVariableValue) {
DfaVariableValue var = (DfaVariableValue)arg;

View File

@@ -0,0 +1,62 @@
/*
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.intellij.codeInspection;
import com.intellij.openapi.project.Project;
import com.intellij.psi.JavaPsiFacade;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiLambdaExpression;
import com.intellij.psi.PsiMethodReferenceExpression;
import com.intellij.refactoring.util.LambdaRefactoringUtil;
import com.intellij.util.ObjectUtils;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
/**
* @author Tagir Valeev
*/
public class ReplaceWithTrivialLambdaFix implements LocalQuickFix {
private final String myValue;
public ReplaceWithTrivialLambdaFix(Object value) {
myValue = String.valueOf(value);
}
@Nls
@NotNull
@Override
public String getName() {
return InspectionsBundle.message("inspection.replace.with.trivial.lambda.fix.name", myValue);
}
@Nls
@NotNull
@Override
public String getFamilyName() {
return InspectionsBundle.message("inspection.replace.with.trivial.lambda.fix.family.name");
}
@Override
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
PsiMethodReferenceExpression methodRef = ObjectUtils.tryCast(descriptor.getStartElement(), PsiMethodReferenceExpression.class);
if (methodRef == null) return;
PsiLambdaExpression lambdaExpression = LambdaRefactoringUtil.convertMethodReferenceToLambda(methodRef, true, true);
if (lambdaExpression == null) return;
PsiElement body = lambdaExpression.getBody();
if (body == null) return;
body.replace(JavaPsiFacade.getElementFactory(project).createExpressionFromText(myValue, lambdaExpression));
}
}

View File

@@ -56,6 +56,11 @@ public class DataFlowInspection extends DataFlowInspectionBase {
return RefactoringUtil.getParentStatement(expression, false) == null ? null : new AddAssertStatementFix(binary);
}
@Override
protected LocalQuickFix createReplaceWithTrivialLambdaFix(Object value) {
return new ReplaceWithTrivialLambdaFix(value);
}
@Override
protected LocalQuickFix createIntroduceVariableFix(final PsiExpression expression) {
return new IntroduceVariableFix(true);

View File

@@ -0,0 +1,29 @@
// "Fix all 'Constant conditions & exceptions' problems in file" "true"
import org.jetbrains.annotations.*;
import java.util.*;
import java.util.stream.Stream;
public class MethodReferenceConstantValue {
@Contract(value = "!null -> false", pure = true)
public boolean strangeMethod(String s) {
return s == null ? new Random().nextBoolean() : false;
}
public void test(Optional<String> opt) {
X x = (methodReferenceConstantValue, s) -> false;
Boolean aBoolean = opt.map(s -> false)
.map(o -> true)
.map(o -> false)
.orElse(false);
if (opt.isPresent()) {
Stream.generate(() -> true)
.limit(10)
.forEach(System.out::println);
}
}
interface X {
boolean action(@Nullable MethodReferenceConstantValue a, @NotNull String b);
}
}

View File

@@ -0,0 +1,29 @@
// "Fix all 'Constant conditions & exceptions' problems in file" "true"
import org.jetbrains.annotations.*;
import java.util.*;
import java.util.stream.Stream;
public class MethodReferenceConstantValue {
@Contract(value = "!null -> false", pure = true)
public boolean strangeMethod(String s) {
return s == null ? new Random().nextBoolean() : false;
}
public void test(Optional<String> opt) {
X x = MethodReferenceConstantValue::strangeMethod;
Boolean aBoolean = opt.map(th<caret>is::strangeMethod)
.map(Objects::nonNull)
.map(Objects::isNull)
.orElse(false);
if (opt.isPresent()) {
Stream.generate(opt::isPresent)
.limit(10)
.forEach(System.out::println);
}
}
interface X {
boolean action(@Nullable MethodReferenceConstantValue a, @NotNull String b);
}
}

View File

@@ -0,0 +1,28 @@
import org.jetbrains.annotations.*;
import java.util.*;
import java.util.stream.Stream;
public class MethodReferenceConstantValue {
@Contract(value = "!null -> false", pure = true)
public boolean strangeMethod(String s) {
return s == null ? new Random().nextBoolean() : false;
}
public void test(Optional<String> opt) {
X x = <warning descr="Method reference result is always 'false'">MethodReferenceConstantValue::strangeMethod</warning>;
Boolean aBoolean = opt.map(<warning descr="Method reference result is always 'false'">this::strangeMethod</warning>)
.map(<warning descr="Method reference result is always 'true'">Objects::nonNull</warning>)
.map(<warning descr="Method reference result is always 'false'">Objects::isNull</warning>)
.orElse(false);
if (opt.isPresent()) {
Stream.generate(<warning descr="Method reference result is always 'true'">opt::isPresent</warning>)
.limit(10)
.forEach(System.out::println);
}
}
interface X {
boolean action(@Nullable MethodReferenceConstantValue a, @NotNull String b);
}
}

View File

@@ -0,0 +1,42 @@
/*
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/*
* User: anna
* Date: 21-Mar-2008
*/
package com.intellij.codeInsight.daemon.quickFix;
import com.intellij.codeInspection.LocalInspectionTool;
import com.intellij.codeInspection.dataFlow.DataFlowInspection;
import org.jetbrains.annotations.NotNull;
public class ReplaceWithTrivialLambdaFixTest extends LightQuickFixParameterizedTestCase {
@NotNull
@Override
protected LocalInspectionTool[] configureLocalInspectionTools() {
return new LocalInspectionTool[]{new DataFlowInspection()};
}
public void test() throws Exception {
doAllTests();
}
@Override
protected String getBasePath() {
return "/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTrivialLambda";
}
}

View File

@@ -59,6 +59,7 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase {
public void testNullableVoidLambda() { doTest(); }
public void testNullableForeachVariable() { doTestWithCustomAnnotations(); }
public void testGenericParameterNullity() { doTestWithCustomAnnotations(); }
public void testMethodReferenceConstantValue() { doTest(); }
public void testOptionalOfNullable() { doTest(); }
public void testOptionalOrElse() { doTest(); }

View File

@@ -55,6 +55,7 @@ public class DataFlowInspectionTestSuite {
suite.addTestSuite(ReplaceWithObjectsEqualsTest.class);
suite.addTestSuite(ReplaceWithOfNullableFixTest.class);
suite.addTestSuite(ReplaceFromOfNullableFixTest.class);
suite.addTestSuite(ReplaceWithTrivialLambdaFixTest.class);
suite.addTestSuite(UnwrapIfStatementFixTest.class);
return suite;
}

View File

@@ -92,6 +92,7 @@ dataflow.message.unboxing.method.reference=Use of <code>#ref</code> #loc would n
dataflow.too.complex=Method <code>#ref</code> is too complex to analyze by data flow algorithm
dataflow.method.fails.with.null.argument=Method will throw an exception when parameter is null
dataflow.message.optional.get.without.is.present=<code>{0}.#ref()</code> without ''isPresent()'' check
dataflow.message.constant.method.reference=Method reference result is always ''{0}''
#deprecated
inspection.deprecated.display.name=Deprecated API usage
@@ -845,4 +846,7 @@ inspection.reflection.member.access.cannot.resolve.constructor.arguments=Cannot
inspection.reflection.member.access.constructor.not.public=Constructor is not public
inspection.reflection.member.access.check.exists=Check that field/method exists in non-final classes
inspection.reflection.member.access.check.exists.exclude=Exclude classes
inspection.reflection.member.access.check.exists.exclude.chooser=Class to exclude
inspection.reflection.member.access.check.exists.exclude.chooser=Class to exclude
inspection.replace.with.trivial.lambda.fix.family.name=Replace with trivial lambda
inspection.replace.with.trivial.lambda.fix.name=Replace with lambda returning ''{0}''