5 files changed
+189
-390
No files matched your search
@@ -14,7 +14,19 @@ This is a {{ site.pmd.release_type }} release.
|
||||
|
||||
### New and noteworthy
|
||||
|
||||
#### Renamed rules
|
||||
|
||||
* The Java rule {% rule java/errorprone/BeanMembersShouldSerialize %} has been renamed to
|
||||
{% rule java/errorprone/NonSerializableClass %}. It has been revamped to only check for classes that are marked with
|
||||
`Serializable` and reports each field in it, that is not serializable.
|
||||
|
||||
The property `prefix` has been deprecated, since in a serializable class all fields have to be
|
||||
serializable regardless of the name.
|
||||
|
||||
### Fixed Issues
|
||||
* java-errorprone
|
||||
* [#1668](https://github.com/pmd/pmd/issues/1668): \[java] BeanMembersShouldSerialize is extremely noisy
|
||||
* [#4176](https://github.com/pmd/pmd/issues/4176): \[java] Rename BeanMembersShouldSerialize to NonSerializableClass
|
||||
|
||||
### API Changes
|
||||
|
||||
|
||||
@@ -28,7 +28,7 @@ public class ASTAnnotation extends AbstractJavaTypeNode {
|
||||
private static final List<String> UNUSED_RULES
|
||||
= Arrays.asList("UnusedPrivateField", "UnusedLocalVariable", "UnusedPrivateMethod", "UnusedFormalParameter", "UnusedAssignment");
|
||||
|
||||
private static final List<String> SERIAL_RULES = Arrays.asList("BeanMembersShouldSerialize", "MissingSerialVersionUID");
|
||||
private static final List<String> SERIAL_RULES = Arrays.asList("BeanMembersShouldSerialize", "NonSerializableClass", "MissingSerialVersionUID");
|
||||
|
||||
@InternalApi
|
||||
@Deprecated
|
||||
|
||||
+69
-97
@@ -6,132 +6,104 @@ package net.sourceforge.pmd.lang.java.rule.errorprone;
|
||||
|
||||
import static net.sourceforge.pmd.properties.PropertyFactory.stringProperty;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collection;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.io.Externalizable;
|
||||
import java.io.ObjectInputStream;
|
||||
import java.io.ObjectOutputStream;
|
||||
import java.io.Serializable;
|
||||
|
||||
import org.apache.commons.lang3.StringUtils;
|
||||
|
||||
import net.sourceforge.pmd.lang.ast.Node;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeBodyDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTResultType;
|
||||
import net.sourceforge.pmd.lang.java.ast.AccessNode;
|
||||
import net.sourceforge.pmd.lang.java.ast.Annotatable;
|
||||
import net.sourceforge.pmd.lang.java.rule.AbstractLombokAwareRule;
|
||||
import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
|
||||
import net.sourceforge.pmd.lang.java.symboltable.MethodNameDeclaration;
|
||||
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
|
||||
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTType;
|
||||
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
|
||||
import net.sourceforge.pmd.lang.java.ast.TypeNode;
|
||||
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
|
||||
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
|
||||
import net.sourceforge.pmd.properties.PropertyDescriptor;
|
||||
|
||||
public class NonSerializableClassRule extends AbstractLombokAwareRule {
|
||||
public class NonSerializableClassRule extends AbstractJavaRule {
|
||||
|
||||
private String prefixProperty;
|
||||
private String currentClassName;
|
||||
|
||||
private static final PropertyDescriptor<String> PREFIX_DESCRIPTOR = stringProperty("prefix").desc("A variable prefix to skip, i.e., m_").defaultValue("").build();
|
||||
private static final PropertyDescriptor<String> PREFIX_DESCRIPTOR = stringProperty("prefix")
|
||||
.desc("deprecated! A variable prefix to skip, i.e., m_").defaultValue("").build();
|
||||
|
||||
public NonSerializableClassRule() {
|
||||
definePropertyDescriptor(PREFIX_DESCRIPTOR);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Collection<String> defaultSuppressionAnnotations() {
|
||||
return Arrays.asList(
|
||||
"lombok.Data",
|
||||
"lombok.Getter",
|
||||
"lombok.Value"
|
||||
);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTCompilationUnit node, Object data) {
|
||||
prefixProperty = getProperty(PREFIX_DESCRIPTOR);
|
||||
super.visit(node, data);
|
||||
return data;
|
||||
}
|
||||
|
||||
private static String[] imagesOf(List<? extends Node> nodes) {
|
||||
|
||||
String[] imageArray = new String[nodes.size()];
|
||||
|
||||
for (int i = 0; i < nodes.size(); i++) {
|
||||
imageArray[i] = nodes.get(i).getImage();
|
||||
}
|
||||
return imageArray;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
|
||||
if (node.isInterface()) {
|
||||
return data;
|
||||
// ignore non-serializable classes
|
||||
if (!TypeTestUtil.isA(Serializable.class, node)
|
||||
// ignore Externalizable classes explicitly
|
||||
|| TypeTestUtil.isA(Externalizable.class, node)
|
||||
// ignore manual serialization
|
||||
|| hasManualSerializationMethod(node)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (hasLombokAnnotation(node)) {
|
||||
return super.visit(node, data);
|
||||
}
|
||||
currentClassName = node.getSimpleName();
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
Map<MethodNameDeclaration, List<NameOccurrence>> methods = node.getScope().getEnclosingScope(ClassScope.class)
|
||||
.getMethodDeclarations();
|
||||
List<ASTMethodDeclarator> getSetMethList = new ArrayList<>(methods.size());
|
||||
for (MethodNameDeclaration d : methods.keySet()) {
|
||||
ASTMethodDeclarator mnd = d.getMethodNameDeclaratorNode();
|
||||
if (isBeanAccessor(mnd)) {
|
||||
getSetMethList.add(mnd);
|
||||
private boolean hasManualSerializationMethod(ASTClassOrInterfaceDeclaration node) {
|
||||
boolean hasWriteObject = false;
|
||||
boolean hasReadObject = false;
|
||||
boolean hasWriteReplace = false;
|
||||
boolean hasReadResolve = false;
|
||||
for (ASTAnyTypeBodyDeclaration decl : node.getDeclarations()) {
|
||||
if (decl.getKind() == ASTAnyTypeBodyDeclaration.DeclarationKind.METHOD) {
|
||||
ASTMethodDeclaration methodDeclaration = (ASTMethodDeclaration) decl.getChild(0);
|
||||
String methodName = methodDeclaration.getName();
|
||||
int parameterCount = methodDeclaration.getFormalParameters().size();
|
||||
ASTFormalParameter firstParameter = methodDeclaration.getFormalParameters().getFirstChildOfType(ASTFormalParameter.class);
|
||||
ASTType resultType = methodDeclaration.getResultType().getFirstChildOfType(ASTType.class);
|
||||
|
||||
hasWriteObject |= "writeObject".equals(methodName) && parameterCount == 1
|
||||
&& TypeTestUtil.isA(ObjectOutputStream.class, firstParameter)
|
||||
&& resultType == null;
|
||||
hasReadObject |= "readObject".equals(methodName) && parameterCount == 1
|
||||
&& TypeTestUtil.isA(ObjectInputStream.class, firstParameter)
|
||||
&& resultType == null;
|
||||
hasWriteReplace |= "writeReplace".equals(methodName) && parameterCount == 0
|
||||
&& TypeTestUtil.isExactlyA(Object.class, resultType);
|
||||
hasReadResolve |= "readResolve".equals(methodName) && parameterCount == 0
|
||||
&& TypeTestUtil.isExactlyA(Object.class, resultType);
|
||||
}
|
||||
}
|
||||
|
||||
String[] methNameArray = imagesOf(getSetMethList);
|
||||
return hasWriteObject && hasReadObject || hasWriteReplace && hasReadResolve;
|
||||
}
|
||||
|
||||
Arrays.sort(methNameArray);
|
||||
@Override
|
||||
public Object visit(ASTFieldDeclaration node, Object data) {
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
Map<VariableNameDeclaration, List<NameOccurrence>> vars = node.getScope()
|
||||
.getDeclarations(VariableNameDeclaration.class);
|
||||
for (Map.Entry<VariableNameDeclaration, List<NameOccurrence>> entry : vars.entrySet()) {
|
||||
VariableNameDeclaration decl = entry.getKey();
|
||||
AccessNode accessNodeParent = decl.getAccessNodeParent();
|
||||
if (entry.getValue().isEmpty() || accessNodeParent.isTransient() || accessNodeParent.isStatic()
|
||||
|| hasIgnoredAnnotation((Annotatable) accessNodeParent)) {
|
||||
continue;
|
||||
}
|
||||
String varName = StringUtils.capitalize(trimIfPrefix(decl.getImage()));
|
||||
boolean hasGetMethod = Arrays.binarySearch(methNameArray, "get" + varName) >= 0
|
||||
|| Arrays.binarySearch(methNameArray, "is" + varName) >= 0;
|
||||
boolean hasSetMethod = Arrays.binarySearch(methNameArray, "set" + varName) >= 0;
|
||||
// Note that a Setter method is not applicable to a final
|
||||
// variable...
|
||||
if (!hasGetMethod || !accessNodeParent.isFinal() && !hasSetMethod) {
|
||||
addViolation(data, decl.getNode(), decl.getImage());
|
||||
}
|
||||
@Override
|
||||
public Object visit(ASTVariableDeclaratorId node, Object data) {
|
||||
if (isNonStaticNonTransientField(node) && isNotSerializable(node)) {
|
||||
asCtx(data).addViolation(node, node.getName(), currentClassName, getTypeName(node.getType()));
|
||||
}
|
||||
return super.visit(node, data);
|
||||
}
|
||||
|
||||
private String trimIfPrefix(String img) {
|
||||
if (prefixProperty != null && img.startsWith(prefixProperty)) {
|
||||
return img.substring(prefixProperty.length());
|
||||
}
|
||||
return img;
|
||||
private boolean isNotSerializable(TypeNode node) {
|
||||
return !TypeTestUtil.isA(Serializable.class, node)
|
||||
&& node.getType() != null && !node.getType().isPrimitive();
|
||||
}
|
||||
|
||||
private boolean isBeanAccessor(ASTMethodDeclarator meth) {
|
||||
private String getTypeName(Class<?> clazz) {
|
||||
return clazz != null ? clazz.getName() : "<unknown>";
|
||||
}
|
||||
|
||||
String methodName = meth.getImage();
|
||||
|
||||
if (methodName.startsWith("get") || methodName.startsWith("set")) {
|
||||
return true;
|
||||
}
|
||||
if (methodName.startsWith("is")) {
|
||||
ASTResultType ret = ((ASTMethodDeclaration) meth.getParent()).getResultType();
|
||||
List<ASTPrimitiveType> primitives = ret.findDescendantsOfType(ASTPrimitiveType.class);
|
||||
if (!primitives.isEmpty() && primitives.get(0).isBoolean()) {
|
||||
return true;
|
||||
}
|
||||
private boolean isNonStaticNonTransientField(ASTVariableDeclaratorId node) {
|
||||
if (node.isField()) {
|
||||
ASTFieldDeclaration field = node.getFirstParentOfType(ASTFieldDeclaration.class);
|
||||
return !field.isStatic() && !field.isTransient();
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -2682,29 +2682,39 @@ public class Foo {
|
||||
<rule name="NonSerializableClass"
|
||||
language="java"
|
||||
since="1.1"
|
||||
message="Found non-transient, non-static member. Please mark as transient or provide accessors."
|
||||
message="The field ''{0}'' of serializable class ''{1}'' is of non-serializable type ''{2}''."
|
||||
class="net.sourceforge.pmd.lang.java.rule.errorprone.NonSerializableClassRule"
|
||||
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#nonserializableclass">
|
||||
<description>
|
||||
If a class is a bean, or is referenced by a bean directly or indirectly it needs to be serializable.
|
||||
Member variables need to be marked as transient, static, or have accessor methods in the class. Marking
|
||||
variables as transient is the safest and easiest modification. Accessor methods should follow the Java
|
||||
naming conventions, i.e. for a variable named foo, getFoo() and setFoo() accessor methods should be provided.
|
||||
If a class is marked as `Serializable`, then all fields need to be serializable as well. In order to exclude
|
||||
a field, it can be marked as transient. Static fields are not considered.
|
||||
|
||||
This rule reports all fields, that are not serializable.
|
||||
|
||||
If a class implements the methods to perform manual serialization (`writeObject`, `readObject`) or uses
|
||||
a replacement object (`writeReplace`, `readResolve`) then this class is ignored.
|
||||
|
||||
Note: This rule has been revamped with PMD 6.52.0. It was previously called "BeanMembersShouldSerialize".
|
||||
The property `prefix` has been deprecated, since in a serializable class all fields have to be
|
||||
serializable regardless of the name.
|
||||
</description>
|
||||
<priority>3</priority>
|
||||
<example>
|
||||
<![CDATA[
|
||||
private transient int someFoo; // good, it's transient
|
||||
private static int otherFoo; // also OK
|
||||
private int moreFoo; // OK, has proper accessors, see below
|
||||
private int badFoo; // bad, should be marked transient
|
||||
class Buzz implements java.io.Serializable {
|
||||
private static final long serialVersionUID = 1L;
|
||||
|
||||
private void setMoreFoo(int moreFoo){
|
||||
this.moreFoo = moreFoo;
|
||||
}
|
||||
private transient int someFoo; // good, it's transient
|
||||
private static int otherFoo; // also OK, it's static
|
||||
private java.io.InputStream stream; // bad - InputStream is not serializable
|
||||
|
||||
private int getMoreFoo(){
|
||||
return this.moreFoo;
|
||||
public void setStream(InputStream stream) {
|
||||
this.stream = stream;
|
||||
}
|
||||
|
||||
public int getSomeFoo() {
|
||||
return this.someFoo;
|
||||
}
|
||||
}
|
||||
]]>
|
||||
</example>
|
||||
|
||||
+83
-278
File diff suppressed because it is too large.
Load diff
Reference in new issue
Block a user