FYI: I have submitted this change to google's internal copy of guice and it should be present at HEAD whenever the repo sync next happens.
any update on the annotation processor? On Fri, May 23, 2014 at 6:33 PM, Christian Gruber <[email protected]> wrote: > Hey Steven - you had the processor in a git repo, right? I haven't had a > chance to fully follow up on it, but we have takers to help get it all > tested. Let's coordinate and get the code into some place where it can be > worked on and tested. > > As to breaking changes, we already have some breaking changes if I recall > in Guice 4, so it might be timely (though folks on this list should pipe up > as to whether that's going to rain doom upon them. :) > > Christian > > > On 23 May 2014, at 18:06, Luke Sandberg wrote: > > On Friday, May 23, 2014 4:20:30 PM UTC-7, Christian Gruber wrote: >> >>> >>> +1. I would even consider making it a warning if it isn't marked final, >>> to encourage people to add those constraints. >>> >> >> >> I'm slightly negative on forcing 'final' modifiers (purely due to >> boilerplate issues), though I'm sure i could be convinced. >> >> >> 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 annotation processor sounds awesome. If it is just a matter of >> writing >> tests I would be willing to offer some help to get that out the door :) >> >> >> The legacy flag is not pretty, but it's certainly a viable option, >>> though here also I would spout a warning on startup. >>> >>> >>> Since all these issues would be detected at configure time, we could >> probably just log warnings/severes if the error is disabled. I agree that >> adding a flag for this is kind of gross, but i assume it is neccesary for >> backwards compatibility. Though i don't know what the policy is for >> breaking changes like this. I don't think that this is a large issue >> (based on a cursory review of googles code base), but still it could >> definitely break users. >> >> 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] <javascript:>. >>>> To post to this group, send email to [email protected]< >>>> javascript:>. >>>> >>> >>> 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] <javascript:> :::: 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. >> > > > 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. To view this discussion on the web visit https://groups.google.com/d/msgid/google-guice/CAO9V1MKOtQn7UhMfMHTY8PwWbO6baBVhBG-TDk-3zMt44UOdFA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
