-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Java: Add 'Useless serialization member in record class' query #19950
Conversation
java/ql/src/Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.ql
Fixed
Show fixed
Hide fixed
17384e2
to
d4cb23a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM—just some trivial comments 👍
@@ -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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
java/ql/src/Violations of Best Practice/Records/UselessMembersOfTheRecordsClass.md
Outdated
Show resolved
Hide resolved
* @kind problem | ||
* @precision very-high | ||
* @problem.severity warning | ||
* @tags quality |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add the query to that .qls before a release happens?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe so. I think that's what we've been doing.
@@ -0,0 +1,24 @@ | |||
/** | |||
* @id java/useless-member-of-the-record-class |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find "the-record-class" weird. Maybe "useless-member-of-a-record-class"? Or just "useless-member-of-record-class"?
@@ -0,0 +1,24 @@ | |||
/** | |||
* @id java/useless-member-of-the-record-class | |||
* @name Useless serialization member of record class |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit nitpicky, but "useless" sounds a bit less formal, just because it also used in a pejorative sense ("you're useless" as an insult). It sounds slightly more formal to say "Serialization member of record class has no effect". Same for query id and alert. But feel free to ignore this, as it is a nitpick.
@owen-mc I pushed an extra commit with the renaming. I changed "useless" to "ignored". |
4d6cec2
to
813ce7d
Compare
This pull request introduces a new query to detect and flag useless serialization-related members in Java 16 record classes. These members are ignored during serialization, and their presence may indicate a misunderstanding of record serialization behavior.
Autofix simply removes the non compliant members. In some cases it adds a comment saying it has removed the member.