Merge branch '7.0.x' into java-typeres-ctxs

2021-04-18 23:56:41 +02:00
commit ea51f0683b
@@ -57,8 +57,9 @@
    <rule ref="category/java/bestpractices.xml/UseAssertSameInsteadOfAssertTrue"/>
    <rule ref="category/java/bestpractices.xml/UseAssertTrueInsteadOfAssertEquals"/>
    <rule ref="category/java/bestpractices.xml/UseCollectionIsEmpty"/>
+    <rule ref="category/java/bestpractices.xml/UseStandardCharsets" />
    <rule ref="category/java/bestpractices.xml/UseTryWithResources"/>
-    <!-- <rule ref="category/java/bestpractices.xml/UseVarargs"/> -->
+    <rule ref="category/java/bestpractices.xml/UseVarargs"/>
    <rule ref="category/java/bestpractices.xml/WhileLoopWithLiteralBoolean"/>

    <!-- codestyle.xml -->
@@ -97,7 +98,7 @@
    <rule ref="category/java/codestyle.xml/NoPackage"/>
    <!-- <rule ref="category/java/codestyle.xml/OnlyOneReturn"/> -->
    <rule ref="category/java/codestyle.xml/PackageCase"/>
-    <!-- <rule ref="category/java/codestyle.xml/PrematureDeclaration"/> -->
+    <rule ref="category/java/codestyle.xml/PrematureDeclaration"/>
    <rule ref="category/java/codestyle.xml/RemoteInterfaceNamingConvention"/>
    <rule ref="category/java/codestyle.xml/RemoteSessionInterfaceNamingConvention"/>
    <rule ref="category/java/codestyle.xml/ShortClassName"/>
@@ -190,7 +191,7 @@
    <!-- <rule ref="category/java/errorprone.xml/AvoidUsingOctalValues"/> -->
    <!-- <rule ref="category/java/errorprone.xml/BadComparison"/> -->
    <!-- <rule ref="category/java/errorprone.xml/BeanMembersShouldSerialize"/> -->
-    <!-- <rule ref="category/java/errorprone.xml/BrokenNullCheck"/> -->
+    <rule ref="category/java/errorprone.xml/BrokenNullCheck"/>
    <!-- <rule ref="category/java/errorprone.xml/CallSuperFirst"/> -->
    <!-- <rule ref="category/java/errorprone.xml/CallSuperLast"/> -->
    <!-- <rule ref="category/java/errorprone.xml/CheckSkipResult"/> -->
@@ -1,2 +1,2 @@
-distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.6.3/apache-maven-3.6.3-bin.zip
+distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.8.1/apache-maven-3.8.1-bin.zip
 wrapperUrl=https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar
@@ -72,6 +72,10 @@ The language module registers only one version (as before), now correctly with v
 Since there is only one version available for JavaScript there is actually no need to selected a specific version.
 The default version is always ES6.

+#### New Rules
+
+*   The Apex rule {% rule "apex/design/UnusedMethod" %} finds unused methods in your code.
+
 #### Changed Rules

 ##### Java
@@ -111,6 +115,8 @@ The following previously deprecated rules have been finally removed:

 ### Fixed Issues

