[MDEP-715] Hamcrest used and unused

[jira-importer]

Elliotte Rusty Harold opened MDEP-715 and commented

This is something I've seen when analyzing several Maven plugins. For example, maven-shared-utils circa July 20, 2020:

 

[WARNING] Used undeclared dependencies found: [WARNING] org.hamcrest:hamcrest:jar:2.2:test [WARNING] org.eclipse.sisu:org.eclipse.sisu.plexus:jar:0.0.0.M2a:test [WARNING] Unused declared dependencies found: [WARNING] org.hamcrest:hamcrest-core:jar:2.2:test [WARNING] org.apache.maven:maven-core:jar:3.1.0:test [WARNING] org.codehaus.plexus:plexus-container-default:jar:2.1.0:provided

 

Easy fix, right? don't declare org.hamcrest:hamcrest-core:jar:2.2:test and instead declare org.hamcrest:hamcrest:jar:2.2:test

 

But when I do that:

 

[WARNING] Used undeclared dependencies found: [WARNING] org.hamcrest:hamcrest-core:jar:1.3:test [WARNING] org.eclipse.sisu:org.eclipse.sisu.plexus:jar:0.0.0.M2a:test [WARNING] Unused declared dependencies found: [WARNING] org.hamcrest:hamcrest:jar:2.2:test [WARNING] org.apache.maven:maven-core:jar:3.1.0:test [WARNING] org.codehaus.plexus:plexus-container-default:jar:2.1.0:provided

 

Figure out what's going on here and fix it. Is hamcrest-core needed and used or not? is hamcrest needed and used or not? Be consistent.

 

---

No further details from MDEP-715

[jira-importer]

Michael Boyles commented

The issue seems to be that a class with the same FQN exists in two dependencies.

When shared-utils dependencies are like this 

<dependency>
    <groupId>junit</groupId>        <!-- has transitive dependency on hamcrest-core:1.3 -->
    <artifactId>junit</artifactId>
    <version>4.13</version>
    <scope>test</scope>
</dependency>
<dependency>
    <groupId>org.hamcrest</groupId>
    <artifactId>hamcrest-core</artifactId>
    <version>2.2</version>
    <scope>test</scope>
</dependency>

The hamcrest-core we explicitly declare overrides Junit's. The 2.2 core is actually deprecated and just contains a text file, but crucially gives us the transitive dependency to hamcrest:2.2, which has the classes we actually want. Actually, we only use 1 class, org.hamcrest.CoreMatchers

So in this case we have a transitive dependency on hamcrest:2.2 which we did not declare, so the output of analyze is correct.

 

However, when we make the seemingly obvious fix and replace hamcrest-core with hamcrest:

<!-- junit here -->
<dependency>
    <groupId>org.hamcrest</groupId>
    <artifactId>hamcrest</artifactId>  <!-- was hamcrest-core -->
    <version>2.2</version>
    <scope>test</scope>
</dependency> 

Then now what we have is a transitive dependency on hamcrest-core:1.3 via Junit and our own dependency on hamcrest:2.2. Both contain a class with the FQN. 

hamcrest-core must "win" the lookup, so the dependency plugin assumes org.hamcrest.CoreMatchers comes only from there. If that assumption were true, hamcrest-core would be used and undeclared, transitive through junit, while hamcrest would be unused, hence the spurious result.

I found 2 ways you can work around it:

• Move Junit after hamcrest. hamcrest "wins" the ownership of the class so is now counted as being "used", while hamcrest-core is "unused" (but that's transitive – "unused" and undeclared, so not a warning) 
• Exclude the hamcrest-core from junit. There are no longer 2 classes with the same FQN, so no ambiguity 

—

The ideal fix is less clear to me.

Should we should emit some warning that we found a class that exists in two or more dependencies, and consider all such dependencies to be used, and advise them to solve the problem manually (via exclusions etc.)?

[jira-importer]

Elliotte Rusty Harold commented

A class that exists in two or more dependencies is an error when using Java 9 or later with the Java Platform Module System. From https://jlbp.dev/JLBP-5

 

"In Java 9 and later overlapping classes become compile-time and runtime errors when named modules are used. It is critical, especially in Java 9 and later, to remove all but one of the artifacts that contain overlapping classes from the classpath. Generally this requires changing the POMs of multiple Maven artifacts so they no longer include any dependencies on the artifacts you need to remove from your project’s classpath."

[jira-importer]

Michael Boyles commented

I suppose the important bit there is "when named modules are used", which is unfortunate. Plenty of people don't bother with the module system

[jira-importer]

Elliotte Rusty Harold commented

See https://github.com/apache/maven-changes-plugin/pull/105

Briefly, a project can fix this by matching the Hamcrest version to the JUnit version. That is, do not upgrade hamcrest to 3.0 but stick with 1.3.

That said, this case is not reported well by the dependecny analyzer and we should do better here.