diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/ConfigurationClassShouldNotBeFinalCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/ConfigurationClassShouldNotBeFinalCheckSample.java new file mode 100644 index 00000000000..a14fbeaed72 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/spring/ConfigurationClassShouldNotBeFinalCheckSample.java @@ -0,0 +1,130 @@ +package checks.spring; + +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.scheduling.annotation.EnableScheduling; +import org.springframework.stereotype.Component; + +@Configuration +final class BasicFinalConfig { // Noncompliant {{Remove the "final" modifier from this "@Configuration" class.}} + + @Bean + public String dataSource() { + return "dataSource"; + } +} + +@Configuration +final class FinalConfigMultipleBeans { // Noncompliant + + @Bean + public String foo() { + return "foo"; + } + + @Bean + public String bar() { + return foo(); + } +} + +@Configuration +@EnableScheduling +final class FinalConfigOtherAnnotations { // Noncompliant + + @Bean + public String scheduler() { + return "scheduler"; + } +} + +class OuterClass { + @Configuration + public static final class NestedFinalConfig { // Noncompliant + + @Bean + public String taskScheduler() { + return "taskScheduler"; + } + } +} + +@Configuration(proxyBeanMethods = true) +final class FinalConfigProxyEnabled { // Noncompliant + + @Bean + public String dataSource() { + return "dataSource"; + } +} + +@Configuration +class NonFinalConfig { + + @Bean + public String dataSource() { + return "dataSource"; + } + + @Bean + public String jdbcTemplate() { + return dataSource(); + } +} + +@Configuration(proxyBeanMethods = false) +final class FinalConfigProxyDisabled { + + @Bean + public String dataSource() { + return "dataSource"; + } +} + +@Component +final class FinalComponentNotConfiguration { + + public String createDataSource() { + return "dataSource"; + } +} + +@Configuration +abstract class AbstractConfig { + + @Bean + public String dataSource() { + return "dataSource"; + } +} + +class NoAnnotation { + public final class NotAConfigurationClass { + public String method() { + return "value"; + } + } +} + +class OuterClassCompliant { + @Configuration + public static class NestedNonFinalConfig { + + @Bean + public String taskScheduler() { + return "taskScheduler"; + } + } +} + +// Meta-annotations like @SpringBootApplication are intentionally not detected by this rule +// to avoid complexity with determining proxyBeanMethods for composed annotations +@SpringBootApplication +final class FinalSpringBootApplication { // Compliant - meta-annotations not detected + + @Bean + public String appBean() { + return "appBean"; + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/ConfigurationClassShouldNotBeFinalCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/ConfigurationClassShouldNotBeFinalCheck.java new file mode 100644 index 00000000000..21bad8678c6 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/ConfigurationClassShouldNotBeFinalCheck.java @@ -0,0 +1,87 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks.spring; + +import java.util.List; +import java.util.Optional; +import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.ExpressionsHelper; +import org.sonar.java.checks.helpers.SpringUtils; +import org.sonar.java.model.ModifiersUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.AnnotationTree; +import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.Modifier; +import org.sonar.plugins.java.api.tree.ModifierKeywordTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S9021") +public class ConfigurationClassShouldNotBeFinalCheck extends IssuableSubscriptionVisitor { + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CLASS); + } + + @Override + public void visitNode(Tree tree) { + ClassTree classTree = (ClassTree) tree; + + // Note: This check only detects direct @Configuration annotations, not meta-annotations + // like @SpringBootApplication. This is intentional to avoid complexity with proxyBeanMethods. + Optional configurationAnnotation = getConfigurationAnnotation(classTree); + if (configurationAnnotation.isEmpty()) { + return; + } + + Optional finalModifier = getFinalModifier(classTree); + if (finalModifier.isEmpty()) { + return; + } + + if (hasProxyBeanMethodsDisabled(configurationAnnotation.get())) { + return; + } + + reportIssue(finalModifier.get(), "Remove the \"final\" modifier from this \"@Configuration\" class."); + } + + private static Optional getConfigurationAnnotation(ClassTree tree) { + return tree.modifiers().annotations().stream() + .filter(annotation -> annotation.symbolType().is(SpringUtils.CONFIGURATION_ANNOTATION)) + .findFirst(); + } + + private static Optional getFinalModifier(ClassTree tree) { + return ModifiersUtils.findModifier(tree.modifiers(), Modifier.FINAL); + } + + private static boolean hasProxyBeanMethodsDisabled(AnnotationTree annotation) { + return annotation.arguments().stream() + .filter(argument -> argument.is(Tree.Kind.ASSIGNMENT)) + .map(AssignmentExpressionTree.class::cast) + .anyMatch(ConfigurationClassShouldNotBeFinalCheck::setsProxyBeanMethodsToFalse); + } + + private static boolean setsProxyBeanMethodsToFalse(AssignmentExpressionTree assignment) { + return "proxyBeanMethods".equals(((IdentifierTree) assignment.variable()).name()) && + Boolean.FALSE.equals(ExpressionsHelper.getConstantValueAsBoolean(assignment.expression()).value()); + } + +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/ConfigurationClassShouldNotBeFinalCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/ConfigurationClassShouldNotBeFinalCheckTest.java new file mode 100644 index 00000000000..1b9da225479 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/ConfigurationClassShouldNotBeFinalCheckTest.java @@ -0,0 +1,34 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks.spring; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class ConfigurationClassShouldNotBeFinalCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/spring/ConfigurationClassShouldNotBeFinalCheckSample.java")) + .withCheck(new ConfigurationClassShouldNotBeFinalCheck()) + .verifyIssues(); + } + +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9021.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9021.html new file mode 100644 index 00000000000..3ade16fe3cd --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9021.html @@ -0,0 +1,97 @@ +

Why is this an issue?

+

Spring Framework uses CGLIB proxies to enforce singleton semantics for beans defined in @Configuration classes. When you call a +@Bean method from within another @Bean method, Spring intercepts the call through a proxy to return the singleton instance +rather than creating a new object.

+

The Java final keyword prevents class inheritance, which blocks CGLIB from creating the necessary subclass proxy. Without this proxy, +inter-bean method calls create new instances instead of retrieving the singleton, breaking Spring's container management.

+

This typically manifests as a ConfigurationClassPostProcessor exception during application startup, preventing the Spring context from +initializing.

+

What is the potential impact?

+

When Spring @Configuration classes are marked final, the application fails to start. The +ConfigurationClassPostProcessor throws an exception because CGLIB cannot create the required proxy subclass.

+

If this check were bypassed, the impact would be more subtle: inter-bean method calls would create duplicate instances instead of using singletons, +leading to:

+
    +
  • Resource leaks: Multiple database connection pools or file handles
    • +
  • Inconsistent state: Different parts of the application working with different object instances
    • +
  • Configuration drift: Bean initialization logic executing multiple times with potentially different results
    • +
+

How to fix it

+

Remove the final modifier from the @Configuration class. This allows Spring to create the CGLIB proxy needed to enforce +singleton semantics for inter-bean method calls.

+

Code examples

+

Noncompliant code example

+
+@Configuration
+public final class DatabaseConfig {  // Noncompliant
+
+    @Bean
+    public DataSource dataSource() {
+        return new HikariDataSource();
+    }
+
+    @Bean
+    public JdbcTemplate jdbcTemplate() {
+        return new JdbcTemplate(dataSource());  // May create duplicate DataSource
+    }
+}
+
+

Compliant solution

+
+@Configuration
+public class DatabaseConfig {
+
+    @Bean
+    public DataSource dataSource() {
+        return new HikariDataSource();
+    }
+
+    @Bean
+    public JdbcTemplate jdbcTemplate() {
+        return new JdbcTemplate(dataSource());  // Uses singleton DataSource
+    }
+}
+
+

If you don't need inter-bean method calls and want to optimize startup performance, use @Configuration(proxyBeanMethods = false). This +disables CGLIB proxying, allowing the class to be final. However, you must use dependency injection instead of calling @Bean +methods directly.

+

Noncompliant code example

+
+@Configuration
+public final class DatabaseConfig {  // Noncompliant
+
+    @Bean
+    public DataSource dataSource() {
+        return new HikariDataSource();
+    }
+
+    @Bean
+    public JdbcTemplate jdbcTemplate() {
+        return new JdbcTemplate(dataSource());  // Direct method call
+    }
+}
+
+

Compliant solution

+
+@Configuration(proxyBeanMethods = false)
+public final class DatabaseConfig {
+
+    @Bean
+    public DataSource dataSource() {
+        return new HikariDataSource();
+    }
+
+    @Bean
+    public JdbcTemplate jdbcTemplate(DataSource dataSource) {  // Injected parameter
+        return new JdbcTemplate(dataSource);
+    }
+}
+
+

Resources

+

Documentation

+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9021.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9021.json new file mode 100644 index 00000000000..cdca057e4e8 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9021.json @@ -0,0 +1,22 @@ +{ + "title": "\"@Configuration\" classes should not be final", + "type": "BUG", + "status": "ready", + "remediation": { + "func": "Constant/Issue", + "constantCost": "5min" + }, + "tags": [ + "spring" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-9021", + "sqKey": "S9021", + "scope": "Main", + "code": { + "impacts": { + "RELIABILITY": "HIGH" + }, + "attribute": "LOGICAL" + } +}