[ https://issues.apache.org/jira/browse/GROOVY-8651?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Paul King closed GROOVY-8651. ----------------------------- > Method override weaker access check does not fully account for > package-private visibility > ----------------------------------------------------------------------------------------- > > Key: GROOVY-8651 > URL: https://issues.apache.org/jira/browse/GROOVY-8651 > Project: Groovy > Issue Type: Bug > Reporter: Eric Milles > Assignee: Paul King > Priority: Minor > Fix For: 3.0.0-beta-1, 2.5.7 > > > 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: > {code:java} > 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; > } > } > } > {code} > I think that this is the proper set of checks: > {code:java} > 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); > } > {code} > For these tests, 1b and 2a are failing: > {code:java} > @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"); > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)