Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive when making a package-private class public that already has a public static method's different return type #365

Closed
garydgregory opened this issue Oct 5, 2023 · 5 comments

Comments

@garydgregory
Copy link

garydgregory commented Oct 5, 2023

I don't see how this breaks binary compatibility:

  • Define a package private class C with a public static method M that returns interface F, release that as version N
  • For version N+1:
    • Change the return type of M to a concrete implementation of F (the kind return type change should not matter)
    • Run JApiCmp and all is well, obviously, since this C is a package-private class
    • Change the visibility of C from package-private to public
    • JApiCmp now fails.

The failure does not make sense since no one outside the package could call C in version N.
For example:

  • git clone -n https://gitbox.apache.org/repos/asf/commons-io.git
  • git checkout 877b9e3f5e553f8ad988b89b8adb6b93fa97a964
  • Make the class org.apache.commons.io.StreamIterator public
  • Run mvn clean install -DskipTests japicmp:cmp -Dcommons.japicmp.version=0.18.1
[ERROR] Failed to execute goal com.github.siom79.japicmp:japicmp-maven-plugin:0.18.1:cmp (default-cli) on project commons-io: There is at least one incompatibility: org.apache.commons.io.filefilter.PathMatcherFileFilter:METHOD_ABSTRACT_ADDED_IN_IMPLEMENTED_INTERFACE,org.apache.commons.io.function.Uncheck.get(org.apache.commons.io.function.IOSupplier,java.util.function.Supplier):CLASS_GENERIC_TEMPLATE_CHANGED,org.apache.commons.io.StreamIterator.iterator(java.util.stream.Stream):METHOD_RETURN_TYPE_CHANGED -> [Help 1]
asfgit pushed a commit to apache/commons-io that referenced this issue Oct 10, 2023
@siom79
Copy link
Owner

siom79 commented Oct 18, 2023

Changing the return type from an interface to a concrete implementation causes the incompatibility METHOD_RETURN_TYPE_CHANGED as now source code assigning the return value to another implementation of the interface won't work any longer.
This is already detected in the first run of japicmp, but the class is package protected and therefore filtered.
If you change the configuration of japicmp to package_protected, you will see the change in the first run:

<plugin>
        <groupId>com.github.siom79.japicmp</groupId>
        <artifactId>japicmp-maven-plugin</artifactId>
        <configuration>
          <parameter>
            <accessModifier>package_protected</accessModifier>
          </parameter>
        </configuration>
      </plugin>

PS: The documentation is currently wrong, as the option is named there protected and not package_protected.

@garydgregory
Copy link
Author

Uh? Please help me understand: If a released version of a library contains a package private class, then that class is NOT accessible to the outside world. So how can there be a break in compatibility for clients in the next version when that class is made public, no matter what changes?

@siom79
Copy link
Owner

siom79 commented Oct 22, 2023

You are right. When the class changes to public, all incompatibilities should be ignored.

@siom79 siom79 reopened this Oct 22, 2023
@garydgregory
Copy link
Author

You are right. When the class changes to public, all incompatibilities should be ignored.

Ah, thank you 😄 😄

@siom79
Copy link
Owner

siom79 commented Nov 2, 2023

Fixed with this commit.

@siom79 siom79 closed this as completed Nov 2, 2023
asfgit pushed a commit to apache/commons-io that referenced this issue Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants