Merge branch '7.0.x' into remove-more-deprecated-things

.github/dependabot.yml

@@ -0,0 +1,13 @@
+version: 2
+updates:
+  - package-ecosystem: "maven"
+    directory: "/"
+    schedule:
+      interval: "weekly"
+    target-branch: "master"
+    open-pull-requests-limit: 0
+  - package-ecosystem: "github-actions"
+    directory: "/"
+    schedule:
+      interval: "weekly"
+    target-branch: "master"

.github/workflows/build.yml

@@ -0,0 +1,29 @@
+name: Java CI
+
+on: [push, pull_request]
+
+jobs:
+  build:
+    runs-on: ${{ matrix.os }}
+    continue-on-error: ${{ matrix.experimental }}
+    strategy:
+      matrix:
+        os: [ ubuntu-latest , windows-latest , macos-latest ]
+        java: [ 11 ]
+        experimental: [ false ]
+
+    steps:
+      - uses: actions/checkout@v2.3.2
+      - uses: actions/cache@v2
+        with:
+          path: ~/.m2/repository
+          key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
+          restore-keys: |
+            ${{ runner.os }}-maven-
+      - name: Set up JDK ${{ matrix.java }}
+        uses: actions/setup-java@v1.4.0
+        with:
+          java-version: ${{ matrix.java }}
+      - name: Build with mvnw
+        run: |
+          ./mvnw clean install

docs/pages/release_notes.md

@@ -22,14 +22,26 @@ This is a {{ site.pmd.release_type }} release.
 ### Fixed Issues
 
 * pmd-java
