From f7276116249755b8fda403dab9690713fb46846f Mon Sep 17 00:00:00 2001 From: Irina Batinic <117161143+irina-batinic-sonarsource@users.noreply.github.com> Date: Mon, 30 Oct 2023 13:58:28 +0100 Subject: [PATCH] =?UTF-8?q?SONARJAVA-4676=20S6829:=20@Autowired=20should?= =?UTF-8?q?=20be=20used=20when=20multiple=20constru=E2=80=A6=20(#4508)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/org/sonar/java/it/AutoScanTest.java | 4 +- .../autoscan/autoscan-diff-by-rules.json | 6 ++ ...structorWhenMultipleConstructorsCheck.java | 93 +++++++++++++++++++ .../java/org/sonar/java/checks/CheckList.java | 2 + ...structorWhenMultipleConstructorsCheck.java | 72 ++++++++++++++ .../org/sonar/l10n/java/rules/java/S6829.html | 68 ++++++++++++++ .../org/sonar/l10n/java/rules/java/S6829.json | 24 +++++ .../java/rules/java/Sonar_way_profile.json | 3 +- ...ctorWhenMultipleConstructorsCheckTest.java | 37 ++++++++ 9 files changed, 306 insertions(+), 3 deletions(-) create mode 100644 java-checks-test-sources/src/main/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheck.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheck.java create mode 100644 java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6829.html create mode 100644 java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6829.json create mode 100644 java-checks/src/test/java/org/sonar/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheckTest.java diff --git a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java index da96222d99d..fd4dbbb7f2c 100644 --- a/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java +++ b/its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java @@ -180,7 +180,7 @@ public void javaCheckTestSources() throws Exception { softly.assertThat(newTotal).isEqualTo(knownTotal); softly.assertThat(rulesCausingFPs).hasSize(6); softly.assertThat(rulesNotReporting).hasSize(7); - softly.assertThat(rulesSilenced).hasSize(74); + softly.assertThat(rulesSilenced).hasSize(75); /** * 4. Check total number of differences (FPs + FNs) @@ -188,7 +188,7 @@ public void javaCheckTestSources() throws Exception { * No differences would mean that we find the same issues with and without the bytecode and libraries */ String differences = Files.readString(pathFor(TARGET_ACTUAL + PROJECT_KEY + "-no-binaries_differences")); - softly.assertThat(differences).isEqualTo("Issues differences: 3473"); + softly.assertThat(differences).isEqualTo("Issues differences: 3478"); softly.assertAll(); } diff --git a/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json b/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json index d670345dfa3..c521820f991 100644 --- a/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json +++ b/its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json @@ -2914,5 +2914,11 @@ "hasTruePositives": false, "falseNegatives": 4, "falsePositives": 0 + }, + { + "ruleKey": "S6829", + "hasTruePositives": false, + "falseNegatives": 5, + "falsePositives": 0 } ] diff --git a/java-checks-test-sources/src/main/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheck.java b/java-checks-test-sources/src/main/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheck.java new file mode 100644 index 00000000000..efaa3811b72 --- /dev/null +++ b/java-checks-test-sources/src/main/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheck.java @@ -0,0 +1,93 @@ +package checks.spring; + +import javax.annotation.Nullable; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +public class AutowiredOnConstructorWhenMultipleConstructorsCheck { // Compliant + + public AutowiredOnConstructorWhenMultipleConstructorsCheck() { + } + + public AutowiredOnConstructorWhenMultipleConstructorsCheck(int i) { + } + + public AutowiredOnConstructorWhenMultipleConstructorsCheck(String s) { + } + + @Component + class SpringComponent { // Noncompliant [[sc=9;ec=24]] {{Add @Autowired to one of the constructors.}} + + public SpringComponent() { + } + + public SpringComponent(int i) { + } + + public SpringComponent(String s) { + } + } + + @Component + class CompliantSpringComponent { // Compliant + + @Autowired + public CompliantSpringComponent() { + } + + public CompliantSpringComponent(int i) { + } + + public CompliantSpringComponent(String s) { + } + } + + @Component + class ComponentWithOtherAnnotations { // Noncompliant + + public ComponentWithOtherAnnotations() { + } + + public ComponentWithOtherAnnotations(int i) { + } + + @Deprecated + public ComponentWithOtherAnnotations(String s) { + } + } + + @Component + class ComponentWithOneConstructorWithAutowired { // Compliant + + public ComponentWithOneConstructorWithAutowired() { + } + } + + @Nullable + class ClassWithSomeOtherAnnotation { // Compliant + + public ClassWithSomeOtherAnnotation() { + } + + public ClassWithSomeOtherAnnotation(int i) { + } + + public ClassWithSomeOtherAnnotation(String s) { + } + } + + @Nullable + @Component + class ClassWithTwoAnnotations { // Noncompliant + + public ClassWithTwoAnnotations() { + } + + public ClassWithTwoAnnotations(int i) { + } + + public ClassWithTwoAnnotations(String s) { + } + } + +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/CheckList.java b/java-checks/src/main/java/org/sonar/java/checks/CheckList.java index 289819dd475..961a6451c82 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/CheckList.java +++ b/java-checks/src/main/java/org/sonar/java/checks/CheckList.java @@ -138,6 +138,7 @@ import org.sonar.java.checks.spring.AsyncMethodsCalledViaThisCheck; import org.sonar.java.checks.spring.AsyncMethodsReturnTypeCheck; import org.sonar.java.checks.spring.AutowiredOnMultipleConstructorsCheck; +import org.sonar.java.checks.spring.AutowiredOnConstructorWhenMultipleConstructorsCheck; import org.sonar.java.checks.spring.ControllerWithSessionAttributesCheck; import org.sonar.java.checks.spring.ModelAttributeNamingConventionForSpELCheck; import org.sonar.java.checks.spring.FieldDependencyInjectionCheck; @@ -284,6 +285,7 @@ public final class CheckList { AsyncMethodsReturnTypeCheck.class, AtLeastOneConstructorCheck.class, AuthorizationsStrongDecisionsCheck.class, + AutowiredOnConstructorWhenMultipleConstructorsCheck.class, AutowiredOnMultipleConstructorsCheck.class, AwsConsumerBuilderUsageCheck.class, AwsCredentialsShouldBeSetExplicitlyCheck.class, diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheck.java new file mode 100644 index 00000000000..dfe3a69d6c2 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheck.java @@ -0,0 +1,72 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2023 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 java.util.List; +import java.util.stream.Collectors; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S6829") +public class AutowiredOnConstructorWhenMultipleConstructorsCheck extends IssuableSubscriptionVisitor { + + private final List annotations = List.of( + "org.springframework.context.annotation.Bean", + "org.springframework.context.annotation.Configuration", + "org.springframework.stereotype.Component", + "org.springframework.stereotype.Controller", + "org.springframework.stereotype.Repository", + "org.springframework.stereotype.Service"); + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CLASS); + } + + @Override + public void visitNode(Tree tree) { + ClassTree classTree = (ClassTree) tree; + + boolean isSpringClass = classTree.modifiers().annotations().stream() + .anyMatch(annotation -> annotations.contains(annotation.symbolType().fullyQualifiedName())); + if (!isSpringClass) { + return; + } + + var constructors = classTree.members().stream() + .filter(member -> member.is(Tree.Kind.CONSTRUCTOR)) + .map(MethodTree.class::cast) + .collect(Collectors.toList()); + + if (constructors.size() > 1) { + boolean anyHasAutowired = constructors.stream() + .anyMatch(constructor -> constructor.modifiers().annotations().stream() + .anyMatch(annotation -> annotation.symbolType().is("org.springframework.beans.factory.annotation.Autowired"))); + + if (!anyHasAutowired) { + reportIssue(classTree.simpleName(), "Add @Autowired to one of the constructors."); + } + } + } + +} diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6829.html b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6829.html new file mode 100644 index 00000000000..f89d35804f8 --- /dev/null +++ b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6829.html @@ -0,0 +1,68 @@ +

The @Autowired annotation in Spring is used for automatic dependency injection. It allows Spring to resolve and inject the required +beans into your bean. For example to inject a @Repository object into a @Service.

+

Why is this an issue?

+

The Spring dependency injection mechanism cannot identify which constructor to use for auto-wiring when multiple constructors are present in a +class. This ambiguity can cause the application to crash at runtime, and it makes the code less clear to understand and more complex to extend and +maintain.

+

What is the potential impact?

+ +

How to fix it

+

Use the @Autowired annotation to specify which constructor to use for auto-wiring.

+

Code examples

+

Noncompliant code example

+
+@Component
+public class ExampleClass { // Noncompliant, multiple constructors present and no @Autowired annotation to specify which one to use
+
+    private final DependencyClass1 dependency1;
+
+    public ExampleClass() {
+        throw new UnsupportedOperationException("Not supported yet.");
+    }
+
+    public ExampleClass(DependencyClass1 dependency1) {
+        this.dependency1 = dependency1;
+    }
+
+    // ...
+}
+
+

Compliant solution

+
+@Component
+public class ExampleClass {
+
+    private final DependencyClass1 dependency1;
+
+    public ExampleClass() {
+        throw new UnsupportedOperationException("Not supported yet.");
+    }
+
+    @Autowired
+    public ExampleClass(DependencyClass1 dependency1) {
+        this.dependency1 = dependency1;
+    }
+
+    // ...
+}
+
+

Resources

+

Documentation

+ +

Articles & blog posts

+ + diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6829.json b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6829.json new file mode 100644 index 00000000000..59c02b57a18 --- /dev/null +++ b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6829.json @@ -0,0 +1,24 @@ +{ + "title": "\"@Autowired\" should be used when multiple constructors are provided", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "spring" + ], + "defaultSeverity": "Minor", + "ruleSpecification": "RSPEC-6829", + "sqKey": "S6829", + "scope": "All", + "quickfix": "infeasible", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM", + "RELIABILITY": "MEDIUM" + }, + "attribute": "CLEAR" + } +} diff --git a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json index 68df912b72e..87e03b7dd2a 100644 --- a/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json +++ b/java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json @@ -486,6 +486,7 @@ "S6810", "S6813", "S6814", - "S6818" + "S6818", + "S6829" ] } diff --git a/java-checks/src/test/java/org/sonar/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheckTest.java new file mode 100644 index 00000000000..abff370ca1d --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheckTest.java @@ -0,0 +1,37 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2023 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.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class AutowiredOnConstructorWhenMultipleConstructorsCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/spring/AutowiredOnConstructorWhenMultipleConstructorsCheck.java")) + .withCheck(new AutowiredOnConstructorWhenMultipleConstructorsCheck()) + .verifyIssues(); + } + +}