Merge branch 'master' into 7.0.x

docs/pages/release_notes.md

@@ -41,6 +41,7 @@ Note that XPath 1.0 support, the default XPath version, is deprecated since PMD
 
 *   apex
     *   [#2210](https://github.com/pmd/pmd/issues/2210): \[apex] ApexCRUDViolation: Support WITH SECURITY_ENFORCED
+    *   [#2358](https://github.com/pmd/pmd/issues/2358): \[apex] Invalid Apex in Cognitive Complexity tests
 
 ### API Changes
 
@@ -106,6 +107,7 @@ implementations, and their corresponding Parser if it exists (in the same packag
 *   [#2312](https://github.com/pmd/pmd/pull/2312): \[apex] Update ApexCRUDViolation Rule - [Joshua S Arquilevich](https://github.com/jarquile)
 *   [#2314](https://github.com/pmd/pmd/pull/2314): \[doc] maven integration - Add version to plugin - [Pham Hai Trung](https://github.com/gpbp)
 *   [#2353](https://github.com/pmd/pmd/pull/2353): \[plsql] xmlforest with optional AS - [Piotr Szymanski](https://github.com/szyman23)
+*   [#2383](https://github.com/pmd/pmd/pull/2383): \[apex] Fix invalid apex in documentation - [Gwilym Kuiper](https://github.com/gwilymatgearset)
 
 {% endtocmaker %}
 

pmd-apex/src/main/resources/category/apex/bestpractices.xml

@@ -24,7 +24,7 @@ improves the readability of test output.
             <![CDATA[
 @isTest
 public class Foo {
-     @isTest
+    @isTest
     static void methodATest() {
         System.assertNotEquals('123', o.StageName); // not good
         System.assertEquals('123', o.StageName, 'Opportunity stageName is wrong.'); // good
@@ -50,12 +50,12 @@ with messages provide the developer a clearer idea of what the test does.
 <![CDATA[
 @isTest
 public class Foo {
-   public static testMethod void testSomething() {
+    public static testMethod void testSomething() {
-      Account a = null;
+        Account a = null;
-   // This is better than having a NullPointerException
+        // This is better than having a NullPointerException
-   // System.assertNotEquals(a, null, 'account not found');
+        // System.assertNotEquals(a, null, 'account not found');
-   a.toString();
+        a.toString();
-   }
+    }
 }
 ]]>
         </example>
@@ -106,12 +106,12 @@ Apex unit tests should not use @isTest(seeAllData=true) because it opens up the
 <![CDATA[
 @isTest(seeAllData = true)
 public class Foo {
-   public static testMethod void testSomething() {
+    public static testMethod void testSomething() {
-      Account a = null;
+        Account a = null;
-   // This is better than having a NullPointerException
+        // This is better than having a NullPointerException
-   // System.assertNotEquals(a, null, 'account not found');
+        // System.assertNotEquals(a, null, 'account not found');
-   a.toString();
+        a.toString();
-   }
+    }
 }
 ]]>
         </example>

pmd-apex/src/main/resources/category/apex/design.xml

@@ -121,7 +121,10 @@ public class Foo {
         if (a.Phone == null) {                          // +1
             a.Phone = phone;
             update a;
+            return true;
         }
+
+        return false;
     }
 
     // Has a cognitive complexity of 5
@@ -189,7 +192,7 @@ same datatype. These situations usually denote the need for new objects to wrap
         <example>
 <![CDATA[
 // too many arguments liable to be mixed up
-public void addPerson(int birthYear, int birthMonth, int birthDate, int height, int weight, int ssn) {
+public void addPerson(Integer birthYear, Integer birthMonth, Integer birthDate, Integer height, Integer weight, Integer ssn) {
     // ...
 }
 // preferred approach
@@ -272,8 +275,8 @@ lines of code that are split are counted as one.
 <![CDATA[
 public class Foo extends Bar {
     //this method only has 1 NCSS lines
-    public Integer methd() {
+    public Integer method() {
-        super.methd();
+        super.method();
 
 
 
@@ -382,11 +385,11 @@ city/state/zip fields could park them within a single Address field.
 <![CDATA[
 public class Person {
     // too many separate fields
-    int birthYear;
+    Integer birthYear;
-    int birthMonth;
+    Integer birthMonth;
-    int birthDate;
+    Integer birthDate;
-    float height;
+    Double height;
-    float weight;
+    Double weight;
 }
 
 public class Person {

pmd-apex/src/main/resources/category/apex/errorprone.xml

@@ -72,7 +72,7 @@ Avoid directly accessing Trigger.old and Trigger.new as it can lead to a bug. Tr
 trigger AccountTrigger on Account (before insert, before update) {
    Account a = Trigger.new[0]; //Bad: Accessing the trigger array directly is not recommended.
 
-   for ( Account a : Trigger.new ){
+   for ( Account a : Trigger.new ) {
         //Good: Iterate through the trigger.new array instead.
    }
 }
@@ -96,9 +96,9 @@ the logic can dynamically identify the proper data to operate against and not fa
 public without sharing class Foo {
     void foo() {
         //Error - hardcoded the record type id
-        if(a.RecordTypeId == '012500000009WAr'){
+        if (a.RecordTypeId == '012500000009WAr') {
             //do some logic here.....
-        } else if(a.RecordTypeId == '0123000000095Km'){
+        } else if (a.RecordTypeId == '0123000000095Km') {
             //do some logic here for a different record type...
         }
     }
@@ -131,12 +131,12 @@ or reported.
         <example>
 <![CDATA[
 public void doSomething() {
-  ...
+    ...
-  try {
+    try {
-    insert accounts;
+        insert accounts;
-  } catch (DmlException dmle) {
+    } catch (DmlException dmle) {
-    // not good
+        // not good
-  }
+    }
 }
 ]]>
         </example>
@@ -165,11 +165,11 @@ Empty If Statement finds instances where a condition is checked but nothing is d
         <example>
 <![CDATA[
 public class Foo {
-  public void bar(Integer x) {
+    public void bar(Integer x) {
-    if (x == 0) {
+        if (x == 0) {
-      // empty!
+            // empty!
+        }
     }
-  }
 }
 ]]>
         </example>
@@ -199,9 +199,9 @@ Empty block statements serve no purpose and should be removed.
 <![CDATA[
 public class Foo {
 
-   private int _bar;
+   private Integer _bar;
 
-   public void setBar(int bar) {
+   public void setBar(Integer bar) {
         // empty
    }
 
@@ -244,7 +244,7 @@ public class Foo {
 public class Foo {
     public void bar() {
         try {
-            int x=2;
+            Integer x=2;
         } finally {
             // empty!
         }

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java

@@ -77,7 +77,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
     /**
      * Representation of an XPath query, created at {@link #initializeXPathExpression()} using {@link #xpath}.
      */
-    private XPathExpression xpathExpression;
+    XPathExpression xpathExpression;
 
     /**
      * Holds the static context later used to match the variables in the dynamic context in

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/ExpressionPrinter.java

@@ -13,8 +13,14 @@ import net.sf.saxon.om.Axis;
 
 /**
  * Simple printer for saxon expressions. Might be useful for debugging / during development.
+ *
+ * <p>Example:
+ * <pre>
+ * ExpressionPrinter printer = new ExpressionPrinter();
+ * printer.visit(query.xpathExpression.getInternalExpression());
+ * </pre>
  */
-public class ExpressionPrinter extends Visitor {
+public class ExpressionPrinter extends SaxonExprVisitor {
     private int depth = 0;
 
     private void print(String s) {

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java

@@ -13,6 +13,7 @@ import net.sf.saxon.Configuration;
 import net.sf.saxon.expr.AxisExpression;
 import net.sf.saxon.expr.Expression;
 import net.sf.saxon.expr.FilterExpression;
+import net.sf.saxon.expr.LazyExpression;
 import net.sf.saxon.expr.PathExpression;
 import net.sf.saxon.expr.RootExpression;
 import net.sf.saxon.om.Axis;
@@ -33,17 +34,22 @@ import net.sf.saxon.type.Type;
  * <p>DocumentSorter expression is removed. The sorting of the resulting nodes needs to be done
  * after all (sub)expressions have been executed.
  */
-public class RuleChainAnalyzer extends Visitor {
+public class RuleChainAnalyzer extends SaxonExprVisitor {
     private final Configuration configuration;
     private String rootElement;
     private boolean rootElementReplaced;
+    private boolean insideLazyExpression;
+    private boolean foundPathInsideLazy;
 
     public RuleChainAnalyzer(Configuration currentConfiguration) {
         this.configuration = currentConfiguration;
     }
 
     public String getRootElement() {
-        return rootElement;
+        if (!foundPathInsideLazy && rootElementReplaced) {
+            return rootElement;
+        }
+        return null;
     }
 
     @Override
@@ -55,7 +61,7 @@ public class RuleChainAnalyzer extends Visitor {
 
     @Override
     public Expression visit(PathExpression e) {
-        if (rootElement == null) {
+        if (!insideLazyExpression && rootElement == null) {
             Expression result = super.visit(e);
             if (rootElement != null && !rootElementReplaced) {
                 if (result instanceof PathExpression) {
@@ -79,6 +85,9 @@ public class RuleChainAnalyzer extends Visitor {
             }
             return result;
         } else {
+            if (insideLazyExpression) {
+                foundPathInsideLazy = true;
+            }
             return super.visit(e);
         }
     }
@@ -96,6 +105,15 @@ public class RuleChainAnalyzer extends Visitor {
         return super.visit(e);
     }
 
+    @Override
+    public Expression visit(LazyExpression e) {
+        boolean prevCtx = insideLazyExpression;
+        insideLazyExpression = true;
+        Expression result = super.visit(e);
+        insideLazyExpression = prevCtx;
+        return result;
+    }
+
     public static Comparator<Node> documentOrderComparator() {
         return net.sourceforge.pmd.lang.rule.xpath.internal.DocumentSorter.INSTANCE;
     }

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/SaxonExprVisitor.java

@@ -5,8 +5,10 @@
 package net.sourceforge.pmd.lang.rule.xpath.internal;
 
 import net.sf.saxon.expr.AxisExpression;
+import net.sf.saxon.expr.BooleanExpression;
 import net.sf.saxon.expr.Expression;
 import net.sf.saxon.expr.FilterExpression;
+import net.sf.saxon.expr.LazyExpression;
 import net.sf.saxon.expr.LetExpression;
 import net.sf.saxon.expr.PathExpression;
 import net.sf.saxon.expr.QuantifiedExpression;
@@ -14,7 +16,7 @@ import net.sf.saxon.expr.RootExpression;
 import net.sf.saxon.expr.VennExpression;
 import net.sf.saxon.sort.DocumentSorter;
 
-abstract class Visitor {
+abstract class SaxonExprVisitor {
     public Expression visit(DocumentSorter e) {
         Expression base = visit(e.getBaseExpression());
         return new DocumentSorter(base);
@@ -63,6 +65,18 @@ abstract class Visitor {
         return result;
     }
 
+    public Expression visit(LazyExpression e) {
+        Expression base = visit(e.getBaseExpression());
+        return LazyExpression.makeLazyExpression(base);
+    }
+
+    public Expression visit(BooleanExpression e) {
+        final Expression[] operands = e.getOperands();
+        Expression operand0 = visit(operands[0]);
+        Expression operand1 = visit(operands[1]);
+        return new BooleanExpression(operand0, e.getOperator(), operand1);
+    }
+
     public Expression visit(Expression expr) {
         Expression result;
         if (expr instanceof DocumentSorter) {
@@ -81,6 +95,10 @@ abstract class Visitor {
             result = visit((QuantifiedExpression) expr);
         } else if (expr instanceof LetExpression) {
             result = visit((LetExpression) expr);
+        } else if (expr instanceof LazyExpression) {
+            result = visit((LazyExpression) expr);
+        } else if (expr instanceof BooleanExpression) {
+            result = visit((BooleanExpression) expr);
         } else {
             result = expr;
         }

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/SplitUnions.java

@@ -17,7 +17,7 @@ import net.sf.saxon.expr.VennExpression;
  * 
  * <p>E.g. "//A | //B | //C" will result in 3 expressions "//A", "//B", and "//C".
  */
-class SplitUnions extends Visitor {
+class SplitUnions extends SaxonExprVisitor {
     private List<Expression> expressions = new ArrayList<>();
 
     @Override

pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java

@@ -17,10 +17,14 @@ import net.sourceforge.pmd.lang.ast.DummyNode;
 import net.sourceforge.pmd.lang.ast.DummyRoot;
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.ast.ParseException;
+import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler;
 import net.sourceforge.pmd.lang.rule.AbstractRuleChainVisitor;
 import net.sourceforge.pmd.lang.rule.ParametricRuleViolation;
 import net.sourceforge.pmd.lang.rule.impl.DefaultRuleViolationFactory;
 
+import net.sf.saxon.expr.XPathContext;
+import net.sf.saxon.sxpath.IndependentContext;
+
 /**
  * Dummy language used for testing PMD.
  */
@@ -67,6 +71,26 @@ public class DummyLanguageModule extends BaseLanguageModule {
             super(DummyAstStages.class);
         }
 
+        public static class TestFunctions {
+            public static boolean typeIs(final XPathContext context, final String fullTypeName) {
+                return false;
+            }
+        }
+
+        @Override
+        public XPathHandler getXPathHandler() {
+            return new DefaultASTXPathHandler() {
+                @Override
+                public void initialize(IndependentContext context) {
+                    super.initialize(context, LanguageRegistry.getLanguage(DummyLanguageModule.NAME), TestFunctions.class);
+                }
+
+                @Override
+                public void initialize() {
+                }
+            };
+        }
+
         @Override
         public RuleViolationFactory getRuleViolationFactory() {
             return new RuleViolationFactory();

pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQueryTest.java

@@ -18,7 +18,6 @@ import net.sourceforge.pmd.properties.PropertyDescriptor;
 import net.sourceforge.pmd.properties.PropertyFactory;
 
 import net.sf.saxon.expr.Expression;
-import net.sf.saxon.expr.XPathContext;
 
 public class SaxonXPathRuleQueryTest {
 
@@ -64,10 +63,42 @@ public class SaxonXPathRuleQueryTest {
         assertExpression("((((/)/descendant::element(dummyNode, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Test2, xs:anyAtomicType)), ($qq:qq1741979653 singleton eq true()))])[QuantifiedExpression(Atomizer(attribute::attribute(Test1, xs:anyAtomicType)), ($qq:qq1529060733 singleton eq false()))])", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
     }
 
-    public static class TestFunctions {
+    @Test
-        public static boolean typeIs(final XPathContext context, final String fullTypeName) {
+    public void ruleChainVisitsCustomFunctions() {
-            return false;
+        SaxonXPathRuleQuery query = createQuery("//dummyNode[pmd-dummy:typeIs(@Image)]");
-        }
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(1, ruleChainVisits.size());
+        Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
+        Assert.assertEquals(2, query.nodeNameToXPaths.size());
+        assertExpression("(self::node()[pmd-dummy:typeIs(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(Image, xs:anyAtomicType))))))])", query.nodeNameToXPaths.get("dummyNode").get(0));
+        assertExpression("DocumentSorter((((/)/descendant-or-self::node())/(child::element(dummyNode, xs:anyType)[pmd-dummy:typeIs(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(Image, xs:anyAtomicType))))))])))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
+    }
+
+    /**
+     * If a query contains another unbounded path expression other than the first one, it must be
+     * excluded from rule chain execution. Saxon itself optimizes this quite good already.
+     */
+    @Test
+    public void ruleChainVisitsUnboundedPathExpressions() {
+        SaxonXPathRuleQuery query = createQuery("//dummyNode[//ClassOrInterfaceType]");
+        List<String> ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(0, ruleChainVisits.size());
+        Assert.assertEquals(1, query.nodeNameToXPaths.size());
+        assertExpression("LetExpression(LazyExpression(((/)/descendant::element(ClassOrInterfaceType, xs:anyType))), (((/)/descendant::element(dummyNode, xs:anyType))[$zz:zz771775563]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
+
+        // second sample, more complex
+        query = createQuery("//dummyNode[ancestor::ClassOrInterfaceDeclaration[//ClassOrInterfaceType]]");
+        ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(0, ruleChainVisits.size());
+        Assert.assertEquals(1, query.nodeNameToXPaths.size());
+        assertExpression("LetExpression(LazyExpression(((/)/descendant::element(ClassOrInterfaceType, xs:anyType))), (((/)/descendant::element(dummyNode, xs:anyType))[(ancestor::element(ClassOrInterfaceDeclaration, xs:anyType)[$zz:zz106374177])]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
+
+        // third example, with boolean expr
+        query = createQuery("//dummyNode[//ClassOrInterfaceType or //OtherNode]");
+        ruleChainVisits = query.getRuleChainVisits();
+        Assert.assertEquals(0, ruleChainVisits.size());
+        Assert.assertEquals(1, query.nodeNameToXPaths.size());
+        assertExpression("LetExpression(LazyExpression((((/)/descendant::element(ClassOrInterfaceType, xs:anyType)) or ((/)/descendant::element(OtherNode, xs:anyType)))), (((/)/descendant::element(dummyNode, xs:anyType))[$zz:zz1364913072]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
     }
 
     @Test

pmd-java/etc/grammar/Java.jjt

@@ -924,7 +924,7 @@ void RecordBodyDeclaration() #void :
 private void RecordCtorLookahead() #void:
 {}
 {
-    Modifiers() [ TypeParameters() ] <IDENTIFIER> ("throws" |  "{")
+    Modifiers() <IDENTIFIER> "{"
 }
 
 void RecordConstructorDeclaration():
@@ -933,7 +933,6 @@ void RecordConstructorDeclaration():
 }
 {
   modifiers = Modifiers() { jjtThis.setModifiers(modifiers); }
-  [TypeParameters()]
   <IDENTIFIER> { jjtThis.setImage(token.image); }
   Block()
 }

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

@@ -14,7 +14,6 @@ import net.sourceforge.pmd.annotation.Experimental;
  *
  * RecordConstructorDeclaration ::=  ({@linkplain ASTAnnotation Annotation})*
  *                                   RecordModifiers
- *                                   {@linkplain ASTTypeParameters TypeParameters}?
  *                                   <IDENTIFIER>
  *                                   {@link ASTBlock Block}
  *