IdempotentLoopBodyInspection: rewritten using control flow

This commit is contained in:
Tagir Valeev
2017-10-13 17:14:39 +07:00
parent 358e6b4c2e
commit e2aaf8754a
4 changed files with 83 additions and 158 deletions
@@ -2,17 +2,14 @@
package com.intellij.codeInspection;
import com.intellij.psi.*;
import com.intellij.psi.controlFlow.*;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.ig.psiutils.SideEffectChecker;
import com.siyeh.ig.psiutils.VariableAccessUtils;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Function;
import java.util.Collection;
import java.util.List;
import static com.intellij.util.ObjectUtils.tryCast;
@@ -25,147 +22,71 @@ public class IdempotentLoopBodyInspection extends AbstractBaseJavaLocalInspectio
public void visitWhileStatement(PsiWhileStatement loop) {
PsiExpression condition = loop.getCondition();
if (condition == null || SideEffectChecker.mayHaveSideEffects(condition)) return;
if (isIdempotent(loop.getBody())) {
holder.registerProblem(loop.getFirstChild(), InspectionsBundle.message("inspection.idempotent.loop.body"));
PsiStatement body = loop.getBody();
if (body == null) return;
if (mayHaveUndesiredSideEffects(loop, body)) return;
final ControlFlow controlFlow;
try {
controlFlow =
ControlFlowFactory.getInstance(loop.getProject()).getControlFlow(body, LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance());
}
catch (AnalysisCanceledException ignored) {
return;
}
checkControlFlow(loop, controlFlow, holder);
}
@Override
public void visitForStatement(PsiForStatement loop) {
PsiExpression condition = loop.getCondition();
if (condition != null && SideEffectChecker.mayHaveSideEffects(condition)) return;
if (isIdempotent(loop.getBody(), loop.getUpdate())) {
PsiStatement body = loop.getBody();
if (body == null) return;
PsiStatement update = loop.getUpdate();
if (mayHaveUndesiredSideEffects(loop, body) || update != null && mayHaveUndesiredSideEffects(loop, update)) return;
ControlFlow controlFlow;
try {
controlFlow =
ControlFlowFactory.getInstance(loop.getProject()).getControlFlow(loop, LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance());
}
catch (AnalysisCanceledException ignored) {
return;
}
int start = controlFlow.getStartOffset(body);
int end = controlFlow.getEndOffset(update == null ? body : update);
if(start == -1 || end == -1) return;
controlFlow = new ControlFlowSubRange(controlFlow, start, end);
checkControlFlow(loop, controlFlow, holder);
}
private void checkControlFlow(PsiLoopStatement loop, ControlFlow bodyFlow, @NotNull ProblemsHolder holder) {
Collection<PsiVariable> variables = ControlFlowUtil.getWrittenVariables(bodyFlow, 0, bodyFlow.getSize(), true);
if (variables.isEmpty()) return;
List<PsiReferenceExpression> reads = ControlFlowUtil.getReadBeforeWrite(bodyFlow);
if (StreamEx.of(reads).map(PsiReferenceExpression::resolve).select(PsiVariable.class).noneMatch(variables::contains)) {
holder.registerProblem(loop.getFirstChild(), InspectionsBundle.message("inspection.idempotent.loop.body"));
}
}
private boolean isIdempotent(PsiStatement... statements) {
Set<PsiVariable> variables = extractWrites(statements);
if (variables == null || variables.isEmpty()) return false;
if (!(variables instanceof HashSet)) {
variables = new HashSet<>(variables);
}
for (PsiStatement statement : statements) {
if (usesInputVariable(statement, variables)) {
return false;
}
}
return true;
}
private boolean usesInputVariable(PsiStatement statement, Set<PsiVariable> variables) {
if (statement == null) return false;
if (statement instanceof PsiBlockStatement) {
for (PsiStatement st : ((PsiBlockStatement)statement).getCodeBlock().getStatements()) {
if (usesInputVariable(st, variables)) {
return true;
}
}
return false;
}
if (statement instanceof PsiExpressionStatement) {
PsiAssignmentExpression assignment = tryCast(((PsiExpressionStatement)statement).getExpression(), PsiAssignmentExpression.class);
if (assignment != null) {
if (anyVariableIsUsed(assignment.getRExpression(), variables)) return true;
PsiReferenceExpression ref =
tryCast(PsiUtil.skipParenthesizedExprDown(assignment.getLExpression()), PsiReferenceExpression.class);
if (ref != null) {
PsiElement var = ref.resolve();
if (var instanceof PsiVariable) {
variables.remove(var);
private boolean mayHaveUndesiredSideEffects(PsiLoopStatement loop, @NotNull PsiElement element) {
return SideEffectChecker.mayHaveSideEffects(element, e -> {
if (e instanceof PsiContinueStatement && ((PsiContinueStatement)e).findContinuedStatement() == loop) return true;
if (e instanceof PsiLocalVariable) return true;
if (e instanceof PsiAssignmentExpression) {
PsiAssignmentExpression assignment = (PsiAssignmentExpression)e;
if (assignment.getOperationTokenType() == JavaTokenType.EQ) {
PsiReferenceExpression ref =
tryCast(PsiUtil.skipParenthesizedExprDown(assignment.getLExpression()), PsiReferenceExpression.class);
if (ref != null) {
PsiElement target = ref.resolve();
if (target instanceof PsiLocalVariable || target instanceof PsiParameter) return true;
}
}
return false;
}
}
if (statement instanceof PsiIfStatement) {
PsiIfStatement ifStatement = (PsiIfStatement)statement;
if (anyVariableIsUsed(ifStatement.getCondition(), variables)) return true;
Set<PsiVariable> thenVars = new HashSet<>(variables);
if (usesInputVariable(ifStatement.getThenBranch(), thenVars)) return true;
Set<PsiVariable> elseVars = new HashSet<>(variables);
if (usesInputVariable(ifStatement.getElseBranch(), elseVars)) return true;
thenVars.addAll(elseVars);
variables.retainAll(thenVars);
return false;
}
if (statement instanceof PsiDeclarationStatement) {
StreamEx.of(((PsiDeclarationStatement)statement).getDeclaredElements()).select(PsiVariable.class)
.forEach(variables::remove);
}
return anyVariableIsUsed(statement, variables);
}
private boolean anyVariableIsUsed(@Nullable PsiElement statement, @NotNull Set<PsiVariable> variables) {
return VariableAccessUtils.collectUsedVariables(statement).stream().anyMatch(variables::contains);
}
/**
* Extract written variables from statement which may affect the next iteration
* @param statement
* @return list of written variables or null if the statement may have unknown side effects, thus further analysis is impossible.
*/
@Nullable
private Set<PsiVariable> extractWrites(@Nullable PsiStatement statement) {
if (statement == null ||
statement instanceof PsiEmptyStatement ||
(statement instanceof PsiContinueStatement && ((PsiContinueStatement)statement).getLabelIdentifier() == null)) {
return Collections.emptySet();
}
if (statement instanceof PsiBlockStatement) {
PsiStatement[] statements = ((PsiBlockStatement)statement).getCodeBlock().getStatements();
return extractWrites(statements);
}
if (statement instanceof PsiDeclarationStatement) {
PsiElement[] elements = ((PsiDeclarationStatement)statement).getDeclaredElements();
for (PsiElement element : elements) {
if (!(element instanceof PsiLocalVariable)) return null;
PsiLocalVariable var = (PsiLocalVariable)element;
PsiExpression initializer = var.getInitializer();
if (initializer != null && SideEffectChecker.mayHaveSideEffects(initializer)) return null;
}
return Collections.emptySet();
}
if (statement instanceof PsiExpressionStatement) {
PsiAssignmentExpression assignment = tryCast(((PsiExpressionStatement)statement).getExpression(), PsiAssignmentExpression.class);
if (assignment == null ||
assignment.getOperationTokenType() != JavaTokenType.EQ ||
assignment.getRExpression() == null ||
SideEffectChecker.mayHaveSideEffects(assignment.getRExpression())) {
return null;
}
PsiReferenceExpression ref =
tryCast(PsiUtil.skipParenthesizedExprDown(assignment.getLExpression()), PsiReferenceExpression.class);
if (ref == null) return null;
PsiElement var = ref.resolve();
if (var instanceof PsiLocalVariable || var instanceof PsiParameter) {
return Collections.singleton((PsiVariable)var);
}
}
if (statement instanceof PsiIfStatement) {
PsiIfStatement ifStatement = (PsiIfStatement)statement;
PsiExpression condition = ifStatement.getCondition();
if (condition == null || SideEffectChecker.mayHaveSideEffects(condition)) return null;
Set<PsiVariable> thenResult = extractWrites(ifStatement.getThenBranch());
if (thenResult == null) return null;
Set<PsiVariable> elseResult = extractWrites(ifStatement.getElseBranch());
if (elseResult == null) return null;
if (thenResult.isEmpty()) return elseResult;
if (elseResult.isEmpty()) return thenResult;
return StreamEx.of(thenResult, elseResult).toFlatCollection(Function.identity(), HashSet::new);
}
return null;
}
@Nullable
private Set<PsiVariable> extractWrites(PsiStatement... statements) {
Set<PsiVariable> result = new HashSet<>();
for (PsiStatement subStatement : statements) {
Set<PsiVariable> subResult = extractWrites(subStatement);
if (subResult == null) return null;
result.addAll(subResult);
}
return result;
});
}
};
}
@@ -24,12 +24,12 @@ import java.util.ArrayList;
import java.util.List;
public class ControlFlowSubRange implements ControlFlow {
private final ControlFlowImpl myControlFlow;
private final ControlFlow myControlFlow;
private final int myStart;
private final int myEnd;
private List<Instruction> myInstructions;
public ControlFlowSubRange(ControlFlowImpl controlFlow, int start, int end) {
public ControlFlowSubRange(ControlFlow controlFlow, int start, int end) {
myControlFlow = controlFlow;
myStart = start;
myEnd = end;
@@ -261,7 +261,8 @@ public class MismatchedStringBuilderQueryUpdateInspection extends BaseInspection
if (hasReferenceToVariable(variable, qualifierExpression)) {
PsiElement parent = PsiTreeUtil.getParentOfType(expression, PsiStatement.class, PsiLambdaExpression.class);
if (parent instanceof PsiStatement &&
!SideEffectChecker.mayHaveSideEffects(parent, this::isSideEffectFreeBuilderMethodCall)) {
!SideEffectChecker.mayHaveSideEffects(
parent, e -> e instanceof PsiMethodCallExpression && isSideEffectFreeBuilderMethodCall((PsiMethodCallExpression)e))) {
return;
}
queried = true;
@@ -65,8 +65,8 @@ public class SideEffectChecker {
return visitor.mayHaveSideEffects();
}
public static boolean mayHaveSideEffects(@NotNull PsiElement element, Predicate<PsiMethodCallExpression> shouldIgnoreCall) {
final SideEffectsVisitor visitor = new SideEffectsVisitor(null, shouldIgnoreCall);
public static boolean mayHaveSideEffects(@NotNull PsiElement element, Predicate<PsiElement> shouldIgnoreElement) {
final SideEffectsVisitor visitor = new SideEffectsVisitor(null, shouldIgnoreElement);
element.accept(visitor);
return visitor.mayHaveSideEffects();
}
@@ -86,39 +86,39 @@ public class SideEffectChecker {
private static class SideEffectsVisitor extends JavaRecursiveElementWalkingVisitor {
private @Nullable final List<PsiElement> mySideEffects;
boolean found;
final Predicate<PsiMethodCallExpression> myIgnoredCallPredicate;
final Predicate<PsiElement> myIgnorePredicate;
SideEffectsVisitor(@Nullable List<PsiElement> sideEffects) {
this(sideEffects, call -> false);
}
SideEffectsVisitor(@Nullable List<PsiElement> sideEffects, Predicate<PsiMethodCallExpression> predicate) {
myIgnoredCallPredicate = predicate;
SideEffectsVisitor(@Nullable List<PsiElement> sideEffects, Predicate<PsiElement> predicate) {
myIgnorePredicate = predicate;
mySideEffects = sideEffects;
}
private void addSideEffect(PsiElement element) {
private boolean addSideEffect(PsiElement element) {
if (myIgnorePredicate.test(element)) return false;
found = true;
if(mySideEffects != null) {
mySideEffects.add(element);
} else {
stopWalking();
}
return true;
}
@Override
public void visitAssignmentExpression(@NotNull PsiAssignmentExpression expression) {
addSideEffect(expression);
if (addSideEffect(expression)) return;
super.visitAssignmentExpression(expression);
}
@Override
public void visitMethodCallExpression(@NotNull PsiMethodCallExpression expression) {
if (!myIgnoredCallPredicate.test(expression)) {
final PsiMethod method = expression.resolveMethod();
if (!isPure(method)) {
addSideEffect(expression);
return;
}
final PsiMethod method = expression.resolveMethod();
if (!isPure(method)) {
if (addSideEffect(expression)) return;
}
super.visitMethodCallExpression(expression);
}
@@ -132,8 +132,7 @@ public class SideEffectChecker {
@Override
public void visitNewExpression(@NotNull PsiNewExpression expression) {
if(!isSideEffectFreeConstructor(expression)) {
addSideEffect(expression);
return;
if (addSideEffect(expression)) return;
}
super.visitNewExpression(expression);
}
@@ -142,20 +141,21 @@ public class SideEffectChecker {
public void visitUnaryExpression(@NotNull PsiUnaryExpression expression) {
final IElementType tokenType = expression.getOperationTokenType();
if (tokenType.equals(JavaTokenType.PLUSPLUS) || tokenType.equals(JavaTokenType.MINUSMINUS)) {
addSideEffect(expression);
return;
if (addSideEffect(expression)) return;
}
super.visitUnaryExpression(expression);
}
@Override
public void visitDeclarationStatement(PsiDeclarationStatement statement) {
addSideEffect(statement);
public void visitVariable(PsiVariable variable) {
if (addSideEffect(variable)) return;
super.visitVariable(variable);
}
@Override
public void visitBreakStatement(PsiBreakStatement statement) {
addSideEffect(statement);
if (addSideEffect(statement)) return;
super.visitBreakStatement(statement);
}
@Override
@@ -165,17 +165,20 @@ public class SideEffectChecker {
@Override
public void visitContinueStatement(PsiContinueStatement statement) {
addSideEffect(statement);
if (addSideEffect(statement)) return;
super.visitContinueStatement(statement);
}
@Override
public void visitReturnStatement(PsiReturnStatement statement) {
addSideEffect(statement);
if (addSideEffect(statement)) return;
super.visitReturnStatement(statement);
}
@Override
public void visitThrowStatement(PsiThrowStatement statement) {
addSideEffect(statement);
if (addSideEffect(statement)) return;
super.visitThrowStatement(statement);
}
@Override