-   * [#2708](https://github.com/pmd/pmd/pull/2708): \[java] False positive FinalFieldCouldBeStatic when using lombok Builder.Default
+    * [#2708](https://github.com/pmd/pmd/issues/2708): \[java] False positive FinalFieldCouldBeStatic when using lombok Builder.Default
+    * [#2738](https://github.com/pmd/pmd/issues/2738): \[java] Custom rule with @ExhaustiveEnumSwitch throws NPE
+    * [#2756](https://github.com/pmd/pmd/issues/2756): \[java] TypeTestUtil fails with NPE for anonymous class
+    * [#2759](https://github.com/pmd/pmd/issues/2759): \[java] False positive in UnusedAssignment
+    * [#2767](https://github.com/pmd/pmd/issues/2767): \[java] IndexOutOfBoundsException when parsing an initializer BlockStatement
 
 
 ### API Changes
 
+#### Deprecated API
+
+##### For removal
+
+*   {% jdoc !!core::RuleViolationComparator %}. Use {% jdoc !!core::RuleViolation#DEFAULT_COMPARATOR %} instead.
+
 ### External Contributions
 
+* [#2735](https://github.com/pmd/pmd/pull/2735): \[ci] Add github actions for a fast view of pr succeed/not - [XenoAmess](https://github.com/XenoAmess)
 * [#2747](https://github.com/pmd/pmd/pull/2747): \[java] Don't trigger FinalFieldCouldBeStatic when field is annotated with lombok @Builder.Default - [Ollie Abbey](https://github.com/ollieabbey)
+* [#2773](https://github.com/pmd/pmd/pull/2773): \[java] issue-2738: Adding null check to avoid npe when switch case is default - [Nimit Patel](https://github.com/nimit-patel)
 
 {% endtocmaker %}
 

pmd-core/src/main/java/net/sourceforge/pmd/Report.java

@@ -180,7 +180,7 @@ public class Report {
      * @param violation the violation to add
      */
     public void addRuleViolation(RuleViolation violation) {
-        int index = Collections.binarySearch(violations, violation, RuleViolationComparator.INSTANCE);
+        int index = Collections.binarySearch(violations, violation, RuleViolation.DEFAULT_COMPARATOR);
         violations.add(index < 0 ? -index - 1 : index, violation);
         for (ThreadSafeReportListener listener : listeners) {
             listener.ruleViolationAdded(violation);
@@ -228,7 +228,7 @@ public class Report {
         suppressedRuleViolations.addAll(r.suppressedRuleViolations);
 
         for (RuleViolation violation : r.getViolations()) {
-            int index = Collections.binarySearch(violations, violation, RuleViolationComparator.INSTANCE);
+            int index = Collections.binarySearch(violations, violation, RuleViolation.DEFAULT_COMPARATOR);
             violations.add(index < 0 ? -index - 1 : index, violation);
         }
     }
@@ -245,7 +245,7 @@ public class Report {
      * Returns an unmodifiable list of violations that have been
      * recorded until now. None of those violations were suppressed.
      *
-     * <p>The violations list is sorted with {@link RuleViolationComparator#INSTANCE}.
+     * <p>The violations list is sorted with {@link RuleViolation#DEFAULT_COMPARATOR}.
      */
     public final List<RuleViolation> getViolations() {
         return Collections.unmodifiableList(violations);

pmd-core/src/main/java/net/sourceforge/pmd/RuleViolation.java

@@ -4,6 +4,8 @@
 
 package net.sourceforge.pmd;
 
+import java.util.Comparator;
+
 /**
  * A RuleViolation is created by a Rule when it identifies a violation of the
  * Rule constraints. RuleViolations are simple data holders that are collected
@@ -16,6 +18,14 @@ package net.sourceforge.pmd;
  */
 public interface RuleViolation {
 
+    /**
+     * A comparator for rule violations. This compares all exposed attributes
+     * of a violation, filename first. The remaining parameters are compared
+     * in an unspecified order.
+     */
+    // TODO in java 8 this can be a chained Comparator.comparing call
+    Comparator<RuleViolation> DEFAULT_COMPARATOR = RuleViolationComparator.INSTANCE;
+
     /**
      * Get the Rule which identified this violation.
      *

pmd-core/src/main/java/net/sourceforge/pmd/RuleViolationComparator.java

@@ -17,7 +17,19 @@ import java.util.Comparator;
  * <li>End column</li>
  * <li>Rule name</li>
  * </ol>
+ *
+ * TODO why is begin line/begin column split?? would make more sense to use
+ *  - filename
+ *  - begin line
+ *  - begin column
+ *  - description
+ *  - rule name
+ *  - end line
+ *  - end column
+ *
+ * @deprecated Use {@link RuleViolation#DEFAULT_COMPARATOR}
  */
+@Deprecated
 public final class RuleViolationComparator implements Comparator<RuleViolation> {
 
     public static final RuleViolationComparator INSTANCE = new RuleViolationComparator();

pmd-core/src/test/java/net/sourceforge/pmd/RuleViolationComparatorTest.java

@@ -58,7 +58,7 @@ public class RuleViolationComparatorTest {
         Collections.shuffle(ruleViolations, random);
 
         // Sort
-        Collections.sort(ruleViolations, RuleViolationComparator.INSTANCE);
+        Collections.sort(ruleViolations, RuleViolation.DEFAULT_COMPARATOR);
 
         // Check
         int count = 0;

pmd-core/src/test/java/net/sourceforge/pmd/RuleViolationTest.java

@@ -8,6 +8,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
+import java.util.Comparator;
 
 import org.junit.Ignore;
 import org.junit.Test;
@@ -51,7 +52,7 @@ public class RuleViolationTest {
     @Test
     public void testComparatorWithDifferentFilenames() {
         Rule rule = new MockRule("name", "desc", "msg", "rulesetname");
-        RuleViolationComparator comp = RuleViolationComparator.INSTANCE;
+        Comparator<RuleViolation> comp = RuleViolation.DEFAULT_COMPARATOR;
         RuleContext ctx = new RuleContext();
         ctx.setSourceCodeFile(new File("filename1"));
         DummyNode s = new DummyNode();
@@ -68,7 +69,7 @@ public class RuleViolationTest {
     @Test
     public void testComparatorWithSameFileDifferentLines() {
         Rule rule = new MockRule("name", "desc", "msg", "rulesetname");
-        RuleViolationComparator comp = RuleViolationComparator.INSTANCE;
+        Comparator<RuleViolation> comp = RuleViolation.DEFAULT_COMPARATOR;
         RuleContext ctx = new RuleContext();
         ctx.setSourceCodeFile(new File("filename"));
         DummyNode s = new DummyNode();
@@ -85,7 +86,7 @@ public class RuleViolationTest {
     @Test
     public void testComparatorWithSameFileSameLines() {
         Rule rule = new MockRule("name", "desc", "msg", "rulesetname");
-        RuleViolationComparator comp = RuleViolationComparator.INSTANCE;
+        Comparator<RuleViolation> comp = RuleViolation.DEFAULT_COMPARATOR;
         RuleContext ctx = new RuleContext();
         ctx.setSourceCodeFile(new File("filename"));
         DummyNode s = new DummyNode();

pmd-java/etc/grammar/Java.jjt

@@ -1731,8 +1731,9 @@ void BlockStatement():
         |
         {
             List<ASTAnnotation> annotations = new ArrayList<ASTAnnotation>();
-            while (jjtree.peekNode() instanceof ASTAnnotation) {
-              annotations.add((ASTAnnotation) jjtree.popNode());
+            if (jjtree.nodeArity() > 0) { // peekNode would throw if the stack is empty
+                while (jjtree.peekNode() instanceof ASTAnnotation) {
+                  annotations.add((ASTAnnotation) jjtree.popNode());}
             }
         }
         LocalVariableDeclaration()

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

@@ -79,7 +79,10 @@ public class ASTSwitchStatement extends AbstractJavaNode implements Iterable<AST
                 // since this is an enum switch, the labels are necessarily
                 // the simple name of some enum constant.
 
-                constantNames.remove(label.getFirstDescendantOfType(ASTName.class).getImage());
+                // descendant can be null for default case
+                if (label.getFirstDescendantOfType(ASTName.class) != null) {
+                    constantNames.remove(label.getFirstDescendantOfType(ASTName.class).getImage());
+                }
 
             }
 

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

@@ -890,11 +890,6 @@ public class UnusedAssignmentRule extends AbstractJavaRule {
             // For the record this has problems with call chains with side effects, like
             //  a.foo(a = 2).bar(a = 3);
 
-            if (!state.isInTryBlock()) {
-                return;
-            }
-            // otherwise any method/ctor call may throw
-
             // In 7.0, with the precise type/overload resolution, we
             // could only target methods that throw checked exceptions
             // (unless some catch block catches an unchecked exceptions)
@@ -902,7 +897,7 @@ public class UnusedAssignmentRule extends AbstractJavaRule {
                 if (child instanceof ASTPrimarySuffix && ((ASTPrimarySuffix) child).isArguments()
                     || child instanceof ASTPrimarySuffix && child.getNumChildren() > 0 && child.getChild(0) instanceof ASTAllocationExpression
                     || child instanceof ASTPrimaryPrefix && child.getNumChildren() > 0 && child.getChild(0) instanceof ASTAllocationExpression) {
-                    state.abruptCompletionByThrow(true);
+                    state.abruptCompletionByThrow(true); // this is a noop if we're outside a try block that has catch/finally
                 }
             }
         }
@@ -1380,11 +1375,6 @@ public class UnusedAssignmentRule extends AbstractJavaRule {
             return this;
         }
 
-        private boolean isInTryBlock() {
-            // ignore resources, once they're initialized everything's fine in the block
-            return !myCatches.isEmpty() || myFinally != null;
-        }
-
         SpanInfo absorb(SpanInfo other) {
             // Merge reaching defs of the other scope into this
             // This is used to join paths after the control flow has forked

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeTestUtil.java

@@ -106,7 +106,11 @@ public final class TypeTestUtil {
 
         final Class<?> clazz = loadClassWithNodeClassloader(node, canonicalName);
 
+
         if (clazz != null) {
+            if (clazz.getCanonicalName() == null) {
+                return false; // no canonical name, give up: we shouldn't be able to access them
+            }
             return clazz.isAssignableFrom(nodeType);
         } else {
             return fallbackIsA(node, canonicalName, true);
@@ -174,8 +178,16 @@ public final class TypeTestUtil {
         }
 
 
-        return node.getType() == null ? fallbackIsA(node, canonicalName, false)
-                                      : node.getType().getCanonicalName().equals(canonicalName);
+        if (node.getType() == null) {
+            return fallbackIsA(node, canonicalName, false);
+        }
+
+        String canoname = node.getType().getCanonicalName();
+        if (canoname == null) {
+            // anonymous/local class, or class nested within one of those
+            return false;
+        }
+        return canoname.equals(canonicalName);
     }
 
 

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTSwitchStatementTest.java

@@ -0,0 +1,21 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.ast;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ASTSwitchStatementTest extends BaseParserTest {
+
+    @Test
+    public void exhaustiveEnumSwitchWithDefault() {
+        ASTSwitchStatement switchStatement = getNodes(ASTSwitchStatement.class,
+                "import java.nio.file.AccessMode; class Foo { void bar(AccessMode m) {"
+                + "switch (m) { case READ: break; default: break; } } }")
+                .get(0);
+        Assert.assertFalse(switchStatement.isExhaustiveEnumSwitch()); // this should not throw a NPE...
+        Assert.assertTrue(switchStatement.hasDefaultCase());
+    }
+}

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java

@@ -172,6 +172,16 @@ public class ParserCornersTest {
         java8.parseResource("GitHubBug207.java");
     }
 
+    @Test
+    public void testGitHubBug2767() {
+        // PMD fails to parse an initializer block.
+        // PMD 6.26.0 parses this code just fine.
+        java.withDefaultVersion("15-preview")
+            .parse("class Foo {\n"
+                       + "    {final int I;}\n"
+                       + "}\n");
+    }
+
     @Test
     public void testBug206() {
         java8.parse("public @interface Foo {" + "\n"

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/TypeTestUtilTest.java

@@ -6,6 +6,7 @@ package net.sourceforge.pmd.lang.java.types;
 
 import java.io.Serializable;
 import java.lang.annotation.Annotation;
+import java.util.concurrent.Callable;
 
 import org.junit.Assert;
 import org.junit.Rule;
@@ -13,6 +14,7 @@ import org.junit.Test;
 import org.junit.function.ThrowingRunnable;
 import org.junit.rules.ExpectedException;
 
+import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration;
@@ -20,6 +22,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTMarkerAnnotation;
 import net.sourceforge.pmd.lang.java.ast.ASTName;
 import net.sourceforge.pmd.lang.java.ast.TypeNode;
 import net.sourceforge.pmd.lang.java.symboltable.BaseNonParserTest;
+import net.sourceforge.pmd.lang.java.types.testdata.SomeClassWithAnon;
 
 public class TypeTestUtilTest extends BaseNonParserTest {
 
@@ -74,6 +77,31 @@ public class TypeTestUtilTest extends BaseNonParserTest {
         assertIsA(klass, Object.class);
     }
 
+    @Test
+    public void testAnonClassTypeNPE() {
+        // #2756
+
+        ASTAllocationExpression anon =
+            java.parseClass(SomeClassWithAnon.class)
+                .getFirstDescendantOfType(ASTAllocationExpression.class);
+
+
+        Assert.assertNotNull("Type should be resolved", anon.getType());
+        Assert.assertTrue("Anon class", anon.isAnonymousClass());
+        Assert.assertTrue("Anon class", anon.getType().isAnonymousClass());
+        Assert.assertTrue("Should be a Runnable", TypeTestUtil.isA(Runnable.class, anon));
+
+        // This is not a canonical name, so we give up early
+        Assert.assertFalse(TypeTestUtil.isA(SomeClassWithAnon.class.getName() + "$1", anon));
+        Assert.assertFalse(TypeTestUtil.isExactlyA(SomeClassWithAnon.class.getName() + "$1", anon));
+
+        // this is the failure case: if the binary name doesn't match, we test the canoname, which was null
+        Assert.assertFalse(TypeTestUtil.isA(Callable.class, anon));
+        Assert.assertFalse(TypeTestUtil.isA(Callable.class.getCanonicalName(), anon));
+        Assert.assertFalse(TypeTestUtil.isExactlyA(Callable.class, anon));
+        Assert.assertFalse(TypeTestUtil.isExactlyA(Callable.class.getCanonicalName(), anon));
+    }
+
     /**
      * If we don't have the annotation on the classpath,
      * we should resolve the full name via the import, if possible

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/testdata/SomeClassWithAnon.java

@@ -0,0 +1,25 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.types.testdata;
+
+/**
+ * #2756
+ */
+public class SomeClassWithAnon {
+
+    {
+        new Runnable() {
+
+            @Override
+            public void run() {
+
+            }
+        };
+
+
+    }
+
+
+}

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedAssignment.xml

@@ -3159,4 +3159,29 @@ public class UnusedAssignmentNative {
             ]]></code>
     </test-code>
 
+    <test-code>
+        <description>False positive with method that may throw in forked state (the if state) #2759</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+            class Test {
+                int a() {
+                    int a = 10;
+                    while (a > 0) {
+                        a--;
+                        try {
+                            if (dummy) {
+                                return somethingThatCanThrowRandomly(1);
+                            } else {
+                                return somethingThatCanThrowRandomly(2);
+                            }
+                        } catch (RuntimeException e) {
+                            // retry
+                        }
+                    }
+                    return 0;
+                }
+            }
+            ]]></code>
+    </test-code>
+
 </test-data>