GROOVY-9282: Allow protected override of package scoped methods

[gesellix]

See GROOVY-9282 for the Jira issue.

When overriding a method as protected with a super method being package-private (no modifier in Java or @PackageScope in Groovy), compilation fails with an error similar to this:

...
exampleMethod(java.lang.Object -> java.lang.Object) in com.example.Child cannot override exampleMethod in com.example.Base; attempting to assign weaker access privileges; was package-private
...

This edge case seems to be missed at GROOVY-8651.

Groovy 2.5.7 and 2.5.8 are affected. I didn't check Groovy 3.x.

Signed-off-by: Tobias Gesellchen tobias@gesellix.de

[daniellansun]

package-private methods can only be accessed by classes in the same package, so the improvement violates the rule of JLS.

[gesellix]

Would that be another check in another verifier? Otherwise I would extend the check in the current method we're looking at. Do you have any preference?

[daniellansun]

It seems that no more checks are required. Here is the rule Java obeys, groovy should obey too:

If a method is declared with default access (that is, not private, protected, nor public), it can only be overridden by methods in the same package. If a method of the same signature is defined in a subclass in a different package, it is a completely separate method and no overriding occurs.

[daniellansun]

Could you add more test cases to cover the following rule:

If a method of the same signature is defined in a subclass in a different package, it is a completely separate method and no overriding occurs.

[gesellix]

Due to Groovy's handling (some would say "ignorance" :) ) of private visibility at fields and methods I'm unsure how to provide such test cases. See https://issues.apache.org/jira/browse/GROOVY-3010 for a collection of tasks regarding that topic.

Would you mind if we consider that scenario unrelated to the current pull request or maybe can you provide a rough idea how such a test case should look like?

[daniellansun]

Here is the test case in Java to show the Java rule, i.e. "If a method of the same signature is defined in a subclass in a different package, it is a completely separate method and no overriding occurs", but it is hard to test in Groovy as Groovy will invoke methods on the instance of specific type(not the declaring type).

@paulk-asert any thoughts on the PR?

src
└─groovy9282
    │  Base.java
    │  DerivedSamePkg.java
    │
    └─subpkg
            DerivedDiffPkg.java

package groovy9282;

import groovy9282.subpkg.DerivedDiffPkg;

public class Base {
    String x() {
        return "Base";
    }

    public static void main(String[] args) {
        Base dsp = new DerivedSamePkg();
        if (!"DerivedSamePkg".equals(dsp.x())) throw new AssertionError();

        Base ddp = new DerivedDiffPkg();
        if (!"Base".equals(ddp.x())) throw new AssertionError(); // the key part
    }
}

package groovy9282;

public class DerivedSamePkg extends Base {
    @Override
    protected String x() {
        return "DerivedSamePkg";
    }
}

package groovy9282.subpkg;

import groovy9282.Base;

public class DerivedDiffPkg extends Base {
//    @Override
    protected String x() {
        return "DerivedDiffPkg";
    }
}

[daniellansun]

@blackdrag Could you review the PR too, thanks in advance :-)

[daniellansun]

Hi @gesellix, The PR was merged into master, but we forgot to add (closes #1038) in the commit comment when merging. Could you close it manually?

bda2edb

At last, thanks for your contribution!

[gesellix]

Thanks a lot!