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] MissingSerialVersionUID false-positive on interfaces #1350

Closed
lgoldstein opened this Issue Sep 17, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@lgoldstein

lgoldstein commented Sep 17, 2018

Affects PMD Version:

6.7.0

Rule:

MissingSerialVersionUID

Description:

I have an interface that extends Serializable - PMD complains that the interface is missing the required serialVersionUID. Obviously, only classes are required to have it.

Code Sample demonstrating the issue:

public interface IDomainObject<ID extends Serializable & Comparable<? super ID>> extends MutablePrimaryIdentifier<ID>, Serializable {
}

Running PMD through: [Maven]

[INFO] --- maven-pmd-plugin:3.9.0:check (pmd-checker) @ cb4-commons-core-persistence ---
[INFO] PMD Failure: com.cb4.commons.persistence.IDomainObject:17 Rule:MissingSerialVersionUID Priority:3 Classes implementing Serializable should set a serialVersionUID.

Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T21:33:14+03:00)
Maven home: /home/lyor/Software/apache-maven-3.5.4
Java version: 1.8.0_181, vendor: Oracle Corporation, runtime: /usr/lib/jvm/java-1.8.0-openjdk-1.8.0.181-7.b13.fc27.x86_64/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "4.17.19-100.fc27.x86_64", arch: "amd64", family: "unix"

@adangel adangel changed the title from [java] MissingSerialVersionUID rule is "mis-firing" on interfaces to [java] MissingSerialVersionUID false-positive on interfaces Sep 17, 2018

@adangel adangel added this to the 6.8.0 milestone Sep 17, 2018

@adangel adangel self-assigned this Sep 17, 2018

adangel added a commit to adangel/pmd that referenced this issue Sep 17, 2018

@adangel adangel added the has:pr label Sep 17, 2018

@lgoldstein

This comment has been minimized.

lgoldstein commented Sep 17, 2018

Fast fix ... 👍 However, while looking at the commit I noticed the following definition in errorprone.xml

[@Abstract = 'false']

If it is related to the MissingSerialVersionUID rule, then the rule should apply to abstract classes but not interfaces. I.e., if an abstract class implements Serializable then it must also have a serialVersionUID value = not just concrete classes.

@adangel

This comment has been minimized.

Member

adangel commented Sep 17, 2018

Good catch! I found this related stackoverflow question: https://stackoverflow.com/questions/893259/should-an-abstract-class-have-a-serialversionuid

For some reason, we have an explicit unit test, that test's against abstract classes and expects no violation:

<test-code>
<description><![CDATA[
abstract case
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.io.Serializable;
public abstract class Foo implements Serializable {
}
]]></code>
</test-code>

Unfortunately without any further comment, why we ignore the abstract classes.

I'll create a new issue for this false-negative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment