You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2022/01/17 04:20:47 UTC

[groovy] branch master updated: GROOVY-10451: Sealed classes incorrectly allow a self-reference in the permitted subclasses

This is an automated email from the ASF dual-hosted git repository.

paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new f72dcbe  GROOVY-10451: Sealed classes incorrectly allow a self-reference in the permitted subclasses
f72dcbe is described below

commit f72dcbe7942811b134a8ee964470f5451e8d6325
Author: Paul King <pa...@asert.com.au>
AuthorDate: Sun Jan 16 23:33:36 2022 +1000

    GROOVY-10451: Sealed classes incorrectly allow a self-reference in the permitted subclasses
---
 src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java |  2 ++
 .../codehaus/groovy/transform/SealedASTTransformation.java    | 10 ++++++++--
 .../org/codehaus/groovy/transform/SealedTransformTest.groovy  | 11 +++++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
index 690547c..ae248a8 100644
--- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -1555,6 +1555,8 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
                                 .map(ClassExpression::new)
                                 .collect(Collectors.toList()));
                 sealedAnnotationNode.setMember("permittedSubclasses", permittedSubclassesListExpression);
+                configureAST(sealedAnnotationNode, ctx.PERMITS());
+                sealedAnnotationNode.setNodeMetaData("permits", true);
             }
             classNode.addAnnotation(sealedAnnotationNode);
         } else if (isNonSealed) {
diff --git a/src/main/java/org/codehaus/groovy/transform/SealedASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/SealedASTTransformation.java
index 04fda04..31dee5e 100644
--- a/src/main/java/org/codehaus/groovy/transform/SealedASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/SealedASTTransformation.java
@@ -43,6 +43,7 @@ import static org.codehaus.groovy.ast.ClassHelper.make;
 public class SealedASTTransformation extends AbstractASTTransformation {
 
     private static final Class<?> SEALED_CLASS = Sealed.class;
+    private static final String SEALED_NAME = "@" + SEALED_CLASS.getSimpleName();
     private static final ClassNode SEALED_TYPE = make(SEALED_CLASS);
     private static final ClassNode SEALED_OPTIONS_TYPE = make(SealedOptions.class);
     public static final String SEALED_ALWAYS_ANNOTATE = "groovy.transform.SealedOptions.alwaysAnnotate";
@@ -57,11 +58,11 @@ public class SealedASTTransformation extends AbstractASTTransformation {
         if (parent instanceof ClassNode) {
             ClassNode cNode = (ClassNode) parent;
             if (cNode.isEnum()) {
-                addError("@" + SEALED_CLASS.getSimpleName() + " not allowed for enum", cNode);
+                addError(SEALED_NAME + " not allowed for enum", cNode);
                 return;
             }
             if (cNode.isAnnotationDefinition()) {
-                addError("@" + SEALED_CLASS.getSimpleName() + " not allowed for annotation definition", cNode);
+                addError(SEALED_NAME + " not allowed for annotation definition", cNode);
                 return;
             }
             cNode.putNodeMetaData(SEALED_CLASS, Boolean.TRUE);
@@ -89,6 +90,11 @@ public class SealedASTTransformation extends AbstractASTTransformation {
             }
             List<ClassNode> newSubclasses = getMemberClassList(anno, "permittedSubclasses");
             if (newSubclasses != null) {
+                newSubclasses.forEach(subnode -> {
+                    if (subnode.equals(cNode)) {
+                        addError("Illegal self-reference: a sealed class cannot have itself as a permitted subclass", anno);
+                    }
+                });
                 cNode.getPermittedSubclasses().addAll(newSubclasses);
             }
         }
diff --git a/src/test/org/codehaus/groovy/transform/SealedTransformTest.groovy b/src/test/org/codehaus/groovy/transform/SealedTransformTest.groovy
index b375a8b..7e23073 100644
--- a/src/test/org/codehaus/groovy/transform/SealedTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/SealedTransformTest.groovy
@@ -222,4 +222,15 @@ class SealedTransformTest {
         ''')
         assert shapeClass.getAnnotation(Sealed) == null
     }
+
+    @Test
+    void testSealedSelfReference() {
+        def expected = "Illegal self-reference: a sealed class cannot have itself as a permitted subclass"
+        assert shouldFail(MultipleCompilationErrorsException, '''
+            @groovy.transform.Sealed(permittedSubclasses=Shape) class Shape { }
+        ''').message.contains(expected)
+        assert shouldFail(MultipleCompilationErrorsException, '''
+            sealed class Other permits Other { }
+        ''').message.contains(expected)
+    }
 }