MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-01-09 08:09:39 +07:00
Code Issues Packages Projects Releases Wiki Activity

BranchingInstruction: remove true/false reachable tracking

Browse Source 
1. handleBranchingInstruction replaced with reportRedundantInstanceOf
2. reportConstants now reports sub-expressions as well

GitOrigin-RevId: deb69fe4c35ad7500b85e21d0ff153f9ffe60b56
This commit is contained in:
Tagir Valeev
2019-11-28 10:26:19 +07:00
committed by intellij-monorepo-bot
parent a1b1d44a54
commit 9fae6c627e
7 changed files with 78 additions and 162 deletions
Download Patch File Download Diff File Expand all files Collapse all files

84
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java
View File

@@ -10,8 +10,6 @@ import com.intellij.codeInspection.*;
import com.intellij.codeInspection.dataFlow.DataFlowInstructionVisitor.ConstantResult;
import com.intellij.codeInspection.dataFlow.NullabilityProblemKind.NullabilityProblem;
import com.intellij.codeInspection.dataFlow.fix.*;
import com.intellij.codeInspection.dataFlow.instructions.BranchingInstruction;
import com.intellij.codeInspection.dataFlow.instructions.ExpressionPushingInstruction;
import com.intellij.codeInspection.dataFlow.instructions.InstanceofInstruction;
import com.intellij.codeInspection.dataFlow.instructions.Instruction;
import com.intellij.codeInspection.dataFlow.value.DfaConstValue;
@@ -19,7 +17,6 @@ import com.intellij.codeInspection.nullable.NullableStuffInspectionBase;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.util.TextRange;
import com.intellij.openapi.util.WriteExternalException;
import com.intellij.openapi.util.registry.Registry;
@@ -157,7 +154,7 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
@Override
public void visitIfStatement(PsiIfStatement statement) {
PsiExpression condition = PsiUtil.skipParenthesizedExprDown(statement.getCondition());
if (BranchingInstruction.isBoolConst(condition)) {
if (BoolUtils.isBooleanLiteral(condition)) {
LocalQuickFix fix = createSimplifyBooleanExpressionFix(condition, condition.textMatches(PsiKeyword.TRUE));
holder.registerProblem(condition, "Condition is always " + condition.getText(), fix);
}
@@ -277,34 +274,22 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
ProblemsHolder holder,
final DataFlowInstructionVisitor visitor,
PsiElement scope) {
Pair<Set<Instruction>, Set<Instruction>> constConditions = runner.getConstConditionalExpressions();
Set<Instruction> trueSet = constConditions.getFirst();
Set<Instruction> falseSet = constConditions.getSecond();
ArrayList<Instruction> allProblems = new ArrayList<>();
allProblems.addAll(trueSet);
allProblems.addAll(falseSet);
StreamEx.of(runner.getInstructions()).select(InstanceofInstruction.class).filter(visitor::isInstanceofRedundant).into(allProblems);
ProblemReporter reporter = new ProblemReporter(holder, scope);
reportFailingCasts(reporter, visitor);
reportUnreachableSwitchBranches(visitor.getSwitchLabelsReachability(), holder);
for (Instruction instruction : allProblems) {
if (instruction instanceof BranchingInstruction) {
handleBranchingInstruction(reporter, visitor, trueSet, (BranchingInstruction)instruction);
}
}
reportAlwaysFailingCalls(reporter, visitor);
List<NullabilityProblem<?>> problems = NullabilityProblemKind.postprocessNullabilityProblems(visitor.problems().toList());
reportNullabilityProblems(reporter, problems, visitor.getConstantExpressions());
reportNullableReturns(reporter, problems, visitor.getConstantExpressions(), scope);
Map<PsiExpression, ConstantResult> constantExpressions = visitor.getConstantExpressions();
reportNullabilityProblems(reporter, problems, constantExpressions);
reportNullableReturns(reporter, problems, constantExpressions, scope);
reportOptionalOfNullableImprovements(reporter, visitor.getOfNullableCalls());
reportRedundantInstanceOf(runner, visitor, reporter);
reportConstants(reporter, visitor);
reportMethodReferenceProblems(holder, visitor);
@@ -326,6 +311,21 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
reportPointlessSameArguments(reporter, visitor);
}
private static void reportRedundantInstanceOf(DataFlowRunner runner,
DataFlowInstructionVisitor visitor,
ProblemReporter reporter) {
for (Instruction instruction : runner.getInstructions()) {
if (instruction instanceof InstanceofInstruction) {
InstanceofInstruction instanceOf = (InstanceofInstruction)instruction;
if (visitor.isInstanceofRedundant(instanceOf)) {
reporter.registerProblem(instanceOf.getPsiAnchor(),
InspectionsBundle.message("dataflow.message.redundant.instanceof"),
new RedundantInstanceofFix());
}
}
}
}
private void reportUnreachableSwitchBranches(Map<PsiExpression, ThreeState> labelReachability, ProblemsHolder holder) {
Set<PsiSwitchBlock> coveredSwitches = new HashSet<>();
@@ -371,8 +371,18 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
}
private void reportConstants(ProblemReporter reporter, DataFlowInstructionVisitor visitor) {
visitor.getConstantExpressions().forEach((expression, result) -> {
visitor.getConstantExpressionChunks().forEach((chunk, result) -> {
if (result == ConstantResult.UNKNOWN) return;
PsiExpression expression = chunk.myExpression;
if (chunk.myRange != null) {
if (result.value() instanceof Boolean) {
// report rare cases like a == b == c where "a == b" part is constant
String message = InspectionsBundle.message("dataflow.message.constant.condition", result.toString());
reporter.registerProblem(expression, chunk.myRange, message);
// do not add to reported anchors if only part of expression was reported
}
return;
}
if (isCondition(expression)) {
if (result.value() instanceof Boolean) {
reportConstantBoolean(reporter, expression, (Boolean)result.value());
@@ -736,36 +746,6 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
});
}
private void handleBranchingInstruction(ProblemReporter reporter,
StandardInstructionVisitor visitor,
Set<Instruction> trueSet,
BranchingInstruction instruction) {
PsiElement psiAnchor = instruction.getPsiAnchor();
if (instruction instanceof InstanceofInstruction && visitor.isInstanceofRedundant((InstanceofInstruction)instruction)) {
if (visitor.canBeNull((InstanceofInstruction)instruction)) {
reporter.registerProblem(psiAnchor,
InspectionsBundle.message("dataflow.message.redundant.instanceof"),
new RedundantInstanceofFix());
}
else {
reportConstantBoolean(reporter, psiAnchor, true);
}
}
else if (psiAnchor != null &&
(!(psiAnchor instanceof PsiExpression) || PsiImplUtil.getSwitchLabel((PsiExpression)psiAnchor) == null) &&
!isFlagCheck(psiAnchor)) {
boolean evaluatesToTrue = trueSet.contains(instruction);
TextRange range =
instruction instanceof ExpressionPushingInstruction ? ((ExpressionPushingInstruction)instruction).getExpressionRange() : null;
if (range != null) {
// report rare cases like a == b == c where "a == b" part is constant
String message = InspectionsBundle.message("dataflow.message.constant.condition", Boolean.toString(evaluatesToTrue));
reporter.registerProblem(psiAnchor, range, message);
// do not add to reported anchors if only part of expression was reported
}
}
}
private void reportConstantBoolean(ProblemReporter reporter, PsiElement psiAnchor, boolean evaluatesToTrue) {
while (psiAnchor instanceof PsiParenthesizedExpression) {
psiAnchor = ((PsiParenthesizedExpression)psiAnchor).getExpression();

51
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java
View File

@@ -36,7 +36,7 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
private final Map<PsiTypeCastExpression, StateInfo> myClassCastProblems = new HashMap<>();
private final Map<PsiTypeCastExpression, TypeConstraint> myRealOperandTypes = new HashMap<>();
private final Map<PsiCallExpression, Boolean> myFailingCalls = new HashMap<>();
private final Map<PsiExpression, ConstantResult> myConstantExpressions = new HashMap<>();
private final Map<ExpressionChunk, ConstantResult> myConstantExpressions = new HashMap<>();
private final Map<PsiElement, ThreeState> myOfNullableCalls = new HashMap<>();
private final Map<PsiAssignmentExpression, Pair<PsiType, PsiType>> myArrayStoreProblems = new HashMap<>();
private final Map<PsiMethodReferenceExpression, DfaValue> myMethodReferenceResults = new HashMap<>();
@@ -91,7 +91,6 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
if (anchor instanceof PsiExpression && PsiImplUtil.getSwitchLabel((PsiExpression)anchor) != null) {
mySwitchLabelsReachability.merge((PsiExpression)anchor, ThreeState.fromBoolean(isTrueBranch), ThreeState::merge);
}
super.beforeConditionalJump(instruction, isTrueBranch);
}
private static boolean isAssignmentToDefaultValueInConstructor(AssignInstruction instruction, DataFlowRunner runner, DfaValue target) {
@@ -159,6 +158,11 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
}
Map<PsiExpression, ConstantResult> getConstantExpressions() {
return EntryStream.of(myConstantExpressions).filterKeys(chunk -> chunk.myRange == null)
.mapKeys(chunk -> chunk.myExpression).toMap();
}
Map<ExpressionChunk, ConstantResult> getConstantExpressionChunks() {
return myConstantExpressions;
}
@@ -196,6 +200,14 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
ContainerUtil.exists(instructions, i -> i instanceof ReturnInstruction && ((ReturnInstruction)i).getAnchor() instanceof PsiReturnStatement);
}
public boolean isInstanceofRedundant(InstanceofInstruction instruction) {
PsiExpression expression = instruction.getExpression();
return expression != null && !myUsefulInstanceofs.contains(instruction) && myReachable.contains(instruction) &&
myConstantExpressions.get(new ExpressionChunk(expression, null)) != ConstantResult.TRUE &&
(!(expression instanceof PsiMethodReferenceExpression) ||
!DfaConstValue.isConstant(myMethodReferenceResults.get(expression), Boolean.TRUE));
}
@Override
protected void beforeExpressionPush(@NotNull DfaValue value,
@NotNull PsiExpression expression,
@@ -214,9 +226,7 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
if (fact == null) fact = TypeConstraint.empty();
myRealOperandTypes.merge((PsiTypeCastExpression)parent, fact, TypeConstraint::unite);
}
if (range == null) {
reportConstantExpressionValue(value, memState, expression);
}
reportConstantExpressionValue(value, memState, expression, range);
}
@Override
@@ -287,9 +297,10 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
contracts.stream().anyMatch(contract -> contract.getReturnValue().isFail() && !contract.isTrivial());
}
private void reportConstantExpressionValue(DfaValue value, DfaMemoryState memState, PsiExpression expression) {
private void reportConstantExpressionValue(DfaValue value, DfaMemoryState memState, PsiExpression expression, TextRange range) {
if (expression instanceof PsiLiteralExpression) return;
ConstantResult curState = myConstantExpressions.get(expression);
ExpressionChunk chunk = new ExpressionChunk(expression, range);
ConstantResult curState = myConstantExpressions.get(chunk);
if (curState == ConstantResult.UNKNOWN) return;
ConstantResult nextState = ConstantResult.UNKNOWN;
DfaConstValue dfaConst = memState.getConstantValue(value);
@@ -299,7 +310,7 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
nextState = ConstantResult.UNKNOWN;
}
}
myConstantExpressions.put(expression, nextState);
myConstantExpressions.put(chunk, nextState);
}
@Override
@@ -417,4 +428,28 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
return UNKNOWN;
}
}
static class ExpressionChunk {
final @NotNull PsiExpression myExpression;
final @Nullable TextRange myRange;
ExpressionChunk(@NotNull PsiExpression expression, @Nullable TextRange range) {
myExpression = expression;
myRange = range;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ExpressionChunk chunk = (ExpressionChunk)o;
return myExpression.equals(chunk.myExpression) &&
Objects.equals(myRange, chunk.myRange);
}
@Override
public int hashCode() {
return 31 * myExpression.hashCode() + Objects.hashCode(myRange);
}
}
}

