False positive METHOD_REMOVED for replacing interface with super-interface

[garydgregory]

In Apache Commons Collections, we are migrating from old custom functional interfaces to Java's own (Predicate, Consumer, and so on).

JApiCmp 0.21.2 incorrectly flags method changes from our custom class Closure that extends Consumer to Consumer as METHOD_REMOVED.

For example:

git clone https://gitbox.apache.org/repos/asf/commons-collections.git
cd commons-collections
git checkout japicmp_false_positive
mvn clean install -DskipTests japicmp:cmp

Incorrectly says:

[ERROR] Failed to execute goal com.github.siom79.japicmp:japicmp-maven-plugin:0.21.2:cmp (default-cli) on project commons-collections4: There is at least one incompatibility: org.apache.commons.collections4.functors.AbstractQuantifierPredicate:METHOD_ABSTRACT_ADDED_IN_IMPLEMENTED_INTERFACE,org.apache.commons.collections4.IteratorUtils.forEach(java.util.Iterator,org.apache.commons.collections4.Closure):METHOD_REMOVED,org.apache.commons.collections4.Transformer:CLASS_GENERIC_TEMPLATE_CHANGED,org.apache.commons.collections4.trie.PatriciaTrie:CLASS_GENERIC_TEMPLATE_CHANGED -> [Help 1]

Specifically:

org.apache.commons.collections4.IteratorUtils.forEach(java.util.Iterator,org.apache.commons.collections4.Closure):METHOD_REMOVED

See that this is a false positive, you can create a Java class like:

package test;

import java.util.Arrays;

import org.apache.commons.collections4.IterableUtils;

public class ColCompatTest {

    public static void main(String[] args) {
        IterableUtils.forEach(Arrays.asList("a", "b"), e -> System.out.println(e));
    }
}

And compile it against:

        <dependency>
            <groupId>org.apache.commons</groupId>
            <artifactId>commons-collections4</artifactId>
            <version>4.5.0-M2</version>
        </dependency>

An example invocation against the "old" library would be:

java -cp target/classes;%HOMEPATH%\.m2\repository\org\apache\commons\commons-collections4\4.5.0-M2\commons-collections4-4.5.0-M2.jar test.ColCompatTest

and against the "new" one (the mvn invocation above installed the SNAPSHOT):

C:\Users\ggregory\OneDrive - Rocket Software, Inc\ew\apache-commons\Test17>java -cp "target/classes;%HOMEPATH%\.m2\repository\org\apache\commons\commons-collections4\4.5.0-M3-SNAPSHOT\commons-collections4-4.5.0-M3-SNAPSHOT.jar" test.ColCompatTest

Both work, hence the FP.

It seems that while JApiCmp detects the add/remove case described in JLS 13.4.14, it does not account for the "matching signature" case described in JLS 13.4.12

[siom79]

When I execute japicmp on the branch you have mentioned, then I get this output:

[ERROR]...,org.apache.commons.collections4.IteratorUtils.forEach(java.util.Iterator,org.apache.commons.collections4.Closure):METHOD_REMOVED,...

The class with the METHOD_REMOVED incompatibility is org.apache.commons.collections4.IteratorUtils (and not IterableUtils like in your example, right?). Its method forEach changed from

public static <E> void forEach(final Iterator<E> iterator, final Closure<? super E> closure) {

to

public static <E> void forEach(final Iterator<E> iterator, final Consumer<? super E> closure)

Closure is a subinterface of Consumer:

public interface Closure<T> extends Consumer<T> {

So the issue here is that the second method parameter changed from a subclass to the super class, as the subclass Closure is marked deprecated, right? And this type of change should not be marked as incompatible as the old type is assignable to the new type?

[garydgregory]

@siom79 The mistake is mine and I have to ask you to forgive me for wasting your time 🤦 My example mistakenly tested IterableUtils instead of IteratorUtils! To confirm my error, I edited the example:

package test;

import java.util.Arrays;
import java.util.List;

import org.apache.commons.collections4.Closure;
import org.apache.commons.collections4.IterableUtils;
import org.apache.commons.collections4.IteratorUtils;

// java -cp target/classes;%HOMEPATH%\.m2\repository\org\apache\commons\commons-collections4\4.4\commons-collections4-4.4.jar test.ColCompatTest
// java -cp target/classes;%HOMEPATH%\.m2\repository\org\apache\commons\commons-collections4\4.5.0-M2\commons-collections4-4.5.0-M2.jar test.ColCompatTest
// java -cp target/classes;%HOMEPATH%\.m2\repository\org\apache\commons\commons-collections4\4.5.0-M3-SNAPSHOT\commons-collections4-4.5.0-M3-SNAPSHOT.jar test.ColCompatTest
public class ColCompatTest {

    public static void main(String[] args) {
        final List<String> list = Arrays.asList("a", "b");
        final Closure<? super String> closure = e -> System.out.println(e);
        IteratorUtils.forEach(list.iterator(), closure);
        IterableUtils.forEach(list, closure);
    }
}

and when I run:

java -cp target/classes;%HOMEPATH%\.m2\repository\org\apache\commons\commons-collections4\4.4\commons-collections4-4.4.jar test.ColCompatTest
java -cp target/classes;%HOMEPATH%\.m2\repository\org\apache\commons\commons-collections4\4.5.0-M2\commons-collections4-4.5.0-M2.jar test.ColCompatTest

all is well, but this fails:

java -cp target/classes;%HOMEPATH%\.m2\repository\org\apache\commons\commons-collections4\4.5.0-M3-SNAPSHOT\commons-collections4-4.5.0-M3-SNAPSHOT.jar test.ColCompatTest

with:

Exception in thread "main" java.lang.NoSuchMethodError: 'void org.apache.commons.collections4.IteratorUtils.forEach(java.util.Iterator, org.apache.commons.collections4.Closure)'
        at test.ColCompatTest.main(ColCompatTest.java:18)

So JApiCmp was correct all along 😄

[siom79]

Yes, if you compile your code against M2 you can see with javap that it intends to invoke the method with the Closure parameter:

29: invokestatic  #28                 // Method org/apache/commons/collections4/IteratorUtils.forEach:(Ljava/util/Iterator;Lorg/apache/commons/collections4/Closure;)V

Once you invoke it against version M3-SNAPSHOT you get:

Exception in thread "main" java.lang.NoSuchMethodError: 'void org.apache.commons.collections4.IteratorUtils.forEach(java.util.Iterator, org.apache.commons.collections4.Closure)'
        at test.Main.main(Main.java:11)