Skip to content

Commit

Permalink
[Sealed Types + Enhanced Switch] Incorrect diagnostic about switch not
Browse files Browse the repository at this point in the history 
being exhaustive

* Fixes eclipse-jdt#2720
  • Loading branch information
@srikanth-sankaran
srikanth-sankaran committed Oct 15, 2024
1 parent 11f45e6 commit d1047d7
Show file tree
Hide file tree
Showing 3 changed files with 243 additions and 41 deletions.
118 changes: 78 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,29 @@ && defaultFound && isExhaustive()) {
if (this.scope != null) this.scope.enclosingCase = null; // no longer inside switch case block
}
}

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)
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 +1427,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 +1442,17 @@ 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 +1488,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
163 changes: 163 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 @@ -8864,4 +8864,167 @@ final class A<T> implements I<X> {
"Type I<X> cannot be safely cast to B<X>\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 d1047d7

Please sign in to comment.