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 2021/07/28 13:57:06 UTC
[groovy] branch groovy10148 updated: GROOVY-10148: Groovy should
not allow classes to extend sealed Java classes
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch groovy10148
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/groovy10148 by this push:
new d79b925 GROOVY-10148: Groovy should not allow classes to extend sealed Java classes
d79b925 is described below
commit d79b925b072e51d299df552e9ab146fa1c6dd4b6
Author: Paul King <pa...@asert.com.au>
AuthorDate: Wed Jul 28 23:56:54 2021 +1000
GROOVY-10148: Groovy should not allow classes to extend sealed Java classes
---
.../java/org/codehaus/groovy/ast/ClassNode.java | 2 +-
.../groovy/classgen/ClassCompletionVerifier.java | 55 +++++++----
.../org/codehaus/groovy/classgen/Verifier.java | 6 ++
.../ASTTransformationCollectorCodeVisitor.java | 2 +-
src/spec/doc/_sealed.adoc | 85 +++++++++++++++++
src/spec/doc/core-object-orientation.adoc | 1 +
src/spec/test/SealedSpecificationTest.groovy | 104 +++++++++++++++++++++
.../groovy/transform/SealedTransformTest.groovy | 42 ++++++++-
8 files changed, 274 insertions(+), 23 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/ClassNode.java b/src/main/java/org/codehaus/groovy/ast/ClassNode.java
index 2c9b75c..4b7091b 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassNode.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassNode.java
@@ -245,7 +245,7 @@ public class ClassNode extends AnnotatedNode {
}
public List<ClassNode> getPermittedSubclasses() {
- return permittedSubclasses;
+ return redirect().permittedSubclasses;
}
/**
diff --git a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index 66bc784..bb974fc 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -48,6 +48,7 @@ import org.codehaus.groovy.control.SourceUnit;
import org.codehaus.groovy.syntax.Types;
import org.codehaus.groovy.transform.trait.Traits;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
@@ -325,12 +326,13 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport {
}
boolean nonSealed = Boolean.TRUE.equals(cn.getNodeMetaData(NonSealed.class));
ClassNode superCN = cn.getSuperClass();
- if (nonSealed && (superCN == null || !superCN.isSealed())) {
+ boolean sealedSuper = superCN != null && superCN.isSealed();
+ boolean sealedInterface = Arrays.stream(cn.getInterfaces()).anyMatch(ClassNode::isSealed);
+ if (nonSealed && !(sealedSuper || sealedInterface)) {
addError("The " + getDescription(cn) + " cannot be non-sealed as it has no sealed parent.", cn);
return;
}
- if (superCN == null) return;
- if (superCN.isSealed()) {
+ if (sealedSuper || sealedInterface) {
if (sealed && nonSealed) {
addError("The " + getDescription(cn) + " cannot be both sealed and non-sealed.", cn);
return;
@@ -339,28 +341,39 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport {
addError("The " + getDescription(cn) + " cannot be both final and non-sealed.", cn);
return;
}
- List<ClassNode> permittedSubclasses = superCN.getPermittedSubclasses();
- boolean found = false;
- for (ClassNode permitted : permittedSubclasses) {
- if (permitted.equals(cn)) {
- found = true;
- break;
- }
- }
- if (!found) {
- addError("The " + getDescription(cn) + " is not a permitted subclass of the sealed " + getDescription(superCN) + ".", cn);
- return;
+ if (sealedSuper) {
+ checkSealedParent(cn, superCN, isFinal, nonSealed);
}
- boolean explicitlyMarked = nonSealed || cn.isSealed() || isFinal;
- if (!explicitlyMarked) {
- addError("The " + getDescription(cn) + " being a child of sealed " + getDescription(superCN) + " must be marked final, sealed, or non-sealed.", cn);
- return;
+ if (sealedInterface) {
+ for (ClassNode candidate : cn.getInterfaces()) {
+ if (candidate.isSealed()) {
+ checkSealedParent(cn, candidate, isFinal, nonSealed);
+ }
+ }
}
}
- if (!isFinal(superCN.getModifiers())) return;
+ if (superCN == null || !isFinal(superCN.getModifiers())) return;
addError("You are not allowed to extend the final " + getDescription(superCN) + ".", cn);
}
+ private void checkSealedParent(ClassNode cn, ClassNode parent, boolean isFinal, boolean nonSealed) {
+ boolean found = false;
+ for (ClassNode permitted : parent.getPermittedSubclasses()) {
+ if (permitted.equals(cn)) {
+ found = true;
+ break;
+ }
+ }
+ if (!found) {
+ addError("The " + getDescription(cn) + " is not a permitted subclass of the sealed " + getDescription(parent) + ".", cn);
+ return;
+ }
+ boolean explicitlyMarked = nonSealed || cn.isSealed() || isFinal;
+ if (!explicitlyMarked) {
+ addError("The " + getDescription(cn) + " being a child of sealed " + getDescription(parent) + " must be marked final, sealed, or non-sealed.", cn);
+ }
+ }
+
private void checkImplementsAndExtends(ClassNode node) {
ClassNode sn = node.getSuperClass();
if (sn.isInterface() && !node.isInterface()) {
@@ -370,7 +383,9 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport {
if (!anInterface.isInterface()) {
addError("You are not allowed to implement the " + getDescription(anInterface) + ", use extends instead.", node);
} else if (anInterface.isSealed()) {
- addError("You are not allowed to implement the sealed " + getDescription(anInterface) + ".", node);
+ boolean nonSealed = Boolean.TRUE.equals(node.getNodeMetaData(NonSealed.class));
+ boolean isFinal = isFinal(node.getModifiers());
+ checkSealedParent(node, anInterface, isFinal, nonSealed);
}
}
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index 8fb4e55..758a3fa 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -231,6 +231,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
if (classNode.getNodeMetaData(ClassNodeSkip.class) == null) {
classNode.setNodeMetaData(ClassNodeSkip.class, true);
}
+ addDetectedSealedClasses(node);
return;
}
@@ -283,6 +284,11 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
if (possibleSubclass.getSuperClass().equals(node)) {
permitted.add(possibleSubclass);
}
+ for (ClassNode iface : possibleSubclass.getInterfaces()) {
+ if (iface.equals(node)) {
+ permitted.add(possibleSubclass);
+ }
+ }
}
List<Expression> names = new ArrayList<>();
for (ClassNode next : permitted) {
diff --git a/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java b/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java
index 8638b72..3b578dc 100644
--- a/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java
@@ -294,7 +294,7 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
source.getErrorCollector().addError(new SimpleMessage(error, source));
}
- if (!Traits.isTrait(classNode) || transformClass == TraitASTTransformation.class) {
+ if (!Traits.isTrait(classNode) || transformClass == TraitASTTransformation.class || transformClass == SealedASTTransformation.class) {
classNode.addTransform((Class<? extends ASTTransformation>) transformClass, annotation);
}
}
diff --git a/src/spec/doc/_sealed.adoc b/src/spec/doc/_sealed.adoc
new file mode 100644
index 0000000..c3c3776
--- /dev/null
+++ b/src/spec/doc/_sealed.adoc
@@ -0,0 +1,85 @@
+//////////////////////////////////////////
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+//////////////////////////////////////////
+
+= Sealed hierarchies
+
+Sealed classes, interfaces and traits restrict which subclasses can extend/implement them.
+A final class allows no extension. A public non-final class allows extension by anyone.
+Access modifiers like protected and package-private give some ability to restrict inheritance
+hierarchies but often at the expense of flexible use of those hierarchies.
+
+Sealed hierarchies provide full inheritance within a known hierarchy of classes, interfaces and traits
+but disable or only provide controlled inheritance outside the hierarchy.
+
+For example, suppose we want to create a shape hierarchy containing
+only circles and squares. We also want a shape interface to
+be able to refer to instances in our hierarchy. We can create the
+hierarchy as follows:
+
+[source,groovy]
+----
+include::../test/SealedSpecificationTest.groovy[tags=simple_interface,indent=0]
+----
+
+We can have a reference of type `ShapeI` which can point to either a `Circle` or `Square`
+and, since our classes are `final`, we know no additional classes will be added to our hierarchy in the future.
+At least not without changing the `permittedSubclasses` and recompiling.
+
+In general, we might want to have some parts of our class hierarchy
+immediately locked down like we have here, where we marked the
+subclasses as `final` but other times we might want to allow further
+controlled inheritance.
+
+[source,groovy]
+----
+include::../test/SealedSpecificationTest.groovy[tags=general_sealed_class,indent=0]
+----
+
+In this example, our permitted subclasses for `Shape` are `Circle`, `Polygon`, and `Rectangle`.
+`Circle` is `final` and hence that part of the hierarchy cannot be extended.
+`Polygon` is marked as `@NonSealed` and that means our heiarchy is open to any further extension
+by subclassing `Polygon` as seen for `Pentagon`.
+`Rectangle` is itself sealed which means that part of the hierarchy can be extended
+but only in a controlled way.
+
+Sealed classes are useful for creating enum-like related classes
+which need to contain instance specific data. For instance, we might have the following enum:
+
+[source,groovy]
+----
+include::../test/SealedSpecificationTest.groovy[tags=weather_enum,indent=0]
+----
+
+but we now wish to also add weather specific instance data to weather forecasts.
+We can alter our abstraction as follows:
+
+[source,groovy]
+----
+include::../test/SealedSpecificationTest.groovy[tags=weather_sealed,indent=0]
+----
+
+Sealed hierarchies are also useful when specifying Algebraic or Abstract Data Types (ADTs) as shown in the following example:
+
+[source,groovy]
+----
+include::../test/SealedSpecificationTest.groovy[tags=sealed_ADT,indent=0]
+----
+
diff --git a/src/spec/doc/core-object-orientation.adoc b/src/spec/doc/core-object-orientation.adoc
index 60e7740..5bcbacc 100644
--- a/src/spec/doc/core-object-orientation.adoc
+++ b/src/spec/doc/core-object-orientation.adoc
@@ -1235,3 +1235,4 @@ single one corresponding to `@CompileStatic(TypeCheckingMode.SKIP)`.
include::_traits.adoc[leveloffset=+1]
+include::_sealed.adoc[leveloffset=+1]
diff --git a/src/spec/test/SealedSpecificationTest.groovy b/src/spec/test/SealedSpecificationTest.groovy
new file mode 100644
index 0000000..6f3a137
--- /dev/null
+++ b/src/spec/test/SealedSpecificationTest.groovy
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+import groovy.test.GroovyTestCase
+
+/**
+ * Specification tests for the traits feature
+ */
+class SealedSpecificationTest extends GroovyTestCase {
+
+ void testSealedADT() {
+ assertScript '''
+// tag::sealed_ADT[]
+import groovy.transform.*
+
+@Sealed interface Tree<T> {}
+@Singleton final class Empty implements Tree {
+ String toString() { 'Empty' }
+}
+@Canonical final class Node<T> implements Tree<T> {
+ T value
+ Tree<T> left, right
+}
+
+Tree<Integer> tree = new Node<>(42, new Node<>(0, Empty.instance, Empty.instance), Empty.instance)
+assert tree.toString() == 'Node(42, Node(0, Empty, Empty), Empty)'
+// end::sealed_ADT[]
+'''
+ }
+
+ void testSimpleSealedHierarchyInterfaces() {
+ assertScript '''
+import groovy.transform.Sealed
+
+// tag::simple_interface[]
+@Sealed(permittedSubclasses='Circle,Square') interface ShapeI { }
+final class Circle implements ShapeI { }
+final class Square implements ShapeI { }
+// end::simple_interface[]
+
+assert [new Circle(), new Square()]*.class.name == ['Circle', 'Square']
+'''
+ }
+
+ void testSimpleSealedHierarchyClasses() {
+ assertScript '''
+import groovy.transform.Sealed
+import groovy.transform.NonSealed
+
+// tag::general_sealed_class[]
+@Sealed(permittedSubclasses='Circle,Polygon,Rectangle') class Shape { }
+final class Circle extends Shape { }
+@NonSealed class Polygon extends Shape { }
+final class Pentagon extends Polygon { }
+@Sealed(permittedSubclasses='Square') class Rectangle extends Shape { }
+final class Square extends Rectangle { }
+// end::general_sealed_class[]
+
+assert [new Circle(), new Square()]*.class.name == ['Circle', 'Square']
+'''
+ }
+
+ void testEnum() {
+ assertScript '''
+// tag::weather_enum[]
+enum Weather { Rainy, Cloudy, Sunny }
+def forecast = [Weather.Rainy, Weather.Sunny, Weather.Cloudy]
+assert forecast.toString() == '[Rainy, Sunny, Cloudy]'
+// end::weather_enum[]
+'''
+ }
+
+ void testSealedWeather() {
+ assertScript '''
+import groovy.transform.*
+
+// tag::weather_sealed[]
+@Sealed abstract class Weather { }
+@Immutable(includeNames=true) class Rainy extends Weather { Integer expectedRainfall }
+@Immutable(includeNames=true) class Sunny extends Weather { Integer expectedTemp }
+@Immutable(includeNames=true) class Cloudy extends Weather { Integer expectedUV }
+def forecast = [new Rainy(12), new Sunny(35), new Cloudy(6)]
+assert forecast.toString() == '[Rainy(expectedRainfall:12), Sunny(expectedTemp:35), Cloudy(expectedUV:6)]'
+// end::weather_sealed[]
+'''
+ }
+}
diff --git a/src/test/org/codehaus/groovy/transform/SealedTransformTest.groovy b/src/test/org/codehaus/groovy/transform/SealedTransformTest.groovy
index 6e9c74d..6c904bd 100644
--- a/src/test/org/codehaus/groovy/transform/SealedTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/SealedTransformTest.groovy
@@ -44,6 +44,19 @@ class SealedTransformTest {
}
@Test
+ void testSimpleSealedHierarchyTraits() {
+ assertScript '''
+ import groovy.transform.Sealed
+
+ @Sealed(permittedSubclasses='Diamond,Star') trait ShapeT { }
+ final class Diamond implements ShapeT { }
+ final class Star implements ShapeT { }
+
+ assert [new Diamond(), new Star()]*.class.name == ['Diamond', 'Star']
+ '''
+ }
+
+ @Test
void testSealedChildMustBeMarked() {
assert shouldFail(MultipleCompilationErrorsException, '''
import groovy.transform.Sealed
@@ -107,7 +120,7 @@ class SealedTransformTest {
}
@Test
- void testDetectedPermittedClasses() {
+ void testInferredPermittedAuxiliaryClasses() {
// If the base class and all subclasses appear in the same source file, the
// permittedSubclasses list will be automatically completed if not specified explicitly.
// If an explicit list is given, it must be the complete list and won't
@@ -121,4 +134,31 @@ class SealedTransformTest {
Shape.getAnnotation(Sealed).permittedSubclasses()
''') == ['Square', 'Circle']
}
+
+ @Test
+ void testInferredPermittedAuxiliaryInterfaces() {
+ assert new GroovyShell().evaluate('''
+ import groovy.transform.Sealed
+
+ @Sealed interface Shape { }
+ @Sealed interface Polygon extends Shape { }
+ final class Circle implements Shape { }
+ final class Rectangle implements Polygon { }
+ [Shape.getAnnotation(Sealed).permittedSubclasses(),
+ Polygon.getAnnotation(Sealed).permittedSubclasses()]
+ ''') == [['Polygon', 'Circle'], ['Rectangle']]
+ }
+
+ @Test
+ void testInferredPermittedNestedClasses() {
+ assert new GroovyShell().evaluate('''
+ import groovy.transform.Sealed
+
+ @Sealed class Shape {
+ final class Triangle extends Shape { }
+ final class Polygon extends Shape { }
+ }
+ Shape.getAnnotation(Sealed).permittedSubclasses()
+ ''') == ['Shape$Triangle', 'Shape$Polygon']
+ }
}