From ee954bc1694aca024dd68adfbdd907a302194d5d Mon Sep 17 00:00:00 2001 From: Nikita Eshkeev Date: Tue, 31 Aug 2021 20:39:58 +0300 Subject: [PATCH] [java switch resolve] IDEA-277110 Compilation error not highlighted when using Pattern Matching for switch Fallthrough to default is not acceptable according to JEP 406. This patch eliminates the default cases from a special case rules and leaves only `case null` as the only special case rule. GitOrigin-RevId: 06c865f92fed01a41c5c87e1aa0a852acb3e7ee0 --- .../java/PsiSwitchLabelStatementImpl.java | 16 +++----- .../PsiSwitchLabeledRuleStatementImpl.java | 31 ++++++++++++++ .../BreakAndOtherStopWords.java | 2 +- .../FallthroughDefault.java | 38 ++++++++++++++++++ .../PatternMatchingInSwitch.java | 6 +-- ...java => CaseNullAfterPatternMatching.java} | 3 -- .../CaseNullAfterPatternMatchingExpr.java | 8 ++++ .../DefaultAfterPatternMatching.java | 11 +++++ .../DefaultAfterPatternMatchingExpr.java | 8 ++++ ...ightPatternsForSwitchHighlightingTest.java | 4 ++ .../navigation/GotoDeclarationTest.java | 40 +++++++++++++------ 11 files changed, 137 insertions(+), 30 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/FallthroughDefault.java rename java/java-tests/testData/codeInsight/gotoDeclaration/{CaseNullDefaultAfterPatternMatching.java => CaseNullAfterPatternMatching.java} (79%) create mode 100644 java/java-tests/testData/codeInsight/gotoDeclaration/CaseNullAfterPatternMatchingExpr.java create mode 100644 java/java-tests/testData/codeInsight/gotoDeclaration/DefaultAfterPatternMatching.java create mode 100644 java/java-tests/testData/codeInsight/gotoDeclaration/DefaultAfterPatternMatchingExpr.java diff --git a/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiSwitchLabelStatementImpl.java b/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiSwitchLabelStatementImpl.java index daa53ce2286c..258c41531f13 100644 --- a/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiSwitchLabelStatementImpl.java +++ b/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiSwitchLabelStatementImpl.java @@ -107,7 +107,7 @@ public class PsiSwitchLabelStatementImpl extends PsiSwitchLabelStatementBaseImpl immediateSwitchLabel = PsiTreeUtil.getPrevSiblingOfType(currentScope, PsiSwitchLabelStatementBase.class); } - while (immediateSwitchLabel != null && isFallthrough(immediateSwitchLabel) && isSpecialCaseLabel(immediateSwitchLabel)) { + while (immediateSwitchLabel != null && isFallthrough(immediateSwitchLabel) && isCaseNull(immediateSwitchLabel)) { immediateSwitchLabel = PsiTreeUtil.getPrevSiblingOfType(immediateSwitchLabel, PsiSwitchLabelStatementBase.class); } @@ -136,18 +136,12 @@ public class PsiSwitchLabelStatementImpl extends PsiSwitchLabelStatementBaseImpl } } - private static boolean isSpecialCaseLabel(@NotNull PsiSwitchLabelStatementBase switchCaseLabel) { - return isCase(switchCaseLabel, JavaTokenType.NULL_KEYWORD) || - isCase(switchCaseLabel, JavaTokenType.DEFAULT_KEYWORD) || - switchCaseLabel.isDefaultCase(); - } + private static boolean isCaseNull(@NotNull PsiSwitchLabelStatementBase switchCaseLabel) { + if (switchCaseLabel.getCaseLabelElementList() == null) return false; - private static boolean isCase(@NotNull PsiSwitchLabelStatementBase item, @NotNull IElementType keyword) { - if (item.getCaseLabelElementList() == null) return false; - - final PsiCaseLabelElement[] elements = item.getCaseLabelElementList().getElements(); + final PsiCaseLabelElement[] elements = switchCaseLabel.getCaseLabelElementList().getElements(); if (elements.length != 1) return false; - return elements[0].getNode().getFirstChildNode().getElementType() == keyword; + return elements[0].getNode().getFirstChildNode().getElementType() == JavaTokenType.NULL_KEYWORD; } } \ No newline at end of file diff --git a/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiSwitchLabeledRuleStatementImpl.java b/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiSwitchLabeledRuleStatementImpl.java index 583f1c79a543..02de643ab50f 100644 --- a/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiSwitchLabeledRuleStatementImpl.java +++ b/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/PsiSwitchLabeledRuleStatementImpl.java @@ -5,7 +5,9 @@ import com.intellij.psi.*; import com.intellij.psi.impl.source.tree.JavaElementType; import com.intellij.psi.scope.PsiScopeProcessor; import com.intellij.psi.tree.TokenSet; +import com.intellij.psi.util.PsiUtil; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; public class PsiSwitchLabeledRuleStatementImpl extends PsiSwitchLabelStatementBaseImpl implements PsiSwitchLabeledRuleStatement { private static final TokenSet BODY_STATEMENTS = @@ -47,8 +49,37 @@ public class PsiSwitchLabeledRuleStatementImpl extends PsiSwitchLabelStatementBa // Do not resolve references that come from the list of elements in this case rule if (lastParent instanceof PsiCaseLabelElementList) return true; + if (!shouldProcess()) return true; + final PsiCaseLabelElementList patternsInCaseLabel = getCaseLabelElementList(); if (patternsInCaseLabel == null) return true; return patternsInCaseLabel.processDeclarations(processor, state, null, place); } + + private boolean shouldProcess() { + final PsiCaseLabelElementList elementList = getCaseLabelElementList(); + if (elementList == null) return false; + + final PsiCaseLabelElement[] elements = elementList.getElements(); + if (elements.length == 1) return true; + else if (elements.length > 2 || elements.length == 0) return false; + + final PsiElement first = stripParensIfNecessary(elements[0]); + final PsiElement second = stripParensIfNecessary(elements[1]); + + if (first == null || second == null) return true; + + return firstPatternVariableSecondNull(first, second) || firstPatternVariableSecondNull(second, first); + } + + private static boolean firstPatternVariableSecondNull(PsiElement first, @NotNull PsiElement second) { + return first instanceof PsiTypeTestPattern && + second.getNode().getFirstChildNode().getElementType() == JavaTokenType.NULL_KEYWORD; + } + + private static @Nullable PsiElement stripParensIfNecessary(@NotNull PsiCaseLabelElement element) { + return element instanceof PsiExpression + ? PsiUtil.skipParenthesizedExprDown((PsiExpression)element) + : element; + } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/BreakAndOtherStopWords.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/BreakAndOtherStopWords.java index f9787d5823be..f9485e45d65d 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/BreakAndOtherStopWords.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/BreakAndOtherStopWords.java @@ -4,9 +4,9 @@ class Main { switch (o) { case Integer i : System.out.println(); - case default: case null: System.out.println(i); + case default: }; } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/FallthroughDefault.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/FallthroughDefault.java new file mode 100644 index 000000000000..16a1feecf120 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/FallthroughDefault.java @@ -0,0 +1,38 @@ + +class Main { + void f(Object o) { + switch (o) { + case Integer i : + System.out.println(i); + break; + case String s: + default: + System.out.println(s); + } + } + + void g(Object o) { + switch (o) { + case Integer i : + System.out.println(i); + break; + case String s: + case default: + System.out.println(s); + } + } + + void ff(Object o) { + switch (o) { + case Integer i -> System.out.println(i); + case String s, default -> System.out.println(s); + } + } + + void gg(Object o) { + switch (o) { + case Integer i -> System.out.println(i); + case default, String s -> System.out.println(s); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/PatternMatchingInSwitch.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/PatternMatchingInSwitch.java index 73e3c95609aa..04f95c06b552 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/PatternMatchingInSwitch.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/PatternMatchingInSwitch.java @@ -21,17 +21,17 @@ class Main { }; int i4 = switch(o) { - case String s, X x -> x.f(); + case String s, X x -> x.f(); default -> 1; }; int i5 = switch(o) { - case String s, X x -> x.f(); + case String s, X x -> x.f(); default -> 1; }; int i6 = switch(o) { - case X x, String s -> x.f(); + case X x, String s -> x.f(); default -> 1; }; return i1 + i2 + i3 + i4 + i5 + i6; diff --git a/java/java-tests/testData/codeInsight/gotoDeclaration/CaseNullDefaultAfterPatternMatching.java b/java/java-tests/testData/codeInsight/gotoDeclaration/CaseNullAfterPatternMatching.java similarity index 79% rename from java/java-tests/testData/codeInsight/gotoDeclaration/CaseNullDefaultAfterPatternMatching.java rename to java/java-tests/testData/codeInsight/gotoDeclaration/CaseNullAfterPatternMatching.java index e496afcd184b..d9799565368d 100644 --- a/java/java-tests/testData/codeInsight/gotoDeclaration/CaseNullDefaultAfterPatternMatching.java +++ b/java/java-tests/testData/codeInsight/gotoDeclaration/CaseNullAfterPatternMatching.java @@ -4,9 +4,6 @@ class Main { switch (o) { case Integer i : System.out.println(); - case null: - default: {} - case default: case null: System.out.println(i); }; diff --git a/java/java-tests/testData/codeInsight/gotoDeclaration/CaseNullAfterPatternMatchingExpr.java b/java/java-tests/testData/codeInsight/gotoDeclaration/CaseNullAfterPatternMatchingExpr.java new file mode 100644 index 000000000000..dee4ea98f68e --- /dev/null +++ b/java/java-tests/testData/codeInsight/gotoDeclaration/CaseNullAfterPatternMatchingExpr.java @@ -0,0 +1,8 @@ +class Main { + private static final int i = 0; + private void f(Object o) { + switch (o) { + case Integer i, null -> System.out.println(i); + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/gotoDeclaration/DefaultAfterPatternMatching.java b/java/java-tests/testData/codeInsight/gotoDeclaration/DefaultAfterPatternMatching.java new file mode 100644 index 000000000000..764beb6b62a4 --- /dev/null +++ b/java/java-tests/testData/codeInsight/gotoDeclaration/DefaultAfterPatternMatching.java @@ -0,0 +1,11 @@ +class Main { + private static final int i = 0; + private void f(Object o) { + switch (o) { + case Integer i : + System.out.println(); + default: + System.out.println(i); + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/gotoDeclaration/DefaultAfterPatternMatchingExpr.java b/java/java-tests/testData/codeInsight/gotoDeclaration/DefaultAfterPatternMatchingExpr.java new file mode 100644 index 000000000000..53b208baae22 --- /dev/null +++ b/java/java-tests/testData/codeInsight/gotoDeclaration/DefaultAfterPatternMatchingExpr.java @@ -0,0 +1,8 @@ +class Main { + private static final int i = 0; + private void f(Object o) { + switch (o) { + case Integer i, default -> System.out.println(i); + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightPatternsForSwitchHighlightingTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightPatternsForSwitchHighlightingTest.java index 335d0d562c70..90ac09960952 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightPatternsForSwitchHighlightingTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightPatternsForSwitchHighlightingTest.java @@ -99,6 +99,10 @@ public class LightPatternsForSwitchHighlightingTest extends LightJavaCodeInsight doTest(); } + public void testFallthroughDefault() { + doTest(); + } + public void testUnusedPatternVariable() { myFixture.enableInspections(new UnusedDeclarationInspection()); doTest(); diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/GotoDeclarationTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/GotoDeclarationTest.java index 26de21c2d002..9aae4456bb95 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/GotoDeclarationTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/GotoDeclarationTest.java @@ -121,38 +121,47 @@ public class GotoDeclarationTest extends LightJavaCodeInsightTestCase { } public void testPatternMatchingGuardInSwitchExpression() { - doTestPatternMatchingGuard(); + doTestGoToField(); } public void testPatternMatchingGuardInSwitchStatement() { - doTestPatternMatchingGuard(); + doTestGoToField(); } public void testPatternMatchingWithParensAroundReference() { - doTestPatternMatchingGuard(); + doTestGoToField(); } public void testReferenceFieldInPatternMatchingInSwitchStatement() { - doTestPatternMatchingGuard(); + doTestGoToField(); } - public void testCaseNullDefaultAfterPatternMatching() { - configure(); - final PsiElement patternVariable = PsiTreeUtil.findChildOfType(getFile(), PsiPatternVariable.class); - final PsiElement element = GotoDeclarationAction.findTargetElement(getProject(), getEditor(), getEditor().getCaretModel().getOffset()); - assertThat(element).isEqualTo(patternVariable); + public void testCaseNullAfterPatternMatching() { + doTestGoToPatternVariable(); + } + + public void testCaseNullAfterPatternMatchingExpr() { + doTestGoToPatternVariable(); + } + + public void testDefaultAfterPatternMatching() { + doTestGoToField(); + } + + public void testDefaultAfterPatternMatchingExpr() { + doTestGoToField(); } public void testGuardWithInstanceOfPatternMatchingInIf() { - doTestGoToPatternVariable(); + doTestGoToSecondPatternVariable(); } public void testGuardWithInstanceOfPatternMatchingInSwitch() { - doTestGoToPatternVariable(); + doTestGoToSecondPatternVariable(); } - private void doTestPatternMatchingGuard() { + private void doTestGoToField() { configure(); final PsiField field = PsiTreeUtil.findChildOfType(getFile(), PsiField.class); final PsiElement element = GotoDeclarationAction.findTargetElement(getProject(), getEditor(), getEditor().getCaretModel().getOffset()); @@ -160,6 +169,13 @@ public class GotoDeclarationTest extends LightJavaCodeInsightTestCase { } private void doTestGoToPatternVariable() { + configure(); + final PsiPatternVariable patternVariable = PsiTreeUtil.findChildOfType(getFile(), PsiPatternVariable.class); + final PsiElement element = GotoDeclarationAction.findTargetElement(getProject(), getEditor(), getEditor().getCaretModel().getOffset()); + assertThat(element).isEqualTo(patternVariable); + } + + private void doTestGoToSecondPatternVariable() { configure(); final Iterator iterator = PsiTreeUtil.findChildrenOfType(getFile(), PsiPatternVariable.class).iterator(); iterator.next();