CollapseIntoLoopAction: disable when expression refers to written variable

Also, EquivalenceChecker.getComplexElementDecision fixed

GitOrigin-RevId: 6848a57ddc1cf4992030fff614c246a6316f4807
This commit is contained in:
Tagir Valeev
2020-06-16 18:16:46 +07:00
committed by intellij-monorepo-bot
parent 51cd561f79
commit 0322598dbc
6 changed files with 55 additions and 11 deletions

View File

@@ -19,16 +19,15 @@ import com.intellij.util.IncorrectOperationException;
import com.intellij.util.containers.ContainerUtil;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import com.siyeh.ig.psiutils.EquivalenceChecker;
import com.siyeh.ig.psiutils.VariableAccessUtils;
import com.siyeh.ig.psiutils.VariableNameGenerator;
import one.util.streamex.MoreCollectors;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.*;
import java.util.function.Function;
import java.util.stream.Collectors;
import static com.intellij.util.ObjectUtils.tryCast;
@@ -233,6 +232,7 @@ public class CollapseIntoLoopAction implements IntentionAction {
PsiType curType = curIterationExpression.getType();
PsiType firstType = firstIterationExpression.getType();
if (curType == null || !curType.equals(firstType)) return false;
Set<PsiVariable> usedVariables;
if (secondIteration) {
if (!expressionsToReplace.isEmpty()) {
PsiExpression firstExpressionToReplace = expressionsToReplace.get(0);
@@ -240,9 +240,16 @@ public class CollapseIntoLoopAction implements IntentionAction {
if (!firstType.equals(firstExpressionToReplace.getType())) return false;
}
expressionsToReplace.add(firstIterationExpression);
usedVariables = StreamEx.of(firstIterationExpression, curIterationExpression)
.map(VariableAccessUtils::collectUsedVariables).toFlatCollection(Function.identity(), HashSet::new);
}
else {
if (!expressionsToReplace.contains(firstIterationExpression)) return false;
usedVariables = VariableAccessUtils.collectUsedVariables(curIterationExpression);
}
if (!usedVariables.isEmpty() &&
statements.subList(0, count).stream().anyMatch(st -> VariableAccessUtils.isAnyVariableAssigned(usedVariables, st))) {
return false;
}
}
if (secondIteration) {

View File

@@ -82,8 +82,10 @@ public class FoldExpressionIntoStreamInspection extends AbstractBaseJavaLocalIns
PsiBinaryExpression binOp = tryCast(PsiUtil.skipParenthesizedExprDown(operands[0]), PsiBinaryExpression.class);
if (binOp != null) {
if (ComparisonUtils.isComparison(binOp) &&
(left == binOp.getLOperand() && ExpressionUtils.isSafelyRecomputableExpression(binOp.getROperand())) ||
(left == binOp.getROperand() && ExpressionUtils.isSafelyRecomputableExpression(binOp.getLOperand()))) {
(left == PsiUtil.skipParenthesizedExprDown(binOp.getLOperand()) &&
ExpressionUtils.isSafelyRecomputableExpression(binOp.getROperand())) ||
(left == PsiUtil.skipParenthesizedExprDown(binOp.getROperand()) &&
ExpressionUtils.isSafelyRecomputableExpression(binOp.getLOperand()))) {
// Disable for simple comparison chains like "a == null && b == null && c == null":
// using Stream API here looks an overkill
return Collections.emptyList();

View File

@@ -0,0 +1,10 @@
// "Collapse into loop" "true"
class X {
public int hashCode() {
int result = super.hashCode();
for (int i : new int[]{field != null ? field.hashCode() : 0, field2 != null ? field2.hashCode() : 0, field3 != null ? field3.hashCode() : 0, field4 != null ? field4.hashCode() : 0, field5 != null ? field5.hashCode() : 0}) {
result = 31 * result + (i);
}
return result;
}
}

View File

@@ -0,0 +1,12 @@
// "Collapse into loop" "true"
class X {
public int hashCode() {
int result = super.hashCode();
<selection>result = 31 * result + (field != null ? field.hashCode() : 0);
result = 31 * result + (field2 != null ? field2.hashCode() : 0);
result = 31 * result + (field3 != null ? field3.hashCode() : 0);
result = 31 * result + (field4 != null ? field4.hashCode() : 0);
result = 31 * result + (field5 != null ? field5.hashCode() : 0);</selection>
return result;
}
}

View File

@@ -0,0 +1,10 @@
// "Collapse into loop" "false"
class X {
public int hashCode() {
int result = 1;
int other = 3;
<selection>result = 2 + other;
result = 2 + result;</selection>
return result;
}
}

View File

@@ -984,10 +984,10 @@ public class EquivalenceChecker {
protected Match binaryExpressionsMatch(@NotNull PsiBinaryExpression binaryExpression1, @NotNull PsiBinaryExpression binaryExpression2) {
final IElementType tokenType1 = binaryExpression1.getOperationTokenType();
final IElementType tokenType2 = binaryExpression2.getOperationTokenType();
final PsiExpression left1 = binaryExpression1.getLOperand();
final PsiExpression left2 = binaryExpression2.getLOperand();
final PsiExpression right1 = binaryExpression1.getROperand();
final PsiExpression right2 = binaryExpression2.getROperand();
final PsiExpression left1 = PsiUtil.skipParenthesizedExprDown(binaryExpression1.getLOperand());
final PsiExpression left2 = PsiUtil.skipParenthesizedExprDown(binaryExpression2.getLOperand());
final PsiExpression right1 = PsiUtil.skipParenthesizedExprDown(binaryExpression1.getROperand());
final PsiExpression right2 = PsiUtil.skipParenthesizedExprDown(binaryExpression2.getROperand());
if (right1 == null || right2 == null) {
return Match.exact(right1 == right2);
}
@@ -1075,6 +1075,9 @@ public class EquivalenceChecker {
else if (equivalence1 == EXACT_MISMATCH) {
return new Match(left1, right1);
}
else {
return equivalence1;
}
}
else if (equivalence2 == EXACT_MISMATCH) {
if (equivalence1 == EXACT_MISMATCH) {
@@ -1084,7 +1087,7 @@ public class EquivalenceChecker {
return new Match(left2, right2);
}
}
return EXACT_MISMATCH;
return equivalence1 == EXACT_MATCH ? equivalence2 : EXACT_MISMATCH;
}
private static boolean modifierListsAreEquivalent(PsiModifierList modifierList1, PsiModifierList modifierList2) {