From eaceefc39e97df247667ccf1990a7fe1b414f19c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 21 Feb 2019 16:51:07 +0100 Subject: [PATCH 01/64] Update XPath rules to 2.0 --- .../resources/category/apex/codestyle.xml | 12 ++-- .../resources/category/apex/errorprone.xml | 2 +- .../pmd/lang/ast/xpath/saxon/ElementNode.java | 7 +++ .../sourceforge/pmd/lang/rule/XPathRule.java | 6 +- .../net/sourceforge/pmd/xml/j2ee.xml | 4 +- .../resources/category/java/bestpractices.xml | 12 ++-- .../resources/category/java/codestyle.xml | 57 ++++++++--------- .../main/resources/category/java/design.xml | 20 +++--- .../resources/category/java/documentation.xml | 6 +- .../resources/category/java/errorprone.xml | 63 +++++++++---------- .../category/java/multithreading.xml | 8 +-- .../resources/category/java/performance.xml | 4 +- .../category/ecmascript/bestpractices.xml | 2 +- .../category/ecmascript/codestyle.xml | 16 ++--- .../category/ecmascript/errorprone.xml | 6 +- .../resources/category/plsql/codestyle.xml | 4 +- .../resources/category/xsl/performance.xml | 2 +- 17 files changed, 117 insertions(+), 114 deletions(-) diff --git a/pmd-apex/src/main/resources/category/apex/codestyle.xml b/pmd-apex/src/main/resources/category/apex/codestyle.xml index 262b6f9757..72c22c83ce 100644 --- a/pmd-apex/src/main/resources/category/apex/codestyle.xml +++ b/pmd-apex/src/main/resources/category/apex/codestyle.xml @@ -48,9 +48,9 @@ from the rest. 0] +//IfBlockStatement/BlockStatement[@CurlyBrace= false()][count(child::*) > 0] | -//IfElseBlockStatement/BlockStatement[@CurlyBrace='false'][count(child::*) > 0] +//IfElseBlockStatement/BlockStatement[@CurlyBrace= false()][count(child::*) > 0] ]]> @@ -85,7 +85,7 @@ controlled from the rest. @@ -142,9 +142,9 @@ from the rest. @@ -348,7 +348,7 @@ controlled from the rest. diff --git a/pmd-apex/src/main/resources/category/apex/errorprone.xml b/pmd-apex/src/main/resources/category/apex/errorprone.xml index c96110f960..17c9182938 100644 --- a/pmd-apex/src/main/resources/category/apex/errorprone.xml +++ b/pmd-apex/src/main/resources/category/apex/errorprone.xml @@ -189,7 +189,7 @@ Empty block statements serve no purpose and should be removed. diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java index 4663e7f1bf..94bec1fbb1 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java @@ -122,6 +122,13 @@ public class ElementNode extends AbstractNodeInfo { return result; } + + @Override + public String getDisplayName() { + return getLocalPart(); + } + + @SuppressWarnings("PMD.MissingBreakInSwitch") @Override public AxisIterator iterateAxis(byte axisNumber) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java index 773d64a5c0..55257976f9 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java @@ -41,8 +41,8 @@ public class XPathRule extends AbstractRule { static { Map tmp = new HashMap<>(); - tmp.put(XPATH_1_0, XPATH_1_0); - tmp.put(XPATH_1_0_COMPATIBILITY, XPATH_1_0_COMPATIBILITY); + tmp.put(XPATH_1_0, XPATH_2_0); + tmp.put(XPATH_1_0_COMPATIBILITY, XPATH_2_0); tmp.put(XPATH_2_0, XPATH_2_0); XPATH_VERSIONS = Collections.unmodifiableMap(tmp); } @@ -51,7 +51,7 @@ public class XPathRule extends AbstractRule { public static final EnumeratedProperty VERSION_DESCRIPTOR = EnumeratedProperty.named("version") .desc("XPath specification version") .mappings(XPATH_VERSIONS) - .defaultValue(XPATH_1_0) + .defaultValue(XPATH_2_0) .type(String.class) .uiOrder(2.0f) .build(); diff --git a/pmd-core/src/test/resources/net/sourceforge/pmd/xml/j2ee.xml b/pmd-core/src/test/resources/net/sourceforge/pmd/xml/j2ee.xml index f158927f95..607967f6fd 100644 --- a/pmd-core/src/test/resources/net/sourceforge/pmd/xml/j2ee.xml +++ b/pmd-core/src/test/resources/net/sourceforge/pmd/xml/j2ee.xml @@ -324,9 +324,9 @@ behavior especially when instances are distributed by the container on several J ) and (./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration[ - (./FieldDeclaration[@Static = 'true']) + (./FieldDeclaration[@Static = true()]) and - (./FieldDeclaration[@Final = 'false']) + (./FieldDeclaration[@Final = false()]) ]) ] ]]> diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 5c089e46d7..7dd321aee1 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -27,8 +27,8 @@ directly) a protected constructor can be provided prevent direct instantiation. @@ -380,7 +380,7 @@ better placed in classes or enums. See Effective Java, item 19. @@ -426,8 +426,8 @@ By convention, the default label should be the last label in a switch statement. @@ -1075,7 +1075,7 @@ can be avoided, they will just return false. //PrimaryExpression[ PrimaryPrefix[Name[(ends-with(@Image, '.equals'))]] [ - (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal[@StringLiteral='true']) + (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal[@StringLiteral= true()]) and ( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 ) ] diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index 6f8e3e2ae4..1cb7beabeb 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -28,12 +28,12 @@ by {% rule java/codestyle/ClassNamingConventions %}. @@ -116,7 +116,7 @@ If the goal is to avoid defining constants in a scope smaller than the class, th @@ -244,12 +244,13 @@ visibility cannot be reduced). Clarify your intent by using private or package a 3 + @@ -318,7 +319,7 @@ prefix for these methods. @@ -623,16 +624,16 @@ usage by developers who should be implementing their own versions in the concret 1 or - string:upper-case(@Image) != @Image + upper-case(@Image) != @Image ] ]]> @@ -947,7 +948,7 @@ by the rule {% rule java/codestyle/ControlStatementBraces %}. @@ -1728,9 +1729,9 @@ by the more general rule {% rule java/codestyle/FieldNamingConventions %}. @@ -1767,7 +1768,7 @@ which class a static member comes from (Sun 1.5 Language Guide). $maximumStaticImports] +.[count(ImportDeclaration[@Static = true()]) > $maximumStaticImports] ]]> @@ -1970,15 +1971,15 @@ which makes the code also more readable. @@ -2031,7 +2032,7 @@ List stringsWithDiamond = new ArrayList<>(); // using the diamond operat not(./CastExpression) and not(./EqualityExpression)] | -//Expression/AdditiveExpression[not(./PrimaryExpression/PrimaryPrefix/Literal[@StringLiteral='true'])] +//Expression/AdditiveExpression[not(./PrimaryExpression/PrimaryPrefix/Literal[@StringLiteral= true()])] /PrimaryExpression[1]/PrimaryPrefix/Expression[ count(*)=1 and not(./CastExpression) and @@ -2093,9 +2094,9 @@ public class Foo { diff --git a/pmd-java/src/main/resources/category/java/design.xml b/pmd-java/src/main/resources/category/java/design.xml index 73008d3e30..4a3dfa92dc 100644 --- a/pmd-java/src/main/resources/category/java/design.xml +++ b/pmd-java/src/main/resources/category/java/design.xml @@ -27,7 +27,7 @@ protected constructor in order to prevent instantiation than make the class misl @@ -339,9 +339,9 @@ is invoked by a inner class. = 1 ] -[count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[(@Public = 'true') or (@Protected = 'true') or (@PackagePrivate = 'true')]) = 0 ] +[@Final = false()] +[count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[@Private = true()]) >= 1 ] +[count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[(@Public = true()) or (@Protected = true()) or (@PackagePrivate = true())]) = 0 ] [not(.//ClassOrInterfaceDeclaration)] ]]> @@ -370,12 +370,12 @@ Sometimes two consecutive 'if' statements can be consolidated by separating thei @@ -727,7 +727,7 @@ in each object at runtime. @@ -1373,7 +1373,7 @@ ConditionalAndExpression [EqualityExpression[@Image='!=']//NullLiteral and InstanceOfExpression - [PrimaryExpression[count(PrimarySuffix[@ArrayDereference='true'])=0] + [PrimaryExpression[count(PrimarySuffix[@ArrayDereference= true()])=0] //Name[not(contains(@Image,'.'))]/@Image = ancestor::ConditionalAndExpression /EqualityExpression/PrimaryExpression/PrimaryPrefix/Name/@Image] and diff --git a/pmd-java/src/main/resources/category/java/documentation.xml b/pmd-java/src/main/resources/category/java/documentation.xml index 7b6d5cd81f..daee1d547d 100644 --- a/pmd-java/src/main/resources/category/java/documentation.xml +++ b/pmd-java/src/main/resources/category/java/documentation.xml @@ -94,8 +94,8 @@ and unintentional empty constructors. @@ -128,7 +128,7 @@ empty methods. diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index 938ca51d55..3415522b1e 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -734,7 +734,7 @@ public String bar(String string) { @Name='onStart' ] /Block[not( - (BlockStatement[1]/Statement/StatementExpression/PrimaryExpression[./PrimaryPrefix[@SuperModifier='true']]/PrimarySuffix[@Image= ancestor::MethodDeclaration/@Name]))] + (BlockStatement[1]/Statement/StatementExpression/PrimaryExpression[./PrimaryPrefix[@SuperModifier= true()]]/PrimarySuffix[@Image= ancestor::MethodDeclaration/@Name]))] [ancestor::ClassOrInterfaceDeclaration[ExtendsList/ClassOrInterfaceType[ pmd-java:typeIs('android.app.Activity') or pmd-java:typeIs('android.app.Application') or @@ -779,7 +779,7 @@ Super should be called at the end of the method @Name='onTerminate' ] /Block/BlockStatement[last()] - [not(Statement/StatementExpression/PrimaryExpression[./PrimaryPrefix[@SuperModifier='true']]/PrimarySuffix[@Image= ancestor::MethodDeclaration/@Name])] + [not(Statement/StatementExpression/PrimaryExpression[./PrimaryPrefix[@SuperModifier= true()]]/PrimarySuffix[@Image= ancestor::MethodDeclaration/@Name])] [ancestor::ClassOrInterfaceDeclaration[ExtendsList/ClassOrInterfaceType[ pmd-java:typeIs('android.app.Activity') or pmd-java:typeIs('android.app.Application') or @@ -892,7 +892,7 @@ Object.clone (which is protected) with a public method." @@ -1013,7 +1013,7 @@ and count(NameList/Name[contains (@Image,'CloneNotSupportedException')]) = 0 ] [ -../../../../ClassOrInterfaceDeclaration[@Final = 'false'] +../../../../ClassOrInterfaceDeclaration[@Final = false()] ] ]]> @@ -1474,7 +1474,7 @@ or reported. /Block /BlockStatement[last()] [not(Statement/StatementExpression/PrimaryExpression - [./PrimaryPrefix[@SuperModifier='true']] + [./PrimaryPrefix[@SuperModifier= true()]] [./PrimarySuffix[@Image='finalize']] ) ] [not(Statement/TryStatement/FinallyStatement /Block/BlockStatement/Statement/StatementExpression/PrimaryExpression - [./PrimaryPrefix[@SuperModifier='true']] + [./PrimaryPrefix[@SuperModifier= true()]] [./PrimarySuffix[@Image='finalize']] ) ] @@ -1924,7 +1924,7 @@ If the finalize() is implemented, it should do something besides just calling su /Block[count(BlockStatement)=1] /BlockStatement[ Statement/StatementExpression/PrimaryExpression - [./PrimaryPrefix[@SuperModifier='true']] + [./PrimaryPrefix[@SuperModifier= true()]] [./PrimarySuffix[@Image='finalize']] ] ]]> @@ -1990,7 +1990,7 @@ Note that Oracle has declared Object.finalize() as deprecated since JDK 9. @@ -2199,7 +2199,7 @@ The suite() method in a JUnit test needs to be both public and static. @@ -2333,7 +2333,7 @@ Either the check is useless (the variable will never be "null") or it is incorre public class Foo { void bar() { if (a.equals(baz) && a != null) {} // a could be null, misplaced null check - + if (a != null && a.equals(baz)) {} // correct null check } } @@ -2344,7 +2344,7 @@ public class Foo { public class Foo { void bar() { if (a.equals(baz) || a == null) {} // a could be null, misplaced null check - + if (a == null || a.equals(baz)) {} // correct null check } } @@ -2372,7 +2372,7 @@ may indicate problematic behaviour. Empty cases are ignored as these indicate an + count(BlockStatement//Statement/ReturnStatement) + count(BlockStatement//Statement/ContinueStatement) + count(BlockStatement//Statement/ThrowStatement) - + count(BlockStatement//Statement/IfStatement[@Else='true' and Statement[2][ReturnStatement|ContinueStatement|ThrowStatement]]/Statement[1][ReturnStatement|ContinueStatement|ThrowStatement]) + + count(BlockStatement//Statement/IfStatement[@Else= true() and Statement[2][ReturnStatement|ContinueStatement|ThrowStatement]]/Statement[1][ReturnStatement|ContinueStatement|ThrowStatement]) + count(SwitchLabel[name(following-sibling::node()) = 'SwitchLabel']) + count(SwitchLabel[count(following-sibling::node()) = 0]) < count (SwitchLabel))] @@ -2421,7 +2421,7 @@ chain needs an own serialVersionUID field. See also [Should an abstract class ha @@ -3377,7 +3372,7 @@ To make sure the full stacktrace is printed out, use the logging statement with concat(ancestor::ClassOrInterfaceDeclaration/ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/FieldDeclaration [Type//ClassOrInterfaceType[@Image='Log']] /VariableDeclarator/VariableDeclaratorId/@Image, '.'))]] -[PrimarySuffix/Arguments[@Size='1']] +[PrimarySuffix/Arguments[@Size= 1]] [PrimarySuffix/Arguments//Name/@Image = ancestor::CatchStatement/FormalParameter/VariableDeclaratorId/@Image] ]]> diff --git a/pmd-java/src/main/resources/category/java/multithreading.xml b/pmd-java/src/main/resources/category/java/multithreading.xml index c1ca1d5c2f..35a0d2ec5c 100644 --- a/pmd-java/src/main/resources/category/java/multithreading.xml +++ b/pmd-java/src/main/resources/category/java/multithreading.xml @@ -23,7 +23,7 @@ gets it. 3 - //MethodDeclaration[@Synchronized='true'] + //MethodDeclaration[@Synchronized= true()] @@ -102,7 +102,7 @@ the volatile keyword should not be used for maintenance purpose and portability. 2 - //FieldDeclaration[contains(@Volatile,'true')] + //FieldDeclaration[@Volatile = true()] @@ -379,7 +379,7 @@ one is chosen. The thread chosen is arbitrary; thus its usually safer to call n - \ No newline at end of file + diff --git a/pmd-java/src/main/resources/category/java/performance.xml b/pmd-java/src/main/resources/category/java/performance.xml index 7d0c75b226..7f1b0e4308 100644 --- a/pmd-java/src/main/resources/category/java/performance.xml +++ b/pmd-java/src/main/resources/category/java/performance.xml @@ -850,7 +850,7 @@ You must use new ArrayList<>(Arrays.asList(...)) if that is inconvenient for you (Arrays.asList(...)) if that is inconvenient for you and PrimaryExpression/PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Name [ - @Image = ancestor::MethodDeclaration[1]//LocalVariableDeclaration/VariableDeclarator/VariableDeclaratorId[@ArrayType="true"]/@Image + @Image = ancestor::MethodDeclaration[1]//LocalVariableDeclaration/VariableDeclarator/VariableDeclaratorId[@ArrayType=true()]/@Image or @Image = ancestor::MethodDeclaration[1]//FormalParameter/VariableDeclaratorId/@Image ] diff --git a/pmd-javascript/src/main/resources/category/ecmascript/bestpractices.xml b/pmd-javascript/src/main/resources/category/ecmascript/bestpractices.xml index 52cdac1822..a6ce322d0e 100644 --- a/pmd-javascript/src/main/resources/category/ecmascript/bestpractices.xml +++ b/pmd-javascript/src/main/resources/category/ecmascript/bestpractices.xml @@ -82,7 +82,7 @@ Global variables can lead to side-effects that are hard to debug. diff --git a/pmd-javascript/src/main/resources/category/ecmascript/codestyle.xml b/pmd-javascript/src/main/resources/category/ecmascript/codestyle.xml index 285262c59d..04d4e1abb2 100644 --- a/pmd-javascript/src/main/resources/category/ecmascript/codestyle.xml +++ b/pmd-javascript/src/main/resources/category/ecmascript/codestyle.xml @@ -25,17 +25,17 @@ indicative of the bug where the assignment operator '=' was used instead of the @@ -110,7 +110,7 @@ Avoid using if..else statements without using curly braces. @@ -149,7 +149,7 @@ Avoid using if statements without using curly braces. diff --git a/pmd-javascript/src/main/resources/category/ecmascript/errorprone.xml b/pmd-javascript/src/main/resources/category/ecmascript/errorprone.xml index 4b48e85dfe..a39190db2e 100644 --- a/pmd-javascript/src/main/resources/category/ecmascript/errorprone.xml +++ b/pmd-javascript/src/main/resources/category/ecmascript/errorprone.xml @@ -23,9 +23,9 @@ This rule helps improve code portability due to differences in browser treatment @@ -62,7 +62,7 @@ same type. The === operator avoids the casting. Date: Thu, 19 Mar 2020 01:49:55 +0100 Subject: [PATCH 02/64] Add relevant property tag --- .../resources/category/apex/codestyle.xml | 5 ++ .../resources/category/apex/errorprone.xml | 6 ++ .../sourceforge/pmd/lang/rule/XPathRule.java | 4 +- .../net/sourceforge/pmd/xml/j2ee.xml | 9 +++ .../test/resources/rulesets/dummy/basic.xml | 4 +- .../resources/rulesets/ruledoctest/sample.xml | 3 + .../rulesets/ruledoctest/sample2.xml | 1 + .../resources/category/java/bestpractices.xml | 22 +++++++ .../resources/category/java/codestyle.xml | 32 ++++++++++ .../main/resources/category/java/design.xml | 18 ++++++ .../resources/category/java/documentation.xml | 4 +- .../resources/category/java/errorprone.xml | 58 +++++++++++++++++++ .../category/java/multithreading.xml | 7 +++ .../resources/category/java/performance.xml | 13 +++++ .../pmd/ant/classpathtest/ruleset.xml | 1 + .../category/ecmascript/bestpractices.xml | 4 ++ .../category/ecmascript/codestyle.xml | 9 +++ .../category/ecmascript/errorprone.xml | 3 + .../resources/category/jsp/bestpractices.xml | 4 ++ .../main/resources/category/jsp/design.xml | 3 + .../resources/category/jsp/errorprone.xml | 1 + .../main/resources/category/jsp/security.xml | 1 + .../category/plsql/bestpractices.xml | 1 + .../resources/category/plsql/codestyle.xml | 2 + .../main/resources/category/plsql/design.xml | 1 + .../resources/category/plsql/errorprone.xml | 3 + .../main/resources/rulesets/dummy/basic.xml | 3 +- .../src/main/resources/category/vm/design.xml | 1 + .../resources/category/pom/errorprone.xml | 2 + .../resources/category/xml/errorprone.xml | 1 + .../main/resources/category/xsl/codestyle.xml | 1 + .../resources/category/xsl/performance.xml | 1 + 32 files changed, 223 insertions(+), 5 deletions(-) diff --git a/pmd-apex/src/main/resources/category/apex/codestyle.xml b/pmd-apex/src/main/resources/category/apex/codestyle.xml index 72c22c83ce..e76d6ebb8a 100644 --- a/pmd-apex/src/main/resources/category/apex/codestyle.xml +++ b/pmd-apex/src/main/resources/category/apex/codestyle.xml @@ -45,6 +45,7 @@ from the rest. 3 + 3 + 3 + 1 + 3 + 3 + 3 + 3 + 3 + 3 + 3 + tmp = new HashMap<>(); - tmp.put(XPATH_1_0, XPATH_2_0); - tmp.put(XPATH_1_0_COMPATIBILITY, XPATH_2_0); + tmp.put(XPATH_1_0, XPATH_1_0); + tmp.put(XPATH_1_0_COMPATIBILITY, XPATH_1_0_COMPATIBILITY); tmp.put(XPATH_2_0, XPATH_2_0); XPATH_VERSIONS = Collections.unmodifiableMap(tmp); } diff --git a/pmd-core/src/test/resources/net/sourceforge/pmd/xml/j2ee.xml b/pmd-core/src/test/resources/net/sourceforge/pmd/xml/j2ee.xml index 607967f6fd..17443c62ab 100644 --- a/pmd-core/src/test/resources/net/sourceforge/pmd/xml/j2ee.xml +++ b/pmd-core/src/test/resources/net/sourceforge/pmd/xml/j2ee.xml @@ -20,6 +20,7 @@ 3 + 4 + 4 + 4 + 4 + 4 + 3 + 3 + 3 + 3 + 3 + - \ No newline at end of file + diff --git a/pmd-doc/src/test/resources/rulesets/ruledoctest/sample.xml b/pmd-doc/src/test/resources/rulesets/ruledoctest/sample.xml index 44e2bb76fb..78591da860 100644 --- a/pmd-doc/src/test/resources/rulesets/ruledoctest/sample.xml +++ b/pmd-doc/src/test/resources/rulesets/ruledoctest/sample.xml @@ -57,6 +57,7 @@ Here might be <script>alert('XSS');</script> as well. And "quotes". 3 + 3 + 3 + 3 + 3 + 3 + 3 + 3 + 3 + + //ForInit/LocalVariableDeclaration[count(VariableDeclarator) > $maximumVariables] @@ -587,6 +593,7 @@ through the @RunWith(Suite.class) annotation. 3 + 3 + 3 + 3 + 3 + 4 + 3 + 3 + 3 + 3 + //Type/ReferenceType/ClassOrInterfaceType[@Image='Hashtable'] @@ -1206,6 +1222,7 @@ Consider replacing Vector usages with the newer java.util.ArrayList if expensive 3 + //Type/ReferenceType/ClassOrInterfaceType[@Image='Vector'] @@ -1267,6 +1284,7 @@ will (and by priority) and avoid clogging the Standard out log. 2 + 3 + 3 + 3 + 3 + 3 + 3 + 4 + 3 + 2 + //Name[starts-with(@Image,'System.loadLibrary')] @@ -314,6 +319,7 @@ prefix for these methods. 4 + 3 + 3 + 1 + No need to explicitly extend Object. 4 + 3 + 3 + //ForStatement[not(Statement/Block)] @@ -861,6 +873,7 @@ Names for references to generic values should be limited to a single uppercase l 4 + 3 + 3 + 4 + 4 + 3 + 4 + 3 + 3 + //PackageDeclaration/Name[lower-case(@Image)!=@Image] @@ -1532,6 +1553,7 @@ Remote Interface of a Session EJB should not have a suffix. 4 + 4 + 4 + 3 + 3 + + 3 + stringsWithDiamond = new ArrayList<>(); // using the diamond operat Useless parentheses should be removed. 4 + 3 + 3 + 3 + //WhileStatement[not(Statement/Block)] diff --git a/pmd-java/src/main/resources/category/java/design.xml b/pmd-java/src/main/resources/category/java/design.xml index 4a3dfa92dc..04ac51af08 100644 --- a/pmd-java/src/main/resources/category/java/design.xml +++ b/pmd-java/src/main/resources/category/java/design.xml @@ -23,6 +23,7 @@ protected constructor in order to prevent instantiation than make the class misl 1 + 3 + 3 + 3 + 1 + 1 + 3 + 1 + 3 + 3 + 3 + 3 + 3 + 3 + 3 + 3 + 3 +
+ +
 /home/pmd/source/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java
