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

SimplifyBooleanExpressionFix: support always-false last disjunct in if

Browse Source 
Fixes IDEA-200961 Simplification of "!list.add(type) is always false" may change code semantic
This commit is contained in:
Tagir Valeev
2018-10-23 17:44:26 +07:00
parent 16bda0dc48
commit 2e36108630
5 changed files with 186 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java
+56 -11
View File
@@ -5,6 +5,7 @@ package com.intellij.codeInsight.daemon.impl.quickfix;
import com.intellij.codeInsight.BlockUtils;
import com.intellij.codeInsight.daemon.QuickFixBundle;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
import com.intellij.codeInsight.intention.impl.SplitConditionUtil;
import com.intellij.codeInspection.CommonQuickFixBundle;
import com.intellij.codeInspection.LocalQuickFixOnPsiElement;
import com.intellij.openapi.diagnostic.Attachment;
@@ -20,6 +21,7 @@ import com.intellij.psi.util.PsiExpressionTrimRenderer;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.refactoring.util.RefactoringUtil;
import com.intellij.util.ArrayUtil;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
@@ -64,6 +66,15 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement {
if (!mySubExpressionValue) {
PsiElement parent = PsiUtil.skipParenthesizedExprUp(subExpression.getParent());
if (parent instanceof PsiWhileStatement || parent instanceof PsiForStatement) return true;
// code like "if (foo || alwaysFalseWithSideEffects) {}"
if (parent instanceof PsiPolyadicExpression) {
PsiPolyadicExpression polyadic = (PsiPolyadicExpression)parent;
if (polyadic.getOperationTokenType().equals(JavaTokenType.OROR)
&& PsiTreeUtil.isAncestor(ArrayUtil.getLastElement(polyadic.getOperands()), subExpression, false)
&& PsiUtil.skipParenthesizedExprUp(parent.getParent()) instanceof PsiIfStatement) {
return true;
}
}
}
}
return false;
@@ -125,17 +136,7 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement {
if (subExpression == null) return;
CommentTracker ct = new CommentTracker();
if (shouldExtractSideEffect()) {
if (!mySubExpressionValue) {
// Prevent extracting while condition to internal 'if'
PsiWhileStatement whileStatement = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprUp(subExpression.getParent()), PsiWhileStatement.class);
if (whileStatement != null && whileStatement.getCondition() != null) {
PsiStatement replacement = JavaPsiFacade.getElementFactory(project)
.createStatementFromText("if(" + whileStatement.getCondition().getText() + ");", whileStatement);
PsiIfStatement ifStatement = (PsiIfStatement)whileStatement.replace(replacement);
subExpression = Objects.requireNonNull(ifStatement.getCondition());
}
}
subExpression = RefactoringUtil.ensureCodeBlock(subExpression);
subExpression = ensureCodeBlock(project, subExpression);
if (subExpression == null) {
LOG.error("ensureCodeBlock returned null", new Attachment("subExpression.txt", getSubExpression().getText()));
return;
@@ -163,6 +164,50 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement {
simplifyExpression(expression);
}
public PsiExpression ensureCodeBlock(@NotNull Project project, PsiExpression subExpression) {
if (!mySubExpressionValue) {
// Prevent extracting while condition to internal 'if'
PsiElement parent = PsiUtil.skipParenthesizedExprUp(subExpression.getParent());
PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
if (parent instanceof PsiWhileStatement) {
PsiWhileStatement whileStatement = (PsiWhileStatement)parent;
if (whileStatement.getCondition() != null) {
PsiStatement replacement =
factory.createStatementFromText("if(" + whileStatement.getCondition().getText() + ");", whileStatement);
PsiIfStatement ifStatement = (PsiIfStatement)whileStatement.replace(replacement);
subExpression = Objects.requireNonNull(ifStatement.getCondition());
}
}
else if (parent instanceof PsiPolyadicExpression) {
PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)parent;
if (JavaTokenType.OROR.equals(polyadicExpression.getOperationTokenType())) {
PsiExpression expression = expandLastIfDisjunct(polyadicExpression, subExpression, factory);
if (expression != null) {
return expression;
}
}
}
}
return RefactoringUtil.ensureCodeBlock(subExpression);
}
@Nullable
private static PsiExpression expandLastIfDisjunct(PsiPolyadicExpression orChain,
PsiExpression subExpression,
PsiElementFactory factory) {
PsiIfStatement ifStatement = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprUp(orChain.getParent()), PsiIfStatement.class);
if (ifStatement == null) return null;
PsiExpression lastOperand = ArrayUtil.getLastElement(orChain.getOperands());
if (!PsiTreeUtil.isAncestor(lastOperand, subExpression, false)) return null;
orChain.replace(SplitConditionUtil.getLOperands(orChain, orChain.getTokenBeforeOperand(lastOperand)));
ControlFlowUtils.ensureElseBranch(ifStatement);
PsiBlockStatement elseBranch = (PsiBlockStatement)Objects.requireNonNull(ifStatement.getElseBranch());
PsiCodeBlock codeBlock = elseBranch.getCodeBlock();
PsiStatement replacement = factory.createStatementFromText("if(" + subExpression.getText() + ");", ifStatement);
PsiIfStatement alwaysFalseIf = (PsiIfStatement)codeBlock.addAfter(replacement, codeBlock.getLBrace());
return Objects.requireNonNull(alwaysFalseIf.getCondition());
}
private static boolean simplifyIfOrLoopStatement(final PsiExpression expression) throws IncorrectOperationException {
boolean condition = Boolean.parseBoolean(expression.getText());
if (!(expression instanceof PsiLiteralExpression) || !PsiType.BOOLEAN.equals(expression.getType())) return false;
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterIfLastDisjunct.java
+54
View File
@@ -0,0 +1,54 @@
// "Fix all 'Constant conditions & exceptions' problems in file" "true"
import java.util.ArrayList;
import java.util.List;
class Mutant {
List<String> types = new ArrayList<>();
void consider(String type, boolean unmodifiable) {
if (unmodifiable) {
if(Math.random() > 0.5) {
System.out.println("1");
}
} else {
types.add(type);
System.out.println("2");
}
if (unmodifiable) {
if(Math.random() > 0.5) {
System.out.println("1");
}
} else {
types.add(type);
}
if (unmodifiable) {
if(Math.random() > 0.5) {
System.out.println("1");
}
} else {
types.add(type);
}
if (unmodifiable)
System.out.println("1");
else {
types.add(type);
System.out.println("2");
}
// Cannot extract side effect in the middle
if (unmodifiable || Math.random() > 0.5)
System.out.println("1");
else
System.out.println("2");
types.add(type);
if (unmodifiable)
System.out.println("1");
else
System.out.println("2");
System.out.println("types = " + types);
}
public static void main(String[] args) {
new Mutant().consider("A", false);
}
}
java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeIfLastDisjunct.java
+45
View File
@@ -0,0 +1,45 @@
// "Fix all 'Constant conditions & exceptions' problems in file" "true"
import java.util.ArrayList;
import java.util.List;
class Mutant {
List<String> types = new ArrayList<>();
void consider(String type, boolean unmodifiable) {
if (unmodifiable || !types.<caret>add(type)) {
if(Math.random() > 0.5) {
System.out.println("1");
}
} else {
System.out.println("2");
}
if (unmodifiable || !types.add(type)) {
if(Math.random() > 0.5) {
System.out.println("1");
}
}
if (unmodifiable || !types.add(type))
if(Math.random() > 0.5) {
System.out.println("1");
}
if (unmodifiable || !types.add(type))
System.out.println("1");
else
System.out.println("2");
// Cannot extract side effect in the middle
if (unmodifiable || !types.add(type) || Math.random() > 0.5)
System.out.println("1");
else
System.out.println("2");
if (!types.add(type) || unmodifiable)
System.out.println("1");
else
System.out.println("2");
System.out.println("types = " + types);
}
public static void main(String[] args) {
new Mutant().consider("A", false);
}
}
java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/UnwrapIfStatementFixTest.java
+8
View File
@@ -19,6 +19,8 @@ package com.intellij.java.codeInsight.daemon.quickFix;
import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase;
import com.intellij.codeInspection.LocalInspectionTool;
import com.intellij.codeInspection.dataFlow.DataFlowInspection;
import com.intellij.testFramework.LightProjectDescriptor;
import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase;
import org.jetbrains.annotations.NotNull;
public class UnwrapIfStatementFixTest extends LightQuickFixParameterizedTestCase {
@@ -28,6 +30,12 @@ public class UnwrapIfStatementFixTest extends LightQuickFixParameterizedTestCase
return new LocalInspectionTool[]{new DataFlowInspection()};
}
@NotNull
@Override
protected LightProjectDescriptor getProjectDescriptor() {
return LightCodeInsightFixtureTestCase.JAVA_10_ANNOTATED;
}
@Override
protected String getBasePath() {
return "/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement";
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java
+23
View File
@@ -15,6 +15,7 @@
*/
package com.siyeh.ig.psiutils;
import com.intellij.codeInsight.BlockUtils;
import com.intellij.psi.*;
import com.intellij.psi.controlFlow.*;
import com.intellij.psi.search.searches.ReferencesSearch;
@@ -1069,6 +1070,28 @@ public class ControlFlowUtils {
return false;
}
/**
* Ensures that the {@code if} statement has the {@code else} branch which is a block statement (adding it if absent)
* @param ifStatement an {@code if} statement to add an else branch or expand it to the block
*/
public static void ensureElseBranch(PsiIfStatement ifStatement) {
PsiStatement elseBranch = ifStatement.getElseBranch();
if (elseBranch != null) {
if (!(elseBranch instanceof PsiBlockStatement)) {
BlockUtils.expandSingleStatementToBlockStatement(elseBranch);
}
} else {
PsiStatement thenBranch = ifStatement.getThenBranch();
PsiBlockStatement emptyBlock = BlockUtils.createBlockStatement(ifStatement.getProject());
if (thenBranch == null) {
ifStatement.setThenBranch(emptyBlock);
} else if (!(thenBranch instanceof PsiBlockStatement)) {
BlockUtils.expandSingleStatementToBlockStatement(thenBranch);
}
ifStatement.setElseBranch(emptyBlock);
}
}
public enum InitializerUsageStatus {
// Variable is declared just before the wanted place
DECLARED_JUST_BEFORE,