Skip to content

Commit

Permalink
improve support for sealed types
Browse files Browse the repository at this point in the history 
#1321
  • Loading branch information
@eric-milles
eric-milles committed Dec 28, 2021
1 parent 526ba6f commit 39183a5
Show file tree
Hide file tree
Showing 5 changed files with 228 additions and 82 deletions.
81 changes: 74 additions & 7 deletions ...t.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/SealedTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void testSealed1() {
" }\n" +
" final int maxDistanceBetweenServicesInKilometers = 100_000\n" +
" @Override int getMaxServiceIntervalInMonths() { return 12 }\n" +
"}",
"}\n",

"Truck.groovy",
"final class Truck extends Vehicle implements Serviceable {\n" +
Expand All @@ -78,7 +78,7 @@ public void testSealed1() {
" }\n" +
" final int maxDistanceBetweenServicesInKilometers = 100_000\n" +
" @Override int getMaxServiceIntervalInMonths() { return 18 }\n" +
"}",
"}\n",
};
//@formatter:on

Expand All @@ -96,28 +96,95 @@ public void testSealed2() {

"Bar.groovy",
"class Bar {\n" + // missing "extends Foo"
"}",
"}\n",

"Baz.groovy",
"class Baz extends Foo {\n" + // missing "final", "sealed" or "non-sealed"
"}",
"}\n",

"Boo.groovy",
"class Boo extends Foo {\n" + // not permitted
"}",
"}\n",
};
//@formatter:on

runNegativeTest(sources,
runNegativeTest(sources, !javaModelSealedSupport()
?
"----------\n" +
"1. ERROR in Boo.groovy (at line 1)\n" +
"\tclass Boo extends Foo {\n" +
"\t ^^^\n" +
"Groovy:The class 'Boo' is not a permitted subclass of the sealed class 'Foo'.\n" +
"----------\n"
:
"----------\n" +
"1. ERROR in Foo.groovy (at line 1)\n" +
"\t@groovy.transform.Sealed(permittedSubclasses=[Bar,Baz])\n" +
"\t ^^^\n" +
"Permitted class Bar does not declare Foo as direct super class\n" +
"----------\n" +
"----------\n" +
"1. ERROR in Baz.groovy (at line 1)\n" +
"\tclass Baz extends Foo {\n" +
"\t ^^^\n" +
"The class Baz with a sealed direct superclass or a sealed direct superinterface Foo should be declared either final, sealed, or non-sealed\n" +
"----------\n" +
"----------\n" +
"1. ERROR in Boo.groovy (at line 1)\n" +
"\tclass Boo extends Foo {\n" +
"\t ^^^\n" +
"The class Boo with a sealed direct superclass or a sealed direct superinterface Foo should be declared either final, sealed, or non-sealed\n" +
"----------\n" +
"2. ERROR in Boo.groovy (at line 1)\n" +
"\tclass Boo extends Foo {\n" +
"\t ^^^\n" +
"Groovy:The class 'Boo' is not a permitted subclass of the sealed class 'Foo'.\n" +
"----------\n" +
"3. ERROR in Boo.groovy (at line 1)\n" +
"\tclass Boo extends Foo {\n" +
"\t ^^^\n" +
"The type Boo extending a sealed class Foo should be a permitted subtype of Foo\n" +
"----------\n");
}

@Test
public void testSealed3() {
assumeTrue(javaModelSealedSupport());

//@formatter:off
String[] sources = {
"p/Foo.groovy",
"package p\n" +
"@groovy.transform.Sealed(permittedSubclasses=[Bar.class,p.Baz])\n" +
"@groovy.transform.PackageScope abstract class Foo {\n" +
"}\n",

"p/Bar.java",
"package p;\n" +
"final class Bar extends Foo {\n" +
"}\n",

"p/Baz.java",
"package p;\n" +
"class Baz extends Foo {\n" + // missing "final", "sealed" or "non-sealed"
"}\n",
};
//@formatter:on

runNegativeTest(sources,
"----------\n" +
"1. ERROR in p\\Baz.java (at line 2)\n" +
"\tclass Baz extends Foo {\n" +
"\t ^^^\n" +
"The class Baz with a sealed direct superclass or a sealed direct superinterface Foo should be declared either final, sealed, or non-sealed\n" +
"----------\n");
}

// non-sealed without extends
// sealed without permits
// java extension

private static boolean javaModelSealedSupport() {
return org.eclipse.jdt.core.JavaCore.getPlugin().getBundle().getVersion()
.compareTo(org.osgi.framework.Version.parseVersion("3.24")) >= 0;
}
}
76 changes: 35 additions & 41 deletions base/org.codehaus.groovy40/src/org/apache/groovy/parser/antlr4/AstBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -769,10 +769,8 @@ public Tuple2<Parameter, Expression> visitEnhancedForControl(final EnhancedForCo
parameter.setColumnNumber(ctx.variableModifiersOpt().getStart().getCharPositionInLine() + 1);
parameter.setStart(locationSupport.findOffset(parameter.getLineNumber(), parameter.getColumnNumber()));

Optional<ModifierNode> var = new ModifierManager(this, this.visitVariableModifiersOpt(ctx.variableModifiersOpt())).get(VAR);
if (var != null && var.isPresent()) {
parameter.setNodeMetaData("reserved.type.name", var.get());
}
new ModifierManager(this, this.visitVariableModifiersOpt(ctx.variableModifiersOpt()))
.get(VAR).ifPresent(var -> parameter.setNodeMetaData("reserved.type.name", var));
// GRECLIPSE end

return tuple(parameter, (Expression) this.visit(ctx.expression()));
Expand Down Expand Up @@ -1459,7 +1457,7 @@ private void initUsingGenerics(final ClassNode classNode) {
public ClassNode visitClassDeclaration(final ClassDeclarationContext ctx) {
String packageName = Optional.ofNullable(moduleNode.getPackageName()).orElse("");
String className = this.visitIdentifier(ctx.identifier());
if (VAR_STR.equals(className)) {
if ("var".equals(className)) {
throw createParsingFailedException("var cannot be used for type declarations", ctx.identifier());
}

Expand Down Expand Up @@ -1496,31 +1494,29 @@ public ClassNode visitClassDeclaration(final ClassDeclarationContext ctx) {
}
}

List<ModifierNode> modifierNodeList = ctx.getNodeMetaData(TYPE_DECLARATION_MODIFIERS);
Objects.requireNonNull(modifierNodeList, "modifierNodeList should not be null");
ModifierManager modifierManager = new ModifierManager(this, modifierNodeList);
ModifierManager modifierManager = new ModifierManager(this, ctx.getNodeMetaData(TYPE_DECLARATION_MODIFIERS));

Optional<ModifierNode> finalModifierNodeOptional = modifierManager.get(FINAL);
Optional<ModifierNode> sealedModifierNodeOptional = modifierManager.get(SEALED);
Optional<ModifierNode> nonSealedModifierNodeOptional = modifierManager.get(NON_SEALED);
boolean isFinal = finalModifierNodeOptional.isPresent();
boolean isSealed = sealedModifierNodeOptional.isPresent();
boolean isNonSealed = nonSealedModifierNodeOptional.isPresent();
Optional<ModifierNode> finalModifier = modifierManager.get(FINAL);
Optional<ModifierNode> sealedModifier = modifierManager.get(SEALED);
Optional<ModifierNode> nonSealedModifier = modifierManager.get(NON_SEALED);
boolean isFinal = finalModifier.isPresent();
boolean isSealed = sealedModifier.isPresent();
boolean isNonSealed = nonSealedModifier.isPresent();

boolean isRecord = asBoolean(ctx.RECORD());
boolean hasRecordHeader = asBoolean(ctx.formalParameters());
if (isRecord) {
if (asBoolean(ctx.EXTENDS())) {
throw createParsingFailedException("No extends clause allowed for record declaration", ctx.EXTENDS());
}
if (!hasRecordHeader) {
throw createParsingFailedException("header declaration of record is expected", ctx.identifier());
}
if (asBoolean(ctx.EXTENDS())) {
throw createParsingFailedException("No extends clause allowed for record declaration", ctx.EXTENDS());
}
if (isSealed) {
throw createParsingFailedException("`sealed` is not allowed for record declaration", sealedModifierNodeOptional.get());
throw createParsingFailedException("`sealed` is not allowed for record declaration", sealedModifier.get());
}
if (isNonSealed) {
throw createParsingFailedException("`non-sealed` is not allowed for record declaration", nonSealedModifierNodeOptional.get());
throw createParsingFailedException("`non-sealed` is not allowed for record declaration", nonSealedModifier.get());
}
} else {
if (hasRecordHeader) {
Expand All @@ -1529,15 +1525,15 @@ public ClassNode visitClassDeclaration(final ClassDeclarationContext ctx) {
}

if (isSealed && isNonSealed) {
throw createParsingFailedException("type cannot be defined with both `sealed` and `non-sealed`", nonSealedModifierNodeOptional.get());
throw createParsingFailedException("type cannot be defined with both `sealed` and `non-sealed`", nonSealedModifier.get());
}

if (isFinal && (isSealed || isNonSealed)) {
throw createParsingFailedException("type cannot be defined with both " + (isSealed ? "`sealed`" : "`non-sealed`") + " and `final`", finalModifierNodeOptional.get());
throw createParsingFailedException("type cannot be defined with both " + (isSealed ? "`sealed`" : "`non-sealed`") + " and `final`", finalModifier.get());
}

if ((isAnnotation || isEnum) && (isSealed || isNonSealed)) {
ModifierNode mn = isSealed ? sealedModifierNodeOptional.get() : nonSealedModifierNodeOptional.get();
ModifierNode mn = isSealed ? sealedModifier.get() : nonSealedModifier.get();
throw createParsingFailedException("modifier `" + mn.getText() + "` is not allowed for " + (isEnum ? "enum" : "annotation definition"), mn);
}

Expand Down Expand Up @@ -1579,7 +1575,16 @@ public ClassNode visitClassDeclaration(final ClassDeclarationContext ctx) {
configureAST(classNode, ctx);
// GRECLIPSE add
ASTNode nameNode = configureAST(new ConstantExpression(className), ctx.identifier());
classNode.setNameStart(nameNode.getStart()); classNode.setNameEnd(nameNode.getEnd() - 1);
classNode.setNameStart(nameNode.getStart()); classNode.setNameEnd(nameNode.getEnd()-1);
// keep track of restricted identifiers for highlighting
if (isRecord || isSealed || isNonSealed || hasPermits) {
List<ASTNode> list = new ArrayList<>(4);
if (isSealed) list.add(sealedModifier.get());
if (isNonSealed) list.add(nonSealedModifier.get());
if (isRecord) list.add(configureAST(new ASTNode(), ctx.RECORD()));
if (hasPermits) list.add(configureAST(new ASTNode(), ctx.PERMITS()));
classNode.putNodeMetaData("special.keyword", Collections.unmodifiableList(list));
}
// GRECLIPSE end
classNode.setSyntheticPublic(syntheticPublic);
classNode.setGenericsTypes(this.visitTypeParameters(ctx.typeParameters()));
Expand Down Expand Up @@ -1661,12 +1666,8 @@ public ClassNode visitClassDeclaration(final ClassDeclarationContext ctx) {
ctx.classBody().putNodeMetaData(CLASS_DECLARATION_CLASS_NODE, classNode);
this.visitClassBody(ctx.classBody());
if (isRecord) {
Optional<FieldNode> fieldNodeOptional =
classNode.getFields().stream()
.filter(f -> !isTrue(f, IS_RECORD_GENERATED) && !f.isStatic()).findFirst();
if (fieldNodeOptional.isPresent()) {
createParsingFailedException("Instance field is not allowed in `record`", fieldNodeOptional.get());
}
classNode.getFields().stream().filter(f -> !isTrue(f, IS_RECORD_GENERATED) && !f.isStatic()).findFirst()
.ifPresent(fn -> createParsingFailedException("Instance field is not allowed in `record`", fn));
}
classNodeStack.pop();

Expand Down Expand Up @@ -2309,10 +2310,8 @@ private DeclarationListStatement createMultiAssignmentDeclarationListStatement(f
),
this.createGroovyTokenByType(ctx.ASSIGN().getSymbol(), Types.ASSIGN),
this.visitVariableInitializer(ctx.variableInitializer()));
Optional<ModifierNode> var = modifierManager.get(VAR);
if (var != null && var.isPresent()) {
de.setNodeMetaData("reserved.type.name", var.get());
}
modifierManager.get(VAR).ifPresent(var ->
de.setNodeMetaData("reserved.type.name", var));
configureAST(modifierManager.attachAnnotations(de), ctx);
return configureAST(new DeclarationListStatement(de), ctx);
// GRECLIPSE end
Expand Down Expand Up @@ -2343,15 +2342,11 @@ public DeclarationListStatement visitVariableDeclaration(final VariableDeclarati

declarationExpressionList.forEach(e -> {
VariableExpression variableExpression = (VariableExpression) e.getLeftExpression();

modifierManager.processVariableExpression(variableExpression);
modifierManager.attachAnnotations(e);
// GRECLIPSE add
Optional<ModifierNode> var = modifierManager.get(VAR);
if (var != null && var.isPresent()) {
e.setNodeMetaData("reserved.type.name", var.get());
}
modifierManager.get(VAR).ifPresent(var -> e.setNodeMetaData("reserved.type.name", var));
// GRECLIPSE end
modifierManager.processVariableExpression(variableExpression);
modifierManager.attachAnnotations(e);
});

int size = declarationExpressionList.size();
Expand Down Expand Up @@ -5475,7 +5470,6 @@ public List<DeclarationExpression> getDeclarationExpressions() {
private static final String SQ_STR = "'";
private static final String DQ_STR = "\"";
private static final String DOLLAR_SLASH_STR = "$/";
private static final String VAR_STR = "var";

private static final Map<String, String> QUOTATION_MAP = Maps.of(
DQ_STR, DQ_STR,
Expand Down

0 comments on commit 39183a5

Please sign in to comment.