Class files are processed in non-deterministic order causing issues for nested classes

[Marcono1234]

When this plugin runs animalsniffer's org.codehaus.mojo.animal_sniffer.ant.CheckSignatureTask.execute(), it appears CheckSignatureTask.paths contains a single org.apache.tools.ant.types.Path which lists all class files (noticed this when remote-debugging Gradle). The problem is that these class files are listed in a non-deterministic order. This is problematic because animal-sniffer must first process enclosing classes before processing any nested classes. Otherwise it can happen that a forbidden API usage is detected in a nested class despite the enclosing class being annotated.

This occurrs for me reliably when building mockito/mockito (which uses this Gradle plugin) on Windows. There it always finds forbidden API usage in InlineByteBuddyMockMaker$1 despite the enclosing class being annotated, but for every other class the annotation works as expected (has also been mentioned in mockito/mockito#2024). Changing the class file name to AInlineByteBuddyMockMaker$1.class (prefixed A) still reproduces it, while ZInlineByteBuddyMockMaker$1.class (prefixed Z) causes the issue to disappear (when running ./gradlew animalsnifferMain -x compileJava). This only occurs on Windows, but apparently not on Linux.

I am not familiar with Groovy which is why I have not found out / understood how CheckSignatureTask.paths is populated by this Gradle plugin, or how Ant's Path is supposed to be used. If the Path is supposed to contain all class file paths (instead of only the source directory), then this is probably an animal-sniffer bug here (and I will report it in their repository) because everywhere else they are sorting the file paths (e.g. here) to make sure enclosing classes are processed first.

Please let me know what you think or if you need additional information.

[xvik]

Thank you for the detailed report!

I did manual sorting so now everything would work (I checked on mockito - it works).
Also, fixed links to source in IDEA console.

1.5.2 released.

But I did not update animalsiffer itself to the latest 1.19 (as before, plugin will use 1.18 by default). If you'll try to update it on mockito:

animalsniffer {
    sourceSets = [sourceSets.main]
    annotation = 'org.mockito.internal.SuppressSignatureCheck'
    toolVersion = '1.19'
}

You'll notice additional errors:

[Unrecognized error] D:\study\mockito\src\main\java\org\mockito\internal\creation\bytebuddy\InlineByteBuddyMockMaker.java: Undefined reference: java.lang.instrument.Instrumentation 

[Unrecognized error] D:\study\mockito\src\main\java\org\mockito\internal\creation\bytebuddy\InlineBytecodeGenerator.java: Undefined reference: java.lang.instrument.Instrumentation 

[Unrecognized error] D:\study\mockito\src\main\java\org\mockito\internal\util\reflection\InstrumentationMemberAccessor.java: Undefined reference: java.lang.instrument.Instrumentation 

[Unrecognized error] D:\study\mockito\src\main\java\org\mockito\internal\util\reflection\InstrumentationMemberAccessor.java: Undefined reference: java.lang.invoke.MethodHandles.Lookup 

It's something related to animalsniffer itself (probably a regression). Sorry, did not look into it.
I saw your comments on animalsniffer issue "Enabled java9+ compatibility", so probably you'll find the cause as you track it.

[Marcono1234]

Thank you very much for this fast response and release!

I have also created mojohaus/animal-sniffer#130 in case animal-sniffer is actually misbehaving and should sort the file names from Path.list() as well.

---

You'll notice additional errors [...]

I had not actually tried upgrading animal-sniffer yet, but I am now indeed seeing the same errors. Though it looks like the changes to use Files.walkFileTree(...) by mojohaus/animal-sniffer#91 are not responsible for that because they only affect signature building.
It appears this is an issue with the newly introduced functionality for checking field types, see mojohaus/animal-sniffer#131.

[xvik]

131 would probobly help to suppress new errors, but the problem is that these errors do not specify the exact source location (code line like ...InlineBytecodeGenerator.java:231).

It looks like an "echo" of already suppressed errors. Not normal.

[Marcono1234]

Ah sorry, I should have mentioned that I debugged through animal-sniffer and it appears the new errors are coming from the newly introduced FieldVisitor.
For the missing source location I created mojohaus/animal-sniffer#132. During debugging I noticed that no line number is set yet when the FieldVisitor is used.

[Marcono1234]

For the missing source location I created mojohaus/animal-sniffer#132. During debugging I noticed that no line number is set yet when the FieldVisitor is used.

Just for your information, mojohaus/animal-sniffer#212 has been merged, so once these changes are released and you upgrade your dependency, you might have to adjust your parsing logic in FormatUtils.groovy.

[xvik]

Thank you for the info!
Changes applied. Will release new plugin version just after animalsniffer release