SONARJAVA-6506 Avoid suggesting records for DTO contracts

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

@@ -28,4 +28,17 @@ public int getI() {
       return i;
     }
   }
+
+  @UnknownFrameworkAnnotation
+  public final class UnknownAnnotatedClass { // Compliant, unknown annotations may represent framework contracts
+    private final int i;
+
+    public UnknownAnnotatedClass(final int i) {
+      this.i = i;
+    }
+
+    public int getI() {
+      return i;
+    }
+  }
 }

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

@@ -1,5 +1,17 @@
 package checks;
 
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.io.Externalizable;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutput;
+import java.io.ObjectOutputStream;
+import java.io.ObjectStreamException;
+import java.io.ObjectStreamField;
+import java.io.Serializable;
 import java.util.Optional;
 
 public class RecordInsteadOfClassCheckSample {
@@ -146,6 +158,10 @@ final class ClassWithPrivateNonFinalField { private int base; }
   final class ClassWithPublicFinalField { public final int base = 0; }
   final class ClassWithoutFields { }
   final class ClassWithoutFinalFields { private int sum; }
+  Object anonymousClass = new Object() {
+    private final int sum = 0;
+    int getSum() { return sum; }
+  };
   abstract class AbstractClass { abstract void foo(); }
   interface NotAClass { void foo(); }
 
@@ -167,6 +183,103 @@ public Optional<String> bar() { // Not the same type as the field bar.
     }
   }
 
+  final class SerializableClass implements Serializable { // Compliant, records have different serialization behavior
+    private final int sum;
+
+    SerializableClass(int sum) { this.sum = sum; }
+    int getSum() { return sum; }
+  }
+
+  final class ExternalizableClass implements Externalizable { // Compliant, records have different serialization behavior
+    private final int sum;
+
+    ExternalizableClass(int sum) { this.sum = sum; }
+    int getSum() { return sum; }
+    public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { }
+    public void writeExternal(ObjectOutput out) throws IOException { }
+  }
+
+  final class ClassWithWriteObject {
+    private final int sum;
+
+    ClassWithWriteObject(int sum) { this.sum = sum; }
+    int getSum() { return sum; }
+    private void writeObject(ObjectOutputStream out) throws IOException { }
+  }
+
+  final class ClassWithReadObject {
+    private final int sum;
+
+    ClassWithReadObject(int sum) { this.sum = sum; }
+    int getSum() { return sum; }
+    private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { }
+  }
+
+  final class ClassWithReadObjectNoData {
+    private final int sum;
+
+    ClassWithReadObjectNoData(int sum) { this.sum = sum; }
+    int getSum() { return sum; }
+    private void readObjectNoData() throws ObjectStreamException { }
+  }
+
+  final class ClassWithWriteReplace {
+    private final int sum;
+
+    ClassWithWriteReplace(int sum) { this.sum = sum; }
+    int getSum() { return sum; }
+    private Object writeReplace() throws ObjectStreamException { return this; }
+  }
+
+  final class ClassWithReadResolve {
+    private final int sum;
+
+    ClassWithReadResolve(int sum) { this.sum = sum; }
+    int getSum() { return sum; }
+    private Object readResolve() throws ObjectStreamException { return this; }
+  }
+
+  final class ClassWithSerialPersistentFields {
+    private static final ObjectStreamField[] serialPersistentFields = new ObjectStreamField[0];
+    private final int sum;
+
+    ClassWithSerialPersistentFields(int sum) { this.sum = sum; }
+    int getSum() { return sum; }
+  }
+
+  @JsonIgnoreProperties(ignoreUnknown = true)
+  final class ClassWithJsonAnnotation { // Compliant, framework metadata owns the class shape
+    private final int sum;
+
+    ClassWithJsonAnnotation(int sum) { this.sum = sum; }
+    int getSum() { return sum; }
+  }
+
+  final class ClassWithJsonCreatorConstructor {
+    private final int sum;
+
+    @JsonCreator
+    ClassWithJsonCreatorConstructor(int sum) { this.sum = sum; }
+    int getSum() { return sum; }
+  }
+
+  final class ClassWithJsonAnnotatedField {
+    @JsonProperty("total")
+    private final int sum;
+
+    ClassWithJsonAnnotatedField(int sum) { this.sum = sum; }
+    int getSum() { return sum; }
+  }
+
+  final class ClassWithJsonAnnotatedGetter {
+    private final int sum;
+
+    ClassWithJsonAnnotatedGetter(int sum) { this.sum = sum; }
+
+    @JsonProperty("total")
+    int getSum() { return sum; }
+  }
+
   // When the constructor has smaller visibility, it is not possible to create a record with the same behavior.
   // Order: Public > protected > package private > private
 

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

