From f508a96cd748c7809ddb40aea278e7937f250657 Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Sat, 7 Sep 2024 12:18:19 +0200 Subject: [PATCH] [23] exhaustiveness signalling difference between javac and ecj + clarify that flagDuplicateDefault() flags only conditionally, renamed to checkDuplicateDefault() - inside this method clarify that only one error is reported per loc. + remove all conflict reporting from CaseStatement.resolveCasePattern - will be done within SwitchStatement.resolve() anyway + remove IProblem.DuplicateTotalPattern and related code - all errors reported here coincided with a dominance error + adjust tests: no longer expect secondary errors fixes https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2915 --- .../.settings/.api_filters | 6 ++ .../eclipse/jdt/core/compiler/IProblem.java | 3 - .../internal/compiler/ast/CaseStatement.java | 31 ++++----- .../compiler/ast/SwitchStatement.java | 3 +- .../compiler/problem/ProblemReporter.java | 8 --- .../compiler/problem/messages.properties | 2 +- .../regression/CompilerInvocationTests.java | 2 - .../regression/PrimitiveInPatternsTestSH.java | 67 +++++++++++++++++++ .../regression/RecordPatternTest.java | 6 -- .../regression/SwitchPatternTest.java | 20 ------ 10 files changed, 89 insertions(+), 59 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/.settings/.api_filters b/org.eclipse.jdt.core.compiler.batch/.settings/.api_filters index f4f4c28e8ce..5224716d0a0 100644 --- a/org.eclipse.jdt.core.compiler.batch/.settings/.api_filters +++ b/org.eclipse.jdt.core.compiler.batch/.settings/.api_filters @@ -23,6 +23,12 @@ + + + + + + diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java index b9a3b558223..7906ea9b00c 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java @@ -2537,9 +2537,6 @@ public interface IProblem { /** @since 3.28 * @noreference preview feature error */ int EnhancedSwitchMissingDefault = PreviewRelated + 1908; - /** @since 3.28 - * @noreference preview feature error */ - int DuplicateTotalPattern = PreviewRelated + 1909; /** @since 3.34 * @noreference preview feature error */ diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java index 6fae5a0dc73..27895bd43e1 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java @@ -179,7 +179,7 @@ public ResolvedCase[] resolveCase(BlockScope scope, TypeBinding switchExpression this.swich = switchStatement; scope.enclosingCase = this; // record entering in a switch case block if (this.constantExpressions == Expression.NO_EXPRESSIONS) { - flagDuplicateDefault(scope, switchStatement, this); + checkDuplicateDefault(scope, switchStatement, this); return ResolvedCase.UnresolvedCase; } @@ -192,7 +192,7 @@ public ResolvedCase[] resolveCase(BlockScope scope, TypeBinding switchExpression for (Expression e : this.constantExpressions) { count++; if (e instanceof FakeDefaultLiteral) { - flagDuplicateDefault(scope, switchStatement, this.constantExpressions.length > 1 ? e : this); + checkDuplicateDefault(scope, switchStatement, this.constantExpressions.length > 1 ? e : this); if (count != 2 || nullCaseCount < 1) { scope.problemReporter().patternSwitchCaseDefaultOnlyAsSecond(e); } @@ -267,16 +267,20 @@ public ResolvedCase[] resolveCase(BlockScope scope, TypeBinding switchExpression return hasResolveErrors ? ResolvedCase.UnresolvedCase : cases.toArray(new ResolvedCase[cases.size()]); } -private void flagDuplicateDefault(BlockScope scope, SwitchStatement switchStatement, ASTNode node) { - // remember the default case into the associated switch statement - if (switchStatement.defaultCase != null) +/** + * Precondition: this is a default case. + * Check if (a) another default or (b) a total type pattern has been recorded already + */ +private void checkDuplicateDefault(BlockScope scope, SwitchStatement switchStatement, ASTNode node) { + if (switchStatement.defaultCase != null) { scope.problemReporter().duplicateDefaultCase(node); + } else if ((switchStatement.switchBits & SwitchStatement.TotalPattern) != 0) { + scope.problemReporter().illegalTotalPatternWithDefault(this); + } + // remember the default case into the associated switch statement // on error the last default will be the selected one ... switchStatement.defaultCase = this; - if ((switchStatement.switchBits & SwitchStatement.TotalPattern) != 0) { - scope.problemReporter().illegalTotalPatternWithDefault(this); - } } @Override @@ -394,17 +398,10 @@ private Constant resolveCasePattern(BlockScope scope, TypeBinding caseType, Type } } if (e.coversType(expressionType, scope)) { - if ((switchStatement.switchBits & SwitchStatement.TotalPattern) != 0) { - scope.problemReporter().duplicateTotalPattern(e); - return IntConstant.fromValue(-1); - } switchStatement.switchBits |= SwitchStatement.Exhaustive; - if (e.isUnconditional(expressionType, scope)) { - switchStatement.switchBits |= SwitchStatement.TotalPattern; - if (switchStatement.defaultCase != null && !(e instanceof RecordPattern)) - scope.problemReporter().illegalTotalPatternWithDefault(this); - } e.isTotalTypeNode = true; + if (e.isUnconditional(expressionType, scope)) // unguarded is implied from 'coversType()' above + switchStatement.switchBits |= SwitchStatement.TotalPattern; if (switchStatement.nullCase == null) constant = IntConstant.fromValue(-1); } diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java index 24a8c834e70..7912dc9addd 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java @@ -1192,8 +1192,7 @@ public void resolve(BlockScope upperScope) { ResolvedCase[] constantsList; final Statement statement = this.statements[i]; if (statement instanceof CaseStatement caseStmt) { - caseNullDefaultFound = caseNullDefaultFound ? caseNullDefaultFound - : isCaseStmtNullDefault(caseStmt); + caseNullDefaultFound |= isCaseStmtNullDefault(caseStmt); defaultFound |= caseStmt.constantExpressions == Expression.NO_EXPRESSIONS; constantsList = caseStmt.resolveCase(this.scope, expressionType, this); patternVariables = statement.bindingsWhenTrue(); diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java index 2e532831e30..b79b28a97a2 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java @@ -12597,14 +12597,6 @@ public void enhancedSwitchMissingDefaultCase(ASTNode element) { element.sourceStart, element.sourceEnd); } -public void duplicateTotalPattern(ASTNode element) { - this.handle( - IProblem.DuplicateTotalPattern, - NoArgument, - NoArgument, - element.sourceStart, - element.sourceEnd); -} public void unexpectedTypeinSwitchPattern(TypeBinding type, ASTNode element) { this.handle( IProblem.UnexpectedTypeinSwitchPattern, diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties index 57967104c56..9432d12e890 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties @@ -1156,7 +1156,7 @@ 1906 = This case label is dominated by one of the preceding case labels 1907 = Switch case cannot have both unconditional pattern and default label 1908 = An enhanced switch statement should be exhaustive; a default label expected -1909 = The switch statement cannot have more than one unconditional pattern +# 1909 removed 1910 = Unnecessary 'null' pattern, the switch selector expression cannot be null 1911 = Unexpected type {0}, expected class or array type 1920 = A null case label has to be either the only expression in a case label or the first expression followed only by a default diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java index 3774f3b63c3..cfa75f1da73 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java @@ -1340,7 +1340,6 @@ class ProblemAttributes { expectedProblemAttributes.put("PatternDominated", new ProblemAttributes(true)); expectedProblemAttributes.put("IllegalTotalPatternWithDefault", new ProblemAttributes(true)); expectedProblemAttributes.put("EnhancedSwitchMissingDefault", new ProblemAttributes(true)); - expectedProblemAttributes.put("DuplicateTotalPattern", new ProblemAttributes(true)); expectedProblemAttributes.put("UnexpectedTypeinSwitchPattern", new ProblemAttributes(true)); expectedProblemAttributes.put("UnexpectedTypeinRecordPattern", new ProblemAttributes(true)); expectedProblemAttributes.put("RecordPatternMismatch", new ProblemAttributes(true)); @@ -2474,7 +2473,6 @@ class ProblemAttributes { expectedProblemAttributes.put("PatternDominated", SKIP); expectedProblemAttributes.put("IllegalTotalPatternWithDefault", SKIP); expectedProblemAttributes.put("EnhancedSwitchMissingDefault", SKIP); - expectedProblemAttributes.put("DuplicateTotalPattern", SKIP); expectedProblemAttributes.put("UnexpectedTypeinSwitchPattern", SKIP); expectedProblemAttributes.put("UnexpectedTypeinRecordPattern", SKIP); expectedProblemAttributes.put("RecordPatternMismatch", SKIP); diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/PrimitiveInPatternsTestSH.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/PrimitiveInPatternsTestSH.java index fad38fa1e6a..8a16e0adc0d 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/PrimitiveInPatternsTestSH.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/PrimitiveInPatternsTestSH.java @@ -2221,6 +2221,7 @@ public static void main(String... args) { } public void testCoversTypePlusDefault() { + // case int i "covers" type Integer but is not unconditional runConformTest(new String[] { "X.java", """ @@ -2241,6 +2242,72 @@ public static void main(String argv[]) { }, "true"); } + + public void testUnconditionPlusDefault() { + // case int i "covers" type int and is unconditional + // various combinations of dominance with/without default + runNegativeTest(new String[] { + "X.java", + """ + public class X { + int foo1(int myInt) { + return switch (myInt) { + case int i -> i; + default -> 0; // conflict with preceding total pattern (unguarded and unconditional) + }; + } + int foo2(int myInt) { + return switch (myInt) { // swapped order of cases + default -> 0; + case int i -> i; // conflict with preceding default + }; + } + int foo3(int myInt) { + return switch (myInt) { + default -> 0; + case int i -> i; // conflict with preceding default + case short s -> s; // additionally dominated by int i + }; + } + int foo4(int myInt) { + return switch (myInt) { + case int i -> i; + case short s -> s; // dominated by int i + }; + } + } + """ + }, + """ + ---------- + 1. ERROR in X.java (at line 5) + default -> 0; // conflict with preceding total pattern (unguarded and unconditional) + ^^^^^^^ + Switch case cannot have both unconditional pattern and default label + ---------- + 2. ERROR in X.java (at line 11) + case int i -> i; // conflict with preceding default + ^^^^^ + This case label is dominated by one of the preceding case labels + ---------- + 3. ERROR in X.java (at line 17) + case int i -> i; // conflict with preceding default + ^^^^^ + This case label is dominated by one of the preceding case labels + ---------- + 4. ERROR in X.java (at line 18) + case short s -> s; // additionally dominated by int i + ^^^^^^^ + This case label is dominated by one of the preceding case labels + ---------- + 5. ERROR in X.java (at line 24) + case short s -> s; // dominated by int i + ^^^^^^^ + This case label is dominated by one of the preceding case labels + ---------- + """); + } + // test from spec public void _testSpec001() { runConformTest(new String[] { diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/RecordPatternTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/RecordPatternTest.java index e0a1f081929..21693add4bd 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/RecordPatternTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/RecordPatternTest.java @@ -367,12 +367,6 @@ public void test009() { " case Rectangle(ColoredPoint(Point(int x, int y), Color c),\n" + " ColoredPoint(Point(int x1, int y1), Color c1)) -> {\n" + " ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + - "The switch statement cannot have more than one unconditional pattern\n" + - "----------\n" + - "2. ERROR in X.java (at line 7)\n" + - " case Rectangle(ColoredPoint(Point(int x, int y), Color c),\n" + - " ColoredPoint(Point(int x1, int y1), Color c1)) -> {\n" + - " ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + "This case label is dominated by one of the preceding case labels\n" + "----------\n"); } diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java index 9e76984d476..de5073ff62d 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java @@ -2876,11 +2876,6 @@ public void testBug575048_01() { "----------\n" + "1. ERROR in X.java (at line 5)\n" + " case Integer i1 -> 0;\n" + - " ^^^^^^^^^^^^^^^\n" + - "Switch case cannot have both unconditional pattern and default label\n" + - "----------\n" + - "2. ERROR in X.java (at line 5)\n" + - " case Integer i1 -> 0;\n" + " ^^^^^^^^^^\n" + "This case label is dominated by one of the preceding case labels\n" + "----------\n"); @@ -3539,11 +3534,6 @@ public void testBug575047_06() { "1. ERROR in X.java (at line 6)\n" + " case String s -> -1;\n" + " ^^^^^^^^\n" + - "The switch statement cannot have more than one unconditional pattern\n" + - "----------\n" + - "2. ERROR in X.java (at line 6)\n" + - " case String s -> -1;\n" + - " ^^^^^^^^\n" + "This case label is dominated by one of the preceding case labels\n" + "----------\n"); } @@ -5751,11 +5741,6 @@ public void testIssue742_1() { "1. ERROR in X.java (at line 7)\n" + " case Integer i -> // Error - dominated case label\n" + " ^^^^^^^^^\n" + - "The switch statement cannot have more than one unconditional pattern\n" + - "----------\n" + - "2. ERROR in X.java (at line 7)\n" + - " case Integer i -> // Error - dominated case label\n" + - " ^^^^^^^^^\n" + "This case label is dominated by one of the preceding case labels\n" + "----------\n"); } @@ -5779,11 +5764,6 @@ public void testIssue742_2() { "1. ERROR in X.java (at line 7)\n" + " case Integer i when true -> // Error - dominated case label\n" + " ^^^^^^^^^\n" + - "The switch statement cannot have more than one unconditional pattern\n" + - "----------\n" + - "2. ERROR in X.java (at line 7)\n" + - " case Integer i when true -> // Error - dominated case label\n" + - " ^^^^^^^^^\n" + "This case label is dominated by one of the preceding case labels\n" + "----------\n"); }