Exclude Spring @Value from GeneralCodingRules#BE_ANNOTATED_WITH_AN_INJECTION_ANNOTATION

[camproto]

I saw your discussions in #288 and I agree that field injection with autowired or similar is worth to complain about and should forced to change to constructor injection.

For the Spring @Value annotation I think it is not harmful at the same level. Forcing constructor annotation for @Value leads to constructor params with @Value like this: public ServiceImpl( @Value("${an.config.property}") String property, ... )

The init of an instance variable from configuration is not so easy to grasp as the @Value annotation would be directly on the field.

Having it still in a testable fashion on the constructor could be achieved with lombok's @RequiredArgsConstructor and an option to copy the @Value from fields to constructor params, see https://github.com/projectlombok/lombok/issues/3322. This works all without even having a coded constructor, what we surely all agree is what we want to achieve.

Using this Lombok approach would lead to an easy readable Service, where you can directly on the instance variables see all configuration parameters and still have constructor injection for easy testing.

I would like to suggest, that the @Value is separated from the other more harmful field injection annotations in BE_ANNOTATED_WITH_AN_INJECTION_ANNOTATION.

Ideal would be to have 2 additional general coding rules, for checking both groups separately.
Autowired,Inject,Inject and Value,Resource

With these 2 new rules a more fine graned architecture check would be possible, without breaking the existing one.

[codecholeric]

I'm not completely sure I'm getting the difference. Why do you think the general arguments against field injection would not apply to @Value? It's also a dependency injection, right? No matter if it's a primitive type or coming from a configuration. It should likely also communicate that it's mandatory and I want simple testability in unit tests without Reflection magic? So I don't really see the difference :thinking: Independently, if there is Lombok involved or not. I'm also not completely sure why your combination of Lombok + @Autowired would work as you desire, but for @Value not? :thinking: If you have a bytecode processor that moves the annotations from the fields to constructor parameters it should work in both cases, no? :thinking: If you have a custom setup with Lombok, where you get some magic, then maybe just writing the rule manually to your desire would make more sense? :thinking: In the end it can be written quite concisely by just following the same pattern of the GeneralCodingRules.

[ColdFireIce]

Well how is this not a real consideration? Look at the following code examples.

Would you rather read and maintain something like this:

@Configuration
public class RetryConfiguration {

    private final long waitDuration;
    private final int maxAttempts;

    public RetryConfiguration(
            @Value("${resilience4j.retry.configs.default.wait-duration:500}") long waitDuration,
            @Value("${resilience4j.retry.configs.default.maxAttempts:5}") int maxAttempts
    ) {
        this.waitDuration = waitDuration;
        this.maxAttempts = maxAttempts;
    }

or this:

@Configuration
@RequiredArgsConstructor
public class RetryConfiguration {

    @Value("${resilience4j.retry.configs.default.wait-duration:500}")
    private final long waitDuration;
    @Value("${resilience4j.retry.configs.default.maxAttempts:5}")
    private final int maxAttempts;

Yes, you would need the lombok.copyableAnnotations += org.springframework.beans.factory.annotation.Value to be set, but as the values are final they must be set in the constructor. So no dark magic with refrection.

both examples will yield the same constructor:

    public RetryConfiguration(
            @Value("${resilience4j.retry.configs.default.wait-duration:500}") long waitDuration,
            @Value("${resilience4j.retry.configs.default.maxAttempts:5}") int maxAttempts
    ) {
        this.waitDuration = waitDuration;
        this.maxAttempts = maxAttempts;
    }

And in both cases Spring will use the constructor injection. Despite that, the ArchUnit Test will fail with the NO_CLASSES_SHOULD_USE_FIELD_INJECTION rule.