Skip to content

Commit

Permalink
Reject illegal combination of modifiers for sealed types. (#2995)
Browse files Browse the repository at this point in the history
* Fixes #2707
  • Loading branch information
srikanth-sankaran authored Sep 23, 2024
1 parent b763f99 commit 5780a5e
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,8 @@ public interface IProblem {
int ShouldImplementHashcode = TypeRelated + 332;
/** @since 3.5 */
int AbstractMethodsInConcreteClass = TypeRelated + 333;
/** @since 3.40 */
int IllegalModifierCombinationForType = TypeRelated + 334;

/** @deprecated - problem is no longer generated, use {@link #UndefinedType} instead */
int SuperclassNotFound = TypeRelated + 329 + ProblemReasons.NotFound; // TypeRelated + 330
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,13 @@ private void checkAndSetModifiers() {
boolean isSealedSupported = JavaFeature.SEALED_CLASSES.isSupported(options);
boolean flagSealedNonModifiers = isSealedSupported &&
(modifiers & (ExtraCompilerModifiers.AccSealed | ExtraCompilerModifiers.AccNonSealed)) != 0;

switch (modifiers & (ExtraCompilerModifiers.AccSealed | ExtraCompilerModifiers.AccNonSealed | ClassFileConstants.AccFinal)) {
case ExtraCompilerModifiers.AccSealed, ExtraCompilerModifiers.AccNonSealed, ClassFileConstants.AccFinal, ClassFileConstants.AccDefault : break;
default :
problemReporter().IllegalModifierCombinationForType(sourceType);
break;
}
if (sourceType.isRecord()) {
/* JLS 14 Records Sec 8.10 - A record declaration is implicitly final. */
modifiers |= ClassFileConstants.AccFinal;
Expand Down Expand Up @@ -1192,7 +1199,7 @@ private boolean connectSuperclass() {
if (superclass != null) { // is null if a cycle was detected cycle or a problem
if (!superclass.isClass() && (superclass.tagBits & TagBits.HasMissingType) == 0) {
problemReporter().superclassMustBeAClass(sourceType, superclassRef, superclass);
} else if (superclass.isFinal()) {
} else if (superclass.isFinal() && !superclass.isSealed()) { // sealed AND final complained about elsewhere, no value in additional complaint here.
if (superclass.isRecord()) {
problemReporter().classExtendFinalRecord(sourceType, superclassRef, superclass);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3430,6 +3430,15 @@ public void illegalVisibilityModifierCombinationForField(ReferenceBinding type,
fieldDecl.sourceStart,
fieldDecl.sourceEnd);
}
public void IllegalModifierCombinationForType(SourceTypeBinding type) {
String[] arguments = new String[] {new String(type.sourceName())};
this.handle(
IProblem.IllegalModifierCombinationForType,
arguments,
arguments,
type.sourceStart(),
type.sourceEnd());
}
public void illegalVisibilityModifierCombinationForMemberType(SourceTypeBinding type) {
String[] arguments = new String[] {new String(type.sourceName())};
this.handle(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@
331 = Redundant superinterface {0} for the type {1}, already defined by {2}
332 = The type {0} should also implement hashCode() since it overrides Object.equals()
333 = The type {0} must be an abstract class to define abstract methods
334 = The type {0} may have only one modifier out of sealed, non-sealed, and final

###[obsolete] 330 = {0} cannot be resolved or is not a valid superclass
###[obsolete] 331 = Superclass {0} is not visible
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@ class ProblemAttributes {
expectedProblemAttributes.put("NamedPatternVariablesDisallowedHere", new ProblemAttributes(CategorizedProblem.CAT_INTERNAL));
expectedProblemAttributes.put("OperandStackExceeds64KLimit", new ProblemAttributes(CategorizedProblem.CAT_INTERNAL));
expectedProblemAttributes.put("OperandStackSizeInappropriate", new ProblemAttributes(CategorizedProblem.CAT_INTERNAL));
expectedProblemAttributes.put("IllegalModifierCombinationForType", new ProblemAttributes(CategorizedProblem.CAT_TYPE));

StringBuilder failures = new StringBuilder();
StringBuilder correctResult = new StringBuilder(70000);
Expand Down Expand Up @@ -2497,6 +2498,7 @@ class ProblemAttributes {
expectedProblemAttributes.put("NamedPatternVariablesDisallowedHere", SKIP);
expectedProblemAttributes.put("OperandStackExceeds64KLimit", SKIP);
expectedProblemAttributes.put("OperandStackSizeInappropriate", SKIP);
expectedProblemAttributes.put("IllegalModifierCombinationForType", SKIP);


Map constantNamesIndex = new HashMap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1067,11 +1067,16 @@ public void testBug563806_032() {
"1. ERROR in p1\\X.java (at line 2)\n" +
" public sealed non-sealed interface X {\n" +
" ^\n" +
"Sealed class or interface lacks the permits clause and no class or interface from the same compilation unit declares X as its direct superclass or superinterface\n" +
"The type X may have only one modifier out of sealed, non-sealed, and final\n" +
"----------\n" +
"2. ERROR in p1\\X.java (at line 2)\n" +
" public sealed non-sealed interface X {\n" +
" ^\n" +
"Sealed class or interface lacks the permits clause and no class or interface from the same compilation unit declares X as its direct superclass or superinterface\n" +
"----------\n" +
"3. ERROR in p1\\X.java (at line 2)\n" +
" public sealed non-sealed interface X {\n" +
" ^\n" +
"An interface X is declared both sealed and non-sealed\n" +
"----------\n");
}
Expand Down Expand Up @@ -1192,6 +1197,11 @@ public void testBug563806_038() {
"1. ERROR in p1\\X.java (at line 4)\n" +
" non-sealed sealed class Y{}\n" +
" ^\n" +
"The type Y may have only one modifier out of sealed, non-sealed, and final\n" +
"----------\n" +
"2. ERROR in p1\\X.java (at line 4)\n" +
" non-sealed sealed class Y{}\n" +
" ^\n" +
"Illegal modifier for the local class Y; only abstract or final is permitted\n" +
"----------\n");
}
Expand Down Expand Up @@ -6322,4 +6332,74 @@ public static void main(String [] args) {
},
"42");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2707
// [Sealed types] ECJ allows a class to be declared as both sealed and non-sealed
public void testIssue2707() {
runNegativeTest(
new String[] {
"X.java",
"""
public sealed class X permits Y {
}
sealed non-sealed class Y extends X permits K {}
final class K extends Y {}
"""
},
"----------\n" +
"1. ERROR in X.java (at line 4)\n" +
" sealed non-sealed class Y extends X permits K {}\n" +
" ^\n" +
"The type Y may have only one modifier out of sealed, non-sealed, and final\n" +
"----------\n");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2707
// [Sealed types] ECJ allows a class to be declared as both sealed and non-sealed
public void testIssue2707_2() {
runNegativeTest(
new String[] {
"X.java",
"""
public sealed class X permits Y {
}
final non-sealed class Y extends X {}
"""
},
"----------\n" +
"1. ERROR in X.java (at line 4)\n" +
" final non-sealed class Y extends X {}\n" +
" ^\n" +
"The type Y may have only one modifier out of sealed, non-sealed, and final\n" +
"----------\n");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2707
// [Sealed types] ECJ allows a class to be declared as both sealed and non-sealed
public void testIssue2707_3() {
runNegativeTest(
new String[] {
"X.java",
"""
public final sealed class X permits Y {
}
final non-sealed class Y extends X {}
"""
},
"----------\n" +
"1. ERROR in X.java (at line 1)\n" +
" public final sealed class X permits Y {\n" +
" ^\n" +
"The type X may have only one modifier out of sealed, non-sealed, and final\n" +
"----------\n" +
"2. ERROR in X.java (at line 4)\n" +
" final non-sealed class Y extends X {}\n" +
" ^\n" +
"The type Y may have only one modifier out of sealed, non-sealed, and final\n" +
"----------\n");
}
}

0 comments on commit 5780a5e

Please sign in to comment.