Merge pull request #19950 from tamasvajk/quality/useless-record-member

Java: Add 'Useless serialization member in record class' query

Author: tamasvajk

java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected

@@ -77,6 +77,7 @@ ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingMethodNam
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/LocalShadowsFieldConfusing.ql
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/SameNameAsSuper.ql
+ql/java/ql/src/Violations of Best Practice/Records/IgnoredSerializationMembersOfRecordClass.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DoNotCallFinalize.ql

java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected

@@ -75,6 +75,7 @@ ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingMethodNam
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/ConfusingOverloading.ql
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/LocalShadowsFieldConfusing.ql
 ql/java/ql/src/Violations of Best Practice/Naming Conventions/SameNameAsSuper.ql
+ql/java/ql/src/Violations of Best Practice/Records/IgnoredSerializationMembersOfRecordClass.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DoNotCallFinalize.ql

java/ql/src/Violations of Best Practice/Records/IgnoredSerializationMembersOfRecordClass.md

@@ -0,0 +1,54 @@
+## Overview
+
+Record types were introduced in Java 16 as a mechanism to provide simpler data handling as an alternative to regular classes. However, record classes behave slightly differently during serialization. Namely any `writeObject`, `readObject`, `readObjectNoData`, `writeExternal`, and `readExternal` methods and `serialPersistentFields` fields declared in these classes cannot be used to affect the serialization process of any `Record` data type.
+
+## Recommendation
+
+Some level of serialization customization is offered by the Java 16 Record feature. The `writeReplace` and `readResolve` methods in a record that implements `java.io.Serializable` can be used to replace the object to be serialized. Otherwise, no further customization of serialization of records is possible, and it is better to consider using a regular class implementing `java.io.Serializable` or `java.io.Externalizable` when customization is needed.
+
+## Example
+
+```java
+record T1() implements Serializable {
+
+    @Serial
+    private static final ObjectStreamField[] serialPersistentFields = new ObjectStreamField[0]; // NON_COMPLIANT
+
+    @Serial
+    private void writeObject(ObjectOutputStream out) throws IOException {} // NON_COMPLIANT
+
+    @Serial
+    private void readObject(ObjectOutputStream out) throws IOException {}// NON_COMPLIANT
+
+    @Serial
+    private void readObjectNoData(ObjectOutputStream out) throws IOException { // NON_COMPLIANT
+    }
+}
+
+record T2() implements Externalizable {
+
+    @Override
+    public void writeExternal(ObjectOutput out) throws IOException { // NON_COMPLIANT
+    }
+
+    @Override
+    public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { // NON_COMPLIANT
+    }
+}
+
+record T3() implements Serializable {
+
+    public Object writeReplace(ObjectOutput out) throws ObjectStreamException { // COMPLIANT
+        return new Object();
+    }
+
+    public Object readResolve(ObjectInput in) throws ObjectStreamException { // COMPLIANT
+        return new Object();
+    }
+}
+```
+
+## References
+
+- Oracle Serialization Documentation: [Serialization of Records](https://docs.oracle.com/en/java/javase/16/docs/specs/serialization/serial-arch.html#serialization-of-records)
+- Java Record: [Feature Specification](https://openjdk.org/jeps/395)

java/ql/src/Violations of Best Practice/Records/IgnoredSerializationMembersOfRecordClass.ql

@@ -0,0 +1,24 @@
+/**
+ * @id java/ignored-serialization-member-of-record-class
+ * @name Ignored serialization member of record class
+ * @description Using certain members of a record class during serialization will result in
+ *              those members being ignored.
+ * @previous-id java/useless-members-of-the-records-class
+ * @kind problem
+ * @precision very-high
+ * @problem.severity warning
+ * @tags quality
+ *       reliability
+ *       correctness
+ */
+
+import java
+
+from Record record, Member m
+where
+  record.getAMember() = m and
+  m.hasName([
+      "writeObject", "readObject", "readObjectNoData", "writeExternal", "readExternal",
+      "serialPersistentFields"
+    ])
+select m, "Ignored serialization member found in record class $@.", record, record.getName()

java/ql/test/query-tests/IgnoredSerializationMembersOfRecordClass/IgnoredSerializationMembersOfRecordClass.expected

@@ -0,0 +1,6 @@
+| Test.java:7:46:7:67 | serialPersistentFields | Ignored serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 |
+| Test.java:10:18:10:28 | writeObject | Ignored serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 |
+| Test.java:13:18:13:27 | readObject | Ignored serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 |
+| Test.java:16:18:16:33 | readObjectNoData | Ignored serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 |
+| Test.java:24:17:24:29 | writeExternal | Ignored serialization member found in record class $@. | Test.java:21:12:21:13 | T2 | T2 |
+| Test.java:28:17:28:28 | readExternal | Ignored serialization member found in record class $@. | Test.java:21:12:21:13 | T2 | T2 |

java/ql/test/query-tests/IgnoredSerializationMembersOfRecordClass/IgnoredSerializationMembersOfRecordClass.qlref

@@ -0,0 +1,2 @@
+query: Violations of Best Practice/Records/IgnoredSerializationMembersOfRecordClass.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

java/ql/test/query-tests/IgnoredSerializationMembersOfRecordClass/Test.java

@@ -0,0 +1,42 @@
+import java.io.*;
+
+public class Test {
+    record T1() implements Serializable {
+
+    @Serial
+    private static final ObjectStreamField[] serialPersistentFields = new ObjectStreamField[0]; // $ Alert
+
+    @Serial
+    private void writeObject(ObjectOutputStream out) throws IOException {} // $ Alert
+
+    @Serial
+    private void readObject(ObjectOutputStream out) throws IOException {} // $ Alert
+
+    @Serial
+    private void readObjectNoData(ObjectOutputStream out) throws IOException { // $ Alert
+    }
+
+}
+
+    record T2() implements Externalizable {
+
+    @Override
+    public void writeExternal(ObjectOutput out) throws IOException { // $ Alert
+    }
+
+    @Override
+    public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { // $ Alert
+    }
+
+    }
+
+    record T3() implements Serializable {
+
+    public Object writeReplace(ObjectOutput out) throws ObjectStreamException { // COMPLIANT
+        return new Object();
+    }
+
+    public Object readResolve(ObjectInput in) throws ObjectStreamException { // COMPLIANT
+        return new Object();
+    }
+}}

java/ql/test/query-tests/IgnoredSerializationMembersOfRecordClass/options

@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -source 16 -target 16