36
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java
View File

@@ -14,7 +14,6 @@ import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.diagnostic.RuntimeExceptionWithAttachments;
import com.intellij.openapi.progress.ProcessCanceledException;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.util.Ref;
import com.intellij.openapi.util.registry.Registry;
import com.intellij.psi.*;
@@ -621,41 +620,6 @@ public class DataFlowRunner {
}
}
@NotNull
public Pair<Set<Instruction>,Set<Instruction>> getConstConditionalExpressions() {
Set<Instruction> trueSet = new HashSet<>();
Set<Instruction> falseSet = new HashSet<>();
for (Instruction instruction : myInstructions) {
if (instruction instanceof BranchingInstruction) {
BranchingInstruction branchingInstruction = (BranchingInstruction)instruction;
if (branchingInstruction.getPsiAnchor() != null && branchingInstruction.isConditionConst()) {
if (!branchingInstruction.isTrueReachable()) {
falseSet.add(branchingInstruction);
}
if (!branchingInstruction.isFalseReachable()) {
trueSet.add(branchingInstruction);
}
}
}
}
for (Instruction instruction : myInstructions) {
if (instruction instanceof BranchingInstruction) {
BranchingInstruction branchingInstruction = (BranchingInstruction)instruction;
if (branchingInstruction.isTrueReachable()) {
falseSet.remove(branchingInstruction);
}
if (branchingInstruction.isFalseReachable()) {
trueSet.remove(branchingInstruction);
}
}
}
return Pair.create(trueSet, falseSet);
}
protected static class TimeStats {
private static final long DFA_EXECUTION_TIME_TO_REPORT_NANOS = TimeUnit.SECONDS.toNanos(30);
private final @Nullable ThreadMXBean myMxBean;

6
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java
View File

@@ -83,12 +83,6 @@ public abstract class InstructionVisitor {
}
protected void beforeConditionalJump(ConditionalGotoInstruction instruction, boolean isTrueBranch) {
if (isTrueBranch ^ instruction.isNegated()) {
instruction.setTrueReachable();
}
else {
instruction.setFalseReachable();
}
}
void pushExpressionResult(@NotNull DfaValue value,

25
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java
View File

@@ -30,9 +30,8 @@ public class StandardInstructionVisitor extends InstructionVisitor {
private static final Logger LOG = Logger.getInstance(StandardInstructionVisitor.class);
private final boolean myStopAnalysisOnNpe;
private final Set<InstanceofInstruction> myReachable = new THashSet<>();
private final Set<InstanceofInstruction> myCanBeNullInInstanceof = new THashSet<>();
private final Set<InstanceofInstruction> myUsefulInstanceofs = new THashSet<>();
final Set<InstanceofInstruction> myReachable = new THashSet<>();
final Set<InstanceofInstruction> myUsefulInstanceofs = new THashSet<>();
public StandardInstructionVisitor() {
myStopAnalysisOnNpe = false;
@@ -744,9 +743,6 @@ public class StandardInstructionVisitor extends InstructionVisitor {
}
pushExpressionResult(result, instruction, memState);
instruction.setTrueReachable(); // Not a branching instruction actually.
instruction.setFalseReachable();
return nextInstruction(instruction, runner, memState);
}
@@ -878,9 +874,6 @@ public class StandardInstructionVisitor extends InstructionVisitor {
DfaValue dfaRight = memState.pop();
DfaValue dfaLeft = memState.pop();
DfaValueFactory factory = runner.getFactory();
if (!memState.isNotNull(dfaLeft)) {
myCanBeNullInInstanceof.add(instruction);
}
boolean unknownTargetType = false;
DfaCondition condition = null;
if (instruction.isClassObjectCheck()) {
@@ -935,20 +928,6 @@ public class StandardInstructionVisitor extends InstructionVisitor {
@NotNull ThreeState result) {
DfaValue value = result == ThreeState.UNSURE ? DfaUnknownValue.getInstance() : runner.getFactory().getBoolean(result.toBoolean());
pushExpressionResult(value, instruction, memState);
if (result != ThreeState.NO) {
instruction.setTrueReachable();
}
if (result != ThreeState.YES) {
instruction.setFalseReachable();
}
return new DfaInstructionState(runner.getInstruction(instruction.getIndex() + 1), memState);
}
public boolean isInstanceofRedundant(InstanceofInstruction instruction) {
return !myUsefulInstanceofs.contains(instruction) && !instruction.isConditionConst() && myReachable.contains(instruction);
}
public boolean canBeNull(InstanceofInstruction instruction) {
return myCanBeNullInInstanceof.contains(instruction);
}
}

37
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/BranchingInstruction.java
View File

@@ -17,53 +17,16 @@
package com.intellij.codeInspection.dataFlow.instructions;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.PsiLiteralExpression;
import com.intellij.psi.util.PsiUtil;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.Nullable;
public abstract class BranchingInstruction extends Instruction {
private boolean myIsTrueReachable;
private boolean myIsFalseReachable;
private final boolean isConstTrue;
private final PsiElement myExpression;
protected BranchingInstruction(@Nullable PsiElement psiAnchor) {
myIsTrueReachable = false;
myIsFalseReachable = false;
myExpression = psiAnchor;
isConstTrue = psiAnchor instanceof PsiExpression && isBoolConst(PsiUtil.skipParenthesizedExprDown((PsiExpression)psiAnchor));
}
public boolean isTrueReachable() {
return myIsTrueReachable;
}
public boolean isFalseReachable() {
return myIsFalseReachable;
}
public PsiElement getPsiAnchor() {
return myExpression;
}
public void setTrueReachable() {
myIsTrueReachable = true;
}
public void setFalseReachable() {
myIsFalseReachable = true;
}
public boolean isConditionConst() {
return !isConstTrue && myIsTrueReachable != myIsFalseReachable;
}
public static boolean isBoolConst(PsiElement condition) {
if (!(condition instanceof PsiLiteralExpression)) return false;
@NonNls String text = condition.getText();
return "true".equals(text) || "false".equals(text);
}
}