[java] Implement rule NonSerializableClass

[adangel]

Related issues

• Fixes [java] Rename BeanMembersShouldSerialize to NonSerializableClass #4176
• Fixes [java] BeanMembersShouldSerialize is extremely noisy #1668

Note: We still need a replacement for BeanMembersShouldSerialize, see #4177

Not sure, whether we should deprecate BeanMembersShouldSerialize without replacements and just add NonSerializableClass as a new rule. The diff shows, that the implementation is completely different...

Ready?

• Added unit tests for fixed bug/feature
• Passing all unit tests
• Complete build ./mvnw clean verify passes (checked automatically by github actions)
• Added (in-code) documentation (if needed)

[pmd-test]

	1 Message
📖	Compared to master: This changeset changes 0 violations, introduces 34 new violations, 0 new errors and 0 new configuration errors, removes 19809 violations, 0 errors and 0 configuration errors. Full report
✅	Compared to master: This changeset changes 0 violations, introduces 34 new violations, 2 new errors and 0 new configuration errors, removes 19809 violations, 0 errors and 0 configuration errors. Full report
✅	Compared to master: This changeset changes 0 violations, introduces 44 new violations, 0 new errors and 0 new configuration errors, removes 19809 violations, 0 errors and 0 configuration errors. Full report
✅	Compared to master: This changeset changes 0 violations, introduces 232 new violations, 0 new errors and 0 new configuration errors, removes 19796 violations, 0 errors and 0 configuration errors. Full report
✅	Compared to master: This changeset changes 0 violations, introduces 75 new violations, 544 new errors and 0 new configuration errors, removes 19796 violations, 0 errors and 0 configuration errors. Full report

Generated by 🚫 Danger

[adangel]

Does this violation make sense? Wdyt?

https://github.com/spring-projects/spring-framework/tree/v5.3.13/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java#L90

AdvisedSupport is serializable. It has a field interfaces of type java.util.List. The List is a interface and it is itself not serializable. But the implementation used at runtime (e.g. at this specific line it is ArrayList) is serializable. So this is actually not a real problem, but just looking statically at the type of the field, we don't know, whether the value will be serializable or not at runtime.
We could make it optional to check this (e.g. by introducing a property checkInterfaces).

Regarding the other open point (whether to rename or deprecate the old rule BeanMembersShouldSerialize): The new rule creates significantly less violations, as it only considers serializable classes (only 232 violations vs. 19796). When we rename the rule, this helps anyone who has enabled BeanMembersShouldSerialize now, because most of the false-positive noise will be gone without changing the ruleset. However, maybe nobody has this rule enabled anyway atm? Then some change in the ruleset will be necessary either way: adding NonSerializableClass or removing the exclude for BeanMembersShouldSerialize. So I guess, it doesn't really matter.

[oowekyala]

So this is actually not a real problem, but just looking statically at the type of the field, we don't know, whether the value will be serializable or not at runtime.

This problem is apparent in other violations, where no interface is involved. For instance here: https://github.com/openjdk/jdk/blob/jdk-11%2B28/src/java.base/share/classes/java/util/AbstractMap.java#L609

While the rule is technically right, it would be very restrictive to require the type parameters to be serializable when the serialization feature might never be used...

I think the rule is "more probably" right (true positive) when the field type is a non-abstract class (and not Object). When it's an interface or abstract class or a type parameter, maybe we should skip it conservatively (with a property). If we could, it would be nice to allow reporting these with a lower priority.

[adangel]

I've added the property "checkAbstractTypes" now.

There is one more special case: the private static final field serialPersistentFields - if this is not defined, then all non-static and non-transient fields must be serializable. Otherwise, only the fields that are defined here, need to be serializable.

That means, that all violations in java.io.ObjectStreamClass (e.g. https://github.com/openjdk/jdk/blob/jdk-11%2B28/src/java.base/share/classes/java/io/ObjectStreamClass.java#L166) are false-positives, because this class defines actually no serializable fields: https://github.com/openjdk/jdk/blob/jdk-11%2B28/src/java.base/share/classes/java/io/ObjectStreamClass.java#L88-L89

This behavior is described here: https://docs.oracle.com/en/java/javase/19/docs/specs/serialization/serial-arch.html#defining-serializable-fields-for-a-class