Question about source compatibility check for interface extension #112

vmassol · 2016-03-02T07:03:30Z

Hi,

I'm wondering why the following is considered a breaking source compatibility change.
I have an interface Extension with ExtensionFile getFile()
and I have another interface LocalExtensionthat extends it and I've added LocalExtensionFile getFile()in it, thus overriding the method in the base interface. The important part is that LocalExtensionFileextend ExtensionFileand thus any previous code using LocalExtension#getFile() would have returned an ExtensionFile before and the change is for me backward compatible.

japicmp reports:

**** MODIFIED INTERFACE: PUBLIC ABSTRACT org.xwiki.extension.LocalExtension  (not serializable)
        +++* NEW METHOD: PUBLIC(+) SYNTHETIC(+) BRIDGE(+) org.xwiki.extension.ExtensionFile getFile()

Is that correct in view of this?

Thanks!

The text was updated successfully, but these errors were encountered:

vmassol · 2016-03-02T07:04:38Z

In addition, any idea why in the reporet japicmp mentions SYNTHETIC. I thought synthetic methods were compiler-generated classes named with $ type of notation. Thanks!

siom79 · 2016-03-02T07:46:36Z

Bridge methods are created when you implement a parameterized interface like Node. If you now have a method that uses the parameter as argument for the method like setValue(T value) then the compiler generates next to the method setValue(Integer value) also the method setValue(Object value) to stay compatible with the implemented interface.

Is one of your interfaces ExtensionFile or LocalExtension parameterized?

vmassol · 2016-03-02T10:35:40Z

Ok thanks for the info.

No, they're not parametrized:

public interface ExtensionFile
public interface LocalExtensionFile extends ExtensionFile

vmassol · 2016-03-03T17:25:10Z

So, would this be a bug? Thanks

…compatible, #113 method that overrides in subinterface method from superinterface is no longer detected as source incompatible

siom79 · 2016-03-05T10:53:39Z

Fixed this on the develop branch.

In case you want to try it out:

git clone https://github.com/siom79/japicmp.git
cd japicmp
git checkout develop
mvn install

vmassol · 2016-03-05T11:32:12Z

Thanks @siom79

I've just tried the develop branch and I still have a problem:

[ERROR] Failed to execute goal com.github.siom79.japicmp:japicmp-maven-plugin:0.7.1-SNAPSHOT:cmp (default) on project xwiki-commons-extension-api: Breaking the build because there is at least one source incompatible class: org.xwiki.extension.ExtensionId -> [Help 1]

And in the diff file:

**** MODIFIED CLASS: PUBLIC org.xwiki.extension.ExtensionId  (compatible)
        +++  NEW INTERFACE: java.lang.Comparable
        +++  NEW CONSTRUCTOR: PUBLIC(+) ExtensionId(java.lang.String)
        +++  NEW METHOD: PUBLIC(+) int compareTo(org.xwiki.extension.ExtensionId)

I don't see the star marker ('*') that indicates source incompatibility and I'm also not sure why this would be a source incompatibility.

Thanks for fixing these 2 bugs quickly!

siom79 · 2016-03-05T11:38:49Z

The maven plugin also creates an XML file (target/<execution-id>/japicmp.xml). In this file the code for the modifications are tracked. Can you post the section for the mentioned class?

vmassol · 2016-03-05T13:21:59Z

