in AutowiredAnnotationBeanPostProcessor,we buildAutowiringMetadata against method annotated @Autowired

private InjectionMetadata buildAutowiringMetadata(final Class<?> clazz) {
    // ....
ReflectionUtils.doWithLocalMethods(targetClass, method -> {
                Method bridgedMethod = BridgeMethodResolver.findBridgedMethod(method);
                if (!BridgeMethodResolver.isVisibilityBridgeMethodPair(method, bridgedMethod)) {
                    return;
                }
                AnnotationAttributes ann = findAutowiredAnnotation(bridgedMethod);
                if (ann != null && method.equals(ClassUtils.getMostSpecificMethod(method, clazz))) {
                    if (Modifier.isStatic(method.getModifiers())) {
                        if (logger.isInfoEnabled()) {
                            logger.info("Autowired annotation is not supported on static methods: " + method);
                        }
                        return;
                    }
                    if (method.getParameterCount() == 0) {
                        if (logger.isInfoEnabled()) {
                            logger.info("Autowired annotation should only be used on methods with parameters: " +
                                    method);
                        }
                    }
                    boolean required = determineRequiredStatus(ann);
                    PropertyDescriptor pd = BeanUtils.findPropertyForMethod(bridgedMethod, clazz);
                    currElements.add(new AutowiredMethodElement(method, required, pd));
                }
            });
    // ...
}

From the above code, I can see that any method with an @Autowired annotation will be treated as an injection point, which means that parameters of method will be injected, if parameters cannot be injected, an error will be reported, and the method will be invoked when method InjectionMetadata.inject executes。

We think there are several problems with the above code:

  • Methods with no arguments should be filtered out,no arguments means do not need autowire

  • BeanUtils.findPropertyForMethod(bridgedMethod, clazz) will return null when bridgedMethod is neither readMethod nor writeMethod。In this case, do we still need to inject?

  • if bridgedMethod is readMethod that means the method look like void getXxx(),do we still need new a AutowiredMethodElement ?

Because I don't know if this code is designed that way,so I make this issue。

Looking forward to your reply. Thank you very much!!!!

Comment From: sbrannen

Methods with no arguments should be filtered out,no arguments means do not need autowire

That's correct.

@jhoeller, are you aware of any reason why we don't simply return early there like with the static check?

* `BeanUtils.findPropertyForMethod(bridgedMethod, clazz)` will return null when `bridgedMethod` is neither `readMethod ` nor `writeMethod`。In this case, do we still need to inject?

The PropertyDescriptor supplied to AutowiredMethodElement is @Nullable. So that's not an issue. Note that autowired methods do not need to be customary "setter" methods -- for example, @Autowired void config(Service1 s1, Service2 s2) is a valid candidate for autowiring.

Returning early for zero arguments would also avoid that code path entirely.

if bridgedMethod is readMethod that means the method look like void getXxx(),do we still need new a AutowiredMethodElement ?

Again, if the method accepts zero arguments, I think we should just return early.

Looking forward to your reply. Thank you very much!!!!

Let's see what @jhoeller says.

Comment From: jhoeller

This is essentially about cases that aren't really proper, and we're just trying to behave reasonably.

In case of an @Autowired method with no parameters, we're nevertheless invoking it since it may perform some initialization logic... even if such logic is better suited in an @PostConstruct method or a constructor.

In case the method doesn't match a JavaBeans property, we're nevertheless invoking it since autowired methods fundamentally may have any signature; we're only trying to determine whether an external property override may affect it.

Comment From: sbrannen

Thanks for the enlightenment, @jhoeller.

In case of an @Autowired method with no parameters, we're nevertheless invoking it since it may perform some initialization logic... even if such logic is better suited in an @PostConstruct method or a constructor.

I wasn't aware that we even "supported" no-args @Autowired methods. Interesting!

Though I wonder if that's documented and/or tested...

Comment From: daimingzhi

If it is designed in this way, is it better for the code to make the following changes?

in AutowiredAnnotationBeanPostProcessor#buildAutowiringMetadata

                    boolean required = false;
                    PropertyDescriptor pd = null;
                    if (method.getParameterCount() == 0) {
                        //In case of an @Autowired method with no parameters, 
                        // we're nevertheless invoking it since it may perform some initialization logic... 
                        // even if such logic is better suited in an @PostConstruct method or a constructor.
                        if (logger.isInfoEnabled()) {
                            logger.info("Autowired annotation should only be used on methods with parameters: " +
                                    method);
                        }
                    } else {
                        required = determineRequiredStatus(ann);
                        pd = BeanUtils.findPropertyForMethod(bridgedMethod, clazz);
                    }
//                  boolean required = determineRequiredStatus(ann);
//                  PropertyDescriptor pd = BeanUtils.findPropertyForMethod(bridgedMethod, clazz);
                    currElements.add(new AutowiredMethodElement(method, required, pd));

The main changes are:

  1. Add a code path for no-args method
  2. Add comments to make this code easy for others to understand(It took me a long time to understand it

in AutowiredMethodElement#inject

if the method which will be invoked is no-args,we can end early

protected void inject(Object bean, @Nullable String beanName, @Nullable PropertyValues pvs) throws Throwable {
    if (checkPropertySkipping(pvs)) {
        return;
    }
    Method method = (Method) this.member;
    Object[] arguments;
    if (this.cached) {
        // Shortcut for avoiding synchronization...
        arguments = resolveCachedArguments(beanName);
    } else if (method.getParameterCount() == 0) {
        arguments = new Object[0];
    } else {
        Class<?>[] paramTypes = method.getParameterTypes();
        // .....
    }
    // .....
}

If my suggestion is acceptable,can I make a PR for this?

Thanks!

Comment From: daimingzhi

@sbrannen @jhoeller I'm sorry to disturb you, but I've been waiting for a long time