+1. I would even consider making it a warning if it isn't marked final,
to encourage people to add those constraints.
There's an unfinished (it works, but is not sufficiently tested)
annotation processor for some guice static analysis, this could also be
implemented there (and have it enabled) so that some of these runtime
errors can be caught at compile-time. (It should remain a runtime error
as well in case people do not run the annotation processor / validator).
If someone does need the old behavior, they may mark the error
suppressed.
The legacy flag is not pretty, but it's certainly a viable option,
though here also I would spout a warning on startup.
Christian.
On 23 May 2014, at 12:20, Luke Sandberg wrote:
While preparing a recent
change<https://code.google.com/p/google-guice/source/detail?r=409e0f578b620c38f6c8626dee78783219d2956e>
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.
--
You received this message because you are subscribed to the Google
Groups "google-guice" group.
To unsubscribe from this group and stop receiving emails from it, send
an email to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/google-guice.
For more options, visit https://groups.google.com/d/optout.
Christian Gruber :: Google, Inc. :: Java Core Libraries :: Dependency
Injection
email: [email protected] :::: mobile: +1 (646) 807-9839
--
You received this message because you are subscribed to the Google Groups
"google-guice" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/google-guice.
For more options, visit https://groups.google.com/d/optout.
