Part of #2524
Several classes implement the RootNode interface in the Apex AST:
- ASTAnonymousClass
- ASTUserClass
- ASTUserEnum
- ASTUserInterface
- ASTUserTrigger
The problem with that is that it doesn't respect the contract of the RootNode interface. Granted, the contract is not documented and the interface is rarely used in PMD, so there's not many occasions to find out implementation errors.
Anyway the unspoken contract for now is that there is only one RootNode instance per AST and that it's at the root of the tree. Eg DocumentNavigator relies on this assumption to identify the document node:
|
public boolean isDocument(Object arg0) { |
|
return arg0 instanceof RootNode; |
|
} |
(This implementation is wrong for reasons outside the scope of this issue, see #1938)
To come back to Apex, the problem is that classes may be nested, so you may find a RootNode instance that is not the root of the tree. The simplest way to respect the contract is to introduce a new node, eg an ASTApexFile, that is at the top of any Apex AST, and to make all current ApexRootNodes unimplement the interface.
Rationale
I think for 7.0, it may be useful to flesh out the RootNode interface to use it to its full potential. Eg, having a method to provide the URI of the source file (or database resource, etc), the full text of the file (not hard to implement, already done for PLSQL), etc. There are also many places in PMD core where the only Nodes we expect are roots and we could maybe be stricter... #1426 uses the interface in that sense.
The point is, that those changes require a strong contract to the RootNode interface. Lastly, having a single implementation of RootNode per language model helps reduce code duplication.
Part of #2524
Several classes implement the RootNode interface in the Apex AST:
The problem with that is that it doesn't respect the contract of the RootNode interface. Granted, the contract is not documented and the interface is rarely used in PMD, so there's not many occasions to find out implementation errors.
Anyway the unspoken contract for now is that there is only one RootNode instance per AST and that it's at the root of the tree. Eg DocumentNavigator relies on this assumption to identify the document node:
pmd/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/DocumentNavigator.java
Lines 98 to 100 in 2d4a30f
(This implementation is wrong for reasons outside the scope of this issue, see #1938)
To come back to Apex, the problem is that classes may be nested, so you may find a RootNode instance that is not the root of the tree. The simplest way to respect the contract is to introduce a new node, eg an
ASTApexFile, that is at the top of any Apex AST, and to make all currentApexRootNodes unimplement the interface.Rationale
I think for 7.0, it may be useful to flesh out the RootNode interface to use it to its full potential. Eg, having a method to provide the URI of the source file (or database resource, etc), the full text of the file (not hard to implement, already done for PLSQL), etc. There are also many places in PMD core where the only Nodes we expect are roots and we could maybe be stricter... #1426 uses the interface in that sense.
The point is, that those changes require a strong contract to the RootNode interface. Lastly, having a single implementation of RootNode per language model helps reduce code duplication.