From 55584525ead55a0cc44c7fed32f88fb2c04f5787 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 24 Oct 2022 18:19:40 +0200 Subject: [PATCH] [java-inspections] IDEA-304462 Nested total pattern is reported as always true, despite component might be null GitOrigin-RevId: 0dcd2be4710c6d7f6ed87b7b05ae7ddf2fca14f7 --- .../analysis/SwitchBlockHighlightingModel.java | 4 ++-- .../dataFlow/java/ControlFlowAnalyzer.java | 3 +-- .../impl/quickfix/UnwrapSwitchLabelFix.java | 4 ++-- .../intellij/psi/util/JavaPsiPatternUtil.java | 16 ++++++++-------- .../SwitchExhaustivenessIn19Java.java | 8 ++++---- ...nditionalDestructuringAndDefaultIn19Java.java | 8 ++++---- .../afterAllVariablesUnused1.java | 4 +++- .../afterAllVariablesUnused2.java | 4 +++- .../deleteSwitchLabel19/afterDeepNesting1.java | 4 +++- .../deleteSwitchLabel19/afterDeepNesting2.java | 4 +++- .../afterRecordComponentCast.java | 2 +- .../beforeAllVariablesUnused1.java | 4 +++- .../beforeAllVariablesUnused2.java | 4 +++- .../deleteSwitchLabel19/beforeDeepNesting1.java | 4 +++- .../deleteSwitchLabel19/beforeDeepNesting2.java | 4 +++- .../afterInnerDeconstruction.java | 4 +++- .../beforeInnerDeconstruction.java | 4 +++- .../dataFlow/fixture/NestedRecordPatterns.java | 15 +++++++++++++++ .../codeInspection/DataFlowInspection19Test.java | 3 +++ 19 files changed, 70 insertions(+), 33 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/NestedRecordPatterns.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/SwitchBlockHighlightingModel.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/SwitchBlockHighlightingModel.java index 3d2691227e10..e9e4e18a4aa7 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/SwitchBlockHighlightingModel.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/SwitchBlockHighlightingModel.java @@ -417,7 +417,7 @@ public class SwitchBlockHighlightingModel { PsiType permittedType = JavaPsiFacade.getElementFactory(psiClass.getProject()).createType(psiClass, substitutor); if (pattern == null && (PsiUtil.getLanguageLevel(permittedClass).isLessThan(LanguageLevel.JDK_18_PREVIEW) || TypeConversionUtil.areTypesConvertible(selectorType, permittedType)) || - pattern != null && !JavaPsiPatternUtil.isTotalForType(pattern, TypeUtils.getType(permittedClass), false)) { + pattern != null && !JavaPsiPatternUtil.isTotalForType(pattern, TypeUtils.getType(permittedClass), true)) { nonVisited.add(permittedClass); } } @@ -1002,7 +1002,7 @@ public class SwitchBlockHighlightingModel { if (exhaustiveGroups.isEmpty()) return false; List patterns = ContainerUtil.map(exhaustiveGroups, it -> it.getValue().iterator().next().get(0)); - if (ContainerUtil.exists(patterns, pattern -> JavaPsiPatternUtil.isTotalForType(pattern, typeToCheck, false))) { + if (ContainerUtil.exists(patterns, pattern -> JavaPsiPatternUtil.isTotalForType(pattern, typeToCheck, true))) { return true; } LinkedHashMap patternClasses = SwitchBlockHighlightingModel.findPatternClasses(patterns); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java index 1ad686e2a80f..b0404a5eabd4 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java @@ -1779,8 +1779,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { PsiTypeElement checkType = InstanceOfUtils.findCheckTypeElement(expression); CFGBuilder builder = new CFGBuilder(this); DfaVariableValue expressionValue; - PsiPatternVariable patternVariable = pattern == null ? null : JavaPsiPatternUtil.getPatternVariable(pattern); - if (patternVariable == null) { + if (pattern == null) { if (checkType == null) { pushUnknown(); } diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/UnwrapSwitchLabelFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/UnwrapSwitchLabelFix.java index 6eb550b821e1..d2bd3ca74902 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/UnwrapSwitchLabelFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/UnwrapSwitchLabelFix.java @@ -223,7 +223,7 @@ public class UnwrapSwitchLabelFix implements LocalQuickFix { String newVarName = generator.byType(JavaPsiPatternUtil.getPatternType(label)).generate(true); declarationStatementText += newVarName + "="; } - if (!JavaPsiPatternUtil.isTotalForType(pattern, selectorType)) { + if (!type.isAssignableFrom(selectorType)) { declarationStatementText += "(" + type.getPresentableText() + ")"; } declarationStatementText += selector.getText() + ";"; @@ -273,7 +273,7 @@ public class UnwrapSwitchLabelFix implements LocalQuickFix { String newVarName = generator.byType(type).generate(true); declarationStatementText += newVarName + "="; } - if (!JavaPsiPatternUtil.isTotalForType(deconstructionComponent, components[i].getType())) { + if (!type.isAssignableFrom(components[i].getType())) { declarationStatementText += "(" + type.getPresentableText() + ")"; } declarationStatementText += variable.getName() + "." + components[i].getName() + "();"; 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 1cc759307ee7..5664752ec7d4 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 @@ -193,26 +193,26 @@ public final class JavaPsiPatternUtil { @Contract(value = "null, _ -> false", pure = true) public static boolean isTotalForType(@Nullable PsiCaseLabelElement pattern, @NotNull PsiType type) { - return isTotalForType(pattern, type, true); + return isTotalForType(pattern, type, false); } - public static boolean isTotalForType(@Nullable PsiCaseLabelElement pattern, @NotNull PsiType type, boolean checkComponents) { + public static boolean isTotalForType(@Nullable PsiCaseLabelElement pattern, @NotNull PsiType type, boolean forDomination) { if (pattern == null) return false; if (pattern instanceof PsiPatternGuard) { PsiPatternGuard guarded = (PsiPatternGuard)pattern; Object constVal = evaluateConstant(guarded.getGuardingExpression()); - return isTotalForType(guarded.getPattern(), type, checkComponents) && Boolean.TRUE.equals(constVal); + return isTotalForType(guarded.getPattern(), type, forDomination) && Boolean.TRUE.equals(constVal); } if (pattern instanceof PsiGuardedPattern) { PsiGuardedPattern guarded = (PsiGuardedPattern)pattern; Object constVal = evaluateConstant(guarded.getGuardingExpression()); - return isTotalForType(guarded.getPrimaryPattern(), type, checkComponents) && Boolean.TRUE.equals(constVal); + return isTotalForType(guarded.getPrimaryPattern(), type, forDomination) && Boolean.TRUE.equals(constVal); } else if (pattern instanceof PsiParenthesizedPattern) { - return isTotalForType(((PsiParenthesizedPattern)pattern).getPattern(), type, checkComponents); + return isTotalForType(((PsiParenthesizedPattern)pattern).getPattern(), type, forDomination); } else if (pattern instanceof PsiDeconstructionPattern) { - return dominates(getPatternType(pattern), type) && (!checkComponents || hasTotalComponents((PsiDeconstructionPattern)pattern)); + return forDomination && dominates(getPatternType(pattern), type); } else if (pattern instanceof PsiTypeTestPattern) { return dominates(getPatternType(pattern), type); @@ -234,7 +234,7 @@ public final class JavaPsiPatternUtil { for (int i = 0; i < patternComponents.length; i++) { PsiPattern patternComponent = patternComponents[i]; PsiType componentType = recordComponents[i].getType(); - if (!isTotalForType(patternComponent, componentType, true)) { + if (!isTotalForType(patternComponent, componentType)) { return false; } } @@ -261,7 +261,7 @@ public final class JavaPsiPatternUtil { public static boolean dominates(@Nullable PsiCaseLabelElement who, @NotNull PsiCaseLabelElement overWhom) { if (who == null) return false; PsiType overWhomType = getPatternType(overWhom); - if (overWhomType == null || !isTotalForType(who, overWhomType, false)) { + if (overWhomType == null || !isTotalForType(who, overWhomType, true)) { return false; } PsiDeconstructionPattern whoDeconstruction = findDeconstructionPattern(who); diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/SwitchExhaustivenessIn19Java.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/SwitchExhaustivenessIn19Java.java index dbd1183024e1..280dc26dfdab 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/SwitchExhaustivenessIn19Java.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/SwitchExhaustivenessIn19Java.java @@ -30,15 +30,15 @@ public class Basic { Generic genericC; void testGenerics(T p, Pair pair1, Pair pair2) { - switch(p) { - case SuperChild sc -> {} - case SuperRecord sr -> {} - } switch (p) { case SuperChild superChild -> {} case SuperRecord(C i) -> {} case SuperRecord(D i) -> {} } + switch(p) { + case SuperChild sc -> {} + case SuperRecord sr -> {} + } switch (pair1.x()) { case SuperChild sc -> {} case SuperRecord sr -> {} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/UnconditionalDestructuringAndDefaultIn19Java.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/UnconditionalDestructuringAndDefaultIn19Java.java index a722feda7092..87e255bfe82e 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/UnconditionalDestructuringAndDefaultIn19Java.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/UnconditionalDestructuringAndDefaultIn19Java.java @@ -10,12 +10,12 @@ public class Totality { void test(){ switch (recordInterface){ - case RecordInterface(I x, I y) -> {} - case default -> {} + case RecordInterface(I x, I y) -> {} + case default -> {} } switch (recordInterface){ - case RecordInterface(I x, I y) r when true-> {} - case default -> {} + case RecordInterface(I x, I y) r when true-> {} + case default -> {} } switch (recordInterface){ case RecordInterface(I x, I y) -> {} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterAllVariablesUnused1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterAllVariablesUnused1.java index f926d4258912..f6eba0cc1a6b 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterAllVariablesUnused1.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterAllVariablesUnused1.java @@ -1,4 +1,6 @@ // "Remove unreachable branches" "true" +import org.jetbrains.annotations.*; + class Test { void test(Object obj) { if (!(obj instanceof Rect)) return; @@ -6,5 +8,5 @@ class Test { } record Point(double x, double y) {} - record Rect(Point point1, Point point2) {} + record Rect(@NotNull Point point1, @NotNull Point point2) {} } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterAllVariablesUnused2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterAllVariablesUnused2.java index f926d4258912..f6eba0cc1a6b 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterAllVariablesUnused2.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterAllVariablesUnused2.java @@ -1,4 +1,6 @@ // "Remove unreachable branches" "true" +import org.jetbrains.annotations.*; + class Test { void test(Object obj) { if (!(obj instanceof Rect)) return; @@ -6,5 +8,5 @@ class Test { } record Point(double x, double y) {} - record Rect(Point point1, Point point2) {} + record Rect(@NotNull Point point1, @NotNull Point point2) {} } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterDeepNesting1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterDeepNesting1.java index 4854a220fd78..68a21eb01de9 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterDeepNesting1.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterDeepNesting1.java @@ -1,4 +1,6 @@ // "Remove unreachable branches" "true" +import org.jetbrains.annotations.*; + class Test { void foo(Object obj) { switch (obj) { @@ -11,4 +13,4 @@ class Test { } } -record X(X x) { } \ No newline at end of file +record X(@NotNull X x) { } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterDeepNesting2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterDeepNesting2.java index 43e010cc8782..875f0c18af3c 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterDeepNesting2.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterDeepNesting2.java @@ -1,4 +1,6 @@ // "Remove unreachable branches" "true" +import org.jetbrains.annotations.*; + class Test { void foo(Object obj) { switch (obj) { @@ -13,4 +15,4 @@ class Test { } } -record X(X x) { } \ No newline at end of file +record X(@NotNull X x) { } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterRecordComponentCast.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterRecordComponentCast.java index c93cf3bba851..82ee1decdde2 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterRecordComponentCast.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/afterRecordComponentCast.java @@ -6,7 +6,7 @@ class Test { default -> { return; } } - ((A) ((Rec) rec).i()).doA(); + ((A) rec.i()).doA(); } } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeAllVariablesUnused1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeAllVariablesUnused1.java index 2b94c66b759c..03b255764ab6 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeAllVariablesUnused1.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeAllVariablesUnused1.java @@ -1,4 +1,6 @@ // "Remove unreachable branches" "true" +import org.jetbrains.annotations.*; + class Test { void test(Object obj) { if (!(obj instanceof Rect)) return; @@ -12,5 +14,5 @@ class Test { } record Point(double x, double y) {} - record Rect(Point point1, Point point2) {} + record Rect(@NotNull Point point1, @NotNull Point point2) {} } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeAllVariablesUnused2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeAllVariablesUnused2.java index 12d90bdaffd7..df184e189eba 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeAllVariablesUnused2.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeAllVariablesUnused2.java @@ -1,4 +1,6 @@ // "Remove unreachable branches" "true" +import org.jetbrains.annotations.*; + class Test { void test(Object obj) { if (!(obj instanceof Rect)) return; @@ -12,5 +14,5 @@ class Test { } record Point(double x, double y) {} - record Rect(Point point1, Point point2) {} + record Rect(@NotNull Point point1, @NotNull Point point2) {} } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeDeepNesting1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeDeepNesting1.java index ea6b49be6014..7e026e61f16d 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeDeepNesting1.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeDeepNesting1.java @@ -1,4 +1,6 @@ // "Remove unreachable branches" "true" +import org.jetbrains.annotations.*; + class Test { void foo(Object obj) { switch (obj) { @@ -13,4 +15,4 @@ class Test { } } -record X(X x) { } \ No newline at end of file +record X(@NotNull X x) { } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeDeepNesting2.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeDeepNesting2.java index f60ed29828da..d254b58de352 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeDeepNesting2.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/deleteSwitchLabel19/beforeDeepNesting2.java @@ -1,4 +1,6 @@ // "Remove unreachable branches" "true" +import org.jetbrains.annotations.*; + class Test { void foo(Object obj) { switch (obj) { @@ -17,4 +19,4 @@ class Test { } } -record X(X x) { } \ No newline at end of file +record X(@NotNull X x) { } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterInnerDeconstruction.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterInnerDeconstruction.java index f017fa2f3961..044283fbee34 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterInnerDeconstruction.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterInnerDeconstruction.java @@ -1,4 +1,6 @@ // "Unwrap 'if' statement extracting side effects" "true-preview" +import org.jetbrains.annotations.NotNull; + class Test { void foo(Object obj) { if (!(obj instanceof Rect)) { @@ -25,5 +27,5 @@ record Point(double x, double y) { record Size(double w, double h) { } -record Rect(Point pos, Size size) { +record Rect(@NotNull Point pos, @NotNull Size size) { } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeInnerDeconstruction.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeInnerDeconstruction.java index 555f12c92ad8..a20db779e581 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeInnerDeconstruction.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeInnerDeconstruction.java @@ -1,4 +1,6 @@ // "Unwrap 'if' statement extracting side effects" "true-preview" +import org.jetbrains.annotations.NotNull; + class Test { void foo(Object obj) { if (!(obj instanceof Rect)) { @@ -20,5 +22,5 @@ record Point(double x, double y) { record Size(double w, double h) { } -record Rect(Point pos, Size size) { +record Rect(@NotNull Point pos, @NotNull Size size) { } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/NestedRecordPatterns.java b/java/java-tests/testData/inspection/dataFlow/fixture/NestedRecordPatterns.java new file mode 100644 index 000000000000..21cf01f2ff13 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/NestedRecordPatterns.java @@ -0,0 +1,15 @@ +class Hello { + record X(X x) {} + + static void foo(Object obj) { + if (!(obj instanceof X)) { + return; + } + + System.out.println(obj instanceof X(X(X(X(X x))))); + } + + public static void main(String[] args) { + foo(new X(new X(null))); + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection19Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection19Test.java index 91f745f8a93e..3707b78416f8 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection19Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection19Test.java @@ -32,4 +32,7 @@ public class DataFlowInspection19Test extends DataFlowInspectionTestCase { public void testRecordPatternAndWhen() { doTest(); } + public void testNestedRecordPatterns() { + doTest(); + } } \ No newline at end of file