Revision: 1096
Author: crazyboblee
Date: Mon Sep 28 19:57:14 2009
Log: We now handle overridden methods properly.
http://code.google.com/p/google-guice/source/detail?r=1096
Modified:
/trunk/src/com/google/inject/spi/InjectionPoint.java
=======================================
--- /trunk/src/com/google/inject/spi/InjectionPoint.java Sun Sep 27
19:51:24 2009
+++ /trunk/src/com/google/inject/spi/InjectionPoint.java Mon Sep 28
19:57:14 2009
@@ -36,16 +36,19 @@
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
+import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Set;
/**
* A constructor, field or method that can receive injections. Typically
this is a member with the
* {...@literal @}...@link Inject} annotation. For non-private, no argument
constructors, the member may
- * omit the annotation.
+ * omit the annotation.
*
* @author [email protected] (Bob Lee)
* @since 2.0
@@ -128,6 +131,7 @@
* Returns the injected constructor, field, or method.
*/
public Member getMember() {
+ // TODO: Don't expose the original member (which probably has
setAccessible(true)).
return member;
}
@@ -293,13 +297,9 @@
* of the valid injection points.
*/
public static Set<InjectionPoint> forStaticMethodsAndFields(TypeLiteral
type) {
- List<InjectionPoint> sink = Lists.newArrayList();
Errors errors = new Errors();
- addInjectionPoints(type, Factory.FIELDS, true, sink, errors);
- addInjectionPoints(type, Factory.METHODS, true, sink, errors);
-
- ImmutableSet<InjectionPoint> result = ImmutableSet.copyOf(sink);
+ Set<InjectionPoint> result = getInjectionPoints(type, true, errors);
if (errors.hasErrors()) {
throw new
ConfigurationException(errors.getMessages()).withPartialValue(result);
}
@@ -333,14 +333,8 @@
* of the valid injection points.
*/
public static Set<InjectionPoint>
forInstanceMethodsAndFields(TypeLiteral<?> type) {
- List<InjectionPoint> sink = Lists.newArrayList();
Errors errors = new Errors();
-
- // TODO (crazybob): Filter out overridden members.
- addInjectionPoints(type, Factory.FIELDS, false, sink, errors);
- addInjectionPoints(type, Factory.METHODS, false, sink, errors);
-
- ImmutableSet<InjectionPoint> result = ImmutableSet.copyOf(sink);
+ Set<InjectionPoint> result = getInjectionPoints(type, false, errors);
if (errors.hasErrors()) {
throw new
ConfigurationException(errors.getMessages()).withPartialValue(result);
}
@@ -362,11 +356,11 @@
return forInstanceMethodsAndFields(TypeLiteral.get(type));
}
- private static void checkForMisplacedBindingAnnotations(Member member,
Errors errors) {
+ private static boolean checkForMisplacedBindingAnnotations(Member
member, Errors errors) {
Annotation misplacedBindingAnnotation =
Annotations.findBindingAnnotation(
errors, member, ((AnnotatedElement) member).getAnnotations());
if (misplacedBindingAnnotation == null) {
- return;
+ return false;
}
// don't warn about misplaced binding annotations on methods when
there's a field with the same
@@ -374,116 +368,254 @@
if (member instanceof Method) {
try {
if
(member.getDeclaringClass().getDeclaredField(member.getName()) != null) {
- return;
+ return false;
}
} catch (NoSuchFieldException ignore) {
}
}
errors.misplacedBindingAnnotation(member, misplacedBindingAnnotation);
+ return true;
}
- private static <M extends Member & AnnotatedElement> void
addInjectionPoints(TypeLiteral<?> type,
- Factory<M> factory, boolean statics, Collection<InjectionPoint>
injectionPoints,
- Errors errors) {
- if (type.getType() == Object.class) {
- return;
+ static abstract class InjectableMember {
+ final TypeLiteral<?> declaringType;
+ final boolean injectable;
+ final boolean optional;
+ final boolean jsr330;
+
+ InjectableMember(TypeLiteral<?> declaringType, AnnotatedElement
member) {
+ this.declaringType = declaringType;
+
+ javax.inject.Inject jsr330Inject =
member.getAnnotation(javax.inject.Inject.class);
+ if (jsr330Inject != null) {
+ injectable = true;
+ optional = false;
+ jsr330 = true;
+ return;
+ }
+
+ jsr330 = false;
+
+ Inject inject = member.getAnnotation(Inject.class);
+ if (inject != null) {
+ injectable = true;
+ optional = inject.optional();
+ return;
+ }
+
+ injectable = false;
+ optional = false;
}
- // Add injectors for superclass first.
- TypeLiteral<?> superType =
type.getSupertype(type.getRawType().getSuperclass());
- addInjectionPoints(superType, factory, statics, injectionPoints,
errors);
-
- // Add injectors for all members next
- addInjectorsForMembers(type, factory, statics, injectionPoints,
errors);
+ abstract InjectionPoint toInjectionPoint(Errors errors);
+ }
+
+ static class InjectableField extends InjectableMember {
+ final Field field;
+ InjectableField(TypeLiteral<?> declaringType, Field field) {
+ super(declaringType, field);
+ this.field = field;
+ }
+
+ InjectionPoint toInjectionPoint(Errors errors) {
+ return new InjectionPoint(declaringType, field, optional);
+ }
}
- private static <M extends Member & AnnotatedElement> void
addInjectorsForMembers(
- TypeLiteral<?> typeLiteral, Factory<M> factory, boolean statics,
- Collection<InjectionPoint> injectionPoints, Errors errors) {
- for (M member : factory.getMembers(getRawType(typeLiteral.getType())))
{
- if (isStatic(member) != statics) {
- continue;
+ static class InjectableMethod extends InjectableMember {
+ final Method method;
+ InjectableMethod(TypeLiteral<?> declaringType, Method method) {
+ super(declaringType, method);
+ this.method = method;
+ }
+
+ InjectionPoint toInjectionPoint(Errors errors) {
+ checkForMisplacedBindingAnnotations(method, errors);
+ return new InjectionPoint(declaringType, method, optional);
+ }
+ }
+
+ private static Set<InjectionPoint> getInjectionPoints(final
TypeLiteral<?> type,
+ boolean statics, Errors errors) {
+ LinkedHashMap<Member, InjectableMember> injectableMembers
+ = new LinkedHashMap<Member, InjectableMember>();
+ HashMap<Signature, List<Method>> bySignature = null;
+
+ for (TypeLiteral<?> current : hierarchyFor(type)) {
+ for (Field field : current.getRawType().getDeclaredFields()) {
+ if (Modifier.isStatic(field.getModifiers()) == statics) {
+ InjectableField injectableField = new InjectableField(current,
field);
+ if (injectableField.injectable) {
+ if (injectableField.jsr330 &&
Modifier.isFinal(field.getModifiers())) {
+ errors.cannotInjectFinalField(field);
+ continue;
+ }
+ injectableMembers.put(field, injectableField);
+ }
+ }
}
- boolean optional;
- javax.inject.Inject javaxInject =
member.getAnnotation(javax.inject.Inject.class);
- if (javaxInject != null) {
- optional = false;
- if (!factory.checkJsr330Compliance(member, errors)) {
- continue; // don't bother to inject noncompliant members
- }
- } else {
- Inject guiceInject = member.getAnnotation(Inject.class);
- if (guiceInject == null) {
- continue;
- }
- optional = guiceInject.optional();
- }
-
+ for (Method method : current.getRawType().getDeclaredMethods()) {
+ boolean isStatic = Modifier.isStatic(method.getModifiers());
+ if (isStatic == statics) {
+ InjectableMethod injectableMethod = new
InjectableMethod(current, method);
+
+ if (injectableMethod.injectable) {
+ boolean result = true;
+ if (injectableMethod.jsr330) {
+ if (Modifier.isAbstract(method.getModifiers())) {
+ errors.cannotInjectAbstractMethod(method);
+ result = false;
+ }
+ if (method.getTypeParameters().length > 0) {
+ errors.cannotInjectMethodWithTypeParameters(method);
+ result = false;
+ }
+ }
+ if (!result) {
+ continue;
+ }
+ if (isStatic && statics) {
+ injectableMembers.put(method, injectableMethod);
+ }
+ }
+
+ if (!isStatic) {
+ // Remove overridden method if present.
+ List<Method> possibleOverrides = null;
+ Signature signature = new Signature(method);
+ if (bySignature == null) {
+ // Lazily initialize the bySignature map.
+ bySignature = new HashMap<Signature, List<Method>>();
+ } else {
+ possibleOverrides = bySignature.get(signature);
+ if (possibleOverrides != null) {
+ Method overridden = removeOverriddenMethod(method,
possibleOverrides);
+ if (overridden != null) {
+ injectableMembers.remove(overridden);
+ }
+ }
+ }
+
+ if (injectableMethod.injectable) {
+ // Keep track of this method in case it gets overridden.
+ if (possibleOverrides == null) {
+ possibleOverrides = new ArrayList<Method>();
+ bySignature.put(signature, possibleOverrides);
+ }
+ possibleOverrides.add(method);
+ injectableMembers.put(method, injectableMethod);
+ }
+ }
+ }
+ }
+ }
+
+ if (injectableMembers.isEmpty()) {
+ return Collections.emptySet();
+ }
+
+ ImmutableSet.Builder<InjectionPoint> builder = ImmutableSet.builder();
+ for (InjectableMember injectableMember : injectableMembers.values()) {
try {
- injectionPoints.add(factory.create(typeLiteral, member, optional,
errors));
+ builder.add(injectableMember.toInjectionPoint(errors));
} catch (ConfigurationException ignorable) {
- if (!optional) {
+ if (!injectableMember.optional) {
errors.merge(ignorable.getErrorMessages());
}
}
}
+ return builder.build();
}
- private static boolean isStatic(Member member) {
- return Modifier.isStatic(member.getModifiers());
+ private static List<TypeLiteral<?>> hierarchyFor(TypeLiteral<?> type) {
+ // Construct type hierarchy from Object.class down.
+ List<TypeLiteral<?>> hierarchy = new ArrayList<TypeLiteral<?>>();
+ TypeLiteral<?> current = type;
+ while (current.getRawType() != Object.class) {
+ hierarchy.add(current);
+ current = current.getSupertype(current.getRawType().getSuperclass());
+ }
+ Collections.reverse(hierarchy);
+ return hierarchy;
}
- private interface Factory<M extends Member & AnnotatedElement> {
- Factory<Field> FIELDS = new Factory<Field>() {
- public Field[] getMembers(Class<?> type) {
- return type.getDeclaredFields();
- }
- public InjectionPoint create(
- TypeLiteral<?> typeLiteral, Field member, boolean optional,
Errors errors) {
- return new InjectionPoint(typeLiteral, member, optional);
- }
- public boolean checkJsr330Compliance(Field member, Errors errors) {
- if (Modifier.isFinal(member.getModifiers())) {
- errors.cannotInjectFinalField(member);
- return false;
- }
- return true;
- }
- };
-
- Factory<Method> METHODS = new Factory<Method>() {
- public Method[] getMembers(Class<?> type) {
- return type.getDeclaredMethods();
- }
- public InjectionPoint create(
- TypeLiteral<?> typeLiteral, Method member, boolean optional,
Errors errors) {
- checkForMisplacedBindingAnnotations(member, errors);
- return new InjectionPoint(typeLiteral, member, optional);
- }
- public boolean checkJsr330Compliance(Method member, Errors errors) {
- boolean result = true;
- if (Modifier.isAbstract(member.getModifiers())) {
- errors.cannotInjectAbstractMethod(member);
- result = false;
- }
- if (member.getTypeParameters().length > 0) {
- errors.cannotInjectMethodWithTypeParameters(member);
- result = false;
- }
- return result;
- }
- };
-
- M[] getMembers(Class<?> type);
- InjectionPoint create(TypeLiteral<?> typeLiteral, M member, boolean
optional, Errors errors);
-
- /**
- * Checks if the member conforms to JSR 330's tight constraints.
- *
- * @return true if the member is conformant.
- */
- boolean checkJsr330Compliance(M member, Errors errors);
+ private static Method removeOverriddenMethod(Method method, List<Method>
possibleOverrides) {
+ for (Iterator<Method> iterator = possibleOverrides.iterator();
iterator.hasNext();) {
+ Method possibleOverride = iterator.next();
+ if (overrides(method, possibleOverride)) {
+ iterator.remove();
+ return possibleOverride;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Returns true if a overrides b. Assumes signatures of a and b are the
same and a's declaring
+ * class is a subclass of b's declaring class.
+ */
+ private static boolean overrides(Method a, Method b) {
+ // See JLS section 8.4.8.1
+ // TODO: Do I need to worry about the visibility of the declaring
class? I don't think so...
+ int modifiers = b.getModifiers();
+ if (Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers)) {
+ return true;
+ }
+ if (Modifier.isPrivate(modifiers)) {
+ return false;
+ }
+ // b must be package-private
+ return
a.getDeclaringClass().getPackage().equals(b.getDeclaringClass().getPackage());
+ }
+
+ /**
+ * A method signature. Used to handle method overridding.
+ */
+ static class Signature {
+
+ final String name;
+ final Class[] parameterTypes;
+ final int hash;
+
+ Signature(Method method) {
+ this.name = method.getName();
+ this.parameterTypes = method.getParameterTypes();
+
+ int h = name.hashCode();
+ h = h * 31 + parameterTypes.length;
+ for (Class parameterType : parameterTypes) {
+ h = h * 31 + parameterType.hashCode();
+ }
+ this.hash = h;
+ }
+
+ @Override public int hashCode() {
+ return this.hash;
+ }
+
+ @Override public boolean equals(Object o) {
+ if (!(o instanceof Signature)) {
+ return false;
+ }
+
+ Signature other = (Signature) o;
+ if (!name.equals(other.name)) {
+ return false;
+ }
+
+ if (parameterTypes.length != other.parameterTypes.length) {
+ return false;
+ }
+
+ for (int i = 0; i < parameterTypes.length; i++) {
+ if (parameterTypes[i] != other.parameterTypes[i]) {
+ return false;
+ }
+ }
+
+ return true;
+ }
}
}
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"google-guice-dev" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/google-guice-dev?hl=en
-~----------~----~----~----~------~----~------~--~---