gnodet-bot commented on code in PR #13327:
URL: https://github.com/apache/maven/pull/13327#discussion_r4174706306
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/SisuDiBridgeModule.java:
##########
@@ -181,9 +201,11 @@ private <Q> Supplier<Q> getBeanSupplier(Dependency<Q> dep,
Key<Q> key) {
// Add DI bindings
list.addAll(getBindings().getOrDefault(key, Set.of()));
// Add Plexus bindings
- for (var bean : locator.get().locate(toGuiceKey(key))) {
- if (isPlexusBean(bean)) {
- list.add(new
BindingToBeanEntry<>(key).toBeanEntry(bean).prioritize(bean.getRank()));
+ if (!sisuFallbackOnly || list.isEmpty()) {
Review Comment:
💡 **Consistency:** This `sisuFallbackOnly` guard is correct here, but the
same pattern is not applied in `getListSupplier()`, `getMapSupplier()`, or
`getAllBindings()` — those methods always add Sisu beans unconditionally.
Today this is safe because the mojo injector binds `Session`, `Project`,
`MojoExecution`, `Log` as singletons, and no mojo would inject those as
`List<Session>`. But if a future mojo injects `List<SomeService>` where
`SomeService` is also bound by the injector itself, Sisu beans will appear in
the list alongside the injector's own binding, breaking the "fallback only"
contract.
Worth applying the same guard for consistency, or documenting that
`sisuFallbackOnly` intentionally applies only to single-bean resolution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