Here it is:

        <class binaryCompatible="true" changeStatus="MODIFIED" fullyQualifiedName="org.xwiki.extension.ExtensionId" javaObjectSerializationCompatible="SERIALIZABLE_COMPATIBLE" javaObjectSerializationCompatibleAsString="compatible" sourceCompatible="false">
            <annotations/>
            <attributes>
                <attribute changeStatus="UNCHANGED" newValue="NON_SYNTHETIC" oldValue="NON_SYNTHETIC"/>
            </attributes>
            <classType changeStatus="UNCHANGED" newType="CLASS" oldType="CLASS"/>
            <compatibilityChanges>
                <compatibilityChange>INTERFACE_ADDED</compatibilityChange>
            </compatibilityChanges>
            <constructors>
                <constructor binaryCompatible="true" changeStatus="NEW" name="ExtensionId" newLineNumber="60" oldLineNumber="n.a." sourceCompatible="true">
                    <annotations/>
                    <attributes>
                        <attribute changeStatus="NEW" newValue="NON_SYNTHETIC" oldValue="n.a."/>
                    </attributes>
                    <compatibilityChanges/>
                    <exceptions/>
                    <modifiers>
                        <modifier changeStatus="NEW" newValue="NON_FINAL" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_STATIC" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="PUBLIC" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_ABSTRACT" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_BRIDGE" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_SYNTHETIC" oldValue="n.a."/>
                    </modifiers>
                    <parameters>
                        <parameter type="java.lang.String"/>
                    </parameters>
                </constructor>
            </constructors>
            <fields/>
            <interfaces>
                <interface binaryCompatible="true" changeStatus="NEW" fullyQualifiedName="java.lang.Comparable" sourceCompatible="true">
                    <compatibilityChanges/>
                </interface>
            </interfaces>
            <methods>
                <method binaryCompatible="true" changeStatus="NEW" name="compareTo" newLineNumber="130" oldLineNumber="n.a." sourceCompatible="true">
                    <annotations/>
                    <attributes>
                        <attribute changeStatus="NEW" newValue="NON_SYNTHETIC" oldValue="n.a."/>
                    </attributes>
                    <compatibilityChanges/>
                    <exceptions/>
                    <modifiers>
                        <modifier changeStatus="NEW" newValue="NON_FINAL" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_STATIC" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="PUBLIC" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_ABSTRACT" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_BRIDGE" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_SYNTHETIC" oldValue="n.a."/>
                    </modifiers>
                    <parameters>
                        <parameter type="org.xwiki.extension.ExtensionId"/>
                    </parameters>
                    <returnType changeStatus="NEW" newValue="int" oldValue="n.a."/>
                </method>
            </methods>
            <modifiers>
                <modifier changeStatus="UNCHANGED" newValue="NON_FINAL" oldValue="NON_FINAL"/>
                <modifier changeStatus="UNCHANGED" newValue="NON_STATIC" oldValue="NON_STATIC"/>
                <modifier changeStatus="UNCHANGED" newValue="PUBLIC" oldValue="PUBLIC"/>
                <modifier changeStatus="UNCHANGED" newValue="NON_ABSTRACT" oldValue="NON_ABSTRACT"/>
                <modifier changeStatus="UNCHANGED" newValue="NON_SYNTHETIC" oldValue="NON_SYNTHETIC"/>
            </modifiers>
            <serialVersionUid serialVersionUidDefaultNew="-5769516289603733440" serialVersionUidDefaultOld="5556231386943193965" serialVersionUidInClassNew="1" serialVersionUidInClassOld="1" serializableNew="true" serializableOld="true"/>
            <superclass binaryCompatible="true" changeStatus="UNCHANGED" sourceCompatible="false" superclassNew="java.lang.Object" superclassOld="java.lang.Object">
                <compatibilityChanges/>
            </superclass>
        </class>

I see sourceCompatible=false in:

<superclass binaryCompatible="true" changeStatus="UNCHANGED" sourceCompatible="false" superclassNew="java.lang.Object" superclassOld="java.lang.Object">
  <compatibilityChanges/>
</superclass>

However old and new seems to be the same...

Any idea?

Thanks

…emented

siom79 · 2016-03-06T18:43:10Z

The issue was the following compatibility change on class level:

<compatibilityChanges>
  <compatibilityChange>INTERFACE_ADDED</compatibilityChange>
</compatibilityChanges>

But adding an interface is only source incompatible, if your class does not implement all methods. But in your case you did implement compareTo(), hence it was a false alarm.

I have fixed it on the develop branch.

vmassol · 2016-03-06T20:01:33Z

