[Java] Spring @Bean methods should return most specific type

Clean Code New rules / language support
,
1

When using the Spring Java Config developers often follow the principle of code to the interface not the implementation and thus make the return type of a @Bean method the interface. This can lead to subtle bugs, if the actual class actually implements more interfaces that are used elsewhere.

Spring uses the method return type to resolve dependencies between beans. When a bean is initialized it will update its knowledge of the bean by looking at the actual instance, so the code that only returns the interface might actually work if by chance the beans are initialized in the right order. However, the order is not strongly guaranteed and can change due to unrelated changes, leading to hard to debug bugs.

As there is no real downside, aside from style and habit, to declaring the actual type as return type it should be the default.

Noncompliant Code:

@Configuration
public class Config {
    @Bean
    MyInterface service() { // Noncompliant code
        return new MyServiceImpl();
   }
}

Compliant Code:

@Configuration
public class Config {
    @Bean
    MyServiceImpl service() { // compliant code
        return new MyServiceImpl();
   }
}

type: Bug

2

Declaring the implementation class instead of the interface may also introduce subtle bugs. When Spring has to create a Proxy, it has to subclass the implementation, so this has to support that and provide a visible no-args constructor. Avoiding these bugs can lead to implementation artifacts in code that otherwise would not care about Spring at all.

Further, this would flag any instance of a strategy pattern using Spring configuration to select the concrete strategy:

@Configuration
public class Config {
    @Bean
    @Profile("production")
    Strategy productionStrategy() {
        return new ProductionStrategy();
   }

   @Bean
   @Profile("!production")
   Strategy testStrategy() {
        return new TestStrategy();
   }
}
3

@ssiegler good point with scopes so, beans annotated with @Scope , @RequestScope , @SessionScope , and @ApplicationScope should be excluded, or if we can use a regex it would be @[a-zA-Z]*Scope.

As for the strategy pattern, what is the downside of declaring ProductionStrategy instead of Strategy? The upside would be if ProductionStrategy was also implementing MetricsBinder, then it would be properly picked up.

Depending on the power of the sonar scanners this rule could look at the class, and only complain if it implements more interfaces than the one the bean method returns. This way you could still return the interface if the class only implements that one, and be warned if you omit other interfaces or superclasses if the class implements more than that one.

4

You are right, this is basically just the style issue in this case.

When using a factory for the strategies, sonar could only infer the interface type anyway, so there would be no issue, right?

@Configuration
public class Config {
    @Bean
    @Profile("production")
    Strategy productionStrategy() {
        return Strategy.withStandardFeatures();
   }

   @Bean
   @Profile("!production")
   Strategy testStrategy() {
        return Strategy.withStandardFeatures().withExperimentalFeatures();
   }
}
5

I’d like to quote the Spring Docs here.

When declaring a @Bean method, provide as much type information as possible in the method’s return type. For example, if your bean’s concrete class implements an interface the bean method’s return type should be the concrete class and not the interface. Providing as much type information as possible in @Bean methods is particularly important when using bean conditions as their evaluation can only rely upon to type information that’s available in the method signature.

7

Hi @leonard84, thanks for your suggestion!

According to the Spring Docs, this does make sense to provide as much information about the type as possible. On the other hand, it looks like this rule might also bring a lot of FPs, and become noisy, so users will switch it off and loose its value.

In our team we agreed that we should provide new rules targeting modern and popular Java frameworks (especially Spring). So we’ll keep your idea in mind, and try to create a good rule from it.

Regards,
Margarita

1 Like