While preparing
a recent change to how @Provides methods are invoked, I came across some confusing behavior when @Provides methods are overridden. The initial issue i saw was for a case like this:
class Super extends AbstractModule {
@Override void configure() {}
@Provides Number provideNumber() {
return 1;
}
}
class Sub extends Super {
@Provides Integer provideNumber() {
return 2;
}
}
Injector injector = Guice.createInjector(new Sub());
assertEquals(2, injector.getInstance(Number.class));
assertEquals(2, injector.getInstance(Integer.class));
This happens because Sub.provideNumber is a covariant override of Super.provideNumber. So 2 provider methods are bound, but both call Sub.provideNumber.
If instead you wrote:
class Sub extends Super {
@Provides Number provideNumber() {
return 2;
}
}
then Guice.createInjector will throw a CreationException for duplicate bindings for Key.get(Number.class).
There are a lot of different combinations for how you could override an @Provides method depending on whether or not you mark the override with @Provides and whether or not you change the key for the method. This gets especially weird if you start adding scoping annotations into the mix (e.g. if the override is marked @Singleton but the parent isn't). Currently, the Guice implementation of @Provides doesn't do anything special to try to detect these cases.
Proposal:
My proposal is to make it an Error to override an @Provides method in all cases. Guice will essentially enforce that @Provides methods are private/final (whether or not they are actually declared as such).
The goal of this proposal is to eliminate opportunities for confusion and to make modules easier to understand.
FAQs
Q: What about users who overrides @Provides methods for testing purposes?
Guice has other mechanisms to override bindings (e.g. Modules.override), or you can structure your test so that the parent @Provides method isn't installed, maybe by splitting it into a separate Module class that you don't install in the test. Or if you really want to use method overriding you can use a template method pattern and just override a different method and have the @Provides method call that one.
Q: What about migration of legacy code that uses this feature?
I'm not sure yet. Guice has a some precedent for using java system properties for feature flags, so we may introduce a system property ('guice_allow_provides_overrides'?) to enable/disable this error.