SONARJAVA-5697 S2441 and S2118 Fix FP with missing semantics of Serializable (#5260)

Author: tomasz-tylenda-sonarsource

java-checks-common/src/main/java/org/sonar/java/checks/helpers/ExpressionsHelper.java

@@ -47,6 +47,7 @@
 
 import static org.sonar.java.checks.helpers.ReassignmentFinder.getInitializerOrExpression;
 import static org.sonar.java.checks.helpers.ReassignmentFinder.getReassignments;
+import static org.sonar.java.model.JUtils.hasUnknownTypeInHierarchy;
 
 public class ExpressionsHelper {
 
@@ -169,6 +170,12 @@ public List<JavaFileScannerContext.Location> valuePath() {
     }
   }
 
+  /**
+   * Checks if the expression is non-serializable.
+   *
+   * <p> If the result cannot be determined due to incomplete semantics,
+   * the method returns false.
+   */
   public static boolean isNotSerializable(ExpressionTree expression) {
     Type symbolType = expression.symbolType();
     if (symbolType.isUnknown()) {
@@ -197,6 +204,9 @@ private static boolean isNonSerializable(Type type) {
       type.isSubtypeOf("java.util.Enumeration")) {
       return false;
     }
+    if(hasUnknownTypeInHierarchy(type.symbol())) {
+      return false;
+    }
     Type erasedType = type.erasure();
     return erasedType.equals(type) || isNonSerializable(erasedType);
   }

java-checks-common/src/test/java/org/sonar/java/checks/helpers/ExpressionsHelperTest.java

@@ -20,7 +20,10 @@
 import javax.annotation.Nullable;
 import org.junit.jupiter.api.Test;
 import org.sonar.plugins.java.api.semantic.Symbol;
+import org.sonar.plugins.java.api.tree.ExpressionStatementTree;
+import org.sonar.plugins.java.api.tree.ExpressionTree;
 import org.sonar.plugins.java.api.tree.IdentifierTree;
+import org.sonar.plugins.java.api.tree.MethodInvocationTree;
 import org.sonar.plugins.java.api.tree.MethodTree;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -149,4 +152,53 @@ private <T> void assertValueResolution(String code, @Nullable T target) {
     Boolean value = ExpressionsHelper.getConstantValueAsBoolean(a).value();
     assertThat(value).isEqualTo(target);
   }
+
+  @Test
+  void isNonSerializable_nonSerializable() {
+    String code = newCode(
+      "static class C {}",
+      "private C c;",
+      "void f() {",
+      "  System.out.println(c);",
+      "}"
+    );
+    ExpressionTree expr = getCallArgument(code);
+    assertThat(ExpressionsHelper.isNotSerializable(expr)).isTrue();
+  }
+
+  @Test
+  void isNonSerializable_javaIoSerializable() {
+    String code = newCode(
+      "static class C implements java.io.Serializable {}",
+      "private C c;",
+      "void f() {",
+      "  System.out.println(c);",
+      "}"
+    );
+    ExpressionTree expr = getCallArgument(code);
+    assertThat(ExpressionsHelper.isNotSerializable(expr)).isFalse();
+  }
+
+  @Test
+  void isNonSerializable_missingImportSerializable() {
+    String code = newCode(
+      "static class C implements Serializable {}",
+      "private C c;",
+      "void f() {",
+      "  System.out.println(c);",
+      "}"
+    );
+    ExpressionTree expr = getCallArgument(code);
+    // We want "false" in case we cannot resolve implemented interfaces,
+    // to avoid FPs in the checks that use this helper.
+    assertThat(ExpressionsHelper.isNotSerializable(expr)).isFalse();
+  }
+
+  /** Returns the {@code c} argument to {@code System.out.println(c)}. */
+  private static ExpressionTree getCallArgument(String code) {
+    var methodTree = (MethodTree) classTree(code).members().get(2);
+    var exprStmtTree = (ExpressionStatementTree) methodTree.block().body().get(0);
+    var methodInvocationTree =  (MethodInvocationTree) exprStmtTree.expression();
+    return methodInvocationTree.arguments().get(0);
+  }
 }

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

@@ -0,0 +1,13 @@
+package checks.serialization;
+
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+
+class NonSerializableWriteCheckMissingImportSample {
+  public record R(String foo, Boolean bar) implements Serializable {}
+
+  public void writeOut(ObjectOutputStream oos) throws IOException {
+    R r = new R("foo", true);
+    oos.writeObject(r);
+  }
+}

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

@@ -0,0 +1,14 @@
+package checks.serialization;
+
+import javax.servlet.http.HttpSession;
+
+class SerializableObjectInSessionCheckMissingImportSample {
+  public static record R(String foo, Boolean bar) implements Serializable {}
+
+  private HttpSession session = null;
+
+  public void usage() {
+    R r = new R("foo", true);
+    session.setAttribute("foo", r);
+  }
+}

java-checks/src/test/java/org/sonar/java/checks/serialization/NonSerializableWriteCheckTest.java

@@ -20,6 +20,7 @@
 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 NonSerializableWriteCheckTest {
 
@@ -31,6 +32,14 @@ void test() {
       .verifyIssues();
   }
 
+  @Test
+  void test_missing_import() {
+    CheckVerifier.newVerifier()
+      .onFile(nonCompilingTestSourcesPath("checks/serialization/NonSerializableWriteCheckMissingImportSample.java"))
+      .withCheck(new NonSerializableWriteCheck())
+      .verifyNoIssues();
+  }
+
   @Test
   void unresolved() {
     CheckVerifier.newVerifier()

java-checks/src/test/java/org/sonar/java/checks/serialization/SerializableObjectInSessionCheckTest.java

@@ -20,6 +20,7 @@
 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 SerializableObjectInSessionCheckTest {
 
@@ -31,6 +32,14 @@ void test() {
       .verifyIssues();
   }
 
+  @Test
+  void test_missing_import() {
+    CheckVerifier.newVerifier()
+      .onFile(nonCompilingTestSourcesPath("checks/serialization/SerializableObjectInSessionCheckMissingImportSample.java"))
+      .withCheck(new SerializableObjectInSessionCheck())
+      .verifyNoIssues();
+  }
+
   @Test
   void unresolved() {
     CheckVerifier.newVerifier()