diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index bb8cef8407..24ed6f31bd 100644 --- a/docs/pages/release_notes.md +++ b/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 %} diff --git a/pmd-apex/src/main/resources/category/apex/bestpractices.xml b/pmd-apex/src/main/resources/category/apex/bestpractices.xml index 4c43789af4..9d2913a771 100644 --- a/pmd-apex/src/main/resources/category/apex/bestpractices.xml +++ b/pmd-apex/src/main/resources/category/apex/bestpractices.xml @@ -24,7 +24,7 @@ improves the readability of test output. @@ -106,12 +106,12 @@ Apex unit tests should not use @isTest(seeAllData=true) because it opens up the diff --git a/pmd-apex/src/main/resources/category/apex/design.xml b/pmd-apex/src/main/resources/category/apex/design.xml index bef21cd3f7..da9bad6e11 100644 --- a/pmd-apex/src/main/resources/category/apex/design.xml +++ b/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 @@ -165,11 +165,11 @@ Empty If Statement finds instances where a condition is checked but nothing is d @@ -199,9 +199,9 @@ Empty block statements serve no purpose and should be removed. Example: + * 
+ * ExpressionPrinter printer = new ExpressionPrinter();
+ * printer.visit(query.xpathExpression.getInternalExpression());
+ *
*/ -public class ExpressionPrinter extends Visitor { +public class ExpressionPrinter extends SaxonExprVisitor { private int depth = 0; private void print(String s) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java index d621878023..9c9bcdb855 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java +++ b/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; *

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 documentOrderComparator() { return net.sourceforge.pmd.lang.rule.xpath.internal.DocumentSorter.INSTANCE; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/Visitor.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/SaxonExprVisitor.java similarity index 79% rename from pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/Visitor.java rename to pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/SaxonExprVisitor.java index 088724df7e..d1dc8dd873 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/Visitor.java +++ b/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; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/SplitUnions.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/SplitUnions.java index 20120c65ef..7d45b544b3 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/SplitUnions.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/SplitUnions.java @@ -17,7 +17,7 @@ import net.sf.saxon.expr.VennExpression; * *

E.g. "//A | //B | //C" will result in 3 expressions "//A", "//B", and "//C". */ -class SplitUnions extends Visitor { +class SplitUnions extends SaxonExprVisitor { private List expressions = new ArrayList<>(); @Override diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java index 0dc68954ab..ae4643c8d6 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java +++ b/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(); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQueryTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQueryTest.java index c2c48be729..93b7d4e3a3 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQueryTest.java +++ b/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 { - public static boolean typeIs(final XPathContext context, final String fullTypeName) { - return false; - } + @Test + public void ruleChainVisitsCustomFunctions() { + SaxonXPathRuleQuery query = createQuery("//dummyNode[pmd-dummy:typeIs(@Image)]"); + List 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 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 diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 31e772b928..4cc5f2b087 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -924,7 +924,7 @@ void RecordBodyDeclaration() #void : private void RecordCtorLookahead() #void: {} { - Modifiers() [ TypeParameters() ] ("throws" | "{") + Modifiers() "{" } void RecordConstructorDeclaration(): @@ -933,7 +933,6 @@ void RecordConstructorDeclaration(): } { modifiers = Modifiers() { jjtThis.setModifiers(modifiers); } - [TypeParameters()] { jjtThis.setImage(token.image); } Block() } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTRecordConstructorDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTRecordConstructorDeclaration.java index cebb53fc4c..a810da9a15 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTRecordConstructorDeclaration.java +++ b/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} *