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

False positive #1180

Open
xenoterracide opened this issue Apr 30, 2024 · 2 comments
Open

False positive #1180

xenoterracide opened this issue Apr 30, 2024 · 2 comments
Labels
bug Something isn't working toolchain:java
Milestone

Comments

@xenoterracide
Copy link

Build scan link

https://scans.gradle.com/s/5ox43obbapdui

Plugin version

1.31.0

Gradle version

------------------------------------------------------------
Gradle 8.7
------------------------------------------------------------

Build time:   2024-03-22 15:52:46 UTC
Revision:     650af14d7653aa949fce5e886e685efc9cf97c10

Kotlin:       1.9.22
Groovy:       3.0.17
Ant:          Apache Ant(TM) version 1.10.13 compiled on January 4 2023
JVM:          21.0.3 (Eclipse Adoptium 21.0.3+9-LTS)
OS:           Linux 6.6.26-1-MANJARO amd64

(Optional) reason output for bugs relating to incorrect advice

Reusing configuration cache.
> Task :jpa:processResources NO-SOURCE
> Task :jpa:processTestFixturesResources NO-SOURCE
> Task :model:processResources NO-SOURCE
> Task :jpa:findDeclaredProcsTest UP-TO-DATE
> Task :jpa:findDeclaredProcsMain UP-TO-DATE
> Task :jpa:findDeclarations UP-TO-DATE
> Task :jpa:explodeCodeSourceMain UP-TO-DATE
> Task :jpa:findDeclaredProcsTestFixtures UP-TO-DATE
> Task :jpa:explodeCodeSourceTestFixtures UP-TO-DATE
> Task :jpa:explodeCodeSourceTest UP-TO-DATE
> Task :model:compileJava UP-TO-DATE
> Task :model:classes UP-TO-DATE
> Task :jpa:graphViewTestFixtures UP-TO-DATE
> Task :jpa:graphViewTest UP-TO-DATE
> Task :jpa:graphViewMain UP-TO-DATE
> Task :model:jar UP-TO-DATE
> Task :jpa:artifactsReportMain UP-TO-DATE
> Task :jpa:serviceLoaderMain UP-TO-DATE
> Task :jpa:findInlineMembersMain UP-TO-DATE
> Task :jpa:explodeJarMain UP-TO-DATE
> Task :jpa:synthesizeDependenciesMain UP-TO-DATE
> Task :jpa:compileJava UP-TO-DATE
> Task :jpa:classes UP-TO-DATE
> Task :jpa:explodeByteCodeSourceMain UP-TO-DATE
> Task :jpa:abiAnalysisMain UP-TO-DATE
> Task :jpa:compileTestFixturesJava NO-SOURCE
> Task :jpa:jar UP-TO-DATE
> Task :jpa:synthesizeProjectViewMain UP-TO-DATE
> Task :jpa:testFixturesClasses UP-TO-DATE
> Task :jpa:explodeByteCodeSourceTestFixtures UP-TO-DATE
> Task :jpa:artifactsReportTestFixtures UP-TO-DATE
> Task :jpa:serviceLoaderTestFixtures UP-TO-DATE
> Task :jpa:computeActualUsageMain UP-TO-DATE
> Task :jpa:abiAnalysisTestFixtures UP-TO-DATE
> Task :jpa:testFixturesJar UP-TO-DATE
> Task :jpa:findInlineMembersTestFixtures UP-TO-DATE
> Task :jpa:explodeJarTestFixtures UP-TO-DATE
> Task :jpa:artifactsReportTest UP-TO-DATE
> Task :jpa:synthesizeDependenciesTestFixtures UP-TO-DATE
> Task :jpa:serviceLoaderTest UP-TO-DATE
> Task :jpa:compileTestJava UP-TO-DATE
> Task :jpa:synthesizeProjectViewTestFixtures UP-TO-DATE
> Task :jpa:computeActualUsageTestFixtures UP-TO-DATE
> Task :jpa:explodeJarTest UP-TO-DATE
> Task :jpa:findInlineMembersTest UP-TO-DATE
> Task :jpa:explodeByteCodeSourceTest UP-TO-DATE
> Task :jpa:synthesizeDependenciesTest UP-TO-DATE
> Task :jpa:synthesizeProjectViewTest UP-TO-DATE
> Task :jpa:computeActualUsageTest UP-TO-DATE
> Task :jpa:computeAdvice UP-TO-DATE
> Task :jpa:filterAdvice UP-TO-DATE

> Task :jpa:reason

