diff --git a/core/src/com/google/inject/internal/DeclaredMembers.java b/core/src/com/google/inject/internal/DeclaredMembers.java index 376ba62d2f..a0e50eda4c 100644 --- a/core/src/com/google/inject/internal/DeclaredMembers.java +++ b/core/src/com/google/inject/internal/DeclaredMembers.java @@ -15,7 +15,8 @@ */ package com.google.inject.internal; -import com.google.common.collect.Ordering; +import static com.google.common.collect.Comparators.lexicographical; + import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.Arrays; @@ -36,24 +37,14 @@ public final class DeclaredMembers { private DeclaredMembers() {} - public static Field[] getDeclaredFields(Class type) { - return Arrays.stream(type.getDeclaredFields()) - .sorted( - Comparator.comparing(Field::getName) - .thenComparing(Field::getType, Comparator.comparing(Class::getName))) - .toArray(Field[]::new); - } + public static final Comparator FIELD_COMPARATOR = + Comparator.comparing(Field::getName) + .thenComparing(Field::getType, Comparator.comparing(Class::getName)); - public static Method[] getDeclaredMethods(Class type) { - return Arrays.stream(type.getDeclaredMethods()) - .sorted( - Comparator.comparing(Method::getName) - .thenComparing(Method::getReturnType, Comparator.comparing(Class::getName)) - .thenComparing( - method -> Arrays.asList(method.getParameterTypes()), - // TODO: use Comparators.lexicographical when it's not @Beta. - Ordering.>from(Comparator.comparing(Class::getName)) - .lexicographical())) - .toArray(Method[]::new); - } + public static final Comparator METHOD_COMPARATOR = + Comparator.comparing(Method::getName) + .thenComparing(Method::getReturnType, Comparator.comparing(Class::getName)) + .thenComparing( + method -> Arrays.asList(method.getParameterTypes()), + lexicographical(Comparator.comparing(Class::getName))); } diff --git a/core/src/com/google/inject/internal/ProviderMethodsModule.java b/core/src/com/google/inject/internal/ProviderMethodsModule.java index ffde0cadcc..96450014cf 100644 --- a/core/src/com/google/inject/internal/ProviderMethodsModule.java +++ b/core/src/com/google/inject/internal/ProviderMethodsModule.java @@ -39,6 +39,7 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Arrays; +import java.util.Comparator; import java.util.List; /** @@ -114,30 +115,30 @@ public List> getProviderMethods(Binder binder) { // The highest class in the type hierarchy that contained a provider method definition. Class superMostClass = getDelegateModuleClass(); for (Class c = superMostClass; c != Object.class && c != null; c = c.getSuperclass()) { - for (Method method : DeclaredMembers.getDeclaredMethods(c)) { - Annotation annotation = getAnnotation(binder, method); - if (annotation != null) { - if (isStaticModule() - && !Modifier.isStatic(method.getModifiers()) - && !Modifier.isAbstract(method.getModifiers())) { - binder.addError( - "%s is an instance method, but a class literal was passed. Make this method" - + " static or pass an instance of the module instead.", - method); - continue; - } - if (result == null) { - result = new ArrayList<>(); - methodsAndAnnotations = new ArrayList<>(); - } + for (MethodAndAnnotation methodAndAnnotation : + getDeclaredProviderAnnotatedMethods(c, binder)) { + Annotation annotation = methodAndAnnotation.annotation; + Method method = methodAndAnnotation.method; + if (isStaticModule() + && !Modifier.isStatic(method.getModifiers()) + && !Modifier.isAbstract(method.getModifiers())) { + binder.addError( + "%s is an instance method, but a class literal was passed. Make this method" + + " static or pass an instance of the module instead.", + method); + continue; + } + if (result == null) { + result = new ArrayList<>(); + methodsAndAnnotations = new ArrayList<>(); + } - ProviderMethod providerMethod = createProviderMethod(binder, method, annotation); - if (providerMethod != null) { - result.add(providerMethod); - } - methodsAndAnnotations.add(new MethodAndAnnotation(method, annotation)); - superMostClass = c; + ProviderMethod providerMethod = createProviderMethod(binder, method, annotation); + if (providerMethod != null) { + result.add(providerMethod); } + methodsAndAnnotations.add(methodAndAnnotation); + superMostClass = c; } } if (result == null) { @@ -201,6 +202,22 @@ public List> getProviderMethods(Binder binder) { return result; } + private static final Comparator METHOD_AND_ANNOTATION_COMPARATOR = + Comparator.comparing( + methodAndAnnotation -> methodAndAnnotation.method, DeclaredMembers.METHOD_COMPARATOR); + + private List getDeclaredProviderAnnotatedMethods(Class c, Binder binder) { + List result = new ArrayList<>(); + for (Method method : c.getDeclaredMethods()) { + Annotation annotation = getAnnotation(binder, method); + if (annotation != null) { + result.add(new MethodAndAnnotation(method, annotation)); + } + } + result.sort(METHOD_AND_ANNOTATION_COMPARATOR); + return result; + } + private static class MethodAndAnnotation { final Method method; final Annotation annotation; diff --git a/core/src/com/google/inject/spi/InjectionPoint.java b/core/src/com/google/inject/spi/InjectionPoint.java index 6963328903..ee13c3891d 100644 --- a/core/src/com/google/inject/spi/InjectionPoint.java +++ b/core/src/com/google/inject/spi/InjectionPoint.java @@ -45,6 +45,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -515,6 +516,7 @@ InjectionPoint toInjectionPoint() { static class InjectableMethod extends InjectableMember { final Method method; + /** * true if this method overrode a method that was annotated with com.google.inject.Inject. used * to allow different override behavior for guice inject vs jsr330 Inject @@ -716,74 +718,51 @@ private static Set getInjectionPoints( TypeLiteral current = hierarchy.get(i); - for (Field field : getDeclaredFields(current)) { - if (Modifier.isStatic(field.getModifiers()) == statics) { - Annotation atInject = getAtInject(field); - if (atInject != null) { - InjectableField injectableField = new InjectableField(current, field, atInject); - if (injectableField.specInject && Modifier.isFinal(field.getModifiers())) { - errors.cannotInjectFinalField(field); - } - injectableMembers.add(injectableField); - } + for (InjectableField injectableField : getDeclaredInjectableFields(current, statics)) { + Field field = injectableField.field; + if (injectableField.specInject && Modifier.isFinal(field.getModifiers())) { + errors.cannotInjectFinalField(field); } + injectableMembers.add(injectableField); } - for (Method method : getDeclaredMethods(current)) { - if (isEligibleForInjection(method, statics)) { - Annotation atInject = getAtInject(method); - if (atInject != null) { - InjectableMethod injectableMethod = new InjectableMethod(current, method, atInject); - if (checkForMisplacedBindingAnnotations(method, errors) - || !isValidMethod(injectableMethod, errors)) { - if (overrideIndex != null) { - boolean removed = - overrideIndex.removeIfOverriddenBy(method, false, injectableMethod); - if (removed) { - logger.log( - Level.WARNING, - "Method: {0} is not a valid injectable method (" - + "because it either has misplaced binding annotations " - + "or specifies type parameters) but is overriding a method that is " - + "valid. Because it is not valid, the method will not be injected. " - + "To fix this, make the method a valid injectable method.", - method); - } - } - continue; - } - if (statics) { - injectableMembers.add(injectableMethod); - } else { - if (overrideIndex == null) { - /* - * Creating the override index lazily means that the first type in the hierarchy - * with injectable methods (not necessarily the top most type) will be treated as - * the TOP position and will enjoy the same optimizations (no checks for overridden - * methods, etc.). - */ - overrideIndex = new OverrideIndex(injectableMembers); - } else { - // Forcibly remove the overridden method, otherwise we'll inject - // it twice. - overrideIndex.removeIfOverriddenBy(method, true, injectableMethod); - } - overrideIndex.add(injectableMethod); + for (InjectableMethod injectableMethod : + getDeclaredInjectableMethods(current, overrideIndex, statics)) { + Method method = injectableMethod.method; + if (checkForMisplacedBindingAnnotations(method, errors) + || !isValidMethod(injectableMethod, errors)) { + if (overrideIndex != null) { + boolean removed = overrideIndex.removeIfOverriddenBy(method, false, injectableMethod); + if (removed) { + logger.log( + Level.WARNING, + "Method: {0} is not a valid injectable method (" + + "because it either has misplaced binding annotations " + + "or specifies type parameters) but is overriding a method that is " + + "valid. Because it is not valid, the method will not be injected. " + + "To fix this, make the method a valid injectable method.", + method); } + } + continue; + } + if (statics) { + injectableMembers.add(injectableMethod); + } else { + if (overrideIndex == null) { + /* + * Creating the override index lazily means that the first type in the hierarchy + * with injectable methods (not necessarily the top most type) will be treated as + * the TOP position and will enjoy the same optimizations (no checks for overridden + * methods, etc.). + */ + overrideIndex = new OverrideIndex(injectableMembers); } else { - if (overrideIndex != null) { - boolean removed = overrideIndex.removeIfOverriddenBy(method, false, null); - if (removed) { - logger.log( - Level.WARNING, - "Method: {0} is not annotated with @Inject but " - + "is overriding a method that is annotated with @jakarta.inject.Inject." - + "Because it is not annotated with @Inject, the method will not be " - + "injected. To fix this, annotate the method with @Inject.", - method); - } - } + // Forcibly remove the overridden method, otherwise we'll inject + // it twice. + overrideIndex.removeIfOverriddenBy(method, true, injectableMethod); } + overrideIndex.add(injectableMethod); } } } @@ -805,12 +784,53 @@ private static Set getInjectionPoints( return builder.build(); } - private static Field[] getDeclaredFields(TypeLiteral type) { - return DeclaredMembers.getDeclaredFields(type.getRawType()); + private static final Comparator INJECTABLE_FIELD_COMPARATOR = + Comparator.comparing(field -> field.field, DeclaredMembers.FIELD_COMPARATOR); + + private static List getDeclaredInjectableFields( + TypeLiteral current, boolean statics) { + List fields = new ArrayList<>(); + for (Field field : current.getRawType().getDeclaredFields()) { + if (Modifier.isStatic(field.getModifiers()) == statics) { + Annotation atInject = getAtInject(field); + if (atInject != null) { + fields.add(new InjectableField(current, field, atInject)); + } + } + } + fields.sort(INJECTABLE_FIELD_COMPARATOR); + return fields; } - private static Method[] getDeclaredMethods(TypeLiteral type) { - return DeclaredMembers.getDeclaredMethods(type.getRawType()); + private static final Comparator INJECTABLE_METHOD_COMPARATOR = + Comparator.comparing(method -> method.method, DeclaredMembers.METHOD_COMPARATOR); + + private static List getDeclaredInjectableMethods( + TypeLiteral current, OverrideIndex overrideIndex, boolean statics) { + List methods = new ArrayList<>(); + for (Method method : current.getRawType().getDeclaredMethods()) { + if (isEligibleForInjection(method, statics)) { + Annotation atInject = getAtInject(method); + if (atInject != null) { + methods.add(new InjectableMethod(current, method, atInject)); + } else { + if (overrideIndex != null) { + boolean removed = overrideIndex.removeIfOverriddenBy(method, false, null); + if (removed) { + logger.log( + Level.WARNING, + "Method: {0} is not annotated with @Inject but " + + "is overriding a method that is annotated with @jakarta.inject.Inject." + + "Because it is not annotated with @Inject, the method will not be " + + "injected. To fix this, annotate the method with @Inject.", + method); + } + } + } + } + } + methods.sort(INJECTABLE_METHOD_COMPARATOR); + return methods; } /**