ArC: detect unsupported Inject annotations #31012
Conversation
- including javax.inject.Inject, com.google.inject.Inject, com.oracle.svm.core.annotate.Inject and org.gradle.internal.impldep.javax.inject.Inject
|
Very nice, thanks! |
| unsupported.add(new UnsupportedAnnotation("com.google.inject.Inject", correctInject)); | ||
| unsupported.add(new UnsupportedAnnotation("com.oracle.svm.core.annotate.Inject", correctInject)); | ||
| unsupported.add(new UnsupportedAnnotation("org.gradle.internal.impldep.javax.inject.Inject", | ||
| correctInject)); |
There was a problem hiding this comment.
Is there a reason why you didn't do it for let's say @ApplicationScoped?
There was a problem hiding this comment.
@Inject and @Singleton are the most common ones and the problematic cases are mainly when you migrate to Quarkus 3 - the compiler won't tell you about these in tests (as seen from changed files).
I am not aware of similar issue with @ApplicationScoped, at least not in basic dependency setup?
There was a problem hiding this comment.
@ApplicationScoped is CDI specific, so it's unlikely to be forked that much. I agree @Inject and @Singleton are the most important. We could add @Named as well. I'm not sure about @Qualifier and @Scope.
There was a problem hiding this comment.
Well, no reason really... except that if you use a wrong scope the bean class is just ignored and usually results in a build failure (because of an unsatisfied dependency) while if you use a wrong @Inject you can get NPEs at runtime.
There was a problem hiding this comment.
Yeah OK. You will have javax.inject around in our test scope so I suppose it makes sense to limit ourselves to these.
I was more thinking of someone who just upgraded the Quarkus version without changing the source code but they will have a ton of other issues anyway so I suppose we can ignore @ApplicationScoped.
There was a problem hiding this comment.
We can try to address scopes in a separate pull request...
This comment has been minimized.
This comment has been minimized.
mkouba
added
the
triage/waiting-for-ci
|
✔️ The latest workflow run for the pull request has completed successfully. It should be safe to merge provided you have a look at the other checks in the summary. |
quarkus-bot
bot
removed
the
triage/waiting-for-ci
org.gradle.internal.impldep.javax.inject.Inject