ControlFlowAnalyzer: use common element from array/list on for-each; cosmetics

This commit is contained in:
Tagir Valeev
2018-04-17 12:46:53 +07:00
parent dd16819fb9
commit e9a41d3abc
3 changed files with 113 additions and 62 deletions
@@ -19,10 +19,18 @@ import com.intellij.codeInsight.AnnotationUtil;
import com.intellij.codeInsight.ExceptionUtil;
import com.intellij.codeInsight.daemon.ImplicitUsageProvider;
import com.intellij.codeInsight.daemon.impl.UnusedSymbolUtil;
import com.intellij.codeInspection.dataFlow.ControlFlow.ControlFlowOffset;
import com.intellij.codeInspection.dataFlow.MethodContract.ValueConstraint;
import com.intellij.codeInspection.dataFlow.Trap.InsideFinally;
import com.intellij.codeInspection.dataFlow.Trap.TryCatch;
import com.intellij.codeInspection.dataFlow.Trap.TryFinally;
import com.intellij.codeInspection.dataFlow.Trap.TwrFinally;
import com.intellij.codeInspection.dataFlow.inliner.*;
import com.intellij.codeInspection.dataFlow.instructions.*;
import com.intellij.codeInspection.dataFlow.instructions.MethodCallInstruction.MethodType;
import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet;
import com.intellij.codeInspection.dataFlow.value.*;
import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.extensions.Extensions;
import com.intellij.openapi.project.Project;
@@ -30,12 +38,14 @@ import com.intellij.openapi.util.registry.Registry;
import com.intellij.psi.*;
import com.intellij.psi.search.GlobalSearchScope;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.CachedValueProvider.Result;
import com.intellij.psi.util.*;
import com.intellij.psi.util.InheritanceUtil;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.FList;
import com.siyeh.ig.callMatcher.CallMatcher;
import com.siyeh.ig.numeric.UnnecessaryExplicitNumericCastInspection;
import com.siyeh.ig.psiutils.*;
import one.util.streamex.StreamEx;
@@ -51,6 +61,9 @@ import static com.intellij.psi.CommonClassNames.*;
public class ControlFlowAnalyzer extends JavaElementVisitor {
private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer");
public static final String ORG_JETBRAINS_ANNOTATIONS_CONTRACT = Contract.class.getName();
private static final CallMatcher LIST_INITIALIZER = CallMatcher.anyOf(
CallMatcher.staticCall(JAVA_UTIL_ARRAYS, "asList"),
CallMatcher.staticCall(JAVA_UTIL_LIST, "of"));
static final int MAX_UNROLL_SIZE = 3;
private final PsiElement myCodeFragment;
private final boolean myIgnoreAssertions;
@@ -164,11 +177,11 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
return myCurrentFlow.getInstructionCount();
}
private ControlFlow.ControlFlowOffset getEndOffset(PsiElement element) {
private ControlFlowOffset getEndOffset(PsiElement element) {
return myCurrentFlow.getEndOffset(element);
}
private ControlFlow.ControlFlowOffset getStartOffset(PsiElement element) {
private ControlFlowOffset getStartOffset(PsiElement element) {
return myCurrentFlow.getStartOffset(element);
}
@@ -300,10 +313,8 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
else if (!field.hasModifierProperty(PsiModifier.FINAL) && !UnusedSymbolUtil.isImplicitWrite(field)) {
// initialize with default value
DfaVariableValue dfaVariable = myFactory.getVarFactory().createVariableValue(field);
addInstruction(new PushInstruction(dfaVariable, null, true));
addInstruction(new PushInstruction(myFactory.getConstFactory().createDefault(field.getType()), null));
addInstruction(new AssignInstruction(null, dfaVariable));
addInstruction(new PopInstruction());
DfaConstValue value = myFactory.getConstFactory().createDefault(field.getType());
new CFGBuilder(this).assignAndPop(dfaVariable, value);
}
}
@@ -476,12 +487,32 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
finishElement(statement);
}
private DfaValue getIteratedElement(PsiExpression iteratedValue) {
PsiExpression[] expressions = null;
if (iteratedValue instanceof PsiNewExpression) {
PsiArrayInitializerExpression initializer = ((PsiNewExpression)iteratedValue).getArrayInitializer();
if (initializer != null) {
expressions = initializer.getInitializers();
}
}
else if (iteratedValue instanceof PsiReferenceExpression) {
PsiElement arrayVar = ((PsiReferenceExpression)iteratedValue).resolve();
if (arrayVar instanceof PsiVariable) {
expressions = ExpressionUtils.getConstantArrayElements((PsiVariable)arrayVar);
}
}
if (iteratedValue instanceof PsiMethodCallExpression && LIST_INITIALIZER.test((PsiMethodCallExpression)iteratedValue)) {
expressions = ((PsiMethodCallExpression)iteratedValue).getArgumentList().getExpressions();
}
return expressions == null ? DfaUnknownValue.getInstance() : getFactory().createCommonValue(expressions);
}
@Override public void visitForeachStatement(PsiForeachStatement statement) {
startElement(statement);
final PsiParameter parameter = statement.getIterationParameter();
final PsiExpression iteratedValue = statement.getIteratedValue();
final PsiExpression iteratedValue = PsiUtil.skipParenthesizedExprDown(statement.getIteratedValue());
ControlFlow.ControlFlowOffset loopEndOffset = getEndOffset(statement);
ControlFlowOffset loopEndOffset = getEndOffset(statement);
boolean hasSizeCheck = false;
if (iteratedValue != null) {
@@ -508,9 +539,9 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
}
ControlFlow.ControlFlowOffset offset = myCurrentFlow.getNextOffset();
ControlFlowOffset offset = myCurrentFlow.getNextOffset();
DfaVariableValue dfaVariable = myFactory.getVarFactory().createVariableValue(parameter);
addInstruction(new FlushVariableInstruction(dfaVariable));
new CFGBuilder(this).assignAndPop(dfaVariable, getIteratedElement(iteratedValue));
if (!hasSizeCheck) {
pushUnknown();
@@ -580,9 +611,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
}
ControlFlow.ControlFlowOffset offset = initialization != null
? getEndOffset(initialization)
: getStartOffset(statement);
ControlFlowOffset offset = initialization != null ? getEndOffset(initialization) : getStartOffset(statement);
addInstruction(new GotoInstruction(offset));
finishElement(statement);
@@ -640,9 +669,9 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
if (origin == null) return false;
long diff = start == null || end == null ? -1 : end - start;
DfaVariableValue loopVar = myFactory.getVarFactory().createVariableValue(counter);
addInstruction(new PushInstruction(loopVar, null, true));
if(diff >= 0 && diff <= MAX_UNROLL_SIZE) {
// Unroll small loops
addInstruction(new PushInstruction(loopVar, null, true));
addInstruction(new PushInstruction(loopVar, null));
addInstruction(new PushInstruction(myFactory.getConstFactory().createFromValue(1, PsiType.INT, null), null));
addInstruction(new BinopInstruction(JavaTokenType.PLUS, null, loopVar.getVariableType()));
@@ -661,15 +690,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
addInstruction(new GotoInstruction(getEndOffset(statement)));
}
else {
addInstruction(new PushInstruction(myFactory.getFactValue(DfaFactType.RANGE, LongRangeSet.range(start + 1L, maxValue)), null));
addInstruction(new AssignInstruction(null, null));
addInstruction(new PopInstruction());
DfaValue range = myFactory.getFactValue(DfaFactType.RANGE, LongRangeSet.range(start + 1L, maxValue));
new CFGBuilder(this).assignAndPop(loopVar, range);
}
} else {
pushUnknown();
addInstruction(new AssignInstruction(null, null));
addInstruction(new PushInstruction(origin, null));
addInstruction(new BinopInstruction(JavaTokenType.LE, null, PsiType.BOOLEAN));
new CFGBuilder(this).assign(loopVar, DfaUnknownValue.getInstance())
.push(origin)
.compare(JavaTokenType.LE);
addInstruction(new ConditionalGotoInstruction(getEndOffset(statement), false, null));
}
return true;
@@ -683,9 +710,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
PsiStatement thenStatement = statement.getThenBranch();
PsiStatement elseStatement = statement.getElseBranch();
ControlFlow.ControlFlowOffset offset = elseStatement != null
? getStartOffset(elseStatement)
: getEndOffset(statement);
ControlFlowOffset offset = elseStatement != null ? getStartOffset(elseStatement) : getEndOffset(statement);
if (condition != null) {
condition.accept(this);
@@ -859,7 +884,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
else {
try {
ControlFlow.ControlFlowOffset offset = getStartOffset(statement);
ControlFlowOffset offset = getStartOffset(statement);
PsiExpression caseValue = psiLabelStatement.getCaseValue();
if (enumValues != null && caseValue instanceof PsiReferenceExpression) {
@@ -894,7 +919,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
if (enumValues == null || !enumValues.isEmpty()) {
ControlFlow.ControlFlowOffset offset = defaultLabel != null ? getStartOffset(defaultLabel) : getEndOffset(body);
ControlFlowOffset offset = defaultLabel != null ? getStartOffset(defaultLabel) : getEndOffset(body);
addInstruction(new GotoInstruction(offset));
}
@@ -978,21 +1003,21 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
PsiCodeBlock tryBlock = statement.getTryBlock();
PsiCodeBlock finallyBlock = statement.getFinallyBlock();
Trap.TryFinally finallyDescriptor = finallyBlock != null ? new Trap.TryFinally(finallyBlock, getStartOffset(finallyBlock)) : null;
TryFinally finallyDescriptor = finallyBlock != null ? new TryFinally(finallyBlock, getStartOffset(finallyBlock)) : null;
if (finallyDescriptor != null) {
myTrapStack = myTrapStack.prepend(finallyDescriptor);
}
PsiCatchSection[] sections = statement.getCatchSections();
if (sections.length > 0) {
LinkedHashMap<PsiCatchSection, ControlFlow.ControlFlowOffset> clauses = new LinkedHashMap<>();
LinkedHashMap<PsiCatchSection, ControlFlowOffset> clauses = new LinkedHashMap<>();
for (PsiCatchSection section : sections) {
PsiCodeBlock catchBlock = section.getCatchBlock();
if (catchBlock != null) {
clauses.put(section, getStartOffset(catchBlock));
}
}
myTrapStack = myTrapStack.prepend(new Trap.TryCatch(statement, clauses));
myTrapStack = myTrapStack.prepend(new TryCatch(statement, clauses));
}
processTryWithResources(resourceList, tryBlock);
@@ -1002,7 +1027,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
controlTransfer(gotoEnd, singleFinally);
if (sections.length > 0) {
assert myTrapStack.getHead() instanceof Trap.TryCatch;
assert myTrapStack.getHead() instanceof TryCatch;
myTrapStack = myTrapStack.getTail();
}
@@ -1015,13 +1040,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
if (finallyBlock != null) {
assert myTrapStack.getHead() instanceof Trap.TryFinally;
myTrapStack = myTrapStack.getTail().prepend(new Trap.InsideFinally(finallyBlock));
assert myTrapStack.getHead() instanceof TryFinally;
myTrapStack = myTrapStack.getTail().prepend(new InsideFinally(finallyBlock));
finallyBlock.accept(this);
addInstruction(new ControlTransferInstruction(null)); // DfaControlTransferValue is on stack
assert myTrapStack.getHead() instanceof Trap.InsideFinally;
assert myTrapStack.getHead() instanceof InsideFinally;
myTrapStack = myTrapStack.getTail();
}
@@ -1030,13 +1055,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
private void processTryWithResources(@Nullable PsiResourceList resourceList, @Nullable PsiCodeBlock tryBlock) {
Set<PsiClassType> closerExceptions = Collections.emptySet();
Trap.TwrFinally twrFinallyDescriptor = null;
TwrFinally twrFinallyDescriptor = null;
if (resourceList != null) {
resourceList.accept(this);
closerExceptions = StreamEx.of(resourceList.iterator()).flatCollection(ExceptionUtil::getCloserExceptions).toSet();
if (!closerExceptions.isEmpty()) {
twrFinallyDescriptor = new Trap.TwrFinally(resourceList, getStartOffset(resourceList));
twrFinallyDescriptor = new TwrFinally(resourceList, getStartOffset(resourceList));
myTrapStack = myTrapStack.prepend(twrFinallyDescriptor);
}
}
@@ -1046,15 +1071,15 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
if (twrFinallyDescriptor != null) {
assert myTrapStack.getHead() instanceof Trap.TwrFinally;
assert myTrapStack.getHead() instanceof TwrFinally;
InstructionTransfer gotoEnd = new InstructionTransfer(getEndOffset(resourceList), getVariablesInside(tryBlock));
controlTransfer(gotoEnd, FList.createFromReversed(ContainerUtil.createMaybeSingletonList(twrFinallyDescriptor)));
myTrapStack = myTrapStack.getTail().prepend(new Trap.InsideFinally(resourceList));
myTrapStack = myTrapStack.getTail().prepend(new InsideFinally(resourceList));
startElement(resourceList);
addThrows(null, closerExceptions.toArray(PsiClassType.EMPTY_ARRAY));
addInstruction(new ControlTransferInstruction(null)); // DfaControlTransferValue is on stack
finishElement(resourceList);
assert myTrapStack.getHead() instanceof Trap.InsideFinally;
assert myTrapStack.getHead() instanceof InsideFinally;
myTrapStack = myTrapStack.getTail();
}
}
@@ -1232,10 +1257,8 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
addInstruction(new AssignInstruction(originalExpression, var));
}
// Declaration: write array length
addInstruction(new PushInstruction(SpecialField.ARRAY_LENGTH.createValue(getFactory(), var), null, true));
addInstruction(new PushInstruction(getFactory().getInt(expression.getInitializers().length), null));
addInstruction(new AssignInstruction(null, null));
addInstruction(new PopInstruction());
DfaConstValue lengthValue = getFactory().getInt(expression.getInitializers().length);
new CFGBuilder(this).assignAndPop(SpecialField.ARRAY_LENGTH.createValue(getFactory(), var), lengthValue);
}
@Override
@@ -1346,7 +1369,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
parent instanceof PsiArrayAccessExpression) {
return true;
}
if (parent instanceof PsiBinaryExpression && DfaRelationValue.RelationType.fromElementType(((PsiBinaryExpression)parent).getOperationTokenType()) != null) {
if (parent instanceof PsiBinaryExpression && RelationType.fromElementType(((PsiBinaryExpression)parent).getOperationTokenType()) != null) {
return true;
}
if (parent instanceof PsiLoopStatement) return false;
@@ -1389,18 +1412,18 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
if (PsiType.VOID.equals(expectedType)) return;
if (TypeConversionUtil.isPrimitiveAndNotNull(expectedType) && TypeConversionUtil.isPrimitiveWrapper(actualType)) {
addInstruction(new MethodCallInstruction(context, MethodCallInstruction.MethodType.UNBOXING, expectedType));
addInstruction(new MethodCallInstruction(context, MethodType.UNBOXING, expectedType));
}
else if (TypeConversionUtil.isPrimitiveAndNotNull(actualType) && TypeConversionUtil.isAssignableFromPrimitiveWrapper(expectedType)) {
addConditionalRuntimeThrow();
addInstruction(new MethodCallInstruction(context, MethodCallInstruction.MethodType.BOXING, expectedType));
addInstruction(new MethodCallInstruction(context, MethodType.BOXING, expectedType));
}
else if (actualType != expectedType &&
TypeConversionUtil.isPrimitiveAndNotNull(actualType) &&
TypeConversionUtil.isPrimitiveAndNotNull(expectedType) &&
TypeConversionUtil.isNumericType(actualType) &&
TypeConversionUtil.isNumericType(expectedType)) {
addInstruction(new MethodCallInstruction(context, MethodCallInstruction.MethodType.CAST, expectedType) {
addInstruction(new MethodCallInstruction(context, MethodType.CAST, expectedType) {
@Override
public DfaInstructionState[] accept(DataFlowRunner runner, DfaMemoryState stateBefore, InstructionVisitor visitor) {
return visitor.visitCast(this, runner, stateBefore);
@@ -1525,7 +1548,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
PsiExpression thenExpression = expression.getThenExpression();
PsiExpression elseExpression = expression.getElseExpression();
final ControlFlow.ControlFlowOffset elseOffset = elseExpression == null ? ControlFlow.deltaOffset(getEndOffset(expression), -1) : getStartOffset(elseExpression);
final ControlFlowOffset elseOffset = elseExpression == null ? ControlFlow.deltaOffset(getEndOffset(expression), -1) : getStartOffset(elseExpression);
if (thenExpression != null) {
condition.accept(this);
generateBoxingUnboxingInstructionFor(condition, PsiType.BOOLEAN);
@@ -1663,7 +1686,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
anchor = expression;
}
addInstruction(instruction);
if (contracts.stream().anyMatch(c -> c.getReturnValue() == MethodContract.ValueConstraint.THROW_EXCEPTION)) {
if (contracts.stream().anyMatch(c -> c.getReturnValue() == ValueConstraint.THROW_EXCEPTION)) {
// if a contract resulted in 'fail', handle it
addInstruction(new DupInstruction());
addInstruction(new PushInstruction(myFactory.getConstFactory().getContractFail(), null));
@@ -1698,14 +1721,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
final int paramCount = method.getParameterList().getParametersCount();
List<StandardMethodContract> applicable = ContainerUtil.filter(StandardMethodContract.parseContract(text),
contract -> contract.arguments.length == paramCount);
return CachedValueProvider.Result.create(applicable, contractAnno, method, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT);
return Result.create(applicable, contractAnno, method, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT);
}
catch (Exception ignored) {
}
}
}
return CachedValueProvider.Result
.create(Collections.emptyList(), method, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT);
return Result.create(Collections.emptyList(), method, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT);
});
}
@@ -1804,17 +1826,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
private void setEmptyCollectionSize(PsiNewExpression expression) {
DfaVariableValue var = getTargetVariable(expression);
if (var != null && ConstructionUtils.isEmptyCollectionInitializer(expression)) {
addInstruction(new PopInstruction());
addInstruction(new PushInstruction(var, null, true));
addInstruction(new PushInstruction(myFactory.withFact(
myFactory.createTypeValue(expression.getType(), Nullness.NOT_NULL), DfaFactType.LOCALITY, true), null));
addInstruction(new AssignInstruction(null, null));
DfaValue collectionValue =
myFactory.withFact(myFactory.createTypeValue(expression.getType(), Nullness.NOT_NULL), DfaFactType.LOCALITY, true);
SpecialField sizeField =
InheritanceUtil.isInheritor(expression.getType(), JAVA_UTIL_MAP) ? SpecialField.MAP_SIZE : SpecialField.COLLECTION_SIZE;
addInstruction(new PushInstruction(sizeField.createValue(myFactory, var), null, true));
addInstruction(new PushInstruction(myFactory.getInt(0), null));
addInstruction(new AssignInstruction(null, null));
addInstruction(new PopInstruction());
new CFGBuilder(this).pop()
.assign(var, collectionValue)
.assignAndPop(sizeField.createValue(myFactory, var), myFactory.getInt(0));
}
}
@@ -1933,8 +1951,11 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
final PsiExpression qualifierExpression = expression.getQualifierExpression();
if (qualifierExpression != null) {
qualifierExpression.accept(this);
addInstruction(expression.resolve() instanceof PsiField ? new DereferenceInstruction(qualifierExpression) : new PopInstruction());
PsiElement target = expression.resolve();
if (!(target instanceof PsiMember) || !((PsiMember)target).hasModifierProperty(PsiModifier.STATIC)) {
qualifierExpression.accept(this);
addInstruction(target instanceof PsiField ? new DereferenceInstruction(qualifierExpression) : new PopInstruction());
}
}
// complex assignments (e.g. "|=") are both reading and writing
@@ -0,0 +1,29 @@
import java.util.*;
import org.jetbrains.annotations.*;
class ForeachCollectionElement {
void test() {
int[] arr = new int [] {10,20,30,40,50,60,70,80};
for(int i : arr) {
if(<warning descr="Condition 'i == 75' is always 'false'">i == 75</warning>) {
System.out.println("Impossible");
}
}
}
void test2() {
for(int i : new int [] {10,20,30,40,50,60,70,80}) {
if(<warning descr="Condition 'i > 71 && i < 79' is always 'false'">i > 71 && <warning descr="Condition 'i < 79' is always 'false' when reached">i < 79</warning></warning>) {
System.out.println("Impossible");
}
}
}
void test3() {
for(String s : Arrays.asList("foo", "bar", "baz")) {
if(<warning descr="Condition 's == null' is always 'false'">s == null</warning>) {
System.out.println("impossible");
}
}
}
}
@@ -226,4 +226,5 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase {
public void testEscapeAnalysis() { doTest(); }
public void testThisAsVariable() { doTest(); }
public void testQueuePeek() { doTest(); }
public void testForeachCollectionElement() { doTest(); }
}