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

Adding a method to an interface should be incompatible modification #59

Closed
robinst opened this issue Jun 13, 2015 · 10 comments
Closed

Adding a method to an interface should be incompatible modification #59

robinst opened this issue Jun 13, 2015 · 10 comments

Comments

@robinst
Copy link

robinst commented Jun 13, 2015

  1. Have a build that has breakBuildOnBinaryIncompatibleModifications set to true
  2. On an included interface, add a new method to it
  3. Run the build

Expected: It should fail
Actual: It doesn't fail

Adding a new method (without default implementation) to an interface breaks compatibility, because existing implementations will break. See Eclipse's reference for this too. Or am I missing some configuration for this?

@robinst
Copy link
Author

robinst commented Jun 13, 2015

Let me clarify a bit. It does indeed not break binary compatibility as defined by the JLS. But it does break source compatibility if the clients are expected to implement the interface (say for a callback).

I guess what I'm after is a way to break the build if source compatibility is broken.

@bondolo
Copy link

bondolo commented Jun 14, 2015

This will apply only to class file version 51 and earlier as Java 8 safely allows additions to interfaces if a default implementation is provided.

@siom79
Copy link
Owner

siom79 commented Jun 16, 2015

Indeed, adding a method to an interface does not break binary compatibility. In theory this is described here in the Java Lang. Specification. And in practice it does really work. Having at runtime a new version of the interface with a new method does still executed the old code, although the class does not implement all methods from the new interface.

Source code compatibility is currently not supported.

@robinst
Copy link
Author

robinst commented Jun 22, 2015

@siom79: Calling the methods that the class already implemented is not a problem, correct. But if the new method is called, it will fail at runtime with an java.lang.AbstractMethodError for implementations that do not implement the new method.

So, in practice, ignoring this may lead to errors at runtime.

This means I can not rely on breakBuildOnBinaryIncompatibleModifications. What I do for now is use breakBuildOnModifications, and manually check for source compatibility. When it is compatible, I'll have to add an exclude for the newly-added method.

@siom79
Copy link
Owner

siom79 commented Jun 22, 2015

@robinst: OK, but if you are calling the new method of the interface, then you have already upgraded to the new version, don't you? Or how does the client code know about the new method in the interface?

The breakBuildOnBinaryIncompatibleModifications option should only check if you can run the new version of your library with applications that have been compiled against the old version. For example, your application is compiled against Google's guava in version 16.0 and now you upgrade to 16.0.1. If in 16.0.1 a new method has been added to an interface, then your application should run without any modifications.

@robinst
Copy link
Author

robinst commented Jun 23, 2015

I'm sorry, I should have been more clear with my example. Let's take a real-world example:

Java 7 has a FileVisitor interface. It is used as a parameter for Files.walkFileTree.

Users of this API are expected to implement this interface and pass it as an argument to walkFileTree. Java then calls the appropriate methods of the interface.

Now, let's say Java adds a new method foo to the FileVisitor interface and calls it in walkFileTree. This would pass the check. But clients that upgrade to the new Java version would now fail at runtime.

So even though the change is binary compatible, compatibility as you define it is broken:

The breakBuildOnBinaryIncompatibleModifications option should only check if you can run the new version of your library with applications that have been compiled against the old version

@siom79
Copy link
Owner

siom79 commented Jun 23, 2015

Thank you for pointing me to the exact problem. Now I can reconstruct the java.lang.AbstractMethodError.

But this problem is hard to solve in a general form. The point is that in the case of the visitor pattern, the library under development itself calls the new method. The application compiled against an older version of this library still does not know about this new method. The exception java.lang.AbstractMethodError is actually thrown in the library code.

On the other hand, binary compatibility in the sense of the Java Language Specification is still not broken.

What could be a solution to the problem?

  • Adding source compatiblity as a new feature is hard, as it is not easy to determine, particularly in the case of wildcard imports (see this blog article written by a JDK engineer).
  • Scanning the library under investigation for invocations of the new method through an interface could help to solve your particular problem, but might not be a general solution.
  • Just making the addition of a method to an interface a binary incompatible change would not be aligned with the Java Language Specification.

Do you have an opinion on that?

@robinst
Copy link
Author

robinst commented Aug 13, 2015

I think what would be useful is if interfaces could be marked as "to be implemented by clients" (e.g. like FileVisitor) as opposed to being implemented by the library itself. (In the Eclipse code base, the opposite is done, interfaces that should not be implemented by clients are marked as @noimplement in Javadoc.)

Then, if an interface is marked as that, adding a new method breaks compatibility. If the interface is not marked, then adding a new method is fine (other modifications such as deleting an existing method is still a break though).

It could be done in a similar fashion to includes/excludes. Not sure how such a section should be named, maybe "implementedByClients" or something more specific, "breakOnAbstractAdditions".

@siom79
Copy link
Owner

siom79 commented Feb 21, 2016

With version 0.7.0 japicmp now supports source compatibilty changes.

@siom79 siom79 closed this as completed Feb 21, 2016
@robinst
Copy link
Author

robinst commented Feb 22, 2016

👍 Nice work, thanks!

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

3 participants