diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index 7c44df9fdafe..4542cb4de314 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -318,7 +318,9 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec if (instruction instanceof InstanceofInstruction) { InstanceofInstruction instanceOf = (InstanceofInstruction)instruction; if (visitor.isInstanceofRedundant(instanceOf)) { - reporter.registerProblem(instanceOf.getExpression(), + PsiExpression expression = instanceOf.getExpression(); + if (expression != null && !JavaPsiPatternUtil.getExposedPatternVariables(expression).isEmpty()) continue; + reporter.registerProblem(expression, InspectionsBundle.message("dataflow.message.redundant.instanceof"), new RedundantInstanceofFix()); } diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java index 936702a8c43e..526a8b8e1e41 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java @@ -17,7 +17,9 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.controlFlow.AnalysisCanceledException; import com.intellij.psi.controlFlow.ControlFlowUtil; +import com.intellij.psi.scope.PatternResolveState; import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.JavaPsiPatternUtil; import com.intellij.psi.util.PsiExpressionTrimRenderer; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; @@ -56,12 +58,20 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement { return getFamilyName(); } String suffix = ""; - if (SideEffectChecker.mayHaveSideEffects(subExpression)) { + if (SideEffectChecker.mayHaveSideEffects(subExpression, e -> shouldIgnore(e, subExpression))) { suffix = canExtractSideEffect(subExpression) ? " extracting side effects" : " (may change semantics)"; } return getIntentionText(subExpression, mySubExpressionValue) + suffix; } + private boolean shouldIgnore(PsiElement e, PsiExpression subExpression) { + return e instanceof PsiInstanceOfExpression && + JavaPsiPatternUtil.getExposedPatternVariables(((PsiInstanceOfExpression)e)) + .stream() + .map(var -> PatternResolveState.stateAtParent(var, subExpression)) + .noneMatch(PatternResolveState.fromBoolean(mySubExpressionValue)::equals); + } + private boolean canExtractSideEffect(PsiExpression subExpression) { if (ControlFlowUtils.canExtractStatement(subExpression)) return true; if (!mySubExpressionValue) { @@ -146,7 +156,9 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement { LOG.error("anchor is null", new Attachment("subExpression.txt", subExpression.getText())); return; } - List sideEffects = SideEffectChecker.extractSideEffectExpressions(subExpression); + PsiExpression finalSubExpression = subExpression; + List sideEffects = SideEffectChecker.extractSideEffectExpressions( + subExpression, e -> shouldIgnore(e, finalSubExpression)); sideEffects.forEach(ct::markUnchanged); PsiStatement[] statements = StatementExtractor.generateStatements(sideEffects, subExpression); if (statements.length > 0) { diff --git a/java/java-psi-impl/src/com/intellij/psi/scope/PatternResolveState.java b/java/java-psi-impl/src/com/intellij/psi/scope/PatternResolveState.java index 1758d2807e52..6229ba944811 100644 --- a/java/java-psi-impl/src/com/intellij/psi/scope/PatternResolveState.java +++ b/java/java-psi-impl/src/com/intellij/psi/scope/PatternResolveState.java @@ -48,7 +48,7 @@ public enum PatternResolveState { state = state.invert(); continue; } - throw new IllegalArgumentException("Variable is not available at parent"); + return WHEN_NONE; } return state; } diff --git a/java/java-psi-impl/src/com/intellij/psi/util/JavaPsiPatternUtil.java b/java/java-psi-impl/src/com/intellij/psi/util/JavaPsiPatternUtil.java index 9839cd565482..ab44eab1411b 100644 --- a/java/java-psi-impl/src/com/intellij/psi/util/JavaPsiPatternUtil.java +++ b/java/java-psi-impl/src/com/intellij/psi/util/JavaPsiPatternUtil.java @@ -52,11 +52,15 @@ public class JavaPsiPatternUtil { */ public static @Nullable String getEffectiveInitializerText(@NotNull PsiPatternVariable variable) { PsiPattern pattern = variable.getPattern(); - if (pattern == null) return null; PsiInstanceOfExpression instanceOf = ObjectUtils.tryCast(pattern.getParent(), PsiInstanceOfExpression.class); if (instanceOf == null) return null; if (pattern instanceof PsiTypeTestPattern) { - return "(" + ((PsiTypeTestPattern)pattern).getCheckType().getText() + ")" + instanceOf.getOperand().getText(); + PsiExpression operand = instanceOf.getOperand(); + PsiTypeElement checkType = ((PsiTypeTestPattern)pattern).getCheckType(); + if (checkType.getType().equals(operand.getType())) { + return operand.getText(); + } + return "(" + checkType.getText() + ")" + operand.getText(); } return null; } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterPatternVarNotUsedAfterwards.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterPatternVarNotUsedAfterwards.java new file mode 100644 index 000000000000..e581d043faff --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterPatternVarNotUsedAfterwards.java @@ -0,0 +1,6 @@ +// "Remove 'if' statement" "true" +class X { + void test() { + Object obj = "Hello from pattern matching"; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterPatternVarUsedAfterwards.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterPatternVarUsedAfterwards.java new file mode 100644 index 000000000000..2e6315269d03 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterPatternVarUsedAfterwards.java @@ -0,0 +1,8 @@ +// "Remove 'if' statement extracting side effects" "true" +class X { + void test() { + Object obj = "Hello from pattern matching"; + String s = (String) obj; + System.out.println(s); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterPatternVarUseless.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterPatternVarUseless.java new file mode 100644 index 000000000000..118a3bfb498a --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterPatternVarUseless.java @@ -0,0 +1,11 @@ +// "Remove 'if' statement extracting side effects" "true" +class X { + void test() { + Object obj = "Hello from pattern matching"; + String s = getString(); + System.out.println(s); + System.out.println(s); + } + + native @org.jetbrains.annotations.NotNull String getString(); +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforePatternVarNotUsedAfterwards.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforePatternVarNotUsedAfterwards.java new file mode 100644 index 000000000000..157753352eec --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforePatternVarNotUsedAfterwards.java @@ -0,0 +1,9 @@ +// "Remove 'if' statement" "true" +class X { + void test() { + Object obj = "Hello from pattern matching"; + if (obj instanceof Integer i) { + System.out.println(i); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforePatternVarUsedAfterwards.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforePatternVarUsedAfterwards.java new file mode 100644 index 000000000000..2cc6573936d0 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforePatternVarUsedAfterwards.java @@ -0,0 +1,8 @@ +// "Remove 'if' statement extracting side effects" "true" +class X { + void test() { + Object obj = "Hello from pattern matching"; + if (!(obj instanceof String s)) return; + System.out.println(s); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforePatternVarUseless.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforePatternVarUseless.java new file mode 100644 index 000000000000..7f251a56846a --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforePatternVarUseless.java @@ -0,0 +1,11 @@ +// "Remove 'if' statement extracting side effects" "true" +class X { + void test() { + Object obj = "Hello from pattern matching"; + if (!(getString() instanceof String s)) return; + System.out.println(s); + System.out.println(s); + } + + native @org.jetbrains.annotations.NotNull String getString(); +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/InstanceOfPattern.java b/java/java-tests/testData/inspection/dataFlow/fixture/InstanceOfPattern.java index 520b103e891a..eefe59256ad6 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/InstanceOfPattern.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/InstanceOfPattern.java @@ -14,6 +14,12 @@ public class InstanceOfPattern { } } + void testNullCheck(String s) { + if (s instanceof String s1) { + System.out.println(s1); + } + } + interface Foo { @Nullable Object bar(); } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java index f89c41ff59f9..96230d09f8d8 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/SideEffectChecker.java @@ -19,10 +19,8 @@ import com.intellij.codeInspection.dataFlow.ContractValue; import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.*; import com.intellij.psi.util.InheritanceUtil; -import com.intellij.psi.util.PropertyUtil; -import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.psi.util.PsiUtil; import com.intellij.util.SmartList; import com.intellij.util.containers.ContainerUtil; import one.util.streamex.StreamEx; @@ -125,8 +123,13 @@ public class SideEffectChecker { } public static List extractSideEffectExpressions(@NotNull PsiExpression element) { + return extractSideEffectExpressions(element, e -> false); + } + + public static List extractSideEffectExpressions(@NotNull PsiExpression element, + @NotNull Predicate ignoreElement) { List list = new SmartList<>(); - element.accept(new SideEffectsVisitor(list, element)); + element.accept(new SideEffectsVisitor(list, element, ignoreElement)); return StreamEx.of(list).select(PsiExpression.class).toList(); } @@ -196,8 +199,19 @@ public class SideEffectChecker { super.visitUnaryExpression(expression); } + @Override + public void visitInstanceOfExpression(PsiInstanceOfExpression expression) { + List variables = JavaPsiPatternUtil.getExposedPatternVariables(expression); + if (!variables.isEmpty() && + !PsiTreeUtil.isAncestor(myStartElement, variables.get(0).getDeclarationScope(), false)) { + if (addSideEffect(expression)) return; + } + super.visitInstanceOfExpression(expression); + } + @Override public void visitVariable(PsiVariable variable) { + if (variable instanceof PsiPatternVariable) return; if (addSideEffect(variable)) return; super.visitVariable(variable); } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/StatementExtractor.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/StatementExtractor.java index fc98e95810a8..f074fc6d15ca 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/StatementExtractor.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/StatementExtractor.java @@ -7,6 +7,7 @@ import com.intellij.openapi.diagnostic.RuntimeExceptionWithAttachments; import com.intellij.openapi.util.Key; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.JavaPsiPatternUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.ObjectUtils; @@ -207,6 +208,18 @@ public class StatementExtractor { } public String toString() { + if (myAnchor instanceof PsiInstanceOfExpression) { + List variables = JavaPsiPatternUtil.getExposedPatternVariables(myAnchor); + StringBuilder sb = new StringBuilder(); + for (PsiPatternVariable variable : variables) { + String initializer = JavaPsiPatternUtil.getEffectiveInitializerText(variable); + if (initializer != null) { + sb.append(variable.getTypeElement().getText()).append(" ").append(variable.getName()).append("=") + .append(initializer).append(";"); + } + } + return sb.toString(); + } return myAnchor.getText() + ";"; } }