SONARJAVA-6588 - Implement S8983: Stateless session beans should not store mutable state in instance variables

its/autoscan/src/test/resources/autoscan/diffs/diff_S6813.json

@@ -1,6 +1,6 @@
 {
   "ruleKey": "S6813",
   "hasTruePositives": true,
-  "falseNegatives": 70,
+  "falseNegatives": 71,
   "falsePositives": 0
 }

its/autoscan/src/test/resources/autoscan/diffs/diff_S8983.json

@@ -0,0 +1,6 @@
+{
+  "ruleKey": "S8983",
+  "hasTruePositives": false,
+  "falseNegatives": 6,
+  "falsePositives": 0
+}

java-checks-test-sources/default/src/main/files/non-compiling/checks/StatelessBeanInstanceFieldCheckJakartaSample.java

@@ -0,0 +1,59 @@
+package checks;
+
+import jakarta.annotation.Resource;
+import jakarta.ejb.EJB;
+import jakarta.ejb.Singleton;
+import jakarta.ejb.Stateful;
+import jakarta.ejb.Stateless;
+import jakarta.inject.Inject;
+import jakarta.persistence.EntityManager;
+import jakarta.persistence.EntityManagerFactory;
+import jakarta.persistence.PersistenceContext;
+import jakarta.persistence.PersistenceUnit;
+import jakarta.xml.ws.WebServiceRef;
+
+@Stateless
+class StatelessWithMutableFieldsJakarta {
+  private int requestCount;           // Noncompliant {{Remove this mutable instance field or replace it with a local variable, a "static final" constant, or an injected resource.}}
+  //          ^^^^^^^^^^^^
+  private String lastClientId;        // Noncompliant
+  private Object cachedResult = null; // Noncompliant
+  public int publicField;             // Noncompliant
+  protected int protectedField;       // Noncompliant
+}
+
+@Stateless
+class StatelessWithSafeFieldsJakarta {
+  private static final int MAX_RETRIES = 3; // Compliant - static final constant
+  private static int sharedCounter;          // Compliant - static field
+  private final String name = "service";     // Compliant - final field
+
+  @EJB
+  private StatelessWithSafeFieldsJakarta ejbRef; // Compliant - injected EJB
+
+  @Inject
+  private Object cdiBean;                    // Compliant - CDI injection
+
+  @PersistenceContext
+  private EntityManager em;                  // Compliant - injected persistence context
+
+  @PersistenceUnit
+  private EntityManagerFactory emf;          // Compliant - injected persistence unit
+
+  @Resource
+  private javax.sql.DataSource dataSource;   // Compliant - injected resource
+
+  @WebServiceRef
+  private Object wsRef;                      // Compliant - injected web service reference
+}
+
+@Stateful
+class StatefulWithMutableFieldsJakarta {
+  private int requestCount; // Compliant - @Stateful beans are designed to hold state
+  private String lastClientId;
+}
+
+@Singleton
+class SingletonWithMutableFieldsJakarta {
+  private int requestCount; // Compliant - not a @Stateless bean
+}

java-checks-test-sources/default/src/main/java/checks/StatelessBeanInstanceFieldCheckSample.java

@@ -0,0 +1,63 @@
+package checks;
+
+import javax.annotation.Resource;
+import javax.ejb.EJB;
+import javax.ejb.Singleton;
+import javax.ejb.Stateful;
+import javax.ejb.Stateless;
+import javax.inject.Inject;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.PersistenceContext;
+import javax.persistence.PersistenceUnit;
+import javax.xml.ws.WebServiceRef;
+
+@Stateless
+class StatelessWithMutableFields {
+  private int requestCount;           // Noncompliant {{Remove this mutable instance field or replace it with a local variable, a "static final" constant, or an injected resource.}}
+  //          ^^^^^^^^^^^^
+  private String lastClientId;        // Noncompliant
+  private Object cachedResult = null; // Noncompliant
+  public int publicField;             // Noncompliant
+  protected int protectedField;       // Noncompliant
+}
+
+@Stateless
+class StatelessWithSafeFields {
+  private static final int MAX_RETRIES = 3; // Compliant - static final constant
+  private static int sharedCounter;          // Compliant - static field
+  private final String name = "service";     // Compliant - final field
+
+  @EJB
+  private StatelessWithSafeFields ejbRef;    // Compliant - injected EJB
+
+  @Inject
+  private Object cdiBean;                    // Compliant - CDI injection
+
+  @PersistenceContext
+  private EntityManager em;                  // Compliant - injected persistence context
+
+  @PersistenceUnit
+  private EntityManagerFactory emf;          // Compliant - injected persistence unit
+
+  @Resource
+  private javax.sql.DataSource dataSource;   // Compliant - injected resource
+
+  @WebServiceRef
+  private Object wsRef;                      // Compliant - injected web service reference
+}
+
+@Stateful
+class StatefulWithMutableFields {
+  private int requestCount; // Compliant - @Stateful beans are designed to hold state
+  private String lastClientId;
+}
+
+@Singleton
+class SingletonWithMutableFields {
+  private int requestCount; // Compliant - not a @Stateless bean
+}
+
+class PlainClassWithMutableFields {
+  private int requestCount; // Compliant - not an EJB at all
+}

