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

don't report constant conditions containing call chains

Browse Source
This commit is contained in:
peter
2012-09-11 20:54:29 +02:00
parent 3719f4d04f
commit 20f79a297e
6 changed files with 133 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java
+3 -2
View File
@@ -1526,15 +1526,16 @@ class ControlFlowAnalyzer extends JavaElementVisitor {
return null;
}
boolean isCall = expression instanceof PsiMethodCallExpression;
PsiExpression qualifier = refExpr.getQualifierExpression();
if (qualifier == null) {
return myFactory.getVarFactory().createVariableValue(var, false, null);
return myFactory.getVarFactory().createVariableValue(var, false, null, isCall);
}
if (var instanceof PsiField && var.hasModifierProperty(PsiModifier.FINAL)) {
DfaVariableValue qualifierValue = createChainedVariableValue(qualifier);
if (qualifierValue != null) {
return myFactory.getVarFactory().createVariableValue(var, false, qualifierValue);
return myFactory.getVarFactory().createVariableValue(var, false, qualifierValue, isCall || qualifierValue.isViaMethods());
}
}
return null;
java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java
+102 -79
View File
@@ -147,16 +147,13 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
Pair<Set<Instruction>, Set<Instruction>> constConditions = runner.getConstConditionalExpressions();
Set<Instruction> trueSet = constConditions.getFirst();
Set<Instruction> falseSet = constConditions.getSecond();
Set<Instruction> npeSet = runner.getNPEInstructions();
Set<Instruction> cceSet = runner.getCCEInstructions();
Set<Instruction> redundantInstanceofs = StandardDataFlowRunner.getRedundantInstanceofs(runner, visitor);
ArrayList<Instruction> allProblems = new ArrayList<Instruction>();
allProblems.addAll(trueSet);
allProblems.addAll(falseSet);
allProblems.addAll(npeSet);
allProblems.addAll(cceSet);
allProblems.addAll(redundantInstanceofs);
allProblems.addAll(runner.getNPEInstructions());
allProblems.addAll(runner.getCCEInstructions());
allProblems.addAll(StandardDataFlowRunner.getRedundantInstanceofs(runner, visitor));
Collections.sort(allProblems, new Comparator<Instruction>() {
public int compare(Instruction i1, Instruction i2) {
@@ -168,82 +165,105 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
for (Instruction instruction : allProblems) {
if (instruction instanceof MethodCallInstruction) {
MethodCallInstruction mcInstruction = (MethodCallInstruction)instruction;
if (mcInstruction.getCallExpression() instanceof PsiMethodCallExpression) {
PsiMethodCallExpression callExpression = (PsiMethodCallExpression)mcInstruction.getCallExpression();
LocalQuickFix[] fix = createNPEFixes(callExpression.getMethodExpression().getQualifierExpression(), callExpression);
holder.registerProblem(callExpression,
InspectionsBundle.message("dataflow.message.npe.method.invocation"),
fix);
}
reportCallMayProduceNpe(holder, (MethodCallInstruction)instruction);
}
else if (instruction instanceof FieldReferenceInstruction) {
FieldReferenceInstruction frInstruction = (FieldReferenceInstruction)instruction;
PsiElement elementToAssert = frInstruction.getElementToAssert();
PsiExpression expression = frInstruction.getExpression();
if (expression instanceof PsiArrayAccessExpression) {
LocalQuickFix[] fix = createNPEFixes((PsiExpression)elementToAssert, expression);
holder.registerProblem(expression,
InspectionsBundle.message("dataflow.message.npe.array.access"),
fix);
}
else {
LocalQuickFix[] fix = createNPEFixes((PsiExpression)elementToAssert, expression);
holder.registerProblem(elementToAssert,
InspectionsBundle.message("dataflow.message.npe.field.access"),
fix);
}
reportFieldAccessMayProduceNpe(holder, (FieldReferenceInstruction)instruction);
}
else if (instruction instanceof TypeCastInstruction) {
TypeCastInstruction tcInstruction = (TypeCastInstruction)instruction;
PsiTypeCastExpression typeCast = tcInstruction.getCastExpression();
holder.registerProblem(typeCast.getCastType(),
InspectionsBundle.message("dataflow.message.cce", typeCast.getOperand().getText()));
reportCastMayFail(holder, (TypeCastInstruction)instruction);
}
else if (instruction instanceof BranchingInstruction) {
PsiElement psiAnchor = ((BranchingInstruction)instruction).getPsiAnchor();
boolean underBinary = isAtRHSOfBooleanAnd(psiAnchor);
if (instruction instanceof InstanceofInstruction && visitor.isInstanceofRedundant((InstanceofInstruction)instruction)) {
if (visitor.canBeNull((BinopInstruction)instruction)) {
holder.registerProblem(psiAnchor,
InspectionsBundle.message("dataflow.message.redundant.instanceof"),
new RedundantInstanceofFix());
}
else {
final LocalQuickFix localQuickFix = createSimplifyBooleanExpressionFix(psiAnchor, true);
holder.registerProblem(psiAnchor,
InspectionsBundle.message(underBinary ? "dataflow.message.constant.condition.whenriched" : "dataflow.message.constant.condition", Boolean.toString(true)),
localQuickFix == null ? null : new LocalQuickFix[]{localQuickFix});
}
}
else if (psiAnchor instanceof PsiSwitchLabelStatement) {
if (falseSet.contains(instruction)) {
holder.registerProblem(psiAnchor,
InspectionsBundle.message("dataflow.message.unreachable.switch.label"));
}
}
else if (psiAnchor != null && !reportedAnchors.contains(psiAnchor) && !isCompileConstantInIfCondition(psiAnchor)) {
boolean evaluatesToTrue = trueSet.contains(instruction);
if (onTheLeftSideOfConditionalAssignemnt(psiAnchor)) {
holder.registerProblem(
psiAnchor,
InspectionsBundle.message("dataflow.message.pointless.assignment.expression", Boolean.toString(evaluatesToTrue)),
createSimplifyToAssignmentFix()
);
}
else if (shouldReportConditionAlwaysTrueOrFalse(psiAnchor, evaluatesToTrue)) {
final LocalQuickFix fix = createSimplifyBooleanExpressionFix(psiAnchor, evaluatesToTrue);
String message = InspectionsBundle.message(underBinary ?
"dataflow.message.constant.condition.whenriched" :
"dataflow.message.constant.condition", Boolean.toString(evaluatesToTrue));
holder.registerProblem(psiAnchor, message, fix == null ? null : new LocalQuickFix[]{fix});
}
reportedAnchors.add(psiAnchor);
}
handleBranchingInstruction(holder, visitor, trueSet, falseSet, reportedAnchors, (BranchingInstruction)instruction);
}
}
reportNullableArguments(runner, holder);
reportNullableAssignments(runner, holder);
reportUnboxedNullables(runner, holder);
reportNullableReturns(runner, holder);
}
private static void reportCallMayProduceNpe(ProblemsHolder holder, MethodCallInstruction mcInstruction) {
if (mcInstruction.getCallExpression() instanceof PsiMethodCallExpression) {
PsiMethodCallExpression callExpression = (PsiMethodCallExpression)mcInstruction.getCallExpression();
LocalQuickFix[] fix = createNPEFixes(callExpression.getMethodExpression().getQualifierExpression(), callExpression);
holder.registerProblem(callExpression,
InspectionsBundle.message("dataflow.message.npe.method.invocation"),
fix);
}
}
private static void reportFieldAccessMayProduceNpe(ProblemsHolder holder, FieldReferenceInstruction frInstruction) {
PsiElement elementToAssert = frInstruction.getElementToAssert();
PsiExpression expression = frInstruction.getExpression();
if (expression instanceof PsiArrayAccessExpression) {
LocalQuickFix[] fix = createNPEFixes((PsiExpression)elementToAssert, expression);
holder.registerProblem(expression,
InspectionsBundle.message("dataflow.message.npe.array.access"),
fix);
}
else {
LocalQuickFix[] fix = createNPEFixes((PsiExpression)elementToAssert, expression);
holder.registerProblem(elementToAssert,
InspectionsBundle.message("dataflow.message.npe.field.access"),
fix);
}
}
private static void reportCastMayFail(ProblemsHolder holder, TypeCastInstruction instruction) {
PsiTypeCastExpression typeCast = instruction.getCastExpression();
holder.registerProblem(typeCast.getCastType(),
InspectionsBundle.message("dataflow.message.cce", typeCast.getOperand().getText()));
}
private void handleBranchingInstruction(ProblemsHolder holder,
StandardInstructionVisitor visitor,
Set<Instruction> trueSet,
Set<Instruction> falseSet, HashSet<PsiElement> reportedAnchors, BranchingInstruction instruction) {
PsiElement psiAnchor = instruction.getPsiAnchor();
boolean underBinary = isAtRHSOfBooleanAnd(psiAnchor);
if (instruction instanceof InstanceofInstruction && visitor.isInstanceofRedundant((InstanceofInstruction)instruction)) {
if (visitor.canBeNull((BinopInstruction)instruction)) {
holder.registerProblem(psiAnchor,
InspectionsBundle.message("dataflow.message.redundant.instanceof"),
new RedundantInstanceofFix());
}
else {
final LocalQuickFix localQuickFix = createSimplifyBooleanExpressionFix(psiAnchor, true);
holder.registerProblem(psiAnchor,
InspectionsBundle.message(underBinary ? "dataflow.message.constant.condition.when.reached" : "dataflow.message.constant.condition", Boolean.toString(true)),
localQuickFix == null ? null : new LocalQuickFix[]{localQuickFix});
}
}
else if (psiAnchor instanceof PsiSwitchLabelStatement) {
if (falseSet.contains(instruction)) {
holder.registerProblem(psiAnchor,
InspectionsBundle.message("dataflow.message.unreachable.switch.label"));
}
}
else if (psiAnchor != null && !reportedAnchors.contains(psiAnchor) && !isCompileConstantInIfCondition(psiAnchor)) {
boolean evaluatesToTrue = trueSet.contains(instruction);
if (onTheLeftSideOfConditionalAssignemnt(psiAnchor)) {
holder.registerProblem(
psiAnchor,
InspectionsBundle.message("dataflow.message.pointless.assignment.expression", Boolean.toString(evaluatesToTrue)),
createSimplifyToAssignmentFix()
);
}
else if (shouldReportConditionAlwaysTrueOrFalse(psiAnchor, evaluatesToTrue) && !visitor.silenceConstantCondition(instruction)) {
final LocalQuickFix fix = createSimplifyBooleanExpressionFix(psiAnchor, evaluatesToTrue);
String message = InspectionsBundle.message(underBinary ?
"dataflow.message.constant.condition.when.reached" :
"dataflow.message.constant.condition", Boolean.toString(evaluatesToTrue));
holder.registerProblem(psiAnchor, message, fix == null ? null : new LocalQuickFix[]{fix});
}
reportedAnchors.add(psiAnchor);
}
}
private static void reportNullableArguments(StandardDataFlowRunner runner, ProblemsHolder holder) {
Set<PsiExpression> exprs = runner.getNullableArguments();
for (PsiExpression expr : exprs) {
final String text = isNullLiteralExpression(expr)
@@ -252,22 +272,25 @@ public class DataFlowInspection extends BaseLocalInspectionTool {
LocalQuickFix[] fixes = createNPEFixes(expr, expr);
holder.registerProblem(expr, text, fixes);
}
}
exprs = runner.getNullableAssignments();
for (PsiExpression expr : exprs) {
private static void reportNullableAssignments(StandardDataFlowRunner runner, ProblemsHolder holder) {
for (PsiExpression expr : runner.getNullableAssignments()) {
final String text = isNullLiteralExpression(expr)
? InspectionsBundle.message("dataflow.message.assigning.null")
: InspectionsBundle.message("dataflow.message.assigning.nullable");
holder.registerProblem(expr, text);
}
}
exprs = runner.getUnboxedNullables();
for (PsiExpression expr : exprs) {
private static void reportUnboxedNullables(StandardDataFlowRunner runner, ProblemsHolder holder) {
for (PsiExpression expr : runner.getUnboxedNullables()) {
holder.registerProblem(expr, InspectionsBundle.message("dataflow.message.unboxing"));
}
}
final Set<PsiReturnStatement> statements = runner.getNullableReturns();
for (PsiReturnStatement statement : statements) {
private static void reportNullableReturns(StandardDataFlowRunner runner, ProblemsHolder holder) {
for (PsiReturnStatement statement : runner.getNullableReturns()) {
final PsiExpression expr = statement.getReturnValue();
if (runner.isInNotNullMethod()) {
final String text = isNullLiteralExpression(expr)
java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java
+13
View File
@@ -36,6 +36,7 @@ import java.util.Set;
public class StandardInstructionVisitor extends InstructionVisitor {
private final Set<BinopInstruction> myReachable = new THashSet<BinopInstruction>();
private final Set<BinopInstruction> myCanBeNullInInstanceof = new THashSet<BinopInstruction>();
private final Set<BinopInstruction> myNotToReportReachability = new THashSet<BinopInstruction>();
private final Set<InstanceofInstruction> myUsefulInstanceofs = new THashSet<InstanceofInstruction>();
private final FactoryMap<MethodCallInstruction, boolean[]> myParametersNotNull = new FactoryMap<MethodCallInstruction, boolean[]>() {
@Override
@@ -298,7 +299,11 @@ public class StandardInstructionVisitor extends InstructionVisitor {
return null;
}
if (isViaMethods(dfaLeft) || isViaMethods(dfaRight)) {
myNotToReportReachability.add(instruction);
}
myCanBeNullInInstanceof.add(instruction);
ArrayList<DfaInstructionState> states = new ArrayList<DfaInstructionState>();
final DfaMemoryState trueCopy = memState.createCopy();
@@ -328,6 +333,10 @@ public class StandardInstructionVisitor extends InstructionVisitor {
return states.toArray(new DfaInstructionState[states.size()]);
}
private static boolean isViaMethods(DfaValue dfa) {
return dfa instanceof DfaVariableValue && ((DfaVariableValue)dfa).isViaMethods();
}
private void handleInstanceof(InstanceofInstruction instruction, DfaValue dfaRight, DfaValue dfaLeft) {
if ((dfaLeft instanceof DfaTypeValue || dfaLeft instanceof DfaNotNullValue) && dfaRight instanceof DfaTypeValue) {
final PsiType leftType;
@@ -397,4 +406,8 @@ public class StandardInstructionVisitor extends InstructionVisitor {
public boolean canBeNull(BinopInstruction instruction) {
return myCanBeNullInInstanceof.contains(instruction);
}
public boolean silenceConstantCondition(BranchingInstruction instruction) {
return instruction instanceof BinopInstruction && myNotToReportReachability.contains(instruction);
}
}
java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java
+14 -5
View File
@@ -33,6 +33,7 @@ import java.util.ArrayList;
import java.util.List;
public class DfaVariableValue extends DfaValue {
public static class Factory {
private final DfaVariableValue mySharedInstance;
private final HashMap<String,ArrayList<DfaVariableValue>> myStringToObject;
@@ -46,12 +47,13 @@ public class DfaVariableValue extends DfaValue {
}
public DfaVariableValue createVariableValue(PsiVariable myVariable, boolean isNegated) {
return createVariableValue(myVariable, isNegated, null);
return createVariableValue(myVariable, isNegated, null, false);
}
public DfaVariableValue createVariableValue(PsiVariable myVariable, boolean isNegated, @Nullable DfaVariableValue qualifier) {
public DfaVariableValue createVariableValue(PsiVariable myVariable, boolean isNegated, @Nullable DfaVariableValue qualifier, boolean viaMethods) {
mySharedInstance.myVariable = myVariable;
mySharedInstance.myIsNegated = isNegated;
mySharedInstance.myQualifier = qualifier;
mySharedInstance.myViaMethods = viaMethods;
String id = mySharedInstance.toString();
ArrayList<DfaVariableValue> conditions = myStringToObject.get(id);
@@ -65,7 +67,7 @@ public class DfaVariableValue extends DfaValue {
}
}
DfaVariableValue result = new DfaVariableValue(myVariable, isNegated, myFactory, qualifier);
DfaVariableValue result = new DfaVariableValue(myVariable, isNegated, myFactory, qualifier, viaMethods);
if (qualifier != null) {
myQualifiersToChainedVariables.putValue(qualifier, result);
}
@@ -87,12 +89,14 @@ public class DfaVariableValue extends DfaValue {
private PsiVariable myVariable;
@Nullable private DfaVariableValue myQualifier;
private boolean myIsNegated;
private boolean myViaMethods;
private DfaVariableValue(PsiVariable variable, boolean isNegated, DfaValueFactory factory, @Nullable DfaVariableValue qualifier) {
private DfaVariableValue(PsiVariable variable, boolean isNegated, DfaValueFactory factory, @Nullable DfaVariableValue qualifier, boolean viaMethods) {
super(factory);
myVariable = variable;
myIsNegated = isNegated;
myQualifier = qualifier;
myViaMethods = viaMethods;
}
private DfaVariableValue(DfaValueFactory factory) {
@@ -111,7 +115,7 @@ public class DfaVariableValue extends DfaValue {
}
public DfaVariableValue createNegated() {
return myFactory.getVarFactory().createVariableValue(myVariable, !myIsNegated, myQualifier);
return myFactory.getVarFactory().createVariableValue(myVariable, !myIsNegated, myQualifier, myViaMethods);
}
@SuppressWarnings({"HardCodedStringLiteral"})
@@ -123,6 +127,7 @@ public class DfaVariableValue extends DfaValue {
private boolean hardEquals(DfaVariableValue aVar) {
return aVar.myVariable == myVariable &&
aVar.myIsNegated == myIsNegated &&
aVar.myViaMethods == myViaMethods &&
(myQualifier == null ? aVar.myQualifier == null : myQualifier.hardEquals(aVar.myQualifier));
}
@@ -130,4 +135,8 @@ public class DfaVariableValue extends DfaValue {
public DfaVariableValue getQualifier() {
return myQualifier;
}
public boolean isViaMethods() {
return myViaMethods;
}
}
java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldAccessorsDfa.java
-2
View File
@@ -15,11 +15,9 @@ public class BrokenAlignment {
if (data.inner() != null) {
System.out.println(data.inner().hashCode());
System.out.println(<warning descr="Method invocation 'data.inner().getText().hashCode()' may produce 'java.lang.NullPointerException'">data.inner().getText().hashCode()</warning>);
/*
if (data.inner() != null) {
System.out.println(data.inner().hashCode());
}
*/
data = new Data(null, null);
System.out.println(<warning descr="Method invocation 'data.inner().hashCode()' may produce 'java.lang.NullPointerException'">data.inner().hashCode()</warning>);
platform/platform-resources-en/src/messages/InspectionsBundle.properties
+1 -1
View File
@@ -59,7 +59,7 @@ dataflow.message.npe.field.access=Dereference of <code>#ref</code> #loc may prod
dataflow.message.cce=Casting <code>{0}</code> to <code>#ref</code> #loc may produce <code>java.lang.ClassCastException</code>
dataflow.message.redundant.instanceof=Condition <code>#ref</code> #loc is redundant and can be replaced with <code>!= null</code>
dataflow.message.constant.condition=Condition <code>#ref</code> #loc is always <code>{0}</code>
dataflow.message.constant.condition.whenriched=Condition <code>#ref</code> #loc is always <code>{0}</code> when reached
dataflow.message.constant.condition.when.reached=Condition <code>#ref</code> #loc is always <code>{0}</code> when reached
dataflow.message.unreachable.switch.label=Switch label<code>#ref</code> #loc is unreachable
dataflow.message.pointless.assignment.expression=Condition <code>#ref</code> #loc at the left side of assignment expression is always <code>{0}</code>. Can be simplified to normal assignment
dataflow.message.passing.null.argument=Passing <code>null</code> argument to parameter annotated as @NotNull