Thanks again Martin. I've rebuilt and re-run japicmp and it's still failing for ExtensionId class. Here's what I get:

        <class binaryCompatible="true" changeStatus="MODIFIED" fullyQualifiedName="org.xwiki.extension.ExtensionId" javaObjectSerializationCompatible="SERIALIZABLE_COMPATIBLE" javaObjectSerializationCompatibleAsString="compatible" sourceCompatible="false">
            <annotations/>
            <attributes>
                <attribute changeStatus="UNCHANGED" newValue="NON_SYNTHETIC" oldValue="NON_SYNTHETIC"/>
            </attributes>
            <classType changeStatus="UNCHANGED" newType="CLASS" oldType="CLASS"/>
            <compatibilityChanges>
                <compatibilityChange>INTERFACE_ADDED</compatibilityChange>
            </compatibilityChanges>
            <constructors>
                <constructor binaryCompatible="true" changeStatus="NEW" name="ExtensionId" newLineNumber="60" oldLineNumber="n.a." sourceCompatible="true">
                    <annotations/>
                    <attributes>
                        <attribute changeStatus="NEW" newValue="NON_SYNTHETIC" oldValue="n.a."/>
                    </attributes>
                    <compatibilityChanges/>
                    <exceptions/>
                    <modifiers>
                        <modifier changeStatus="NEW" newValue="NON_FINAL" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_STATIC" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="PUBLIC" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_ABSTRACT" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_BRIDGE" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_SYNTHETIC" oldValue="n.a."/>
                    </modifiers>
                    <parameters>
                        <parameter type="java.lang.String"/>
                    </parameters>
                </constructor>
            </constructors>
            <fields/>
            <interfaces>
                <interface binaryCompatible="true" changeStatus="NEW" fullyQualifiedName="java.lang.Comparable" sourceCompatible="true">
                    <compatibilityChanges/>
                </interface>
            </interfaces>
            <methods>
                <method binaryCompatible="true" changeStatus="NEW" name="compareTo" newLineNumber="130" oldLineNumber="n.a." sourceCompatible="true">
                    <annotations/>
                    <attributes>
                        <attribute changeStatus="NEW" newValue="NON_SYNTHETIC" oldValue="n.a."/>
                    </attributes>
                    <compatibilityChanges/>
                    <exceptions/>
                    <modifiers>
                        <modifier changeStatus="NEW" newValue="NON_FINAL" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_STATIC" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="PUBLIC" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_ABSTRACT" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_BRIDGE" oldValue="n.a."/>
                        <modifier changeStatus="NEW" newValue="NON_SYNTHETIC" oldValue="n.a."/>
                    </modifiers>
                    <parameters>
                        <parameter type="org.xwiki.extension.ExtensionId"/>
                    </parameters>
                    <returnType changeStatus="NEW" newValue="int" oldValue="n.a."/>
                </method>
            </methods>
            <modifiers>
                <modifier changeStatus="UNCHANGED" newValue="NON_FINAL" oldValue="NON_FINAL"/>
                <modifier changeStatus="UNCHANGED" newValue="NON_STATIC" oldValue="NON_STATIC"/>
                <modifier changeStatus="UNCHANGED" newValue="PUBLIC" oldValue="PUBLIC"/>
                <modifier changeStatus="UNCHANGED" newValue="NON_ABSTRACT" oldValue="NON_ABSTRACT"/>
                <modifier changeStatus="UNCHANGED" newValue="NON_SYNTHETIC" oldValue="NON_SYNTHETIC"/>
            </modifiers>
            <serialVersionUid serialVersionUidDefaultNew="-5769516289603733440" serialVersionUidDefaultOld="5556231386943193965" serialVersionUidInClassNew="1" serialVersionUidInClassOld="1" serializableNew="true" serializableOld="true"/>
            <superclass binaryCompatible="true" changeStatus="UNCHANGED" sourceCompatible="false" superclassNew="java.lang.Object" superclassOld="java.lang.Object">
                <compatibilityChanges/>
            </superclass>
        </class>

Basically nothing has changed. Note that I've git pulled and rebuilt twice to make sure I had the latest version...

Any idea?

Thanks

siom79 · 2016-03-06T21:28:37Z

Interesting output. What compiler do you use?

As it looks, your compiler does not create the "bridge" method compareTo(java.lang.Object) to comply with the interface java.lang.Comparable.

siom79 · 2016-03-06T21:30:56Z

When I add the interface java.lang.Comparable to a class, the openjdk version "1.8.0_72-internal" version creates two methods:

***  MODIFIED CLASS: PUBLIC STATIC japicmp.test.Interfaces$ClassImplementsComparable  (not serializable)
    +++  NEW INTERFACE: java.lang.Comparable
    +++  NEW METHOD: PUBLIC(+) int compareTo(japicmp.test.Interfaces$ClassImplementsComparable)
    +++  NEW METHOD: PUBLIC(+) SYNTHETIC(+) BRIDGE(+) int compareTo(java.lang.Object)

vmassol · 2016-03-06T23:22:41Z