124   Logger calls should be surrounded by log level guards.
+ +
 /home/pmd/source/pmd-core/src/main/java/net/sourceforge/pmd/benchmark/Benchmarker.java
58   This for loop can be replaced by a foreach loop

 Problems found
/home/pmd/source/pmd-core/src/test/resources/net/sourceforge/pmd/cpd/files/file_with_ISO-8859-1_encoding.java
net.sourceforge.pmd.PMDException: Error while parsing /home/pmd/source/pmd-core/src/test/resources/net/sourceforge/pmd/cpd/files/file_with_ISO-8859-1_encoding.java
+    at net.sourceforge.pmd.SourceCodeProcessor.processSourceCodeWithoutCache(SourceCodeProcessor.java:110)
+    at net.sourceforge.pmd.SourceCodeProcessor.processSourceCode(SourceCodeProcessor.java:89)
+    at net.sourceforge.pmd.SourceCodeProcessor.processSourceCode(SourceCodeProcessor.java:51)
+    at net.sourceforge.pmd.processor.PmdRunnable.call(PmdRunnable.java:78)
+    at net.sourceforge.pmd.processor.PmdRunnable.call(PmdRunnable.java:24)
+    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
+    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
+    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
+    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
+    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
+    at java.base/java.lang.Thread.run(Thread.java:832)
+Caused by: net.sourceforge.pmd.lang.java.ast.ParseException: Encountered " "-" "- "" at line 6, column 30.
+Was expecting one of:
+    "extends" ...
+    "implements" ...
+    "{" ...
+    "<" ...
+    
+    at net.sourceforge.pmd.lang.java.ast.JavaParser.generateParseException(JavaParser.java:12713)
+    at net.sourceforge.pmd.lang.java.ast.JavaParser.jj_consume_token(JavaParser.java:12597)
+    at net.sourceforge.pmd.lang.java.ast.JavaParser.ClassOrInterfaceBody(JavaParser.java:1554)
+    at net.sourceforge.pmd.lang.java.ast.JavaParser.ClassOrInterfaceDeclaration(JavaParser.java:732)
+    at net.sourceforge.pmd.lang.java.ast.JavaParser.TypeDeclaration(JavaParser.java:639)
+    at net.sourceforge.pmd.lang.java.ast.JavaParser.CompilationUnit(JavaParser.java:373)
+    at net.sourceforge.pmd.lang.java.AbstractJavaParser.parse(AbstractJavaParser.java:62)
+    at net.sourceforge.pmd.SourceCodeProcessor.parse(SourceCodeProcessor.java:121)
+    at net.sourceforge.pmd.SourceCodeProcessor.processSource(SourceCodeProcessor.java:185)
+    at net.sourceforge.pmd.SourceCodeProcessor.processSourceCodeWithoutCache(SourceCodeProcessor.java:107)
+    ... 10 more
+
 Configuration problems found
LoosePackageCouplingNo packages or classes specified
diff --git a/docs/report-examples/pmd-report-yahtml/Benchmarker.html b/docs/report-examples/pmd-report-yahtml/Benchmarker.html new file mode 100644 index 0000000000..2e0da807d1 --- /dev/null +++ b/docs/report-examples/pmd-report-yahtml/Benchmarker.html @@ -0,0 +1,15 @@ + + + + + PMD - Benchmarker + + +

Class View

+

Class: Benchmarker

+ + + +
MethodViolation
findBooleanSwitch
Rule:ForLoopCanBeForeach
Description:This for loop can be replaced by a foreach loop
Line:58 and 62
+ + diff --git a/docs/report-examples/pmd-report-yahtml/RuleContext.html b/docs/report-examples/pmd-report-yahtml/RuleContext.html new file mode 100644 index 0000000000..1f401103f4 --- /dev/null +++ b/docs/report-examples/pmd-report-yahtml/RuleContext.html @@ -0,0 +1,15 @@ + + + + + PMD - RuleContext + + +

Class View

+

Class: RuleContext

+ + + +
MethodViolation
setSourceCodeFilename
Rule:GuardLogStatement
Description:Logger calls should be surrounded by log level guards.
Line:124 and 125
+ + diff --git a/docs/report-examples/pmd-report-yahtml/index.html b/docs/report-examples/pmd-report-yahtml/index.html new file mode 100644 index 0000000000..c8a696b998 --- /dev/null +++ b/docs/report-examples/pmd-report-yahtml/index.html @@ -0,0 +1,20 @@ + + + + + PMD + + +

Package View

