SONARJAVA-4649 Implement S6813: Avoid field dependency injection (#4483)

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

@@ -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"));
-    assertThat(differences).isEqualTo("Issues differences: 3304");
+    assertThat(differences).isEqualTo("Issues differences: 3335");
   }
 
   private static Path pathFor(String path) {

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

@@ -2878,5 +2878,11 @@
     "hasTruePositives": false,
     "falseNegatives": 5,
     "falsePositives": 0
+  },
+  {
+    "ruleKey": "S6813",
+    "hasTruePositives": true,
+    "falseNegatives": 31,
+    "falsePositives": 0
   }
 ]

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

@@ -0,0 +1,19 @@
+package checks.spring;
+
+import jakarta.inject.Inject;
+import org.springframework.stereotype.Component;
+
+@Component
+public class FieldDependencyInjectionCheckJakartaSample {
+  @Inject // Noncompliant
+  private String injected;
+
+  @Inject // Compliant
+  public FieldDependencyInjectionCheckJakartaSample(String injected) {
+  }
+
+  @Inject // Compliant
+  public void setInjected(String injected) {
+    this.injected = injected;
+  }
+}

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

@@ -0,0 +1,42 @@
+package checks.spring;
+
+import javax.annotation.Nullable;
+import javax.inject.Inject;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+@Component
+public class FieldDependencyInjectionCheckSample {
+  @Autowired // Noncompliant [[sc=3;ec=13]] {{Remove this field injection and use constructor injection instead.}}
+  private String autowired;
+
+  @Inject // Noncompliant
+  private String injected;
+
+  @Autowired // Noncompliant
+  @Nullable
+  private String injectedAndNullable;
+
+  @Autowired // Compliant
+  public FieldDependencyInjectionCheckSample() {
+  }
+
+  @Inject // Compliant
+  public FieldDependencyInjectionCheckSample(String injected) {
+  }
+
+  @Autowired // Compliant
+  public void setAutowired(String autowired) {
+    this.autowired = autowired;
+  }
+
+  @Inject // Compliant
+  public void setInjected(String injected) {
+    this.injected = injected;
+  }
+
+  private String notInjected;
+
+  @Nullable
+  private String annotatedButNotInjected;
+}

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.AsyncMethodsReturnTypeCheck;
 import org.sonar.java.checks.spring.ControllerWithSessionAttributesCheck;
+import org.sonar.java.checks.spring.FieldDependencyInjectionCheck;
 import org.sonar.java.checks.spring.PersistentEntityUsedAsRequestParameterCheck;
 import org.sonar.java.checks.spring.RequestMappingMethodPublicCheck;
 import org.sonar.java.checks.spring.SpringAntMatcherOrderCheck;
@@ -402,6 +403,7 @@ public final class CheckList {
     ExcessiveContentRequestCheck.class,
     ExpressionComplexityCheck.class,
     ExternalizableClassConstructorCheck.class,
+    FieldDependencyInjectionCheck.class,
     FieldModifierCheck.class,
     FileHeaderCheck.class,
     FilePermissionsCheck.class,

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

@@ -0,0 +1,56 @@
+/*
+ * 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 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.Tree;
+import org.sonar.plugins.java.api.tree.VariableTree;
+
+@Rule(key = "S6813")
+public class FieldDependencyInjectionCheck extends IssuableSubscriptionVisitor {
+  private static final List<String> INJECTION_ANNOTATIONS = List.of(
+    "org.springframework.beans.factory.annotation.Autowired",
+    "javax.inject.Inject",
+    "jakarta.inject.Inject");
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return List.of(Tree.Kind.CLASS);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    var ct = (ClassTree) tree;
+    ct.members().forEach(member -> {
+      if (member.is(Tree.Kind.VARIABLE)) {
+        var vt = (VariableTree) member;
+
+        vt.modifiers().annotations().stream()
+          .filter(annotationTree -> INJECTION_ANNOTATIONS.stream()
+            .anyMatch(targetAnnotation -> annotationTree.symbolType().is(targetAnnotation)))
+          .findFirst()
+          .ifPresent(annotationTree -> reportIssue(annotationTree, "Remove this field injection and use constructor injection instead."));
+      }
+    });
+  }
+}

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

@@ -0,0 +1,40 @@
+<h2>Why is this an issue?</h2>
+<p>Spring supports dependency injection by using annotations such as <code>@Autowired</code>. This annotation can be used to inject beans via
+constructor, setter, and field injection.</p>
+<p>Generally speaking, field injection is discouraged. It allows the creation of objects in an invalid state and makes testing more difficult. The
+dependencies are not explicit when instantiating a class that uses field injection.</p>
+<p>Apart from that, field injection is not compatible with final fields. Keeping dependencies immutable where possible makes the code easier to
+understand, easing development and maintenance.</p>
+<p>This rule raises an issue when the <code>@Autowired</code> or <code>@Inject</code> annotations are used on a field.</p>
+<h2>How to fix it</h2>
+<p>Use constructor injection instead.</p>
+<p>By using constructor injection, the dependencies are explicit and must be passed during an object’s construction. This avoids the possibility of
+instantiating an object in an invalid state and makes types more testable. Fields can be declared final, which makes the code easier to understand, as
+dependencies don’t change after instantiation.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+public class SomeService {
+    @Autowired
+    private SomeDependency someDependency;
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+public class SomeService {
+    private final SomeDependency someDependency;
+
+    @Autowired
+    public SomeService(SomeDependency someDependency) {
+        this.someDependency = someDependency;
+    }
+}
+</pre>
+<h2>Resources</h2>
+<h3>Articles &amp; blog posts</h3>
+<ul>
+  <li> Baeldung - <a href="https://www.baeldung.com/java-spring-field-injection-cons">Why Is Field Injection Not Recommended?</a> </li>
+  <li> Baeldung - <a href="https://www.baeldung.com/constructor-injection-in-spring">Constructor Dependency Injection in Spring</a> </li>
+  <li> Oliver Drotbohm - <a href="https://odrotbohm.de/2013/11/why-field-injection-is-evil/">Why field injection is evil</a> </li>
+</ul>
+

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

@@ -0,0 +1,22 @@
+{
+  "title": "Avoid field dependency injection",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6813",
+  "sqKey": "S6813",
+  "scope": "All",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM",
+      "RELIABILITY": "MEDIUM"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}

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

@@ -480,6 +480,7 @@
     "S6539",
     "S6541",
     "S6548",
-    "S6810"
+    "S6810",
+    "S6813"
   ]
 }

java-checks/src/test/java/org/sonar/java/checks/spring/FieldDependencyInjectionCheckTest.java

@@ -0,0 +1,43 @@
+/*
+ * 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 FieldDependencyInjectionCheckTest {
+  @Test
+  void test() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/spring/FieldDependencyInjectionCheckSample.java"))
+      .withCheck(new FieldDependencyInjectionCheck())
+      .verifyIssues();
+  }
+
+  @Test
+  void testJakarta() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/spring/FieldDependencyInjectionCheckJakartaSample.java"))
+      .withCheck(new FieldDependencyInjectionCheck())
+      .verifyIssues();
+  }
+}