On Friday, 11 April 2014 09:53:45 UTC+1, Jochen Wiedmann wrote:
>
> I have spotted what I believe to be a bug in the Guice source code, namely
> ProviderMethodsModule.getProviderMethods:
>
> public List<ProviderMethod<?>> getProviderMethods(Binder binder) {
> List<ProviderMethod<?>> result = Lists.newArrayList();
> for (Class<?> c = delegate.getClass(); c != Object.class; c =
> c.getSuperclass()) {
> for (Method method : c.getDeclaredMethods()) {
> if (method.isAnnotationPresent(Provides.class)) {
> result.add(createProviderMethod(binder, method));
> }
> }
> }
> return result;
> }
>
> The bug is that using c.getDeclaredMethods() is a simplification, because
> it would miss the following case:
>
> public class FooSuper {
> @PostConstruct
> public void start() {
> // Whatever
> }
> }
>
> public class FooSub {
> public void start() {
> super.start();
> }
> }
>
The JSR250 spec which covers @PostConstruct states:
"Member-level annotations on a hidden or overridden member are always
ignored"
Similarly the JSR330 spec has this to say for @Inject on methods:
"A method with no @Inject annotation that overrides a method annotated with
@Inject will not be injected."
Note that this is different from the behaviour of @com.google.inject.Inject
which preceded @javax.inject.Inject:
https://code.google.com/p/error-prone/wiki/OverridesGuiceInjectableMethod
https://code.google.com/p/google-guice/wiki/JSR330
Basically it’s not always the case that overridden members inherit
annotations or other annotated behaviour - it’s up to the specification (or
implementation when there is no spec).
Getting back to the code you listed above, @Provides is a Guice-specific
annotation and not yet standardised. Looking at the javadoc it doesn’t
state whether overridden methods should inherit or hide any @Provides
annotation from the original method. The above implementation as it stands
at the moment will do neither, which does look like an oversight. If this
was to change I’d personally prefer the JSR style behaviour where
"Member-level annotations on a hidden or overridden member are always
ignored”, which could be implemented by adding some minor book-keeping to
the above method while still using getDeclaredMethods.
PS. while there is a @java.lang.annotation.Inherited meta-annotation, this
only applies to class-level annotations and does not apply to annotations
on interfaces or methods:
http://docs.oracle.com/javase/7/docs/api/java/lang/annotation/Inherited.html
> Fact is, that FooSub.getDeclaredMethods() returns FooSub.start (a method
> without annotation), and not FooSuper.start (the annotated method).
>
> I can provide
> a) a test case, that demonstrates the asserted behaviour of
> Class.getDeclaredMethods()
> b) a replacement method, that avoids that problem by iterating over
> super classes and implemented interfaces.
>
> However, I can't provide a test case that exposes the problem by using
> Guice, because it is not clear to me, how to reach
> ProviderMethodsModule.getProviderMethods().
> For the same reason, I cannot judge, whether the problem is negligible, or
> not. However, I suspect that Class.getDeclaredMethods() might be used
> elsewhere.
>
> Any interest?
>
> Jochen
>
>
>
--
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.