I'm using Oracle's JDK 1.8.0_73-b02 (and thus Oracle's javac).

siom79 · 2016-03-07T06:50:09Z

I also tested it with Oracle's JDK but I got the same two methods as with the openjdk. Are you using any specific compiler flags?

vmassol · 2016-03-07T07:41:48Z

I'm using the following compiler flags:

        <plugin>
          <artifactId>maven-compiler-plugin</artifactId>
          <version>3.2</version>
[...]
          <configuration>
            <source>1.8</source>
            <target>1.8</target>
          </configuration>
        </plugin>

Note: If you wish to try it:

Modify your maven settings.xml to add the xwiki maven repository, you can copy what's on http://dev.xwiki.org/xwiki/bin/view/Community/Building#HInstallingMaven
Checkout xwiki-commons: git clone https://github.com/xwiki/xwiki-commons.git
branch: XCOMMONS-891: git checkout XCOMMONS-891
cd xwiki-commons-core/xwiki-commons-extension/xwiki-commons-extension-api
mvn verify

Thanks

siom79 · 2016-03-07T22:02:45Z

Thank you for providing the details. With the help of them I could reproduce the problem.

The root cause is of course not the java compiler. It generates as expected the synthetic bridge method:

javap ./target/classes/org/xwiki/extension/ExtensionId.class
public class org.xwiki.extension.ExtensionId implements java.io.Serializable, java.lang.Comparable<org.xwiki.extension.ExtensionId> {
  ...
  public int compareTo(org.xwiki.extension.ExtensionId);
  public int compareTo(java.lang.Object);
}

The problem was the configuration option in your base pom.xml:

<includeSynthetic>false</includeSynthetic>

It was implemented as a pre-analysis filter and therefore removed the synthetic methods before the code that evaluates if all inherited abstract methods are implemented. I guess that you have set it due to the problem with LocalExtension. But this class also gets a synthetic bridge method as it overrides the method of a parameterized interface. Complicated. 😄

I have corrected that on the develop branch. Now mvn verify runs through on the module xwiki-commons-extension-api.

vmassol · 2016-03-08T07:34:02Z

Thanks a lot Martin for fixing this. I've also tested it and it works now indeed on xwiki-commons. Next step: I'll run it through the other XWiki repos (xwiki-rendering, xwiki-platform) and see how it goes.

Regarding includeSynthetic I put it to false since I wasn't sure what this was about and indeed when I put it to true it was failing on LocalExtension and CLIRR wasn't reporting any backward compatibility issue with it so I thought I didn't need it. Are you saying that the changes we brought to LocalExtensionare not backward compatible? If so we definitely need to enable includeSyntheticto true.

I'll let you know how it goes on xwiki-rendering and xwiki-platform.

Thanks

siom79 · 2016-03-09T20:18:35Z

You don't need to enable includeSynthetic as the bridge methods are only generated by the compiler for classes that implement parameterized interfaces. Strictly spoken these bridge methods do of course extend an existing API as every user of the class can invoke them, but normally you will invoke the parameterized method and not additionally the generated method with java.lang.Object as argument.

vmassol · 2016-03-09T22:10:32Z

Ok I understand, thanks for the explanation.

vmassol · 2016-03-13T17:08:31Z

@siom79 When do you think we could expect version 0.7.1 to be released? I'm eager to use it on XWiki :) Thanks

siom79 · 2016-03-14T18:01:22Z

Just released 0.7.1. It will take a few hours until it appears on Maven Central.

vmassol · 2016-03-14T18:19:52Z

great! Thanks

siom79 added a commit that referenced this issue Mar 5, 2016

#112 method added to new interface is no longer detected as source in…

7a79885

…compatible, #113 method that overrides in subinterface method from superinterface is no longer detected as source incompatible

siom79 closed this as completed Mar 5, 2016

siom79 added a commit that referenced this issue Mar 6, 2016

#112 interface added only source incompatible if not all methods impl…

7243ac1

…emented

siom79 added a commit that referenced this issue Mar 7, 2016

#112 moved includeSynthetic filter to OutputFilter

be2656d

siom79 reopened this Mar 7, 2016

siom79 closed this as completed Mar 9, 2016

cowwoc mentioned this issue Oct 9, 2016

Not clear why build is breaking #151

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Question about source compatibility check for interface extension #112

Question about source compatibility check for interface extension #112

vmassol commented Mar 2, 2016

vmassol commented Mar 2, 2016

siom79 commented Mar 2, 2016

vmassol commented Mar 2, 2016

vmassol commented Mar 3, 2016

siom79 commented Mar 5, 2016

vmassol commented Mar 5, 2016

siom79 commented Mar 5, 2016

vmassol commented Mar 5, 2016

siom79 commented Mar 6, 2016

vmassol commented Mar 6, 2016

siom79 commented Mar 6, 2016

siom79 commented Mar 6, 2016

vmassol commented Mar 6, 2016

siom79 commented Mar 7, 2016

vmassol commented Mar 7, 2016

siom79 commented Mar 7, 2016

vmassol commented Mar 8, 2016

siom79 commented Mar 9, 2016

vmassol commented Mar 9, 2016

vmassol commented Mar 13, 2016

siom79 commented Mar 14, 2016

vmassol commented Mar 14, 2016

Question about source compatibility check for interface extension #112

Question about source compatibility check for interface extension #112

Comments

vmassol commented Mar 2, 2016

vmassol commented Mar 2, 2016

siom79 commented Mar 2, 2016

vmassol commented Mar 2, 2016

vmassol commented Mar 3, 2016

siom79 commented Mar 5, 2016

vmassol commented Mar 5, 2016

siom79 commented Mar 5, 2016

vmassol commented Mar 5, 2016

siom79 commented Mar 6, 2016

vmassol commented Mar 6, 2016

siom79 commented Mar 6, 2016

siom79 commented Mar 6, 2016

vmassol commented Mar 6, 2016

siom79 commented Mar 7, 2016

vmassol commented Mar 7, 2016

siom79 commented Mar 7, 2016

vmassol commented Mar 8, 2016

siom79 commented Mar 9, 2016

vmassol commented Mar 9, 2016

vmassol commented Mar 13, 2016

siom79 commented Mar 14, 2016

vmassol commented Mar 14, 2016