Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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/UselessMembersOfTheRecordsClass.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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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/UselessMembersOfTheRecordsClass.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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
## Overview

Record types were introduced in Java 16 as a mechanism to provide simpler data handling that is an alternative to regular classes. Record classes behave slightly differently during serialization however, 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Record types were introduced in Java 16 as a mechanism to provide simpler data handling that is an alternative to regular classes. Record classes behave slightly differently during serialization however, 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.
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.
Some level of serialization customization is offered by the Java 16 Record feature. The `writeReplace` and `readResolve` methods in a record that implement `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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should still be implements, as it's a record that implements ....


## 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)
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* @id java/useless-member-of-the-record-class
* @name Useless 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a new query, the ccr.qls suite needs to be updated in the Autofix repo (otherwise the query will not get any autofix suggestions).

* reliability
* correctness
*/

import java

from Record record, Member m
where
record.getAMember() = m and
m.hasName([
"writeObject", "readObject", "readObjectNoData", "writeExternal", "readExternal",
"serialPersistentFields"
])
select m, "Useless serialization member found in record class $@.", record, record.getName()
Original file line number Diff line number Diff line change
@@ -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();
}
}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
| Test.java:7:46:7:67 | serialPersistentFields | Useless serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 |
| Test.java:10:18:10:28 | writeObject | Useless serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 |
| Test.java:13:18:13:27 | readObject | Useless serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 |
| Test.java:16:18:16:33 | readObjectNoData | Useless serialization member found in record class $@. | Test.java:4:12:4:13 | T1 | T1 |
| Test.java:24:17:24:29 | writeExternal | Useless serialization member found in record class $@. | Test.java:21:12:21:13 | T2 | T2 |
| Test.java:28:17:28:28 | readExternal | Useless serialization member found in record class $@. | Test.java:21:12:21:13 | T2 | T2 |
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.ql
postprocess: utils/test/InlineExpectationsTestQuery.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
//semmle-extractor-options: --javac-args -source 16 -target 16
Loading