�[1m----------------------------------------
You asked about the dependency 'com.xenoterracide:tools:0.0.1-alpha.0.75+79910741 (libs.java.tools)'.�[0m
You have been advised to remove this dependency from '�[31mtestImplementation�[0m'.
�[1m----------------------------------------�[0m

�[1mShortest path from :jpa to com.xenoterracide:tools:0.0.1-alpha.0.75+79910741 (libs.java.tools) for compileClasspath:�[0m
:jpa
\--- com.xenoterracide:tools:0.0.1-alpha.0.75+79910741

�[1mThere is no path from :jpa to com.xenoterracide:tools:0.0.1-alpha.0.75+79910741 (libs.java.tools) for runtimeClasspath
�[0m

�[1mShortest path from :jpa to com.xenoterracide:tools:0.0.1-alpha.0.75+79910741 (libs.java.tools) for testCompileClasspath:�[0m
:jpa
\--- com.xenoterracide:tools:0.0.1-alpha.0.75+79910741

�[1mShortest path from :jpa to com.xenoterracide:tools:0.0.1-alpha.0.75+79910741 (libs.java.tools) for testRuntimeClasspath:�[0m
:jpa
\--- com.xenoterracide:tools:0.0.1-alpha.0.75+79910741

�[1mThere is no path from :jpa to com.xenoterracide:tools:0.0.1-alpha.0.75+79910741 (libs.java.tools) for testFixturesCompileClasspath
�[0m

�[1mThere is no path from :jpa to com.xenoterracide:tools:0.0.1-alpha.0.75+79910741 (libs.java.tools) for testFixturesRuntimeClasspath
�[0m

�[1mSource: main
------------�[0m
* Uses 1 class: com.xenoterracide.tools.java.annotation.Initializer (implies implementation).

�[1mSource: test
------------�[0m
* Uses 2 classes: com.xenoterracide.tools.java.annotation.Initializer, com.xenoterracide.tools.java.function.PredicateTools (implies testImplementation).

�[1mSource: testFixtures
--------------------�[0m
(no usages)


BUILD SUCCESSFUL in 658ms
45 actionable tasks: 1 executed, 44 up-to-date
Configuration cache entry reused.

Describe the bug

So even the reason here implies that it knows I need testImplementation

* Uses 2 classes: com.xenoterracide.tools.java.annotation.Initializer, com.xenoterracide.tools.java.function.PredicateTools (implies testImplementation).

but then I get this

Advice for :jpa
Unused dependencies which should be removed:
  testImplementation(libs.java.tools)

To Reproduce

sorry, I haven't created a minimal reproducer, I actually can't imagine with this being the only case I see right now what would cause it. Here's the pointers to the code, but to use the repo you'll have to follow the instructions to set up github packages.

build scan was taken with this commented out https://github.com/xenoterracide/spring-app-commons/blob/ea49791ed7597642e7aca910a9a0203c8b6fb806/build.gradle.kts#L35-L41

this is where the import for the module being complained about is used

https://github.com/xenoterracide/spring-app-commons/blob/ea49791ed7597642e7aca910a9a0203c8b6fb806/module/jpa/src/test/java/com/xenoterracide/jpa/FooAggregate.java#L6

and obviously this is needed at runtime

https://github.com/xenoterracide/spring-app-commons/blob/ea49791ed7597642e7aca910a9a0203c8b6fb806/module/jpa/src/test/java/com/xenoterracide/jpa/FooAggregate.java#L56

Expected behavior

module is not unused.

Additional context

I wonder if it has anything to do with usage of JPMS, or if it has something to do with this using a "3rd party" repo.

@autonomousapps autonomousapps added bug Something isn't working toolchain:java labels May 1, 2024
@autonomousapps autonomousapps added this to the next milestone May 1, 2024
@jjohannes
Copy link
Collaborator

This looks to me like you run into the special treatment of test. The behavior you see is currently expected (it's not a bug).

If something is already implementation, the plugin will advice to not add testImplementation in addition. Because (historically) that is how things are wired in Gradle: testImplementation extend implementation. See: https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_configurations_graph

However, if you want to look at test as separate "Module", this is counterintuitive. But it depends on how you look at test in your project.

There is a proposal to make this configurable (#900). Meaning, the plugin could introduce an option do disable this special treatment of test. Please have a look at that issue.

Note: This has nothing to do with JPMS. It's the same behavior for any Jar.

I think this can be closed in favor of #900.

@xenoterracide
Copy link
Author

xenoterracide commented May 3, 2024

Hmm... except Initializer is a compileOnly annotation. So I think the problem is that its scope as implementation is wrong. Sorry, I didn't notice it said "implementation" for that in its detection.

https://github.com/xenoterracide/java-commons/blob/main/module/tools/src/main/java/com/xenoterracide/tools/java/annotation/Initializer.java

annotations are weird, and a bit obnoxious, because even if an annotation were to describe itself as runtime that doesn't mean it should be exposed via api... some annotations like jspecify does that so that runtime tools like spotbugs can see them, but they aren't used beyond tests usually, not real runtime (imho). In this case though I believe this is appropriately defined as a compile only since I didn't specify runtime retention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working toolchain:java
Projects
None yet
Development

No branches or pull requests

3 participants