[java-inspection] IDEA-281947 Useless pattern guard not suggested for removal

- remove guard if it is always true

GitOrigin-RevId: 168a103e93a42c9a82657abffcbd7626047cab3d
This commit is contained in:
Mikhail Pyltsin
2023-12-14 13:42:42 +01:00
committed by intellij-monorepo-bot
parent d32a572c78
commit 43ad0b4707
11 changed files with 121 additions and 32 deletions

View File

@@ -18,7 +18,6 @@ import com.intellij.codeInspection.dataFlow.types.DfTypes;
import com.intellij.codeInspection.options.OptPane;
import com.intellij.java.analysis.JavaAnalysisBundle;
import com.intellij.modcommand.ModCommandAction;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.util.Ref;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.*;
@@ -87,6 +86,25 @@ public final class ConstantValueInspection extends AbstractBaseJavaLocalInspecti
}
}
@Override
public void visitSwitchLabeledRuleStatement(@NotNull PsiSwitchLabeledRuleStatement statement) {
checkSwitchCaseGuard(PsiUtil.skipParenthesizedExprDown(statement.getGuardExpression()));
}
@Override
public void visitSwitchLabelStatement(@NotNull PsiSwitchLabelStatement statement) {
checkSwitchCaseGuard(PsiUtil.skipParenthesizedExprDown(statement.getGuardExpression()));
}
private void checkSwitchCaseGuard(@Nullable PsiExpression guard) {
if (guard == null) return;
if (BoolUtils.isTrue(guard)) {
LocalQuickFix fix = createSimplifyBooleanExpressionFix(guard, guard.textMatches(PsiKeyword.TRUE));
holder.registerProblem(guard, JavaAnalysisBundle
.message("dataflow.message.constant.no.ref", guard.textMatches(PsiKeyword.TRUE) ? 1 : 0), LocalQuickFix.notNullElements(fix));
}
}
@Override
public void visitIfStatement(@NotNull PsiIfStatement statement) {
PsiExpression condition = PsiUtil.skipParenthesizedExprDown(statement.getCondition());

View File

@@ -42,6 +42,8 @@ import org.jetbrains.annotations.Nullable;
import java.util.*;
import static com.intellij.psi.JavaTokenType.WHEN_KEYWORD;
public class SimplifyBooleanExpressionFix extends PsiUpdateModCommandAction<PsiExpression> {
private static final Logger LOG = Logger.getInstance(SimplifyBooleanExpressionFix.class);
@@ -87,6 +89,10 @@ public class SimplifyBooleanExpressionFix extends PsiUpdateModCommandAction<PsiE
@NotNull
public static @IntentionName String getIntentionText(@NotNull PsiExpression expression, boolean constantValue) {
PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent());
if (constantValue && parent instanceof PsiSwitchLabelStatementBase switchLabel &&
PsiTreeUtil.isAncestor(switchLabel.getGuardExpression(), expression, false)) {
return CommonQuickFixBundle.message("fix.remove.guard");
}
if (parent instanceof PsiIfStatement) {
return constantValue ?
CommonQuickFixBundle.message("fix.unwrap.statement", PsiKeyword.IF) :
@@ -350,7 +356,8 @@ public class SimplifyBooleanExpressionFix extends PsiUpdateModCommandAction<PsiE
}
}
private static void replaceWithStatements(@NotNull PsiStatement orig, @Nullable PsiStatement statement) throws IncorrectOperationException {
private static void replaceWithStatements(@NotNull PsiStatement orig, @Nullable PsiStatement statement)
throws IncorrectOperationException {
if (statement == null) {
orig.delete();
return;
@@ -439,8 +446,21 @@ public class SimplifyBooleanExpressionFix extends PsiUpdateModCommandAction<PsiE
parent.delete();
return;
}
if (parent instanceof PsiSwitchLabelStatementBase label && Boolean.FALSE.equals(value)) {
DeleteSwitchLabelFix.deleteLabel(label);
if (parent instanceof PsiSwitchLabelStatementBase label && PsiTreeUtil.isAncestor(label.getGuardExpression(), newExpression, false)) {
if (Boolean.TRUE.equals(value)) {
CommentTracker tracker = new CommentTracker();
PsiExpression guardExpression = label.getGuardExpression();
PsiKeyword psiKeyword = PsiTreeUtil.getPrevSiblingOfType(guardExpression, PsiKeyword.class);
if (psiKeyword != null && psiKeyword.getTokenType() == WHEN_KEYWORD) {
tracker.delete(psiKeyword);
}
tracker.delete(guardExpression);
return;
}
if (Boolean.FALSE.equals(value)) {
DeleteSwitchLabelFix.deleteLabel(label);
return;
}
}
}
if (!simplifyIfOrLoopStatement(newExpression)) {
@@ -574,18 +594,21 @@ public class SimplifyBooleanExpressionFix extends PsiUpdateModCommandAction<PsiE
}
if (expressions.isEmpty()) {
resultExpression = negate ? trueExpression : falseExpression;
} else {
}
else {
String simplifiedText = StringUtil.join(expressions, PsiElement::getText, " ^ ");
if (negate) {
if (expressions.size() > 1) {
simplifiedText = "!(" + simplifiedText + ")";
} else {
}
else {
simplifiedText = BoolUtils.getNegatedExpressionText(expressions.get(0));
}
}
resultExpression = JavaPsiFacade.getElementFactory(expression.getProject()).createExpressionFromText(simplifiedText, expression);
}
} else {
}
else {
for (int i = 1; i < operands.length; i++) {
Boolean l = getConstBoolean(lExpr);
PsiExpression operand = operands[i];
@@ -600,7 +623,8 @@ public class SimplifyBooleanExpressionFix extends PsiUpdateModCommandAction<PsiE
final PsiJavaToken javaToken = expression.getTokenBeforeOperand(operand);
if (javaToken != null && !PsiTreeUtil.hasErrorElements(operand) && !PsiTreeUtil.hasErrorElements(lExpr)) {
try {
resultExpression = JavaPsiFacade.getElementFactory(expression.getProject()).createExpressionFromText(lExpr.getText() + javaToken.getText() + operand.getText(), expression);
resultExpression = JavaPsiFacade.getElementFactory(expression.getProject())
.createExpressionFromText(lExpr.getText() + javaToken.getText() + operand.getText(), expression);
}
catch (IncorrectOperationException e) {
resultExpression = null;

View File

@@ -0,0 +1,9 @@
// "Remove guard expression" "true-preview"
class X {
private static void test(Object o) {
switch (o) {
case String t -> System.out.println(1);
case Object o1 -> System.out.println(1);
}
}
}

View File

@@ -0,0 +1,14 @@
// "Remove guard expression" "true-preview"
class X {
private static void test2(Object o) {
switch (o) {
case String t:
System.out.println(1);
break;
case Object o1:
System.out.println(1);
break;
}
}
}

View File

@@ -0,0 +1,9 @@
// "Remove guard expression" "true-preview"
class X {
private static void test(Object o) {
switch (o) {
case String t when 1==<caret>1 -> System.out.println(1);
case Object o1 -> System.out.println(1);
}
}
}

View File

@@ -0,0 +1,14 @@
// "Remove guard expression" "true-preview"
class X {
private static void test2(Object o) {
switch (o) {
case String t when tr<caret>ue:
System.out.println(1);
break;
case Object o1:
System.out.println(1);
break;
}
}
}

View File

@@ -6,7 +6,7 @@ class Test {
switch (<warning descr="Unboxing of 'i' may produce 'NullPointerException'">i</warning>) {
case 1:
break;
case Integer ii when true:
case Integer ii when <warning descr="Condition is always true">true</warning>:
break;
}
}
@@ -16,7 +16,7 @@ class Test {
switch (<warning descr="Unboxing of 'i' may produce 'NullPointerException'">i</warning>) {
case 1:
break;
case Integer ii when true:
case Integer ii when <warning descr="Condition is always true">true</warning>:
break;
}
}
@@ -26,7 +26,7 @@ class Test {
switch (i) {
case <warning descr="Switch label '1' is the only reachable in the whole switch">1</warning>:
break;
case Integer ii when true:
case Integer ii when <warning descr="Condition is always true">true</warning>:
break;
}
}
@@ -35,7 +35,7 @@ class Test {
switch (i) {
case 1:
break;
case Integer ii when true:
case Integer ii when <warning descr="Condition is always true">true</warning>:
break;
}
}
@@ -45,7 +45,7 @@ class Test {
switch (<warning descr="Unboxing of 'i' may produce 'NullPointerException'">i</warning>) {
case 1:
break;
case Integer ii when true:
case Integer ii when <warning descr="Condition is always true">true</warning>:
break;
}
}
@@ -64,7 +64,7 @@ class Test {
switch (i) {
case 1:
break;
case Integer ii when true:
case Integer ii when <warning descr="Condition is always true">true</warning>:
break;
}
}
@@ -76,7 +76,7 @@ class Test {
break;
case Integer ii when Math.random() > 0.5:
break;
case Integer ii when true:
case Integer ii when <warning descr="Condition is always true">true</warning>:
break;
}
}
@@ -115,7 +115,7 @@ class Test {
switch (createValue()) {
case 1:
break;
case Integer ii when true:
case Integer ii when <warning descr="Condition is always true">true</warning>:
break;
}
}
@@ -124,7 +124,7 @@ class Test {
switch (createNotNullValue()) {
case 1, 2:
break;
case Object o when true:
case Object o when <warning descr="Condition is always true">true</warning>:
break;
}
}
@@ -134,7 +134,7 @@ class Test {
int nullableWithUnconditionalPatternLabelExpr(@Nullable Integer i) {
return switch (<warning descr="Unboxing of 'i' may produce 'NullPointerException'">i</warning>) {
case 1 -> 1;
case Integer ii when true -> 2;
case Integer ii when <warning descr="Condition is always true">true</warning> -> 2;
};
}
@@ -142,7 +142,7 @@ class Test {
i = null;
return switch (<warning descr="Unboxing of 'i' may produce 'NullPointerException'">i</warning>) {
case 1 -> 1;
case Integer ii when true -> 2;
case Integer ii when <warning descr="Condition is always true">true</warning> -> 2;
};
}
@@ -150,14 +150,14 @@ class Test {
i = 1;
return switch (i) {
case <warning descr="Switch label '1' is the only reachable in the whole switch">1</warning> -> 1;
case Integer ii when true -> 2;
case Integer ii when <warning descr="Condition is always true">true</warning> -> 2;
};
}
int unknownWithUnconditionalPatternLabelExpr(Integer i) {
return switch (i) {
case 1 -> 1;
case Integer ii when true -> 2;
case Integer ii when <warning descr="Condition is always true">true</warning> -> 2;
};
}
@@ -165,7 +165,7 @@ class Test {
i = null;
return switch (<warning descr="Unboxing of 'i' may produce 'NullPointerException'">i</warning>) {
case 1 -> 1;
case Integer ii when true -> 2;
case Integer ii when <warning descr="Condition is always true">true</warning> -> 2;
};
}
@@ -180,7 +180,7 @@ class Test {
int notNullWithUnconditionalPatternLabelExpr(@NotNull Integer i) {
return switch (i) {
case 1 -> 1;
case Integer ii when true -> 2;
case Integer ii when <warning descr="Condition is always true">true</warning> -> 2;
};
}
@@ -189,7 +189,7 @@ class Test {
return switch (<warning descr="Unboxing of 'i' may produce 'NullPointerException'">i</warning>) {
case 1 -> 1;
case Integer ii when Math.random() > 0.5 -> 2;
case Integer ii when true -> 3;
case Integer ii when <warning descr="Condition is always true">true</warning> -> 3;
};
}
@@ -219,14 +219,14 @@ class Test {
int unknownCallWithUnconditionalPatternLabelExpr() {
return switch (createValue()) {
case 1 -> 1;
case Integer ii when true -> 2;
case Integer ii when <warning descr="Condition is always true">true</warning> -> 2;
};
}
int notNullCallWithUnconditionalPatternLabelExpr() {
return switch (createNotNullValue()) {
case 1, 2 -> 1;
case Object o when true -> 2;
case Object o when <warning descr="Condition is always true">true</warning> -> 2;
};
}

View File

@@ -18,14 +18,14 @@ public class DuplicateLabels {
break;
case <error descr="Duplicate unconditional pattern">Object oo</error>:
break;
case <error descr="Duplicate unconditional pattern">Object oo</error> when true:
case <error descr="Duplicate unconditional pattern">Object oo</error> when <warning descr="Condition is always true">true</warning>:
break;
}
}
void testDominatedPatterns(Object o) {
switch (o) {
case String ss when true:
case String ss when <warning descr="Condition is always true">true</warning>:
break;
case <error descr="Label is dominated by a preceding case label 'String ss'">String ss</error>:
break;

View File

@@ -1,7 +1,7 @@
public class Test {
void testDominatedPatterns(Object o) {
switch (o) {
case String ss when true:
case String ss when <warning descr="Condition is always true">true</warning>:
break;
case <error descr="Label is dominated by a preceding case label 'String ss'">String ss</error>:
break;
@@ -12,12 +12,12 @@ public class Test {
int testDominatedConstLabel(Integer i, E e) {
switch (e) {
case <warning descr="Switch label 'E d' is the only reachable in the whole switch">E d</warning> when true: return 1;
case <warning descr="Switch label 'E d' is the only reachable in the whole switch">E d</warning> when <warning descr="Condition is always true">true</warning>: return 1;
case <error descr="Label is dominated by a preceding case label 'E d'">A</error>: return -1;
}
return switch (i) {
case Integer ii when true -> 1;
case Integer ii when <warning descr="Condition is always true">true</warning> -> 1;
case <error descr="Label is dominated by a preceding case label 'Integer ii'">2</error> -> 2;
default -> 3;
};

View File

@@ -1,7 +1,7 @@
public class Test {
void testDominatedPatterns(Object obj) {
switch (obj) {
case Number i when true -> System.out.println("A number");
case Number i when <warning descr="Condition is always true">true</warning> -> System.out.println("A number");
case <error descr="Label is dominated by a preceding case label 'Number i'">Integer i</error> -> System.out.println("An integer");
default -> {}
}

View File

@@ -12,6 +12,7 @@ fix.remove.title=Remove {0}
fix.remove.title.x=Remove {0} ''{1}''
fix.remove.redundant=Remove redundant ''{0}''
fix.remove.statement=Remove ''{0}'' statement
fix.remove.guard=Remove guard expression
fix.replace.with.x=Replace with ''{0}''
fix.replace.with.x.call=Replace with ''{0}'' call