[java] AccessNode API changes

[oowekyala]

The purpose of AccessNode is to denote a node which can have declaration modifiers, eg abstract, final, public, etc. Some nodes implement it just to get the "final" modifier:

• ASTFormalParameter
• ASTResource (relates to [java] AST inconsistencies around FormalParameter #998)
• ASTLocalVariableDeclaration

ASTLambdaExpression implements it for nothing, due to an implementation error: AbstractMethodLikeNode extends AbstractJavaAccessNode...

1. I think all of these should stop implementing the interface in 7.0.0. We could have a separate base class for nodes that can be final, or implement it ad-hoc with a boolean since FormalParameter is a TypeNode and so we'd have to have several base classes. The amount of duplication seems acceptable
2. The setters should be removed from the interface. They're only used by the parser, and could be package-private

[adangel]

TODOs:

pmd/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AccessNode.java

Line 24 in 05b8ae4

* TODO make modifiers accessible from XPath

pmd/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AccessNode.java

Line 31 in 05b8ae4

* TODO rename to ModifierOwner, kept out from PR to reduce diff

[oowekyala]

I think the gist of this ticket was implemented, namely:

• setters were made package-private
• isPublic/isPrivate/isProtected/isPackagePrivate are replaced with getVisibility, and the corresponding @Visibility XPath attribute.
• isSyntactically* are all replaced with AccessNode#hasExplicitModifiers, bzw ASTModifierList#hasAllExplicitly and hasAnyExplicitly for more control. In XPath these are exposed as a pmd-java:explicitModifiers() function, introduced by [java] New Java XPath functions #2917
• isAbstract is still supported as a member of ASTMethodOrConstructorDeclaration and ASTAnyTypeDeclaration, and I think it makes sense
• isFinal is for now a member of the new interface FinalizableNode, but it's not used that much and we could get rid of it (see below)
• Lesser used modifier methods, like isSynchronized and isVolatile are deprecated for removal, as they don't really carry their weight and one can use hasModifier

I think what's I'd like to see to close this ticket is

• push down isStatic to just ASTMethodOrConstructorDeclaration, ASTFieldDeclaration and ASTAnyTypeDeclaration as it's convenient (like isAbstract)
• add a hasVisibility(Visibility) method to AccessNode, to replace the isPublic/Private/*, etc, for the convenience of java rules
• remove FinalizableNode, as it's not a solid abstraction, and just push down isFinal to ASTVariableDeclaratorId, ASTAnyTypeDeclaration, ASTFieldDeclaration, and ASTMethodOrConstructorDeclaration for convenience

Having a stripped down ModifierOwner interface means it's ok to implement it in eg ASTFormalParameter (see the top comment here), without adding a bunch of irrelevant methods and XPath attributes to the API, like isSynchronized.

In any case I think the XPath API should expose as few attributes as possible, and boil down to

• @Visibility, eg @Visibility = "public"
• pmd-java:modifiers()
• pmd-java:explicitModifiers()

as these apply to every ModifierOwner. Accordingly, we should use @NoAttribute everywhere else.

I think this ticket may be closed before the renaming of AccessNode to ModifierOwner, as this is already tracked by #2701 .

[adangel]

Note: Rename AccessNode to ModifierOwner is in #3751, scheduled for 7.0.0-RC3