Add support for jakarta annotations to some checks #4782
Conversation
| @@ -168,6 +173,7 @@ public final class UnusedVariable extends BugChecker implements CompilationUnitT | |||
| "com.google.inject.multibindings.ProvidesIntoMap", | |||
| "com.google.inject.multibindings.ProvidesIntoSet", | |||
| "dagger.Provides", | |||
| "jakarta.inject.Inject", | |||
There was a problem hiding this comment.
Maybe also add this to UnnecessarilyVisible? (as hinted in the TODO comment above the constant)
There was a problem hiding this comment.
Added it there as well as to few other places I found. Thanks!
I've seen it also in InjectMatchers and other places that seem to relate to dependency injection / Guice. I didn't change that, because the logic is a bit more complicated, and since we're not using Guice, I'm not that confident in changing those. I'm not sure whether it even supports Jakarta EE already.
There was a problem hiding this comment.
since we're not using Guice, I'm not that confident in changing those. I'm not sure whether it even supports Jakarta EE already.
FYI, it does!
https://github.com/google/guice/wiki/Guice600
https://github.com/google/guice/wiki/Guice700
There was a problem hiding this comment.
@tbroyer good to know :) But do you think we can have these checks merged separately from Guice or are they somehow intertwined?
I also noticed that sometimes there are suggested fixes to add javax.inject.Inject annotation, but it doesn't make sense if the project is already using jakarta / Guice 7 🤷 I think there could be some heuristic, but I'm not the best person to define that as we're using spring instead 😅
…ssarilyVisible, UnnecessaryAssignment
gavlyukovskiy
changed the title
This MR adds jakarta annotations to the exempt lists.
We're currently migrating to Jakarta EE and we have to temporarily disable the rule or add exceptions in the code.
I didn't add new tests since it's just additional exempt annotations, but let me know if you want explicit tests.