@@ -21,25 +21,79 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.SpringUtils;
 import org.sonar.plugins.java.api.JavaVersionAwareVisitor;
 import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
 import org.sonar.plugins.java.api.JavaVersion;
+import org.sonar.plugins.java.api.semantic.MethodMatchers;
 import org.sonar.plugins.java.api.semantic.Symbol;
+import org.sonar.plugins.java.api.semantic.SymbolMetadata;
 import org.sonar.plugins.java.api.semantic.Type;
 import org.sonar.plugins.java.api.tree.ArrayTypeTree;
 import org.sonar.plugins.java.api.tree.ClassTree;
 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.ParameterizedTypeTree;
 import org.sonar.plugins.java.api.tree.PrimitiveTypeTree;
 import org.sonar.plugins.java.api.tree.Tree;
 import org.sonar.plugins.java.api.tree.TypeTree;
 import org.sonar.plugins.java.api.tree.VariableTree;
 
+import static org.sonar.java.checks.helpers.AnnotationsHelper.hasUnknownAnnotation;
+
 @Rule(key = "S6206")
 public class RecordInsteadOfClassCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor {
 
+  private static final String JAVA_IO_EXTERNALIZABLE = "java.io.Externalizable";
+  private static final String JAVA_IO_SERIALIZABLE = "java.io.Serializable";
+
+  private static final Set<String> JACKSON_ANNOTATION_PACKAGES = Set.of(
+    "com.fasterxml.jackson.annotation.",
+    "com.fasterxml.jackson.databind.annotation.");
+  private static final Set<String> GSON_ANNOTATION_PACKAGES = Set.of("com.google.gson.annotations.");
+  private static final Set<String> MICRONAUT_ANNOTATION_PACKAGES = Set.of(
+    "io.micronaut.core.annotation.",
+    "io.micronaut.data.annotation.",
+    "io.micronaut.serde.annotation.");
+  private static final Set<String> JAKARTA_EE_ANNOTATION_PACKAGES = Set.of(
+    "jakarta.inject.",
+    "jakarta.persistence.",
+    "jakarta.xml.bind.annotation.");
+  private static final Set<String> JAVA_EE_ANNOTATION_PACKAGES = Set.of(
+    "javax.inject.",
+    "javax.persistence.",
+    "javax.xml.bind.annotation.");
+  private static final Set<String> LOMBOK_ANNOTATION_PACKAGES = Set.of("lombok.");
+  private static final Set<String> SPRING_ANNOTATION_PACKAGES = Set.of(
+    SpringUtils.BEANS_FACTORY_ANNOTATION_PACKAGE,
+    SpringUtils.BOOT_CONTEXT_PROPERTIES_PACKAGE,
+    SpringUtils.DATA_PACKAGE + "annotation.");
+
+  private static final Set<String> FRAMEWORK_ANNOTATION_PREFIXES = Stream.of(
+    JACKSON_ANNOTATION_PACKAGES,
+    GSON_ANNOTATION_PACKAGES,
+    MICRONAUT_ANNOTATION_PACKAGES,
+    JAKARTA_EE_ANNOTATION_PACKAGES,
+    JAVA_EE_ANNOTATION_PACKAGES,
+    LOMBOK_ANNOTATION_PACKAGES,
+    SPRING_ANNOTATION_PACKAGES)
+    .flatMap(Set::stream)
+    .collect(Collectors.toUnmodifiableSet());
+
+  private static final MethodMatchers SERIALIZATION_CONTRACT_METHODS = MethodMatchers.or(
+    methodMatcher("readObject", "java.io.ObjectInputStream"),
+    methodMatcher("writeObject", "java.io.ObjectOutputStream"),
+    methodMatcher("readExternal", "java.io.ObjectInput"),
+    methodMatcher("writeExternal", "java.io.ObjectOutput"),
+    MethodMatchers.create()
+      .ofAnyType()
+      .names("readObjectNoData", "writeReplace", "readResolve")
+      .addWithoutParametersMatcher()
+      .build());
+
   @Override
   public boolean isCompatibleWithJavaVersion(JavaVersion version) {
     return version.isJava16Compatible();
@@ -53,6 +107,10 @@ public List<Tree.Kind> nodesToVisit() {
   @Override
   public void visitNode(Tree tree) {
     ClassTree classTree = (ClassTree) tree;
+    if (classTree.simpleName() == null) {
+      // Anonymous classes can not be converted to records.
+      return;
+    }
     if (classTree.superClass() != null) {
       // records can not extends other classes
       return;
@@ -66,27 +124,92 @@ public void visitNode(Tree tree) {
       // records can not be extended
       return;
     }
+    if (hasSerializationContract(classTree)) {
+      // records have special serialization behavior, so this refactoring is not behavior-preserving.
+      return;
+    }
 
     List<Symbol.VariableSymbol> fields = classFields(classSymbol);
     if (fields.isEmpty() || !hasOnlyPrivateFinalFields(fields)) {
       return;
     }
     List<Symbol.MethodSymbol> methods = classMethods(classSymbol);
-    Map<String, Type> fieldsNameToType = fields.stream().collect(Collectors.toMap(Symbol::name, Symbol::type));
-
-    if (!hasGetterForEveryField(methods, fieldsNameToType)) {
-      return;
-    }
     List<Symbol.MethodSymbol> constructors = classConstructors(methods);
     if (constructors.size() != 1) {
       return;
     }
     Symbol.MethodSymbol constructor = constructors.get(0);
+    Map<String, Type> fieldsNameToType = fields.stream().collect(Collectors.toMap(Symbol::name, Symbol::type));
+    if (hasFrameworkContract(classSymbol, fields, methods, fieldsNameToType, constructor)) {
+      return;
+    }
+
+    if (!hasGetterForEveryField(methods, fieldsNameToType)) {
+      return;
+    }
     if (hasParameterForEveryField(constructor, fieldsNameToType.keySet()) && !constructorHasSmallerVisibility(constructor, classSymbol)) {
       reportIssue(classTree.simpleName(), String.format("Refactor this class declaration to use 'record %s'.", recordName(classTree, constructor)));
     }
   }
 
+  private static boolean hasSerializationContract(ClassTree classTree) {
+    Type type = classTree.symbol().type();
+    return type.isSubtypeOf(JAVA_IO_SERIALIZABLE)
+      || type.isSubtypeOf(JAVA_IO_EXTERNALIZABLE)
+      || classTree.members().stream().anyMatch(RecordInsteadOfClassCheck::isSerializationContractMember);
+  }
+
+  private static boolean isSerializationContractMember(Tree member) {
+    if (member.is(Tree.Kind.METHOD)) {
+      return SERIALIZATION_CONTRACT_METHODS.matches((MethodTree) member);
+    }
+    if (member.is(Tree.Kind.VARIABLE)) {
+      return isSerialPersistentFields(((VariableTree) member).symbol());
+    }
+    return false;
+  }
+
+  private static boolean isSerialPersistentFields(Symbol field) {
+    return "serialPersistentFields".equals(field.name())
+      && field.isPrivate()
+      && field.isStatic()
+      && field.isFinal()
+      && field.type().is("java.io.ObjectStreamField[]");
+  }
+
+  private static MethodMatchers methodMatcher(String methodName, String parameterType) {
+    return MethodMatchers.create()
+      .ofAnyType()
+      .names(methodName)
+      .addParametersMatcher(parameterType)
+      .build();
+  }
+
+  private static boolean hasFrameworkContract(
+    Symbol.TypeSymbol classSymbol,
+    List<Symbol.VariableSymbol> fields,
+    List<Symbol.MethodSymbol> methods,
+    Map<String, Type> fieldsNameToType,
+    Symbol.MethodSymbol constructor) {
+
+    return hasFrameworkAnnotation(classSymbol.metadata())
+      || hasFrameworkAnnotation(constructor.metadata())
+      || fields.stream().anyMatch(field -> hasFrameworkAnnotation(field.metadata()))
+      || methods.stream()
+        .filter(method -> isGetter(method, fieldsNameToType))
+        .anyMatch(method -> hasFrameworkAnnotation(method.metadata()));
+  }
+
+  private static boolean hasFrameworkAnnotation(SymbolMetadata metadata) {
+    return hasUnknownAnnotation(metadata) || metadata.annotations().stream().anyMatch(RecordInsteadOfClassCheck::isFrameworkAnnotation);
+  }
+
+  private static boolean isFrameworkAnnotation(SymbolMetadata.AnnotationInstance annotation) {
+    Type annotationType = annotation.symbol().type();
+    return !annotationType.isUnknown()
+      && FRAMEWORK_ANNOTATION_PREFIXES.stream().anyMatch(annotationType.fullyQualifiedName()::startsWith);
+  }
+
   private static boolean constructorHasSmallerVisibility(Symbol.MethodSymbol constructor, Symbol.TypeSymbol classSymbol) {
     boolean constructorIsPrivate = constructor.isPrivate();
     boolean constructorIsPackageVisibility = constructor.isPackageVisibility();

java-checks/src/main/java/org/sonar/java/checks/helpers/SpringUtils.java

@@ -26,19 +26,24 @@
 
 public final class SpringUtils {
 
+  public static final String BEANS_FACTORY_ANNOTATION_PACKAGE = "org.springframework.beans.factory.annotation.";
+  public static final String BOOT_CONTEXT_PROPERTIES_PACKAGE = "org.springframework.boot.context.properties.";
+  public static final String CONTEXT_ANNOTATION_PACKAGE = "org.springframework.context.annotation.";
+  public static final String DATA_PACKAGE = "org.springframework.data.";
+
   public static final String SPRING_BOOT_APP_ANNOTATION = "org.springframework.boot.autoconfigure.SpringBootApplication";
   public static final String CONTROLLER_ANNOTATION = "org.springframework.stereotype.Controller";
   public static final String COMPONENT_ANNOTATION = "org.springframework.stereotype.Component";
   public static final String REPOSITORY_ANNOTATION = "org.springframework.stereotype.Repository";
   public static final String SERVICE_ANNOTATION = "org.springframework.stereotype.Service";
-  public static final String AUTOWIRED_ANNOTATION = "org.springframework.beans.factory.annotation.Autowired";
-  public static final String VALUE_ANNOTATION = "org.springframework.beans.factory.annotation.Value";
+  public static final String AUTOWIRED_ANNOTATION = BEANS_FACTORY_ANNOTATION_PACKAGE + "Autowired";
+  public static final String VALUE_ANNOTATION = BEANS_FACTORY_ANNOTATION_PACKAGE + "Value";
   public static final String TRANSACTIONAL_ANNOTATION = "org.springframework.transaction.annotation.Transactional";
-  public static final String BEAN_ANNOTATION = "org.springframework.context.annotation.Bean";
-  public static final String SCOPE_ANNOTATION = "org.springframework.context.annotation.Scope";
-  public static final String CONFIGURATION_ANNOTATION = "org.springframework.context.annotation.Configuration";
+  public static final String BEAN_ANNOTATION = CONTEXT_ANNOTATION_PACKAGE + "Bean";
+  public static final String SCOPE_ANNOTATION = CONTEXT_ANNOTATION_PACKAGE + "Scope";
+  public static final String CONFIGURATION_ANNOTATION = CONTEXT_ANNOTATION_PACKAGE + "Configuration";
   public static final String ASYNC_ANNOTATION = "org.springframework.scheduling.annotation.Async";
-  public static final String DATA_REPOSITORY_ANNOTATION = "org.springframework.data.repository.Repository";
+  public static final String DATA_REPOSITORY_ANNOTATION = DATA_PACKAGE + "repository.Repository";
   public static final String REST_CONTROLLER_ANNOTATION = "org.springframework.web.bind.annotation.RestController";
   public static final String SPRING_BOOT_TEST_ANNOTATION = "org.springframework.boot.test.context.SpringBootTest";