You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2024/02/27 16:15:11 UTC
(groovy) branch master updated: fix extends interface verification
This is an automated email from the ASF dual-hosted git repository.
emilles 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 d58386b31a fix extends interface verification
d58386b31a is described below
commit d58386b31a57779225f088ee5bf39f5a2949fd36
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Feb 27 09:53:50 2024 -0600
fix extends interface verification
---
.../groovy/classgen/ClassCompletionVerifier.java | 20 ++--
.../classgen/ClassCompletionVerifierTest.java | 129 ++++++++++++++-------
2 files changed, 97 insertions(+), 52 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index 417b34ee47..be87e1df61 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -378,15 +378,17 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport {
}
private void checkImplementsAndExtends(final ClassNode node) {
- ClassNode sn = node.getSuperClass();
- if (sn != null && sn.isInterface() && !node.isInterface()) {
- addError("You are not allowed to extend the " + getDescription(sn) + ", use implements instead.", node);
- }
- for (ClassNode anInterface : node.getInterfaces()) {
- if (!anInterface.isInterface()) {
- addError("You are not allowed to implement the " + getDescription(anInterface) + ", use extends instead.", node);
- } else if (anInterface.isSealed()) {
- checkSealedParent(node, anInterface);
+ if (!node.isInterface()) {
+ ClassNode type = node.getUnresolvedSuperClass();
+ if (type != null && type.isInterface()) {
+ addError("You are not allowed to extend the " + getDescription(type) + ", use implements instead.", node);
+ }
+ }
+ for (ClassNode type : node.getInterfaces()) {
+ if (!type.isInterface()) {
+ addError("You are not allowed to implement the " + getDescription(type) + ", use extends instead.", node);
+ } else if (type.isSealed()) {
+ checkSealedParent(node, type);
}
}
}
diff --git a/src/test/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java b/src/test/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java
index 24be768e9d..fa2aa61ba8 100644
--- a/src/test/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java
+++ b/src/test/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java
@@ -20,51 +20,62 @@ package org.codehaus.groovy.classgen;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
-import org.codehaus.groovy.ast.FieldNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.control.SourceUnit;
+import org.junit.Test;
import java.io.PrintWriter;
import java.io.StringWriter;
import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
-
-public final class ClassCompletionVerifierTest extends TestSupport {
-
- private SourceUnit source;
- private ClassCompletionVerifier verifier;
-
- @Override
- protected void setUp() throws Exception {
- super.setUp();
- source = SourceUnit.create("dummy.groovy", "");
- verifier = new ClassCompletionVerifier(source);
- }
-
- public void testDetectsAbstractPrivateMethod() {
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.objectweb.asm.Opcodes.ACC_ABSTRACT;
+import static org.objectweb.asm.Opcodes.ACC_FINAL;
+import static org.objectweb.asm.Opcodes.ACC_INTERFACE;
+import static org.objectweb.asm.Opcodes.ACC_NATIVE;
+import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
+import static org.objectweb.asm.Opcodes.ACC_PROTECTED;
+import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
+import static org.objectweb.asm.Opcodes.ACC_STATIC;
+import static org.objectweb.asm.Opcodes.ACC_STRICT;
+import static org.objectweb.asm.Opcodes.ACC_SYNCHRONIZED;
+import static org.objectweb.asm.Opcodes.ACC_TRANSIENT;
+import static org.objectweb.asm.Opcodes.ACC_VOLATILE;
+
+public final class ClassCompletionVerifierTest {
+
+ private SourceUnit source = SourceUnit.create("dummy.groovy", "");
+ private ClassCompletionVerifier verifier = new ClassCompletionVerifier(source);
+
+ @Test
+ public void shouldDetectAbstractPrivateMethod() {
ClassNode node = new ClassNode("X", ACC_ABSTRACT, ClassHelper.OBJECT_TYPE);
node.addMethod(new MethodNode("y", ACC_PRIVATE | ACC_ABSTRACT, ClassHelper.VOID_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
verifier.visitClass(node);
checkErrorMessage("The method 'y' must not be private as it is declared abstract in class 'X'.");
}
- public void testDetectsFinalAbstractClass() {
+ @Test
+ public void shouldDetectFinalAbstractClass() {
checkVisitErrors("FinalClass", ACC_FINAL, false);
checkVisitErrors("AbstractClass", ACC_ABSTRACT, false);
checkVisitErrors("AbstractFinalClass", ACC_ABSTRACT | ACC_FINAL, true);
checkErrorMessage("The class 'AbstractFinalClass' must not be both final and abstract.");
}
- public void testDetectsDuplicateMethodsForClassNoParams() {
- checkDetectsDuplicateMethods(ACC_ABSTRACT, "Repetitive method name/signature for method 'java.lang.Object xxx()' in class 'zzz'.");
+ @Test
+ public void shouldDetectDuplicateMethodsForClassNoParams() {
+ tryDetectDuplicateMethods(ACC_ABSTRACT, "Repetitive method name/signature for method 'java.lang.Object xxx()' in class 'zzz'.");
}
- public void testDetectsDuplicateMethodsForInterfaceOneParam() {
- checkDetectsDuplicateMethods(ACC_INTERFACE, "Repetitive method name/signature for method 'java.lang.Object xxx(java.lang.String)' in interface 'zzz'.", new Parameter(ClassHelper.STRING_TYPE, "x"));
+ @Test
+ public void shouldDetectDuplicateMethodsForInterfaceOneParam() {
+ tryDetectDuplicateMethods(ACC_INTERFACE, "Repetitive method name/signature for method 'java.lang.Object xxx(java.lang.String)' in interface 'zzz'.", new Parameter(ClassHelper.STRING_TYPE, "x"));
}
- private void checkDetectsDuplicateMethods(int modifiers, String expectedErrorMessage, Parameter... params) {
+ private void tryDetectDuplicateMethods(int modifiers, String expectedErrorMessage, Parameter... params) {
ClassNode node = new ClassNode("zzz", modifiers, ClassHelper.OBJECT_TYPE);
node.addMethod(new MethodNode("xxx", ACC_PUBLIC, ClassHelper.OBJECT_TYPE, params, ClassNode.EMPTY_ARRAY, null));
node.addMethod(new MethodNode("xxx", ACC_PUBLIC, ClassHelper.OBJECT_TYPE, params, ClassNode.EMPTY_ARRAY, null));
@@ -73,7 +84,8 @@ public final class ClassCompletionVerifierTest extends TestSupport {
checkErrorMessage(expectedErrorMessage);
}
- public void testDetectsIncorrectOtherModifier() {
+ @Test
+ public void shouldDetectIncorrectOtherModifier() {
// can't check synchronized here as it doubles up with ACC_SUPER
checkVisitErrors("DodgyClass", ACC_TRANSIENT | ACC_VOLATILE | ACC_NATIVE, true);
checkErrorMessage("The class 'DodgyClass' has an incorrect modifier transient.");
@@ -81,28 +93,31 @@ public final class ClassCompletionVerifierTest extends TestSupport {
checkErrorMessage("The class 'DodgyClass' has an incorrect modifier native.");
}
- public void testDetectsFinalAbstractInterface() {
+ @Test
+ public void shouldDetectFinalAbstractInterface() {
checkVisitErrors("FinalInterface", ACC_ABSTRACT | ACC_FINAL | ACC_INTERFACE, true);
checkErrorMessage("The interface 'FinalInterface' must not be final. It is by definition abstract.");
}
- public void testDetectsFinalMethodsInInterface() {
+ @Test
+ public void shouldDetectFinalMethodsInInterface() {
ClassNode node = new ClassNode("zzz", ACC_ABSTRACT | ACC_INTERFACE, ClassHelper.OBJECT_TYPE);
node.addMethod(new MethodNode("xxx", ACC_PUBLIC | ACC_FINAL, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
- addDummyConstructor(node);
+ addStaticConstructor(node);
verifier.visitClass(node);
checkErrorCount(1);
checkErrorMessage("The method 'java.lang.Object xxx()' from interface 'zzz' must not be final. It is by definition abstract.");
}
- public void testDetectsIncorrectMethodModifiersInInterface() {
+ @Test
+ public void shouldDetectIncorrectMethodModifiersInInterface() {
ClassNode node = new ClassNode("zzz", ACC_ABSTRACT | ACC_INTERFACE, ClassHelper.OBJECT_TYPE);
node.addMethod(new MethodNode("st", ACC_PUBLIC | ACC_STRICT , ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
node.addMethod(new MethodNode("na", ACC_PUBLIC | ACC_NATIVE , ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
node.addMethod(new MethodNode("sy", ACC_PUBLIC | ACC_SYNCHRONIZED , ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
node.addMethod(new MethodNode("tr", ACC_PUBLIC | ACC_TRANSIENT , ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
// can't check volatile here as it doubles up with bridge
- addDummyConstructor(node);
+ addStaticConstructor(node);
verifier.visitClass(node);
checkErrorCount(4);
checkErrorMessage("The method 'java.lang.Object st()' has an incorrect modifier strictfp.");
@@ -111,7 +126,8 @@ public final class ClassCompletionVerifierTest extends TestSupport {
checkErrorMessage("The method 'java.lang.Object tr()' has an incorrect modifier transient.");
}
- public void testDetectsIncorrectMemberVisibilityInInterface() {
+ @Test
+ public void shouldDetectIncorrectMemberVisibilityInInterface() {
ClassNode node = new ClassNode("zzz", ACC_ABSTRACT | ACC_INTERFACE, ClassHelper.OBJECT_TYPE);
node.addMethod(new MethodNode("pro1", ACC_ABSTRACT | ACC_PROTECTED, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
node.addMethod(new MethodNode("pro2", ACC_STATIC | ACC_PROTECTED, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
@@ -121,7 +137,7 @@ public final class ClassCompletionVerifierTest extends TestSupport {
node.addField("prof", ACC_PROTECTED, ClassHelper.OBJECT_TYPE, null);
node.addField("prif", ACC_PRIVATE , ClassHelper.OBJECT_TYPE, null);
node.addField("pp_f", 0 , ClassHelper.OBJECT_TYPE, null);
- addDummyConstructor(node);
+ addStaticConstructor(node);
verifier.visitClass(node);
checkErrorCount(8);
checkErrorMessage("The field 'prof' is not 'public static final' but is defined in interface 'zzz'");
@@ -134,39 +150,66 @@ public final class ClassCompletionVerifierTest extends TestSupport {
checkErrorMessage("The method 'pp_2' is package-private but must be public or private in interface 'zzz'");
}
- public void testDetectsCorrectMethodModifiersInClass() {
+ @Test
+ public void shouldDetectCorrectMethodModifiersInClass() {
// can't check volatile here as it doubles up with bridge
ClassNode node = new ClassNode("zzz", ACC_PUBLIC, ClassHelper.OBJECT_TYPE);
node.addMethod(new MethodNode("st", ACC_STRICT, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
node.addMethod(new MethodNode("na", ACC_NATIVE, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
node.addMethod(new MethodNode("sy", ACC_SYNCHRONIZED, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
- addDummyConstructor(node);
+ addStaticConstructor(node);
verifier.visitClass(node);
checkErrorCount(0);
}
- public void testDetectsIncorrectMethodModifiersInClass() {
+ @Test
+ public void shouldDetectIncorrectMethodModifiersInClass() {
// can't check volatile here as it doubles up with bridge
ClassNode node = new ClassNode("zzz", ACC_PUBLIC, ClassHelper.OBJECT_TYPE);
node.addMethod(new MethodNode("tr", ACC_TRANSIENT, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
- addDummyConstructor(node);
+ addStaticConstructor(node);
verifier.visitClass(node);
checkErrorCount(1);
checkErrorMessage("The method 'java.lang.Object tr()' has an incorrect modifier transient.");
}
- public void testDetectsInvalidFieldModifiers() {
- ClassNode foo = new ClassNode("foo", ACC_PUBLIC, ClassHelper.OBJECT_TYPE);
- FieldNode bar = new FieldNode("bar", (ACC_FINAL | ACC_VOLATILE), ClassHelper.STRING_TYPE, foo, null);
- foo.addField(bar);
- verifier.visitClass(foo);
+ @Test
+ public void shouldDetectInvalidFieldModifiers() {
+ ClassNode node = new ClassNode("foo", ACC_PUBLIC, ClassHelper.OBJECT_TYPE);
+ node.addField("bar", ACC_FINAL | ACC_VOLATILE, ClassHelper.STRING_TYPE, null);
+ verifier.visitClass(node);
checkErrorCount(1);
checkErrorMessage("Illegal combination of modifiers, final and volatile, for field 'bar'");
}
+ @Test
+ public void shouldDetectClassExtendsInterface() {
+ ClassNode node = new ClassNode("C", ACC_PUBLIC, ClassHelper.SERIALIZABLE_TYPE);
+ verifier.visitClass(node);
+ checkErrorCount(1);
+ checkErrorMessage("You are not allowed to extend the interface 'java.io.Serializable', use implements instead.");
+ }
+
+ @Test
+ public void shouldDetectClassImplementsNonInterface() {
+ ClassNode[] impl = {ClassHelper.OBJECT_TYPE, ClassHelper.SERIALIZABLE_TYPE, ClassHelper.ELEMENT_TYPE_TYPE};
+ ClassNode node = new ClassNode("C", ACC_PUBLIC, ClassHelper.OBJECT_TYPE, impl, null);
+ verifier.visitClass(node);
+ checkErrorCount(2);
+ checkErrorMessage("You are not allowed to implement the class 'java.lang.Object', use extends instead.");
+ checkErrorMessage("You are not allowed to implement the enum 'java.lang.annotation.ElementType', use extends instead.");
+ }
+
+ @Test
+ public void shouldAcceptInterfaceExtendsInterface() {
+ ClassNode node = new ClassNode("I", ACC_PUBLIC | ACC_INTERFACE, ClassHelper.LIST_TYPE.getPlainNodeReference());
+ verifier.visitClass(node);
+ checkErrorCount(0);
+ }
+
//--------------------------------------------------------------------------
- private void addDummyConstructor(ClassNode node) {
+ private void addStaticConstructor(ClassNode node) {
// constructors should not be treated as errors (they have no real meaning for interfaces anyway)
node.addMethod(new MethodNode("<clinit>", ACC_PUBLIC | ACC_STATIC, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null));
}
@@ -197,11 +240,11 @@ public final class ClassCompletionVerifierTest extends TestSupport {
private String flattenErrorMessage() {
StringWriter stringWriter = new StringWriter();
- PrintWriter writer = new PrintWriter(stringWriter, true);
- for (int i = source.getErrorCollector().getErrorCount() - 1; i >= 0; i--) {
- source.getErrorCollector().getError(i).write(writer);
+ try (PrintWriter printWriter = new PrintWriter(stringWriter, true)) {
+ for (int i = source.getErrorCollector().getErrorCount() - 1; i >= 0; i -= 1) {
+ source.getErrorCollector().getError(i).write(printWriter);
+ }
}
- writer.close();
return stringWriter.toString();
}
}