rules_scala icon indicating copy to clipboard operation
rules_scala copied to clipboard

unused_dependency_checker false positives with _suite macros

Open gkossakowski opened this issue 6 years ago • 4 comments

Unused dependency checker gives false positives when _suite macros are involved. Let's consider:

scala_library_suite(
   name = "foo",
   srcs = ["A.scala", "B.scala"],
   deps = ["dep1", "dep2"],
)

A.scala depends on dep1 and B.scala depends on dep2. In this scenario, both dep1 and dep2 will be reported as unused. It's because _suite is expanded into several targets with deps copied to each of them.

Users' expectation here is that dependency is reported as an unused only when it's not used in all srcs from the suite but now it's reported if it's unused in some source from the suite. Fixing this would require a post-processing step that takes union of deps information from all expanded targets of the suite.

gkossakowski avatar Sep 21 '18 18:09 gkossakowski

yeah, @gkk-stripe and I talked about this offline...

A probably better way to go would be to just emit an extra output (or maybe resource in the jar) of what dependencies were actually used. Then have an additional bazel verification step (which can be disabled).

Then this suite macro can disable the check on each of the internal targets, and then add another check task that the union of the used dependencies matches the full set of dependencies.

johnynek avatar Sep 21 '18 20:09 johnynek

generating an additional output would make debugging of this functionality easier too

gkossakowski avatar Sep 21 '18 20:09 gkossakowski

+1

johnynek avatar Sep 21 '18 20:09 johnynek

Can we also think how this works for a scala_library with java and scala? Feels related (though definitely not the same). We (Wix) would like to use this feature later on and this will need to be a supported feature (even if we add this later on). @iirina @lberki, Is there an output we can take from java_common.compile to later warn about unused deps from skylark? Want to have an aggregate of java and scala.

On Fri, 21 Sep 2018 at 23:24 P. Oscar Boykin [email protected] wrote:

+1

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/issues/617#issuecomment-423660457, or mute the thread https://github.com/notifications/unsubscribe-auth/ABUIF9nTRZXxxBGlF7cd6MB9l9bP_dBdks5udUr9gaJpZM4W0o0E .

ittaiz avatar Sep 22 '18 13:09 ittaiz