diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CommonDataflow.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CommonDataflow.java index 3e7dbbb27f87..06265a122fe2 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CommonDataflow.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CommonDataflow.java @@ -241,7 +241,7 @@ public class CommonDataflow { if (block == null) return null; DataFlowRunner runner = new DataFlowRunner(false, block); CommonDataflowVisitor visitor = new CommonDataflowVisitor(); - RunnerResult result = runner.analyzeMethodRecursively(block, visitor); + RunnerResult result = runner.analyzeMethodRecursively(block, visitor, false); if (result != RunnerResult.OK) return new DataflowResult(result); if (!(block instanceof PsiClass)) return visitor.myResult; DataflowResult dfr = visitor.myResult.copy(); @@ -256,7 +256,7 @@ public class CommonDataflow { } else { initialStates = StreamEx.of(states).map(DfaMemoryState::createCopy).toList(); } - if(runner.analyzeBlockRecursively(body, initialStates, visitor) == RunnerResult.OK) { + if(runner.analyzeBlockRecursively(body, initialStates, visitor, false) == RunnerResult.OK) { dfr = visitor.myResult.copy(); } else { visitor.myResult = dfr; 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 fc02dc40ba16..2c21a3b6e65c 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 @@ -408,6 +408,9 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool InspectionsBundle.message("inspection.data.flow.turn.off.true.asserts.quickfix"), true)); } } + if (reporter.isOnTheFly()) { + ContainerUtil.addIfNotNull(fixes, createExplainFix(ref, new TrackingRunner.ValueDfaProblemType(value))); + } String valueText; ProblemHighlightType type; @@ -751,12 +754,21 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool } ContainerUtil.addIfNotNull(fixes, createReplaceWithNullCheckFix(psiAnchor, evaluatesToTrue)); } + if (reporter.isOnTheFly() && psiAnchor instanceof PsiExpression) { + ContainerUtil.addIfNotNull(fixes, createExplainFix( + (PsiExpression)psiAnchor, new TrackingRunner.ValueDfaProblemType(evaluatesToTrue))); + } String message = InspectionsBundle.message(isAtRHSOfBooleanAnd(psiAnchor) ? "dataflow.message.constant.condition.when.reached" : "dataflow.message.constant.condition", Boolean.toString(evaluatesToTrue)); reporter.registerProblem(psiAnchor, message, fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); } + @Nullable + protected LocalQuickFix createExplainFix(PsiExpression anchor, TrackingRunner.DfaProblemType problemType) { + return null; + } + private static boolean isCoveredBySurroundingFix(PsiElement anchor, boolean evaluatesToTrue) { PsiElement parent = PsiUtil.skipParenthesizedExprUp(anchor.getParent()); if (parent instanceof PsiPolyadicExpression) { 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 df718b6e6f6c..3c3d43a871c6 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 @@ -363,21 +363,22 @@ public class DataFlowRunner { LOG.error(new RuntimeExceptionWithAttachments(e, attachments)); } - public RunnerResult analyzeMethodRecursively(@NotNull PsiElement block, StandardInstructionVisitor visitor) { + public RunnerResult analyzeMethodRecursively(@NotNull PsiElement block, StandardInstructionVisitor visitor, boolean ignoreAssertions) { Collection states = createInitialStates(block, visitor, false); if (states == null) return RunnerResult.NOT_APPLICABLE; - return analyzeBlockRecursively(block, states, visitor); + return analyzeBlockRecursively(block, states, visitor, ignoreAssertions); } public RunnerResult analyzeBlockRecursively(@NotNull PsiElement block, Collection states, - StandardInstructionVisitor visitor) { - RunnerResult result = analyzeMethod(block, visitor, false, states); + StandardInstructionVisitor visitor, + boolean ignoreAssertions) { + RunnerResult result = analyzeMethod(block, visitor, ignoreAssertions, states); if (result != RunnerResult.OK) return result; Ref ref = Ref.create(RunnerResult.OK); forNestedClosures((closure, nestedStates) -> { - RunnerResult res = analyzeBlockRecursively(closure, nestedStates, visitor); + RunnerResult res = analyzeBlockRecursively(closure, nestedStates, visitor, ignoreAssertions); if (res != RunnerResult.OK) { ref.set(res); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index a6d1df962168..3a43cea1cb98 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -1643,6 +1643,15 @@ public class DfaMemoryStateImpl implements DfaMemoryState { mergeStacks(other); myCachedHash = null; myCachedNonTrivialEqClasses = null; + afterMerge(other); + } + + /** + * Custom logic to be implemented by subclasses + * @param other + */ + protected void afterMerge(DfaMemoryStateImpl other) { + } private void mergeStacks(DfaMemoryStateImpl other) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StateMerger.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StateMerger.java index 4f26f64cb589..245e0dca33dc 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StateMerger.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StateMerger.java @@ -564,6 +564,9 @@ class StateMerger { for (Map.Entry> entry : strippedToOriginals.entrySet()) { Collection merged = entry.getValue(); if (merged.size() > 1) { + for (DfaMemoryStateImpl state : merged) { + entry.getKey().afterMerge(state); + } myRemovedStates.addAll(merged); myMerged.add(entry.getKey()); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingDfaMemoryState.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingDfaMemoryState.java new file mode 100644 index 000000000000..9d8860915ad3 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingDfaMemoryState.java @@ -0,0 +1,385 @@ +// Copyright 2000-2019 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. +package com.intellij.codeInspection.dataFlow; + +import com.intellij.codeInspection.dataFlow.instructions.ConditionalGotoInstruction; +import com.intellij.codeInspection.dataFlow.instructions.ExpressionPushingInstruction; +import com.intellij.codeInspection.dataFlow.instructions.Instruction; +import com.intellij.codeInspection.dataFlow.value.*; +import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType; +import com.intellij.openapi.util.Pair; +import com.intellij.psi.PsiExpression; +import com.intellij.util.ObjectUtils; +import one.util.streamex.EntryStream; +import one.util.streamex.StreamEx; +import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.*; +import java.util.function.Predicate; + +public class TrackingDfaMemoryState extends DfaMemoryStateImpl { + private final List myHistory; + + protected TrackingDfaMemoryState(DfaValueFactory factory) { + super(factory); + myHistory = new ArrayList<>(1); + myHistory.add(null); + } + + protected TrackingDfaMemoryState(TrackingDfaMemoryState toCopy) { + super(toCopy); + myHistory = new ArrayList<>(toCopy.myHistory); + } + + @NotNull + @Override + public TrackingDfaMemoryState createCopy() { + return new TrackingDfaMemoryState(this); + } + + @Override + protected void afterMerge(DfaMemoryStateImpl other) { + super.afterMerge(other); + assert other instanceof TrackingDfaMemoryState; + for (MemoryStateChange change : ((TrackingDfaMemoryState)other).myHistory) { + if (!tryMerge(change)) { + myHistory.add(change); + } + } + } + + private boolean tryMerge(MemoryStateChange change) { + for (ListIterator iterator = myHistory.listIterator(); iterator.hasNext(); ) { + MemoryStateChange myChange = iterator.next(); + MemoryStateChange merge = myChange.tryMerge(change); + if (merge != null) { + iterator.set(merge); + return true; + } + } + return false; + } + + private Map> getRelations() { + Map> result = new HashMap<>(); + for (EqClass eqClass : getNonTrivialEqClasses()) { + DfaConstValue constant = eqClass.findConstant(); + List vars = eqClass.getVariables(false); + for (DfaVariableValue var : vars) { + Set set = result.computeIfAbsent(var, k -> new HashSet<>()); + if (constant != null) { + set.add(new Relation(RelationType.EQ, constant)); + } + for (DfaVariableValue eqVar : vars) { + if (eqVar != var) { + set.add(new Relation(RelationType.EQ, eqVar)); + } + } + } + } + + for (DistinctPairSet.DistinctPair classPair : getDistinctClassPairs()) { + EqClass first = classPair.getFirst(); + EqClass second = classPair.getSecond(); + RelationType plain = classPair.isOrdered() ? RelationType.LT : RelationType.NE; + RelationType flipped = Objects.requireNonNull(plain.getFlipped()); + + List firstVars = first.getVariables(false); + List secondVars = second.getVariables(false); + for (DfaVariableValue var1 : firstVars) { + for (DfaVariableValue var2 : secondVars) { + result.computeIfAbsent(var1, k -> new HashSet<>()).add(new Relation(plain, var2)); + result.computeIfAbsent(var2, k -> new HashSet<>()).add(new Relation(flipped, var1)); + } + } + DfaConstValue firstConst = first.findConstant(); + if (firstConst != null) { + for (DfaVariableValue var2 : secondVars) { + result.computeIfAbsent(var2, k -> new HashSet<>()).add(new Relation(flipped, firstConst)); + } + } + DfaConstValue secondConst = second.findConstant(); + if (secondConst != null) { + for (DfaVariableValue var1 : firstVars) { + result.computeIfAbsent(var1, k -> new HashSet<>()).add(new Relation(plain, secondConst)); + } + } + } + + return result; + } + + + void recordChange(Instruction instruction, TrackingDfaMemoryState previous) { + Map result = new HashMap<>(); + Set varsToCheck = new HashSet<>(); + previous.forVariableStates((value, state) -> varsToCheck.add(value)); + forVariableStates((value, state) -> varsToCheck.add(value)); + for (DfaVariableValue value : varsToCheck) { + DfaFactMap newMap = getVariableState(value).myFactMap; + DfaFactMap oldMap = previous.getVariableState(value).myFactMap; + if (!newMap.equals(oldMap)) { + DfaFactMap added = DfaFactMap.EMPTY; + DfaFactMap removed = DfaFactMap.EMPTY; + for (DfaFactType type : DfaFactType.getTypes()) { + Object oldVal = oldMap.get(type); + Object newVal = newMap.get(type); + if (!Objects.equals(oldVal, newVal)) { + //noinspection unchecked + added = added.with((DfaFactType)type, newVal); + //noinspection unchecked + removed = removed.with((DfaFactType)type, oldVal); + } + } + result.put(value, new Change(Collections.emptySet(), Collections.emptySet(), removed, added)); + } + } + Map> oldRelations = previous.getRelations(); + Map> newRelations = getRelations(); + varsToCheck.clear(); + varsToCheck.addAll(oldRelations.keySet()); + varsToCheck.addAll(newRelations.keySet()); + for (DfaVariableValue value : varsToCheck) { + Set oldValueRelations = oldRelations.getOrDefault(value, Collections.emptySet()); + Set newValueRelations = newRelations.getOrDefault(value, Collections.emptySet()); + if (!oldValueRelations.equals(newValueRelations)) { + Set added = new HashSet<>(newValueRelations); + added.removeAll(oldValueRelations); + Set removed = new HashSet<>(oldValueRelations); + removed.removeAll(newValueRelations); + result.compute( + value, (v, change) -> change == null + ? Change.create(removed, added, DfaFactMap.EMPTY, DfaFactMap.EMPTY) + : Change.create(removed, added, change.myRemovedFacts, change.myAddedFacts)); + } + } + DfaValue value = isEmptyStack() ? DfaUnknownValue.getInstance() : peek(); + myHistory.replaceAll(prev -> MemoryStateChange.create(prev, instruction, result, value)); + } + + List getHistory() { + return myHistory; + } + + static class Relation { + final @NotNull RelationType myRelationType; + final @NotNull DfaValue myCounterpart; + + Relation(@NotNull RelationType type, @NotNull DfaValue counterpart) { + myRelationType = type; + myCounterpart = counterpart; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Relation relation = (Relation)o; + return myRelationType == relation.myRelationType && + myCounterpart.equals(relation.myCounterpart); + } + + @Override + public int hashCode() { + return Objects.hash(myRelationType, myCounterpart); + } + + @Override + public String toString() { + return myRelationType + " " + myCounterpart; + } + } + + static class Change { + final Set myRemovedRelations; + final Set myAddedRelations; + final DfaFactMap myRemovedFacts; + final DfaFactMap myAddedFacts; + + private Change(Set removedRelations, Set addedRelations, DfaFactMap removedFacts, DfaFactMap addedFacts) { + myRemovedRelations = removedRelations.isEmpty() ? Collections.emptySet() : removedRelations; + myAddedRelations = addedRelations.isEmpty() ? Collections.emptySet() : addedRelations; + myRemovedFacts = removedFacts; + myAddedFacts = addedFacts; + } + + static Change create(Set removedRelations, Set addedRelations, DfaFactMap removedFacts, DfaFactMap addedFacts) { + if (removedRelations.isEmpty() && addedRelations.isEmpty() && removedFacts == DfaFactMap.EMPTY && addedFacts == DfaFactMap.EMPTY) { + return null; + } + return new Change(removedRelations, addedRelations, removedFacts, addedFacts); + } + + @Override + public String toString() { + String removed = StreamEx.of(myRemovedRelations).map(Object::toString).append(myRemovedFacts.toString()) + .without("").joining(", "); + String added = StreamEx.of(myAddedRelations).map(Object::toString).append(myAddedFacts.toString()) + .without("").joining(", "); + return (removed.isEmpty() ? "" : "-{" + removed + "} ") + (added.isEmpty() ? "" : "+{" + added + "}"); + } + } + + static final class MemoryStateChange { + final @Nullable MemoryStateChange myPrevious; + final @NotNull Instruction myInstruction; + final @NotNull Map myChanges; + final @NotNull DfaValue myTopOfStack; + + private MemoryStateChange(@Nullable MemoryStateChange previous, + @NotNull Instruction instruction, + @NotNull Map changes, + @NotNull DfaValue topOfStack) { + myPrevious = previous; + myInstruction = instruction; + myChanges = changes; + myTopOfStack = topOfStack; + } + + @Contract("null -> null") + @Nullable + MemoryStateChange findExpressionPush(@Nullable PsiExpression expression) { + if (expression == null) return null; + return findChange(change -> change.getExpression() == expression); + } + + MemoryStateChange findRelation(DfaVariableValue value, @NotNull Predicate relationPredicate) { + return findChange(change -> { + Change varChange = change.myChanges.get(value); + return varChange != null && varChange.myAddedRelations.stream().anyMatch(relationPredicate); + }); + } + + @NotNull + Pair findFact(DfaValue value, DfaFactType type) { + if (value instanceof DfaVariableValue) { + for (MemoryStateChange change = this; change != null; change = change.myPrevious) { + Change varChange = change.myChanges.get(value); + if (varChange != null) { + T added = varChange.myAddedFacts.get(type); + if (added != null) { + return Pair.create(change, added); + } + if (varChange.myRemovedFacts.get(type) != null) { + return Pair.create(change, null); + } + } + } + return Pair.create(null, ((DfaVariableValue)value).getInherentFacts().get(type)); + } + return Pair.create(null, type.fromDfaValue(value)); + } + + @Nullable + private MemoryStateChange findChange(@NotNull Predicate predicate) { + for (MemoryStateChange change = myPrevious; change != null; change = change.myPrevious) { + if (predicate.test(change)) { + return change; + } + } + return null; + } + + @Nullable + PsiExpression getExpression() { + if (myInstruction instanceof ExpressionPushingInstruction && + ((ExpressionPushingInstruction)myInstruction).getExpressionRange() == null) { + return ((ExpressionPushingInstruction)myInstruction).getExpression(); + } + if (myInstruction instanceof ConditionalGotoInstruction) { + return ObjectUtils.tryCast(((ConditionalGotoInstruction)myInstruction).getPsiAnchor(), PsiExpression.class); + } + return null; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + MemoryStateChange change = (MemoryStateChange)o; + return myInstruction.equals(change.myInstruction) && + myTopOfStack.equals(change.myTopOfStack) && + myChanges.equals(change.myChanges) && + Objects.equals(myPrevious, change.myPrevious); + } + + @Override + public int hashCode() { + return Objects.hash(myPrevious, myInstruction, myChanges, myTopOfStack); + } + + @Nullable + public MemoryStateChange tryMerge(MemoryStateChange change) { + MemoryStateChange[] thisFlat = flatten(); + MemoryStateChange[] thatFlat = change.flatten(); + MemoryStateChange result = null; + int thisIndex = 0, thatIndex = 0; + while (true) { + int curThis = thisIndex; + if (thisIndex == thisFlat.length && thatIndex == thatFlat.length) { + return result; + } + if (thisIndex == thisFlat.length || thatIndex == thatFlat.length) return null; + while (thisIndex < thisFlat.length) { + MemoryStateChange thisChange = thisFlat[thisIndex]; + if (thisChange.myInstruction == thatFlat[thatIndex].myInstruction) break; + if (!thisChange.myChanges.isEmpty()) { + thisIndex = thisFlat.length; + break; + } + thisIndex++; + } + if (thisIndex == thisFlat.length) { + thisIndex = curThis; + while (thatIndex < thatFlat.length) { + MemoryStateChange thatChange = thatFlat[thatIndex]; + if (thatChange.myInstruction == thisFlat[thisIndex].myInstruction) break; + if (!thatChange.myChanges.isEmpty()) return null; + thatIndex++; + } + if (thatIndex == thatFlat.length) return null; + } + MemoryStateChange thisChange = thisFlat[thisIndex]; + MemoryStateChange thatChange = thatFlat[thatIndex]; + if (thisChange == thatChange) { + result = thisChange; + } else { + assert thisChange.myInstruction == thatChange.myInstruction; + if (!thisChange.myChanges.equals(thatChange.myChanges)) return null; + result = create(result, thisChange.myInstruction, thisChange.myChanges, thisChange.myTopOfStack.unite(thatChange.myTopOfStack)); + } + thisIndex++; + thatIndex++; + } + } + + @Nullable + static MemoryStateChange create(@Nullable MemoryStateChange previous, + @NotNull Instruction instruction, + @NotNull Map result, + @NotNull DfaValue value) { + if (result.isEmpty() && value == DfaUnknownValue.getInstance()) { + return previous; + } + return new MemoryStateChange(previous, instruction, result, value); + } + + MemoryStateChange[] flatten() { + List changes = StreamEx.iterate(this, Objects::nonNull, change -> change.myPrevious).toList(); + Collections.reverse(changes); + return changes.toArray(new MemoryStateChange[0]); + } + + String dump() { + return StreamEx.of(flatten()).joining("\n"); + } + + @Override + public String toString() { + return myInstruction.getIndex() + " " + myInstruction + ": " + myTopOfStack + + (myChanges.isEmpty() ? "" : + "; Changes: " + EntryStream.of(myChanges).join(": ", "\n\t", "").joining()); + } + } +} diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java new file mode 100644 index 000000000000..136e4defdf3b --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java @@ -0,0 +1,563 @@ +// Copyright 2000-2019 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. +package com.intellij.codeInspection.dataFlow; + +import com.intellij.codeInsight.NullabilityAnnotationInfo; +import com.intellij.codeInsight.NullableNotNullManager; +import com.intellij.codeInspection.dataFlow.TrackingDfaMemoryState.MemoryStateChange; +import com.intellij.codeInspection.dataFlow.TrackingDfaMemoryState.Relation; +import com.intellij.codeInspection.dataFlow.instructions.*; +import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet; +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.DfaVariableValue; +import com.intellij.codeInspection.dataFlow.value.VariableDescriptor; +import com.intellij.openapi.util.Pair; +import com.intellij.openapi.util.text.StringUtil; +import com.intellij.psi.*; +import com.intellij.util.ArrayUtil; +import com.intellij.util.ObjectUtils; +import com.intellij.util.containers.ContainerUtil; +import com.siyeh.ig.psiutils.BoolUtils; +import com.siyeh.ig.psiutils.ExpressionUtils; +import one.util.streamex.StreamEx; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.*; +import java.util.stream.Stream; + +@SuppressWarnings("SuspiciousNameCombination") +public class TrackingRunner extends StandardDataFlowRunner { + private final List myHistoryForContext = new ArrayList<>(); + private final PsiExpression myExpression; + + public TrackingRunner(boolean unknownMembersAreNullable, @Nullable PsiElement context, PsiExpression expression) { + super(unknownMembersAreNullable, context); + myExpression = expression; + } + + @NotNull + @Override + protected DfaMemoryState createMemoryState() { + return new TrackingDfaMemoryState(getFactory()); + } + + @NotNull + @Override + protected DfaInstructionState[] acceptInstruction(@NotNull InstructionVisitor visitor, @NotNull DfaInstructionState instructionState) { + Instruction instruction = instructionState.getInstruction(); + TrackingDfaMemoryState memState = (TrackingDfaMemoryState)instructionState.getMemoryState().createCopy(); + DfaInstructionState[] states = super.acceptInstruction(visitor, instructionState); + for (DfaInstructionState state : states) { + ((TrackingDfaMemoryState)state.getMemoryState()).recordChange(instruction, memState); + } + if (instruction instanceof ExpressionPushingInstruction) { + ExpressionPushingInstruction pushing = (ExpressionPushingInstruction)instruction; + if (pushing.getExpression() == myExpression && pushing.getExpressionRange() == null) { + for (DfaInstructionState state : states) { + myHistoryForContext.addAll(((TrackingDfaMemoryState)state.getMemoryState()).getHistory()); + } + } + } + return states; + } + + public static List findProblemCause(boolean unknownAreNullables, + boolean ignoreAssertions, + PsiExpression expression, + DfaProblemType type) { + PsiElement body = DfaUtil.getDataflowContext(expression); + if (body == null) return Collections.emptyList(); + TrackingRunner runner = new TrackingRunner(unknownAreNullables, body, expression); + StandardInstructionVisitor visitor = new StandardInstructionVisitor(); + RunnerResult result = runner.analyzeMethodRecursively(body, visitor, ignoreAssertions); + if (result != RunnerResult.OK) return Collections.emptyList(); + List results = new ArrayList<>(); + for (MemoryStateChange history : runner.myHistoryForContext) { + CauseItem root = findCauseChain(expression, history, type); + if (!results.contains(root)) { + results.add(root); + } + } + return results; + } + + public abstract static class DfaProblemType { + public abstract String toString(); + } + + public static class CauseItem { + final @NotNull List myChildren; + final @NotNull DfaProblemType myProblem; + final @Nullable PsiElement myTarget; + + CauseItem(@NotNull String problem, @Nullable PsiElement target) { + this(new CustomDfaProblemType(problem), target); + } + + CauseItem(@NotNull DfaProblemType problem, @Nullable PsiElement target) { + myChildren = new ArrayList<>(); + myProblem = problem; + myTarget = target; + } + + CauseItem(@NotNull String problem, @NotNull MemoryStateChange change) { + this(new CustomDfaProblemType(problem), change); + } + + CauseItem(@NotNull DfaProblemType problem, @NotNull MemoryStateChange change) { + this(problem, change.getExpression()); + } + + void addChildren(CauseItem... causes) { + ContainerUtil.addAllNotNull(myChildren, causes); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + CauseItem item = (CauseItem)o; + return myChildren.equals(item.myChildren) && + myProblem.toString().equals(item.myProblem.toString()) && + Objects.equals(myTarget, item.myTarget); + } + + @Override + public int hashCode() { + return Objects.hash(myChildren, myProblem, myTarget); + } + + public String dump() { + return dump(0); + } + + private String dump(int indent) { + return StringUtil.repeat(" ", indent) + toString() + (myTarget == null ? "" : " (" + myTarget.getText() + ")") + "\n" + + StreamEx.of(myChildren).map(child -> child.dump(indent + 1)).joining(); + } + + public Stream children() { + return StreamEx.of(myChildren); + } + + @Nullable + public PsiElement getTarget() { + return myTarget; + } + + @Override + public String toString() { + return myProblem.toString(); + } + } + + public static class ValueDfaProblemType extends DfaProblemType { + final Object myValue; + + public ValueDfaProblemType(Object value) { + myValue = value; + } + + @Override + public String toString() { + return "Value is always " + myValue; + } + } + + static class CustomDfaProblemType extends DfaProblemType { + private final String myMessage; + + CustomDfaProblemType(String message) { + myMessage = message; + } + + @Override + public String toString() { + return myMessage; + } + } + + /* + TODO: 1. Find causes of other warnings: + Cause for possible NPE + Cause for AIOOBE + Cause for "Contract always fails" + Cause for possible CCE + Cause for "modifying an immutable collection" + Cause for "Collection is always empty" (separate inspection now) + TODO: 2. Describe causes in more cases: + Warning caused by contract + Warning caused by CustomMethodHandler + Warning caused by mismatched type constraints + TODO: 3. Check how it works with + Inliners (notably: Stream API) + Ternary operators + TODO: 4. Check for possible performance disasters (likely on some code patterns current algo might blow up) + TODO: 5. Problem when interesting state doesn't reach the current condition, need to do something with this + */ + @NotNull + private static CauseItem findCauseChain(PsiExpression expression, MemoryStateChange history, DfaProblemType type) { + CauseItem root = new CauseItem(type, expression); + if (history.getExpression() != expression) return root; + if (type instanceof ValueDfaProblemType) { + Object expectedValue = ((ValueDfaProblemType)type).myValue; + CauseItem[] causes = findConstantValueCause(expression, history, expectedValue); + root.addChildren(causes); + } + return root; + } + + @NotNull + private static CauseItem[] findConstantValueCause(PsiExpression expression, MemoryStateChange history, Object expectedValue) { + Object constantExpressionValue = ExpressionUtils.computeConstantExpression(expression); + DfaValue value = history.myTopOfStack; + if (constantExpressionValue != null && constantExpressionValue.equals(expectedValue)) { + return new CauseItem[]{new CauseItem("It's compile-time constant which evaluates to '" + value + "'", expression)}; + } + if (value instanceof DfaConstValue) { + Object constValue = ((DfaConstValue)value).getValue(); + if (Objects.equals(constValue, expectedValue) && constValue instanceof Boolean) { + return findBooleanResultCauses(expression, history, ((Boolean)constValue).booleanValue()); + } + } + if (value instanceof DfaVariableValue) { + MemoryStateChange change = history.findRelation( + (DfaVariableValue)value, rel -> rel.myRelationType == RelationType.EQ && rel.myCounterpart instanceof DfaConstValue && + Objects.equals(expectedValue, ((DfaConstValue)rel.myCounterpart).getValue())); + if (change != null) { + PsiExpression varSourceExpression = change.getExpression(); + Instruction instruction = change.myInstruction; + if (instruction instanceof AssignInstruction && change.myTopOfStack == value) { + PsiExpression rValue = ((AssignInstruction)instruction).getRExpression(); + CauseItem item = new CauseItem("'" + value + "' was assigned", rValue); + MemoryStateChange push = change.findExpressionPush(rValue); + if (push != null) { + item.addChildren(findConstantValueCause(rValue, push, expectedValue)); + } + return new CauseItem[]{item}; + } + else if (expectedValue instanceof Boolean && varSourceExpression != null) { + return new CauseItem[]{new CauseItem("'" + value + " == "+expectedValue+"' was established from condition", varSourceExpression)}; + } + } + } + return new CauseItem[0]; + } + + private static CauseItem[] findBooleanResultCauses(PsiExpression expression, + MemoryStateChange history, + boolean value) { + if (BoolUtils.isNegation(expression)) { + PsiExpression negated = BoolUtils.getNegated(expression); + if (negated != null) { + MemoryStateChange negatedPush = history.findExpressionPush(negated); + if (negatedPush != null) { + CauseItem cause = new CauseItem("Value '" + negated.getText() + "' is always '" + !value + "'", negated); + cause.addChildren(findConstantValueCause(negated, negatedPush, !value)); + return new CauseItem[]{cause}; + } + } + } + if (expression instanceof PsiBinaryExpression) { + PsiBinaryExpression binOp = (PsiBinaryExpression)expression; + RelationType relationType = + RelationType.fromElementType(binOp.getOperationTokenType()); + if (relationType != null) { + if (!value) { + relationType = relationType.getNegated(); + } + PsiExpression leftOperand = binOp.getLOperand(); + PsiExpression rightOperand = binOp.getROperand(); + MemoryStateChange leftChange = history.findExpressionPush(leftOperand); + MemoryStateChange rightChange = history.findExpressionPush(rightOperand); + if (leftChange != null && rightChange != null) { + DfaValue leftValue = leftChange.myTopOfStack; + DfaValue rightValue = rightChange.myTopOfStack; + if (leftValue == rightValue && + (leftValue instanceof DfaVariableValue || leftValue instanceof DfaConstValue)) { + return new CauseItem[]{new CauseItem("Comparison arguments are the same", binOp.getOperationSign())}; + } + if (leftValue != rightValue && relationType.isInequality() && + leftValue instanceof DfaConstValue && rightValue instanceof DfaConstValue) { + return new CauseItem[]{ + new CauseItem("Comparison arguments are different constants", binOp.getOperationSign())}; + } + return findRelationCause(relationType, leftChange, rightChange); + } + } + } + return new CauseItem[0]; + } + + @NotNull + private static CauseItem[] findRelationCause(RelationType relationType, + MemoryStateChange leftChange, + MemoryStateChange rightChange) { + DfaValue leftValue = leftChange.myTopOfStack; + DfaValue rightValue = rightChange.myTopOfStack; + if (leftValue instanceof DfaVariableValue) { + Relation relation = new Relation(relationType, rightValue); + MemoryStateChange change = findRelationAddedChange(leftChange, (DfaVariableValue)leftValue, relation); + if (change != null) { + CauseItem cause = findRelationCause(change, (DfaVariableValue)leftValue, relation, rightChange); + if (cause != null) { + Instruction instruction = change.myInstruction; + if (instruction instanceof AssignInstruction) { + PsiExpression expression = ((AssignInstruction)instruction).getRExpression(); + MemoryStateChange assignmentChange = change.findExpressionPush(expression); + if (assignmentChange != null) { + DfaValue target = change.myTopOfStack; + if (target == rightValue) { + return ArrayUtil.prepend(cause, findRelationCause(relationType, leftChange, assignmentChange)); + } + } + } + } + return new CauseItem[]{cause}; + } + } + if (rightValue instanceof DfaVariableValue) { + Relation relation = new Relation( + Objects.requireNonNull(relationType.getFlipped()), leftValue); + MemoryStateChange change = findRelationAddedChange(rightChange, (DfaVariableValue)rightValue, relation); + if (change != null) { + return new CauseItem[]{findRelationCause(change, (DfaVariableValue)rightValue, relation, leftChange)}; + } + } + Pair leftNullability = leftChange.findFact(leftValue, DfaFactType.NULLABILITY); + Pair rightNullability = rightChange.findFact(rightValue, DfaFactType.NULLABILITY); + if ((leftNullability.second == DfaNullability.NULL && rightNullability.second == DfaNullability.NOT_NULL) || + (rightNullability.second == DfaNullability.NULL && leftNullability.second == DfaNullability.NOT_NULL)) { + return new CauseItem[]{findNullabilityCause(leftChange, leftNullability.first, leftNullability.second), + findNullabilityCause(rightChange, rightNullability.first, rightNullability.second)}; + } + + Pair leftRange = leftChange.findFact(leftValue, DfaFactType.RANGE); + Pair rightRange = rightChange.findFact(rightValue, DfaFactType.RANGE); + if (leftRange.second != null && rightRange.second != null) { + LongRangeSet fromRelation = rightRange.second.fromRelation(relationType.getNegated()); + if (fromRelation != null && !fromRelation.intersects(leftRange.second)) { + return new CauseItem[]{ + findRangeCause(leftChange, leftRange.first, leftRange.second, "Left operand range is %s"), + findRangeCause(rightChange, rightRange.first, rightRange.second, "Right operand range is %s")}; + } + } + return new CauseItem[0]; + } + + private static CauseItem findRelationCause(MemoryStateChange change, + DfaVariableValue value, + Relation relation, MemoryStateChange counterPartChange) { + Instruction instruction = change.myInstruction; + String condition = value + " " + relation; + if (relation.myCounterpart instanceof DfaConstValue && + ((DfaConstValue)relation.myCounterpart).getValue() == null && relation.myRelationType == RelationType.NE) { + if (instruction instanceof CheckNotNullInstruction) { + PsiExpression expression = ((CheckNotNullInstruction)instruction).getProblem().getDereferencedExpression(); + String text = expression == null ? value.toString() : expression.getText(); + return new CauseItem("'" + text + "' was dereferenced", expression); + } + if (instruction instanceof InstanceofInstruction) { + PsiExpression expression = ((InstanceofInstruction)instruction).getExpression(); + return new CauseItem("The 'instanceof' check implies non-nullity", expression); + } + } + if (instruction instanceof AssignInstruction) { + DfaValue target = change.myTopOfStack; + PsiExpression rValue = ((AssignInstruction)instruction).getRExpression(); + if (target == value) { + CauseItem item = new CauseItem("'" + target + "' was assigned", rValue); + MemoryStateChange rValuePush = change.findExpressionPush(rValue); + if (rValuePush != null) { + item.addChildren(findRelationCause(relation.myRelationType, rValuePush, counterPartChange)); + } + return item; + } + if (target == relation.myCounterpart) { + return new CauseItem("'" + target + "' was assigned", rValue); + } + } + PsiExpression expression = change.getExpression(); + if (expression != null) { + if (expression instanceof PsiBinaryExpression) { + PsiExpression lOperand = ((PsiBinaryExpression)expression).getLOperand(); + PsiExpression rOperand = ((PsiBinaryExpression)expression).getROperand(); + MemoryStateChange leftPos = change.findExpressionPush(lOperand); + MemoryStateChange rightPos = change.findExpressionPush(rOperand); + if (leftPos != null && rightPos != null) { + DfaValue leftValue = leftPos.myTopOfStack; + DfaValue rightValue = rightPos.myTopOfStack; + if (leftValue == value && rightValue == relation.myCounterpart || + rightValue == value && leftValue == relation.myCounterpart) { + return new CauseItem(new CustomDfaProblemType(condition + " was checked before"), expression); + } + } + } + return new CauseItem(new CustomDfaProblemType(condition + " is known from here"), expression); + } + return null; + } + + private static CauseItem findNullabilityCause(MemoryStateChange factUse, MemoryStateChange factDef, DfaNullability nullability) { + PsiExpression expression = factUse.getExpression(); + if (factDef != null && expression != null) { + PsiExpression defExpression = factDef.getExpression(); + if (defExpression != null) { + return new CauseItem( + new CustomDfaProblemType(expression.getText() + " is known to be '"+nullability.getPresentationName()+"' from here"), + defExpression); + } + } + if (expression instanceof PsiMethodCallExpression) { + PsiMethodCallExpression call = (PsiMethodCallExpression)expression; + PsiMethod method = call.resolveMethod(); + return fromMemberNullability(nullability, method, "Method", call.getMethodExpression().getReferenceNameElement()); + } + if (expression instanceof PsiReferenceExpression) { + PsiVariable variable = ObjectUtils.tryCast(((PsiReferenceExpression)expression).resolve(), PsiVariable.class); + if (variable instanceof PsiField) { + return fromMemberNullability(nullability, variable, "Field", ((PsiReferenceExpression)expression).getReferenceNameElement()); + } + if (variable instanceof PsiParameter) { + return fromMemberNullability(nullability, variable, "Parameter", ((PsiReferenceExpression)expression).getReferenceNameElement()); + } + if (variable != null) { + return fromMemberNullability(nullability, variable, "Variable", ((PsiReferenceExpression)expression).getReferenceNameElement()); + } + } + return null; + } + + private static CauseItem fromMemberNullability(DfaNullability nullability, + PsiModifierListOwner owner, + String memberName, PsiElement anchor) { + if (owner != null) { + NullabilityAnnotationInfo info = NullableNotNullManager.getInstance(owner.getProject()).findEffectiveNullabilityInfo(owner); + if (info != null && DfaNullability.fromNullability(info.getNullability()) == nullability) { + String message; + String name = ((PsiNamedElement)owner).getName(); + if (info.isInferred()) { + message = memberName + " '" + name + "' was inferred to be '" + nullability.getPresentationName() + "'"; + } + else if (info.isExternal()) { + message = memberName + " '" + name + "' is externally annotated as '" + nullability.getPresentationName() + "'"; + } + else if (info.isContainer()) { + message = memberName + " '" + name + "' inherits container annotation, thus '" + nullability.getPresentationName() + "'"; + } + else { + message = memberName + " '" + name + "' is annotated as '" + nullability.getPresentationName() + "'"; + } + if (owner.getContainingFile() == anchor.getContainingFile()) { + anchor = owner.getNavigationElement(); + } + return new CauseItem(message, anchor); + } + } + return null; + } + + private static CauseItem findRangeCause(MemoryStateChange factUse, + MemoryStateChange factDef, + LongRangeSet range, + String template) { + DfaValue value = factUse.myTopOfStack; + if (value instanceof DfaVariableValue) { + VariableDescriptor descriptor = ((DfaVariableValue)value).getDescriptor(); + if (descriptor instanceof SpecialField && range.equals(LongRangeSet.indexRange())) { + switch (((SpecialField)descriptor)) { + case ARRAY_LENGTH: + return new CauseItem("Array length is always non-negative", factUse); + case STRING_LENGTH: + return new CauseItem("String length is always non-negative", factUse); + case COLLECTION_SIZE: + return new CauseItem("Collection size is always non-negative", factUse); + default: + } + } + } + PsiExpression expression = factUse.getExpression(); + if (expression != null) { + if (expression instanceof PsiLiteralExpression) { + return null; // Literal range is quite evident + } + if (expression instanceof PsiMethodCallExpression) { + PsiMethodCallExpression call = (PsiMethodCallExpression)expression; + PsiMethod method = call.resolveMethod(); + if (method != null) { + LongRangeSet fromAnnotation = LongRangeSet.fromPsiElement(method); + if (fromAnnotation.equals(range)) { + return new CauseItem("The range of '" + method.getName() + "' is specified by annotation as " + range, + call.getMethodExpression().getReferenceNameElement()); + } + } + } + if (expression instanceof PsiBinaryExpression && + (PsiType.LONG.equals(expression.getType()) || PsiType.INT.equals(expression.getType()))) { + boolean isLong = PsiType.LONG.equals(expression.getType()); + PsiBinaryExpression binOp = (PsiBinaryExpression)expression; + PsiExpression left = binOp.getLOperand(); + PsiExpression right = binOp.getROperand(); + MemoryStateChange leftPush = factUse.findExpressionPush(left); + MemoryStateChange rightPush = factUse.findExpressionPush(right); + if (leftPush != null && rightPush != null) { + DfaValue leftValue = leftPush.myTopOfStack; + DfaValue rightValue = rightPush.myTopOfStack; + Pair leftSet = leftPush.findFact(leftValue, DfaFactType.RANGE); + Pair rightSet = rightPush.findFact(rightValue, DfaFactType.RANGE); + LongRangeSet fromType = Objects.requireNonNull(LongRangeSet.fromType(expression.getType())); + if (leftSet.second == null) { + leftSet = Pair.create(null, fromType); + } + if (rightSet.second == null) { + rightSet = Pair.create(null, fromType); + } + LongRangeSet result = leftSet.second.binOpFromToken(binOp.getOperationTokenType(), rightSet.second, isLong); + if (range.equals(result)) { + CauseItem cause = new CauseItem("Range of '" + binOp.getOperationSign().getText() + "' result is " + range, factUse); + CauseItem leftCause = null, rightCause = null; + if (!leftSet.second.equals(fromType)) { + leftCause = findRangeCause(leftPush, leftSet.first, leftSet.second, "Left operand range is %s"); + } + if (!rightSet.second.equals(fromType)) { + rightCause = findRangeCause(rightPush, rightSet.first, rightSet.second, "Right operand range is %s"); + } + cause.addChildren(leftCause, rightCause); + return cause; + } + } + } + } + CauseItem item = new CauseItem(String.format(template, range.toString()), factUse); + if (factDef != null) { + PsiExpression defExpression = factDef.getExpression(); + if (defExpression != null) { + item.addChildren(new CauseItem("Range is known from here", defExpression)); + } + } + return item; + } + + private static MemoryStateChange findRelationAddedChange(MemoryStateChange history, DfaVariableValue var, Relation relation) { + List subRelations; + switch (relation.myRelationType) { + case NE: + subRelations = Arrays.asList(relation, new Relation(RelationType.GT, relation.myCounterpart), + new Relation(RelationType.LT, relation.myCounterpart)); + break; + case LE: + subRelations = Arrays.asList(new Relation(RelationType.EQ, relation.myCounterpart), + new Relation(RelationType.LT, relation.myCounterpart)); + break; + case GE: + subRelations = Arrays.asList(new Relation(RelationType.EQ, relation.myCounterpart), + new Relation(RelationType.GT, relation.myCounterpart)); + break; + default: + subRelations = Collections.singletonList(relation); + } + return history.findRelation(var, subRelations::contains); + } +} 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 76b135289f04..72222a7f4edf 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java @@ -20,6 +20,7 @@ import com.intellij.codeInsight.daemon.impl.quickfix.DeleteSideEffectsAwareFix; import com.intellij.codeInsight.daemon.impl.quickfix.SimplifyBooleanExpressionFix; import com.intellij.codeInsight.daemon.impl.quickfix.UnwrapSwitchLabelFix; import com.intellij.codeInspection.*; +import com.intellij.codeInspection.dataFlow.fix.FindDfaProblemCauseFix; import com.intellij.codeInspection.dataFlow.fix.SurroundWithRequireNonNullFix; import com.intellij.codeInspection.nullable.NullableStuffInspection; import com.intellij.pom.java.LanguageLevel; @@ -76,6 +77,12 @@ public class DataFlowInspection extends DataFlowInspectionBase { return WrapWithMutableCollectionFix.createFix(violation, onTheFly); } + @Nullable + @Override + protected LocalQuickFix createExplainFix(PsiExpression anchor, TrackingRunner.DfaProblemType problemType) { + return new FindDfaProblemCauseFix(TREAT_UNKNOWN_MEMBERS_AS_NULLABLE, IGNORE_ASSERT_STATEMENTS, anchor, problemType); + } + @Nullable @Override protected LocalQuickFix createUnwrapSwitchLabelFix() { diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/fix/FindDfaProblemCauseFix.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/fix/FindDfaProblemCauseFix.java new file mode 100644 index 000000000000..82d121928923 --- /dev/null +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/fix/FindDfaProblemCauseFix.java @@ -0,0 +1,147 @@ +// Copyright 2000-2019 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. +package com.intellij.codeInspection.dataFlow.fix; + +import com.intellij.codeInsight.hint.HintManager; +import com.intellij.codeInsight.hint.HintManagerImpl; +import com.intellij.codeInsight.intention.LowPriorityAction; +import com.intellij.codeInsight.unwrap.ScopeHighlighter; +import com.intellij.codeInspection.LocalQuickFix; +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.codeInspection.dataFlow.TrackingRunner; +import com.intellij.ide.util.PsiNavigationSupport; +import com.intellij.lang.injection.InjectedLanguageManager; +import com.intellij.openapi.editor.Document; +import com.intellij.openapi.editor.Editor; +import com.intellij.openapi.fileEditor.FileEditorManager; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.ui.popup.JBPopup; +import com.intellij.openapi.ui.popup.JBPopupAdapter; +import com.intellij.openapi.ui.popup.JBPopupFactory; +import com.intellij.openapi.ui.popup.LightweightWindowEvent; +import com.intellij.openapi.util.Pair; +import com.intellij.openapi.util.TextRange; +import com.intellij.openapi.util.text.StringUtil; +import com.intellij.psi.*; +import com.intellij.util.containers.ContainerUtil; +import one.util.streamex.EntryStream; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; + +import javax.swing.*; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; + +public class FindDfaProblemCauseFix implements LocalQuickFix, LowPriorityAction { + private final boolean myUnknownMembersAsNullable; + private final boolean myIgnoreAssertStatements; + private final SmartPsiElementPointer myAnchor; + private final TrackingRunner.DfaProblemType myProblemType; + + public FindDfaProblemCauseFix(boolean unknownMembersAsNullable, + boolean ignoreAssertStatements, + PsiExpression anchor, + TrackingRunner.DfaProblemType problemType) { + myUnknownMembersAsNullable = unknownMembersAsNullable; + myIgnoreAssertStatements = ignoreAssertStatements; + myAnchor = SmartPointerManager.createPointer(anchor); + myProblemType = problemType; + } + + @Override + public boolean startInWriteAction() { + return false; + } + + @Nls(capitalization = Nls.Capitalization.Sentence) + @NotNull + @Override + public String getFamilyName() { + return "Find cause"; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiExpression element = myAnchor.getElement(); + if (element == null) return; + Editor editor = FileEditorManager.getInstance(project).getSelectedTextEditor(); + if (editor == null) return; + Document document = editor.getDocument(); + PsiFile file = element.getContainingFile(); + PsiFile topLevelFile = InjectedLanguageManager.getInstance(project).getTopLevelFile(file); + if (topLevelFile == null || document != topLevelFile.getViewProvider().getDocument()) return; + List items = + TrackingRunner.findProblemCause(myUnknownMembersAsNullable, myIgnoreAssertStatements, element, myProblemType); + TrackingRunner.CauseItem root = ContainerUtil.getOnlyItem(items); + if (root == null) { + HintManagerImpl hintManager = (HintManagerImpl)HintManager.getInstance(); + hintManager.showErrorHint(editor, "Unable to find the cause"); + return; + } + class CauseWithDepth { + final int myDepth; + final TrackingRunner.CauseItem myCauseItem; + + CauseWithDepth(int depth, TrackingRunner.CauseItem item) { + myDepth = depth; + myCauseItem = item; + } + + @Override + public String toString() { + return StringUtil.repeat(" ", myDepth - 1) + myCauseItem; + } + } + List causes = + EntryStream.ofTree(root, (depth, c) -> c.children()).skip(1).mapKeyValue((d, i) -> new CauseWithDepth(d, i)).toList(); + if (causes.isEmpty()) { + HintManagerImpl hintManager = (HintManagerImpl)HintManager.getInstance(); + hintManager.showErrorHint(editor, "Unable to find the cause"); + return; + } + if (causes.size() == 1) { + TrackingRunner.CauseItem item = causes.get(0).myCauseItem; + navigate(editor, file, item); + return; + } + AtomicReference highlighter = new AtomicReference<>(new ScopeHighlighter(editor)); + JBPopup popup = JBPopupFactory.getInstance().createPopupChooserBuilder(causes) + .setSelectionMode(ListSelectionModel.SINGLE_SELECTION) + .setAccessibleName(root.toString()) + .setTitle(StringUtil.wordsToBeginFromUpperCase(root.toString())) + .setMovable(false) + .setResizable(false) + .setRequestFocus(true) + .setItemSelectedCallback((cause) -> { + ScopeHighlighter h = highlighter.get(); + if (h == null) return; + h.dropHighlight(); + if (cause == null) return; + PsiElement target = cause.myCauseItem.getTarget(); + if (target == null || !target.isValid()) return; + TextRange range = target.getTextRange(); + h.highlight(Pair.create(range, Collections.singletonList(range))); + }) + .addListener(new JBPopupAdapter() { + @Override + public void onClosed(@NotNull LightweightWindowEvent event) { + highlighter.getAndSet(null).dropHighlight(); + } + }) + .setItemChosenCallback(cause -> navigate(editor, file, cause.myCauseItem)) + .createPopup(); + popup.showInBestPositionFor(editor); + } + + private static void navigate(Editor editor, PsiFile file, TrackingRunner.CauseItem item) { + PsiElement target = item.getTarget(); + if (target == null) return; + TextRange range = target.getTextRange(); + PsiFile targetFile = target.getContainingFile(); + assert targetFile == file; + PsiNavigationSupport.getInstance().createNavigatable(file.getProject(), targetFile.getVirtualFile(), range.getStartOffset()) + .navigate(true); + HintManagerImpl hintManager = (HintManagerImpl)HintManager.getInstance(); + hintManager.showInformationHint(editor, StringUtil.escapeXmlEntities(item.toString())); + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/ArrayLength.java b/java/java-tests/testData/inspection/dataFlow/tracker/ArrayLength.java new file mode 100644 index 000000000000..8cad5d63fc48 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/ArrayLength.java @@ -0,0 +1,14 @@ +/* +Value is always true (s.length > 0) + Left operand range is {1..Integer.MAX_VALUE} (s.length) + Range is known from here (s[0]) + */ +class Test { + void test(String[] s) { + if (s[0].isEmpty()) return; + + if (s.length > 0) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/ArrayLengthCollectionSize.java b/java/java-tests/testData/inspection/dataFlow/tracker/ArrayLengthCollectionSize.java new file mode 100644 index 000000000000..bf4834e36c38 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/ArrayLengthCollectionSize.java @@ -0,0 +1,19 @@ +/* +Value is always false (s.length == list.size()) + Left operand range is {1..Integer.MAX_VALUE} (s.length) + Range is known from here (s[0]) + Right operand range is {0} (list.size()) + Range is known from here (list.isEmpty()) + */ +import java.util.List; + +class Test { + void test(String[] s, List list) { + if (!s[0].isEmpty()) return; + if (!list.isEmpty()) return; + + if (s.length == list.size()) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/AssignmentChain.java b/java/java-tests/testData/inspection/dataFlow/tracker/AssignmentChain.java new file mode 100644 index 000000000000..932721e433b8 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/AssignmentChain.java @@ -0,0 +1,14 @@ +/* +Value is always false (s == null) + 's' was assigned (s1) + s1 != null was checked before (s1 == null) + */ +class Test { + void test(String s, String s1) { + if (s1 == null) return; + s = s1; + if (s == null) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/BooleanChecked.java b/java/java-tests/testData/inspection/dataFlow/tracker/BooleanChecked.java new file mode 100644 index 000000000000..9bd01e0dee17 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/BooleanChecked.java @@ -0,0 +1,13 @@ +/* +Value is always true (b) + 'b == true' was established from condition (!b) + */ +import org.jetbrains.annotations.NotNull; + +class Test { + void test(boolean b) { + if (!b) return; + + if(b) {} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/BooleanReassignedMultipleTimes.java b/java/java-tests/testData/inspection/dataFlow/tracker/BooleanReassignedMultipleTimes.java new file mode 100644 index 000000000000..d8c52295a223 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/BooleanReassignedMultipleTimes.java @@ -0,0 +1,20 @@ +/* +Value is always false (b4) + 'b4' was assigned (b3) + 'b3' was assigned (b2) + 'b2' was assigned (b1) + 'b1' was assigned (b) + 'b == false' was established from condition (b) + */ +import org.jetbrains.annotations.NotNull; + +class Test { + void test(boolean b) { + if (b) return; + boolean b1 = b; + boolean b2 = b1; + boolean b3 = b2; + boolean b4 = b3; + if (b4) {} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/BooleanUnderNegation.java b/java/java-tests/testData/inspection/dataFlow/tracker/BooleanUnderNegation.java new file mode 100644 index 000000000000..90248eab4025 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/BooleanUnderNegation.java @@ -0,0 +1,11 @@ +/* +Value is always false (b) + 'b == false' was established from condition (!b) + */ +class Test { + void test(boolean b, int limit) { + for(int i=0; i<=limit; i++) { + if (!b) System.out.println(b); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/Constants.java b/java/java-tests/testData/inspection/dataFlow/tracker/Constants.java new file mode 100644 index 000000000000..01e2f0dc44bb --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/Constants.java @@ -0,0 +1,13 @@ +/* +Value is always false (E.A == E.B) + Comparison arguments are different constants (==) + */ +class Test { + enum E {A, B, C} + + void test() { + if (E.A == E.B) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/Ditto.java b/java/java-tests/testData/inspection/dataFlow/tracker/Ditto.java new file mode 100644 index 000000000000..52782531dbe9 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/Ditto.java @@ -0,0 +1,9 @@ +/* +Value is always true (a == a) + Comparison arguments are the same (==) + */ +class Test { + void test(int a) { + if (a == a) {} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/ModOutOfRange.java b/java/java-tests/testData/inspection/dataFlow/tracker/ModOutOfRange.java new file mode 100644 index 000000000000..cf2152c9075c --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/ModOutOfRange.java @@ -0,0 +1,11 @@ +/* +Value is always false (x % 10 == 15) + Range of '%' result is {-9..9} (x % 10) + */ +class Test { + void test(int x) { + if (x % 10 == 15) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/NotNullAnnotated.java b/java/java-tests/testData/inspection/dataFlow/tracker/NotNullAnnotated.java new file mode 100644 index 000000000000..3068cea4665e --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/NotNullAnnotated.java @@ -0,0 +1,13 @@ +/* +Value is always false (s1 == null) + 's1' was assigned (s.trim()) + Method 'trim' is externally annotated as 'non-null' (trim) + */ +class Test { + void test(String s) { + String s1 = s.trim(); + if (s1 == null) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/NotNullParameter.java b/java/java-tests/testData/inspection/dataFlow/tracker/NotNullParameter.java new file mode 100644 index 000000000000..a155f9113db7 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/NotNullParameter.java @@ -0,0 +1,11 @@ +/* +Value is always false (null == s) + Parameter 's' is annotated as 'non-null' (@NotNull String s) + */ +import org.jetbrains.annotations.NotNull; + +class Test { + void test(@NotNull String s) { + if (null == s) return; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/NotNullParameterAssigned.java b/java/java-tests/testData/inspection/dataFlow/tracker/NotNullParameterAssigned.java new file mode 100644 index 000000000000..93672617a45c --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/NotNullParameterAssigned.java @@ -0,0 +1,12 @@ +/* +Value is always true (null == s) + 's' was assigned (null) + */ +import org.jetbrains.annotations.NotNull; + +class Test { + void test(@NotNull String s) { + s = null; + if (null == s) return; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/NullWasChecked.java b/java/java-tests/testData/inspection/dataFlow/tracker/NullWasChecked.java new file mode 100644 index 000000000000..1257aeee1b6e --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/NullWasChecked.java @@ -0,0 +1,13 @@ +/* +Value is always false (s == null) + s != null was checked before (null == s) + */ +class Test { + void test(String s) { + if (null == s) return; + System.out.println(s.trim()); + if (s == null) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/NullWasCheckedInstanceOf.java b/java/java-tests/testData/inspection/dataFlow/tracker/NullWasCheckedInstanceOf.java new file mode 100644 index 000000000000..129cfccc4eca --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/NullWasCheckedInstanceOf.java @@ -0,0 +1,13 @@ +/* +Value is always false (s == null) + The 'instanceof' check implies non-nullity (s instanceof String) + */ +class Test { + void test(Object s) { + if (s instanceof String) { + if (s == null){ + + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/NullWasCheckedNullAssigned.java b/java/java-tests/testData/inspection/dataFlow/tracker/NullWasCheckedNullAssigned.java new file mode 100644 index 000000000000..adf0ca2f8c07 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/NullWasCheckedNullAssigned.java @@ -0,0 +1,14 @@ +/* +Value is always false (s == s1) + 's1' was assigned (null) + s != null was checked before (null == s) + */ +class Test { + void test(String s) { + if (null == s) return; + String s1 = null; + if (s == s1) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/OddityCheck.java b/java/java-tests/testData/inspection/dataFlow/tracker/OddityCheck.java new file mode 100644 index 000000000000..561b89eb5a2c --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/OddityCheck.java @@ -0,0 +1,14 @@ +/* +Value is always false (x * 2 == y * 2 + 1) + Range of '*' result is {Integer.MIN_VALUE..Integer.MAX_VALUE-1}: even (x * 2) + Range of '+' result is {-2147483647..Integer.MAX_VALUE}: odd (y * 2 + 1) + Range of '*' result is {Integer.MIN_VALUE..Integer.MAX_VALUE-1}: even (y * 2) + */ +class Test { + void test(int x, int y) { + // May produce better result in future (keep only mod info (even/odd), remove range which is irrelevant here) + if (x * 2 == y * 2 + 1) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/RepeatingIntegerComparison.java b/java/java-tests/testData/inspection/dataFlow/tracker/RepeatingIntegerComparison.java new file mode 100644 index 000000000000..bda6df8db974 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/RepeatingIntegerComparison.java @@ -0,0 +1,13 @@ +/* +Value is always true (x <= y) + x <= y was checked before (x > y) + */ +class Test { + void test(int x, int y) { + if (x > y) return; + + if (x <= y) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/RepeatingIntegerComparisonThreeBranches.java b/java/java-tests/testData/inspection/dataFlow/tracker/RepeatingIntegerComparisonThreeBranches.java new file mode 100644 index 000000000000..3e0d40b6d97c --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/RepeatingIntegerComparisonThreeBranches.java @@ -0,0 +1,17 @@ +/* +Value is always true (x < y) + x < y was checked before (x > y) + */ +class Test { + void test(int x, int y) { + if (x > y) return; + + if (x == y) { // explanation doesn't point here: not entirely correct, but hard to fix; postponed + return; + } + + if (x < y) { + return; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/SimpleDeref.java b/java/java-tests/testData/inspection/dataFlow/tracker/SimpleDeref.java new file mode 100644 index 000000000000..1e1b412cbcfa --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/SimpleDeref.java @@ -0,0 +1,12 @@ +/* +Value is always false (s == null) + 's' was dereferenced (s) + */ +class Test { + void test(String s) { + System.out.println(s.trim()); + if (s == null) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/StringLength.java b/java/java-tests/testData/inspection/dataFlow/tracker/StringLength.java new file mode 100644 index 000000000000..49d0c8fca20a --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/StringLength.java @@ -0,0 +1,11 @@ +/* +Value is always false (s.length() < 0) + String length is always non-negative (s.length()) + */ +class Test { + void test(String s) { + if (s.length() < 0) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java new file mode 100644 index 000000000000..18f446228d8b --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java @@ -0,0 +1,117 @@ +// Copyright 2000-2019 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. +package com.intellij.java.codeInspection; + +import com.intellij.JavaTestUtil; +import com.intellij.codeInspection.dataFlow.CommonDataflow; +import com.intellij.codeInspection.dataFlow.TrackingRunner; +import com.intellij.openapi.editor.SelectionModel; +import com.intellij.openapi.util.TextRange; +import com.intellij.openapi.util.text.StringUtil; +import com.intellij.psi.*; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.rt.execution.junit.FileComparisonFailure; +import com.intellij.testFramework.LightProjectDescriptor; +import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; +import one.util.streamex.StreamEx; +import org.jetbrains.annotations.NotNull; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.List; +import java.util.Set; + +public class DataFlowInspectionTrackerTest extends LightCodeInsightFixtureTestCase { + @Override + protected String getTestDataPath() { + return JavaTestUtil.getJavaTestDataPath() + "/inspection/dataFlow/tracker/"; + } + + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return JAVA_8_ANNOTATED; + } + + protected void doTest() { + String filePath = getTestName(false) + ".java"; + PsiFile file = myFixture.configureByFile(filePath); + SelectionModel model = myFixture.getEditor().getSelectionModel(); + int start = model.getSelectionStart(); + int end = model.getSelectionEnd(); + if (start == end) { + fail("No selection!"); + } + TextRange range = new TextRange(start, end); + PsiElement element = file.findElementAt(start); + while (element != null && element != file && + (!range.equals(element.getTextRange()) || element.getParent().getTextRange().equals(range))) { + element = element.getParent(); + } + String selectedText = model.getSelectedText(); + assertNotNull("Failed to find element at selection: " + selectedText, element); + assertTrue("Selected element is not an expression: " + selectedText, element instanceof PsiExpression); + PsiExpression expression = (PsiExpression)element; + CommonDataflow.DataflowResult result = CommonDataflow.getDataflowResult(expression); + assertNotNull("No common dataflow result for expression: " + selectedText, result); + Set values = result.getExpressionValues(expression); + assertEquals("No single value for expression: "+selectedText, 1, values.size()); + Object singleValue = values.iterator().next(); + List items = TrackingRunner.findProblemCause( + true, false, expression, new TrackingRunner.ValueDfaProblemType(singleValue)); + String dump = StreamEx.of(items).map(TrackingRunner.CauseItem::dump).joining("\n----\n"); + PsiComment firstComment = PsiTreeUtil.findChildOfType(file, PsiComment.class); + if (firstComment == null) { + fail("Comment not found"); + } + String text = firstComment.getText(); + String prefix = StringUtil.substringBefore(text, "\n") + "\n"; + String suffix = StringUtil.substringAfterLast(text, "\n"); + String actual = prefix + dump + suffix; + if (!text.equals(actual)) { + PsiElement fileCopy = file.copy(); + PsiTreeUtil.findChildOfType(fileCopy, PsiComment.class) + .replace(JavaPsiFacade.getElementFactory(getProject()).createCommentFromText(actual, null)); + int diff = actual.length() - text.length(); + String actualFile = fileCopy.getText(); + String origPath = myFixture.getTestDataPath() + "/" + filePath; + actualFile = actualFile.substring(0, start + diff) + + "" + + actualFile.substring(start + diff, end + diff) + + "" + + actualFile.substring(end + diff); + String expectedFile; + try { + expectedFile = new String(Files.readAllBytes(Paths.get(origPath)), StandardCharsets.UTF_8); + } + catch (IOException e) { + throw new UncheckedIOException(e); + } + throw new FileComparisonFailure("Dump differs", expectedFile, actualFile, origPath); + } + assertEquals(text, actual); + } + + public void testDitto() { doTest(); } + public void testConstants() { doTest(); } + public void testSimpleDeref() { doTest(); } + public void testNullWasChecked() { doTest(); } + public void testNullWasCheckedNullAssigned() { doTest(); } + public void testNullWasCheckedInstanceOf() { doTest(); } + public void testNotNullAnnotated() { doTest(); } + public void testNotNullParameter() { doTest(); } + public void testNotNullParameterAssigned() { doTest(); } + public void testStringLength() { doTest(); } + public void testModOutOfRange() { doTest(); } + public void testOddityCheck() { doTest(); } + public void testRepeatingIntegerComparison() { doTest(); } + public void testRepeatingIntegerComparisonThreeBranches() { doTest(); } + public void testAssignmentChain() { doTest(); } + public void testBooleanChecked() { doTest(); } + public void testBooleanReassignedMultipleTimes() { doTest(); } + public void testBooleanUnderNegation() { doTest(); } + public void testArrayLength() { doTest(); } + public void testArrayLengthCollectionSize() { doTest(); } +}