SONARJAVA-4654 Implement S6818: Avoid Using "@Autowired" on Multiple … (

#4501)

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(73);
+    softly.assertThat(rulesSilenced).hasSize(74);
 
     /**
      * 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: 3469");
+    softly.assertThat(differences).isEqualTo("Issues differences: 3473");
 
     softly.assertAll();
   }

its/ruling/src/test/resources/autoscan/autoscan-diff-by-rules.json

@@ -2908,5 +2908,11 @@
     "hasTruePositives": false,
     "falseNegatives": 5,
     "falsePositives": 0
+  },
+  {
+    "ruleKey": "S6818",
+    "hasTruePositives": false,
+    "falseNegatives": 4,
+    "falsePositives": 0
   }
 ]

java-checks-test-sources/src/main/java/checks/spring/AutowiredOnMultipleConstructorsCheckSample.java

@@ -0,0 +1,84 @@
+package checks.spring;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.springframework.beans.factory.annotation.Autowired;
+
+public class AutowiredOnMultipleConstructorsCheckSample {
+
+  private final Object myService;
+
+  public AutowiredOnMultipleConstructorsCheckSample(Integer i, Object myService) {
+    // ...
+    this.myService = myService;
+  }
+
+  @Autowired
+  public AutowiredOnMultipleConstructorsCheckSample(Object myService) {
+    this.myService = myService;
+  }
+
+  @Autowired // Noncompliant [[sc=3;ec=13]] {{Remove this "@Autowired" annotation.}}
+  public AutowiredOnMultipleConstructorsCheckSample(List<Object> list, Object myService) {
+    // ...
+    this.myService = myService;
+  }
+
+  public AutowiredOnMultipleConstructorsCheckSample(Long l, Object myService) {
+    // ...
+    this.myService = myService;
+  }
+
+  @Autowired // Noncompliant
+  public AutowiredOnMultipleConstructorsCheckSample(Double d, Object myService) {
+    // ...
+    this.myService = myService;
+  }
+
+}
+
+class Test {
+
+  private final Object myService;
+
+  @Nullable
+  @Autowired // Compliant
+  public Test(Object myService) {
+    this.myService = myService;
+  }
+
+}
+
+class MyComponent {
+  private final Object myService;
+
+  @Autowired
+  public MyComponent(Object myService) {
+    this.myService = myService;
+    // ...
+  }
+
+  @Autowired // Noncompliant
+  public MyComponent(Object myService, Integer i) {
+    this.myService = myService;
+    // ...
+  }
+
+  @Autowired(required = true) // Noncompliant
+  public MyComponent(Object myService, Integer i, String s) {
+    this.myService = myService;
+    // ...
+  }
+
+  @Nullable // Compliant
+  public MyComponent(Object myService, int i) {
+    this.myService = myService;
+    // ...
+  }
+
+  @Autowired(required = false) // Compliant
+  public MyComponent(Object myService, int i, String s) {
+    this.myService = myService;
+    // ...
+  }
+}

java-checks/src/main/java/org/sonar/java/checks/CheckList.java

@@ -137,6 +137,7 @@
 import org.sonar.java.checks.serialization.SerializableSuperConstructorCheck;
 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.ControllerWithSessionAttributesCheck;
 import org.sonar.java.checks.spring.ModelAttributeNamingConventionForSpELCheck;
 import org.sonar.java.checks.spring.FieldDependencyInjectionCheck;
@@ -283,6 +284,7 @@ public final class CheckList {
     AsyncMethodsReturnTypeCheck.class,
     AtLeastOneConstructorCheck.class,
     AuthorizationsStrongDecisionsCheck.class,
+    AutowiredOnMultipleConstructorsCheck.class,
     AwsConsumerBuilderUsageCheck.class,
     AwsCredentialsShouldBeSetExplicitlyCheck.class,
     AwsLambdaSyncCallCheck.class,

java-checks/src/main/java/org/sonar/java/checks/spring/AutowiredOnMultipleConstructorsCheck.java

@@ -0,0 +1,84 @@
+/*
+ * 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.Optional;
+import java.util.stream.Collectors;
+import org.sonar.check.Rule;
+import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
+import org.sonar.plugins.java.api.semantic.Symbol;
+import org.sonar.plugins.java.api.semantic.SymbolMetadata;
+import org.sonar.plugins.java.api.tree.AnnotationTree;
+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 = "S6818")
+public class AutowiredOnMultipleConstructorsCheck extends IssuableSubscriptionVisitor {
+
+  private static final String AUTOWIRED_ANNOTATION = "org.springframework.beans.factory.annotation.Autowired";
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return List.of(Tree.Kind.CLASS);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    ClassTree classTree = (ClassTree) tree;
+    List<MethodTree> constructors = classTree.members().stream()
+      .filter(m -> m.is(Tree.Kind.CONSTRUCTOR))
+      .map(m -> (MethodTree) m)
+      .collect(Collectors.toList());
+
+    if (constructors.size() > 1) {
+      boolean isAutowiredAlreadyFound = false;
+      for (MethodTree constructor : constructors) {
+        boolean isAutowired = checkConstructor(constructor, isAutowiredAlreadyFound);
+        if (isAutowired) {
+          isAutowiredAlreadyFound = true;
+        }
+      }
+    }
+  }
+
+  private boolean checkConstructor(MethodTree methodTree, boolean isAutowiredAlreadyFound) {
+    boolean isAutowired = isAutowired(methodTree.symbol());
+
+    if (isAutowiredAlreadyFound && isAutowired) {
+      Optional<AnnotationTree> autowiredAnnotation = methodTree.modifiers().annotations().stream()
+        .filter(a -> a.annotationType().symbolType().is(AUTOWIRED_ANNOTATION))
+        .findFirst();
+      autowiredAnnotation.ifPresent(annotationTree -> reportIssue(annotationTree, "Remove this \"@Autowired\" annotation."));
+    }
+
+    return isAutowired;
+  }
+
+  private static boolean isAutowired(Symbol s) {
+    if (s.metadata().isAnnotatedWith(AUTOWIRED_ANNOTATION)) {
+      List<SymbolMetadata.AnnotationValue> annotationValues = s.metadata().valuesForAnnotation(AUTOWIRED_ANNOTATION);
+      return annotationValues.isEmpty() || annotationValues.stream().anyMatch(a -> a.value().equals(true));
+    }
+    return false;
+  }
+
+}

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

@@ -0,0 +1,122 @@
+<h2>Why is this an issue?</h2>
+<p><code>@Autowired</code> is an annotation in the Spring Framework for automatic dependency injection. It tells Spring to automatically provide the
+required dependencies (such as other beans or components) to a class’s fields, methods, or constructors, allowing for easier and more flexible
+management of dependencies in a Spring application. In other words, it’s a way to wire up and inject dependencies into Spring components
+automatically, reducing the need for manual configuration and enhancing modularity and maintainability.</p>
+<p>In any bean class, only one constructor is permitted to declare <code>@Autowired</code> with the <code>required</code> attribute set to true. This
+signifies the constructor to be automatically wired when used as a Spring bean. Consequently, when the required attribute remains at its default value
+(true), only a singular constructor can bear the <code>@Autowired</code> annotation. In cases where multiple constructors have this annotation, they
+must all specify <code>required=false</code> to be eligible as candidates for auto-wiring.</p>
+<h2>How to fix it</h2>
+<p>To maintain code clarity and ensure that the Spring context can create beans correctly, have only one constructor annotated with
+<code>@Autowired</code> within a Spring component or set <code>required = false</code>.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+@Component
+public class MyComponent {
+  private final MyService myService;
+
+  @Autowired
+  public MyComponent(MyService myService) {
+    this.myService = myService;
+    // ...
+  }
+
+  @Autowired  // Noncompliant
+  public MyComponent(MyService myService, Integer i) {
+    this.myService = myService;
+    // ...
+  }
+
+  @Autowired  // Noncompliant
+  public MyComponent(MyService myService, Integer i, String s) {
+    this.myService = myService;
+    // ...
+  }
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+@Component
+public class MyComponent {
+  private final MyService myService;
+
+  @Autowired
+  public MyComponent(MyService myService) {
+    this.myService = myService;
+    // ...
+  }
+
+  public MyComponent(MyService myService, Integer i) {
+    this.myService = myService;
+    // ...
+  }
+
+  public MyComponent(MyService myService, Integer i, String s) {
+    this.myService = myService;
+    // ...
+  }
+}
+</pre>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+@Component
+public class MyComponent {
+  private final MyService myService;
+
+  @Autowired
+  public MyComponent(MyService myService) {
+    this.myService = myService;
+    // ...
+  }
+
+  @Autowired  // Noncompliant
+  public MyComponent(MyService myService, Integer i) {
+    this.myService = myService;
+    // ...
+  }
+
+  @Autowired  // Noncompliant
+  public MyComponent(MyService myService, Integer i, String s) {
+    this.myService = myService;
+    // ...
+  }
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="2" data-diff-type="compliant">
+@Component
+public class MyComponent {
+  private final MyService myService;
+
+  @Autowired
+  public MyComponent(MyService myService) {
+    this.myService = myService;
+    // ...
+  }
+
+  @Autowired(required=false)  // Compliant
+  public MyComponent(MyService myService, Integer i) {
+    this.myService = myService;
+    // ...
+  }
+
+  @Autowired(required=false)  // Compliant
+  public MyComponent(MyService myService, Integer i, String s) {
+    this.myService = myService;
+    // ...
+  }
+}
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> <a href="https://docs.spring.io/spring-framework/reference/core/beans/annotation-config/autowired.html">Spring Framework - Using @Autowired</a>
+  </li>
+</ul>
+<h3>Articles &amp; blog posts</h3>
+<ul>
+  <li> <a href="https://www.baeldung.com/spring-autowire">Baeldung - Guide to Spring @Autowired</a> </li>
+</ul>
+

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/S6818.json

@@ -0,0 +1,24 @@
+{
+  "title": "Avoid Using \"@Autowired\" on Multiple Constructors in a Spring Component",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "spring"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6818",
+  "sqKey": "S6818",
+  "scope": "All",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM",
+      "RELIABILITY": "HIGH"
+    },
+    "attribute": "LOGICAL"
+  }
+}

java-checks/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json

@@ -485,6 +485,7 @@
     "S6809",
     "S6810",
     "S6813",
-    "S6814"
+    "S6814",
+    "S6818"
   ]
 }