Details
-
Bug
-
Status: Closed
-
Minor
-
Resolution: Fixed
-
None
-
None
-
None
Description
If override method is package-private (via PackageScope) and super method is public (or protected?), no error is produced. If protected is indeed more visible than package-private, there may be some additional cases uncovered below.
org.codehaus.groovy.classgen.ClassCompletionVerifier:
private void checkMethodForWeakerAccessPrivileges(MethodNode mn, ClassNode cn) {
if (mn.isPublic()) return;
Parameter[] params = mn.getParameters();
for (MethodNode superMethod : cn.getSuperClass().getMethods(mn.getName())) {
Parameter[] superParams = superMethod.getParameters();
if (!hasEqualParameterTypes(params, superParams)) continue;
if ((mn.isPrivate() && !superMethod.isPrivate()) ||
(mn.isProtected() && superMethod.isPublic())) {
addWeakerAccessError(cn, mn, params, superMethod);
return;
}
}
}
I think that this is the proper set of checks:
if ((mn.isPrivate() && !superMethod.isPrivate()) ||
(mn.isProtected() && !superMethod.isProtected() && !superMethod.isPrivate()) ||
(!mn.isPrivate() && !mn.isProtected() && !mn.isPublic() && (superMethod.isPublic() || superMethod.isProtected()))) {
addWeakerAccessError(cn, mn, params, superMethod);
...
// also the error message need some change to indicate default/package-private
private void addWeakerAccessError(ClassNode cn, MethodNode method, Parameter[] parameters, MethodNode superMethod) {
StringBuilder msg = new StringBuilder();
msg.append(method.getName());
appendParamsDescription(parameters, msg);
msg.append(" in ");
msg.append(cn.getName());
msg.append(" cannot override ");
msg.append(superMethod.getName());
msg.append(" in ");
msg.append(superMethod.getDeclaringClass().getName());
msg.append("; attempting to assign weaker access privileges; was ");
// GRECLIPSE edit
//msg.append(superMethod.isPublic() ? "public" : "protected");
msg.append(superMethod.isPublic() ? "public" : (superMethod.isProtected() ? "protected" : "package-private"));
// GRECLIPSE end
addError(msg.toString(), method);
}
For these tests, 1b and 2a are failing:
@Test
public void testOverriding_ReducedVisibility1() {
runNegativeTest(new String[] {
"Bar.groovy",
"class Bar { public def baz() {} }\n",
"Foo.groovy",
"class Foo extends Bar { private def baz() {}\n }\n",
}, "----------\n" +
"1. ERROR in Foo.groovy (at line 1)\n" +
"\tclass Foo extends Bar { private def baz() {}\n" +
"\t ^^^^^^^^^^^^^^^^^^^^\n" +
"Groovy:baz() in Foo cannot override baz in Bar; attempting to assign weaker access privileges; was public\n" +
"----------\n");
}
@Test
public void testOverriding_ReducedVisibility1a() {
runNegativeTest(new String[] {
"Bar.groovy",
"class Bar { public def baz() {} }\n",
"Foo.groovy",
"class Foo extends Bar { protected def baz() {}\n }\n",
}, "----------\n" +
"1. ERROR in Foo.groovy (at line 1)\n" +
"\tclass Foo extends Bar { protected def baz() {}\n" +
"\t ^^^^^^^^^^^^^^^^^^^^^^\n" +
"Groovy:baz() in Foo cannot override baz in Bar; attempting to assign weaker access privileges; was public\n" +
"----------\n");
}
@Test
public void testOverriding_ReducedVisibility1b() {
runNegativeTest(new String[] {
"Bar.groovy",
"class Bar { public def baz() {} }\n",
"Foo.groovy",
"class Foo extends Bar { @groovy.transform.PackageScope def baz() {}\n }\n",
}, "----------\n" +
"1. ERROR in Foo.groovy (at line 1)\n" +
"\tclass Foo extends Bar { @groovy.transform.PackageScope def baz() {}\n" +
"\t ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
"Groovy:baz() in Foo cannot override baz in Bar; attempting to assign weaker access privileges; was public\n" +
"----------\n");
}
@Test
public void testOverriding_ReducedVisibility2() {
runNegativeTest(new String[] {
"Bar.groovy",
"class Bar { protected def baz() {} }\n",
"Foo.groovy",
"class Foo extends Bar { private def baz() {}\n }\n",
}, "----------\n" +
"1. ERROR in Foo.groovy (at line 1)\n" +
"\tclass Foo extends Bar { private def baz() {}\n" +
"\t ^^^^^^^^^^^^^^^^^^^^\n" +
"Groovy:baz() in Foo cannot override baz in Bar; attempting to assign weaker access privileges; was protected\n" +
"----------\n");
}
@Test
public void testOverriding_ReducedVisibility2a() {
runNegativeTest(new String[] {
"Bar.groovy",
"class Bar { protected def baz() {} }\n",
"Foo.groovy",
"class Foo extends Bar { @groovy.transform.PackageScope def baz() {}\n }\n",
}, "----------\n" +
"1. ERROR in Foo.groovy (at line 1)\n" +
"\tclass Foo extends Bar { @groovy.transform.PackageScope def baz() {}\n" +
"\t ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
"Groovy:baz() in Foo cannot override baz in Bar; attempting to assign weaker access privileges; was protected\n" +
"----------\n");
}
@Test
public void testOverriding_ReducedVisibility3() {
runNegativeTest(new String[] {
"Bar.groovy",
"class Bar { @groovy.transform.PackageScope def baz() {} }\n",
"Foo.groovy",
"class Foo extends Bar { private def baz() {}\n }\n",
}, "----------\n" +
"1. ERROR in Foo.groovy (at line 1)\n" +
"\tclass Foo extends Bar { private def baz() {}\n" +
"\t ^^^^^^^^^^^^^^^^^^^^\n" +
"Groovy:baz() in Foo cannot override baz in Bar; attempting to assign weaker access privileges; was protected\n" +
"----------\n");
}
@Test
public void testOverriding_ReducedVisibility3a() {
runNegativeTest(new String[] {
"Bar.java",
"public class Bar { Object baz() { return null; } }\n",
"Foo.groovy",
"class Foo extends Bar { private def baz() {}\n }\n",
}, "----------\n" +
"1. ERROR in Foo.groovy (at line 1)\n" +
"\tclass Foo extends Bar { private def baz() {}\n" +
"\t ^^^^^^^^^^^^^^^^^^^^\n" +
"Groovy:baz() in Foo cannot override baz in Bar; attempting to assign weaker access privileges; was package-private\n" +
"----------\n");
}