When @Autowired is used, dependencies need to be resolved when the class is instantiated, which may cause early initialization of
+beans or lead the context to look in places it shouldn't to find the bean. To avoid this tricky issue and optimize the way the context loads,
+dependencies should be requested as late as possible. That means using parameter injection instead of field injection for dependencies that are only
+used in a single @Bean method.
+@Configuration
+public class FooConfiguration {
+
+ @Autowired private DataSource dataSource; // Noncompliant
+
+ @Bean
+ public MyService myService() {
+ return new MyService(this.dataSource);
+ }
+}
+
+
+@Configuration
+public class FooConfiguration {
+
+ @Bean
+ public MyService myService(DataSource dataSource) {
+ return new MyService(dataSource);
+ }
+}
+
+Fields used in methods that are called directly by other methods in the application (as opposed to being invoked automatically by the Spring +framework) are ignored by this rule so that direct callers don't have to provide the dependencies themselves.
+ diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/S3305_java.json b/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/S3305_java.json new file mode 100644 index 00000000000..77e1f995871 --- /dev/null +++ b/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/S3305_java.json @@ -0,0 +1,17 @@ +{ + "title": "Factory method injection should be used in \"@Configuration\" classes", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "spring", + "performance" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-3305", + "sqKey": "S3305", + "scope": "Main" +} diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/Sonar_way_profile.json b/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/Sonar_way_profile.json index 9a955976bf5..53da947249e 100644 --- a/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/Sonar_way_profile.json +++ b/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/Sonar_way_profile.json @@ -257,6 +257,7 @@ "S3066", "S3067", "S3281", + "S3305", "S3346", "S3355", "S3358", diff --git a/java-checks/src/test/files/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java b/java-checks/src/test/files/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java new file mode 100644 index 00000000000..abbc9198a2d --- /dev/null +++ b/java-checks/src/test/files/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java @@ -0,0 +1,118 @@ +package src.test.files.checks.spring; + +import javax.inject.Inject; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +class Bar { } + +class Foo { + private final Bar bar; + public Foo(Bar bar) { this.bar = bar; } +} + +@Configuration +class A { + + @Autowired private Bar singleUsage; // Noncompliant [[sc=26;ec=37]] {{Inject this field value directly into "method", the only method that uses it.}} + @Inject private Bar jsr330; // Noncompliant [[sc=23;ec=29]] {{Inject this field value directly into "jsr330", the only method that uses it.}} + @Autowired private Bar multipleUsage; + @Autowired private Bar notUsedInBeanMethod; + @Autowired private Bar notUsed; + private Bar notAutowired; + + @Bean + public Foo method() { + return new Foo(this.singleUsage); + } + + @Bean + public Foo jsr330() { + return new Foo(this.jsr330); + } + + @Bean + public Foo method2() { + return new Foo(this.multipleUsage); + } + + @Bean + public Foo method3() { + return new Foo(this.multipleUsage); + } + + public Foo method4() { + return new Foo(this.notUsedInBeanMethod); + } + + @Bean + public Foo method5() { + return new Foo(this.notAutowired); + } +} + +@Configuration +class B { + @Autowired private Bar multipleUsage; + @Bean + public Foo method() { + return indirectMethod(); + } + private Foo indirectMethod() { + return new Foo(this.multipleUsage); + } + @Bean Foo method2() { + return new Foo(this.multipleUsage); + } +} + +@Configuration +class FalseNegative { + + private Bar bar; // FN + + @Autowired + public void setBar(Bar bar) { + this.bar = bar; + } + + @Bean + public Foo method() { + return new Foo(this.bar); + } +} + +@Configuration +class Ok { + + @Bean + public Foo method(Bar bar) { + return new Foo(bar); + } +} + +@Configuration +public class NestedConfig { + + @Configuration + public class InnerConfig { + @Autowired private Bar x; // Noncompliant + @Bean + public Foo method() { + return new Foo(this.x); + } + } +} + +public class StaticConfig { + @Configuration + public static class InnerStaticConfig { + @Autowired private Bar x; // Noncompliant + + @Bean + public Foo method() { + return new Foo(this.x); + } + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/SpringConfigurationWithAutowiredFieldsCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/SpringConfigurationWithAutowiredFieldsCheckTest.java new file mode 100644 index 00000000000..40d290cec71 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/SpringConfigurationWithAutowiredFieldsCheckTest.java @@ -0,0 +1,33 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * 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 GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.java.checks.spring; + +import org.junit.Test; +import org.sonar.java.checks.verifier.JavaCheckVerifier; + +public class SpringConfigurationWithAutowiredFieldsCheckTest { + + @Test + public void test() { + JavaCheckVerifier.verify("src/test/files/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java", new SpringConfigurationWithAutowiredFieldsCheck()); + JavaCheckVerifier.verifyNoIssueWithoutSemantic("src/test/files/checks/spring/SpringConfigurationWithAutowiredFieldsCheck.java", new SpringConfigurationWithAutowiredFieldsCheck()); + } + +}