Skip to content

Commit

Permalink
[Sealed Types + Enhanced Switch] Incorrect diagnostic about switch no…
Browse files Browse the repository at this point in the history 
…t being exhaustive (#3080)

* Fixes #2720
  • Loading branch information
@srikanth-sankaran
srikanth-sankaran authored Oct 15, 2024
1 parent dbd7428 commit 4494017
Show file tree
Hide file tree
Showing 3 changed files with 246 additions and 41 deletions.
120 changes: 80 additions & 40 deletions ...se.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@
import java.lang.invoke.ConstantBootstraps;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.function.Function;
import java.util.function.IntPredicate;
import org.eclipse.jdt.core.compiler.CharOperation;
Expand All @@ -43,7 +46,15 @@
import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
import org.eclipse.jdt.internal.compiler.impl.Constant;
import org.eclipse.jdt.internal.compiler.impl.JavaFeature;
import org.eclipse.jdt.internal.compiler.lookup.*;
import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
import org.eclipse.jdt.internal.compiler.lookup.FieldBinding;
import org.eclipse.jdt.internal.compiler.lookup.LocalVariableBinding;
import org.eclipse.jdt.internal.compiler.lookup.RecordComponentBinding;
import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding;
import org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.SyntheticMethodBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeIds;
import org.eclipse.jdt.internal.compiler.problem.ProblemSeverities;

@SuppressWarnings("rawtypes")
Expand Down Expand Up @@ -1307,23 +1318,18 @@ && defaultFound && isExhaustive()) {
((this.containsPatterns || this.containsNull) ||
(constantCount >= this.caseCount &&
constantCount != ((ReferenceBinding)expressionType).enumConstantCount()))) {
FieldBinding[] enumFields = ((ReferenceBinding) expressionType.erasure()).fields();
for (int i = 0, max = enumFields.length; i < max; i++) {
FieldBinding enumConstant = enumFields[i];
if ((enumConstant.modifiers & ClassFileConstants.AccEnum) == 0) continue;
findConstant : {
for (int j = 0; j < constantCount; j++) {
if ((enumConstant.id + 1) == this.otherConstants[j].c.intValue()) // zero should not be returned see bug 141810
break findConstant;
}
this.switchBits &= ~SwitchStatement.Exhaustive;
// enum constant did not get referenced from switch
boolean suppress = (this.defaultCase != null && (this.defaultCase.bits & DocumentedCasesOmitted) != 0);
if (!suppress) {
if (isEnhanced)
upperScope.problemReporter().enhancedSwitchMissingDefaultCase(this.expression);
else
Set<FieldBinding> unenumeratedConstants = unenumeratedConstants((ReferenceBinding) expressionType, constantCount);
if (unenumeratedConstants.size() != 0) {
this.switchBits &= ~SwitchStatement.Exhaustive;
// enum constant did not get referenced from switch
boolean suppress = (this.defaultCase != null && (this.defaultCase.bits & DocumentedCasesOmitted) != 0);
if (!suppress) {
if (isEnhanced)
upperScope.problemReporter().enhancedSwitchMissingDefaultCase(this.expression);
else {
for (FieldBinding enumConstant : unenumeratedConstants) {
reportMissingEnumConstantCase(upperScope, enumConstant);
}
}
}
}
Expand All @@ -1341,6 +1347,32 @@ && defaultFound && isExhaustive()) {
if (this.scope != null) this.scope.enclosingCase = null; // no longer inside switch case block
}
}

// Return the set of enumerations belonging to the selector enum type that are not listed in case statements.
private Set<FieldBinding> unenumeratedConstants(ReferenceBinding enumType, int constantCount) {
FieldBinding[] enumFields = ((ReferenceBinding) enumType.erasure()).fields();
Set<FieldBinding> unenumerated = new HashSet<>(Arrays.asList(enumFields));
for (int i = 0, max = enumFields.length; i < max; i++) {
FieldBinding enumConstant = enumFields[i];
if ((enumConstant.modifiers & ClassFileConstants.AccEnum) == 0) {
unenumerated.remove(enumConstant);
continue;
}
for (int j = 0; j < constantCount; j++) {
if (TypeBinding.equalsEquals(this.otherConstants[j].e.resolvedType, enumType)) {
if (this.otherConstants[j].e instanceof NameReference reference) {
FieldBinding field = reference.fieldBinding();
int intValue = field.original().id + 1;
if ((enumConstant.id + 1) == intValue) { // zero should not be returned see bug 141810
unenumerated.remove(enumConstant);
break;
}
}
}
}
}
return unenumerated;
}
private boolean isCaseStmtNullDefault(CaseStatement caseStmt) {
return caseStmt != null
&& caseStmt.constantExpressions.length == 2
Expand Down Expand Up @@ -1398,20 +1430,11 @@ private boolean checkAndSetEnhanced(BlockScope upperScope, TypeBinding expressio
return false;
}
private boolean checkAndFlagDefaultSealed(BlockScope skope, CompilerOptions compilerOptions) {
if (this.defaultCase != null) { // mark covered as a side effect (since covers is intro in 406)
if (this.defaultCase != null) {
this.switchBits |= SwitchStatement.Exhaustive;
return false;
}
boolean checkSealed = this.containsPatterns
&& JavaFeature.SEALED_CLASSES.isSupported(compilerOptions)
&& JavaFeature.PATTERN_MATCHING_IN_SWITCH.isSupported(compilerOptions)
&& this.expression.resolvedType instanceof ReferenceBinding;
if (!checkSealed) return false;
ReferenceBinding ref = (ReferenceBinding) this.expression.resolvedType;
if (!(ref.isClass() || ref.isInterface() || ref.isTypeVariable() || ref.isIntersectionType()))
return false;

if (ref.isRecord()) {
if (this.expression.resolvedType instanceof ReferenceBinding ref && ref.isRecord()) {
boolean isRecordPattern = false;
for (Pattern pattern : this.caseLabelElements) {
if (pattern instanceof RecordPattern) {
Expand All @@ -1422,7 +1445,16 @@ private boolean checkAndFlagDefaultSealed(BlockScope skope, CompilerOptions comp
if (isRecordPattern)
return checkAndFlagDefaultRecord(skope, compilerOptions, ref);
}
if (!ref.isSealed()) return false;
boolean checkSealed = JavaFeature.SEALED_CLASSES.isSupported(compilerOptions)
&& JavaFeature.PATTERN_MATCHING_IN_SWITCH.isSupported(compilerOptions)
&& this.expression.resolvedType instanceof ReferenceBinding
&& this.expression.resolvedType.isSealed();

if (!checkSealed) return false;
ReferenceBinding ref = (ReferenceBinding) this.expression.resolvedType;
if (!(ref.isClass() || ref.isInterface() || ref.isTypeVariable() || ref.isIntersectionType()))
return false;

if (!isExhaustiveWithCaseTypes(ref.getAllEnumerableReferenceTypes(), this.caseLabelElementTypes)) {
if (this instanceof SwitchExpression) // non-exhaustive switch expressions will be flagged later.
return false;
Expand Down Expand Up @@ -1458,25 +1490,33 @@ private boolean checkAndFlagDefaultRecord(BlockScope skope, CompilerOptions comp
return false;
}
private boolean isExhaustiveWithCaseTypes(List<ReferenceBinding> allAllowedTypes, List<TypeBinding> listedTypes) {
int pendingTypes = allAllowedTypes.size();
for (ReferenceBinding pt : allAllowedTypes) {
/* Per JLS 14.11.1.1: A type T that names an abstract sealed class or sealed interface is covered
if every permitted direct subclass or subinterface of it is covered. These subtypes are already
added to allAllowedTypes and subject to cover test.
*/
if (pt.isAbstract() && pt.isSealed()) {
--pendingTypes;
Iterator<ReferenceBinding> iterator = allAllowedTypes.iterator();
while (iterator.hasNext()) {
ReferenceBinding next = iterator.next();
if (next.isAbstract() && next.isSealed()) {
/* Per JLS 14.11.1.1: A type T that names an abstract sealed class or sealed interface is covered
if every permitted direct subclass or subinterface of it is covered. These subtypes are already
added to allAllowedTypes and subject to cover test.
*/
iterator.remove();
continue;
}
for (TypeBinding type : listedTypes) {
// permits specifies classes, not parameterizations
if (pt.erasure().isCompatibleWith(type.erasure())) {
--pendingTypes;
if (next.erasure().isCompatibleWith(type.erasure())) {
iterator.remove();
break;
}
}
if (next.isEnum()) {
int constantCount = this.otherConstants == null ? 0 : this.otherConstants.length;
Set<FieldBinding> unenumeratedConstants = unenumeratedConstants(next, constantCount);
if (unenumeratedConstants.size() == 0) {
iterator.remove();
}
}
}
return pendingTypes == 0;
return allAllowedTypes.size() == 0;
}
private boolean needPatternDispatchCopy() {
if (this.containsPatterns || (this.switchBits & QualifiedEnum) != 0)
Expand Down
3 changes: 2 additions & 1 deletion ...dt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
*******************************************************************************/
package org.eclipse.jdt.internal.compiler.lookup;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
Expand Down Expand Up @@ -2563,7 +2564,7 @@ public List<ReferenceBinding> getAllEnumerableReferenceTypes() {
oldSet = permSet;
permSet = tmp;
} while (oldSet.size() != permSet.size());
return Arrays.asList(permSet.toArray(new ReferenceBinding[0]));
return new ArrayList<>(permSet);
}

// 5.1.6.1 Allowed Narrowing Reference Conversion
Expand Down
164 changes: 164 additions & 0 deletions ....tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8908,4 +8908,168 @@ public static void main(String[] args) {
"This case label is dominated by one of the preceding case labels\n" +
"----------\n");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2720
// [Sealed Types + Enhanced Switch] Incorrect diagnostic about switch not being exhaustive
public void testIssue2720() {
runConformTest(
new String[] {
"X.java",
"""
sealed interface I {
enum E implements I {
A, B, C;
}
}
public class X {
static void d(I i) {
switch (i) { // error: An enhanced switch statement should be exhaustive; a default label expected
case I.E.A -> { System.out.println("I.E.A"); }
case I.E.B -> { System.out.println("I.E.B"); }
case I.E.C -> { System.out.println("I.E.C"); }
}
}
public static void main(String [] args) {
d(I.E.A);
d(I.E.B);
d(I.E.C);
}
}
"""
},
"I.E.A\n" +
"I.E.B\n" +
"I.E.C");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2720
// [Sealed Types + Enhanced Switch] Incorrect diagnostic about switch not being exhaustive
public void testIssue2720_2() {
runNegativeTest(
new String[] {
"X.java",
"""
sealed interface I {
enum E implements I {
A, B, C;
}
enum K implements I {
D, E, F;
}
}
class Test {
void d(I i) {
switch (i) {
case I.E.A -> {}
case I.E.B -> {}
case I.E.C -> {}
case I.K k -> {}
}
switch (i) {
case I.E.A -> {}
case I.E.B -> {}
case I.E.C -> {}
default -> {}
}
switch (i) {
case I.E.A -> {}
case I.E.B -> {}
case I.E.C -> {}
case I.K.D -> {}
case I.K.E -> {}
case I.K.F -> {}
default -> {}
}
switch (i) {
case I.E.A -> {}
case I.E.B -> {}
case I.E.C -> {}
case I.K.D -> {}
case I.K.E -> {}
default -> {}
}
switch (i) {
case I.E.A -> {}
case I.E.B -> {}
case I.E.C -> {}
case I.K.D -> {}
case I.K.E -> {}
}
switch (i) {
case I.E.A -> {}
case I.E.B -> {}
case I.E.C -> {}
case I.K.D -> {}
case I.K.E -> {}
case I.K.F -> {}
}
}
}
"""
},
"----------\n" +
"1. ERROR in X.java (at line 44)\n" +
" switch (i) {\n" +
" ^\n" +
"An enhanced switch statement should be exhaustive; a default label expected\n" +
"----------\n");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2720
// [Sealed Types + Enhanced Switch] Incorrect diagnostic about switch not being exhaustive
public void testIssue2720_3() {
runConformTest(
new String[] {
"X.java",
"""
sealed interface I {
enum E implements I {
A, B, C;
}
enum K implements I {
D, E, F;
}
}
public class X {
static void d(I i) {
switch (i) {
case I.E.A -> { System.out.println("I.E.A"); }
case I.E.B -> { System.out.println("I.E.B"); }
case I.E.C -> { System.out.println("I.E.C"); }
case I.K.D -> { System.out.println("I.K.D"); }
case I.K.E -> { System.out.println("I.K.E"); }
case I.K.F -> { System.out.println("I.K.F"); }
}
}
public static void main(String [] args) {
d(I.E.A);
d(I.E.B);
d(I.E.C);
d(I.K.D);
d(I.K.E);
d(I.K.F);
}
}
"""
},
"I.E.A\n" +
"I.E.B\n" +
"I.E.C\n" +
"I.K.D\n" +
"I.K.E\n" +
"I.K.F");
}
}

0 comments on commit 4494017

Please sign in to comment.