java-checks-test-sources/default/src/main/java/jakarta/ejb/EJB.java

@@ -0,0 +1,14 @@
+package jakarta.ejb;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+@Retention(RUNTIME)
+@Target({TYPE, METHOD, FIELD})
+public @interface EJB {
+}

java-checks-test-sources/default/src/main/java/jakarta/xml/ws/WebServiceRef.java

@@ -0,0 +1,14 @@
+package jakarta.xml.ws;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+@Retention(RUNTIME)
+@Target({TYPE, METHOD, FIELD})
+public @interface WebServiceRef {
+}

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

@@ -0,0 +1,85 @@
+/*
+ * SonarQube Java
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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 Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.java.checks;
+
+import java.util.List;
+import org.sonar.check.Rule;
+import org.sonar.java.model.ModifiersUtils;
+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.Modifier;
+import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.VariableTree;
+
+@Rule(key = "S8983")
+public class StatelessBeanInstanceFieldCheck extends IssuableSubscriptionVisitor {
+
+  private static final List<String> STATELESS_ANNOTATIONS = List.of(
+    "javax.ejb.Stateless",
+    "jakarta.ejb.Stateless"
+  );
+
+  private static final List<String> SAFE_FIELD_ANNOTATIONS = List.of(
+    "javax.ejb.EJB",
+    "jakarta.ejb.EJB",
+    "javax.inject.Inject",
+    "jakarta.inject.Inject",
+    "javax.persistence.PersistenceContext",
+    "jakarta.persistence.PersistenceContext",
+    "javax.persistence.PersistenceUnit",
+    "jakarta.persistence.PersistenceUnit",
+    "javax.annotation.Resource",
+    "jakarta.annotation.Resource",
+    "javax.xml.ws.WebServiceRef",
+    "jakarta.xml.ws.WebServiceRef"
+  );
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return List.of(Tree.Kind.VARIABLE);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    var variable = (VariableTree) tree;
+    if (isInstanceFieldOfStatelessBean(variable) && !isSafe(variable)) {
+      reportIssue(variable.simpleName(),
+        "Remove this mutable instance field or replace it with a local variable, a \"static final\" constant, or an injected resource.");
+    }
+  }
+
+  private static boolean isInstanceFieldOfStatelessBean(VariableTree variable) {
+    if (!variable.parent().is(Tree.Kind.CLASS)) {
+      return false;
+    }
+    Symbol owner = variable.symbol().owner();
+    if (!owner.isTypeSymbol()) {
+      return false;
+    }
+    SymbolMetadata ownerMetadata = owner.metadata();
+    return STATELESS_ANNOTATIONS.stream().anyMatch(ownerMetadata::isAnnotatedWith);
+  }
+
+  private static boolean isSafe(VariableTree variable) {
+    if (ModifiersUtils.hasAnyOf(variable.modifiers(), Modifier.STATIC, Modifier.FINAL)) {
+      return true;
+    }
+    SymbolMetadata varMetadata = variable.symbol().metadata();
+    return SAFE_FIELD_ANNOTATIONS.stream().anyMatch(varMetadata::isAnnotatedWith);
+  }
+}

java-checks/src/test/java/org/sonar/java/checks/StatelessBeanInstanceFieldCheckTest.java

@@ -0,0 +1,51 @@
+/*
+ * SonarQube Java
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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 Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.java.checks;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.java.checks.verifier.CheckVerifier;
+
+import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
+import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath;
+
+class StatelessBeanInstanceFieldCheckTest {
+
+  @Test
+  void test() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/StatelessBeanInstanceFieldCheckSample.java"))
+      .withCheck(new StatelessBeanInstanceFieldCheck())
+      .verifyIssues();
+  }
+
+  @Test
+  void test_jakarta() {
+    CheckVerifier.newVerifier()
+      .onFile(nonCompilingTestSourcesPath("checks/StatelessBeanInstanceFieldCheckJakartaSample.java"))
+      .withCheck(new StatelessBeanInstanceFieldCheck())
+      .verifyIssues();
+  }
+
+  @Test
+  void test_without_semantic() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/StatelessBeanInstanceFieldCheckSample.java"))
+      .withCheck(new StatelessBeanInstanceFieldCheck())
+      .withoutSemantic()
+      .verifyNoIssues();
+  }
+}