[java-dfa] Minor improvements in inlining

1. Report always failing method references
2. Fix boxing in MapUpdateInliner
3. Inline List.of(...).forEach() and Arrays.asList(...).forEach()

GitOrigin-RevId: 0e2df9c1b90ccf253d2c9f74c7c7543732c9b103
This commit is contained in:
Tagir Valeev
2023-06-02 10:11:41 +00:00
committed by intellij-monorepo-bot
parent da9763db72
commit 10c86955e7
6 changed files with 124 additions and 63 deletions
@@ -606,10 +606,12 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
}
private void reportAlwaysFailingCalls(ProblemReporter reporter, DataFlowInstructionVisitor visitor) {
visitor.alwaysFailingCalls().remove(TestUtils::isExceptionExpected).forEach(call -> {
String message = getContractMessage(JavaMethodContractUtil.getMethodCallContracts(call));
LocalQuickFix causeFix = createExplainFix(call, new TrackingRunner.FailingCallDfaProblemType());
reporter.registerProblem(getElementToHighlight(call), message, LocalQuickFix.notNullElements(causeFix));
visitor.alwaysFailingCalls().remove(TestUtils::isExceptionExpected).forEach(anchor -> {
List<? extends MethodContract> contracts = DataFlowInstructionVisitor.getContracts(anchor);
if (contracts == null) return;
String message = getContractMessage(contracts);
LocalQuickFix causeFix = createExplainFix(anchor, new TrackingRunner.FailingCallDfaProblemType());
reporter.registerProblem(getElementToHighlight(anchor), message, LocalQuickFix.notNullElements(causeFix));
});
}
@@ -620,22 +622,22 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
return JavaAnalysisBundle.message("dataflow.message.contract.fail");
}
private static @NotNull PsiElement getElementToHighlight(@NotNull PsiCall call) {
private static @NotNull PsiElement getElementToHighlight(@NotNull PsiElement element) {
PsiJavaCodeReferenceElement ref;
if (call instanceof PsiNewExpression) {
ref = ((PsiNewExpression)call).getClassReference();
if (element instanceof PsiNewExpression newExpression) {
ref = newExpression.getClassReference();
}
else if (call instanceof PsiMethodCallExpression) {
ref = ((PsiMethodCallExpression)call).getMethodExpression();
else if (element instanceof PsiMethodCallExpression callExpression) {
ref = callExpression.getMethodExpression();
}
else {
return call;
return element;
}
if (ref != null) {
PsiElement name = ref.getReferenceNameElement();
return name != null ? name : ref;
}
return call;
return element;
}
private void reportNullableArgumentPassedToNonAnnotatedMethodRef(@NotNull ProblemReporter reporter,
@@ -195,8 +195,8 @@ final class DataFlowInstructionVisitor implements JavaDfaListener {
(element, info) -> info.alwaysFails());
}
StreamEx<PsiCallExpression> alwaysFailingCalls() {
return StreamEx.ofKeys(myFailingCalls, v -> v).map(ContractFailureProblem::getAnchor).select(PsiCallExpression.class).distinct();
StreamEx<PsiExpression> alwaysFailingCalls() {
return StreamEx.ofKeys(myFailingCalls, v -> v).map(ContractFailureProblem::getAnchor).distinct();
}
boolean isAlwaysReturnsNotNull(Instruction[] instructions) {
@@ -326,11 +326,9 @@ final class DataFlowInstructionVisitor implements JavaDfaListener {
myArrayStoreProblems.put(storeProblem.getAnchor(), Pair.create(storeProblem.getFromType(), storeProblem.getToType()));
}
else if (problem instanceof ContractFailureProblem contractFailure) {
if (contractFailure.getAnchor() instanceof PsiCallExpression call) {
Boolean isFailing = myFailingCalls.get(problem);
if (isFailing != null || !hasTrivialFailContract(call)) {
myFailingCalls.put(contractFailure, failed == ThreeState.YES && !Boolean.FALSE.equals(isFailing));
}
Boolean isFailing = myFailingCalls.get(problem);
if (isFailing != null || !hasTrivialFailContract(contractFailure.getAnchor())) {
myFailingCalls.put(contractFailure, failed == ThreeState.YES && !Boolean.FALSE.equals(isFailing));
}
}
else if (problem instanceof NullabilityProblemKind.NullabilityProblem<?> nullabilityProblem) {
@@ -352,9 +350,9 @@ final class DataFlowInstructionVisitor implements JavaDfaListener {
myEndOfInitializerStates.add(state.createCopy());
}
private static boolean hasTrivialFailContract(PsiCallExpression call) {
List<? extends MethodContract> contracts = JavaMethodContractUtil.getMethodCallContracts(call);
return contracts.size() == 1 && contracts.get(0).isTrivial() && contracts.get(0).getReturnValue().isFail();
private static boolean hasTrivialFailContract(@NotNull PsiExpression call) {
List<? extends MethodContract> contracts = getContracts(call);
return contracts != null && contracts.size() == 1 && contracts.get(0).isTrivial() && contracts.get(0).getReturnValue().isFail();
}
private void reportMutabilityViolation(boolean receiver, @NotNull PsiElement anchor) {
@@ -374,6 +372,21 @@ final class DataFlowInstructionVisitor implements JavaDfaListener {
}
}
@Nullable
static List<? extends MethodContract> getContracts(@NotNull PsiExpression anchor) {
List<? extends MethodContract> contracts;
if (anchor instanceof PsiCallExpression call) {
contracts = JavaMethodContractUtil.getMethodCallContracts(call);
}
else if (anchor instanceof PsiMethodReferenceExpression methodRef && methodRef.resolve() instanceof PsiMethod method) {
contracts = JavaMethodContractUtil.getMethodContracts(method);
}
else {
return null;
}
return contracts;
}
private static class StateInfo {
boolean ephemeralException;
boolean normalException;
@@ -1,6 +1,4 @@
/*
* Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
*/
// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package com.intellij.codeInspection.dataFlow.java.inliner;
import com.intellij.codeInsight.Nullability;
@@ -14,6 +12,7 @@ import com.intellij.psi.CommonClassNames;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.PsiMethodCallExpression;
import com.intellij.psi.PsiType;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.ig.callMatcher.CallMatcher;
import com.siyeh.ig.psiutils.ExpectedTypeUtils;
import org.jetbrains.annotations.NotNull;
@@ -40,6 +39,7 @@ public class MapUpdateInliner implements CallInliner {
.pushExpression(qualifier) // stack: .. qualifier
.ensure(RelationType.IS, Mutability.MUTABLE.asDfType(), new MutabilityProblem(call, true), null)
.pushExpression(key) // stack: .. qualifier; key
.boxUnbox(key, PsiUtil.substituteTypeParameter(qualifier.getType(), CommonClassNames.JAVA_UTIL_MAP, 0, true))
.evaluateFunction(function);
String name = Objects.requireNonNull(call.getMethodExpression().getReferenceName());
switch (name) {
@@ -1,18 +1,4 @@
/*
* 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.
*/
// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package com.intellij.codeInspection.dataFlow.java.inliner;
import com.intellij.codeInsight.Nullability;
@@ -56,6 +42,8 @@ public class StreamChainInliner implements CallInliner {
"average", "forEach", "forEachOrdered", "min", "max", "toList", "toSet"};
private static final CallMatcher TERMINAL_CALL = instanceCall(JAVA_UTIL_STREAM_BASE_STREAM, TERMINALS);
private static final CallMatcher FOR_COLLECTION_TERMINAL = instanceCall(JAVA_LANG_ITERABLE, "forEach").parameterCount(1);
private static final CallMatcher COLLECTION_SOURCE = anyOf(staticCall(JAVA_UTIL_LIST, "of"), staticCall(JAVA_UTIL_ARRAYS, "asList"));
private static final CallMatcher FOR_TERMINAL = instanceCall(JAVA_UTIL_STREAM_BASE_STREAM, "forEach", "forEachOrdered").parameterCount(1);
private static final CallMatcher MATCH_TERMINAL = instanceCall(JAVA_UTIL_STREAM_BASE_STREAM, "anyMatch", "allMatch",
"noneMatch").parameterCount(1);
@@ -824,6 +812,14 @@ public class StreamChainInliner implements CallInliner {
@Override
public boolean tryInlineCall(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call) {
if (FOR_COLLECTION_TERMINAL.test(call)) {
PsiMethodCallExpression qualifierCall = MethodCallUtils.getQualifierMethodCall(call);
if (qualifierCall == null || !InheritanceUtil.isInheritor(qualifierCall.getType(), JAVA_UTIL_COLLECTION)) return false;
Step terminalStep = new LambdaTerminalStep(call);
startStreamFromCollection(builder, terminalStep, qualifierCall);
terminalStep.pushResult(builder);
return true;
}
if (TERMINAL_CALL.test(call)) {
return inlineCompleteStream(builder, call);
}
@@ -916,35 +912,68 @@ public class StreamChainInliner implements CallInliner {
.chain(firstStep::iteration);
return;
}
PsiExpression qualifierExpression = null;
SpecialField sizeField = null;
if (array) {
qualifierExpression = sourceCall.getArgumentList().getExpressions()[0];
sizeField = SpecialField.ARRAY_LENGTH;
PsiExpression arrayExpression = sourceCall.getArgumentList().getExpressions()[0];
startStreamFromContainer(builder, firstStep, arrayExpression, SpecialField.ARRAY_LENGTH, inType);
return;
}
else if (COLLECTION_STREAM.test(sourceCall)) {
qualifierExpression = sourceCall.getMethodExpression().getQualifierExpression();
sizeField = SpecialField.COLLECTION_SIZE;
}
if (qualifierExpression != null) {
builder.pushExpression(qualifierExpression)
.chain(firstStep::before)
.unwrap(sizeField)
.push(DfTypes.intValue(0))
.ifCondition(RelationType.GT);
} else {
if (!originalQualifierAlreadyChecked) {
builder
.pushExpression(originalQualifier)
.chain(b -> checkAndMarkConsumed(b, originalQualifier))
.pop();
PsiExpression collectionExpression = sourceCall.getMethodExpression().getQualifierExpression();
if (collectionExpression != null) {
startStreamFromCollection(builder, firstStep, collectionExpression);
return;
}
}
startStreamUnknown(builder, firstStep, originalQualifier, originalQualifierAlreadyChecked, inType);
}
private static void startStreamFromCollection(CFGBuilder builder, Step firstStep, PsiExpression collectionExpression) {
PsiType elementType = PsiUtil.substituteTypeParameter(collectionExpression.getType(), JAVA_UTIL_COLLECTION, 0, false);
if (collectionExpression instanceof PsiMethodCallExpression call &&
COLLECTION_SOURCE.matches(call)) {
PsiExpression[] args = call.getArgumentList().getExpressions();
if (args.length == 1 && !MethodCallUtils.isVarArgCall(call)) {
startStreamFromContainer(builder, firstStep, args[0], SpecialField.ARRAY_LENGTH, elementType);
} else {
builder
.chain(firstStep::before)
.loopOver(args, builder.createTempVariable(elementType), elementType)
.chain(firstStep::iteration).end();
}
} else {
startStreamFromContainer(builder, firstStep, collectionExpression, SpecialField.COLLECTION_SIZE, elementType);
}
}
private static void startStreamUnknown(@NotNull CFGBuilder builder,
@NotNull Step firstStep,
@NotNull PsiExpression originalQualifier,
boolean originalQualifierAlreadyChecked,
@Nullable PsiType inType) {
if (!originalQualifierAlreadyChecked) {
builder
.chain(firstStep::before)
.pushUnknown()
.ifConditionIs(true);
.pushExpression(originalQualifier)
.chain(b -> checkAndMarkConsumed(b, originalQualifier))
.pop();
}
builder
.chain(firstStep::before)
.pushUnknown()
.ifConditionIs(true)
.chain(b -> makeMainLoop(b, firstStep, inType))
.end();
}
private static void startStreamFromContainer(@NotNull CFGBuilder builder,
@NotNull Step firstStep,
@NotNull PsiExpression collectionExpression,
@NotNull SpecialField sizeField,
@Nullable PsiType inType) {
builder.pushExpression(collectionExpression)
.chain(firstStep::before)
.unwrap(sizeField)
.push(DfTypes.intValue(0))
.ifCondition(RelationType.GT)
.chain(b -> makeMainLoop(b, firstStep, inType))
.end();
}
@@ -1,10 +1,21 @@
import foo.*;
import java.util.Map;
import java.util.HashMap;
import java.util.*;
import java.util.stream.*;
import java.util.function.Function;
class MapUpdateInlining {
void testArrayListCtor(Map<Integer, List<String>> map) {
map.computeIfAbsent(-1, <warning descr="The call to 'ArrayList::new' always fails as an argument is out of bounds">ArrayList::new</warning>);
map.computeIfAbsent(-1, x -> new <warning descr="The call to 'ArrayList' always fails as an argument is out of bounds">ArrayList</warning><>(x));
Arrays.asList(-1, -2, -3).forEach(x -> map.computeIfAbsent(x, a -> new <warning descr="The call to 'ArrayList' always fails as an argument is out of bounds">ArrayList</warning><>(a)));
Arrays.asList(-1, -2, -3).stream().forEach(x -> map.computeIfAbsent(x, a -> new <warning descr="The call to 'ArrayList' always fails as an argument is out of bounds">ArrayList</warning><>(a)));
Stream.of(-1, -2, -3).forEach(x -> map.computeIfAbsent(x, a -> new <warning descr="The call to 'ArrayList' always fails as an argument is out of bounds">ArrayList</warning><>(a)));
for (Integer x : Arrays.asList(-1, 0, 1)) {
map.computeIfAbsent(x, a -> new ArrayList<>(a));
}
}
void testKey(Map<String, String> map) {
System.out.println(map.computeIfAbsent("foo",
k -> <warning descr="Condition 'k.equals(\"blahblah\")' is always 'false'">k.equals("blahblah")</warning> ? "bar" : "baz").trim());
@@ -318,4 +318,10 @@ public class StreamInlining {
list.stream().forEach(l -> count[0]++);
System.out.println(count[0]);
}
void testFromArray(String[] data) {
if (data.length != 0) return;
long result = <warning descr="Result of 'Arrays.asList(data).stream().count()' is always '0'">Arrays.asList(data).stream().count()</warning>;
if (<warning descr="Condition 'result == 0' is always 'true'">result == 0</warning>) {}
}
}