Skip to content
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] New Rule InvalidJavaBean #4177

Closed
adangel opened this issue Oct 27, 2022 · 1 comment · Fixed by #4203
Closed

[java] New Rule InvalidJavaBean #4177

adangel opened this issue Oct 27, 2022 · 1 comment · Fixed by #4203
Labels
a:new-rule Proposal to add a new built-in rule
Milestone

Comments

@adangel
Copy link
Member

adangel commented Oct 27, 2022

Proposed Rule Name: InvalidJavaBean

Proposed Category: Design

Description:

This rule would provide the "bean" part of BeanMembersShouldSerialize.
It would check, that the analyzed class adheres to the Java Beans Specification, that means:

  • Each field has a correctly named getter and setter
  • for indexed properties, check the field is an array and the types match
  • event listeners implement java.util.EventListener
  • event listeners always come in pairs (addXXListener, removeXXListener)
  • the bean as a default no-arg constructor
  • the bean is serializable (that means, it implements java.io.Serializable- for the actually check, see the rule NonSerializableClass [java] Rename BeanMembersShouldSerialize to NonSerializableClass #4176)

Care must be taken to make this rule not as noisy as #1668 . One strategy could be:

  • consider only classes, that have at least one getter or setter (that means, a method matching the naming conventions). These classes could be beans.
  • consider imports: PropertyChangeListener, VetoableChangeListener, EventListener might give a hint, that the class under analysis might be a bean.
  • maybe have a minimum number of properties - so that the rule is not triggered yet, when a class has only one getter?
  • providing a property to configure a package or a naming convention for bean classes?
  • Consider annotations in java.beans

Related issues:

References:

Code Sample:

public class MyBean {        // <-- bean is not serializable, missing "implements Serializable"
    private String label;    // <-- missing setter for property "label"

    public String getLabel() {
        return label;
    }
}

Possible Properties:

  • package - a regex to configure the packages where this rule should be applied to
  • ensureSerialization - whether or not to check for java.io.Serializable
@adangel adangel added the a:new-rule Proposal to add a new built-in rule label Oct 27, 2022
@jsotuyod
Copy link
Member

jsotuyod commented Oct 27, 2022

Some personal comments…

Firstly, I'd go for the approach of having to setup the rule with a required package to avoid the noisiness. The criteria for inference are too vague and would produce significant FPs / FNs.

I'd also have the check for serialization be optional through a property. Why? Because even though the actual usage of Java Beans as communication value objects has declined with JavaEE, the idea of an object with a default constructor with getters and setters has endured, and is used massively in other scenarios, such as:

  • Bean Validation (JSR 303 / 380), used standalone but also supported by Jersey, Spring and plenty of others.
  • JAXB
  • Jackson JSON library (when no annotations present)

Being able to enforce the compliance of these would be useful, as an error is mostly silent, with properties being dropped…

@adangel adangel added this to the 6.52.0 milestone Oct 27, 2022
adangel added a commit to adangel/pmd that referenced this issue Nov 10, 2022
adangel added a commit to adangel/pmd that referenced this issue Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:new-rule Proposal to add a new built-in rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants