[java-inspections] SimplifyBooleanExpressionFix: try to move pattern variable to another instanceof when possible

Fixes IDEA-336123 "Constant values" fix produces red code when pattern variable is removed

GitOrigin-RevId: 7faf8dd77569c21d69a9a7226d6db1b0097f9231
This commit is contained in:
Tagir Valeev
2023-11-15 21:43:24 +00:00
committed by intellij-monorepo-bot
parent c49a062434
commit ff15d064cd
6 changed files with 93 additions and 17 deletions
@@ -9,6 +9,7 @@ import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil;
import com.intellij.codeInspection.dataFlow.MethodContract;
import com.intellij.psi.*;
import com.intellij.psi.controlFlow.*;
import com.intellij.psi.impl.source.DummyHolder;
import com.intellij.psi.impl.source.tree.JavaSharedImplUtil;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.JavaPsiPatternUtil;
@@ -191,24 +192,24 @@ public final class InstanceOfUtils {
*/
@Nullable
public static PsiInstanceOfExpression findCorrespondingInstanceOf(@NotNull PsiTypeCastExpression cast) {
PsiElement context = PsiUtil.skipParenthesizedExprUp(cast.getParent());
PsiElement context = PsiUtil.skipParenthesizedExprUp(cast.getContext());
if (context instanceof PsiLocalVariable) {
context = context.getParent();
context = context.getContext();
} else {
while (true) {
if (context instanceof PsiPolyadicExpression) {
IElementType tokenType = ((PsiPolyadicExpression)context).getOperationTokenType();
if (context instanceof PsiPolyadicExpression polyadic) {
IElementType tokenType = polyadic.getOperationTokenType();
if (tokenType.equals(JavaTokenType.ANDAND) || tokenType.equals(JavaTokenType.OROR)) {
PsiInstanceOfExpression instanceOf = findInstanceOf((PsiExpression)context, cast, tokenType.equals(JavaTokenType.ANDAND));
PsiInstanceOfExpression instanceOf = findInstanceOf(polyadic, cast, tokenType.equals(JavaTokenType.ANDAND));
if (instanceOf != null) {
return instanceOf;
}
}
}
if (context instanceof PsiConditionalExpression) {
PsiExpression condition = ((PsiConditionalExpression)context).getCondition();
if (context instanceof PsiConditionalExpression conditional) {
PsiExpression condition = conditional.getCondition();
if (!PsiTreeUtil.isAncestor(condition, cast, true)) {
boolean whenTrue = PsiTreeUtil.isAncestor(((PsiConditionalExpression)context).getThenExpression(), cast, false);
boolean whenTrue = PsiTreeUtil.isAncestor(conditional.getThenExpression(), cast, false);
PsiInstanceOfExpression instanceOf = findInstanceOf(condition, cast, whenTrue);
if (instanceOf != null) {
return instanceOf;
@@ -216,15 +217,17 @@ public final class InstanceOfUtils {
}
}
if ((context instanceof PsiExpression && !(context instanceof PsiLambdaExpression)) ||
context instanceof PsiExpressionList || context instanceof PsiLocalVariable) {
context = context.getParent();
context instanceof PsiExpressionList || context instanceof PsiLocalVariable ||
context instanceof DummyHolder) {
context = context.getContext();
continue;
}
break;
}
if (!(context instanceof PsiStatement)) return null;
}
PsiElement parent = context.getParent();
if (context == null) return null;
PsiElement parent = context.getContext();
if (parent instanceof PsiCodeBlock) {
for (PsiElement stmt = context.getPrevSibling(); stmt != null; stmt = stmt.getPrevSibling()) {
if (stmt instanceof PsiIfStatement ifStatement) {
@@ -242,7 +245,7 @@ public final class InstanceOfUtils {
if (stmt instanceof PsiWhileStatement || stmt instanceof PsiDoWhileStatement || stmt instanceof PsiForStatement) {
PsiConditionalLoopStatement loop = (PsiConditionalLoopStatement)stmt;
if (PsiTreeUtil.processElements(
loop, e -> !(e instanceof PsiBreakStatement) || ((PsiBreakStatement)e).findExitedStatement() != loop)) {
loop, e -> !(e instanceof PsiBreakStatement breakStatement) || breakStatement.findExitedStatement() != loop)) {
PsiInstanceOfExpression instanceOf = findInstanceOf(loop.getCondition(), cast, false);
if (instanceOf != null) {
return instanceOf;
@@ -251,9 +254,9 @@ public final class InstanceOfUtils {
}
if (stmt instanceof PsiSwitchLabelStatementBase) break;
}
if (parent.getParent() instanceof PsiBlockStatement) {
context = parent.getParent();
parent = context.getParent();
if (parent.getContext() instanceof PsiBlockStatement) {
context = parent.getContext();
parent = context.getContext();
}
}
return processParent(cast, context, parent);
@@ -315,7 +318,7 @@ public final class InstanceOfUtils {
if (tokenType == JavaTokenType.ANDAND && whenTrue ||
tokenType == JavaTokenType.OROR && !whenTrue) {
for (PsiExpression operand : polyadic.getOperands()) {
if (PsiTreeUtil.isAncestor(operand, cast, false)) return null;
if (PsiTreeUtil.isContextAncestor(operand, cast, false)) return null;
PsiInstanceOfExpression result = findInstanceOf(operand, cast, whenTrue);
if (result != null) {
return result;
@@ -66,7 +66,8 @@ public class SimplifyBooleanExpressionFix extends PsiUpdateModCommandAction<PsiE
return e instanceof PsiInstanceOfExpression &&
!ContainerUtil.exists(JavaPsiPatternUtil.getExposedPatternVariables(((PsiInstanceOfExpression)e)),
var -> PatternResolveState.fromBoolean(mySubExpressionValue)
.equals(PatternResolveState.stateAtParent(var, subExpression)));
.equals(PatternResolveState.stateAtParent(var, subExpression)) &&
newTargetForPatternVariable(subExpression, var) == null);
}
private boolean canExtractSideEffect(PsiExpression subExpression) {
@@ -127,6 +128,20 @@ public class SimplifyBooleanExpressionFix extends PsiUpdateModCommandAction<PsiE
return true;
}
private static @Nullable PsiInstanceOfExpression newTargetForPatternVariable(@NotNull PsiExpression expression,
@NotNull PsiPatternVariable variable) {
if (!(variable.getPattern() instanceof PsiTypeTestPattern typeTest)) return null;
PsiTypeElement checkType = typeTest.getCheckType();
if (checkType == null) return null;
if (!(typeTest.getParent() instanceof PsiInstanceOfExpression instanceOf)) return null;
PsiTypeCastExpression cast =
(PsiTypeCastExpression)JavaPsiFacade.getElementFactory(expression.getProject()).createExpressionFromText("(a)b", expression);
Objects.requireNonNull(cast.getCastType()).replace(checkType);
Objects.requireNonNull(cast.getOperand()).replace(instanceOf.getOperand());
return InstanceOfUtils.findPatternCandidate(cast);
}
private static boolean containsBreakOrContinue(PsiDoWhileStatement doWhileLoop) {
return SyntaxTraverser.psiTraverser(doWhileLoop).filter(e -> isBreakOrContinue(e, doWhileLoop)).iterator().hasNext();
}
@@ -157,6 +172,7 @@ public class SimplifyBooleanExpressionFix extends PsiUpdateModCommandAction<PsiE
public void invoke(@NotNull PsiExpression subExpression) {
CommentTracker ct = new CommentTracker();
processPatternVariables(subExpression);
if (SideEffectChecker.mayHaveSideEffects(subExpression) && canExtractSideEffect(subExpression)) {
PsiExpression orig = subExpression;
subExpression = ensureCodeBlock(subExpression.getProject(), subExpression);
@@ -189,6 +205,27 @@ public class SimplifyBooleanExpressionFix extends PsiUpdateModCommandAction<PsiE
simplifyExpression(expression);
}
private static void processPatternVariables(@NotNull PsiExpression subExpression) {
List<PsiPatternVariable> variables = JavaPsiPatternUtil.getExposedPatternVariables(subExpression);
for (PsiPatternVariable variable : variables) {
if (VariableAccessUtils.variableIsUsed(variable, variable.getDeclarationScope())) {
PsiInstanceOfExpression target = newTargetForPatternVariable(subExpression, variable);
if (target != null) {
PsiInstanceOfExpression updated = (PsiInstanceOfExpression)JavaPsiFacade.getElementFactory(subExpression.getProject())
.createExpressionFromText("x instanceof T t", target);
updated.getOperand().replace(target.getOperand());
PsiTypeTestPattern newPattern = (PsiTypeTestPattern)Objects.requireNonNull(updated.getPattern());
PsiTypeElement checkType = target.getCheckType();
if (checkType == null) continue;
Objects.requireNonNull(newPattern.getCheckType()).replace(checkType);
Objects.requireNonNull(newPattern.getPatternVariable()).replace(variable);
target.replace(updated);
variable.delete();
}
}
}
}
private PsiExpression ensureCodeBlock(@NotNull Project project, PsiExpression subExpression) {
if (!mySubExpressionValue) {
// Prevent extracting while condition to internal 'if'
@@ -0,0 +1,8 @@
// "Simplify 'obj instanceof String str' to true" "true"
class Test {
void test(Object obj) {
if (!(obj instanceof String str) || str.isEmpty()) {
}
}
}
@@ -0,0 +1,10 @@
// "Simplify 'obj instanceof String str' to true" "true"
class Test {
void test(Object obj) {
if (obj instanceof String str) {
if (str.isEmpty()) {
}
}
}
}
@@ -0,0 +1,8 @@
// "Simplify 'obj instanceof String str' to true" "true"
class Test {
void test(Object obj) {
if (!(obj instanceof String) || obj<caret> instanceof String str && str.isEmpty()) {
}
}
}
@@ -0,0 +1,10 @@
// "Simplify 'obj instanceof String str' to true" "true"
class Test {
void test(Object obj) {
if (obj instanceof String) {
if (obj instanceof<caret> String str && str.isEmpty()) {
}
}
}
}