+*   apex-design
+    *   [#2667](https://github.com/pmd/pmd/issues/2667): \[apex] Integrate nawforce/ApexLink to build robust Unused rule
 *   core
    *   [#1451](https://github.com/pmd/pmd/issues/1451): \[core] RulesetFactoryCompatibility stores the whole ruleset file in memory as a string

@@ -139,6 +145,8 @@ The following previously deprecated rules have been finally removed:
    * [#2883](https://github.com/pmd/pmd/issues/2883): \[java] JUnitAssertionsShouldIncludeMessage false positive with method call
    * [#2890](https://github.com/pmd/pmd/issues/2890): \[java] UnusedPrivateMethod false positive with generics
 * java-codestyle
+    * [#1208](https://github.com/pmd/pmd/issues/1208): \[java] PrematureDeclaration rule false-positive on variable declared to measure time
+    * [#1429](https://github.com/pmd/pmd/issues/1429): \[java] PrematureDeclaration as result of method call (false positive)
    * [#1673](https://github.com/pmd/pmd/issues/1673): \[java] UselessParentheses false positive with conditional operator
    * [#1790](https://github.com/pmd/pmd/issues/1790): \[java] UnnecessaryFullyQualifiedName false positive with enum constant
    * [#1918](https://github.com/pmd/pmd/issues/1918): \[java] UselessParentheses false positive with boolean operators
@@ -147,11 +155,13 @@ The following previously deprecated rules have been finally removed:
    * [#2739](https://github.com/pmd/pmd/issues/2739): \[java] UselessParentheses false positive for string concatenation
    * [#3195](https://github.com/pmd/pmd/pull/3195): \[java] Improve rule UnnecessaryReturn to detect more cases
    * [#3218](https://github.com/pmd/pmd/pull/3218): \[java] Generalize UnnecessaryCast to flag all unnecessary casts
+    * [#3221](https://github.com/pmd/pmd/issues/3221): \[java] PrematureDeclaration false positive for unused variables
 * java-errorprone
    * [#1005](https://github.com/pmd/pmd/issues/1005): \[java] CloneMethodMustImplementCloneable triggers for interfaces
    * [#2532](https://github.com/pmd/pmd/issues/2532): \[java] AvoidDecimalLiteralsInBigDecimalConstructor can not detect the case new BigDecimal(Expression)
    * [#2716](https://github.com/pmd/pmd/issues/2716): \[java] CompareObjectsWithEqualsRule: False positive with Enums
    * [#2880](https://github.com/pmd/pmd/issues/2880): \[java] CompareObjectsWithEquals - false negative with type res
+    * [#3071](https://github.com/pmd/pmd/issues/3071): \[java] BrokenNullCheck FP with PMD 6.30.0
 * java-multithreading
    * [#2537](https://github.com/pmd/pmd/issues/2537): \[java] DontCallThreadRun can't detect the case that call run() in `this.run()`
    * [#2538](https://github.com/pmd/pmd/issues/2538): \[java] DontCallThreadRun can't detect the case that call run() in `foo.bar.run()`
@@ -203,6 +213,7 @@ The metrics framework has been made simpler and more general.
 *   [#1774](https://github.com/pmd/pmd/pull/1774): \[core] Antlr visitor rules - [Lucas Soncini](https://github.com/lsoncini)
 *   [#1877](https://github.com/pmd/pmd/pull/1877): \[swift] Feature/swift rules - [Matias Fraga](https://github.com/matifraga)
 *   [#1882](https://github.com/pmd/pmd/pull/1882): \[swift] UnavailableFunction Swift rule - [Tomás de Lucca](https://github.com/tomidelucca)
+*   [#2830](https://github.com/pmd/pmd/pull/2830): \[apex] Apexlink POC - [Kevin Jones](https://github.com/nawforce)

 {% endtocmaker %}

@@ -54,6 +54,13 @@
            <type>pom</type>
        </dependency>

+        <dependency>
+            <groupId>com.github.nawforce</groupId>
+            <artifactId>apexlink</artifactId>
+            <version>1.4.1</version>
+        </dependency>
+
+
        <dependency>
            <groupId>commons-io</groupId>
            <artifactId>commons-io</artifactId>
@@ -17,6 +17,7 @@ import net.sourceforge.pmd.lang.ast.Parser;
 import net.sourceforge.pmd.lang.metrics.LanguageMetricsProvider;
 import net.sourceforge.pmd.lang.metrics.Metric;
 import net.sourceforge.pmd.lang.rule.RuleViolationFactory;
+import net.sourceforge.pmd.properties.PropertySource;

@InternalApi
 public class ApexHandler extends AbstractPmdLanguageVersionHandler {
@@ -34,6 +35,11 @@ public class ApexHandler extends AbstractPmdLanguageVersionHandler {
        return new ApexParser();
    }

+    @Override
+    public void declareParserTaskProperties(PropertySource source) {
+        source.definePropertyDescriptor(ApexParser.MULTIFILE_DIRECTORY);
+        overridePropertiesFromEnv(ApexLanguageModule.TERSE_NAME, source);
+    }

    @Override
    public LanguageMetricsProvider getLanguageMetricsProvider() {
@@ -4,10 +4,12 @@

 package net.sourceforge.pmd.lang.apex.ast;

+import java.util.List;
 import java.util.Map;

 import org.checkerframework.checker.nullness.qual.NonNull;

+import net.sourceforge.pmd.lang.apex.multifile.ApexMultifileAnalysis;
 import net.sourceforge.pmd.lang.ast.AstInfo;
 import net.sourceforge.pmd.lang.ast.Parser.ParserTask;
 import net.sourceforge.pmd.lang.ast.RootNode;
@@ -15,18 +17,22 @@ import net.sourceforge.pmd.lang.ast.SourceCodePositioner;

 import apex.jorje.semantic.ast.AstNode;
 import apex.jorje.semantic.ast.compilation.Compilation;
+import com.nawforce.common.diagnostics.Issue;

 public final class ASTApexFile extends AbstractApexNode<AstNode> implements RootNode {

    private final AstInfo<ASTApexFile> astInfo;
+    private final @NonNull ApexMultifileAnalysis multifileAnalysis;

    ASTApexFile(SourceCodePositioner source,
                ParserTask task,
                AbstractApexNode<? extends Compilation> child,
-                Map<Integer, String> suppressMap) {
+                Map<Integer, String> suppressMap,
+                @NonNull ApexMultifileAnalysis multifileAnalysis) {
        super(child.getNode());
        this.astInfo = new AstInfo<>(task, this, suppressMap);
        addChild(child, 0);
+        this.multifileAnalysis = multifileAnalysis;
        this.beginLine = 1;
        this.endLine = source.getLastLine();
        this.beginColumn = 1;
@@ -67,4 +73,7 @@ public final class ASTApexFile extends AbstractApexNode<AstNode> implements Root
        return visitor.visit(this, data);
    }

+    public List<Issue> getGlobalIssues() {
+        return multifileAnalysis.getFileIssues(getAstInfo().getFileName());
+    }
 }
@@ -6,9 +6,12 @@ package net.sourceforge.pmd.lang.apex.ast;

 import net.sourceforge.pmd.annotation.InternalApi;
 import net.sourceforge.pmd.lang.apex.ApexJorjeLogging;
+import net.sourceforge.pmd.lang.apex.multifile.ApexMultifileAnalysis;
 import net.sourceforge.pmd.lang.ast.ParseException;
 import net.sourceforge.pmd.lang.ast.Parser;
 import net.sourceforge.pmd.lang.ast.SourceCodePositioner;
+import net.sourceforge.pmd.properties.PropertyDescriptor;
+import net.sourceforge.pmd.properties.PropertyFactory;

 import apex.jorje.data.Locations;
 import apex.jorje.semantic.ast.compilation.Compilation;
@@ -16,6 +19,14 @@ import apex.jorje.semantic.ast.compilation.Compilation;
@InternalApi
 public final class ApexParser implements Parser {

+    @InternalApi // todo change that to optional<file> when properties are updated
+    public static final PropertyDescriptor<String> MULTIFILE_DIRECTORY =
+        PropertyFactory.stringProperty("rootDirectory")
+                       .desc("The root directory of the Salesforce metadata, where `sfdx-project.json` resides. "
+                                 + "Set environment variable PMD_APEX_ROOTDIRECTORY to use this.")
+                       .defaultValue("") // is this ok?
+                       .build();
+
    public ApexParser() {
        ApexJorjeLogging.disableLogging();
        Locations.useIndexFactory();
@@ -31,10 +42,13 @@ public final class ApexParser implements Parser {
                throw new ParseException("Couldn't parse the source - there is not root node - Syntax Error??");
            }

+            String property = task.getProperties().getProperty(MULTIFILE_DIRECTORY);
+            ApexMultifileAnalysis analysisHandler = ApexMultifileAnalysis.getAnalysisInstance(property);
+
            SourceCodePositioner positioner = new SourceCodePositioner(sourceCode);
            final ApexTreeBuilder treeBuilder = new ApexTreeBuilder(sourceCode, task.getCommentMarker(), positioner);
            AbstractApexNode<Compilation> treeRoot = treeBuilder.build(astRoot);
-            return new ASTApexFile(positioner, task, treeRoot, treeBuilder.getSuppressMap());
+            return new ASTApexFile(positioner, task, treeRoot, treeBuilder.getSuppressMap(), analysisHandler);
        } catch (apex.jorje.services.exception.ParseException e) {
            throw new ParseException(e);
        }
@@ -0,0 +1,148 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.apex.multifile;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.logging.Logger;
+
+import org.apache.commons.lang3.exception.ExceptionUtils;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import net.sourceforge.pmd.annotation.Experimental;
+import net.sourceforge.pmd.lang.apex.ast.ApexParser;
+
+import com.nawforce.common.api.FileIssueOptions;
+import com.nawforce.common.api.Org;
+import com.nawforce.common.api.ServerOps;
+import com.nawforce.common.diagnostics.Issue;
+
+/**
+ * Stores multi-file analysis data. The 'Org' here is the primary ApexLink structure for maintaining information
+ * about the Salesforce metadata. We load 'Packages' into it to perform analysis. Once constructed you
+ * can get 'Issue' information from it on what was found. The 'Org' holds mutable state for IDE use that can get quite
+ * large (a few hundred MB on very large projects). An alternative way to use this would be to cache the
+ * issues after packages are loaded and throw away the 'Org'. That would be a better model if all you wanted was the
+ * issues but more complex rules will need the ability to traverse the internal graph of the 'Org'.
+ *
+ * @author Kevin Jones
+ */
+@Experimental
+public final class ApexMultifileAnalysis {
+
+    // test only
+    static final Logger LOG = Logger.getLogger(ApexMultifileAnalysis.class.getName());
+
+    /**
+     * Instances of the apexlink index and data structures ({@link Org})
+     * are stored statically for now. TODO make that language-wide (#2518).
+     */
+    private static final Map<String, ApexMultifileAnalysis> INSTANCE_MAP = new ConcurrentHashMap<>();
+
+    // An arbitrary large number of errors to report
+    private static final Integer MAX_ERRORS_PER_FILE = 100;
+
+    // Create a new org for each analysis
+    // Null if failed.
+    private final @Nullable Org org;
+    private final FileIssueOptions options = makeOptions();
+
+    private static final ApexMultifileAnalysis FAILED_INSTANCE = new ApexMultifileAnalysis();
+
+    /** Ctor for the failed instance. */
+    private ApexMultifileAnalysis() {
+        org = null;
+    }
+
+    private ApexMultifileAnalysis(String multiFileAnalysisDirectory) {
+        LOG.fine("MultiFile Analysis created for " + multiFileAnalysisDirectory);
+        org = Org.newOrg();
+        if (multiFileAnalysisDirectory != null && !multiFileAnalysisDirectory.isEmpty()) {
+            // Load the package into the org, this can take some time!
+            org.newSFDXPackage(multiFileAnalysisDirectory); // this may fail if the config is wrong
+            org.flush();
+
+            // FIXME: Syntax & Semantic errors found during Org loading are not currently being reported. These
+            // should be routed to the new SemanticErrorReporter but that is not available for use just yet.
+        }
+    }
+
+    private static FileIssueOptions makeOptions() {
+        FileIssueOptions options = new FileIssueOptions();
+        // Default issue options, zombies gets us unused methods & fields as well as deploy problems
+        options.includeZombies_$eq(true);
+        options.maxErrorsPerFile_$eq(MAX_ERRORS_PER_FILE);
+        return options;
+    }
+
+    /**
+     * Returns true if this is analysis index is in a failed state.
+     * This object is then useless. The failed instance is returned
+     * from {@link #getAnalysisInstance(String)} if loading the org
+     * failed, maybe because of malformed configuration.
+     */
+    public boolean isFailed() {
+        return org == null;
+    }
+
+    public List<Issue> getFileIssues(String filename) {
+        // Extract issues for a specific metadata file from the org
+        return org == null ? Collections.emptyList()
+                           : Collections.unmodifiableList(Arrays.asList(org.getFileIssues(filename, options)));
+    }
+
+    /**
+     * Returns the analysis instance. Returns a {@linkplain #isFailed() failed instance}
+     * if this fails.
+     *
+     * @param multiFileAnalysisDirectory Root directory of the configuration (see {@link ApexParser#MULTIFILE_DIRECTORY}).
+     */
+    public static @NonNull ApexMultifileAnalysis getAnalysisInstance(String multiFileAnalysisDirectory) {
+        if (INSTANCE_MAP.isEmpty()) {
+            // Default some library wide settings
+            ServerOps.setAutoFlush(false);
+            ServerOps.setLogger(new AnalysisLogger());
+            ServerOps.setDebugLogging(new String[] { "ALL" });
+        }
+
+        return INSTANCE_MAP.computeIfAbsent(
+            multiFileAnalysisDirectory,
+            dir -> {
+                try {
+                    return new ApexMultifileAnalysis(dir);
+                } catch (Exception e) {
+                    LOG.severe("Exception while initializing Apexlink (" + e.getMessage() + ")");
+                    LOG.severe(ExceptionUtils.getStackTrace(e));
+                    LOG.severe("PMD will not attempt to initialize Apexlink further, this can cause rules like UnusedMethod to be dysfunctional");
+                    return FAILED_INSTANCE;
+                }
+            });
+    }
+
+    /*
+     * Very simple logger to aid debugging, relays ApexLink logging into PMD
+     */
+    private static final class AnalysisLogger implements com.nawforce.common.api.Logger {
+
+        @Override
+        public void error(String message) {
+            LOG.severe(message);
+        }
+
+        @Override
+        public void info(String message) {
+            LOG.info(message);
+        }
+
+        @Override
+        public void debug(String message) {
+            LOG.fine(message);
+        }
+    }
+}
@@ -0,0 +1,25 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.apex.rule.design;
+
+import net.sourceforge.pmd.lang.apex.ast.ASTMethod;
+import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
+
+import com.nawforce.common.api.UNUSED_CATEGORY$;
+
+public class UnusedMethodRule extends AbstractApexRule {
+
+    @Override
+    public Object visit(ASTMethod node, Object data) {
+
+        // Check if any 'Unused' Issues align with this method
+        node.getRoot().getGlobalIssues().stream()
+            .filter(issue -> UNUSED_CATEGORY$.MODULE$ == issue.diagnostic().category())
+            .filter(issue -> issue.diagnostic().location().startLine() == node.getBeginLine())
+            .filter(issue -> issue.diagnostic().location().endLine() <= node.getBeginLine())
+            .forEach(issue -> addViolation(data, node));
+        return data;
+    }
+}
@@ -36,6 +36,75 @@ public class Foo {
        </example>
    </rule>

+    <rule name="UnusedMethod"
+          language="apex"
+          since="7.0.0"
+          message="Unused methods make understanding code harder"
+          class="net.sourceforge.pmd.lang.apex.rule.design.UnusedMethodRule"
+          externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_design.html#unusedmethod">
+        <description><![CDATA[
+Avoid having unused methods since they make understanding and maintaining code harder.
+
+This rule finds not only unused private methods, but public methods as well.
+
+[ApexLink](https://github.com/nawforce/ApexLink) is used to make this possible and this needs
+additional configuration. The environment variable `PMD_APEX_ROOTDIRECTORY` needs to be set prior to executing
+PMD. With this variable the root directory of the Salesforce metadata, where `sfdx-project.json` resides, is
+specified. ApexLink can then load all the classes in the project and figure out, whether a method is used or not.
+
+For an accurate analysis it is important that the `PMD_APEX_ROOTDIRECTORY` contains a complete set of metadata that
+may be referenced from the Apex source code, such as Custom Objects, Visualforce Pages, Flows and Labels. The
+`PMD_APEX_ROOTDIRECTORY` directory must contain a `sfdx-project.json`, but metadata may be either in the
+[SFDX Source format](https://developer.salesforce.com/docs/atlas.en-us.sfdx_dev.meta/sfdx_dev/sfdx_dev_source_file_format.htm)
+or the older MDAPI format. The `packageDirectories` entries in `sfdx-project.json` are used to determine which
+directories to search for metadata, if a `.forceignore` file is present it will be respected.
+
+If the Apex code references external packages via namespace(s) you should declare these in your `sfdx-project.json`
+file using the 'plugins' syntax shown in the example below to avoid errors. Here's an example of a
+well-formed `sfdx-project.json`:
+```json
+{
+    "packageDirectories": [
+      {
+        "path": "src",
+        "default": true
+      }
+    ],
+    "namespace": "my_namespace",
+    "sfdcLoginUrl": "https://login.salesforce.com",
+    "sourceApiVersion": "52.0",
+    "plugins": {
+        "dependencies": [
+            {"namespace": "aa"}
+        ]
+    }
+}
+```
+]]>
+        </description>
+        <priority>3</priority>
+        <example>
+            <![CDATA[
+public class Triangle {
+    private Double side1;
+    private Double side2;
+    private Double side3;
+
+    public Triangle(Double side1, Double side2, Double side3) {
+        this.side1 = side1;
+        this.side2 = side2;
+        this.side3 = side3;
+    }
+
+    // Method is not invoked so can be removed
+    public Double area() {
+        return (side1 + side2 + side3)/2;
+    }
+}
+]]>
+        </example>
+    </rule>
+
    <rule name="CyclomaticComplexity"
          language="apex"
          message="The {0} ''{1}'' has a{2} cyclomatic complexity of {3}."
@@ -0,0 +1,75 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.apex.multifile;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.util.Arrays;
+
+import org.apache.commons.io.IOUtils;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import net.sourceforge.pmd.test.util.JavaUtilLoggingRule;
+
+/**
+ *
+ */
+public class ApexMultifileAnalysisTest {
+
+    @Rule
+    public final JavaUtilLoggingRule loggingRule = new JavaUtilLoggingRule(ApexMultifileAnalysis.LOG);
+
+    @Rule
+    public final TemporaryFolder tempFolder = new TemporaryFolder();
+
+    @Test
+    public void testNoSfdxProjectJsonProducesFailedAnalysis() {
+        ApexMultifileAnalysis analysisInstance = getAnalysisForTempFolder();
+
+        assertTrue(analysisInstance.isFailed());
+        assertTrue(analysisInstance.getFileIssues("any file").isEmpty());
+        loggingRule.assertContainsIgnoringCase("Missing project file");
+    }
+
+    @Test
+    public void testMalformedSfdxProjectJsonProducesFailedAnalysis() throws IOException {
+        copyResource("malformedSfdxFile.json", "sfdx-project.json");
+
+        ApexMultifileAnalysis analysisInstance = getAnalysisForTempFolder();
+
+        assertTrue(analysisInstance.isFailed());
+        assertTrue(analysisInstance.getFileIssues("any file").isEmpty());
+        loggingRule.assertContainsIgnoringCase("error: 'path' is required for all 'packageDirectories' elements");
+    }
+
+    @Test
+    public void testWellFormedSfdxProjectJsonProducesFunctionalAnalysis() throws IOException {
+        copyResource("correctSfdxFile.json", "sfdx-project.json");
+
+        ApexMultifileAnalysis analysisInstance = getAnalysisForTempFolder();
+
+        assertFalse(analysisInstance.isFailed());
+        loggingRule.assertEmpty();
+    }
+
+    private @NonNull ApexMultifileAnalysis getAnalysisForTempFolder() {
+        return ApexMultifileAnalysis.getAnalysisInstance(tempFolder.getRoot().getAbsolutePath());
+    }
+
+    private void copyResource(String resourcePath, String relativePathInTempDir) throws IOException {
+        File file = tempFolder.newFile(relativePathInTempDir);
+        String fileContents = IOUtils.toString(getClass().getResourceAsStream(resourcePath), StandardCharsets.UTF_8);
+        Files.write(file.toPath(), Arrays.asList(fileContents.split("\\R").clone()));
+    }
+
+}
@@ -0,0 +1,9 @@
+{
+  "packageDirectories": [
+    {
+      "path": "force-app"
+    }
+  ],
+  "namespace": "",
+  "sourceApiVersion": "39.0"
+}
@@ -0,0 +1,8 @@
+{
+  "packageDirectories": [
+    {
+    }
+  ],
+  "namespace": "",
+  "sourceApiVersion": "39.0"
+}
@@ -0,0 +1,14 @@
+<?xml version="1.0"?>
+
+<ruleset name="700"
+    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>
+This ruleset contains links to rules that are new in PMD v7.0.0
+  </description>
+
+    <rule ref="category/apex/design.xml/UnusedMethod"/>
+
+</ruleset>
+
@@ -9,6 +9,7 @@ import org.checkerframework.checker.nullness.qual.Nullable;

 import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken;
 import net.sourceforge.pmd.lang.java.symbols.JMethodSymbol;
+import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
 import net.sourceforge.pmd.lang.rule.xpath.DeprecatedAttribute;


@@ -145,4 +146,14 @@ public final class ASTMethodDeclaration extends AbstractMethodOrConstructorDecla
        return children(ASTArrayDimensions.class).first();
    }

+    /**
+     * Returns whether this is a main method declaration.
+     */
+    public boolean isMainMethod() {
+        return this.hasModifiers(JModifier.PUBLIC, JModifier.STATIC)
+            && "main".equals(this.getName())
+            && this.isVoid()
+            && this.getArity() == 1
+            && TypeTestUtil.isExactlyA(String[].class, this.getFormalParameters().get(0));
+    }
 }
@@ -32,8 +32,8 @@ import net.sourceforge.pmd.lang.java.ast.BinaryOp;
 import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
 import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
 import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
+import net.sourceforge.pmd.lang.java.types.InvocationMatcher;
 import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
-import net.sourceforge.pmd.lang.java.types.TypeTestUtil.InvocationMatcher;

 /**
 * @author Clément Fournier
@@ -4,31 +4,28 @@

 package net.sourceforge.pmd.lang.java.rule.bestpractices;

-import static net.sourceforge.pmd.util.CollectionUtil.listOf;
-
-import java.util.List;
-
 import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
 import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
 import net.sourceforge.pmd.lang.java.rule.internal.TestFrameworksUtil;
-import net.sourceforge.pmd.lang.java.types.TypeTestUtil.InvocationMatcher;
+import net.sourceforge.pmd.lang.java.types.InvocationMatcher;
+import net.sourceforge.pmd.lang.java.types.InvocationMatcher.CompoundInvocationMatcher;

 public class JUnitAssertionsShouldIncludeMessageRule extends AbstractJavaRulechainRule {

-    private final List<InvocationMatcher> checks =
-        listOf(
-            InvocationMatcher.parse("_#assertEquals(_,_)"),
-            InvocationMatcher.parse("_#assertTrue(_)"),
-            InvocationMatcher.parse("_#assertFalse(_)"),
-            InvocationMatcher.parse("_#assertSame(_,_)"),
-            InvocationMatcher.parse("_#assertNotSame(_,_)"),
-            InvocationMatcher.parse("_#assertNull(_)"),
-            InvocationMatcher.parse("_#assertNotNull(_)"),
-            InvocationMatcher.parse("_#assertArrayEquals(_,_)"),
-            InvocationMatcher.parse("_#assertThat(_,_)"),
-            InvocationMatcher.parse("_#fail()"),
-            InvocationMatcher.parse("_#assertEquals(float,float,float)"),
-            InvocationMatcher.parse("_#assertEquals(double,double,double)")
+    private final CompoundInvocationMatcher checks =
+        InvocationMatcher.parseAll(
+            "_#assertEquals(_,_)",
+            "_#assertTrue(_)",
+            "_#assertFalse(_)",
+            "_#assertSame(_,_)",
+            "_#assertNotSame(_,_)",
+            "_#assertNull(_)",
+            "_#assertNotNull(_)",
+            "_#assertArrayEquals(_,_)",
+            "_#assertThat(_,_)",
+            "_#fail()",
+            "_#assertEquals(float,float,float)",
+            "_#assertEquals(double,double,double)"
        );

    public JUnitAssertionsShouldIncludeMessageRule() {
@@ -38,11 +35,8 @@ public class JUnitAssertionsShouldIncludeMessageRule extends AbstractJavaRulecha
    @Override
    public Object visit(ASTMethodCall node, Object data) {
        if (TestFrameworksUtil.isCallOnAssertionContainer(node)) {
-            for (InvocationMatcher check : checks) {
-                if (check.matchesCall(node)) {
-                    addViolation(data, node);
-                    break;
-                }
+            if (checks.anyMatch(node)) {
+                addViolation(data, node);
            }
        }
        return null;
@@ -4,20 +4,28 @@

 package net.sourceforge.pmd.lang.java.rule.codestyle;

+import static java.util.Collections.emptySet;
 import static net.sourceforge.pmd.lang.ast.NodeStream.asInstanceOf;

-import java.util.ArrayList;
-import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;

-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
+import net.sourceforge.pmd.lang.ast.NodeStream;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
+import net.sourceforge.pmd.lang.java.ast.ASTExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTForInit;
 import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTName;
+import net.sourceforge.pmd.lang.java.ast.ASTResource;
 import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTStatement;
 import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess;
 import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
-import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
+import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
+import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
+import net.sourceforge.pmd.lang.java.types.InvocationMatcher;
+import net.sourceforge.pmd.lang.java.types.InvocationMatcher.CompoundInvocationMatcher;

 /**
 * Checks for variables in methods that are defined before they are really
@@ -27,90 +35,98 @@ import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
 *
 * @author Brian Remedios
 */
-public class PrematureDeclarationRule extends AbstractJavaRule {
+public class PrematureDeclarationRule extends AbstractJavaRulechainRule {

+    private static final CompoundInvocationMatcher TIME_METHODS =
+        InvocationMatcher.parseAll(
+            "java.lang.System#nanoTime()",
+            "java.lang.System#currentTimeMillis()"
+        );
+
+    public PrematureDeclarationRule() {
+        super(ASTLocalVariableDeclaration.class);
+    }

    @Override
    public Object visit(ASTLocalVariableDeclaration node, Object data) {
-
-        // is it part of a for-loop declaration?
-        if (node.getParent() instanceof ASTForInit) {
-            // yes, those don't count
-            return super.visit(node, data);
+        if (node.getParent() instanceof ASTForInit
+            || node.getParent() instanceof ASTResource) {
+            // those don't count
+            return null;
        }

        for (ASTVariableDeclaratorId id : node) {
-            for (ASTBlockStatement block : statementsAfter(node)) {
-                if (hasReferencesIn(block, id.getVariableName())) {
+            ASTExpression initializer = id.getInitializer();
+
+            if (JavaRuleUtil.isNeverUsed(id) // avoid the duplicate with unused variables
+                || cannotBeMoved(initializer)
+                || JavaRuleUtil.hasSideEffect(initializer, emptySet())) {
+                continue;
+            }
+
+            Set<JVariableSymbol> refsInInitializer = getReferencedVars(initializer);
+            // If there's no initializer, or the initializer doesn't depend on anything (eg, a literal),
+            // then we don't care about side-effects
+            boolean hasStatefulInitializer = !refsInInitializer.isEmpty() || JavaRuleUtil.hasSideEffect(initializer, emptySet());
+            for (ASTStatement stmt : statementsAfter(node)) {
+                if (hasReferencesIn(stmt, id)
+                    || hasStatefulInitializer && JavaRuleUtil.hasSideEffect(stmt, refsInInitializer)) {
                    break;
                }

-                if (hasExit(block)) {
-                    addViolation(data, node);
+                if (hasExit(stmt)) {
+                    addViolation(data, node, id.getName());
                    break;
                }
            }
        }

-
-        return super.visit(node, data);
+        return null;
    }

+    /**
+     * Returns the set of local variables referenced inside the expression.
+     */
+    private static Set<JVariableSymbol> getReferencedVars(ASTExpression term) {
+        return term == null ? emptySet()
+                            : term.descendantsOrSelf()
+                                  .filterIs(ASTNamedReferenceExpr.class)
+                                  .filter(it -> it.getReferencedSym() != null)
+                                  .collect(Collectors.mapping(ASTNamedReferenceExpr::getReferencedSym, Collectors.toSet()));
+    }
+
+    /**
+     * Time methods cannot be moved ever, even when there are no side-effects.
+     * The side effect they depend on is the program being executed. Are they
+     * the only methods like that?
+     */
+    private boolean cannotBeMoved(ASTExpression initializer) {
+        return TIME_METHODS.anyMatch(initializer);
+    }

    /**
     * Returns whether the block contains a return call or throws an exception.
     * Exclude blocks that have these things as part of an inner class.
     */
-    private boolean hasExit(ASTBlockStatement block) {
-        return block.descendants().map(asInstanceOf(ASTThrowStatement.class, ASTReturnStatement.class)).nonEmpty();
+    private static boolean hasExit(ASTStatement block) {
+        return block.descendants()
+                    .map(asInstanceOf(ASTThrowStatement.class, ASTReturnStatement.class))
+                    .nonEmpty();
    }


    /**
     * Returns whether the variable is mentioned within the statement or not.
     */
-    private static boolean hasReferencesIn(ASTBlockStatement block, String varName) {
-
-        // allow for closures on the var
-        for (ASTName name : block.findDescendantsOfType(ASTName.class, true)) {
-            if (isReference(varName, name.getImage())) {
-                return true;
-            }
-        }
-        return false;
+    private static boolean hasReferencesIn(ASTStatement stmt, ASTVariableDeclaratorId var) {
+        return stmt.descendants(ASTVariableAccess.class)
+                   .crossFindBoundaries()
+                   .filterMatching(ASTNamedReferenceExpr::getReferencedSym, var.getSymbol())
+                   .nonEmpty();
    }

-
-    /**
-     * Return whether the shortName is part of the compound name by itself or as
-     * a method call receiver.
-     */
-    private static boolean isReference(String shortName, String compoundName) {
-        int dotPos = compoundName.indexOf('.');
-
-        return dotPos < 0 ? shortName.equals(compoundName) : shortName.equals(compoundName.substring(0, dotPos));
-    }
-
-
-    /**
-     * Returns all the block statements following the given local var declaration.
-     */
-    private static List<ASTBlockStatement> statementsAfter(ASTLocalVariableDeclaration node) {
-
-        Node blockOrSwitch = node.getParent().getParent();
-
-        int count = blockOrSwitch.getNumChildren();
-        int start = node.getParent().getIndexInParent() + 1;
-
-        List<ASTBlockStatement> nextBlocks = new ArrayList<>(count - start);
-
-        for (int i = start; i < count; i++) {
-            Node maybeBlock = blockOrSwitch.getChild(i);
-            if (maybeBlock instanceof ASTBlockStatement) {
-                nextBlocks.add((ASTBlockStatement) maybeBlock);
-            }
-        }
-
-        return nextBlocks;
+    /** Returns all the statements following the given local var declaration. */
+    private static NodeStream<ASTStatement> statementsAfter(ASTLocalVariableDeclaration node) {
+        return node.asStream().followingSiblings().filterIs(ASTStatement.class);
    }
 }
@@ -4,187 +4,113 @@

 package net.sourceforge.pmd.lang.java.rule.errorprone;

-import java.util.ArrayList;
-import java.util.List;
-
-import org.apache.commons.lang3.StringUtils;
-
-import net.sourceforge.pmd.lang.ast.Node;
-import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator;
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
-import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTConditionalOrExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression;
+import net.sourceforge.pmd.RuleContext;
+import net.sourceforge.pmd.lang.ast.NodeStream;
 import net.sourceforge.pmd.lang.java.ast.ASTExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
-import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
-import net.sourceforge.pmd.lang.java.ast.ASTName;
+import net.sourceforge.pmd.lang.java.ast.ASTInfixExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral;
-import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
-import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
-import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
+import net.sourceforge.pmd.lang.java.ast.BinaryOp;
+import net.sourceforge.pmd.lang.java.ast.QualifiableExpression;
 import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
+import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
+import net.sourceforge.pmd.lang.java.rule.internal.StablePathMatcher;

 public class BrokenNullCheckRule extends AbstractJavaRulechainRule {

    public BrokenNullCheckRule() {
-        super(ASTIfStatement.class);
+        super(ASTInfixExpression.class);
    }

    @Override
-    public Object visit(ASTIfStatement node, Object data) {
-        ASTExpression expression = (ASTExpression) node.getChild(0);
-
-        ASTConditionalAndExpression conditionalAndExpression = expression
-                .getFirstDescendantOfType(ASTConditionalAndExpression.class);
-        if (conditionalAndExpression != null) {
-            checkForViolations(node, data, conditionalAndExpression);
-        }
-
-        ASTConditionalOrExpression conditionalOrExpression = expression
-                .getFirstDescendantOfType(ASTConditionalOrExpression.class);
-        if (conditionalOrExpression != null) {
-            checkForViolations(node, data, conditionalOrExpression);
-        }
-
-        return super.visit(node, data);
+    public Object visit(ASTInfixExpression node, Object data) {
+        checkBrokenNullCheck(node, (RuleContext) data);
+        return data;
    }

-    private void checkForViolations(ASTIfStatement node, Object data, Node conditionalExpression) {
-        ASTEqualityExpression equalityExpression = conditionalExpression
-                .getFirstChildOfType(ASTEqualityExpression.class);
-        if (equalityExpression == null) {
-            return;
-        }
-        if (conditionalExpression instanceof ASTConditionalAndExpression
-                && !"==".equals(equalityExpression.getImage())) {
-            return;
-        }
-        if (conditionalExpression instanceof ASTConditionalOrExpression
-                && !"!=".equals(equalityExpression.getImage())) {
-            return;
-        }
-        ASTNullLiteral nullLiteral = equalityExpression.getFirstDescendantOfType(ASTNullLiteral.class);
-        if (nullLiteral == null) {
-            return; // No null check
-        }
-        // If there is an assignment in the equalityExpression we give up,
-        // because things get too complex
-        if (conditionalExpression.hasDescendantOfType(ASTAssignmentOperator.class)) {
+    private void checkBrokenNullCheck(ASTInfixExpression enclosingConditional, RuleContext ctx) {
+        ASTExpression left = enclosingConditional.getLeftOperand();
+        if (!(left instanceof ASTInfixExpression)) {
            return;
        }

-        // Find the expression used in the null compare
-        ASTPrimaryExpression nullCompareExpression = findNullCompareExpression(equalityExpression);
-        if (nullCompareExpression == null) {
-            return; // No good null check
+        BinaryOp op = ((ASTInfixExpression) left).getOperator();
+        if (op != BinaryOp.EQ && op != BinaryOp.NE) {
+            return;
+        } else if (op == BinaryOp.NE && enclosingConditional.getOperator() == BinaryOp.CONDITIONAL_AND
+            || op == BinaryOp.EQ && enclosingConditional.getOperator() == BinaryOp.CONDITIONAL_OR) {
+            return; // not problematic
        }

-        // Now we find the expression to compare to and do the comparison
-        for (int i = 0; i < conditionalExpression.getNumChildren(); i++) {
-            Node conditionalSubnode = conditionalExpression.getChild(i);
+        ASTNullLiteral nullLit = left.children(ASTNullLiteral.class).first();
+        if (nullLit == null) {
+            return;
+        }

-            // We skip the null compare branch
-            ASTEqualityExpression nullEqualityExpression = nullLiteral
-                    .getFirstParentOfType(ASTEqualityExpression.class);
-            if (conditionalSubnode.equals(nullEqualityExpression)) {
-                continue;
-            }
-            ASTPrimaryExpression conditionalPrimaryExpression;
-            if (conditionalSubnode instanceof ASTPrimaryExpression) {
-                conditionalPrimaryExpression = (ASTPrimaryExpression) conditionalSubnode;
-            } else {
-                // The ASTPrimaryExpression is hidden (in a negation, braces or
-                // EqualityExpression)
-                conditionalPrimaryExpression = conditionalSubnode.getFirstDescendantOfType(ASTPrimaryExpression.class);
-            }
+        ASTExpression otherChild = JavaRuleUtil.getOtherOperandIfInInfixExpr(nullLit);
+        StablePathMatcher pathToNullVar = StablePathMatcher.matching(otherChild);
+        if (pathToNullVar == null) {
+            // cannot be matched, because it's not stable
+            return;
+        }

-            if (primaryExpressionsAreEqual(nullCompareExpression, conditionalPrimaryExpression)) {
-                addViolation(data, node); // We have a match
-            }
+        NodeStream<ASTExpression> exprsToCheck = enclosingConditional.getRightOperand()
+                                                                     .descendantsOrSelf()
+                                                                     .filterIs(ASTExpression.class);

+        for (ASTExpression subexpr : exprsToCheck) {
+            NpeReason npeReason = willNpeWithReason(subexpr, pathToNullVar);
+            if (npeReason != null) {
+                addViolationWithMessage(ctx, subexpr, npeReason.formatMessage);
+            }
+        }
+
+    }
+
+    private static NpeReason willNpeWithReason(ASTExpression e, StablePathMatcher pathToNullVar) {
+        if (e instanceof QualifiableExpression) {
+            ASTExpression qualifier = ((QualifiableExpression) e).getQualifier();
+            if (pathToNullVar.matches(qualifier)) {
+                return NpeReason.DEREFERENCE;
+            }
+        }
+
+        if (e.getParent() instanceof ASTInfixExpression) {
+            ASTInfixExpression infix = (ASTInfixExpression) e.getParent();
+            if (pathToNullVar.matches(e) && operatorUnboxesOperand(infix)) {
+                return NpeReason.UNBOXING;
+            }
+        }
+        return null;
+    }
+
+    private static boolean operatorUnboxesOperand(ASTInfixExpression infix) {
+        BinaryOp operator = infix.getOperator();
+
+        if (operator == BinaryOp.INSTANCEOF) {
+            return false;
+        }
+
+        boolean leftIsPrimitive = infix.getLeftOperand().getTypeMirror().isPrimitive();
+        boolean rightIsPrimitive = infix.getRightOperand().getTypeMirror().isPrimitive();
+        if (leftIsPrimitive != rightIsPrimitive) {
+            return true;
+        } else {
+            assert !leftIsPrimitive || !rightIsPrimitive : "We know at least one of the operands is null";
+            // So both are reference types
+            // With these ops, in this case no unboxing takes place
+            return operator != BinaryOp.NE && operator != BinaryOp.EQ;
        }
    }

-    private boolean primaryExpressionsAreEqual(ASTPrimaryExpression nullCompareVariable,
-            ASTPrimaryExpression expressionUsage) {
-        List<String> nullCompareNames = new ArrayList<>();
-        findExpressionNames(nullCompareVariable, nullCompareNames);
+    enum NpeReason {
+        DEREFERENCE("Dereferencing the qualifier of this expression will throw a NullPointerException"),
+        UNBOXING("Unboxing this operand will throw a NullPointerException");

-        List<String> expressionUsageNames = new ArrayList<>();
-        findExpressionNames(expressionUsage, expressionUsageNames);
+        private final String formatMessage;

-        for (int i = 0; i < nullCompareNames.size(); i++) {
-            if (expressionUsageNames.size() == i) {
-                // The used expression is shorter than the null
-                // compare expression (and we don't want to crash
-                // below)
-                return false;
-            }
-
-            String nullCompareExpressionName = nullCompareNames.get(i);
-            String expressionUsageName = expressionUsageNames.get(i);
-
-            // Variablenames should match or the expressionUsage should have the
-            // variable with a method call (ie. var.equals())
-            if (!nullCompareExpressionName.equals(expressionUsageName)
-                    && !expressionUsageName.startsWith(nullCompareExpressionName + ".")) {
-                // Some other expression is being used after the
-                // null compare
-                return false;
-            }
+        NpeReason(String formatMessage) {
+            this.formatMessage = formatMessage;
        }
-
-        return true;
-    }
-
-    /**
-     * Find the names of variables, methods and array arguments in a
-     * PrimaryExpression.
-     */
-    private void findExpressionNames(Node nullCompareVariable, List<String> results) {
-        for (int i = 0; i < nullCompareVariable.getNumChildren(); i++) {
-            Node child = nullCompareVariable.getChild(i);
-
-            if (child instanceof ASTName) {
-                // Variable names and some method calls
-                results.add(((ASTName) child).getImage());
-            } else if (child instanceof ASTLiteral) { // Array arguments
-                String literalImage = ((ASTLiteral) child).getImage();
-                // Skip other null checks
-                if (literalImage != null) {
-                    results.add(literalImage);
-                }
-            } else if (child instanceof ASTPrimarySuffix) { // More method calls
-                String name = ((ASTPrimarySuffix) child).getImage();
-                if (StringUtils.isNotBlank(name)) {
-                    results.add(name);
-                }
-            } else if (child instanceof ASTClassOrInterfaceType) {
-                // A class can be an argument too
-                String name = ((ASTClassOrInterfaceType) child).getImage();
-                results.add(name);
-            }
-
-            if (child.getNumChildren() > 0) {
-                findExpressionNames(child, results);
-            }
-        }
-    }
-
-    private ASTPrimaryExpression findNullCompareExpression(ASTEqualityExpression equalityExpression) {
-        List<ASTPrimaryExpression> primaryExpressions = equalityExpression
-                .findDescendantsOfType(ASTPrimaryExpression.class);
-        for (ASTPrimaryExpression primaryExpression : primaryExpressions) {
-            List<ASTPrimaryPrefix> primaryPrefixes = primaryExpression.findDescendantsOfType(ASTPrimaryPrefix.class);
-            for (ASTPrimaryPrefix primaryPrefix : primaryPrefixes) {
-                if (primaryPrefix.hasDescendantOfType(ASTName.class)) {
-                    // We found the variable that is compared to null
-                    return primaryExpression;
-                }
-            }
-        }
-        return null; // Nothing found
    }

 }
@@ -28,6 +28,8 @@ import net.sourceforge.pmd.lang.ast.NodeStream;
 import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken;
 import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
+import net.sourceforge.pmd.lang.java.ast.ASTArrayAccess;
+import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr;
 import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
 import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType;
 import net.sourceforge.pmd.lang.java.ast.ASTAssignmentExpression;
@@ -55,6 +57,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTNumericLiteral;
 import net.sourceforge.pmd.lang.java.ast.ASTStatement;
 import net.sourceforge.pmd.lang.java.ast.ASTSuperExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTThisExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement;
 import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess;
 import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
@@ -68,6 +71,8 @@ import net.sourceforge.pmd.lang.java.ast.TypeNode;
 import net.sourceforge.pmd.lang.java.ast.UnaryOp;
 import net.sourceforge.pmd.lang.java.symbols.JFieldSymbol;
 import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
+import net.sourceforge.pmd.lang.java.types.InvocationMatcher;
+import net.sourceforge.pmd.lang.java.types.InvocationMatcher.CompoundInvocationMatcher;
 import net.sourceforge.pmd.lang.java.types.JPrimitiveType.PrimitiveTypeKind;
 import net.sourceforge.pmd.lang.java.types.JTypeMirror;
 import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
@@ -78,6 +83,22 @@ import net.sourceforge.pmd.util.CollectionUtil;
 */
 public final class JavaRuleUtil {

+    // this is a hacky way to do it, but let's see where this goes
+    private static final CompoundInvocationMatcher KNOWN_PURE_METHODS = InvocationMatcher.parseAll(
+        "_#toString()",
+        "_#hashCode()",
+        "_#equals(java.lang.Object)",
+        "java.lang.String#_(_*)",
+        // actually not all of them, probs only stream of some type
+        // arg which doesn't implement Closeable...
+        "java.util.stream.Stream#_(_*)",
+        "java.util.Collection#size()",
+        "java.util.List#get(int)",
+        "java.util.Map#get(_)",
+        "java.lang.Iterable#iterator()",
+        "java.lang.Comparable#compareTo(_)"
+    );
+
    private JavaRuleUtil() {
        // utility class
    }
@@ -186,14 +207,7 @@ public final class JavaRuleUtil {
     */
    public static boolean isMainMethod(JavaNode node) {
        if (node instanceof ASTMethodDeclaration) {
-            ASTMethodDeclaration decl = (ASTMethodDeclaration) node;
-
-
-            return decl.hasModifiers(JModifier.PUBLIC, JModifier.STATIC)
-                && "main".equals(decl.getName())
-                && decl.isVoid()
-                && decl.getArity() == 1
-                && TypeTestUtil.isExactlyA(String[].class, decl.getFormalParameters().get(0));
+            return ((ASTMethodDeclaration) node).isMainMethod();
        }
        return false;
    }
@@ -297,10 +311,17 @@ public final class JavaRuleUtil {
    }

    public static boolean isGetterOrSetterCall(ASTMethodCall call) {
+        return isGetterCall(call) || isSetterCall(call);
+    }
+
+    private static boolean isSetterCall(ASTMethodCall call) {
+        return call.getArguments().size() > 0 && startsWithCamelCaseWord(call.getMethodName(), "set");
+    }
+
+    public static boolean isGetterCall(ASTMethodCall call) {
        return call.getArguments().size() == 0
            && (startsWithCamelCaseWord(call.getMethodName(), "get")
-            || startsWithCamelCaseWord(call.getMethodName(), "is"))
-            || call.getArguments().size() > 0 && startsWithCamelCaseWord(call.getMethodName(), "set");
+            || startsWithCamelCaseWord(call.getMethodName(), "is"));
    }


@@ -644,6 +665,21 @@ public final class JavaRuleUtil {
        return false;
    }

+    public static boolean isUnqualifiedThis(ASTExpression e) {
+        return e instanceof ASTThisExpression && ((ASTThisExpression) e).getQualifier() == null;
+    }
+
+    /**
+     * Returns true if the expression is a {@link ASTNamedReferenceExpr}
+     * that references any of the symbol in the set.
+     */
+    public static boolean isReferenceToVar(@Nullable ASTExpression expression, @NonNull Set<? extends JVariableSymbol> symbols) {
+        if (expression instanceof ASTNamedReferenceExpr) {
+            return symbols.contains(((ASTNamedReferenceExpr) expression).getReferencedSym());
+        }
+        return false;
+    }
+
    /**
     * Returns true if both expressions refer to the same variable.
     * A "variable" here can also means a field path, eg, {@code this.field.a}.
@@ -684,6 +720,17 @@ public final class JavaRuleUtil {
        return false;
    }

+    /**
+     * Returns true if the expression is a reference to a local variable.
+     */
+    public static boolean isReferenceToLocal(ASTExpression expr) {
+        if (expr instanceof ASTVariableAccess) {
+            JVariableSymbol sym = ((ASTVariableAccess) expr).getReferencedSym();
+            return sym != null && !sym.isField();
+        }
+        return false;
+    }
+
    /**
     * Returns true if the expression has the form `field`, or `this.field`,
     * where `field` is a field declared in the enclosing class.
@@ -746,4 +793,59 @@ public final class JavaRuleUtil {
        Node parent = it.getParent();
        return parent == null || it.getIndexInParent() == parent.getNumChildren() - 1;
    }
+
+
+    /**
+     * Whether the node or one of its descendants is an expression with
+     * side effects. Conservatively, any method call is a potential side-effect,
+     * as well as assignments to fields or array elements. We could relax
+     * this assumption with (much) more data-flow logic, including a memory model.
+     *
+     * <p>By default assignments to locals are not counted as side-effects,
+     * unless the lhs is in the given set of symbols.
+     *
+     * @param node             A node
+     * @param localVarsToTrack Local variables to track
+     */
+    public static boolean hasSideEffect(@Nullable JavaNode node, Set<? extends JVariableSymbol> localVarsToTrack) {
+        return node != null && node.descendantsOrSelf()
+                                   .filterIs(ASTExpression.class)
+                                   .any(e -> hasSideEffectNonRecursive(e, localVarsToTrack));
+    }
+
+    /**
+     * Returns true if the expression has side effects we don't track.
+     * Does not recurse into sub-expressions.
+     */
+    private static boolean hasSideEffectNonRecursive(ASTExpression e, Set<? extends JVariableSymbol> localVarsToTrack) {
+        if (e instanceof ASTAssignmentExpression) {
+            ASTAssignableExpr lhs = ((ASTAssignmentExpression) e).getLeftOperand();
+            return isNonLocalLhs(lhs) || isReferenceToVar(lhs, localVarsToTrack);
+        } else if (e instanceof ASTUnaryExpression) {
+            ASTUnaryExpression unary = (ASTUnaryExpression) e;
+            ASTExpression lhs = unary.getOperand();
+            return !unary.getOperator().isPure()
+                && (isNonLocalLhs(lhs) || isReferenceToVar(lhs, localVarsToTrack));
+        }
+
+        if (e.ancestors(ASTThrowStatement.class).nonEmpty()) {
+            // then this side effect can never be observed in containing code,
+            // because control flow jumps out of the method
+            return false;
+        }
+
+        return e instanceof ASTMethodCall && !isPure((ASTMethodCall) e)
+            || e instanceof ASTConstructorCall;
+    }
+
+    private static boolean isNonLocalLhs(ASTExpression lhs) {
+        return lhs instanceof ASTArrayAccess || !isReferenceToLocal(lhs);
+    }
+
+    /**
+     * Whether the invocation has no side-effects. Very conservative.
+     */
+    private static boolean isPure(ASTMethodCall call) {
+        return isGetterCall(call) || KNOWN_PURE_METHODS.anyMatch(call);
+    }
 }
@@ -0,0 +1,147 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.rule.internal;
+
+import java.util.ArrayDeque;
+import java.util.Objects;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import net.sourceforge.pmd.lang.java.ast.ASTExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTFieldAccess;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
+import net.sourceforge.pmd.lang.java.ast.ASTSuperExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTThisExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess;
+import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
+
+/**
+ * A matcher for an expression like {@code a}, {@code a.b}, {@code a.getFoo()}.
+ * Those are expressions we assume to be pure, and to be referring to
+ * the same reference when they're called repeatedly if no side effect
+ * occurs between calls.
+ *
+ * <p>Note that this is not relocatable: you must use a matcher in the
+ * same scope it has been created in, to avoid bugs with accessibility/shadowing/etc.
+ * You must take care yourself that no side-effect occurs.
+ */
+public final class StablePathMatcher {
+
+    // if owner == null, then the owner is `this`.
+    private final @Nullable JVariableSymbol owner;
+    private final ArrayDeque<Segment> path;
+
+    private StablePathMatcher(@Nullable JVariableSymbol owner, ArrayDeque<Segment> path) {
+        this.owner = owner;
+        this.path = path;
+    }
+
+    /**
+     * Returns true if the expression matches the path.
+     */
+    public boolean matches(@Nullable ASTExpression e) {
+        if (e == null) {
+            return false;
+        }
+        for (Segment segment : path) {
+            boolean isField = segment.isField;
+            String name = segment.name;
+            if (isField) {
+                if (!(e instanceof ASTFieldAccess)) {
+                    return false;
+                }
+                ASTFieldAccess access = (ASTFieldAccess) e;
+                if (!access.getName().equals(name)) {
+                    return false;
+                }
+                e = access.getQualifier();
+            } else {
+                if (!(e instanceof ASTMethodCall)) {
+                    return false;
+                }
+
+                ASTMethodCall call = (ASTMethodCall) e;
+                if (!call.getMethodName().equals(name) || call.getArguments().size() != 0) {
+                    return false;
+                }
+                e = call.getQualifier();
+            }
+        }
+
+
+        if (e instanceof ASTVariableAccess) {
+            return Objects.equals(((ASTVariableAccess) e).getReferencedSym(), owner);
+        } else if (e instanceof ASTFieldAccess) {
+            ASTFieldAccess fieldAccess = (ASTFieldAccess) e;
+            if (!JavaRuleUtil.isUnqualifiedThis(fieldAccess.getQualifier())) {
+                return false;
+            }
+            return Objects.equals(fieldAccess.getReferencedSym(), owner);
+        }
+        return false;
+    }
+
+    /**
+     * Returns a matcher matching the given expression if it is stable.
+     * Otherwise returns null.
+     */
+    public static @Nullable StablePathMatcher matching(ASTExpression e) {
+        JVariableSymbol owner = null;
+        ArrayDeque<Segment> segments = new ArrayDeque<>();
+
+        while (e != null) {
+            if (e instanceof ASTFieldAccess) {
+                ASTFieldAccess access = (ASTFieldAccess) e;
+                segments.addLast(new Segment(access.getName(), true));
+                e = access.getQualifier();
+            } else if (e instanceof ASTMethodCall) {
+                ASTMethodCall call = (ASTMethodCall) e;
+                if (JavaRuleUtil.isGetterCall(call)) {
+                    segments.addLast(new Segment(call.getMethodName(), false));
+                    e = call.getQualifier();
+                } else {
+                    return null;
+                }
+            } else if (e instanceof ASTVariableAccess) {
+                owner = ((ASTVariableAccess) e).getReferencedSym();
+                if (owner == null) {
+                    return null; // unresolved
+                }
+                break;
+            } else if (e instanceof ASTThisExpression) {
+                if (((ASTThisExpression) e).getQualifier() != null) {
+                    return null;
+                }
+                break;
+            } else if (e instanceof ASTSuperExpression) {
+                if (((ASTSuperExpression) e).getQualifier() != null) {
+                    return null;
+                }
+                break;
+            } else {
+                return null;
+            }
+        }
+
+        return new StablePathMatcher(owner, segments);
+    }
+
+    private static final class Segment {
+
+        final String name;
+        final boolean isField;
+
+        Segment(String name, boolean isField) {
+            this.name = name;
+            this.isField = isField;
+        }
+
+        @Override
+        public String toString() {
+            return isField ? "." + name
+                           : "." + name + "()";
+        }
+    }
+}
@@ -5,7 +5,7 @@
 package net.sourceforge.pmd.lang.java.rule.xpath.internal;

 import net.sourceforge.pmd.lang.java.ast.InvocationNode;
-import net.sourceforge.pmd.lang.java.types.TypeTestUtil.InvocationMatcher;
+import net.sourceforge.pmd.lang.java.types.InvocationMatcher;

 import net.sf.saxon.trans.XPathException;

@@ -1800,35 +1800,22 @@ public class TryWithResources {
          class="net.sourceforge.pmd.lang.rule.XPathRule"
          externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#usevarargs">
        <description>
-Java 5 introduced the varargs parameter declaration for methods and constructors.  This syntactic
+Java 5 introduced the varargs parameter declaration for methods and constructors. This syntactic
 sugar provides flexibility for users of these methods and constructors, allowing them to avoid
 having to deal with the creation of an array.
+
+Byte arrays in any method and String arrays in `public static void main(String[])` methods are ignored.
        </description>
        <priority>4</priority>
        <properties>
-            <property name="version" value="2.0" />
            <property name="xpath">
                <value>
 <![CDATA[
-//FormalParameters/FormalParameter
-    [position()=last()]
-    [VariableDeclaratorId/@ArrayType=true()]
-    [@Varargs=false()]
-    [not (./Type[@ArrayType=true()]/ReferenceType[PrimitiveType[@Image='byte']])]
-    [not (./Type/ReferenceType[ClassOrInterfaceType[@Image='Byte']])]
-    [not (./Type/PrimitiveType[@Image='byte'])]
-    [not (ancestor::MethodDeclaration/preceding-sibling::Annotation/*/Name[@Image='Override'])]
-    [not(
-         ancestor::MethodDeclaration
-            [    @Public=true()
-             and @Static=true()
-             and child::ResultType[@Void=true()]
-             and @Name = 'main'
-             and @Arity = 1
-            ]
-         (: Type of the formal parameter here. :)
-         and pmd-java:typeIs('java.lang.String[]')
-    )]
+//FormalParameters[not(parent::MethodDeclaration[@Overridden=true() or @MainMethod=true()])]
+  /FormalParameter[position()=last()]
+   [@Varargs=false()]
+   [ArrayType[not(PrimitiveType[@Kind = "byte"] or ClassOrInterfaceType[pmd-java:typeIs('java.lang.Byte')])]
+    or VariableDeclaratorId[ArrayDimensions] and (PrimitiveType[not(@Kind="byte")] or ClassOrInterfaceType[not(pmd-java:typeIs('java.lang.Byte'))])]
 ]]>
                </value>
            </property>
@@ -1204,18 +1204,28 @@ public class SomeClass {
    <rule name="PrematureDeclaration"
          language="java"
          since="5.0"
-          message="Avoid declaring a variable if it is unreferenced before a possible exit point."
+          message="Declaration of ''{0}'' can be moved closer to its usages"
          class="net.sourceforge.pmd.lang.java.rule.codestyle.PrematureDeclarationRule"
          externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_codestyle.html#prematuredeclaration">
        <description>
-Checks for variables that are defined before they might be used. A reference is deemed to be premature if it is created right before a block of code that doesn't use it that also has the ability to return or throw an exception.
+Checks for variables that are defined before they might be used. A declaration is
+deemed to be premature if there are some statements that may return or throw an
+exception between the time the variable is declared and the time it is first read.
+
+Some variables cannot be declared close to their first usage because of side-effects
+occurring before they're first used. We try to avoid reporting those by considering
+most method and constructor invocations to be impure. See the second example.
+
+Note that this rule is meant to improve code readability but is not an optimization.
+A smart JIT will not care whether the variable is declared prematurely or not, as it
+can reorder code.
        </description>
        <priority>3</priority>
        <example>
 <![CDATA[
 public int getLength(String[] strings) {

-    int length = 0; // declared prematurely
+    int length = 0; // could be moved closer to the loop

    if (strings == null || strings.length == 0) return 0;

@@ -1225,6 +1235,25 @@ public int getLength(String[] strings) {

    return length;
 }
+]]>
+        </example>
+        <example>
+<![CDATA[
+public int getLength(String[] strings) {
+
+    int startTime = System.nanoTime(); // cannot be moved because initializer is impure
+
+    if (strings == null || strings.length == 0) {
+        // some error logic
+        throw new SomeException(...);
+    }
+
+    for (String str : strings) {
+        length += str.length();
+    }
+
+    return System.nanoTime() - startTime;
+}
 ]]>
        </example>
    </rule>
@@ -697,7 +697,7 @@ private int getMoreFoo(){
    <rule name="BrokenNullCheck"
          language="java"
          since="3.8"
-          message="Method call on object which may be null"
+          message="This expression will throw a NullPointerException"
          class="net.sourceforge.pmd.lang.java.rule.errorprone.BrokenNullCheckRule"
          externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#brokennullcheck">
        <description>
@@ -13,6 +13,7 @@ import org.checkerframework.checker.nullness.qual.NonNull;
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.ast.test.RelevantAttributePrinter;
 import net.sourceforge.pmd.lang.java.ast.ASTExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTModifierList;
 import net.sourceforge.pmd.lang.java.ast.JModifier;
 import net.sourceforge.pmd.lang.rule.xpath.Attribute;
@@ -40,7 +41,13 @@ public class JavaAttributesPrinter extends RelevantAttributePrinter {
            // everytime. OTOH failing dump tests would warn us that we removed
            // something that wasn't deprecated
            || attribute.isDeprecated()
-            || attribute.getName().equals("Expression") && node instanceof ASTExpression;
+            || "MainMethod".equals(attribute.getName()) && node instanceof ASTMethodDeclaration && !isBooleanTrue(attribute.getValue())
+            || "Expression".equals(attribute.getName()) && node instanceof ASTExpression;
+    }
+
+    private boolean isBooleanTrue(Object o) {
+        // for some reason Boolean::new is called somewhere in the reflection layer
+        return o instanceof Boolean && (Boolean) o;
    }

    private AttributeInfo getModifierAttr(String name, Set<JModifier> mods) {
@@ -11,22 +11,18 @@ import org.junit.Test;

 import net.sourceforge.pmd.lang.ast.ParseException;
 import net.sourceforge.pmd.lang.ast.test.BaseParsingHelper;
-import net.sourceforge.pmd.lang.ast.test.BaseTreeDumpTest;
-import net.sourceforge.pmd.lang.ast.test.RelevantAttributePrinter;
+import net.sourceforge.pmd.lang.java.BaseJavaTreeDumpTest;
 import net.sourceforge.pmd.lang.java.JavaParsingHelper;
 import net.sourceforge.pmd.lang.java.symbols.JElementSymbol;
 import net.sourceforge.pmd.lang.java.types.JPrimitiveType;

-public class Java16TreeDumpTest extends BaseTreeDumpTest {
+public class Java16TreeDumpTest extends BaseJavaTreeDumpTest {
    private final JavaParsingHelper java16 =
            JavaParsingHelper.WITH_PROCESSING.withDefaultVersion("16")
                    .withResourceContext(Java15TreeDumpTest.class, "jdkversiontests/java16/");
    private final JavaParsingHelper java16p = java16.withDefaultVersion("16-preview");
    private final JavaParsingHelper java15 = java16.withDefaultVersion("15");

-    public Java16TreeDumpTest() {
-        super(new RelevantAttributePrinter(), ".java");
-    }

    @Override
    public BaseParsingHelper<?, ?> getParser() {
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices;

 import net.sourceforge.pmd.testframework.PmdRuleTst;

-@org.junit.Ignore("Rule has not been updated yet")
 public class UseVarargsTest extends PmdRuleTst {
    // no additional unit tests
 }