diff --git a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java index 87a2e4865de..8a884b31e32 100644 --- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java @@ -142,7 +142,7 @@ private void checkNoStaticMethodWithSameSignatureAsNonStatic(final ClassNode nod if (parent != null) { result = parent.getDeclaredMethodsMap(); } else { - result = new HashMap(); + result = new HashMap<>(); } // add in unimplemented abstract methods from the interfaces ClassNodeUtils.addDeclaredMethodsFromInterfaces(node, result); @@ -187,11 +187,15 @@ private void checkNoStaticMethodWithSameSignatureAsNonStatic(final ClassNode nod private void checkInterfaceMethodVisibility(final ClassNode node) { if (!node.isInterface()) return; for (MethodNode method : node.getMethods()) { - if (method.isPrivate() && method.isAbstract()) { - addError("Method '" + method.getName() + "' from " + getDescription(node) + - " must not be private as it is declared as an abstract method.", method); - } else if (method.isProtected()) { - addError("Method '" + method.getName() + "' is protected but should be public in " + getDescription(currentClass) + ".", method); + if (!method.isPublic()) { + if (method.isAbstract()) { + addError("Method '" + method.getName() + "' from " + getDescription(node) + + " must be public as it is declared as an abstract method.", method); + } else if (!method.isPrivate() && !method.isStaticConstructor()) { + addError("Method '" + method.getName() + "' is " + + (method.isProtected() ? "protected" : "package-private") + + " but should be public or private in " + getDescription(node) + ".", method); + } } } } @@ -418,10 +422,6 @@ private void checkMethodsForIncorrectModifiers(final ClassNode cn) { addError("The " + getDescription(method) + " from " + getDescription(cn) + " must not be final. It is by definition abstract.", method); } - if (method.isStatic() && !method.isStaticConstructor()) { - addError("The " + getDescription(method) + " from " + getDescription(cn) + - " must not be static. Only fields may be static in an interface.", method); - } } } diff --git a/src/spec/test/ClassTest.groovy b/src/spec/test/ClassTest.groovy index 2d4f3146d3f..79c15414282 100644 --- a/src/spec/test/ClassTest.groovy +++ b/src/spec/test/ClassTest.groovy @@ -229,7 +229,7 @@ class ClassTest extends GroovyTestCase { } // end::protected_forbidden[] ''' - assert err.contains("Method 'greet' is protected but should be public in interface 'Greeter'") + assert err.contains("Method 'greet' from interface 'Greeter' must be public as it is declared as an abstract method.") /* err = shouldFail '''import groovy.transform.* // tag::package_private_forbidden[] diff --git a/src/test/groovy/AbstractClassAndInterfaceTest.groovy b/src/test/groovy/AbstractClassAndInterfaceTest.groovy index 85c254c8115..ad8ef85fc99 100644 --- a/src/test/groovy/AbstractClassAndInterfaceTest.groovy +++ b/src/test/groovy/AbstractClassAndInterfaceTest.groovy @@ -39,7 +39,7 @@ class AbstractClassAndInterfaceTest extends CompilableTestSupport { } } - def b = new B(); + def b = new B() return b.methodTwo() """ def retVal = shell.evaluate(text) @@ -105,7 +105,7 @@ class AbstractClassAndInterfaceTest extends CompilableTestSupport { void methodOne(Object o){assert true} } - def b = new B(); + def b = new B() return b.methodTwo() """ def retVal = shell.evaluate(text) @@ -321,6 +321,6 @@ class AbstractClassAndInterfaceTest extends CompilableTestSupport { private abstract void y() } """ - assert msg.contains("Method 'y' from interface 'X' must not be private as it is declared as an abstract method.") + assert msg.contains("Method 'y' from interface 'X' must be public as it is declared as an abstract method.") } } diff --git a/src/test/groovy/InterfaceTest.groovy b/src/test/groovy/InterfaceTest.groovy index b7ed01ee3f8..28620aafc77 100644 --- a/src/test/groovy/InterfaceTest.groovy +++ b/src/test/groovy/InterfaceTest.groovy @@ -91,4 +91,27 @@ final class InterfaceTest extends CompilableTestSupport { assert impl2.foo() == 'hello from Foo#foo' ''' } + + // GROOVY-11237 + void testStaticInterfaceMethod() { + assertScript ''' + import static groovy.test.GroovyAssert.shouldFail + + interface Foo { + static hello(where) { "hello $where" } + static String BAR = 'bar' + String BAA = 'baa' // implicit static + } + + assert Foo.hello('world') == 'hello world' + assert Foo.BAR == 'bar' + assert Foo.BAA == 'baa' + shouldFail(MissingMethodException) { + Foo.getBAR() + } + shouldFail(MissingMethodException) { + Foo.getBAA() + } + ''' + } } diff --git a/src/test/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java b/src/test/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java index d926bc705a2..758bb292caa 100644 --- a/src/test/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java +++ b/src/test/org/codehaus/groovy/classgen/ClassCompletionVerifierTest.java @@ -39,8 +39,6 @@ public class ClassCompletionVerifierTest extends TestSupport { "The interface '" + FINAL_INTERFACE + "' must not be final. It is by definition abstract."; private static final String EXPECTED_INTERFACE_FINAL_METHOD_ERROR_MESSAGE = "The method 'java.lang.Object xxx()' from interface 'zzz' must not be final. It is by definition abstract."; - private static final String EXPECTED_INTERFACE_STATIC_METHOD_ERROR_MESSAGE = - "The method 'java.lang.Object yyy()' from interface 'zzz' must not be static. Only fields may be static in an interface."; private static final String EXPECTED_TRANSIENT_CLASS_ERROR_MESSAGE = "The class 'DodgyClass' has an incorrect modifier transient."; /* can't check synchronized here as it doubles up with ACC_SUPER @@ -59,20 +57,8 @@ public class ClassCompletionVerifierTest extends TestSupport { private static final String EXPECTED_VOLATILE_METHOD_ERROR_MESSAGE = "The method 'java.lang.Object vo()' has an incorrect modifier volatile."; */ - private static final String EXPECTED_STRICT_METHOD_ERROR_MESSAGE = - "The method 'java.lang.Object st()' has an incorrect modifier strictfp."; - private static final String EXPECTED_NATIVE_METHOD_ERROR_MESSAGE = - "The method 'java.lang.Object na()' has an incorrect modifier native."; - private static final String EXPECTED_SYNCHRONIZED_METHOD_ERROR_MESSAGE = - "The method 'java.lang.Object sy()' has an incorrect modifier synchronized."; private static final String EXPECTED_TRANSIENT_METHOD_ERROR_MESSAGE = "The method 'java.lang.Object tr()' has an incorrect modifier transient."; - private static final String EXPECTED_PROTECTED_FIELD_ERROR_MESSAGE = - "The field 'prof' is not 'public static final' but is defined in interface 'zzz'."; - private static final String EXPECTED_PRIVATE_FIELD_ERROR_MESSAGE = - "The field 'prif' is not 'public static final' but is defined in interface 'zzz'."; - private static final String EXPECTED_PROTECTED_METHOD_ERROR_MESSAGE = - "Method 'prom' is protected but should be public in interface 'zzz'."; private static final String EXPECTED_ABSTRACT_PRIVATE_METHOD_ERROR_MESSAGE = "Method 'y' from class 'X' must not be private as it is declared as an abstract method."; @@ -127,44 +113,44 @@ public void testDetectsFinalAbstractInterface() throws Exception { checkErrorMessage(EXPECTED_INTERFACE_MODIFIER_ERROR_MESSAGE); } - public void testDetectsFinalAndStaticMethodsInInterface() throws Exception { + public void testDetectsFinalMethodsInInterface() throws Exception { 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)); - node.addMethod(new MethodNode("yyy", ACC_PUBLIC | ACC_STATIC, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null)); addDummyConstructor(node); verifier.visitClass(node); - checkErrorCount(2); + checkErrorCount(1); checkErrorMessage(EXPECTED_INTERFACE_FINAL_METHOD_ERROR_MESSAGE); - checkErrorMessage(EXPECTED_INTERFACE_STATIC_METHOD_ERROR_MESSAGE); } public void testDetectsIncorrectMethodModifiersInInterface() throws Exception { // can't check volatile here as it doubles up with bridge ClassNode node = new ClassNode("zzz", ACC_ABSTRACT | ACC_INTERFACE, 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)); - node.addMethod(new MethodNode("tr", ACC_TRANSIENT, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null)); + 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)); addDummyConstructor(node); verifier.visitClass(node); checkErrorCount(4); - checkErrorMessage(EXPECTED_STRICT_METHOD_ERROR_MESSAGE); - checkErrorMessage(EXPECTED_NATIVE_METHOD_ERROR_MESSAGE); - checkErrorMessage(EXPECTED_SYNCHRONIZED_METHOD_ERROR_MESSAGE); + checkErrorMessage("The method 'java.lang.Object st()' has an incorrect modifier strictfp."); + checkErrorMessage("The method 'java.lang.Object na()' has an incorrect modifier native."); + checkErrorMessage("The method 'java.lang.Object sy()' has an incorrect modifier synchronized."); checkErrorMessage(EXPECTED_TRANSIENT_METHOD_ERROR_MESSAGE); } public void testDetectsIncorrectMemberVisibilityInInterface() throws Exception { ClassNode node = new ClassNode("zzz", ACC_ABSTRACT | ACC_INTERFACE, ClassHelper.OBJECT_TYPE); node.addMethod(new MethodNode("prom", ACC_PROTECTED, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null)); + node.addMethod(new MethodNode("ppm", 0, ClassHelper.OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null)); node.addField("prif", ACC_PRIVATE, ClassHelper.OBJECT_TYPE, null); node.addField("prof", ACC_PROTECTED, ClassHelper.OBJECT_TYPE, null); addDummyConstructor(node); verifier.visitClass(node); - checkErrorCount(3); - checkErrorMessage(EXPECTED_PROTECTED_FIELD_ERROR_MESSAGE); - checkErrorMessage(EXPECTED_PRIVATE_FIELD_ERROR_MESSAGE); - checkErrorMessage(EXPECTED_PROTECTED_METHOD_ERROR_MESSAGE); + checkErrorCount(4); + checkErrorMessage("The field 'prof' is not 'public static final' but is defined in interface 'zzz'."); + checkErrorMessage("The field 'prif' is not 'public static final' but is defined in interface 'zzz'."); + checkErrorMessage("Method 'prom' is protected but should be public or private in interface 'zzz'."); + checkErrorMessage("Method 'ppm' is package-private but should be public or private in interface 'zzz'."); } public void testDetectsCorrectMethodModifiersInClass() throws Exception {