From b3ba7b6783f90398dff1affd3b3ff339175bd98f Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 29 Apr 2020 17:33:19 +0700 Subject: [PATCH] IDEA-239116 Switch expression without any results is not highlighted as erroneous GitOrigin-RevId: 66580d3aa4a95e1fdb32b1a3131544560c9c432a --- .../daemon/impl/analysis/HighlightUtil.java | 61 ++++++++++++++++--- .../src/messages/JavaErrorBundle.properties | 1 + .../SwitchExpressionHasResult.java | 4 +- .../SwitchExpressionsNoResult.java | 18 ++++++ .../JavaSwitchExpressionsHighlightingTest.kt | 1 + 5 files changed, 74 insertions(+), 11 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/SwitchExpressionsNoResult.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java index 042216c2a01c..1a0e5d5b2087 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java @@ -2108,31 +2108,74 @@ public class HighlightUtil { PsiCodeBlock switchBody = switchExpression.getBody(); if (switchBody != null) { PsiStatement lastStatement = PsiTreeUtil.getPrevSiblingOfType(switchBody.getRBrace(), PsiStatement.class); + boolean hasResult = false; if (lastStatement instanceof PsiSwitchLabeledRuleStatement) { Collection results = new ArrayList<>(); for (PsiSwitchLabeledRuleStatement rule = (PsiSwitchLabeledRuleStatement)lastStatement; rule != null; rule = PsiTreeUtil.getPrevSiblingOfType(rule, PsiSwitchLabeledRuleStatement.class)) { PsiStatement ruleBody = rule.getBody(); + if (ruleBody instanceof PsiExpressionStatement) { + hasResult = true; + } // the expression and throw statements are fine, only the block statement could be an issue - if (ruleBody instanceof PsiBlockStatement && ControlFlowUtils.statementMayCompleteNormally(ruleBody)) { - PsiElement target = ObjectUtils.notNull(ObjectUtils.tryCast(rule.getFirstChild(), PsiKeyword.class), rule); - String message = JavaErrorBundle.message("switch.expr.rule.should.produce.result"); - results.add(HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(target).descriptionAndTooltip(message).create()); + if (ruleBody instanceof PsiBlockStatement) { + if (ControlFlowUtils.statementMayCompleteNormally(ruleBody)) { + PsiElement target = ObjectUtils.notNull(ObjectUtils.tryCast(rule.getFirstChild(), PsiKeyword.class), rule); + String message = JavaErrorBundle.message("switch.expr.rule.should.produce.result"); + results.add(HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(target).descriptionAndTooltip(message).create()); + } + else if (!hasResult && hasYield(switchExpression, ruleBody)) { + hasResult = true; + } } } - return results; + if (!results.isEmpty()) { + return results; + } + } else { + // previous statements may have no result as well, but in that case they fall through to the last one, which needs to be checked anyway + if (lastStatement != null && ControlFlowUtils.statementMayCompleteNormally(lastStatement)) { + PsiElement target = ObjectUtils.notNull(ObjectUtils.tryCast(switchExpression.getFirstChild(), PsiKeyword.class), switchExpression); + String message = JavaErrorBundle.message("switch.expr.should.produce.result"); + return Collections.singletonList(HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(target).descriptionAndTooltip(message).create()); + } + hasResult = hasYield(switchExpression, switchBody); } - // previous statements may have no result as well, but in that case they fall through to the last one, which needs to be checked anyway - if (lastStatement != null && ControlFlowUtils.statementMayCompleteNormally(lastStatement)) { + if (!hasResult) { PsiElement target = ObjectUtils.notNull(ObjectUtils.tryCast(switchExpression.getFirstChild(), PsiKeyword.class), switchExpression); - String message = JavaErrorBundle.message("switch.expr.should.produce.result"); - return Collections.singletonList(HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(target).descriptionAndTooltip(message).create()); + return Collections.singletonList(HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(target) + .descriptionAndTooltip(JavaErrorBundle.message("switch.expr.no.result")).create()); } } return Collections.emptyList(); } + private static boolean hasYield(@NotNull PsiSwitchExpression switchExpression, PsiElement scope) { + class YieldFinder extends JavaRecursiveElementWalkingVisitor { + boolean hasYield = false; + + @Override + public void visitYieldStatement(PsiYieldStatement statement) { + if (statement.findEnclosingExpression() == switchExpression) { + hasYield = true; + stopWalking(); + } + } + + // do not go inside to save time: declarations cannot contain yield that points to outer switch expression + @Override + public void visitDeclarationStatement(PsiDeclarationStatement statement) {} + + // do not go inside to save time: expressions cannot contain yield that points to outer switch expression + @Override + public void visitExpression(PsiExpression expression) {} + } + YieldFinder finder = new YieldFinder(); + scope.accept(finder); + return finder.hasYield; + } + /** * See JLS 8.3.2.3. */ diff --git a/java/java-psi-impl/src/messages/JavaErrorBundle.properties b/java/java-psi-impl/src/messages/JavaErrorBundle.properties index e450b47efe9a..32e2fe1ebf62 100644 --- a/java/java-psi-impl/src/messages/JavaErrorBundle.properties +++ b/java/java-psi-impl/src/messages/JavaErrorBundle.properties @@ -241,6 +241,7 @@ duplicate.switch.label=Duplicate label ''{0}'' switch.colon.expected.after.case.label=':' expected switch.expr.empty='switch' expression does not have any case clauses switch.expr.incomplete='switch' expression does not cover all possible input values +switch.expr.no.result=Switch expression does not have any result expressions switch.expr.should.produce.result=Switch expression should produce result in all execution paths switch.expr.rule.should.produce.result=Switch expression rule should produce result in all execution paths diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/SwitchExpressionHasResult.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/SwitchExpressionHasResult.java index f8380ef8fd5c..1156848446e6 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/SwitchExpressionHasResult.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/SwitchExpressionHasResult.java @@ -28,13 +28,13 @@ class C { } void defaultBranchAlwaysThrows(int n) { - String s = switch (n) { + String s = switch (n) { default: throw new RuntimeException(); }; } void defaultRuleAlwaysThrows(int n) { - String s = switch (n) { + String s = switch (n) { default -> throw new RuntimeException(); }; } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/SwitchExpressionsNoResult.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/SwitchExpressionsNoResult.java new file mode 100644 index 000000000000..e275400e4a6c --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/switchExpressions/SwitchExpressionsNoResult.java @@ -0,0 +1,18 @@ +class Test { + void test() { + int i = switch(0) { + default -> throw new NullPointerException(); + }; + } + + void test2() { + int i = switch(0) { + case 0 -> {while(true);} + case 1 -> { + throw new RuntimeException(); + } + default -> throw new NullPointerException(); + }; + + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/JavaSwitchExpressionsHighlightingTest.kt b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/JavaSwitchExpressionsHighlightingTest.kt index be0712d8a2b3..62e0f8d4d11e 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/JavaSwitchExpressionsHighlightingTest.kt +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/JavaSwitchExpressionsHighlightingTest.kt @@ -11,6 +11,7 @@ class JavaSwitchExpressionsHighlightingTest : LightJavaCodeInsightFixtureTestCas fun testEnhancedSwitchStatements() = doTest() fun testSwitchExpressions() = doTest() + fun testSwitchExpressionsNoResult() = doTest() fun testSwitchExpressionsEnumResolve() = doTest() fun testSwitchNumericPromotion() = doTest() fun testSimpleInferenceCases() = doTest()