Merge branch '7.0.x' into update-all-properties

BUILDING.md

@@ -1,8 +1,8 @@
 # How to build PMD
 
-PMD uses [Maven](https://maven.apache.org/) and requires at least Java 10 for building.
-You can get Java 10 from [Oracle](http://www.oracle.com/technetwork/java/javase/downloads/index.html)
-or from the [OpenJDK Project](http://jdk.java.net).
+PMD uses [Maven](https://maven.apache.org/) and requires at least Java 11 for building.
+You can get Java 11 from [Oracle](http://www.oracle.com/technetwork/java/javase/downloads/index.html)
+or from [AdoptOpenJdk](https://adoptopenjdk.net/).
 
 PMD uses the [maven wrapper](https://github.com/takari/maven-wrapper), so you can simply build PMD as following:
 
@@ -16,7 +16,7 @@ This will create the zip files in the directory `pmd-dist/target`:
 
 That's all !
 
-**Note:** While Java 10 is required for building, running PMD only requires Java 7 (or Java 8 for Apex and the Designer).
+**Note:** While Java 11 is required for building, running PMD only requires Java 7 (or Java 8 for Apex and the Designer).
 
 ## How to build the documentation?
 

CONTRIBUTING.md

@@ -7,7 +7,7 @@ By participating in this project you agree to abide by its terms.
 
 You can find the code of conduct in the file [code_of_conduct.md](code_of_conduct.md).
 
-| NB: the rule designer is developed over at [pmd/pmd-designer](https://github.com/pmd/pmd-designer). Please refer to the specific [contributor documentation](https://github.com/pmd/pmd-designer#contributing) if your issue, feature request or PR touches the designer.  |
+| NB: the rule designer is developed over at [pmd/pmd-designer](https://github.com/pmd/pmd-designer). Please refer to the specific [contributor documentation](https://github.com/pmd/pmd-designer/blob/master/CONTRIBUTING.md) if your issue, feature request or PR touches the designer.  |
 | --- |
 
 ## Pull requests

README.md

@@ -15,7 +15,7 @@ Modelica, PLSQL, Apache Velocity, XML, XSL, Scala.
 
 Additionally it includes **CPD**, the copy-paste-detector. CPD finds duplicated code in
 C/C++, C#, Dart, Fortran, Go, Groovy, Java, JavaScript, JSP, Kotlin, Lua, Matlab, Modelica,
-Objective-C, Perl, PHP, PLSQL, Python, Ruby, Salesforce.com Apex, Scala, Swift and Visualforce.
+Objective-C, Perl, PHP, PLSQL, Python, Ruby, Salesforce.com Apex, Scala, Swift, Visualforce and XML.
 
 ## Support
 

docs/pages/pmd/devdocs/building.md

@@ -10,9 +10,9 @@ author: Tom Copeland, Xavier Le Vourch <xlv@users.sourceforge.net>
 
 # Compiling PMD
 
-*   JDK 10 or higher
+*   JDK 11 or higher
 
-{% include note.html content="While Java 10 is required for building, running PMD only requires Java 7 (or Java 8 for Apex and the Designer)." %}
+{% include note.html content="While Java 11 is required for building, running PMD only requires Java 7 (or Java 8 for Apex and the Designer)." %}
 
 You’ll need to either check out the source code or download the latest source release. Assuming you’ve got the latest source release, unzip it to a directory:
 

docs/pages/pmd/devdocs/major_contributions/adding_new_cpd_language.md

@@ -50,12 +50,18 @@ All you need to do is follow this few steps:
     
     **You are almost there!**
     
-4. Please don't forget to add some test, you can again.. look at Go implementation ;)
+4. Update the list of supported languages
+
+   - Write the fully-qualified name of your Language class to the file `src/main/resources/META-INF/services/net.sourceforge.pmd.cpd.Language`
+
+   - Update the test that asserts the list of supported languages by updating the `SUPPORTED_LANGUAGES` constant in [BinaryDistributionIT](https://github.com/pmd/pmd/blob/master/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java)
+
+5. Please don't forget to add some test, you can again.. look at Go implementation ;)
     
     If you read this far, I'm keen to think you would also love to support some extra CPD configuration (ignore imports or crazy things like that)    
     If that's your case , you came to the right place! 
     
-5. You can add your custom properties using a Token filter 
+6. You can add your custom properties using a Token filter
     
     -   For Antlr grammars all you need to do is implement your own [AntlrTokenFilter](https://github.com/pmd/pmd/blob/master/pmd-core/src/main/java/net/sourceforge/pmd/cpd/token/AntlrTokenFilter.java)
         

docs/pages/pmd/devdocs/major_contributions/adding_new_language.md

@@ -69,15 +69,62 @@ folder: pmd/devdocs
 *   Add for each version of your language a call to `addVersion` in your language module’s constructor.
 *   Create the service registration via the text file `src/main/resources/META-INF/services/net.sourceforge.pmd.lang.Language`. Add your fully qualified class name as a single line into it.
 
-## 12. Create an abstract rule class for the language
+## 12. Add AST regression tests
+
+For languages, that use an external library for parsing, the AST can easily change when upgrading the library.
+Also for languages, where we have the grammar under our control, it useful to have such tests.
+
+The tests parse one or more source files and generate a textual representation of the AST. This text is compared
+against a previously recorded version. If there are differences, the test fails.
+
+This helps to detect anything in the AST structure, that changed, maybe unexpectedly.
+
+*   Create a test class in the package `net.sourceforge.pmd.lang.$lang.ast` with the name `$langTreeDumpTest`.
+*   This test class must extend `net.sourceforge.pmd.lang.ast.test.BaseTreeDumpTest`. Note: This class
+    is written in kotlin and is available in the module "lang-test".
+*   Add a default constructor, that calls the super constructor like so:
+    
+    ```java
+        public $langTreeDumpTest() {
+            super(NodePrintersKt.getSimpleNodePrinter(), ".$extension");
+        }
+    ```
+    
+    Replace "$lang" and "$extension" accordingly.
+*   Implement the method `getParser()`. It must return a
+    subclass of `net.sourceforge.pmd.lang.ast.test.BaseParsingHelper`. See 
+    `net.sourceforge.pmd.lang.ecmascript.ast.JsParsingHelper` for a example.
+    With this parser helper you can also specify, where the test files are searched, by using
+    the method `withResourceContext(Class<?>, String)`.
+*   Add one or more test methods. Each test method parses one file and compares the result. The base
+    class has a helper method `doTest(String)` that does all the work. This method just needs to be called:
+    
+    ```java
+        @Test
+        public void myFirstAstTest() {
+            doTest("filename-without-extension");
+        }
+    ```
+*   On the first test run the test fails. A text file (with the extension `.txt`) is created, that records the
+    current AST. On the next run, the text file is used as comparison and the test should pass. Don't forget
+    to commit the generated text file.
+
+A complete example can be seen in the JavaScript module: `net.sourceforge.pmd.lang.ecmascript.ast.JsTreeDumpTest`.
+The test resources are in the subpackage "testdata": `pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/ast/testdata/`.
+
+The Scala module also has a test, written in Kotlin instead of Java:
+`net.sourceforge.pmd.lang.scala.ast.ScalaParserTests`.
+
+
+## 13. Create an abstract rule class for the language
 *   Extend `AbstractRule` and implement the parser visitor interface for your language *(see AbstractVmRule for example)*
 *   All other rules for your language should extend this class. The purpose of this class is to implement visit methods for all AST types to simply delegate to default behavior. This is useful because most rules care only about specific AST nodes, but PMD needs to know what to do with each node - so this just lets you use default behavior for nodes you don’t care about.
 
-## 13. Create rules
-*   Rules are created by extending the abstract rule class created in step 12 *(see `EmptyForeachStmtRule` for example)*
+## 14. Create rules
+*   Rules are created by extending the abstract rule class created in step 13 *(see `EmptyForeachStmtRule` for example)*
 *   Creating rules is already pretty well documented in PMD - and it’s no different for a new language, except you may have different AST nodes.
 
-## 14. Test the rules
+## 15. Test the rules
 *   See BasicRulesTest for example
 *   You have to create a rule set for your language *(see vm/basic.xml for example)*
 *   For each rule in this set you want to test, call `addRule` method in setUp of the unit test

docs/pages/pmd/userdocs/cpd/cpd.md

@@ -207,7 +207,6 @@ This behavior has been introduced to ease CPD integration into scripts or hooks,
 
 ## Supported Languages
 
-* Apex
 * C#
 * C/C++
 * Dart
@@ -220,15 +219,18 @@ This behavior has been introduced to ease CPD integration into scripts or hooks,
 * Kotlin
 * Lua
 * Matlab
+* Modelica
 * Objective-C
 * Perl
 * PHP
 * PL/SQL
 * Python
 * Ruby
+* Salesforce.com Apex
 * Scala
 * Swift
 * Visualforce
+* XML
 
 
 ## Available report formats

docs/pages/pmd/userdocs/making_rulesets.md

@@ -18,7 +18,7 @@ author: Tom Copeland <tomcopeland@users.sourceforge.net>, Clément Fournier <cle
 
 The first step is to create a new empty ruleset. You can use the following template:
 
-``` xml
+```xml
 <?xml version="1.0"?>
 
 <ruleset name="Custom Rules"
@@ -44,7 +44,7 @@ To use the built-in rules PMD provides, you need to add some *references* to the
 basic rule reference:
 
 ```xml
-    <rule ref="category/java/errorprone.xml/EmptyCatchBlock" />
+<rule ref="category/java/errorprone.xml/EmptyCatchBlock" />
 ```
 
 Adding that element into the `ruleset` element adds the rule [EmptyCatchBlock](pmd_rules_java_errorprone.html#emptycatchblock)
@@ -80,10 +80,10 @@ How you can configure individual rules is described on [Configuring Rules](pmd_u
 You can also reference rules in bulk by referencing a complete category or ruleset, possibly excluding certain rules, like in the following:
 
 ```xml
-  <rule ref="category/java/codestyle.xml">
+<rule ref="category/java/codestyle.xml">
     <exclude name="WhileLoopsMustUseBraces"/>
     <exclude name="IfElseStmtsMustUseBraces"/>
-  </rule>
+</rule>
 ```
 
 Here, the `ref` attribute references a whole category. You can also use a file system path or classpath relative path. In any case, the path must address an accessible ruleset XML file.
@@ -108,13 +108,13 @@ You can exclude some files from being processed by a ruleset using **exclude pat
 		xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
 		xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 		xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
-	<description>My ruleset</description>
+    <description>My ruleset</description>
 
-	<exclude-pattern>.*/some/package/.*</exclude-pattern>
-	<exclude-pattern>.*/some/other/package/FunkyClassNamePrefix.*</exclude-pattern>
-	<include-pattern>.*/some/package/ButNotThisClass.*</include-pattern>
+    <exclude-pattern>.*/some/package/.*</exclude-pattern>
+    <exclude-pattern>.*/some/other/package/FunkyClassNamePrefix.*</exclude-pattern>
+    <include-pattern>.*/some/package/ButNotThisClass.*</include-pattern>
 
-	<!-- Rules here ... -->
+    <!-- Rules here ... -->
 
 </ruleset>
 ```

docs/pages/release_notes.md

@@ -19,11 +19,68 @@ This is a {{ site.pmd.release_type }} release.
 
 ### New and noteworthy
 
+#### CPD now supports XML as well
+
+Thanks to [Fernando Cosso](https://github.com/xnYi9wRezm) CPD can now find duplicates in XML files as well.
+This is useful to find duplicated sections in XML files.
+
+#### New Rules
+
+*   The new Java Rule {% rule "java/bestpractices/LiteralsFirstInComparisons" %} (`java-bestpractices`)
+    find String literals, that are used in comparisons and are not positioned first. Using the String literal
+    as the receiver of e.g. `equals` helps to avoid NullPointerExceptions.
+    
+    This rule is replacing the two old rules {% rule "java/bestpractices/PositionLiteralsFirstInComparisons" %}
+    and {% rule "java/bestpractices/PositionLiteralsFirstInCaseInsensitiveComparisons" %} and extends the check
+    for the methods `compareTo`, `compareToIgnoreCase` and `contentEquals` in addition to `equals` and
+    `equalsIgnoreCase`.
+    
+    Note: This rule also replaces the two mentioned rules in Java's quickstart ruleset.
+
 ### Fixed Issues
 
+*   apex-bestpractices
+    *   [#2468](https://github.com/pmd/pmd/issues/2468): \[apex] Unused Local Variable fails on blocks
+*   core
+    *   [#2484](https://github.com/pmd/pmd/issues/2484): \[core] Update maven-enforcer-plugin to require Java 118
+*   java
+    *   [#2472](https://github.com/pmd/pmd/issues/2472): \[java] JavaCharStream throws an Error on invalid escape
+*   java-bestpractices
+    *   [#2288](https://github.com/pmd/pmd/issues/2288): \[java] JUnitTestsShouldIncludeAssert: Add support for Hamcrest MatcherAssert.assertThat
+*   java-errorprone
+    *   [#2477](https://github.com/pmd/pmd/issues/2477): \[java] JUnitSpelling false-positive for JUnit5/4 tests
+*   swift
+    *   [#2473](https://github.com/pmd/pmd/issues/2473): \[swift] Swift 5 (up to 5.2) support for CPD
+
 ### API Changes
 
+#### Deprecated APIs
+
+*   {% jdoc !ca!core::lang.BaseLanguageModule#addVersion(String, LanguageVersionHandler, boolean) %}
+*   Some members of {% jdoc core::lang.ast.TokenMgrError %}, in particular, a new constructor is available
+    that should be preferred to the old ones
+*   {% jdoc core::lang.antlr.AntlrTokenManager.ANTLRSyntaxError %}
+
+#### Experimental APIs
+
+**Note:** Experimental APIs are identified with the annotation {% jdoc core::annotation.Experimental %},
+see its javadoc for details
+
+* The experimental methods in {% jdoc !ca!core::lang.BaseLanguageModule %} have been replaced by a
+definitive API.
+
 ### External Contributions
 
+*   [#2446](https://github.com/pmd/pmd/pull/2446): \[core] Update maven-compiler-plugin to 3.8.1 - [Artem Krosheninnikov](https://github.com/KroArtem)
+*   [#2448](https://github.com/pmd/pmd/pull/2448): \[java] Operator Wrap check - [Harsh Kukreja](https://github.com/harsh-kukreja)
+*   [#2449](https://github.com/pmd/pmd/pull/2449): \[plsql] Additional info in SqlStatement, FormalParameter and FetchStatement - [Grzegorz Sudolski](https://github.com/zgrzyt93)
+*   [#2452](https://github.com/pmd/pmd/pull/2452): \[doc] Fix "Making Rulesets" doc sample code indentation - [Artur Dryomov](https://github.com/arturdryomov)
+*   [#2457](https://github.com/pmd/pmd/pull/2457): \[xml] Adding XML to CPD supported languages - [Fernando Cosso](https://github.com/xnYi9wRezm)
+*   [#2469](https://github.com/pmd/pmd/pull/2469): \[apex] fix false positive unused variable if only a method is called - [Gwilym Kuiper](https://github.com/gwilymatgearset)
+*   [#2475](https://github.com/pmd/pmd/pull/2475): \[swift] Swift 4.2-5.2 support - [kenji21](https://github.com/kenji21)
+*   [#2478](https://github.com/pmd/pmd/pull/2478): \[java] New rule: LiteralsFirstInComparisons - [John-Teng](https://github.com/John-Teng)
+*   [#2479](https://github.com/pmd/pmd/pull/2479): \[java] False positive with Hamcrest's assertThat - [andreoss](https://github.com/andreoss)
+*   [#2481](https://github.com/pmd/pmd/pull/2481): \[java] Fix JUnitSpellingRule false positive - [Artem Krosheninnikov](https://github.com/KroArtem)
+
 {% endtocmaker %}
 

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ApexParser.java

@@ -8,8 +8,8 @@ import java.io.Reader;
 
 import net.sourceforge.pmd.lang.AbstractParser;
 import net.sourceforge.pmd.lang.ParserOptions;
-import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.ast.ParseException;
+import net.sourceforge.pmd.lang.ast.RootNode;
 
 /**
  * Adapter for the Apex jorje parser
@@ -23,7 +23,7 @@ public class ApexParser extends AbstractParser {
     }
 
     @Override
-    public Node parse(String fileName, Reader source) throws ParseException {
+    public RootNode parse(String fileName, Reader source) throws ParseException {
         return apexParser.parse(source);
     }
 

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java

@@ -5,37 +5,100 @@
 package net.sourceforge.pmd.lang.apex.ast;
 
 import net.sourceforge.pmd.annotation.InternalApi;
-import net.sourceforge.pmd.lang.ast.NodeStream;
+import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.ast.SourceCodePositioner;
+import net.sourceforge.pmd.lang.ast.impl.AbstractNodeWithTextCoordinates;
 
 import apex.jorje.data.Location;
 import apex.jorje.data.Locations;
 import apex.jorje.semantic.ast.AstNode;
 import apex.jorje.semantic.exception.UnexpectedCodePathException;
 
-abstract class AbstractApexNode<T extends AstNode> extends AbstractApexNodeBase implements ApexNode<T> {
+abstract class AbstractApexNode<T extends AstNode> extends AbstractNodeWithTextCoordinates<AbstractApexNode<?>, ApexNode<?>> implements ApexNode<T> {
 
     protected final T node;
 
     protected AbstractApexNode(T node) {
-        super(node.getClass());
         this.node = node;
     }
 
+    // overridden to make them visible
     @Override
-    public ApexNode<?> getChild(int index) {
-        return (ApexNode<?>) super.getChild(index);
+    protected void addChild(AbstractApexNode<?> child, int index) {
+        super.addChild(child, index);
     }
 
     @Override
-    public ApexNode<?> getParent() {
-        return (ApexNode<?>) super.getParent();
+    protected void insertChild(AbstractApexNode<?> child, int index) {
+        super.insertChild(child, index);
+    }
+
+    /* package */ void calculateLineNumbers(SourceCodePositioner positioner, int startOffset, int endOffset) {
+        // end column will be interpreted as inclusive, while endOffset/endIndex
+        // is exclusive
+        endOffset -= 1;
+
+        this.beginLine = positioner.lineNumberFromOffset(startOffset);
+        this.beginColumn = positioner.columnFromOffset(this.beginLine, startOffset);
+        this.endLine = positioner.lineNumberFromOffset(endOffset);
+        this.endColumn = positioner.columnFromOffset(this.endLine, endOffset);
+
+        if (this.endColumn < 0) {
+            this.endColumn = 0;
+        }
     }
 
     @Override
-    @SuppressWarnings("unchecked")
-    public NodeStream<? extends ApexNode<?>> children() {
-        return (NodeStream<? extends ApexNode<?>>) super.children();
+    public int getBeginLine() {
+        if (this.beginLine > 0) {
+            return this.beginLine;
+        }
+        Node parent = getParent();
+        if (parent != null) {
+            return parent.getBeginLine();
+        }
+        throw new RuntimeException("Unable to determine beginning line of Node.");
+    }
+
+    @Override
+    public int getBeginColumn() {
+        if (this.beginColumn > 0) {
+            return this.beginColumn;
+        }
+        Node parent = getParent();
+        if (parent != null) {
+            return parent.getBeginColumn();
+        }
+        throw new RuntimeException("Unable to determine beginning column of Node.");
+    }
+
+    @Override
+    public int getEndLine() {
+        if (this.endLine > 0) {
+            return this.endLine;
+        }
+        Node parent = getParent();
+        if (parent != null) {
+            return parent.getEndLine();
+        }
+        throw new RuntimeException("Unable to determine ending line of Node.");
+    }
+
+    @Override
+    public int getEndColumn() {
+        if (this.endColumn > 0) {
+            return this.endColumn;
+        }
+        Node parent = getParent();
+        if (parent != null) {
+            return parent.getEndColumn();
+        }
+        throw new RuntimeException("Unable to determine ending column of Node.");
+    }
+
+    @Override
+    public final String getXPathNodeName() {
+        return this.getClass().getSimpleName().replaceFirst("^AST", "");
     }
 
     void calculateLineNumbers(SourceCodePositioner positioner) {

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNodeBase.java

@@ -1,102 +0,0 @@
-/*
- * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
- */
-
-package net.sourceforge.pmd.lang.apex.ast;
-
-import net.sourceforge.pmd.lang.ast.AbstractNode;
-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.ast.SourceCodePositioner;
-
-abstract class AbstractApexNodeBase extends AbstractNode {
-
-    protected AbstractApexNodeBase(Class<?> klass) {
-        super(klass.hashCode());
-    }
-
-    /* package */ void calculateLineNumbers(SourceCodePositioner positioner, int startOffset, int endOffset) {
-        // end column will be interpreted as inclusive, while endOffset/endIndex
-        // is exclusive
-        endOffset -= 1;
-
-        this.beginLine = positioner.lineNumberFromOffset(startOffset);
-        this.beginColumn = positioner.columnFromOffset(this.beginLine, startOffset);
-        this.endLine = positioner.lineNumberFromOffset(endOffset);
-        this.endColumn = positioner.columnFromOffset(this.endLine, endOffset);
-
-        if (this.endColumn < 0) {
-            this.endColumn = 0;
-        }
-    }
-
-    /**
-     * Accept the visitor. *
-     */
-    public abstract Object jjtAccept(ApexParserVisitor visitor, Object data);
-
-    /**
-     * Accept the visitor. *
-     */
-    public Object childrenAccept(ApexParserVisitor visitor, Object data) {
-        for (int i = 0; i < children.length; ++i) {
-            // we know that the children here are all ApexNodes
-            AbstractApexNodeBase apexNode = (AbstractApexNodeBase) children[i];
-            apexNode.jjtAccept(visitor, data);
-        }
-
-        return data;
-    }
-
-    @Override
-    public int getBeginLine() {
-        if (this.beginLine > 0) {
-            return this.beginLine;
-        }
-        Node parent = getParent();
-        if (parent != null) {
-            return parent.getBeginLine();
-        }
-        throw new RuntimeException("Unable to determine beginning line of Node.");
-    }
-
-    @Override
-    public int getBeginColumn() {
-        if (this.beginColumn > 0) {
-            return this.beginColumn;
-        }
-        Node parent = getParent();
-        if (parent != null) {
-            return parent.getBeginColumn();
-        }
-        throw new RuntimeException("Unable to determine beginning column of Node.");
-    }
-
-    @Override
-    public int getEndLine() {
-        if (this.endLine > 0) {
-            return this.endLine;
-        }
-        Node parent = getParent();
-        if (parent != null) {
-            return parent.getEndLine();
-        }
-        throw new RuntimeException("Unable to determine ending line of Node.");
-    }
-
-    @Override
-    public int getEndColumn() {
-        if (this.endColumn > 0) {
-            return this.endColumn;
-        }
-        Node parent = getParent();
-        if (parent != null) {
-            return parent.getEndColumn();
-        }
-        throw new RuntimeException("Unable to determine ending column of Node.");
-    }
-
-    @Override
-    public final String getXPathNodeName() {
-        return this.getClass().getSimpleName().replaceFirst("^AST", "");
-    }
-}

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexNode.java

@@ -4,8 +4,7 @@
 
 package net.sourceforge.pmd.lang.apex.ast;
 
-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.ast.NodeStream;
+import net.sourceforge.pmd.lang.ast.impl.GenericNode;
 
 import apex.jorje.semantic.ast.AstNode;
 
@@ -16,7 +15,7 @@ import apex.jorje.semantic.ast.AstNode;
  *
  * @param <T> Type of the underlying Jorje node
  */
-public interface ApexNode<T extends AstNode> extends Node {
+public interface ApexNode<T extends AstNode> extends GenericNode<ApexNode<?>> {
 
     /**
      * Accept the visitor.
@@ -24,16 +23,6 @@ public interface ApexNode<T extends AstNode> extends Node {
     Object jjtAccept(ApexParserVisitor visitor, Object data);
 
 
-    /**
-     * Accept the visitor. *
-     *
-     * @deprecated This method is not useful, the logic for combining
-     *     children values should be present on the visitor, not the node
-     */
-    @Deprecated
-    Object childrenAccept(ApexParserVisitor visitor, Object data);
-
-
     /**
      * Get the underlying AST node.
      * @deprecated the underlying AST node should not be available outside of the AST node.
@@ -43,18 +32,6 @@ public interface ApexNode<T extends AstNode> extends Node {
     @Deprecated
     T getNode();
 
-
-    @Override
-    NodeStream<? extends ApexNode<?>> children();
-
-
-    @Override
-    ApexNode<?> getChild(int index);
-
-
-    @Override
-    ApexNode<?> getParent();
-
     boolean hasRealLoc();
 
     String getDefiningType();

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexParser.java

@@ -39,7 +39,7 @@ public class ApexParser {
         return visitor.getTopLevel();
     }
 
-    public ApexNode<Compilation> parse(final Reader reader) {
+    public ApexRootNode<Compilation> parse(final Reader reader) {
         try {
             final String sourceCode = IOUtils.toString(reader);
             final Compilation astRoot = parseApex(sourceCode);

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.java

@@ -16,7 +16,6 @@ import org.antlr.runtime.ANTLRStringStream;
 import org.antlr.runtime.Token;
 
 import net.sourceforge.pmd.lang.apex.ApexParserOptions;
-import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.ast.SourceCodePositioner;
 
 import apex.jorje.data.Location;
@@ -234,7 +233,7 @@ final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
     }
 
     // The nodes having children built.
-    private final Stack<Node> nodes = new Stack<>();
+    private final Stack<AbstractApexNode<?>> nodes = new Stack<>();
 
     // The Apex nodes with children to build.
     private final Stack<AstNode> parents = new Stack<>();
@@ -281,10 +280,9 @@ final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
         node.handleSourceCode(sourceCode);
 
         // Append to parent
-        Node parent = nodes.isEmpty() ? null : nodes.peek();
+        AbstractApexNode<?> parent = nodes.isEmpty() ? null : nodes.peek();
         if (parent != null) {
-            parent.jjtAddChild(node, parent.getNumChildren());
-            node.jjtSetParent(parent);
+            parent.addChild(node, parent.getNumChildren());
         }
 
         // Build the children...
@@ -304,27 +302,20 @@ final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
 
     private void addFormalComments() {
         for (ApexDocTokenLocation tokenLocation : apexDocTokenLocations) {
-            ApexNode<?> parent = tokenLocation.nearestNode;
+            AbstractApexNode<?> parent = tokenLocation.nearestNode;
             if (parent != null) {
                 ASTFormalComment comment = new ASTFormalComment(tokenLocation.token);
                 comment.calculateLineNumbers(sourceCodePositioner, tokenLocation.index,
                         tokenLocation.index + tokenLocation.token.getText().length());
 
-                // move existing nodes so that we can insert the comment as the first node
-                for (int i = parent.getNumChildren(); i > 0; i--) {
-                    parent.jjtAddChild(parent.getChild(i - 1), i);
-                }
-
-                parent.jjtAddChild(comment, 0);
-                comment.jjtSetParent(parent);
+                parent.insertChild(comment, 0);
             }
         }
     }
 
     private void buildFormalComment(AstNode node) {
         if (parents.peek() == node) {
-            ApexNode<?> parent = (ApexNode<?>) nodes.peek();
-            assignApexDocTokenToNode(node, parent);
+            assignApexDocTokenToNode(node, nodes.peek());
         }
     }
 
@@ -337,7 +328,7 @@ final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
      * @param jorjeNode the original node
      * @param node the potential parent node, to which the comment could belong
      */
-    private void assignApexDocTokenToNode(AstNode jorjeNode, ApexNode<?> node) {
+    private void assignApexDocTokenToNode(AstNode jorjeNode, AbstractApexNode<?> node) {
         Location loc = jorjeNode.getLoc();
         if (!Locations.isReal(loc)) {
             // Synthetic nodes such as "<clinit>" don't have a location in the
@@ -411,7 +402,7 @@ final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
     private static class ApexDocTokenLocation {
         int index;
         Token token;
-        ApexNode<?> nearestNode;
+        AbstractApexNode<?> nearestNode;
         int nearestNodeDistance;
 
         ApexDocTokenLocation(int index, Token token) {

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/AvoidGlobalModifierRule.java

@@ -33,6 +33,10 @@ public class AvoidGlobalModifierRule extends AbstractApexRule {
             addViolation(data, node);
         }
 
+        // Note, the rule reports the whole class, since that's enough and stops to visit right here.
+        // It also doesn't use rulechain, since it the top level type needs to global.
+        // if a member is global, that class has to be global as well to be valid apex.
+        // See also https://github.com/pmd/pmd/issues/2298
         return data;
     }
 

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java

@@ -4,11 +4,14 @@
 
 package net.sourceforge.pmd.lang.apex.rule.bestpractices;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import net.sourceforge.pmd.lang.apex.ast.ASTBlockStatement;
+import net.sourceforge.pmd.lang.apex.ast.ASTReferenceExpression;
 import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration;
 import net.sourceforge.pmd.lang.apex.ast.ASTVariableExpression;
+import net.sourceforge.pmd.lang.apex.ast.ApexNode;
 import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
 
 public class UnusedLocalVariableRule extends AbstractApexRule {
@@ -21,13 +24,20 @@ public class UnusedLocalVariableRule extends AbstractApexRule {
         String variableName = node.getImage();
 
         ASTBlockStatement variableContext = node.getFirstParentOfType(ASTBlockStatement.class);
-        List<ASTVariableExpression> potentialUsages = variableContext.findDescendantsOfType(ASTVariableExpression.class);
 
-        for (ASTVariableExpression usage : potentialUsages) {
+        List<ApexNode<?>> potentialUsages = new ArrayList<>();
+
+        // Variable expression catch things like the `a` in `a + b`
+        potentialUsages.addAll(variableContext.findDescendantsOfType(ASTVariableExpression.class));
+        // Reference expressions catch things like the `a` in `a.foo()`
+        potentialUsages.addAll(variableContext.findDescendantsOfType(ASTReferenceExpression.class));
+
+        for (ApexNode<?> usage : potentialUsages) {
             if (usage.getParent() == node) {
                 continue;
             }
-            if (usage.getImage().equals(variableName)) {
+
+            if (usage.hasImageEqualTo(variableName)) {
                 return data;
             }
         }

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/AvoidDmlStatementsInLoopsRule.java

@@ -15,7 +15,6 @@ import net.sourceforge.pmd.lang.apex.ast.ASTForEachStatement;
 import net.sourceforge.pmd.lang.apex.ast.ASTForLoopStatement;
 import net.sourceforge.pmd.lang.apex.ast.ASTWhileLoopStatement;
 import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
-import net.sourceforge.pmd.lang.ast.AbstractNode;
 import net.sourceforge.pmd.lang.ast.Node;
 
 public class AvoidDmlStatementsInLoopsRule extends AbstractApexRule {
@@ -68,7 +67,7 @@ public class AvoidDmlStatementsInLoopsRule extends AbstractApexRule {
         return data;
     }
 
-    private boolean insideLoop(AbstractNode node) {
+    private boolean insideLoop(Node node) {
         Node n = node.getParent();
 
         while (n != null) {

pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/AvoidGlobalModifier.xml

@@ -28,26 +28,32 @@ global interface Foo {
     <test-code>
         <description>Global method</description>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>1</expected-linenumbers>
         <code><![CDATA[
 global class Foo {
-	global Integer bar() {
-	
-	}
+    global Integer bar() {
+       // Note, the rule reports the whole class, since that's enough:
+       // if a member is global, that class has to be global as well to be valid apex.
+       // See also https://github.com/pmd/pmd/issues/2298
+    }
 }
-		]]></code>
-	</test-code>
+        ]]></code>
+    </test-code>
 
     <test-code>
         <description>Global inner interface</description>
         <expected-problems>1</expected-problems>
+        <expected-linenumbers>1</expected-linenumbers>
         <code><![CDATA[
 global class Foo {
-	global interface Bar {
-
-	}
+    global interface Bar {
+       // Note, the rule reports the whole class, since that's enough:
+       // if a member is global, that class has to be global as well to be valid apex.
+       // See also https://github.com/pmd/pmd/issues/2298
+    }
 }
-		]]></code>
-	</test-code>
+        ]]></code>
+    </test-code>
 
     <test-code>
         <description>#1348 [apex] AvoidGlobalModifierRule gives warning even when its a REST webservice - false positive</description>