+ + + + + + + + + +
PackageClass#
Aggregate - 2
net - 2
net.sourceforge - 2
net.sourceforge.pmd - 2
net.sourceforge.pmd RuleContext 1
net.sourceforge.pmd.benchmark - 1
net.sourceforge.pmd.benchmark Benchmarker 1
+ + From 97f35f60d472adabe87b9871bb70fd142991a946 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 11 Apr 2020 19:48:48 +0200 Subject: [PATCH 21/64] [doc] Fix doc for IDEA tool integration (refs #2117) --- docs/pages/pmd/userdocs/tools/tools.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/pages/pmd/userdocs/tools/tools.md b/docs/pages/pmd/userdocs/tools/tools.md index b8427311fe..b3e8d550c9 100644 --- a/docs/pages/pmd/userdocs/tools/tools.md +++ b/docs/pages/pmd/userdocs/tools/tools.md @@ -243,11 +243,11 @@ Here's how to set it up as an "External Tool": * Name: PMD * Description: PMD, good for what ails you. * Menu: Select the "Main menu", "Project views", "Editor menu", and "Search results" checkboxes. - * Program: $JDKPath$\bin\java.exe + * Program: `c:\pmd\bin\pmd.bat` * For the next parameter you'll need to plug in the location of your PMD installation and the rulesets you want to use * Parameters: - `-cp %CLASSPATH%;c:\pmd\lib\pmd-{{pmd.site.version}}.jar;c:\pmd\lib\asm-3.2.jar;c:\pmd\lib\jaxen-1.1.1.jar net.sourceforge.pmd.PMD "$FilePath$" ideaj unusedcode,imports "$Sourcepath$" $FileClass$.method $FileName$` + `-d "$FilePath$" -f ideaj -R rulesets/java/quickstart.xml -P sourcePath="$Sourcepath$" -P classAndMethodName=$FileClass$.method -P fileName=$FileName$` That's pretty much it. Now you can right click on a source directory and select PMD, it'll run recursively on the source files, and the results should From ad1e196173096cd84090abbbf7b8153b5ee23d1e Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 11 Apr 2020 19:49:11 +0200 Subject: [PATCH 22/64] [core] CodeClimateRenderer: correct links to spec --- .../pmd/renderers/CodeClimateRenderer.java | 6 +++--- .../pmd/renderers/CodeClimateRendererTest.java | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/CodeClimateRenderer.java b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/CodeClimateRenderer.java index 4cc5645b92..0463368e8a 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/CodeClimateRenderer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/CodeClimateRenderer.java @@ -33,7 +33,7 @@ public class CodeClimateRenderer extends AbstractIncrementingRenderer { public static final int REMEDIATION_POINTS_DEFAULT = 50000; public static final String[] CODECLIMATE_DEFAULT_CATEGORIES = new String[] {"Style"}; - // Note: required by https://github.com/codeclimate/spec/blob/master/SPEC.md + // Note: required by https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md protected static final String NULL_CHARACTER = "\u0000"; protected static final List INTERNAL_DEV_PROPERTIES = Arrays.asList("version", "xpath"); private static final String PMD_PROPERTIES_URL = getPmdPropertiesURL(); @@ -146,9 +146,9 @@ public class CodeClimateRenderer extends AbstractIncrementingRenderer { private String getBody() { String result = "## " + rule.getName() + "\\n\\n" + "Since: PMD " + rule.getSince() + "\\n\\n" + "Priority: " + rule.getPriority() + "\\n\\n" - + "[Categories](https://github.com/codeclimate/spec/blob/master/SPEC.md#categories): " + + "[Categories](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#categories): " + Arrays.toString(getCategories()).replaceAll("[\\[\\]]", "") + "\\n\\n" - + "[Remediation Points](https://github.com/codeclimate/spec/blob/master/SPEC.md#remediation-points): " + + "[Remediation Points](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#remediation-points): " + getRemediationPoints() + "\\n\\n" + cleaned(rule.getDescription()); if (!rule.getExamples().isEmpty()) { diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java index 28ca250b06..c81b425668 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java @@ -30,8 +30,8 @@ public class CodeClimateRendererTest extends AbstractRendererTest { public String getExpected() { return "{\"type\":\"issue\",\"check_name\":\"Foo\",\"description\":\"blah\"," + "\"content\":{\"body\":\"## Foo\\n\\nSince: PMD null\\n\\nPriority: Low\\n\\n" - + "[Categories](https://github.com/codeclimate/spec/blob/master/SPEC.md#categories): Style\\n\\n" - + "[Remediation Points](https://github.com/codeclimate/spec/blob/master/SPEC.md#remediation-points): 50000\\n\\n" + + "[Categories](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#categories): Style\\n\\n" + + "[Remediation Points](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#remediation-points): 50000\\n\\n" + "desc\\n\\n" + "### [PMD properties](https://pmd.github.io/latest/pmd_devdocs_working_with_properties.html)\\n\\n" + "Name | Value | Description\\n" + "--- | --- | ---\\n" @@ -45,8 +45,8 @@ public class CodeClimateRendererTest extends AbstractRendererTest { public String getExpectedWithProperties() { return "{\"type\":\"issue\",\"check_name\":\"Foo\",\"description\":\"blah\"," + "\"content\":{\"body\":\"## Foo\\n\\nSince: PMD null\\n\\nPriority: Low\\n\\n" - + "[Categories](https://github.com/codeclimate/spec/blob/master/SPEC.md#categories): Style\\n\\n" - + "[Remediation Points](https://github.com/codeclimate/spec/blob/master/SPEC.md#remediation-points): 50000\\n\\n" + + "[Categories](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#categories): Style\\n\\n" + + "[Remediation Points](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#remediation-points): 50000\\n\\n" + "desc\\n\\n" + "### [PMD properties](https://pmd.github.io/latest/pmd_devdocs_working_with_properties.html)\\n\\n" + "Name | Value | Description\\n" + "--- | --- | ---\\n" @@ -67,8 +67,8 @@ public class CodeClimateRendererTest extends AbstractRendererTest { public String getExpectedMultiple() { return "{\"type\":\"issue\",\"check_name\":\"Foo\",\"description\":\"blah\"," + "\"content\":{\"body\":\"## Foo\\n\\nSince: PMD null\\n\\nPriority: Low\\n\\n" - + "[Categories](https://github.com/codeclimate/spec/blob/master/SPEC.md#categories): Style\\n\\n" - + "[Remediation Points](https://github.com/codeclimate/spec/blob/master/SPEC.md#remediation-points): 50000\\n\\n" + + "[Categories](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#categories): Style\\n\\n" + + "[Remediation Points](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#remediation-points): 50000\\n\\n" + "desc\\n\\n" + "### [PMD properties](https://pmd.github.io/latest/pmd_devdocs_working_with_properties.html)\\n\\n" + "Name | Value | Description\\n" + "--- | --- | ---\\n" @@ -77,8 +77,8 @@ public class CodeClimateRendererTest extends AbstractRendererTest { + "\"},\"categories\":[\"Style\"],\"location\":{\"path\":\"" + getSourceCodeFilename() + "\",\"lines\":{\"begin\":1,\"end\":1}},\"severity\":\"info\",\"remediation_points\":50000}" + "\u0000" + PMD.EOL + "{\"type\":\"issue\",\"check_name\":\"Foo\",\"description\":\"blah\"," + "\"content\":{\"body\":\"## Foo\\n\\nSince: PMD null\\n\\nPriority: Low\\n\\n" - + "[Categories](https://github.com/codeclimate/spec/blob/master/SPEC.md#categories): Style\\n\\n" - + "[Remediation Points](https://github.com/codeclimate/spec/blob/master/SPEC.md#remediation-points): 50000\\n\\n" + + "[Categories](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#categories): Style\\n\\n" + + "[Remediation Points](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#remediation-points): 50000\\n\\n" + "desc\\n\\n" + "### [PMD properties](https://pmd.github.io/latest/pmd_devdocs_working_with_properties.html)\\n\\n" + "Name | Value | Description\\n" + "--- | --- | ---\\n" From a007b0b0cc9b8128ca2eebf08d143d89f9c376e4 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 11 Apr 2020 21:22:35 +0200 Subject: [PATCH 23/64] Fix dead links --- docs/pages/pmd/userdocs/cli_reference.md | 2 +- .../java/net/sourceforge/pmd/docs/DeadLinksChecker.java | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/pages/pmd/userdocs/cli_reference.md b/docs/pages/pmd/userdocs/cli_reference.md index 25fb1da3b0..a43c809081 100644 --- a/docs/pages/pmd/userdocs/cli_reference.md +++ b/docs/pages/pmd/userdocs/cli_reference.md @@ -198,5 +198,5 @@ Example: ## Available Report Formats PMD comes with many different renderers. -All formats are described at [PMD Report formats](pmd_userdos_report_formats.html) +All formats are described at [PMD Report formats](pmd_userdocs_report_formats.html) diff --git a/pmd-doc/src/main/java/net/sourceforge/pmd/docs/DeadLinksChecker.java b/pmd-doc/src/main/java/net/sourceforge/pmd/docs/DeadLinksChecker.java index 1ab25d40a6..66d8c45a7e 100644 --- a/pmd-doc/src/main/java/net/sourceforge/pmd/docs/DeadLinksChecker.java +++ b/pmd-doc/src/main/java/net/sourceforge/pmd/docs/DeadLinksChecker.java @@ -177,6 +177,12 @@ public class DeadLinksChecker { } else { linkOk = linkTarget.isEmpty() || htmlPages.contains(linkTarget); } + + // maybe a local file + if (!linkOk) { + Path localResource = docsDirectory.resolve(linkTarget); + linkOk = Files.exists(localResource); + } } if (!linkOk) { From 363f70b4816f25b96d24aa543e9fba0841de2b49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 11 Apr 2020 21:49:43 +0200 Subject: [PATCH 24/64] Update release notes, refs #2411 --- docs/pages/release_notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index a470494bab..e84236e7b9 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -136,6 +136,7 @@ implementations, and their corresponding Parser if it exists (in the same packag * [#2397](https://github.com/pmd/pmd/pull/2397): \[apex] fixed WITH SECURITY_ENFORCED regex to recognise line break characters - [Kieran Black](https://github.com/kieranlblack) * [#2401](https://github.com/pmd/pmd/pull/2401): \[doc] Update DoNotUseThreads rule documentation - [Saikat Sengupta](https://github.com/s4ik4t) * [#2403](https://github.com/pmd/pmd/pull/2403): \[java] #2402 fix false-positives on Primitive Streams - [Bernd Farka](https://github.com/BerndFarkaDyna) +* [#2411](https://github.com/pmd/pmd/pull/2411): \[java] Fix UseAssertEqualsInsteadOfAssertTrue Example - [Moritz Scheve](https://github.com/Blightbuster) {% endtocmaker %} From cc2894eaf47ff90069a5f9328c90c02d5ad8dcc3 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 13 Apr 2020 12:04:56 +0200 Subject: [PATCH 25/64] [doc] Update release notes, refs #2409, fixes #1164 --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 9978d9b8b8..5fac2fa8d1 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -51,6 +51,7 @@ Note that XPath 1.0 support, the default XPath version, is deprecated since PMD * java * [#2378](https://github.com/pmd/pmd/issues/2378): \[java] AbstractJUnitRule has bad performance on large code bases * java-codestyle + * [#1164](https://github.com/pmd/pmd/issues/1164): \[java] ClassNamingConventions suggests to add Util for class containing only static constants * [#1723](https://github.com/pmd/pmd/issues/1723): \[java] UseDiamondOperator false-positive inside lambda * java-design * [#2390](https://github.com/pmd/pmd/issues/2390): \[java] AbstractClassWithoutAnyMethod: missing violation for nested classes @@ -135,6 +136,7 @@ implementations, and their corresponding Parser if it exists (in the same packag * [#2397](https://github.com/pmd/pmd/pull/2397): \[apex] fixed WITH SECURITY_ENFORCED regex to recognise line break characters - [Kieran Black](https://github.com/kieranlblack) * [#2401](https://github.com/pmd/pmd/pull/2401): \[doc] Update DoNotUseThreads rule documentation - [Saikat Sengupta](https://github.com/s4ik4t) * [#2403](https://github.com/pmd/pmd/pull/2403): \[java] #2402 fix false-positives on Primitive Streams - [Bernd Farka](https://github.com/BerndFarkaDyna) +* [#2409](https://github.com/pmd/pmd/pull/2409): \[java] ClassNamingConventions suggests to add Util for class containing only static constants, fixes #1164 - [Binu R J](https://github.com/binu-r) {% endtocmaker %} From 5c31d40ec274ed7b1b19f02960bcd621b2849c69 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 13 Apr 2020 12:16:55 +0200 Subject: [PATCH 26/64] [core] XPathRule: use == for enum comparison --- .../src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java index 18dff8fab3..d2c438a6df 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java @@ -178,7 +178,7 @@ public class XPathRule extends AbstractRule { throw new IllegalStateException("Invalid XPath version, should have been caught by Rule::dysfunctionReason"); } - if (version.equals(XPathVersion.XPATH_1_0)) { + if (version == XPathVersion.XPATH_1_0) { xpathRuleQuery = new JaxenXPathRuleQuery(); } else { xpathRuleQuery = new SaxonXPathRuleQuery(); From 161af4dfd32a53f0f090c5dc6a25c7819f97ebb1 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 13 Apr 2020 12:17:13 +0200 Subject: [PATCH 27/64] [doc] Update release notes with deprecations, see #2407 --- docs/pages/release_notes.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 12d545d173..bd68d803c1 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -64,6 +64,11 @@ Note that XPath 1.0 support, the default XPath version, is deprecated since PMD Those APIs are not intended to be used by clients, and will be hidden or removed with PMD 7.0.0. You can identify them with the `@InternalApi` annotation. You'll also get a deprecation warning. +* {% jdoc core::lang.rule.xpath.AbstractXPathRuleQuery %} +* {% jdoc core::lang.rule.xpath.JaxenXPathRuleQuery %} +* {% jdoc core::lang.rule.xpath.SaxonXPathRuleQuery %} +* {% jdoc core::lang.rule.xpath.XPathRuleQuery %} + ##### In ASTs As part of the changes we'd like to do to AST classes for 7.0.0, we would like to @@ -119,6 +124,9 @@ implementations, and their corresponding Parser if it exists (in the same packag * {% jdoc !!core::lang.TokenManager#setFileName(java.lang.String) %} * {% jdoc !!core::lang.ast.AbstractTokenManager#setFileName(java.lang.String) %} * {% jdoc !!core::lang.ast.AbstractTokenManager#getFileName(java.lang.String) %} +* {% jdoc core::lang.rule.ImmutableLanguage %} +* {% jdoc core::lang.rule.MockRule %} +* Multiple fields, constructors and methods in {% jdoc core::lang.rule.XPathRule %}. See javadoc for details. ### External Contributions From 954bfb28d4b0f19bca4fd41d5522d4a5beed391d Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Wed, 15 Apr 2020 19:37:19 +0200 Subject: [PATCH 28/64] [ci] Update java to 11.0.7+10 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index bcfbfccb5c..2276f63309 100644 --- a/.travis.yml +++ b/.travis.yml @@ -49,7 +49,7 @@ jobs: env: BUILD=publish before_install: - - bash .travis/before_install.sh "11.0.6+10" + - bash .travis/before_install.sh "11.0.7+10" - source ${HOME}/java.env install: true before_script: true From ea532e6d2df36d1bd59c9a5a9c9c8761f9cb1cd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 16 Apr 2020 14:26:42 +0200 Subject: [PATCH 29/64] Rename RecordDeclaration::getComponentList to getRecordComponents This name is clearer when seen from the interface ASTAnyTypeDeclaration, also it matches the Class API --- docs/pages/release_notes.md | 1 + .../pmd/lang/java/ast/ASTRecordDeclaration.java | 11 +++++++++++ .../pmd/lang/java/ast/AbstractAnyTypeDeclaration.java | 10 ++++++++++ 3 files changed, 22 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 311abf5f12..794293b75c 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -132,6 +132,7 @@ implementations, and their corresponding Parser if it exists (in the same packag * {% jdoc !!core::cpd.token.AntlrToken#getType() %} - use `getKind()` instead. * {% jdoc core::lang.rule.ImmutableLanguage %} * {% jdoc core::lang.rule.MockRule %} +* {% jdoc !!java::lang.java.ast.ASTRecordDeclaration#getComponentList() %} * Multiple fields, constructors and methods in {% jdoc core::lang.rule.XPathRule %}. See javadoc for details. ### External Contributions diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTRecordDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTRecordDeclaration.java index 553190dd74..925425f8f5 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTRecordDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTRecordDeclaration.java @@ -57,7 +57,18 @@ public final class ASTRecordDeclaration extends AbstractAnyTypeDeclaration { return isNested(); } + /** + * @deprecated Renamed to {@link #getRecordComponents()} + */ + @Deprecated public ASTRecordComponentList getComponentList() { + return getRecordComponents(); + } + + /** Returns the record component list. */ + // @NonNull + @Override + public ASTRecordComponentList getRecordComponents() { return getFirstChildOfType(ASTRecordComponentList.class); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractAnyTypeDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractAnyTypeDeclaration.java index fbdacd93bc..4bd5fc0607 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractAnyTypeDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractAnyTypeDeclaration.java @@ -53,6 +53,16 @@ public abstract class AbstractAnyTypeDeclaration extends AbstractJavaAccessTypeN return getImage(); } + + /** + * Returns the record component list, or null if this is not a record + * declaration. + */ + // @Nullable // TODO pull up to ASTAnyTypeDecl on 7.0.x + public ASTRecordComponentList getRecordComponents() { + return getFirstChildOfType(ASTRecordComponentList.class); + } + /** * Returns true if the enclosing type of this type declaration * is any of the given kinds. If this declaration is a top-level From 04b5372f237b48a21809fb89b1ecb419c56dd91a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 16 Apr 2020 17:16:29 +0200 Subject: [PATCH 30/64] Fix issue with Rhino localization --- .../lang/ecmascript/ast/EcmascriptTreeBuilder.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java b/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java index e60282d493..cb44494be3 100644 --- a/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java +++ b/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java @@ -11,6 +11,7 @@ import java.util.List; import java.util.Map; import java.util.Stack; +import org.mozilla.javascript.ScriptRuntime; import org.mozilla.javascript.ast.ArrayComprehension; import org.mozilla.javascript.ast.ArrayComprehensionLoop; import org.mozilla.javascript.ast.ArrayLiteral; @@ -71,6 +72,7 @@ import net.sourceforge.pmd.lang.ast.SourceCodePositioner; public final class EcmascriptTreeBuilder implements NodeVisitor { private static final Map, Constructor>> NODE_TYPE_TO_NODE_ADAPTER_TYPE = new HashMap<>(); + private static String trailingCommaLocalizedMessage = null; static { register(ArrayComprehension.class, ASTArrayComprehension.class); @@ -221,12 +223,19 @@ public final class EcmascriptTreeBuilder implements NodeVisitor { int nodeStart = node.getNode().getAbsolutePosition(); int nodeEnd = nodeStart + node.getNode().getLength() - 1; for (ParseProblem parseProblem : parseProblems) { + + if (trailingCommaLocalizedMessage == null) { + // This will fetch the localized message, at most once, and only if there are + // some problems (so we avoid parsing the message bundle for nothing) + // See https://github.com/pmd/pmd/issues/384 + trailingCommaLocalizedMessage = ScriptRuntime.getMessage0("msg.extra.trailing.comma"); + } + // The node overlaps the comma (i.e. end of the problem)? int problemStart = parseProblem.getFileOffset(); int commaPosition = problemStart + parseProblem.getLength() - 1; if (nodeStart <= commaPosition && commaPosition <= nodeEnd) { - if ("Trailing comma is not legal in an ECMA-262 object initializer" - .equals(parseProblem.getMessage())) { + if (trailingCommaLocalizedMessage.equals(parseProblem.getMessage())) { // Report on the shortest code block containing the // problem (i.e. inner most code in nested structures). EcmascriptNode currentNode = (EcmascriptNode) parseProblemToNode.get(parseProblem); From c7172fe78f71d7fcff19dba2fc26e60bb0ad0ce1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 16 Apr 2020 17:56:11 +0200 Subject: [PATCH 31/64] Add tests --- .../ecmascript/ast/EcmascriptTreeBuilder.java | 13 ++--- .../lang/ecmascript/ast/DefaultLocale.java | 56 +++++++++++++++++++ .../ecmascript/ast/TrailingCommaTest.java | 41 ++++++++++++++ 3 files changed, 102 insertions(+), 8 deletions(-) create mode 100644 pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/DefaultLocale.java create mode 100644 pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/TrailingCommaTest.java diff --git a/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java b/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java index cb44494be3..86cd811622 100644 --- a/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java +++ b/pmd-javascript/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java @@ -72,7 +72,6 @@ import net.sourceforge.pmd.lang.ast.SourceCodePositioner; public final class EcmascriptTreeBuilder implements NodeVisitor { private static final Map, Constructor>> NODE_TYPE_TO_NODE_ADAPTER_TYPE = new HashMap<>(); - private static String trailingCommaLocalizedMessage = null; static { register(ArrayComprehension.class, ASTArrayComprehension.class); @@ -222,14 +221,12 @@ public final class EcmascriptTreeBuilder implements NodeVisitor { TrailingCommaNode trailingCommaNode = (TrailingCommaNode) node; int nodeStart = node.getNode().getAbsolutePosition(); int nodeEnd = nodeStart + node.getNode().getLength() - 1; - for (ParseProblem parseProblem : parseProblems) { - if (trailingCommaLocalizedMessage == null) { - // This will fetch the localized message, at most once, and only if there are - // some problems (so we avoid parsing the message bundle for nothing) - // See https://github.com/pmd/pmd/issues/384 - trailingCommaLocalizedMessage = ScriptRuntime.getMessage0("msg.extra.trailing.comma"); - } + // This will fetch the localized message + // See https://github.com/pmd/pmd/issues/384 + String trailingCommaLocalizedMessage = ScriptRuntime.getMessage0("msg.extra.trailing.comma"); + + for (ParseProblem parseProblem : parseProblems) { // The node overlaps the comma (i.e. end of the problem)? int problemStart = parseProblem.getFileOffset(); diff --git a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/DefaultLocale.java b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/DefaultLocale.java new file mode 100644 index 0000000000..e0cb18f4a5 --- /dev/null +++ b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/DefaultLocale.java @@ -0,0 +1,56 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.ecmascript.ast; + +import java.util.Locale; + +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +/** + * A JUnit rule to change the system locale during a test. + */ +public class DefaultLocale implements TestRule { + + private boolean statementIsExecuting = false; + private Locale loc = Locale.getDefault(); + + /** Set the locale value (overwrites previously set value). */ + public void set(Locale locale) { + if (statementIsExecuting) { + Locale.setDefault(locale); + } else { + this.loc = locale; + } + } + + @Override + public Statement apply(Statement base, Description description) { + return new EnvironmentVariablesStatement(base); + } + + private class EnvironmentVariablesStatement extends Statement { + + final Statement baseStatement; + + EnvironmentVariablesStatement(Statement baseStatement) { + this.baseStatement = baseStatement; + } + + @Override + public void evaluate() throws Throwable { + Locale prev = Locale.getDefault(); + statementIsExecuting = true; + try { + Locale.setDefault(loc); + baseStatement.evaluate(); + } finally { + statementIsExecuting = false; + Locale.setDefault(prev); + } + } + } +} diff --git a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/TrailingCommaTest.java b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/TrailingCommaTest.java new file mode 100644 index 0000000000..1c5bcc72bb --- /dev/null +++ b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/TrailingCommaTest.java @@ -0,0 +1,41 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.ecmascript.ast; + +import java.util.Locale; + +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; + +public class TrailingCommaTest extends EcmascriptParserTestBase { + + @Rule + public DefaultLocale defaultLocale = new DefaultLocale(); + + + @Test + public void testTrailingComma_DefaultLocale() { + testTrailingComma(); + } + + @Test + public void testTrailingComma_FrFr() { + defaultLocale.set(Locale.FRANCE); + testTrailingComma(); + } + + @Test + public void testTrailingComma_RootLocale() { + defaultLocale.set(Locale.ROOT); + testTrailingComma(); + } + + public void testTrailingComma() { + ASTAstRoot node = js.parse("x = {a : 1, };\n"); + ASTObjectLiteral fn = node.getFirstDescendantOfType(ASTObjectLiteral.class); + Assert.assertTrue(fn.isTrailingComma()); + } +} From fa7b286d31dceecb2af043367b302dd8c6f77899 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 16 Apr 2020 18:17:33 +0200 Subject: [PATCH 32/64] Checkstyle --- .../pmd/lang/ecmascript/ast/TrailingCommaTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/TrailingCommaTest.java b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/TrailingCommaTest.java index 1c5bcc72bb..82318930b2 100644 --- a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/TrailingCommaTest.java +++ b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/TrailingCommaTest.java @@ -17,18 +17,18 @@ public class TrailingCommaTest extends EcmascriptParserTestBase { @Test - public void testTrailingComma_DefaultLocale() { + public void testTrailingCommaDefaultLocale() { testTrailingComma(); } @Test - public void testTrailingComma_FrFr() { + public void testTrailingCommaFrFr() { defaultLocale.set(Locale.FRANCE); testTrailingComma(); } @Test - public void testTrailingComma_RootLocale() { + public void testTrailingCommaRootLocale() { defaultLocale.set(Locale.ROOT); testTrailingComma(); } From feee0b8f9174bbbb52c70acefc88a6df3702d673 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 16 Apr 2020 18:57:10 +0200 Subject: [PATCH 33/64] [java] Replace XPath ProperCloneImplementation with Java Rule --- .../ProperCloneImplementationRule.java | 52 +++++++++++++++++++ .../resources/category/java/errorprone.xml | 17 +----- 2 files changed, 53 insertions(+), 16 deletions(-) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ProperCloneImplementationRule.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ProperCloneImplementationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ProperCloneImplementationRule.java new file mode 100644 index 0000000000..19262a5535 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ProperCloneImplementationRule.java @@ -0,0 +1,52 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + + +package net.sourceforge.pmd.lang.java.rule.errorprone; + +import java.util.List; + +import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; +import net.sourceforge.pmd.lang.java.ast.ASTBlock; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; + +public class ProperCloneImplementationRule extends AbstractJavaRule { + + public ProperCloneImplementationRule() { + addRuleChainVisit(ASTMethodDeclaration.class); + } + + @Override + public Object visit(ASTMethodDeclaration node, Object data) { + if (!"clone".equals(node.getName()) || node.getArity() > 0) { + return data; + } + + ASTBlock block = node.getFirstChildOfType(ASTBlock.class); + if (block == null) { + return data; + } + + String enclosingClassName = node.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class).getSimpleName(); + if (blockHasAllocations(block, enclosingClassName)) { + addViolation(data, node); + } + + return data; + } + + private boolean blockHasAllocations(ASTBlock block, String enclosingClassName) { + List allocations = block.findDescendantsOfType(ASTAllocationExpression.class); + for (ASTAllocationExpression alloc : allocations) { + ASTClassOrInterfaceType type = alloc.getFirstChildOfType(ASTClassOrInterfaceType.class); + if (type.hasImageEqualTo(enclosingClassName)) { + return true; + } + } + return false; + } +} diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index 15c60185ea..3617a84f41 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -2719,27 +2719,12 @@ public class Foo { // perfect, both methods provided language="java" since="1.4" message="Object clone() should be implemented with super.clone()" - class="net.sourceforge.pmd.lang.rule.XPathRule" + class="net.sourceforge.pmd.lang.java.rule.errorprone.ProperCloneImplementationRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#propercloneimplementation"> Object clone() should be implemented with super.clone(). 2 - - - - - - - - Date: Thu, 16 Apr 2020 18:57:36 +0200 Subject: [PATCH 34/64] [java] Simplify AvoidDecimalLiteralsInBigDecimalConstructor with type res --- .../resources/category/java/errorprone.xml | 22 +++------- ...DecimalLiteralsInBigDecimalConstructor.xml | 40 ++++++++++--------- 2 files changed, 28 insertions(+), 34 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index 3617a84f41..e0de33036d 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -288,23 +288,13 @@ exactly equal to 0.1, as one would expect. Therefore, it is generally recommend diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidDecimalLiteralsInBigDecimalConstructor.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidDecimalLiteralsInBigDecimalConstructor.xml index 0c20b4e280..9098d2fb3f 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidDecimalLiteralsInBigDecimalConstructor.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidDecimalLiteralsInBigDecimalConstructor.xml @@ -4,11 +4,11 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd"> - + bad, new BigDecimal(.1) 1 - + ok, new BigDecimal(".1") 0 ok, new BigDecimal(10) 0 - + bad, same as #1 but outside an initializer context 1 #1187 double variable with AvoidDecimalLiteralsInBigDecimalConstructor 8 - 4,8,11,14,18,22,25,28 + 6,10,13,16,20,24,27,30 From 12e843cb47d81390610778185e50a596d3fa242a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 17 Apr 2020 08:36:28 +0200 Subject: [PATCH 35/64] Pool names, optimize getNodeKind NameTest::matches is a very hot spot in saxon. Profiling shows that most of its runtime is spent doing virtual method dispatch on NodeInfo#getNodeKind We optimise this by making a base class with the node kind as a field. Also, we now pool xpath names using saxon's name pool, which reduces name tests to integer comparison instead of string.equals Saxon now spends 80x less time in the name test overall shaving off 30% of execution time of XPath rules. --- .../lang/ast/xpath/saxon/AttributeNode.java | 24 ++------ .../lang/ast/xpath/saxon/BaseNodeInfo.java | 59 +++++++++++++++++++ .../lang/ast/xpath/saxon/DocumentNode.java | 21 ++++--- .../pmd/lang/ast/xpath/saxon/ElementNode.java | 36 +++++------ .../lang/rule/xpath/SaxonXPathRuleQuery.java | 12 +++- 5 files changed, 103 insertions(+), 49 deletions(-) create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/BaseNodeInfo.java diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java index 5bea99b879..79dc9273cf 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java @@ -23,9 +23,8 @@ import net.sf.saxon.value.Value; */ @Deprecated @InternalApi -public class AttributeNode extends AbstractNodeInfo { +public class AttributeNode extends BaseNodeInfo { - private final ElementNode parent; protected final Attribute attribute; protected final int id; protected Value value; @@ -34,35 +33,20 @@ public class AttributeNode extends AbstractNodeInfo { /** * Creates a new AttributeNode from a PMD Attribute. * - * @param parent - * @param id The index within the attribute order + * @param parent Parent elemtn + * @param id The index within the attribute order */ public AttributeNode(ElementNode parent, Attribute attribute, int id) { - this.parent = parent; + super(Type.ATTRIBUTE, parent.getNamePool(), attribute.getName(), parent); this.attribute = attribute; this.id = id; } - @Override - public int getNodeKind() { - return Type.ATTRIBUTE; - } - @Override public String getLocalPart() { return attribute.getName(); } - @Override - public String getURI() { - return ""; - } - - @Override - public ElementNode getParent() { - return parent; - } - @Override public Value atomize() { if (value == null) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/BaseNodeInfo.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/BaseNodeInfo.java new file mode 100644 index 0000000000..a460cfb6d7 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/BaseNodeInfo.java @@ -0,0 +1,59 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.ast.xpath.saxon; + + +import net.sf.saxon.om.FingerprintedNode; +import net.sf.saxon.om.NamePool; +import net.sf.saxon.om.NodeInfo; +import net.sf.saxon.om.SiblingCountingNode; +import net.sf.saxon.om.VirtualNode; + +abstract class BaseNodeInfo extends AbstractNodeInfo implements VirtualNode, SiblingCountingNode, FingerprintedNode { + + private final int nodeKind; + private final NamePool namePool; + private final int fingerprint; + + private final ElementNode parent; + + BaseNodeInfo(int nodeKind, NamePool namePool, String localName, ElementNode parent) { + this.nodeKind = nodeKind; + this.namePool = namePool; + this.fingerprint = namePool.allocate("", "", localName); + this.parent = parent; + } + + @Override + public final String getURI() { + return ""; + } + + @Override + public final String getBaseURI() { + return ""; + } + + @Override + public final NodeInfo getParent() { + return parent; + } + + @Override + public final int getFingerprint() { + return fingerprint; + } + + @Override + public final NamePool getNamePool() { + return namePool; + } + + @Override + public final int getNodeKind() { + return nodeKind; + } + +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/DocumentNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/DocumentNode.java index f91718c816..1a9213c826 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/DocumentNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/DocumentNode.java @@ -10,10 +10,12 @@ import java.util.Map; import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery; import net.sf.saxon.om.Axis; import net.sf.saxon.om.AxisIterator; import net.sf.saxon.om.DocumentInfo; +import net.sf.saxon.om.NamePool; import net.sf.saxon.om.Navigator; import net.sf.saxon.om.NodeInfo; import net.sf.saxon.om.SingleNodeIterator; @@ -24,7 +26,7 @@ import net.sf.saxon.type.Type; */ @Deprecated @InternalApi -public class DocumentNode extends AbstractNodeInfo implements DocumentInfo { +public class DocumentNode extends BaseNodeInfo implements DocumentInfo { /** * The root ElementNode of the DocumentNode. @@ -40,13 +42,19 @@ public class DocumentNode extends AbstractNodeInfo implements DocumentInfo { * Construct a DocumentNode, with the given AST Node serving as the root * ElementNode. * - * @param node - * The root AST Node. + * @param node The root AST Node. + * @param namePool Pool to share names * * @see ElementNode */ + public DocumentNode(Node node, NamePool namePool) { + super(Type.DOCUMENT, namePool, "", null); + this.rootNode = new ElementNode(this, new IdGenerator(), null, node, -1, namePool); + } + + @Deprecated public DocumentNode(Node node) { - this.rootNode = new ElementNode(this, new IdGenerator(), null, node, -1); + this(node, SaxonXPathRuleQuery.getNamePool()); } @Override @@ -64,11 +72,6 @@ public class DocumentNode extends AbstractNodeInfo implements DocumentInfo { throw createUnsupportedOperationException("DocumentInfo.selectID(String)"); } - @Override - public int getNodeKind() { - return Type.DOCUMENT; - } - @Override public DocumentInfo getDocumentRoot() { return this; diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java index ef600ed7ab..4e0c1ec698 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java @@ -6,11 +6,13 @@ package net.sourceforge.pmd.lang.ast.xpath.saxon; import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery; import net.sf.saxon.om.Axis; import net.sf.saxon.om.AxisIterator; import net.sf.saxon.om.DocumentInfo; import net.sf.saxon.om.EmptyIterator; +import net.sf.saxon.om.NamePool; import net.sf.saxon.om.Navigator; import net.sf.saxon.om.NodeArrayIterator; import net.sf.saxon.om.NodeInfo; @@ -28,7 +30,7 @@ import net.sf.saxon.value.Value; */ @Deprecated @InternalApi -public class ElementNode extends AbstractNodeInfo { +public class ElementNode extends BaseNodeInfo { protected final DocumentNode document; protected final ElementNode parent; @@ -37,17 +39,29 @@ public class ElementNode extends AbstractNodeInfo { protected final int siblingPosition; protected final NodeInfo[] children; - public ElementNode(DocumentNode document, IdGenerator idGenerator, ElementNode parent, Node node, - int siblingPosition) { + @Deprecated + public ElementNode(DocumentNode document, IdGenerator idGenerator, ElementNode parent, Node node, int siblingPosition) { + this(document, idGenerator, parent, node, siblingPosition, SaxonXPathRuleQuery.getNamePool()); + } + + public ElementNode(DocumentNode document, + IdGenerator idGenerator, + ElementNode parent, + Node node, + int siblingPosition, + NamePool namePool) { + super(Type.ELEMENT, namePool, node.getXPathNodeName(), parent); + this.document = document; this.parent = parent; this.node = node; this.id = idGenerator.getNextId(); this.siblingPosition = siblingPosition; + if (node.getNumChildren() > 0) { this.children = new NodeInfo[node.getNumChildren()]; for (int i = 0; i < children.length; i++) { - children[i] = new ElementNode(document, idGenerator, this, node.getChild(i), i); + children[i] = new ElementNode(document, idGenerator, this, node.getChild(i), i, namePool); } } else { this.children = null; @@ -80,11 +94,6 @@ public class ElementNode extends AbstractNodeInfo { return children != null; } - @Override - public int getNodeKind() { - return Type.ELEMENT; - } - @Override public DocumentInfo getDocumentRoot() { return document; @@ -95,15 +104,6 @@ public class ElementNode extends AbstractNodeInfo { return node.getXPathNodeName(); } - @Override - public String getURI() { - return ""; - } - - @Override - public NodeInfo getParent() { - return parent; - } @Override public SequenceIterator getTypedValue() { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java index 231bd0a917..856c4ed794 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java @@ -26,6 +26,7 @@ import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sf.saxon.expr.Expression; import net.sf.saxon.om.Item; +import net.sf.saxon.om.NamePool; import net.sf.saxon.om.NamespaceConstant; import net.sf.saxon.om.SequenceIterator; import net.sf.saxon.om.ValueRepresentation; @@ -57,6 +58,7 @@ import net.sf.saxon.value.Value; @Deprecated @InternalApi public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { + /** * Special nodeName that references the root expression. */ @@ -64,6 +66,8 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { private static final Logger LOG = Logger.getLogger(SaxonXPathRuleQuery.class.getName()); + private static final NamePool NAME_POOL = NamePool.getDefaultNamePool(); + private static final int MAX_CACHE_SIZE = 20; private static final Map CACHE = new LinkedHashMap(MAX_CACHE_SIZE) { private static final long serialVersionUID = -7653916493967142443L; @@ -97,7 +101,6 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { } @Override - @SuppressWarnings("unchecked") public List evaluate(final Node node, final RuleContext data) { initializeXPathExpression(); @@ -191,7 +194,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { synchronized (CACHE) { documentNode = CACHE.get(root); if (documentNode == null) { - documentNode = new DocumentNode(root); + documentNode = new DocumentNode(root, NAME_POOL); CACHE.put(root, documentNode); } } @@ -229,6 +232,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { try { final XPathEvaluator xpathEvaluator = new XPathEvaluator(); final XPathStaticContext xpathStaticContext = xpathEvaluator.getStaticContext(); + xpathStaticContext.getConfiguration().setNamePool(NAME_POOL); // Enable XPath 1.0 compatibility if (XPATH_1_0_COMPATIBILITY.equals(version)) { @@ -354,4 +358,8 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { initializeXPathExpression(); return super.getRuleChainVisits(); } + + public static NamePool getNamePool() { + return NAME_POOL; + } } From dde5b29da8f62bc4b2047bd1c4e5b582289441cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 17 Apr 2020 09:02:19 +0200 Subject: [PATCH 36/64] Optimise attribute fetch Use a map to fetch the attribute, also don't recreate attribute nodes so much, so that the cache behind Method::invoke in Attribute amounts to something. --- .../ast/xpath/saxon/AbstractNodeInfo.java | 10 +-- .../lang/ast/xpath/saxon/BaseNodeInfo.java | 21 +++++- .../pmd/lang/ast/xpath/saxon/ElementNode.java | 66 ++++++++++++++++++- 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AbstractNodeInfo.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AbstractNodeInfo.java index 1e7c8e0825..822bb9d797 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AbstractNodeInfo.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AbstractNodeInfo.java @@ -260,11 +260,11 @@ public class AbstractNodeInfo implements VirtualNode, SiblingCountingNode { */ @Override public AxisIterator iterateAxis(byte axisNumber, NodeTest nodeTest) { - AxisIterator axisIterator = iterateAxis(axisNumber); - if (nodeTest != null) { - axisIterator = new AxisFilter(axisIterator, nodeTest); - } - return axisIterator; + return filter(iterateAxis(axisNumber), nodeTest); + } + + protected static AxisIterator filter(AxisIterator axisIterator, NodeTest nodeTest) { + return nodeTest != null ? new AxisFilter(axisIterator, nodeTest) : axisIterator; } /** diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/BaseNodeInfo.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/BaseNodeInfo.java index a460cfb6d7..ea6e4477ac 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/BaseNodeInfo.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/BaseNodeInfo.java @@ -13,16 +13,19 @@ import net.sf.saxon.om.VirtualNode; abstract class BaseNodeInfo extends AbstractNodeInfo implements VirtualNode, SiblingCountingNode, FingerprintedNode { + // It's important that all our NodeInfo implementations share the + // same getNodeKind implementation, otherwise NameTest spends a lot + // of time in virtual dispatch private final int nodeKind; private final NamePool namePool; private final int fingerprint; - private final ElementNode parent; + protected final ElementNode parent; BaseNodeInfo(int nodeKind, NamePool namePool, String localName, ElementNode parent) { this.nodeKind = nodeKind; this.namePool = namePool; - this.fingerprint = namePool.allocate("", "", localName); + this.fingerprint = namePool.allocate("", "", localName) & NamePool.FP_MASK; this.parent = parent; } @@ -36,11 +39,25 @@ abstract class BaseNodeInfo extends AbstractNodeInfo implements VirtualNode, Sib return ""; } + @Override + public String getPrefix() { + return ""; + } + @Override public final NodeInfo getParent() { return parent; } + @Override + public int getNameCode() { + // note: the nameCode is only equal to the fingerprint because + // this implementation does not use namespace prefixes + // if we change that (eg for embedded language support) then + // we'll need to worry about this. See NamePool.FP_MASK + return fingerprint; + } + @Override public final int getFingerprint() { return fingerprint; diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java index 4e0c1ec698..146b7f3553 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java @@ -4,8 +4,13 @@ package net.sourceforge.pmd.lang.ast.xpath.saxon; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; + import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.xpath.Attribute; import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery; import net.sf.saxon.om.Axis; @@ -14,11 +19,14 @@ import net.sf.saxon.om.DocumentInfo; import net.sf.saxon.om.EmptyIterator; import net.sf.saxon.om.NamePool; import net.sf.saxon.om.Navigator; +import net.sf.saxon.om.Navigator.BaseEnumeration; import net.sf.saxon.om.NodeArrayIterator; import net.sf.saxon.om.NodeInfo; import net.sf.saxon.om.SequenceIterator; import net.sf.saxon.om.SingleNodeIterator; import net.sf.saxon.om.SingletonIterator; +import net.sf.saxon.pattern.NameTest; +import net.sf.saxon.pattern.NodeTest; import net.sf.saxon.type.Type; import net.sf.saxon.value.AtomicValue; import net.sf.saxon.value.StringValue; @@ -39,6 +47,8 @@ public class ElementNode extends BaseNodeInfo { protected final int siblingPosition; protected final NodeInfo[] children; + private Map attributes; + @Deprecated public ElementNode(DocumentNode document, IdGenerator idGenerator, ElementNode parent, Node node, int siblingPosition) { this(document, idGenerator, parent, node, siblingPosition, SaxonXPathRuleQuery.getNamePool()); @@ -69,6 +79,20 @@ public class ElementNode extends BaseNodeInfo { document.nodeToElementNode.put(node, this); } + private Map getAttributes() { + if (attributes == null) { + attributes = new HashMap<>(); + Iterator iter = node.getXPathAttributesIterator(); + int idx = 0; + while (iter.hasNext()) { + Attribute next = iter.next(); + AttributeNode attrNode = new AttributeNode(this, next, idx++); + attributes.put(attrNode.getFingerprint(), attrNode); + } + } + return attributes; + } + @Override public Object getUnderlyingNode() { return node; @@ -156,16 +180,33 @@ public class ElementNode extends BaseNodeInfo { } + @Override + public AxisIterator iterateAxis(byte axisNumber, NodeTest nodeTest) { + if (axisNumber == Axis.ATTRIBUTE) { + if (nodeTest instanceof NameTest) { + if ((nodeTest.getNodeKindMask() & (1 << Type.ATTRIBUTE)) == 0) { + return EmptyIterator.getInstance(); + } else { + int fp = nodeTest.getFingerprint(); + if (fp != -1) { + return SingleNodeIterator.makeIterator(getAttributes().get(fp)); + } + } + } + } + return super.iterateAxis(axisNumber, nodeTest); + } + @SuppressWarnings("PMD.MissingBreakInSwitch") @Override - public AxisIterator iterateAxis(byte axisNumber) { + public AxisIterator iterateAxis(final byte axisNumber) { switch (axisNumber) { case Axis.ANCESTOR: return new Navigator.AncestorEnumeration(this, false); case Axis.ANCESTOR_OR_SELF: return new Navigator.AncestorEnumeration(this, true); case Axis.ATTRIBUTE: - return new AttributeAxisIterator(this); + return new AttributeEnumeration(); case Axis.CHILD: if (children == null) { return EmptyIterator.getInstance(); @@ -205,4 +246,25 @@ public class ElementNode extends BaseNodeInfo { } } + private class AttributeEnumeration extends BaseEnumeration { + + private final Iterator iter = getAttributes().values().iterator(); + + public AttributeEnumeration() { + } + + @Override + public void advance() { + if (iter.hasNext()) { + current = iter.next(); + } else { + current = null; + } + } + + @Override + public SequenceIterator getAnother() { + return new AttributeEnumeration(); + } + } } From e81fee9ee54d74f7a0c1ace57a5efd32f1bc2e2a Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 17 Apr 2020 11:24:23 +0200 Subject: [PATCH 37/64] [java] Convert PositionLiteralsFirstIn*Comparisons to Java rules --- ...actPositionLiteralsFirstInComparisons.java | 119 ++++++++++++++++++ ...FirstInCaseInsensitiveComparisonsRule.java | 13 ++ ...ositionLiteralsFirstInComparisonsRule.java | 13 ++ .../resources/category/java/bestpractices.xml | 46 +------ .../PositionLiteralsFirstInComparisons.xml | 9 ++ 5 files changed, 156 insertions(+), 44 deletions(-) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractPositionLiteralsFirstInComparisons.java create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInCaseInsensitiveComparisonsRule.java create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInComparisonsRule.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractPositionLiteralsFirstInComparisons.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractPositionLiteralsFirstInComparisons.java new file mode 100644 index 0000000000..3b09462725 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractPositionLiteralsFirstInComparisons.java @@ -0,0 +1,119 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; +import net.sourceforge.pmd.lang.java.ast.ASTArguments; +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.lang.java.ast.ASTExpression; +import net.sourceforge.pmd.lang.java.ast.ASTLiteral; +import net.sourceforge.pmd.lang.java.ast.ASTName; +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.JavaNode; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; + +class AbstractPositionLiteralsFirstInComparisons extends AbstractJavaRule { + + private final String equalsImage; + + AbstractPositionLiteralsFirstInComparisons(String equalsImage) { + addRuleChainVisit(ASTPrimaryExpression.class); + this.equalsImage = equalsImage; + } + + @Override + public Object visit(ASTPrimaryExpression node, Object data) { + ASTPrimaryPrefix primaryPrefix = node.getFirstChildOfType(ASTPrimaryPrefix.class); + ASTPrimarySuffix primarySuffix = node.getFirstChildOfType(ASTPrimarySuffix.class); + if (primaryPrefix != null && primarySuffix != null) { + ASTName name = primaryPrefix.getFirstChildOfType(ASTName.class); + if (name == null || !name.getImage().endsWith(equalsImage)) { + return data; + } + if (!isSingleStringLiteralArgument(primarySuffix)) { + return data; + } + if (isWithinNullComparison(node)) { + return data; + } + addViolation(data, node); + } + return node; + } + + private boolean isWithinNullComparison(ASTPrimaryExpression node) { + for (ASTExpression parentExpr : node.getParentsOfType(ASTExpression.class)) { + if (isComparisonWithNull(parentExpr, "==", ASTConditionalOrExpression.class) + || isComparisonWithNull(parentExpr, "!=", ASTConditionalAndExpression.class)) { + return true; + } + } + return false; + } + + /* + * Expression/ConditionalAndExpression//EqualityExpression(@Image='!=']//NullLiteral + * Expression/ConditionalOrExpression//EqualityExpression(@Image='==']//NullLiteral + */ + private boolean isComparisonWithNull(ASTExpression parentExpr, String equalOperator, Class condition) { + Node condExpr = null; + ASTEqualityExpression eqExpr = null; + if (parentExpr != null) { + condExpr = parentExpr.getFirstChildOfType(condition); + } + if (condExpr != null) { + eqExpr = condExpr.getFirstDescendantOfType(ASTEqualityExpression.class); + } + if (eqExpr != null) { + return eqExpr.hasImageEqualTo(equalOperator) && eqExpr.hasDescendantOfType(ASTNullLiteral.class); + } + return false; + } + + /* + * This corresponds to the following XPath expression: + * (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal[@StringLiteral= true()]) + * and + * ( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 ) + */ + private boolean isSingleStringLiteralArgument(ASTPrimarySuffix primarySuffix) { + if (!primarySuffix.isArguments() || primarySuffix.getArgumentCount() != 1) { + return false; + } + Node node = primarySuffix; + node = node.getFirstChildOfType(ASTArguments.class); + if (node != null) { + node = node.getFirstChildOfType(ASTArgumentList.class); + if (node.getNumChildren() != 1) { + return false; + } + } + if (node != null) { + node = node.getFirstChildOfType(ASTExpression.class); + } + if (node != null) { + node = node.getFirstChildOfType(ASTPrimaryExpression.class); + } + if (node != null) { + node = node.getFirstChildOfType(ASTPrimaryPrefix.class); + } + if (node != null) { + node = node.getFirstChildOfType(ASTLiteral.class); + } + if (node != null) { + ASTLiteral literal = (ASTLiteral) node; + if (literal.isStringLiteral()) { + return true; + } + } + return false; + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInCaseInsensitiveComparisonsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInCaseInsensitiveComparisonsRule.java new file mode 100644 index 0000000000..1c84769c01 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInCaseInsensitiveComparisonsRule.java @@ -0,0 +1,13 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +public class PositionLiteralsFirstInCaseInsensitiveComparisonsRule extends AbstractPositionLiteralsFirstInComparisons { + + public PositionLiteralsFirstInCaseInsensitiveComparisonsRule() { + super(".equalsIgnoreCase"); + } + +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInComparisonsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInComparisonsRule.java new file mode 100644 index 0000000000..2b9edd12c4 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/PositionLiteralsFirstInComparisonsRule.java @@ -0,0 +1,13 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +public class PositionLiteralsFirstInComparisonsRule extends AbstractPositionLiteralsFirstInComparisons { + + public PositionLiteralsFirstInComparisonsRule() { + super(".equals"); + } + +} diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index aa0c9b6115..1087f49b67 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1029,36 +1029,13 @@ String name, language="java" since="5.1" message="Position literals first in String comparisons for EqualsIgnoreCase" - class="net.sourceforge.pmd.lang.rule.XPathRule" + class="net.sourceforge.pmd.lang.java.rule.bestpractices.PositionLiteralsFirstInCaseInsensitiveComparisonsRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#positionliteralsfirstincaseinsensitivecomparisons"> Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false. 3 - - - - - - - - Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false. 3 - - - - - - - - From 797a965c4b34a1e2454078b5b1c97f6a387cd552 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 17 Apr 2020 11:51:06 +0200 Subject: [PATCH 38/64] [java] Convert JUnitSpelling to Java rule --- .../rule/errorprone/JUnitSpellingRule.java | 28 +++++++++++++++++++ .../resources/category/java/errorprone.xml | 27 +----------------- 2 files changed, 29 insertions(+), 26 deletions(-) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitSpellingRule.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitSpellingRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitSpellingRule.java new file mode 100644 index 0000000000..8fd2249133 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitSpellingRule.java @@ -0,0 +1,28 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + + +package net.sourceforge.pmd.lang.java.rule.errorprone; + +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractJUnitRule; + +public class JUnitSpellingRule extends AbstractJUnitRule { + + @Override + public Object visit(ASTMethodDeclaration node, Object data) { + if (node.getArity() != 0) { + return super.visit(node, data); + } + + String name = node.getName(); + if (!"setUp".equals(name) && "setup".equalsIgnoreCase(name)) { + addViolation(data, node); + } + if (!"tearDown".equals(name) && "teardown".equalsIgnoreCase(name)) { + addViolation(data, node); + } + return super.visit(node, data); + } +} diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index e0de33036d..755221f73e 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -2172,37 +2172,12 @@ public class JumbledIncrementerRule1 { language="java" since="1.0" message="You may have misspelled a JUnit framework method (setUp or tearDown)" - class="net.sourceforge.pmd.lang.rule.XPathRule" + class="net.sourceforge.pmd.lang.java.rule.errorprone.JUnitSpellingRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#junitspelling"> Some JUnit framework methods are easy to misspell. 3 - - - - - - - - Date: Fri, 17 Apr 2020 12:01:27 +0200 Subject: [PATCH 39/64] [java] Convert AbstractClassWithoutAbstractMethod to Java rule --- ...bstractClassWithoutAbstractMethodRule.java | 50 +++++++++++++++++++ .../resources/category/java/bestpractices.xml | 16 +----- 2 files changed, 51 insertions(+), 15 deletions(-) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java new file mode 100644 index 0000000000..3f20b559d5 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AbstractClassWithoutAbstractMethodRule.java @@ -0,0 +1,50 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeBodyDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeBodyDeclaration.DeclarationKind; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTExtendsList; +import net.sourceforge.pmd.lang.java.ast.ASTImplementsList; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; + +public class AbstractClassWithoutAbstractMethodRule extends AbstractJavaRule { + + public AbstractClassWithoutAbstractMethodRule() { + addRuleChainVisit(ASTClassOrInterfaceDeclaration.class); + } + + @Override + public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { + if (!node.isAbstract() || doesExtend(node) || doesImplement(node)) { + return data; + } + + int countOfAbstractMethods = 0; + for (ASTAnyTypeBodyDeclaration decl : node.getDeclarations()) { + if (decl.getKind() == DeclarationKind.METHOD) { + ASTMethodDeclaration methodDecl = (ASTMethodDeclaration) decl.getDeclarationNode(); + if (methodDecl.isAbstract()) { + countOfAbstractMethods++; + } + } + } + if (countOfAbstractMethods == 0) { + addViolation(data, node); + } + return data; + } + + private boolean doesExtend(ASTClassOrInterfaceDeclaration node) { + return node.getFirstChildOfType(ASTExtendsList.class) != null; + } + + private boolean doesImplement(ASTClassOrInterfaceDeclaration node) { + return node.getFirstChildOfType(ASTImplementsList.class) != null; + } +} diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 1087f49b67..3264cd40a3 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -13,7 +13,7 @@ Rules which enforce generally accepted best practices. language="java" since="3.0" message="This abstract class does not have any abstract methods" - class="net.sourceforge.pmd.lang.rule.XPathRule" + class="net.sourceforge.pmd.lang.java.rule.bestpractices.AbstractClassWithoutAbstractMethodRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#abstractclasswithoutabstractmethod"> The abstract class does not contain any abstract methods. An abstract class suggests @@ -22,20 +22,6 @@ abstract methods. If the class is intended to be used as a base class only (not directly) a protected constructor can be provided prevent direct instantiation. 3 - - - - - - - - Date: Fri, 17 Apr 2020 14:53:26 +0200 Subject: [PATCH 40/64] [java] UseCorrectExceptionLogging - fix problem with multiple loggers --- .../resources/category/java/errorprone.xml | 16 ++++--- .../xml/UseCorrectExceptionLogging.xml | 44 ++++++++++++------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index 755221f73e..2b4d81319a 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -3380,12 +3380,16 @@ To make sure the full stacktrace is printed out, use the logging statement with diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseCorrectExceptionLogging.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseCorrectExceptionLogging.xml index cc94e27c55..6722f48456 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseCorrectExceptionLogging.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseCorrectExceptionLogging.xml @@ -3,10 +3,9 @@ xmlns="http://pmd.sourceforge.net/rule-tests" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd"> + - + ok 0 + - + failure case - two calls 2 + - + must be in a catch block 0 + - + bug 1626232, the rule should not be confused by inner classes 0 + - + bug 1626232, should work with a static block 1 + + + XPath problem: A sequence of more than one item is not allowed as the first argument of concat() + 0 + + From de875c955a5c030e035e5a44b7919d369666a099 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 17 Apr 2020 15:21:58 +0200 Subject: [PATCH 41/64] [java] Convert JUnitStaticSuite to Java rule --- .../rule/errorprone/JUnitStaticSuiteRule.java | 24 ++++++++++++++++++ .../resources/category/java/errorprone.xml | 25 +------------------ 2 files changed, 25 insertions(+), 24 deletions(-) create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitStaticSuiteRule.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitStaticSuiteRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitStaticSuiteRule.java new file mode 100644 index 0000000000..de0f71ada6 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/JUnitStaticSuiteRule.java @@ -0,0 +1,24 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + + +package net.sourceforge.pmd.lang.java.rule.errorprone; + +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractJUnitRule; + +public class JUnitStaticSuiteRule extends AbstractJUnitRule { + + @Override + public Object visit(ASTMethodDeclaration node, Object data) { + if (node.getArity() != 0) { + return super.visit(node, data); + } + String name = node.getName(); + if ("suite".equals(name) && (!node.isStatic() || !node.isPublic())) { + addViolation(data, node); + } + return super.visit(node, data); + } +} diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index 2b4d81319a..d4268f7b1b 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -2194,36 +2194,13 @@ public class Foo extends TestCase { language="java" since="1.0" message="You have a suite() method that is not both public and static, so JUnit won't call it to get your TestSuite. Is that what you wanted to do?" - class="net.sourceforge.pmd.lang.rule.XPathRule" + class="net.sourceforge.pmd.lang.java.rule.errorprone.JUnitStaticSuiteRule" typeResolution="true" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#junitstaticsuite"> The suite() method in a JUnit test needs to be both public and static. 3 - - - - - - - - Date: Fri, 17 Apr 2020 16:30:37 +0200 Subject: [PATCH 42/64] Checstyle --- .../net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java index 146b7f3553..6db157eac4 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/ElementNode.java @@ -250,9 +250,6 @@ public class ElementNode extends BaseNodeInfo { private final Iterator iter = getAttributes().values().iterator(); - public AttributeEnumeration() { - } - @Override public void advance() { if (iter.hasNext()) { From 83bf4ceefcb474008f58bee4cfb3d1fc89372ac9 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 17 Apr 2020 16:33:11 +0200 Subject: [PATCH 43/64] [doc] Update release notes, fixes #384, refs #2419 --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 794293b75c..320b997bb8 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -59,6 +59,8 @@ Note that XPath 1.0 support, the default XPath version, is deprecated since PMD * [#2402](https://github.com/pmd/pmd/issues/2402): \[java] CloseResource possible false positive with Primitive Streams * java-multithreading * [#2313](https://github.com/pmd/pmd/issues/2313): \[java] Documenation for DoNotUseThreads is outdated +* javascript-errorprone + * [#384](https://github.com/pmd/pmd/issues/384): \[javascript] Trailing commas not detected on French default locale ### API Changes From 2253f38ccb999430b70b4b14c47cd6eb2b1758f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 17 Apr 2020 17:10:16 +0200 Subject: [PATCH 44/64] don't use global namepool to avoid resource contention --- .../pmd/lang/rule/xpath/SaxonXPathRuleQuery.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java index 856c4ed794..54cd8b6681 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java @@ -66,7 +66,14 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { private static final Logger LOG = Logger.getLogger(SaxonXPathRuleQuery.class.getName()); - private static final NamePool NAME_POOL = NamePool.getDefaultNamePool(); + // Name pools are thread local to avoid contention + // (allocation *and* access are synchronized in this version of saxon) + private static final ThreadLocal NAME_POOL = new ThreadLocal() { + @Override + protected NamePool initialValue() { + return new NamePool(); + } + }; private static final int MAX_CACHE_SIZE = 20; private static final Map CACHE = new LinkedHashMap(MAX_CACHE_SIZE) { @@ -194,7 +201,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { synchronized (CACHE) { documentNode = CACHE.get(root); if (documentNode == null) { - documentNode = new DocumentNode(root, NAME_POOL); + documentNode = new DocumentNode(root, getNamePool()); CACHE.put(root, documentNode); } } @@ -232,7 +239,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { try { final XPathEvaluator xpathEvaluator = new XPathEvaluator(); final XPathStaticContext xpathStaticContext = xpathEvaluator.getStaticContext(); - xpathStaticContext.getConfiguration().setNamePool(NAME_POOL); + xpathStaticContext.getConfiguration().setNamePool(getNamePool()); // Enable XPath 1.0 compatibility if (XPATH_1_0_COMPATIBILITY.equals(version)) { @@ -360,6 +367,6 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { } public static NamePool getNamePool() { - return NAME_POOL; + return NAME_POOL.get(); } } From 7a37be55db49865cac52b5c7aedadec50af8dcf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 17 Apr 2020 18:03:57 +0200 Subject: [PATCH 45/64] Revert "don't use global namepool to avoid resource contention" Actually benchmarking with different levels of parallelism shows that there is no contention, and that sharing the namepool is beneficial even with many threads. When the number of threads is very high, using independent namepools increases the amount of work to allocate names. This reverts commit 2253f38ccb999430b70b4b14c47cd6eb2b1758f7. --- .../pmd/lang/rule/xpath/SaxonXPathRuleQuery.java | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java index 54cd8b6681..856c4ed794 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java @@ -66,14 +66,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { private static final Logger LOG = Logger.getLogger(SaxonXPathRuleQuery.class.getName()); - // Name pools are thread local to avoid contention - // (allocation *and* access are synchronized in this version of saxon) - private static final ThreadLocal NAME_POOL = new ThreadLocal() { - @Override - protected NamePool initialValue() { - return new NamePool(); - } - }; + private static final NamePool NAME_POOL = NamePool.getDefaultNamePool(); private static final int MAX_CACHE_SIZE = 20; private static final Map CACHE = new LinkedHashMap(MAX_CACHE_SIZE) { @@ -201,7 +194,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { synchronized (CACHE) { documentNode = CACHE.get(root); if (documentNode == null) { - documentNode = new DocumentNode(root, getNamePool()); + documentNode = new DocumentNode(root, NAME_POOL); CACHE.put(root, documentNode); } } @@ -239,7 +232,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { try { final XPathEvaluator xpathEvaluator = new XPathEvaluator(); final XPathStaticContext xpathStaticContext = xpathEvaluator.getStaticContext(); - xpathStaticContext.getConfiguration().setNamePool(getNamePool()); + xpathStaticContext.getConfiguration().setNamePool(NAME_POOL); // Enable XPath 1.0 compatibility if (XPATH_1_0_COMPATIBILITY.equals(version)) { @@ -367,6 +360,6 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { } public static NamePool getNamePool() { - return NAME_POOL.get(); + return NAME_POOL; } } From 798cce1666da6ec947ecda813f1600ec53f6248e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 17 Apr 2020 19:59:01 +0200 Subject: [PATCH 46/64] Dont use global tree cache --- .../lang/rule/xpath/SaxonXPathRuleQuery.java | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java index 856c4ed794..431fc86a79 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java @@ -7,7 +7,6 @@ package net.sourceforge.pmd.lang.rule.xpath; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -23,6 +22,9 @@ import net.sourceforge.pmd.lang.ast.xpath.saxon.ElementNode; import net.sourceforge.pmd.lang.rule.xpath.internal.RuleChainAnalyzer; import net.sourceforge.pmd.lang.xpath.Initializer; import net.sourceforge.pmd.properties.PropertyDescriptor; +import net.sourceforge.pmd.util.DataMap; +import net.sourceforge.pmd.util.DataMap.DataKey; +import net.sourceforge.pmd.util.DataMap.SimpleDataKey; import net.sf.saxon.expr.Expression; import net.sf.saxon.om.Item; @@ -68,15 +70,9 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { private static final NamePool NAME_POOL = NamePool.getDefaultNamePool(); - private static final int MAX_CACHE_SIZE = 20; - private static final Map CACHE = new LinkedHashMap(MAX_CACHE_SIZE) { - private static final long serialVersionUID = -7653916493967142443L; + /** Cache key for the wrapped tree for saxon. */ + private static final SimpleDataKey SAXON_TREE_CACHE_KEY = DataMap.simpleDataKey("saxon.tree"); - @Override - protected boolean removeEldestEntry(final Map.Entry eldest) { - return size() > MAX_CACHE_SIZE; - } - }; /** * Contains for each nodeName a sub expression, used for implementing rule chain. @@ -190,15 +186,13 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { private DocumentNode getDocumentNodeForRootNode(final Node node) { final Node root = getRootNode(node); - DocumentNode documentNode; - synchronized (CACHE) { - documentNode = CACHE.get(root); - if (documentNode == null) { - documentNode = new DocumentNode(root, NAME_POOL); - CACHE.put(root, documentNode); - } + DataMap> userMap = root.getUserMap(); + DocumentNode docNode = userMap.get(SAXON_TREE_CACHE_KEY); + if (docNode == null) { + docNode = new DocumentNode(root, getNamePool()); + userMap.set(SAXON_TREE_CACHE_KEY, docNode); } - return documentNode; + return docNode; } /** From 48294ae66839810d221a809fd32fb7a0c46b6285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 17 Apr 2020 20:03:39 +0200 Subject: [PATCH 47/64] Cleanup --- .../pmd/lang/rule/xpath/SaxonXPathRuleQuery.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java index 431fc86a79..9e8b976732 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java @@ -68,7 +68,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { private static final Logger LOG = Logger.getLogger(SaxonXPathRuleQuery.class.getName()); - private static final NamePool NAME_POOL = NamePool.getDefaultNamePool(); + private static final NamePool NAME_POOL = new NamePool(); /** Cache key for the wrapped tree for saxon. */ private static final SimpleDataKey SAXON_TREE_CACHE_KEY = DataMap.simpleDataKey("saxon.tree"); @@ -226,14 +226,14 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { try { final XPathEvaluator xpathEvaluator = new XPathEvaluator(); final XPathStaticContext xpathStaticContext = xpathEvaluator.getStaticContext(); - xpathStaticContext.getConfiguration().setNamePool(NAME_POOL); + xpathStaticContext.getConfiguration().setNamePool(getNamePool()); // Enable XPath 1.0 compatibility if (XPATH_1_0_COMPATIBILITY.equals(version)) { ((AbstractStaticContext) xpathStaticContext).setBackwardsCompatibilityMode(true); } - ((IndependentContext) xpathEvaluator.getStaticContext()).declareNamespace("fn", NamespaceConstant.FN); + ((IndependentContext) xpathStaticContext).declareNamespace("fn", NamespaceConstant.FN); // Register PMD functions Initializer.initialize((IndependentContext) xpathStaticContext); From 9dca569cc4efd99bca4165f1d6beb16df5467f05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 17 Apr 2020 00:10:12 +0200 Subject: [PATCH 48/64] Make deprecation warnings for xpath rules mention the name of the rule Fix #2019 --- .../sourceforge/pmd/lang/XPathHandler.java | 3 +- .../pmd/lang/ast/xpath/Attribute.java | 24 +---- .../internal/ContextualizedNavigator.java | 26 ++++++ .../xpath/internal/DeprecatedAttrLogger.java | 87 ++++++++++++++++++ .../lang/ast/xpath/saxon/AttributeNode.java | 13 +++ .../lang/ast/xpath/saxon/DocumentNode.java | 11 +++ .../sourceforge/pmd/lang/rule/XPathRule.java | 17 +++- .../lang/rule/xpath/JaxenXPathRuleQuery.java | 23 ++++- .../lang/rule/xpath/SaxonXPathRuleQuery.java | 15 ++- .../sourceforge/pmd/lang/ast/DummyNode.java | 15 +++ .../ast/xpath/AttributeAxisIteratorTest.java | 34 +------ .../pmd/lang/rule/XPathRuleTest.java | 92 ++++++++++++++++++- 12 files changed, 295 insertions(+), 65 deletions(-) create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/ContextualizedNavigator.java create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/XPathHandler.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/XPathHandler.java index 2895550346..ebdf801d80 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/XPathHandler.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/XPathHandler.java @@ -38,7 +38,8 @@ public interface XPathHandler { * Get a Jaxen Navigator for this Language. May return null if * there is no Jaxen Navigation for this language. * - * @deprecated Support for Jaxen will be removed come 7.0.0 + * @deprecated Support for Jaxen will be removed come 7.0.0. This isn't used + * anymore */ @Deprecated Navigator getNavigator(); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java index cee0734a9c..a1259b816d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java @@ -9,10 +9,6 @@ import java.lang.reflect.Method; import java.util.Collections; import java.util.List; import java.util.Objects; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; -import java.util.logging.Level; -import java.util.logging.Logger; import net.sourceforge.pmd.annotation.Experimental; import net.sourceforge.pmd.lang.ast.Node; @@ -29,11 +25,6 @@ import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttribute; * @author daniels */ public class Attribute { - - - private static final Logger LOG = Logger.getLogger(Attribute.class.getName()); - static final ConcurrentMap DETECTED_DEPRECATED_ATTRIBUTES = new ConcurrentHashMap<>(); - private static final Object[] EMPTY_OBJ_ARRAY = new Object[0]; private final Node parent; @@ -73,9 +64,9 @@ public class Attribute { return method == null ? String.class : method.getReturnType(); } - private boolean isAttributeDeprecated() { + public boolean isAttributeDeprecated() { return method != null && (method.isAnnotationPresent(Deprecated.class) - || method.isAnnotationPresent(DeprecatedAttribute.class)); + || method.isAnnotationPresent(DeprecatedAttribute.class)); } public Object getValue() { @@ -83,12 +74,6 @@ public class Attribute { return value.get(0); } - if (LOG.isLoggable(Level.WARNING) && isAttributeDeprecated() - && DETECTED_DEPRECATED_ATTRIBUTES.putIfAbsent(getLoggableAttributeName(), Boolean.TRUE) == null) { - // this message needs to be kept in sync with PMDCoverageTest / BinaryDistributionIT - LOG.warning("Use of deprecated attribute '" + getLoggableAttributeName() + "' in XPath query"); - } - // this lazy loading reduces calls to Method.invoke() by about 90% try { value = Collections.singletonList(method.invoke(parent, EMPTY_OBJ_ARRAY)); @@ -129,11 +114,6 @@ public class Attribute { return Objects.hash(parent, name); } - - private String getLoggableAttributeName() { - return parent.getXPathNodeName() + "/@" + name; - } - @Override public String toString() { return name + ':' + getValue() + ':' + parent.getXPathNodeName(); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/ContextualizedNavigator.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/ContextualizedNavigator.java new file mode 100644 index 0000000000..f48dc03031 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/ContextualizedNavigator.java @@ -0,0 +1,26 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ +package net.sourceforge.pmd.lang.ast.xpath.internal; + +import net.sourceforge.pmd.lang.ast.xpath.Attribute; +import net.sourceforge.pmd.lang.ast.xpath.DocumentNavigator; + +/** + * Navigator that records attribute usages. + */ +public class ContextualizedNavigator extends DocumentNavigator { + + private final DeprecatedAttrLogger ctx; + + public ContextualizedNavigator(DeprecatedAttrLogger ctx) { + this.ctx = ctx; + } + + @Override + public String getAttributeStringValue(Object arg0) { + Attribute attr = (Attribute) arg0; + ctx.recordUsageOf(attr); + return attr.getStringValue(); + } +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java new file mode 100644 index 0000000000..3dd180f211 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java @@ -0,0 +1,87 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.ast.xpath.internal; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.logging.Level; +import java.util.logging.Logger; + +import net.sourceforge.pmd.lang.ast.xpath.Attribute; +import net.sourceforge.pmd.lang.rule.XPathRule; + +/** + * Records usages of deprecated attributes in XPath rules. This needs + * to be threadsafe, XPath rules have one each (and share it). + */ +public abstract class DeprecatedAttrLogger { + + private static final Logger LOG = Logger.getLogger(Attribute.class.getName()); + + + public abstract void recordUsageOf(Attribute attribute); + + /** + * Create a new context for the given rule, returns a noop implementation + * if the warnings would be ignored anyway. + */ + public static DeprecatedAttrLogger create(XPathRule rule) { + if (LOG.isLoggable(Level.WARNING)) { + return new AttrLoggerImpl(rule); + } else { + return noop(); + } + } + + public static Noop noop() { + return Noop.INSTANCE; + } + + private static String getLoggableAttributeName(Attribute attr) { + return attr.getParent().getXPathNodeName() + "/@" + attr.getName(); + } + + private static class Noop extends DeprecatedAttrLogger { + + static final Noop INSTANCE = new Noop(); + + @Override + public void recordUsageOf(Attribute attribute) { + // do nothing + } + } + + private static class AttrLoggerImpl extends DeprecatedAttrLogger { + + private final ConcurrentMap deprecated = new ConcurrentHashMap<>(); + private final XPathRule rule; + + private AttrLoggerImpl(XPathRule rule) { + this.rule = rule; + } + + @Override + public void recordUsageOf(Attribute attribute) { + if (attribute.isAttributeDeprecated()) { + String name = getLoggableAttributeName(attribute); + Boolean b = deprecated.putIfAbsent(name, Boolean.TRUE); + if (b == null) { + // this message needs to be kept in sync with PMDCoverageTest / BinaryDistributionIT + LOG.warning("Use of deprecated attribute '" + name + "' by rule " + ruleToString()); + } + } + } + + public String ruleToString() { + // we can't compute that beforehand because the name is set + // outside of the rule constructor + String name = rule.getName(); + if (rule.getRuleSetName() != null) { + name = rule.getRuleSetName() + "/" + name; + } + return name; + } + } +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java index 79dc9273cf..9da1b6753c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java @@ -8,6 +8,7 @@ import java.util.List; import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.xpath.Attribute; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttrLogger; import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery; import net.sf.saxon.om.NodeInfo; @@ -47,8 +48,19 @@ public class AttributeNode extends BaseNodeInfo { return attribute.getName(); } + private DeprecatedAttrLogger getAttrCtx() { + return parent == null ? DeprecatedAttrLogger.noop() + : parent.document.getAttrCtx(); + } + + @Override + public ElementNode getParent() { + return parent; + } + @Override public Value atomize() { + getAttrCtx().recordUsageOf(attribute); if (value == null) { Object data = attribute.getValue(); if (data instanceof List) { @@ -72,6 +84,7 @@ public class AttributeNode extends BaseNodeInfo { @Override public int compareOrder(NodeInfo other) { + return Integer.signum(this.id - ((AttributeNode) other).id); } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/DocumentNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/DocumentNode.java index 1a9213c826..d91f1828a6 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/DocumentNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/DocumentNode.java @@ -10,6 +10,7 @@ import java.util.Map; import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttrLogger; import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery; import net.sf.saxon.om.Axis; @@ -38,6 +39,8 @@ public class DocumentNode extends BaseNodeInfo implements DocumentInfo { */ public final Map nodeToElementNode = new HashMap<>(); + private DeprecatedAttrLogger attrCtx; + /** * Construct a DocumentNode, with the given AST Node serving as the root * ElementNode. @@ -95,4 +98,12 @@ public class DocumentNode extends BaseNodeInfo implements DocumentInfo { return super.iterateAxis(axisNumber); } } + + public DeprecatedAttrLogger getAttrCtx() { + return attrCtx == null ? DeprecatedAttrLogger.noop() : attrCtx; + } + + public void setAttrCtx(DeprecatedAttrLogger attrCtx) { + this.attrCtx = attrCtx; + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java index d2c438a6df..0f91715bcc 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/XPathRule.java @@ -16,8 +16,10 @@ import java.util.Objects; import org.apache.commons.lang3.StringUtils; +import net.sourceforge.pmd.Rule; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttrLogger; import net.sourceforge.pmd.lang.rule.xpath.JaxenXPathRuleQuery; import net.sourceforge.pmd.lang.rule.xpath.SaxonXPathRuleQuery; import net.sourceforge.pmd.lang.rule.xpath.XPathRuleQuery; @@ -68,6 +70,9 @@ public class XPathRule extends AbstractRule { */ private XPathRuleQuery xpathRuleQuery; + // this is shared with rules forked by deepCopy, used by the XPathRuleQuery + private DeprecatedAttrLogger attrLogger = DeprecatedAttrLogger.create(this); + /** * Creates a new XPathRule without the corresponding XPath query. * @@ -106,6 +111,14 @@ public class XPathRule extends AbstractRule { setVersion(version.getXmlName()); } + + @Override + public Rule deepCopy() { + XPathRule rule = (XPathRule) super.deepCopy(); + rule.attrLogger = this.attrLogger; + return rule; + } + /** * Returns the version for this rule. Returns null if this is not * set or invalid. @@ -179,9 +192,9 @@ public class XPathRule extends AbstractRule { } if (version == XPathVersion.XPATH_1_0) { - xpathRuleQuery = new JaxenXPathRuleQuery(); + xpathRuleQuery = new JaxenXPathRuleQuery(attrLogger); } else { - xpathRuleQuery = new SaxonXPathRuleQuery(); + xpathRuleQuery = new SaxonXPathRuleQuery(attrLogger); } xpathRuleQuery.setXPath(xpath); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/JaxenXPathRuleQuery.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/JaxenXPathRuleQuery.java index 1e331a7b28..4ef992f19d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/JaxenXPathRuleQuery.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/JaxenXPathRuleQuery.java @@ -33,6 +33,8 @@ import org.jaxen.saxpath.Axis; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.xpath.internal.ContextualizedNavigator; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttrLogger; import net.sourceforge.pmd.properties.PropertyDescriptor; /** @@ -46,15 +48,21 @@ public class JaxenXPathRuleQuery extends AbstractXPathRuleQuery { private static final Logger LOG = Logger.getLogger(JaxenXPathRuleQuery.class.getName()); - private enum InitializationStatus { - NONE, PARTIAL, FULL - } + static final String AST_ROOT = "_AST_ROOT_"; private InitializationStatus initializationStatus = InitializationStatus.NONE; // Mapping from Node name to applicable XPath queries Map> nodeNameToXPaths; - static final String AST_ROOT = "_AST_ROOT_"; + private final DeprecatedAttrLogger attrCtx; + + public JaxenXPathRuleQuery() { + this(DeprecatedAttrLogger.noop()); + } + + public JaxenXPathRuleQuery(DeprecatedAttrLogger attrCtx) { + this.attrCtx = attrCtx; + } @Override public boolean isSupportedVersion(String version) { @@ -66,7 +74,7 @@ public class JaxenXPathRuleQuery extends AbstractXPathRuleQuery { final List results = new ArrayList<>(); try { - initializeExpressionIfStatusIsNoneOrPartial(data.getLanguageVersion().getLanguageVersionHandler().getXPathHandler().getNavigator()); + initializeExpressionIfStatusIsNoneOrPartial(new ContextualizedNavigator(attrCtx)); List xPaths = getXPathsForNodeOrDefault(node.getXPathNodeName()); for (XPath xpath : xPaths) { @@ -266,4 +274,9 @@ public class JaxenXPathRuleQuery extends AbstractXPathRuleQuery { } return xpath; } + + + private enum InitializationStatus { + NONE, PARTIAL, FULL + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java index 9e8b976732..4bcb8f7423 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java @@ -17,6 +17,7 @@ import java.util.regex.Pattern; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttrLogger; import net.sourceforge.pmd.lang.ast.xpath.saxon.DocumentNode; import net.sourceforge.pmd.lang.ast.xpath.saxon.ElementNode; import net.sourceforge.pmd.lang.rule.xpath.internal.RuleChainAnalyzer; @@ -73,7 +74,6 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { /** Cache key for the wrapped tree for saxon. */ private static final SimpleDataKey SAXON_TREE_CACHE_KEY = DataMap.simpleDataKey("saxon.tree"); - /** * Contains for each nodeName a sub expression, used for implementing rule chain. */ @@ -91,6 +91,17 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { */ private List xpathVariables; + private final DeprecatedAttrLogger attrCtx; + + @Deprecated + public SaxonXPathRuleQuery() { + this(DeprecatedAttrLogger.noop()); + } + + public SaxonXPathRuleQuery(DeprecatedAttrLogger attrCtx) { + this.attrCtx = attrCtx; + } + @Override public boolean isSupportedVersion(String version) { return XPATH_1_0_COMPATIBILITY.equals(version) || XPATH_2_0.equals(version); @@ -102,9 +113,11 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { try { final DocumentNode documentNode = getDocumentNodeForRootNode(node); + documentNode.setAttrCtx(attrCtx); // // Map AST Node -> Saxon Node final ElementNode rootElementNode = documentNode.nodeToElementNode.get(node); + assert rootElementNode != null : "Cannot find " + node; final XPathDynamicContext xpathDynamicContext = createDynamicContext(rootElementNode); final List nodes = new LinkedList<>(); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNode.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNode.java index 8cfa55925e..10ddb52a1d 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNode.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNode.java @@ -26,6 +26,21 @@ public class DummyNode extends AbstractNode { this.xpathName = xpathName; } + public void setBeginColumn(int i) { + beginColumn = i; + } + + public void setBeginLine(int i) { + beginLine = i; + } + + public void setCoords(int bline, int bcol, int eline, int ecol) { + beginLine = bline; + beginColumn = bcol; + endLine = eline; + endColumn = ecol; + } + @Override public String toString() { return xpathName; diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java index 1d9e274921..d5697ed3bd 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java @@ -5,7 +5,6 @@ package net.sourceforge.pmd.lang.ast.xpath; -import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -16,15 +15,10 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import org.hamcrest.Matchers; -import org.hamcrest.collection.IsMapContaining; import org.junit.Assert; -import org.junit.Rule; import org.junit.Test; -import net.sourceforge.pmd.junit.JavaUtilLoggingRule; import net.sourceforge.pmd.lang.ast.DummyNode; -import net.sourceforge.pmd.lang.ast.DummyNodeWithDeprecatedAttribute; import net.sourceforge.pmd.lang.ast.Node; @@ -33,32 +27,6 @@ import net.sourceforge.pmd.lang.ast.Node; */ public class AttributeAxisIteratorTest { - @Rule - public JavaUtilLoggingRule loggingRule = new JavaUtilLoggingRule(Attribute.class.getName()); - - /** - * Verifies that attributes are returned, even if they are deprecated. - * Deprecated attributes are still accessible, but a warning is logged, when - * the value is used. - */ - @Test - public void testAttributeDeprecation() { - // make sure, we log - Attribute.DETECTED_DEPRECATED_ATTRIBUTES.clear(); - - Node dummy = new DummyNodeWithDeprecatedAttribute(2); - Map attributes = toMap(new AttributeAxisIterator(dummy)); - assertThat(attributes, IsMapContaining.hasKey("Size")); - assertThat(attributes, IsMapContaining.hasKey("Name")); - - assertThat(attributes.get("Size").getStringValue(), Matchers.is("2")); - assertThat(attributes.get("Name").getStringValue(), Matchers.is("foo")); - - String log = loggingRule.getLog(); - assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Size' in XPath query")); - assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Name' in XPath query")); - } - /** * Test hasNext and next. */ @@ -103,7 +71,7 @@ public class AttributeAxisIteratorTest { assertFalse(atts.containsKey("NodeList")); } - private Map toMap(AttributeAxisIterator it) { + public Map toMap(AttributeAxisIterator it) { Map atts = new HashMap<>(); while (it.hasNext()) { Attribute attribute = it.next(); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java index e9bc85a985..9d906d7708 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java @@ -4,18 +4,37 @@ package net.sourceforge.pmd.lang.rule; +import static java.util.Collections.singletonList; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; + +import org.hamcrest.Matchers; import org.junit.Assert; +import org.junit.Rule; import org.junit.Test; +import net.sourceforge.pmd.RuleContext; +import net.sourceforge.pmd.junit.JavaUtilLoggingRule; +import net.sourceforge.pmd.lang.DummyLanguageModule; +import net.sourceforge.pmd.lang.LanguageRegistry; +import net.sourceforge.pmd.lang.ast.DummyNode; +import net.sourceforge.pmd.lang.ast.DummyNodeWithDeprecatedAttribute; +import net.sourceforge.pmd.lang.ast.xpath.Attribute; +import net.sourceforge.pmd.lang.rule.xpath.XPathVersion; + public class XPathRuleTest { + @Rule + public JavaUtilLoggingRule loggingRule = new JavaUtilLoggingRule(Attribute.class.getName()); + /** * It's easy to forget the attribute "typeResolution=true" when * defining XPath rules in xml. Therefore we by default enable * typeresolution. For Java rules, type resolution was enabled by * default long time ago. * - * @see #2048 [core] Enable type resolution by default for XPath rules + * @see #2048 [core] Enable type resolution by default for XPath + * rules */ @Test public void typeResolutionShouldBeEnabledByDefault() { @@ -26,4 +45,75 @@ public class XPathRuleTest { Assert.assertTrue(rule2.isTypeResolution()); } + + @Test + public void testAttributeDeprecation10() { + testDeprecation(XPathVersion.XPATH_1_0); + } + + @Test + public void testAttributeDeprecation20() { + testDeprecation(XPathVersion.XPATH_2_0); + } + + public void testDeprecation(XPathVersion version) { + XPathRule xpr = makeRule(version, "SomeRule"); + + loggingRule.clear(); + + RuleContext ctx = new RuleContext(); + ctx.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); + DummyNode firstNode = newNode(); + eval(ctx, xpr, firstNode); + assertEquals(1, ctx.getReport().size()); + + String log = loggingRule.getLog(); + assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Size' by rule SomeRule")); + assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Name' by rule SomeRule")); + + + loggingRule.clear(); + + eval(ctx, xpr, newNode()); // with another node + assertEquals(2, ctx.getReport().size()); + + assertEquals("", loggingRule.getLog()); // no additional warnings + + + // with another rule forked from the same one (in multithreaded processor) + eval(ctx, xpr.deepCopy(), newNode()); + assertEquals(3, ctx.getReport().size()); + + assertEquals("", loggingRule.getLog()); // no additional warnings + + // with another rule on the same node, new warnings + XPathRule otherRule = makeRule(version, "OtherRule"); + otherRule.setRuleSetName("rset.xml"); + eval(ctx, otherRule, firstNode); + assertEquals(4, ctx.getReport().size()); + + log = loggingRule.getLog(); + assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Size' by rule rset.xml/OtherRule")); + assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Name' by rule rset.xml/OtherRule")); + + } + + public XPathRule makeRule(XPathVersion version, String name) { + XPathRule xpr = new XPathRule(version, "//dummyNode[@Size >= 2 and @Name='foo']"); + xpr.setName(name); + xpr.setMessage("gotcha"); + return xpr; + } + + public void eval(RuleContext ctx, net.sourceforge.pmd.Rule rule, DummyNode node) { + rule.apply(singletonList(node), ctx); + } + + public DummyNode newNode() { + DummyNode dummy = new DummyNodeWithDeprecatedAttribute(2); + dummy.setCoords(1, 1, 1, 2); + return dummy; + } + + } From be152e92a8a58df7889c7db58b68c62148151942 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 17 Apr 2020 20:18:32 +0200 Subject: [PATCH 49/64] Output replacement as well --- .../pmd/lang/ast/xpath/Attribute.java | 20 +++++++++++++++++++ .../xpath/internal/DeprecatedAttrLogger.java | 9 +++++++-- .../xpath/internal/DeprecatedAttribute.java | 9 +++++++++ .../lang/ast/xpath/saxon/AttributeNode.java | 5 ----- .../lang/java/ast/ASTAdditiveExpression.java | 11 ++++++++++ .../lang/java/ast/ASTAnyTypeDeclaration.java | 2 ++ 6 files changed, 49 insertions(+), 7 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java index a1259b816d..8c9ed3e59d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java @@ -11,6 +11,7 @@ import java.util.List; import java.util.Objects; import net.sourceforge.pmd.annotation.Experimental; +import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttribute; @@ -64,11 +65,30 @@ public class Attribute { return method == null ? String.class : method.getReturnType(); } + @InternalApi public boolean isAttributeDeprecated() { return method != null && (method.isAnnotationPresent(Deprecated.class) || method.isAnnotationPresent(DeprecatedAttribute.class)); } + /** + * Returns null for "not deprecated", empty string for "deprecated for removal", + * otherwise name of replacement attribute. + */ + @InternalApi + public String replacementIfDeprecated() { + if (method == null) { + return null; + } else { + DeprecatedAttribute annot = method.getAnnotation(DeprecatedAttribute.class); + if (annot == null) { + return method.isAnnotationPresent(Deprecated.class) ? DeprecatedAttribute.NO_REPLACEMENT + : null; + } + return annot.replaceWith(); + } + } + public Object getValue() { if (value != null) { return value.get(0); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java index 3dd180f211..22a9a64d4a 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java @@ -64,12 +64,17 @@ public abstract class DeprecatedAttrLogger { @Override public void recordUsageOf(Attribute attribute) { - if (attribute.isAttributeDeprecated()) { + String replacement = attribute.replacementIfDeprecated(); + if (replacement != null) { String name = getLoggableAttributeName(attribute); Boolean b = deprecated.putIfAbsent(name, Boolean.TRUE); if (b == null) { // this message needs to be kept in sync with PMDCoverageTest / BinaryDistributionIT - LOG.warning("Use of deprecated attribute '" + name + "' by rule " + ruleToString()); + String msg = "Use of deprecated attribute '" + name + "' by rule " + ruleToString(); + if (!replacement.isEmpty()) { + msg += ", please use " + replacement + " instead"; + } + LOG.warning(msg); } } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttribute.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttribute.java index 75e80d4f5f..cb443d35ae 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttribute.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttribute.java @@ -22,4 +22,13 @@ import java.lang.annotation.Target; @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) public @interface DeprecatedAttribute { + + String NO_REPLACEMENT = ""; + + + /** + * The simple name of the attribute to use for replacement. + * If empty, then the attribute is deprecated for removal. + */ + String replaceWith() default NO_REPLACEMENT; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java index 9da1b6753c..7a919967ef 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/saxon/AttributeNode.java @@ -53,11 +53,6 @@ public class AttributeNode extends BaseNodeInfo { : parent.document.getAttrCtx(); } - @Override - public ElementNode getParent() { - return parent; - } - @Override public Value atomize() { getAttrCtx().recordUsageOf(attribute); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAdditiveExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAdditiveExpression.java index b2176402ff..9d6c995cb9 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAdditiveExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAdditiveExpression.java @@ -5,6 +5,7 @@ package net.sourceforge.pmd.lang.java.ast; import net.sourceforge.pmd.annotation.InternalApi; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttribute; /** * Represents an addition operation on two or more values, or string concatenation. @@ -40,6 +41,16 @@ public class ASTAdditiveExpression extends AbstractJavaTypeNode { } + /** + * @deprecated Use {@link #getOperator()} + */ + @Override + @Deprecated + @DeprecatedAttribute(replaceWith = "@Operator") + public String getImage() { + return super.getImage(); + } + /** * Returns the image of the operator, i.e. "+" or "-". */ diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnyTypeDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnyTypeDeclaration.java index 2c39b1dbf8..d672a49046 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnyTypeDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnyTypeDeclaration.java @@ -7,6 +7,7 @@ package net.sourceforge.pmd.lang.java.ast; import java.util.List; import java.util.Locale; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttribute; import net.sourceforge.pmd.lang.java.ast.internal.PrettyPrintingUtil; import net.sourceforge.pmd.lang.java.qname.JavaTypeQualifiedName; @@ -32,6 +33,7 @@ public interface ASTAnyTypeDeclaration extends TypeNode, JavaQualifiableNode, Ac * @deprecated Use {@link #getSimpleName()} */ @Deprecated + @DeprecatedAttribute(replaceWith = "@SimpleName") @Override String getImage(); From f3db39641a393f050bc93457266f1e8f60df097a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 17 Apr 2020 20:28:00 +0200 Subject: [PATCH 50/64] Test with ASTVariableDeclaratorId#getName --- .../xpath/internal/DeprecatedAttrLogger.java | 5 ++++- .../xpath/internal/DeprecatedAttribute.java | 2 +- .../lang/java/ast/ASTMethodDeclaration.java | 2 ++ .../lang/java/ast/ASTMethodDeclarator.java | 3 +++ .../java/ast/ASTVariableDeclaratorId.java | 22 ++++++++++++++++++- 5 files changed, 31 insertions(+), 3 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java index 22a9a64d4a..5b7993d78c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java @@ -74,7 +74,10 @@ public abstract class DeprecatedAttrLogger { if (!replacement.isEmpty()) { msg += ", please use " + replacement + " instead"; } - LOG.warning(msg); + // ok this circumvents the logger, because otherwise + // messages get lost in ugly header lines + System.err.println("WARNING: " + msg); + // LOG.warning(msg); } } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttribute.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttribute.java index cb443d35ae..430cf5dd73 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttribute.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttribute.java @@ -27,7 +27,7 @@ public @interface DeprecatedAttribute { /** - * The simple name of the attribute to use for replacement. + * The simple name of the attribute to use for replacement (with '@' prefix). * If empty, then the attribute is deprecated for removal. */ String replaceWith() default NO_REPLACEMENT; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclaration.java index fe39f9db94..3715ac4a7f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclaration.java @@ -6,6 +6,7 @@ package net.sourceforge.pmd.lang.java.ast; import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttribute; import net.sourceforge.pmd.lang.dfa.DFAGraphMethod; @@ -42,6 +43,7 @@ public class ASTMethodDeclaration extends AbstractMethodOrConstructorDeclaration * @deprecated Use {@link #getName()} */ @Deprecated + @DeprecatedAttribute(replaceWith = "@Name") public String getMethodName() { return getName(); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclarator.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclarator.java index c8f17b95ab..be6ad8216d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclarator.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclarator.java @@ -5,6 +5,7 @@ package net.sourceforge.pmd.lang.java.ast; import net.sourceforge.pmd.annotation.InternalApi; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttribute; /** * @deprecated This node will be removed with 7.0.0. You @@ -29,6 +30,7 @@ public class ASTMethodDeclarator extends AbstractJavaNode { /** * @deprecated Use {@link ASTMethodDeclaration#getArity()} */ + @DeprecatedAttribute(replaceWith = "MethodDeclaration/@Arity") @Deprecated public int getParameterCount() { return getFirstChildOfType(ASTFormalParameters.class).size(); @@ -38,6 +40,7 @@ public class ASTMethodDeclarator extends AbstractJavaNode { * @deprecated Use {@link ASTMethodDeclaration#getName()} */ @Deprecated + @DeprecatedAttribute(replaceWith = "MethodDeclaration/@Name") @Override public String getImage() { return super.getImage(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java index a8f04258ab..cde67aa449 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java @@ -9,6 +9,7 @@ import java.util.List; import net.sourceforge.pmd.annotation.Experimental; import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttribute; import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; @@ -98,6 +99,21 @@ public class ASTVariableDeclaratorId extends AbstractJavaTypeNode implements Dim return arrayDepth > 0; } + /** + * @deprecated Use {@link #getName()} + * @return + */ + @Override + @DeprecatedAttribute(replaceWith = "@Name") + @Deprecated + public String getImage() { + return super.getImage(); + } + + /** Returns the name of the variable. */ + public String getName() { + return getImage(); + } /** * Returns true if the declared variable has an array type. @@ -161,9 +177,13 @@ public class ASTVariableDeclaratorId extends AbstractJavaTypeNode implements Dim /** * Returns the name of the variable. + * + * @deprecated Use {@link #getName()} */ + @Deprecated + @DeprecatedAttribute(replaceWith = "@Name") public String getVariableName() { - return getImage(); + return getName(); } From e2ec5450ffe69103fad814bfbed2d16dcad5b081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 17 Apr 2020 20:31:09 +0200 Subject: [PATCH 51/64] Update rules WARNING: Use of deprecated attribute 'VariableDeclaratorId/@Image' by rule Error Prone/AvoidEnumAsIdentifier, please use @Name instead WARNING: Use of deprecated attribute 'VariableDeclaratorId/@Image' by rule Error Prone/AvoidAssertAsIdentifier, please use @Name instead WARNING: Use of deprecated attribute 'VariableDeclaratorId/@Image' by rule Error Prone/ProperLogger, please use @Name instead WARNING: Use of deprecated attribute 'VariableDeclaratorId/@Image' by rule Error Prone/JumbledIncrementer, please use @Name instead WARNING: Use of deprecated attribute 'VariableDeclaratorId/@Image' by rule Error Prone/AvoidLosingExceptionInformation, please use @Name instead WARNING: Use of deprecated attribute 'VariableDeclaratorId/@Image' by rule Error Prone/EmptyCatchBlock, please use @Name instead --- .../resources/category/java/codestyle.xml | 4 ++-- .../resources/category/java/errorprone.xml | 24 +++++++++---------- .../resources/category/java/performance.xml | 6 ++--- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index 57d59a88bd..4de7330e0d 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -1236,7 +1236,7 @@ Fields, formal arguments, or local variable names that are too long can make the $minimum] +//VariableDeclaratorId[string-length(@Name) > $minimum] ]]> @@ -1704,7 +1704,7 @@ Fields, local variables, or parameter names that are very short are not helpful - //VariableDeclaratorId[@Image='assert'] + //VariableDeclaratorId[@Name='assert'] @@ -347,7 +347,7 @@ Use of the term 'enum' will conflict with newer versions of Java since it is a r - //VariableDeclaratorId[@Image='enum'] + //VariableDeclaratorId[@Name='enum'] @@ -546,15 +546,15 @@ only add to code size. Either remove the invocation, or use the return result. @@ -1487,7 +1487,7 @@ or reported. [FormalParameter/Type/ReferenceType /ClassOrInterfaceType[@Image != 'InterruptedException' and @Image != 'CloneNotSupportedException'] ] - [FormalParameter/VariableDeclaratorId[not(matches(@Image, $allowExceptionNameRegex))]] + [FormalParameter/VariableDeclaratorId[not(matches(@Name, $allowExceptionNameRegex))]] ]]> @@ -2147,7 +2147,7 @@ Avoid jumbled loop incrementers - its usually a mistake, and is confusing even i [ ForUpdate/StatementExpressionList/StatementExpression/PostfixExpression/PrimaryExpression/PrimaryPrefix/Name/@Image = - ancestor::ForStatement/ForInit//VariableDeclaratorId/@Image + ancestor::ForStatement/ForInit//VariableDeclaratorId/@Name ] ]]> @@ -2702,8 +2702,8 @@ with the restriction that the logger needs to be passed into the constructor. (: check modifiers :) (@Private = false() or @Final = false()) (: check logger name :) - or (@Static and VariableDeclarator/VariableDeclaratorId[@Image != $staticLoggerName]) - or (@Static = false() and VariableDeclarator/VariableDeclaratorId[@Image != $loggerName]) + or (@Static and VariableDeclarator/VariableDeclaratorId[@Name != $staticLoggerName]) + or (@Static = false() and VariableDeclarator/VariableDeclaratorId[@Name != $loggerName]) (: check logger argument type matches class or enum name :) or .//ArgumentList//ClassOrInterfaceType[@Image != ancestor::ClassOrInterfaceDeclaration/@SimpleName] @@ -2717,7 +2717,7 @@ with the restriction that the logger needs to be passed into the constructor. ancestor::ClassOrInterfaceBody//ConstructorDeclaration//StatementExpression [PrimaryExpression[PrimaryPrefix[@ThisModifier]]/PrimarySuffix[@Image=$loggerName]] [AssignmentOperator[@Image = '=']] - [Expression/PrimaryExpression/PrimaryPrefix/Name[@Image = ancestor::ConstructorDeclaration//FormalParameter/VariableDeclaratorId/@Image]] + [Expression/PrimaryExpression/PrimaryPrefix/Name[@Image = ancestor::ConstructorDeclaration//FormalParameter/VariableDeclaratorId/@Name]] [not(.//AllocationExpression)] ) ] diff --git a/pmd-java/src/main/resources/category/java/performance.xml b/pmd-java/src/main/resources/category/java/performance.xml index 9afd4f220b..8213f85f83 100644 --- a/pmd-java/src/main/resources/category/java/performance.xml +++ b/pmd-java/src/main/resources/category/java/performance.xml @@ -878,14 +878,14 @@ You must use new ArrayList<>(Arrays.asList(...)) if that is inconvenient for you @Image="ArrayList" ] )=1 - ]/@Image + ]/@Name ] and PrimaryExpression/PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Name [ - @Image = ancestor::MethodDeclaration[1]//LocalVariableDeclaration/VariableDeclarator/VariableDeclaratorId[@ArrayType=true()]/@Image + @Image = ancestor::MethodDeclaration[1]//LocalVariableDeclaration/VariableDeclarator/VariableDeclaratorId[@ArrayType=true()]/@Name or - @Image = ancestor::MethodDeclaration[1]//FormalParameter/VariableDeclaratorId/@Image + @Image = ancestor::MethodDeclaration[1]//FormalParameter/VariableDeclaratorId/@Name ] /../..[count(.//PrimarySuffix) =1]/PrimarySuffix/Expression/PrimaryExpression/PrimaryPrefix From be19da5a07557677293b5dc080b19360f62e6672 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 17 Apr 2020 20:38:40 +0200 Subject: [PATCH 52/64] Test in pmd-core --- .../pmd/lang/ast/xpath/Attribute.java | 18 ++++++------------ .../xpath/internal/DeprecatedAttrLogger.java | 11 ++++------- .../ast/DummyNodeWithDeprecatedAttribute.java | 2 +- .../pmd/lang/rule/XPathRuleTest.java | 8 ++++---- 4 files changed, 15 insertions(+), 24 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java index 8c9ed3e59d..7f4573ce97 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/Attribute.java @@ -65,14 +65,8 @@ public class Attribute { return method == null ? String.class : method.getReturnType(); } - @InternalApi - public boolean isAttributeDeprecated() { - return method != null && (method.isAnnotationPresent(Deprecated.class) - || method.isAnnotationPresent(DeprecatedAttribute.class)); - } - /** - * Returns null for "not deprecated", empty string for "deprecated for removal", + * Returns null for "not deprecated", empty string for "deprecated without replacement", * otherwise name of replacement attribute. */ @InternalApi @@ -81,11 +75,11 @@ public class Attribute { return null; } else { DeprecatedAttribute annot = method.getAnnotation(DeprecatedAttribute.class); - if (annot == null) { - return method.isAnnotationPresent(Deprecated.class) ? DeprecatedAttribute.NO_REPLACEMENT - : null; - } - return annot.replaceWith(); + return annot != null + ? annot.replaceWith() + : method.isAnnotationPresent(Deprecated.class) + ? DeprecatedAttribute.NO_REPLACEMENT + : null; } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java index 5b7993d78c..b0501e7a78 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java @@ -70,14 +70,11 @@ public abstract class DeprecatedAttrLogger { Boolean b = deprecated.putIfAbsent(name, Boolean.TRUE); if (b == null) { // this message needs to be kept in sync with PMDCoverageTest / BinaryDistributionIT - String msg = "Use of deprecated attribute '" + name + "' by rule " + ruleToString(); + String msg = "Use of deprecated attribute '" + name + "' by XPath rule " + ruleToString(); if (!replacement.isEmpty()) { msg += ", please use " + replacement + " instead"; } - // ok this circumvents the logger, because otherwise - // messages get lost in ugly header lines - System.err.println("WARNING: " + msg); - // LOG.warning(msg); + LOG.warning(msg); } } } @@ -85,9 +82,9 @@ public abstract class DeprecatedAttrLogger { public String ruleToString() { // we can't compute that beforehand because the name is set // outside of the rule constructor - String name = rule.getName(); + String name = "'" + rule.getName() + "'"; if (rule.getRuleSetName() != null) { - name = rule.getRuleSetName() + "/" + name; + name += " (in ruleset '" + rule.getRuleSetName() + "')"; } return name; } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNodeWithDeprecatedAttribute.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNodeWithDeprecatedAttribute.java index 7deca9984b..0e5dd20606 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNodeWithDeprecatedAttribute.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNodeWithDeprecatedAttribute.java @@ -25,7 +25,7 @@ public class DummyNodeWithDeprecatedAttribute extends DummyNode { // this is a attribute that is deprecated for xpath, because it will be removed. // it should still be available via Java. - @DeprecatedAttribute + @DeprecatedAttribute(replaceWith = "@Image") public String getName() { return "foo"; } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java index 9d906d7708..07605202d5 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java @@ -68,8 +68,8 @@ public class XPathRuleTest { assertEquals(1, ctx.getReport().size()); String log = loggingRule.getLog(); - assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Size' by rule SomeRule")); - assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Name' by rule SomeRule")); + assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Size' by XPath rule 'SomeRule'")); + assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Name' by XPath rule 'SomeRule', please use @Image instead")); loggingRule.clear(); @@ -93,8 +93,8 @@ public class XPathRuleTest { assertEquals(4, ctx.getReport().size()); log = loggingRule.getLog(); - assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Size' by rule rset.xml/OtherRule")); - assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Name' by rule rset.xml/OtherRule")); + assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Size' by XPath rule 'OtherRule' (in ruleset 'rset.xml')")); + assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Name' by XPath rule 'OtherRule' (in ruleset 'rset.xml'), please use @Image instead")); } From 9c98508fec572dd8b489a622a27ef1b34ad71a3a Mon Sep 17 00:00:00 2001 From: Harsh Kukreja Date: Sat, 18 Apr 2020 02:34:14 +0530 Subject: [PATCH 53/64] Operator Wrap Issue Solved --- .../java/net/sourceforge/pmd/cpd/AbstractTokenizer.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/AbstractTokenizer.java b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/AbstractTokenizer.java index 87c5acb924..66870ef1ef 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/AbstractTokenizer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/AbstractTokenizer.java @@ -117,14 +117,14 @@ public abstract class AbstractTokenizer implements Tokenizer { loc++; } // Handling multiple lines string - if (!done && // ... we didn't find the end of the string - loc >= currentLine.length() && // ... we have reach the end of + if (!done // ... we didn't find the end of the string + && loc >= currentLine.length() // ... we have reach the end of // the line ( the String is // incomplete, for the moment at // least) - spanMultipleLinesString && // ... the language allow multiple + && spanMultipleLinesString // ... the language allow multiple // line span Strings - lineNumber < code.size() - 1 // ... there is still more lines to + && lineNumber < code.size() - 1 // ... there is still more lines to // parse ) { // removes last character, if it is the line continuation (e.g. From 9caadee7640e9dbff25af5d0008f1d7b3b589f6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 18 Apr 2020 01:53:44 +0200 Subject: [PATCH 54/64] Update release notes, refs #2423 --- docs/pages/release_notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 320b997bb8..5f1a7eebc0 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -150,6 +150,7 @@ implementations, and their corresponding Parser if it exists (in the same packag * [#2403](https://github.com/pmd/pmd/pull/2403): \[java] #2402 fix false-positives on Primitive Streams - [Bernd Farka](https://github.com/BerndFarkaDyna) * [#2409](https://github.com/pmd/pmd/pull/2409): \[java] ClassNamingConventions suggests to add Util for class containing only static constants, fixes #1164 - [Binu R J](https://github.com/binu-r) * [#2411](https://github.com/pmd/pmd/pull/2411): \[java] Fix UseAssertEqualsInsteadOfAssertTrue Example - [Moritz Scheve](https://github.com/Blightbuster) +* [#2423](https://github.com/pmd/pmd/pull/2423): \[core] Fix Checkstyle OperatorWrap in AbstractTokenizer - [Harsh Kukreja](https://github.com/harsh-kukreja) {% endtocmaker %} From 4db656a6795b292199f2f4ad35825c4df1f6800e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 18 Apr 2020 01:56:10 +0200 Subject: [PATCH 55/64] Cleanup --- .../pmd/cpd/AbstractTokenizer.java | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/AbstractTokenizer.java b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/AbstractTokenizer.java index 66870ef1ef..a79aee80d4 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/AbstractTokenizer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/AbstractTokenizer.java @@ -99,7 +99,7 @@ public abstract class AbstractTokenizer implements Tokenizer { private int parseString(StringBuilder token, int loc, char stringDelimiter) { boolean escaped = false; boolean done = false; - char tok = ' '; // this will be replaced. + char tok; while (loc < currentLine.length() && !done) { tok = currentLine.charAt(loc); if (escaped && tok == stringDelimiter) { // Found an escaped string @@ -107,30 +107,24 @@ public abstract class AbstractTokenizer implements Tokenizer { } else if (tok == stringDelimiter && token.length() > 0) { // We are done, we found the end of the string... done = true; - } else if (tok == '\\') { // Found an escaped char - escaped = true; - } else { // Adding char... - escaped = false; + } else { + // Found an escaped char? + escaped = tok == '\\'; } // Adding char to String:" + token.toString()); token.append(tok); loc++; } // Handling multiple lines string - if (!done // ... we didn't find the end of the string - && loc >= currentLine.length() // ... we have reach the end of - // the line ( the String is - // incomplete, for the moment at - // least) - && spanMultipleLinesString // ... the language allow multiple - // line span Strings - && lineNumber < code.size() - 1 // ... there is still more lines to - // parse + if (!done // ... we didn't find the end of the string (but the end of the line) + && spanMultipleLinesString // ... the language allow multiple line span Strings + && lineNumber < code.size() - 1 // ... there is still more lines to parse ) { // removes last character, if it is the line continuation (e.g. // backslash) character - if (spanMultipleLinesLineContinuationCharacter != null && token.length() > 0 - && token.charAt(token.length() - 1) == spanMultipleLinesLineContinuationCharacter.charValue()) { + if (spanMultipleLinesLineContinuationCharacter != null + && token.length() > 0 + && token.charAt(token.length() - 1) == spanMultipleLinesLineContinuationCharacter) { token.deleteCharAt(token.length() - 1); } // parsing new line From 815c87bd17b62a4c59aff29f2f32a0a2523733a9 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 18 Apr 2020 10:28:37 +0200 Subject: [PATCH 56/64] Add simple stats for releases --- do-release.sh | 38 ++++++++++++++++--- .../pmd/projectdocs/committers/releasing.md | 19 ++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/do-release.sh b/do-release.sh index 614c8f12c4..f276238301 100755 --- a/do-release.sh +++ b/do-release.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash set -e # Make sure, everything is English... @@ -15,7 +15,7 @@ if [ ! -f pom.xml -o ! -d ../pmd.github.io ]; then exit 1 fi - +LAST_VERSION= RELEASE_VERSION= DEVELOPMENT_VERSION= CURRENT_BRANCH= @@ -33,11 +33,16 @@ PATCH=$(echo $RELEASE_VERSION | cut -d . -f 3) if [ "$PATCH" == "0" ]; then NEXT_MINOR=$(expr ${MINOR} + 1) NEXT_PATCH="0" + LAST_MINOR=$(expr ${MINOR} - 1) + LAST_PATCH="0" else # this is a bugfixing release NEXT_MINOR="${MINOR}" NEXT_PATCH=$(expr ${PATCH} + 1) + LAST_MINOR="${MINOR}" + LAST_PATCH=$(expr ${PATCH} - 1) fi +LAST_VERSION="$MAJOR.$LAST_MINOR.$LAST_PATCH" DEVELOPMENT_VERSION="$MAJOR.$NEXT_MINOR.$NEXT_PATCH" DEVELOPMENT_VERSION="${DEVELOPMENT_VERSION}-SNAPSHOT" @@ -52,17 +57,18 @@ CURRENT_BRANCH=$(git symbolic-ref -q HEAD) CURRENT_BRANCH=${CURRENT_BRANCH##refs/heads/} CURRENT_BRANCH=${CURRENT_BRANCH:-HEAD} -echo "RELEASE_VERSION: ${RELEASE_VERSION}" -echo "DEVELOPMENT_VERSION: ${DEVELOPMENT_VERSION}" +echo "LAST_VERSION: ${LAST_VERSION}" +echo "RELEASE_VERSION: ${RELEASE_VERSION} (this release)" +echo "DEVELOPMENT_VERSION: ${DEVELOPMENT_VERSION} (the next version after the release)" echo "CURRENT_BRANCH: ${CURRENT_BRANCH}" echo echo "Is this correct?" echo -echo "Press enter to continue..." +echo "Press enter to continue... (or CTRL+C to cancel)" read - +export LAST_VERSION export RELEASE_VERSION export DEVELOPMENT_VERSION export CURRENT_BRANCH @@ -89,6 +95,26 @@ echo echo "Press enter to continue..." read + +# calculating stats for release notes + +STATS=$( +echo "### Stats" +echo "* $(git log pmd_releases/${LAST_VERSION}..HEAD --oneline --no-merges |wc -l) commits" +echo "* $(curl -s https://api.github.com/repos/pmd/pmd/milestones|jq ".[] | select(.title == \"$RELEASE_VERSION\") | .closed_issues") closed tickets & PRs" +echo "* Days since last release: $(( ( $(date +%s) - $(git log --max-count=1 --format="%at" pmd_releases/${LAST_VERSION}) ) / 86400))" +) + +TEMP_RELEASE_NOTES=$(cat docs/pages/release_notes.md) +TEMP_RELEASE_NOTES=${TEMP_RELEASE_NOTES/\{\% endtocmaker \%\}/$STATS$'\n'$'\n'\{\% endtocmaker \%\}$'\n'} +echo "${TEMP_RELEASE_NOTES}" > docs/pages/release_notes.md + +echo +echo "Updated stats in release notes:" +echo "$STATS" +echo +echo + # install bundles needed for rendering release notes bundle install --with=release_notes_preprocessing --path vendor/bundle diff --git a/docs/pages/pmd/projectdocs/committers/releasing.md b/docs/pages/pmd/projectdocs/committers/releasing.md index 0951ac7e16..f8966a7730 100644 --- a/docs/pages/pmd/projectdocs/committers/releasing.md +++ b/docs/pages/pmd/projectdocs/committers/releasing.md @@ -53,6 +53,25 @@ The designer lives at [pmd/pmd-designer](https://github.com/pmd/pmd-designer). Update property `pmd-designer.version` in **pom.xml** to reference the latest pmd-designer release. See for the available releases. +Starting with PMD 6.23.0 we'll provide small statistics for every release. This needs to be added +to the release notes as the last section. To count the closed issues and pull requests, the milestone +on github with the title of the new release is searched. Make sure, there is a milestone +on . The following snippet will +create the numbers, that can be attached to the release notes as a last section: + +```shell +LAST_VERSION=6.22.0 +NEW_VERSION=6.23.0 +NEW_VERSION_COMMITISH=HEAD + +echo "### Stats" +echo "* $(git log pmd_releases/${LAST_VERSION}..${NEW_VERSION_COMMITISH} --oneline --no-merges |wc -l) commits" +echo "* $(curl -s https://api.github.com/repos/pmd/pmd/milestones|jq ".[] | select(.title == \"$NEW_VERSION\") | .closed_issues") closed tickets & PRs" +echo "* Days since last release: $(( ( $(date +%s) - $(git log --max-count=1 --format="%at" pmd_releases/${LAST_VERSION}) ) / 86400))" +``` + +Note: this part is also integrated into `do-release.sh`. + Check in all (version) changes to branch master or any other branch, from which the release takes place: $ git commit -a -m "Prepare pmd release " From 2da636305e06500eaf7e530212ca9d772940ee68 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 18 Apr 2020 10:34:21 +0200 Subject: [PATCH 57/64] Update pull request template --- .github/PULL_REQUEST_TEMPLATE.md | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 6d529e9e99..a1c9a7bd37 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,10 +1,19 @@ - +## Describe the PR -Before submitting a PR, please check that: - - [ ] The PR is submitted against `master`. The PMD team will merge back to support branches as needed. - - [ ] `./mvnw clean verify` passes. This will [build](https://github.com/pmd/pmd/blob/master/BUILDING.md) and test PMD, execute PMD and checkstyle rules. [Check this for more info](https://github.com/pmd/pmd/blob/master/CONTRIBUTING.md#code-style) + -**PR Description:** +## Related issues + + + +- Fixes # + +## Ready? + + + +- [ ] Added unit tests for fixed bug/feature +- [ ] Passing all unit tests +- [ ] Complete build `./mvnw clean verify` passes (checked automatically by travis) +- [ ] Added (in-code) documentation (if needed) From 054b3acf6735c44f18a608af78d7c49177779b94 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 18 Apr 2020 11:49:37 +0200 Subject: [PATCH 58/64] [doc] Update release notes, fixes #2398 --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 794293b75c..4e45593dd7 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -50,6 +50,8 @@ Note that XPath 1.0 support, the default XPath version, is deprecated since PMD * [#2356](https://github.com/pmd/pmd/issues/2356): \[doc] Add missing doc about pmd.github.io * java * [#2378](https://github.com/pmd/pmd/issues/2378): \[java] AbstractJUnitRule has bad performance on large code bases +* java-bestpractices + * [#2398](https://github.com/pmd/pmd/issues/2398): \[java] AbstractClassWithoutAbstractMethod false negative with inner abstract classes * java-codestyle * [#1164](https://github.com/pmd/pmd/issues/1164): \[java] ClassNamingConventions suggests to add Util for class containing only static constants * [#1723](https://github.com/pmd/pmd/issues/1723): \[java] UseDiamondOperator false-positive inside lambda From 400ca5dca5a52bdc93de17b0e06290f640914bd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 18 Apr 2020 17:42:56 +0200 Subject: [PATCH 59/64] Delete test for findChildNodesWithXPath --- .../pmd/lang/ast/AbstractNodeTest.java | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java index b5ece64c46..64136ef446 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java @@ -6,10 +6,8 @@ package net.sourceforge.pmd.lang.ast; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import org.jaxen.JaxenException; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -228,24 +226,4 @@ public class AbstractNodeTest { assertEquals(0, grandChild.getNumChildren()); } - - @Test - public void testDeprecatedAttributeXPathQuery() throws JaxenException { - class MyRootNode extends DummyNode implements RootNode { - - private MyRootNode(int id) { - super(id); - } - } - - addChild(new MyRootNode(nextId()), new DummyNodeWithDeprecatedAttribute(2)).findChildNodesWithXPath("//dummyNode[@Size=1]"); - - String log = loggingRule.getLog(); - - assertTrue(log.contains("deprecated")); - assertTrue(log.contains("attribute")); - assertTrue(log.contains("dummyNode/@Size")); - } - - } From 4a9aa1a926ae07cc6b9dd4e4424eb02c17551278 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 18 Apr 2020 17:43:18 +0200 Subject: [PATCH 60/64] [core] Log deprecated attribute usage in findChildNodesWithXPath queries --- .../pmd/lang/ast/AbstractNode.java | 5 +++- .../internal/ContextualizedNavigator.java | 1 + .../xpath/internal/DeprecatedAttrLogger.java | 25 +++++++++++++++++++ .../pmd/lang/ast/AbstractNodeTest.java | 3 ++- 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java index c14ede1d12..2f8d3eb78c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java @@ -25,6 +25,8 @@ import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.xpath.Attribute; import net.sourceforge.pmd.lang.ast.xpath.AttributeAxisIterator; import net.sourceforge.pmd.lang.ast.xpath.DocumentNavigator; +import net.sourceforge.pmd.lang.ast.xpath.internal.ContextualizedNavigator; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttrLogger; import net.sourceforge.pmd.lang.dfa.DataFlowNode; import net.sourceforge.pmd.util.DataMap; import net.sourceforge.pmd.util.DataMap.DataKey; @@ -513,7 +515,8 @@ public abstract class AbstractNode implements Node { @Override @SuppressWarnings("unchecked") public List findChildNodesWithXPath(final String xpathString) throws JaxenException { - return new BaseXPath(xpathString, new DocumentNavigator()).selectNodes(this); + return new BaseXPath(xpathString, new ContextualizedNavigator(DeprecatedAttrLogger.createAdHocLogger())) + .selectNodes(this); } @Override diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/ContextualizedNavigator.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/ContextualizedNavigator.java index f48dc03031..358d7d7ce7 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/ContextualizedNavigator.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/ContextualizedNavigator.java @@ -1,6 +1,7 @@ /* * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ + package net.sourceforge.pmd.lang.ast.xpath.internal; import net.sourceforge.pmd.lang.ast.xpath.Attribute; diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java index b0501e7a78..e6c2358a06 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/xpath/internal/DeprecatedAttrLogger.java @@ -35,6 +35,14 @@ public abstract class DeprecatedAttrLogger { } } + public static DeprecatedAttrLogger createAdHocLogger() { + if (LOG.isLoggable(Level.WARNING)) { + return new AdhocLoggerImpl(); + } else { + return noop(); + } + } + public static Noop noop() { return Noop.INSTANCE; } @@ -89,4 +97,21 @@ public abstract class DeprecatedAttrLogger { return name; } } + + private static class AdhocLoggerImpl extends DeprecatedAttrLogger { + @Override + public void recordUsageOf(Attribute attribute) { + String replacement = attribute.replacementIfDeprecated(); + if (replacement != null) { + String name = getLoggableAttributeName(attribute); + // this message needs to be kept in sync with PMDCoverageTest / BinaryDistributionIT + String msg = "Use of deprecated attribute '" + name + "' in a findChildNodesWithXPath navigation"; + if (!replacement.isEmpty()) { + msg += ", please use " + replacement + " instead"; + } + // log with execption stack trace to help figure out where exactly the xpath is used. + LOG.log(Level.WARNING, msg, new RuntimeException(msg)); + } + } + } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java index b5ece64c46..5cc793c267 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java @@ -238,7 +238,8 @@ public class AbstractNodeTest { } } - addChild(new MyRootNode(nextId()), new DummyNodeWithDeprecatedAttribute(2)).findChildNodesWithXPath("//dummyNode[@Size=1]"); + Node root = addChild(new MyRootNode(nextId()), new DummyNodeWithDeprecatedAttribute(2)); + root.findChildNodesWithXPath("//dummyNode[@Size=1]"); String log = loggingRule.getLog(); From bcae7e52437da422ab85e97afd9230808f913b04 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 18 Apr 2020 18:10:05 +0200 Subject: [PATCH 61/64] Small corrections --- .../pmd/lang/ast/xpath/AttributeAxisIteratorTest.java | 2 +- .../pmd/lang/java/ast/ASTAdditiveExpression.java | 4 ++-- .../pmd/lang/java/ast/ASTVariableDeclaratorId.java | 4 ++-- .../pmd/lang/java/ast/AbstractAnyTypeDeclaration.java | 11 ++++++++--- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java index d5697ed3bd..a1dda2cb58 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/xpath/AttributeAxisIteratorTest.java @@ -71,7 +71,7 @@ public class AttributeAxisIteratorTest { assertFalse(atts.containsKey("NodeList")); } - public Map toMap(AttributeAxisIterator it) { + private Map toMap(AttributeAxisIterator it) { Map atts = new HashMap<>(); while (it.hasNext()) { Attribute attribute = it.next(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAdditiveExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAdditiveExpression.java index 9d6c995cb9..f60a30e32d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAdditiveExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAdditiveExpression.java @@ -48,13 +48,13 @@ public class ASTAdditiveExpression extends AbstractJavaTypeNode { @Deprecated @DeprecatedAttribute(replaceWith = "@Operator") public String getImage() { - return super.getImage(); + return getOperator(); } /** * Returns the image of the operator, i.e. "+" or "-". */ public String getOperator() { - return getImage(); + return super.getImage(); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java index cde67aa449..f20ecb7034 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java @@ -107,12 +107,12 @@ public class ASTVariableDeclaratorId extends AbstractJavaTypeNode implements Dim @DeprecatedAttribute(replaceWith = "@Name") @Deprecated public String getImage() { - return super.getImage(); + return getName(); } /** Returns the name of the variable. */ public String getName() { - return getImage(); + return super.getImage(); } /** diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractAnyTypeDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractAnyTypeDeclaration.java index 4bd5fc0607..ed9076c669 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractAnyTypeDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractAnyTypeDeclaration.java @@ -6,6 +6,7 @@ package net.sourceforge.pmd.lang.java.ast; import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.xpath.internal.DeprecatedAttribute; import net.sourceforge.pmd.lang.java.qname.JavaTypeQualifiedName; import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition; @@ -37,10 +38,14 @@ public abstract class AbstractAnyTypeDeclaration extends AbstractJavaAccessTypeN || getParent() instanceof ASTRecordBody; } - @Override + /** + * @deprecated Use {@link #getSimpleName()} + */ @Deprecated + @DeprecatedAttribute(replaceWith = "@SimpleName") + @Override public String getImage() { - return super.getImage(); + return getSimpleName(); } @Override @@ -50,7 +55,7 @@ public abstract class AbstractAnyTypeDeclaration extends AbstractJavaAccessTypeN @Override public String getSimpleName() { - return getImage(); + return super.getImage(); } From 5cad2ff741b4dacc312d1769b9b5894f513e9035 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 18 Apr 2020 18:22:07 +0200 Subject: [PATCH 62/64] [doc] Update release notes, refs #2425, fixes #2019 --- docs/pages/release_notes.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 794293b75c..5b7f998098 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -46,6 +46,7 @@ Note that XPath 1.0 support, the default XPath version, is deprecated since PMD * [#2210](https://github.com/pmd/pmd/issues/2210): \[apex] ApexCRUDViolation: Support WITH SECURITY_ENFORCED * [#2399](https://github.com/pmd/pmd/issues/2399): \[apex] ApexCRUDViolation: false positive with security enforced with line break * core + * [#2019](https://github.com/pmd/pmd/issues/2019): \[core] Insufficient deprecation warnings for XPath attributes * [#2355](https://github.com/pmd/pmd/issues/2355): \[doc] Improve documentation about incremental analysis * [#2356](https://github.com/pmd/pmd/issues/2356): \[doc] Add missing doc about pmd.github.io * java @@ -123,6 +124,12 @@ implementations, and their corresponding Parser if it exists (in the same packag * {% jdoc matlab::lang.matlab.MatlabTokenManager %} * {% jdoc objectivec::lang.objectivec.ObjectiveCTokenManager %} +In the **Java AST** the following attributes are deprecated and will issue a warning when used in XPath rules: + +* {% jdoc !!java::lang.java.ast.ASTAdditiveExpression#getImage() %} - use `getOperator()` instead +* {% jdoc !!java::lang.java.ast.ASTVariableDeclaratorId#getImage() %} - use `getName()` instead +* {% jdoc !!java::lang.java.ast.ASTVariableDeclaratorId#getVariableName() %} - use `getName()` instead + ##### For removal * {% jdoc !!core::lang.Parser#getTokenManager(java.lang.String,java.io.Reader) %} From aee04b4c6890b3bf64236c8d7665ac31db9e4536 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 18 Apr 2020 19:06:29 +0200 Subject: [PATCH 63/64] [java] Fix more deprecated attribute usages --- .../ForLoopCanBeForeachRule.java | 2 +- .../resources/category/java/codestyle.xml | 8 ++--- .../main/resources/category/java/design.xml | 2 +- .../resources/category/java/errorprone.xml | 8 ++--- .../java/net/sourceforge/pmd/ReportTest.java | 2 +- .../pmd/lang/java/rule/XPathRuleTest.java | 12 ++++---- .../typeresolution/ClassTypeResolverTest.java | 29 ++++++++----------- .../xml/UnusedFormalParameter.xml | 2 +- 8 files changed, 30 insertions(+), 35 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java index fccfd94132..fb7873371d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java @@ -229,7 +229,7 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule { + "/Name[matches(@Image,'\\w+\\.(size|length)')]" + "|" + "./RelationalExpression[@Image='<=']/AdditiveExpression[count(*)=2 and " - + "@Image='-' and PrimaryExpression/PrimaryPrefix/Literal[@Image='1']]" + + "@Operator='-' and PrimaryExpression/PrimaryPrefix/Literal[@Image='1']]" + "/PrimaryExpression/PrimaryPrefix/Name[matches(@Image,'\\w+\\.(size|length)')]"); if (left.isEmpty()) { diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index 4de7330e0d..e8995ca3a9 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -164,7 +164,7 @@ by the more general rule {% rule java/codestyle/FormalParameterNamingConventions @@ -1364,7 +1364,7 @@ by the more general rule @@ -1758,7 +1758,7 @@ by the more general rule {% rule java/codestyle/FieldNamingConventions %}. //ClassOrInterfaceDeclaration[@Interface= false()] /ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/FieldDeclaration [@Final= false()] - [VariableDeclarator/VariableDeclaratorId[upper-case(@Image)=@Image]] + [VariableDeclarator/VariableDeclaratorId[upper-case(@Name)=@Name]] ]]> @@ -2067,7 +2067,7 @@ List stringsWithDiamond = new ArrayList<>(); // using the diamond operat /PrimaryExpression[1]/PrimaryPrefix/Expression[ count(*)=1 and not(./CastExpression) and - not(./AdditiveExpression[@Image = '-']) and + not(./AdditiveExpression[@Operator = '-']) and not(./ShiftExpression) and not(./RelationalExpression) and not(./InstanceOfExpression) and diff --git a/pmd-java/src/main/resources/category/java/design.xml b/pmd-java/src/main/resources/category/java/design.xml index fd13c4cf42..dba3f14cec 100644 --- a/pmd-java/src/main/resources/category/java/design.xml +++ b/pmd-java/src/main/resources/category/java/design.xml @@ -133,7 +133,7 @@ Catch blocks that merely rethrow a caught exception only add to code size and ru diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index 5b2b1f5929..07d265c0ba 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -422,7 +422,7 @@ Each caught exception type should be handled in its own catch clause. /following-sibling::Block//InstanceOfExpression/PrimaryExpression/PrimaryPrefix /Name[ @Image = ./ancestor::Block/preceding-sibling::FormalParameter - /VariableDeclaratorId/@Image + /VariableDeclaratorId/@Name ] ]]> @@ -2414,7 +2414,7 @@ chain needs an own serialVersionUID field. See also [Should an abstract class ha //ClassOrInterfaceDeclaration [@Interface = false()] [count(ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration - /FieldDeclaration/VariableDeclarator/VariableDeclaratorId[@Image='serialVersionUID']) = 0] + /FieldDeclaration/VariableDeclarator/VariableDeclaratorId[@Name='serialVersionUID']) = 0] [(ImplementsList | ExtendsList)/ClassOrInterfaceType[pmd-java:typeIs('java.io.Serializable')]] ]]> @@ -3362,11 +3362,11 @@ To make sure the full stacktrace is printed out, use the logging statement with [starts-with(@Image, concat((ancestor::ClassOrInterfaceDeclaration/ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/FieldDeclaration [Type//ClassOrInterfaceType[@Image='Log']] - /VariableDeclarator/VariableDeclaratorId/@Image)[1], '.')) + /VariableDeclarator/VariableDeclaratorId/@Name)[1], '.')) ] ] [PrimarySuffix/Arguments[@Size= 1]] - [PrimarySuffix/Arguments//Name/@Image = ancestor::CatchStatement/FormalParameter/VariableDeclaratorId/@Image] + [PrimarySuffix/Arguments//Name/@Image = ancestor::CatchStatement/FormalParameter/VariableDeclaratorId/@Name] ]]> diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/ReportTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/ReportTest.java index cac861109b..383d64cbde 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/ReportTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/ReportTest.java @@ -40,7 +40,7 @@ public class ReportTest extends RuleTst { public void testExclusionsInReportWithRuleViolationSuppressXPath() { Report rpt = new Report(); Rule rule = new FooRule(); - rule.setProperty(Rule.VIOLATION_SUPPRESS_XPATH_DESCRIPTOR, ".[@Image = 'Foo']"); + rule.setProperty(Rule.VIOLATION_SUPPRESS_XPATH_DESCRIPTOR, ".[@SimpleName = 'Foo']"); runTestFromString(TEST1, rule, rpt, defaultLanguage); assertTrue(rpt.isEmpty()); assertEquals(1, rpt.getSuppressedRuleViolations().size()); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java index a82c36426b..90e01fabe5 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java @@ -1,4 +1,4 @@ -/** +/* * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ @@ -53,7 +53,7 @@ public class XPathRuleTest extends RuleTst { @Test public void testPluginname() throws Exception { - XPathRule rule = makeXPath("//VariableDeclaratorId[string-length(@Image) < 3]"); + XPathRule rule = makeXPath("//VariableDeclaratorId[string-length(@Name) < 3]"); rule.setMessage("{0}"); Report report = getReportForTestString(rule, TEST1); RuleViolation rv = report.iterator().next(); @@ -63,7 +63,7 @@ public class XPathRuleTest extends RuleTst { @Test public void testXPathMultiProperty() throws Exception { - XPathRule rule = makeXPath("//VariableDeclaratorId[@Image=$forbiddenNames]"); + XPathRule rule = makeXPath("//VariableDeclaratorId[@Name=$forbiddenNames]"); rule.setMessage("Avoid vars"); PropertyDescriptor> varDescriptor = PropertyFactory.stringListProperty("forbiddenNames") @@ -86,7 +86,7 @@ public class XPathRuleTest extends RuleTst { @Test public void testVariables() throws Exception { - XPathRule rule = makeXPath("//VariableDeclaratorId[@Image=$var]"); + XPathRule rule = makeXPath("//VariableDeclaratorId[@Name=$var]"); rule.setMessage("Avoid vars"); PropertyDescriptor varDescriptor = PropertyFactory.stringProperty("var").desc("Test var").defaultValue("").build(); @@ -99,7 +99,7 @@ public class XPathRuleTest extends RuleTst { @Test public void testFnPrefixOnSaxon() throws Exception { - XPathRule rule = makeXPath("//VariableDeclaratorId[fn:matches(@Image, 'fiddle')]"); + XPathRule rule = makeXPath("//VariableDeclaratorId[fn:matches(@Name, 'fiddle')]"); Report report = getReportForTestString(rule, TEST2); RuleViolation rv = report.iterator().next(); assertEquals(3, rv.getBeginLine()); @@ -107,7 +107,7 @@ public class XPathRuleTest extends RuleTst { @Test public void testNoFnPrefixOnSaxon() throws Exception { - XPathRule rule = makeXPath("//VariableDeclaratorId[matches(@Image, 'fiddle')]"); + XPathRule rule = makeXPath("//VariableDeclaratorId[matches(@Name, 'fiddle')]"); Report report = getReportForTestString(rule, TEST2); RuleViolation rv = report.iterator().next(); assertEquals(3, rv.getBeginLine()); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java index 57ae0deec3..94756287bb 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/ClassTypeResolverTest.java @@ -452,8 +452,7 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = java5.parseClass(Promotion.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " - + "'unaryNumericPromotion']]//Expression[UnaryExpression]"), + "//MethodDeclaration[@Name = 'unaryNumericPromotion']/Block//Expression[UnaryExpression]"), ASTExpression.class); int index = 0; @@ -474,8 +473,7 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = java5.parseClass(Promotion.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " - + "'binaryNumericPromotion']]//Expression[AdditiveExpression]"), + "//MethodDeclaration[@Name = 'binaryNumericPromotion']/Block//Expression[AdditiveExpression]"), ASTExpression.class); int index = 0; @@ -545,7 +543,7 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = java5.parseClass(Promotion.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = 'binaryStringPromotion']]//Expression"), + "//MethodDeclaration[@Name = 'binaryStringPromotion']/Block//Expression"), ASTExpression.class); int index = 0; @@ -564,7 +562,7 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = java5.parseClass(Operators.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = 'unaryLogicalOperators']]//Expression"), + "//MethodDeclaration[@Name = 'unaryLogicalOperators']/Block//Expression"), ASTExpression.class); int index = 0; @@ -580,7 +578,7 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = java5.parseClass(Operators.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = 'binaryLogicalOperators']]//Expression"), + "//MethodDeclaration[@Name = 'binaryLogicalOperators']/Block//Expression"), ASTExpression.class); int index = 0; @@ -606,24 +604,22 @@ public class ClassTypeResolverTest { public void testUnaryNumericOperators() throws JaxenException { ASTCompilationUnit acu = java5.parseClass(Operators.class); List expressions = new ArrayList<>(); + final String baseXPath = "//MethodDeclaration[@Name = 'unaryNumericOperators']/Block"; expressions.addAll(convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = 'unaryNumericOperators']]//Expression"), + baseXPath + "//Expression"), TypeNode.class)); expressions.addAll(convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " - + "'unaryNumericOperators']]//PostfixExpression"), + baseXPath + "//PostfixExpression"), TypeNode.class)); expressions.addAll(convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " - + "'unaryNumericOperators']]//PreIncrementExpression"), + baseXPath + "//PreIncrementExpression"), TypeNode.class)); expressions.addAll(convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " - + "'unaryNumericOperators']]//PreDecrementExpression"), + baseXPath + "//PreDecrementExpression"), TypeNode.class)); int index = 0; @@ -643,7 +639,7 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = java5.parseClass(Operators.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = 'binaryNumericOperators']]//Expression"), + "//MethodDeclaration[@Name = 'binaryNumericOperators']/Block//Expression"), ASTExpression.class); int index = 0; @@ -665,8 +661,7 @@ public class ClassTypeResolverTest { ASTCompilationUnit acu = java5.parseClass(Operators.class); List expressions = convertList( acu.findChildNodesWithXPath( - "//Block[preceding-sibling::MethodDeclarator[@Image = " - + "'assignmentOperators']]//StatementExpression"), + "//MethodDeclaration[@Name = 'assignmentOperators']/Block//StatementExpression"), ASTStatementExpression.class); int index = 0; diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedFormalParameter.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedFormalParameter.xml index ba281bbf8a..879d7d748a 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedFormalParameter.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedFormalParameter.xml @@ -217,7 +217,7 @@ class Foo { violation suppression xpath works, by name ]]> - .[@Image = 'paramB'] + .[@Name = 'paramB'] 0 Date: Mon, 20 Apr 2020 09:46:11 +0200 Subject: [PATCH 64/64] Update release notes Refs #2413 --- docs/pages/release_notes.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index a470494bab..5265da4b23 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -45,9 +45,10 @@ Note that XPath 1.0 support, the default XPath version, is deprecated since PMD * apex-security * [#2210](https://github.com/pmd/pmd/issues/2210): \[apex] ApexCRUDViolation: Support WITH SECURITY_ENFORCED * [#2399](https://github.com/pmd/pmd/issues/2399): \[apex] ApexCRUDViolation: false positive with security enforced with line break -* core +* doc * [#2355](https://github.com/pmd/pmd/issues/2355): \[doc] Improve documentation about incremental analysis * [#2356](https://github.com/pmd/pmd/issues/2356): \[doc] Add missing doc about pmd.github.io + * [#2413](https://github.com/pmd/pmd/issues/2413): \[doc] Improve documentation about the available renderers (PMD/CPD) * java * [#2378](https://github.com/pmd/pmd/issues/2378): \[java] AbstractJUnitRule has bad performance on large code bases * java-codestyle