SONARJAVA-4676 S6829: @Autowired should be used when multiple constru… (

#4508)

its/ruling/src/test/java/org/sonar/java/it/AutoScanTest.java

@@ -180,15 +180,15 @@ 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)
      *
      * 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();
   }

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
   }
 ]

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) {
+    }
+  }
+
+}

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,

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<String> 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<Tree.Kind> 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.");
+      }
+    }
+  }
+
+}

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6829.html

@@ -0,0 +1,68 @@
+<p>The <code>@Autowired</code> 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 <code>@Repository</code> object into a <code>@Service</code>.</p>
+<h2>Why is this an issue?</h2>
+<p>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.</p>
+<h3>What is the potential impact?</h3>
+<ul>
+  <li> <strong>Incorrect Instantiation</strong>: the wrong constructor is selected for instantiation, leading to a bean not being correctly
+  initialized. </li>
+  <li> <strong>Unsatisfied Dependency Exception</strong>: the constructor selected by Spring requires beans that are not available in the Spring
+  context. </li>
+  <li> <strong>Non-Deterministic Behavior</strong>: the constructor selected by Spring can vary, based on the number of dependencies that can be
+  satisfied at runtime, leading to unpredictable application behavior. </li>
+  <li> <strong>Maintainability Issues</strong>: adding more constructors in the future could lead to further confusion and potential bugs. </li>
+</ul>
+<h2>How to fix it</h2>
+<p>Use the <code>@Autowired</code> annotation to specify which constructor to use for auto-wiring.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+@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;
+    }
+
+    // ...
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+@Component
+public class ExampleClass {
+
+    private final DependencyClass1 dependency1;
+
+    public ExampleClass() {
+        throw new UnsupportedOperationException("Not supported yet.");
+    }
+
+    @Autowired
+    public ExampleClass(DependencyClass1 dependency1) {
+        this.dependency1 = dependency1;
+    }
+
+    // ...
+}
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Spring - <a href="https://docs.spring.io/spring-framework/reference/core/beans/annotation-config/autowired.html">Annotation Config:
+  Autowired</a> </li>
+</ul>
+<h3>Articles &amp; blog posts</h3>
+<ul>
+  <li> Java Guides - <a href="https://www.javaguides.net/2023/08/unsatisfieddependencyexception-in.html">UnsatisfiedDependencyException in Spring
+  Boot</a> </li>
+</ul>
+

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"
+  }
+}

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"
   ]
 }

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();
+  }
+
+}