SONARJAVA-4061 S2226 should ignore fields assigned in init method (#…

…4881)

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

@@ -2,9 +2,42 @@
 
 import java.util.function.Function;
 import javax.annotation.Resource;
+import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import org.apache.struts.action.Action;
 
+class S2226FP extends HttpServlet {
+  private int first; // Compliant
+  private long second; // Compliant
+
+  @Override
+  public void init() throws ServletException {
+    this.first = 42;
+    second = this.first * 2;
+  }
+}
+
+class S2226FP2 extends HttpServlet {
+  private int first; // Noncompliant {{Remove this misleading mutable servlet instance field or make it "static" and/or "final"}}
+  private final S2226FP2 inst = null;
+
+  @Override
+  public void init() throws ServletException {
+    inst.first = 42;
+  }
+}
+
+abstract class S2226FP3 extends HttpServlet {
+  private int first; // Noncompliant {{Remove this misleading mutable servlet instance field or make it "static" and/or "final"}}
+
+  @Override
+  public void init() throws ServletException {
+    getInstance().first = 42;
+  }
+
+  abstract S2226FP3 getInstance();
+}
+
 @interface Inject{}
 
 class HttpServletA {
@@ -83,3 +116,4 @@ public static String valueOf(String storageType) {
     }
   }
 }
+

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

@@ -22,6 +22,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import javax.annotation.Nullable;
 import org.sonar.check.Rule;
 import org.sonar.java.model.ModifiersUtils;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
@@ -32,6 +33,7 @@
 import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
 import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
 import org.sonar.plugins.java.api.tree.IdentifierTree;
+import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
 import org.sonar.plugins.java.api.tree.MethodTree;
 import org.sonar.plugins.java.api.tree.Modifier;
 import org.sonar.plugins.java.api.tree.Tree;
@@ -107,11 +109,21 @@ private void reportIssuesOnVariable() {
   private class AssignmentVisitor extends BaseTreeVisitor {
     @Override
     public void visitAssignmentExpression(AssignmentExpressionTree tree) {
-      if (tree.variable().is(Kind.IDENTIFIER)) {
-        Tree declaration = ((IdentifierTree) tree.variable()).symbol().declaration();
-        if (declaration != null && declaration.is(Kind.VARIABLE)) {
-          excludedVariables.add((VariableTree) declaration);
-        }
+      var variable = tree.variable();
+      if (variable instanceof  IdentifierTree identifier) {
+        // handles e.g. "second = this.first * 2;" assignments -> no "this" prefix to member "second"
+        addVariableToExcluded(identifier.symbol().declaration());
+      } else if (variable instanceof MemberSelectExpressionTree memberSelectTree
+        && memberSelectTree.expression() instanceof IdentifierTree identifier
+        && "this".equals(identifier.identifierToken().text())) {
+        // handles e.g. "this.first = 42;" assignments -> member "first" is prefixed with "this."
+        addVariableToExcluded(memberSelectTree.identifier().symbol().declaration());
+      }
+    }
+    private void addVariableToExcluded(@Nullable Tree declaration) {
+      // if declaration set, and a variable, then add to excluded
+      if (declaration != null && declaration.is(Kind.VARIABLE)) {
+        excludedVariables.add((VariableTree) declaration);
       }
     }
   }