From 6989b45067c25f2cfe211ccad82e09c6751168fb Mon Sep 17 00:00:00 2001 From: Jan Aertgeerts Date: Mon, 9 Oct 2017 23:24:47 +0200 Subject: [PATCH 01/62] [apex] avoid hardcoding id's --- docs/pages/pmd/rules/apex/security.md | 40 +++++++++++++- .../rule/security/AvoidHardcodingIdRule.java | 42 +++++++++++++++ .../main/resources/rulesets/apex/ruleset.xml | 9 ++++ .../main/resources/rulesets/apex/security.xml | 27 ++++++++++ .../apex/rule/security/SecurityRulesTest.java | 1 + .../rule/security/xml/AvoidHardcodingId.xml | 53 +++++++++++++++++++ 6 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/AvoidHardcodingIdRule.java create mode 100644 pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/AvoidHardcodingId.xml diff --git a/docs/pages/pmd/rules/apex/security.md b/docs/pages/pmd/rules/apex/security.md index 09ca5f3905..ba4e03c0d4 100644 --- a/docs/pages/pmd/rules/apex/security.md +++ b/docs/pages/pmd/rules/apex/security.md @@ -5,7 +5,7 @@ permalink: pmd_rules_apex_security.html folder: pmd/rules/apex sidebaractiveurl: /pmd_rules_apex.html editmepath: ../pmd-apex/src/main/resources/rulesets/apex/security.xml -keywords: Security, ApexSharingViolations, ApexOpenRedirect, ApexInsecureEndpoint, ApexXSSFromURLParam, ApexXSSFromEscapeFalse, ApexBadCrypto, ApexCSRF, ApexSOQLInjection, ApexCRUDViolation, ApexDangerousMethods, ApexSuggestUsingNamedCred +keywords: Security, ApexSharingViolations, ApexOpenRedirect, ApexInsecureEndpoint, ApexXSSFromURLParam, ApexXSSFromEscapeFalse, ApexBadCrypto, ApexCSRF, ApexSOQLInjection, ApexCRUDViolation, ApexDangerousMethods, ApexSuggestUsingNamedCred, AvoidHardcodingId --- ## ApexBadCrypto @@ -412,3 +412,41 @@ public without sharing class Foo { ``` +## AvoidHardcodingId + +**Since:** PMD 6.0.0 + +**Priority:** Medium (3) + +When deploying Apex code between sandbox and production environments, or installing Force.com AppExchange packages, it is essential to avoid hardcoding IDs in the Apex code. By doing so, if the record IDs change between environments, the logic can dynamically identify the proper data to operate against and not fail. + +**This rule is defined by the following Java class:** [net.sourceforge.pmd.lang.apex.rule.security.AvoidHardcodingIdRule](https://github.com/pmd/pmd/blob/master/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/AvoidHardcodingIdRule.java) + +**Example(s):** + +``` java +public without sharing class Foo { + for(Account a: Trigger.new){ + //Error - hardcoded the record type id + if(a.RecordTypeId=='012500000009WAr'){ + //do some logic here..... + } else if(a.RecordTypeId=='0123000000095Km'){ + //do some logic here for a different record type... + } + } +} +``` + +**This rule has the following properties:** + +|Name|Default Value|Description| +|----|-------------|-----------| +|cc_categories|[Style]|Code Climate Categories| +|cc_remediation_points_multiplier|1|Code Climate Remediation Points multiplier| +|cc_block_highlighting|false|Code Climate Block Highlighting| + +**Use this rule by referencing it:** +``` xml + +``` + diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/AvoidHardcodingIdRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/AvoidHardcodingIdRule.java new file mode 100644 index 0000000000..f7ca99afc7 --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/AvoidHardcodingIdRule.java @@ -0,0 +1,42 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.security; + +import java.util.regex.Pattern; + +import net.sourceforge.pmd.lang.apex.ast.ASTAssignmentExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTLiteralExpression; +import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode; +import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; + +public class AvoidHardcodingIdRule extends AbstractApexRule { + private static final Pattern PATTERN = Pattern.compile("^[a-zA-Z0-9]{5}[0][a-zA-Z0-9]{9,12}$", Pattern.CASE_INSENSITIVE); + + public AvoidHardcodingIdRule() { + setProperty(CODECLIMATE_CATEGORIES, "Security"); + setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); + setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); + } + + @Override + public Object visit(ASTAssignmentExpression node, Object data) { + findHardcodedId(node, data); + return data; + } + + private void findHardcodedId(AbstractApexNode node, Object data) { + ASTLiteralExpression literalNode = node.getFirstChildOfType(ASTLiteralExpression.class); + + if (literalNode != null) { + Object o = literalNode.getNode().getLiteral(); + if (o instanceof String) { + String literal = (String) o; + if (PATTERN.matcher(literal).matches()) { + addViolation(data, literalNode); + } + } + } + } +} diff --git a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml index 768ecd8994..074e35a711 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml @@ -287,6 +287,15 @@ + + 3 + + + + + + + 3 diff --git a/pmd-apex/src/main/resources/rulesets/apex/security.xml b/pmd-apex/src/main/resources/rulesets/apex/security.xml index 8f111016ac..8dc07c0bca 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/security.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/security.xml @@ -272,4 +272,31 @@ public class Foo { + + +When deploying Apex code between sandbox and production environments, or installing Force.com AppExchange packages, +it is essential to avoid hardcoding IDs in the Apex code. By doing so, if the record IDs change between environments, +the logic can dynamically identify the proper data to operate against and not fail. + + 3 + + + + + diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java index 703f112d86..7d2e8cb89e 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java @@ -23,5 +23,6 @@ public class SecurityRulesTest extends SimpleAggregatorTst { addRule(RULESET, "ApexCRUDViolation"); addRule(RULESET, "ApexDangerousMethods"); addRule(RULESET, "ApexSuggestUsingNamedCred"); + addRule(RULESET, "AvoidHardcodingId"); } } diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/AvoidHardcodingId.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/AvoidHardcodingId.xml new file mode 100644 index 0000000000..51e02c2869 --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/AvoidHardcodingId.xml @@ -0,0 +1,53 @@ + + + + + + Non compliant scenario: Hardcoded Id + 1 + + + + + Compliant scenario, getting ID dynamically + 1 + + + + + Test for random string combinations + 0 + + + From 9845f74f680f9a4672157b76a308b9078a1ffe77 Mon Sep 17 00:00:00 2001 From: Jan Aertgeerts Date: Mon, 9 Oct 2017 23:35:14 +0200 Subject: [PATCH 02/62] [apex] rework visit --- .../rule/security/AvoidHardcodingIdRule.java | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/AvoidHardcodingIdRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/AvoidHardcodingIdRule.java index f7ca99afc7..97799220ce 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/AvoidHardcodingIdRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/AvoidHardcodingIdRule.java @@ -6,9 +6,7 @@ package net.sourceforge.pmd.lang.apex.rule.security; import java.util.regex.Pattern; -import net.sourceforge.pmd.lang.apex.ast.ASTAssignmentExpression; import net.sourceforge.pmd.lang.apex.ast.ASTLiteralExpression; -import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode; import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; public class AvoidHardcodingIdRule extends AbstractApexRule { @@ -21,22 +19,14 @@ public class AvoidHardcodingIdRule extends AbstractApexRule { } @Override - public Object visit(ASTAssignmentExpression node, Object data) { - findHardcodedId(node, data); - return data; - } - - private void findHardcodedId(AbstractApexNode node, Object data) { - ASTLiteralExpression literalNode = node.getFirstChildOfType(ASTLiteralExpression.class); - - if (literalNode != null) { - Object o = literalNode.getNode().getLiteral(); - if (o instanceof String) { - String literal = (String) o; - if (PATTERN.matcher(literal).matches()) { - addViolation(data, literalNode); - } + public Object visit(ASTLiteralExpression node, Object data) { + Object o = node.getNode().getLiteral(); + if (o instanceof String) { + String literal = (String) o; + if (PATTERN.matcher(literal).matches()) { + addViolation(data, node); } } + return data; } } From 475405e18f4384b7d775e9cfd4a8414905e5ba61 Mon Sep 17 00:00:00 2001 From: Jan Aertgeerts Date: Mon, 9 Oct 2017 23:43:43 +0200 Subject: [PATCH 03/62] [apex] test issue - count --- .../pmd/lang/apex/rule/security/xml/AvoidHardcodingId.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/AvoidHardcodingId.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/AvoidHardcodingId.xml index 51e02c2869..b7d77b9f05 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/AvoidHardcodingId.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/AvoidHardcodingId.xml @@ -23,7 +23,7 @@ public class Foo { Compliant scenario, getting ID dynamically - 1 + 0 Date: Fri, 13 Oct 2017 11:29:51 +0200 Subject: [PATCH 04/62] [java] Move rule LooseCoupling from typeresolution to coupling Replace existing rule with the typeresolution-based implementation. --- docs/pages/release_notes.md | 33 ++-- .../pmd/RuleSetFactoryCompatibility.java | 2 +- .../java/rule/coupling/LooseCouplingRule.java | 12 +- .../typeresolution/rules/LooseCoupling.java | 52 ------ .../main/resources/rulesets/java/coupling.xml | 30 ++-- .../rulesets/java/typeresolution.xml | 28 +-- .../typeresolution/LooseCouplingTest.java | 17 -- .../java/rule/coupling/xml/LooseCoupling.xml | 10 ++ .../rule/typeresolution/xml/LooseCoupling.xml | 164 ------------------ 9 files changed, 55 insertions(+), 293 deletions(-) delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/LooseCoupling.java delete mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/LooseCouplingTest.java delete mode 100644 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/LooseCoupling.xml diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 14e22053a1..79eee5eec9 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -114,20 +114,20 @@ providing configuration error reporting are: As we move forward we will be able to detect and report more configuration errors (ie: incomplete `auxclasspath`) and include them to such reports. -#### Apex Rule Suppression - -Apex violations can now be suppressed very similarly to how it's done in Java, by making use of a -`@SuppressWarnings` annotation. - -Supported syntax includes: - -``` -@SupressWarnings('PMD') // to supress all Apex rules -@SupressWarnings('all') // to supress all Apex rules -@SupressWarnings('PMD.ARuleName') // to supress only the rule named ARuleName -@SupressWarnings('PMD.ARuleName, PMD.AnotherRuleName') // to supress only the rule named ARuleName or AnotherRuleName -``` - +#### Apex Rule Suppression + +Apex violations can now be suppressed very similarly to how it's done in Java, by making use of a +`@SuppressWarnings` annotation. + +Supported syntax includes: + +``` +@SupressWarnings('PMD') // to supress all Apex rules +@SupressWarnings('all') // to supress all Apex rules +@SupressWarnings('PMD.ARuleName') // to supress only the rule named ARuleName +@SupressWarnings('PMD.ARuleName, PMD.AnotherRuleName') // to supress only the rule named ARuleName or AnotherRuleName +``` + Notice this last scenario is slightly different to the Java syntax. This is due to differences in the Apex grammar for annotations. #### New Rules @@ -172,11 +172,15 @@ Notice this last scenario is slightly different to the Java syntax. This is due * The rule `GodClass` (ruleset `java-design`) has been revamped to use the new metrics framework. +* The rule `LooseCoupling` (ruleset `java-coupling`) has been replaced by the typeresolution-based implementation. + #### Deprecated Rules * The rules `NcssConstructorCount`, `NcssMethodCount`, and `NcssTypeCount` (ruleset `java-codesize`) have been deprecated. They will be replaced by the new rule `NcssCount` in the same ruleset. +* The rule `LooseCoupling` in ruleset `java-typeresolution` is deprecated. Use the rule with the same name from ruleset `java-coupling` instead. + #### Removed Rules * The deprecated rule `UseSingleton` has been removed from the ruleset `java-design`. The rule has been renamed @@ -383,4 +387,3 @@ a warning will now be produced suggesting users to adopt it for better performan * [#632](https://github.com/pmd/pmd/pull/632): \[apex] Add AvoidDirectAccessTriggerMap rule to the style set - [Jan Aertgeerts](https://github.com/JAertgeerts) * [#644](https://github.com/pmd/pmd/pull/644): \[core] Prevent internal dev-properties from being displayed on CodeClimate renderer - [Filipe Esperandio](https://github.com/filipesperandio) * [#660](https://github.com/pmd/pmd/pull/660): \[apex] avoid sosl in loops - [Jan Aertgeerts](https://github.com/JAertgeerts) - diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java index af39be8cf4..5db5a712e3 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java @@ -71,7 +71,7 @@ public class RuleSetFactoryCompatibility { // PMD 6.0.0 addFilterRuleMoved("java", "controversial", "unnecessary", "UnnecessaryParentheses"); addFilterRuleRenamed("java", "unnecessary", "UnnecessaryParentheses", "UselessParentheses"); - + addFilterRuleMoved("java", "typeresolution", "coupling", "LooseCoupling"); } void addFilterRuleRenamed(String language, String ruleset, String oldName, String newName) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/coupling/LooseCouplingRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/coupling/LooseCouplingRule.java index 5000de0f74..51b1ee39a1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/coupling/LooseCouplingRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/coupling/LooseCouplingRule.java @@ -29,15 +29,17 @@ public class LooseCouplingRule extends AbstractJavaRule { // "java.util.TreeMap", "java.util.Vector" // }); + @Override public Object visit(ASTClassOrInterfaceType node, Object data) { if (methodHasOverride(node)) { return data; } Node parent = node.getNthParent(3); - String typeName = node.getImage(); - if (CollectionUtil.isCollectionType(typeName, false) && (parent instanceof ASTFieldDeclaration - || parent instanceof ASTFormalParameter || parent instanceof ASTResultType)) { - addViolation(data, node, typeName); + Class clazzType = node.getType(); + boolean isType = CollectionUtil.isCollectionType(clazzType, false); + if (isType && (parent instanceof ASTFieldDeclaration || parent instanceof ASTFormalParameter + || parent instanceof ASTResultType)) { + addViolation(data, node, node.getImage()); } return data; } @@ -48,7 +50,7 @@ public class LooseCouplingRule extends AbstractJavaRule { ASTMarkerAnnotation marker = method.getFirstDescendantOfType(ASTMarkerAnnotation.class); if (marker != null && marker.getFirstChildOfType(ASTName.class) != null) { ASTName name = marker.getFirstChildOfType(ASTName.class); - if ("Override".equals(name.getImage())) { + if (name.getType() == Override.class) { return true; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/LooseCoupling.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/LooseCoupling.java deleted file mode 100644 index c60c0a1b4f..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/LooseCoupling.java +++ /dev/null @@ -1,52 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.typeresolution.rules; - -import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; -import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; -import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter; -import net.sourceforge.pmd.lang.java.ast.ASTMarkerAnnotation; -import net.sourceforge.pmd.lang.java.ast.ASTName; -import net.sourceforge.pmd.lang.java.ast.ASTResultType; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; -import net.sourceforge.pmd.util.CollectionUtil; - -/** - * This is a separate rule, uses the type resolution facade - */ -public class LooseCoupling extends AbstractJavaRule { - - @Override - public Object visit(ASTClassOrInterfaceType node, Object data) { - if (methodHasOverride(node)) { - return data; - } - Node parent = node.getNthParent(3); - Class clazzType = node.getType(); - boolean isType = CollectionUtil.isCollectionType(clazzType, false); - if (isType && (parent instanceof ASTFieldDeclaration || parent instanceof ASTFormalParameter - || parent instanceof ASTResultType)) { - addViolation(data, node, node.getImage()); - } - return data; - } - - private boolean methodHasOverride(Node node) { - ASTClassOrInterfaceBodyDeclaration method = node.getFirstParentOfType(ASTClassOrInterfaceBodyDeclaration.class); - if (method != null && method.jjtGetNumChildren() > 0 && method.jjtGetChild(0) instanceof ASTAnnotation) { - ASTMarkerAnnotation marker = method.getFirstDescendantOfType(ASTMarkerAnnotation.class); - if (marker != null && marker.getFirstChildOfType(ASTName.class) != null) { - ASTName name = marker.getFirstChildOfType(ASTName.class); - if (name.getType() == Override.class) { - return true; - } - } - } - return false; - } -} diff --git a/pmd-java/src/main/resources/rulesets/java/coupling.xml b/pmd-java/src/main/resources/rulesets/java/coupling.xml index 43c9a63a02..8d4fda1579 100644 --- a/pmd-java/src/main/resources/rulesets/java/coupling.xml +++ b/pmd-java/src/main/resources/rulesets/java/coupling.xml @@ -68,27 +68,33 @@ public class Foo { since="0.7" message="Avoid using implementation types like ''{0}''; use the interface instead" class="net.sourceforge.pmd.lang.java.rule.coupling.LooseCouplingRule" + typeResolution="true" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_coupling.html#loosecoupling"> -The use of implementation types as object references limits your ability to use alternate -implementations in the future as requirements change. Whenever available, referencing objects -by their interface types provides much more flexibility. +The use of implementation types (i.e., HashSet) as object references limits your ability to use alternate +implementations in the future as requirements change. Whenever available, referencing objects +by their interface types (i.e, Set) provides much more flexibility. 3 list = new ArrayList<>(); +import java.util.ArrayList; +import java.util.HashSet; -public HashSet getFoo() { - return new HashSet(); -} +public class Bar { + // sub-optimal approach + private ArrayList list = new ArrayList<>(); -// preferred approach -private List list = new ArrayList<>(); + public HashSet getFoo() { + return new HashSet(); + } -public Set getFoo() { - return new HashSet(); + // preferred approach + private List list = new ArrayList<>(); + + public Set getFoo() { + return new HashSet(); + } } ]]> diff --git a/pmd-java/src/main/resources/rulesets/java/typeresolution.xml b/pmd-java/src/main/resources/rulesets/java/typeresolution.xml index 27fe9c11d3..ced5389296 100644 --- a/pmd-java/src/main/resources/rulesets/java/typeresolution.xml +++ b/pmd-java/src/main/resources/rulesets/java/typeresolution.xml @@ -9,33 +9,7 @@ These are rules which resolve java Class files for comparison, as opposed to a String - - - -Avoid using implementation types (i.e., HashSet); use the interface (i.e, Set) instead - - 3 - - - - + 1 0 1 2 2 1 #938 False positive on LooseCoupling for overriding methods 0 - - - - 1 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 1 - - - - - 2 - - - - - 2 - - - - - 1 - - - - - 1 - - - - - 1 - - - - - 1 - - - - - #938 False positive on LooseCoupling for overriding methods - 0 - - - From a89d449169d74391ad12bc11eb577c7bebc90547 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 13 Oct 2017 12:27:59 +0200 Subject: [PATCH 05/62] [java] Move rule CloneMethodMustImplementCloneable from typeresolution to clone Replace existing rule with the typeresolution-based implementation. --- docs/pages/release_notes.md | 6 + .../pmd/RuleSetFactoryCompatibility.java | 1 + ...loneMethodMustImplementCloneableRule.java} | 4 +- .../main/resources/rulesets/java/clone.xml | 28 +-- .../rulesets/java/typeresolution.xml | 24 +-- ...CloneMethodMustImplementCloneableTest.java | 16 -- .../xml/CloneMethodMustImplementCloneable.xml | 95 ++++++++ .../xml/CloneMethodMustImplementCloneable.xml | 202 ------------------ 8 files changed, 111 insertions(+), 265 deletions(-) rename pmd-java/src/main/java/net/sourceforge/pmd/lang/java/{typeresolution/rules/CloneMethodMustImplementCloneable.java => rule/clone/CloneMethodMustImplementCloneableRule.java} (98%) delete mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/CloneMethodMustImplementCloneableTest.java delete mode 100644 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/CloneMethodMustImplementCloneable.xml diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 79eee5eec9..e4e910ed90 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -174,6 +174,9 @@ Notice this last scenario is slightly different to the Java syntax. This is due * The rule `LooseCoupling` (ruleset `java-coupling`) has been replaced by the typeresolution-based implementation. +* The rule `CloneMethodMustImplementCloneable` (ruleset `java-clone`) has been replaced by the typeresolution-based + implementation and is now able to detect cases if a class implements or extends a Cloneable class/interface. + #### Deprecated Rules * The rules `NcssConstructorCount`, `NcssMethodCount`, and `NcssTypeCount` (ruleset `java-codesize`) have been @@ -181,6 +184,9 @@ Notice this last scenario is slightly different to the Java syntax. This is due * The rule `LooseCoupling` in ruleset `java-typeresolution` is deprecated. Use the rule with the same name from ruleset `java-coupling` instead. +* The rule `CloneMethodMustImplementCloneable` in ruleset `java-typeresolution`is deprecated. Use the rule with + the same name from ruleset `java-clone` instead. + #### Removed Rules * The deprecated rule `UseSingleton` has been removed from the ruleset `java-design`. The rule has been renamed diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java index 5db5a712e3..fd0257c1b4 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java @@ -72,6 +72,7 @@ public class RuleSetFactoryCompatibility { addFilterRuleMoved("java", "controversial", "unnecessary", "UnnecessaryParentheses"); addFilterRuleRenamed("java", "unnecessary", "UnnecessaryParentheses", "UselessParentheses"); addFilterRuleMoved("java", "typeresolution", "coupling", "LooseCoupling"); + addFilterRuleMoved("java", "typeresolution", "clone", "CloneMethodMustImplementCloneable"); } void addFilterRuleRenamed(String language, String ruleset, String oldName, String newName) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/CloneMethodMustImplementCloneable.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/clone/CloneMethodMustImplementCloneableRule.java similarity index 98% rename from pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/CloneMethodMustImplementCloneable.java rename to pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/clone/CloneMethodMustImplementCloneableRule.java index 9bf9e3603b..cc8b5bf8de 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/CloneMethodMustImplementCloneable.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/clone/CloneMethodMustImplementCloneableRule.java @@ -2,7 +2,7 @@ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ -package net.sourceforge.pmd.lang.java.typeresolution.rules; +package net.sourceforge.pmd.lang.java.rule.clone; import java.util.HashSet; import java.util.List; @@ -30,7 +30,7 @@ import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; * * @author acaplan */ -public class CloneMethodMustImplementCloneable extends AbstractJavaRule { +public class CloneMethodMustImplementCloneableRule extends AbstractJavaRule { @Override public Object visit(final ASTClassOrInterfaceDeclaration node, final Object data) { diff --git a/pmd-java/src/main/resources/rulesets/java/clone.xml b/pmd-java/src/main/resources/rulesets/java/clone.xml index 01350d2b5d..1d8dced5d6 100644 --- a/pmd-java/src/main/resources/rulesets/java/clone.xml +++ b/pmd-java/src/main/resources/rulesets/java/clone.xml @@ -90,32 +90,16 @@ public class MyClass implements Cloneable{ language="java" since="1.9" message="clone() method should be implemented only if implementing Cloneable interface" - class="net.sourceforge.pmd.lang.rule.XPathRule" + class="net.sourceforge.pmd.lang.java.rule.clone.CloneMethodMustImplementCloneableRule" + typeResolution="true" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_clone.html#clonemethodmustimplementcloneable"> -The method clone() should only be implemented if the class implements the Cloneable interface with the exception of a final method that only throws CloneNotSupportedException. +The method clone() should only be implemented if the class implements the Cloneable interface with the exception of +a final method that only throws CloneNotSupportedException. + +The rule can also detect, if the class implements or extends a Cloneable class. 3 - - - - - - - - - - -The method clone() should only be implemented if the class implements the Cloneable interface with the exception -of a final method that only throws CloneNotSupportedException. This version uses PMD's type resolution facilities, -and can detect if the class implements or extends a Cloneable class. - - 3 - - - - + + 0 + + + + + 0 + + + + + 0 + + + + + 0 + + + + 0 @@ -83,6 +129,38 @@ public final class Foo { public Object clone() throws CloneNotSupportedException { throw new CloneNotSupportedException(); } +} + ]]> + + + + 0 + + + + + 0 + @@ -104,4 +182,21 @@ public class UnmodifiableList implements @Readonly List<@Readonly T> {} } ]]> + + + #1532 [java] CloneMethodMustImplementCloneable: Implemented Interface extends Cloneable + 0 + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/CloneMethodMustImplementCloneable.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/CloneMethodMustImplementCloneable.xml deleted file mode 100644 index 391b3457c9..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/CloneMethodMustImplementCloneable.xml +++ /dev/null @@ -1,202 +0,0 @@ - - - - - 0 - - - - - 1 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - #1534 [java] CloneMethodMustImplementCloneable: ClassCastException with Annotation (java8) - 0 - implements @Readonly List<@Readonly T> {} - ]]> - - - - #1532 [java] CloneMethodMustImplementCloneable: Implemented Interface extends Cloneable - part 1: interface - 0 - - - - - #1532 [java] CloneMethodMustImplementCloneable: Implemented Interface extends Cloneable - 0 - - - From 8470d3232205331e75160e3669d14fc1c542bd56 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 13 Oct 2017 12:54:26 +0200 Subject: [PATCH 06/62] [java] Move rule UnusedImports from typeresolution to imports Replace existing rule with the typeresolution-based implementation. --- docs/pages/release_notes.md | 11 +- .../pmd/RuleSetFactoryCompatibility.java | 1 + .../java/rule/imports/UnusedImportsRule.java | 42 ++- .../rules/imports/UnusedImports.java | 53 ---- .../main/resources/rulesets/java/imports.xml | 9 +- .../rulesets/java/typeresolution.xml | 20 +- .../typeresolution/UnusedImportsTest.java | 17 -- .../java/rule/imports/xml/UnusedImports.xml | 68 ++++- .../rule/typeresolution/xml/UnusedImports.xml | 285 ------------------ 9 files changed, 107 insertions(+), 399 deletions(-) delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/imports/UnusedImports.java delete mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/UnusedImportsTest.java delete mode 100644 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/UnusedImports.xml diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index e4e910ed90..81874dc828 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -177,16 +177,23 @@ Notice this last scenario is slightly different to the Java syntax. This is due * The rule `CloneMethodMustImplementCloneable` (ruleset `java-clone`) has been replaced by the typeresolution-based implementation and is now able to detect cases if a class implements or extends a Cloneable class/interface. +* The rule `UnusedImports` (ruleset `java-imports`) has been replaced by the typeresolution-based + implementation and is now able to detect unused on-demand imports. + #### Deprecated Rules * The rules `NcssConstructorCount`, `NcssMethodCount`, and `NcssTypeCount` (ruleset `java-codesize`) have been deprecated. They will be replaced by the new rule `NcssCount` in the same ruleset. -* The rule `LooseCoupling` in ruleset `java-typeresolution` is deprecated. Use the rule with the same name from ruleset `java-coupling` instead. +* The rule `LooseCoupling` in ruleset `java-typeresolution` is deprecated. Use the rule with the same name + from ruleset `java-coupling` instead. -* The rule `CloneMethodMustImplementCloneable` in ruleset `java-typeresolution`is deprecated. Use the rule with +* The rule `CloneMethodMustImplementCloneable` in ruleset `java-typeresolution` is deprecated. Use the rule with the same name from ruleset `java-clone` instead. +* The rule `UnusedImports` in ruleset `java-typeresolution` is deprecated. Use the rule with + the same name from ruleset `java-imports` instead. + #### Removed Rules * The deprecated rule `UseSingleton` has been removed from the ruleset `java-design`. The rule has been renamed diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java index fd0257c1b4..5754de36c3 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java @@ -73,6 +73,7 @@ public class RuleSetFactoryCompatibility { addFilterRuleRenamed("java", "unnecessary", "UnnecessaryParentheses", "UselessParentheses"); addFilterRuleMoved("java", "typeresolution", "coupling", "LooseCoupling"); addFilterRuleMoved("java", "typeresolution", "clone", "CloneMethodMustImplementCloneable"); + addFilterRuleMoved("java", "typeresolution", "imports", "UnusedImports"); } void addFilterRuleRenamed(String language, String ruleset, String oldName, String newName) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnusedImportsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnusedImportsRule.java index 419f8962c8..8a9437b261 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnusedImportsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnusedImportsRule.java @@ -5,6 +5,7 @@ package net.sourceforge.pmd.lang.java.rule.imports; import java.util.HashSet; +import java.util.Iterator; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -18,6 +19,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTPackageDeclaration; import net.sourceforge.pmd.lang.java.ast.Comment; import net.sourceforge.pmd.lang.java.ast.DummyJavaNode; import net.sourceforge.pmd.lang.java.ast.FormalComment; +import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.rule.ImportWrapper; @@ -108,18 +110,22 @@ public class UnusedImportsRule extends AbstractJavaRule { @Override public Object visit(ASTImportDeclaration node, Object data) { - if (!node.isImportOnDemand()) { + if (node.isImportOnDemand()) { ASTName importedType = (ASTName) node.jjtGetChild(0); - String className; - if (isQualifiedName(importedType)) { - int lastDot = importedType.getImage().lastIndexOf('.') + 1; - className = importedType.getImage().substring(lastDot); - } else { - className = importedType.getImage(); + imports.add(new ImportWrapper(importedType.getImage(), null, node, node.getType(), node.isStatic())); + } else { + if (!node.isImportOnDemand()) { + ASTName importedType = (ASTName) node.jjtGetChild(0); + String className; + if (isQualifiedName(importedType)) { + int lastDot = importedType.getImage().lastIndexOf('.') + 1; + className = importedType.getImage().substring(lastDot); + } else { + className = importedType.getImage(); + } + imports.add(new ImportWrapper(importedType.getImage(), className, node)); } - imports.add(new ImportWrapper(importedType.getImage(), className, node)); } - return data; } @@ -140,8 +146,22 @@ public class UnusedImportsRule extends AbstractJavaRule { return; } ImportWrapper candidate = getImportWrapper(node); - if (imports.contains(candidate)) { - imports.remove(candidate); + Iterator it = imports.iterator(); + while (it.hasNext()) { + ImportWrapper i = it.next(); + if (i.matches(candidate)) { + it.remove(); + return; + } + } + if (TypeNode.class.isAssignableFrom(node.getClass()) && ((TypeNode) node).getType() != null) { + Class c = ((TypeNode) node).getType(); + if (c.getPackage() != null) { + candidate = new ImportWrapper(c.getPackage().getName(), null); + if (imports.contains(candidate)) { + imports.remove(candidate); + } + } } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/imports/UnusedImports.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/imports/UnusedImports.java deleted file mode 100644 index 23f7b23cef..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/imports/UnusedImports.java +++ /dev/null @@ -1,53 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.typeresolution.rules.imports; - -import java.util.Iterator; - -import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTName; -import net.sourceforge.pmd.lang.java.ast.TypeNode; -import net.sourceforge.pmd.lang.java.rule.imports.UnusedImportsRule; -import net.sourceforge.pmd.lang.rule.ImportWrapper; - -public class UnusedImports extends UnusedImportsRule { - - @Override - public Object visit(ASTImportDeclaration node, Object data) { - if (node.isImportOnDemand()) { - ASTName importedType = (ASTName) node.jjtGetChild(0); - imports.add(new ImportWrapper(importedType.getImage(), null, node, node.getType(), node.isStatic())); - } else { - super.visit(node, data); - } - return data; - } - - @Override - protected void check(Node node) { - if (imports.isEmpty()) { - return; - } - ImportWrapper candidate = getImportWrapper(node); - Iterator it = imports.iterator(); - while (it.hasNext()) { - ImportWrapper i = it.next(); - if (i.matches(candidate)) { - it.remove(); - return; - } - } - if (TypeNode.class.isAssignableFrom(node.getClass()) && ((TypeNode) node).getType() != null) { - Class c = ((TypeNode) node).getType(); - if (c.getPackage() != null) { - candidate = new ImportWrapper(c.getPackage().getName(), null); - if (imports.contains(candidate)) { - imports.remove(candidate); - } - } - } - } -} diff --git a/pmd-java/src/main/resources/rulesets/java/imports.xml b/pmd-java/src/main/resources/rulesets/java/imports.xml index db98a90186..65504f5548 100644 --- a/pmd-java/src/main/resources/rulesets/java/imports.xml +++ b/pmd-java/src/main/resources/rulesets/java/imports.xml @@ -55,15 +55,18 @@ public class Foo {} since="1.0" message="Avoid unused imports such as ''{0}''" class="net.sourceforge.pmd.lang.java.rule.imports.UnusedImportsRule" + typeResolution="true" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_imports.html#unusedimports"> -Avoid the use of unused import statements to prevent unwanted dependencies. +Avoid unused import statements to prevent unwanted dependencies. +This rule will also find unused on demand imports, i.e. import com.foo.*. 4 diff --git a/pmd-java/src/main/resources/rulesets/java/typeresolution.xml b/pmd-java/src/main/resources/rulesets/java/typeresolution.xml index 5efe0dc0f9..fb457abdc8 100644 --- a/pmd-java/src/main/resources/rulesets/java/typeresolution.xml +++ b/pmd-java/src/main/resources/rulesets/java/typeresolution.xml @@ -11,25 +11,7 @@ These are rules which resolve java Class files for comparison, as opposed to a S - - - -Avoid unused import statements. This rule will find unused on demand imports, i.e. import com.foo.*. - - 4 - - - - + + 0 + x = new ArrayList(); +} + ]]> + java 1.5 + + + 0 @@ -186,7 +201,7 @@ public class Foo { 0 + + #1280 False Positive in UnusedImports when import used in javadoc + 0 + + + + + #348 False Positive UnusedImports with javadoc for public static inner classes of imports + 0 + + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/UnusedImports.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/UnusedImports.xml deleted file mode 100644 index db7edb84b4..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/UnusedImports.xml +++ /dev/null @@ -1,285 +0,0 @@ - - - - - 1 - - - - - 0 - - - - - 2 - - - - - 0 - - - - - 0 - - - - - 0 - - java 1.5 - - - - 0 - x = new ArrayList(); -} - ]]> - java 1.5 - - - - 0 - - java 1.5 - - - - 0 - - java 1.5 - - - - 0 - - - - - 1 - - - - - 0 - - - - - 0 - - - - #1280 False Positive in UnusedImports when import used in javadoc - 0 - - - - #914 False +ve from UnusedImports with wildcard static imports - 0 - - - - - #1465 False Positve UnusedImports with javadoc @link - 0 - - * An agent is active if it has not posted a {@link AgentStateChangeEvent} containing {@link AgentState#TERMINATED}. - * - * @return agent handles. - * @see OtherState#TERMINATED - */ - Iterable getAgentHandles(); -} - ]]> - - - - #1547 False Positve UnusedImports with javadoc for identifiers with underscores - 0 - - - - #348 False Positive UnusedImports with javadoc for public static inner classes of imports - 0 - - - From 944b2f448bfaa188aedb4b6df8629097a75ee57a Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 13 Oct 2017 14:04:27 +0200 Subject: [PATCH 07/62] [java] Move rule SignatureDeclareThrowsException from typeresolution to strictexception Replace existing rule with the typeresolution-based implementation. --- docs/pages/release_notes.md | 7 + .../pmd/RuleSetFactoryCompatibility.java | 1 + .../SignatureDeclareThrowsExceptionRule.java | 120 +++++++++-- .../SignatureDeclareThrowsException.java | 195 ------------------ .../rulesets/java/strictexception.xml | 9 +- .../rulesets/java/typeresolution.xml | 30 +-- .../xml => strictexception}/MyInterface.java | 2 +- .../xml => strictexception}/MyTestCase.java | 6 +- .../SignatureDeclareThrowsExceptionTest.java | 17 -- .../xml/SignatureDeclareThrowsException.xml | 98 +++++++++ .../xml/SignatureDeclareThrowsException.xml | 169 --------------- 11 files changed, 224 insertions(+), 430 deletions(-) delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/SignatureDeclareThrowsException.java rename pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/{typeresolution/xml => strictexception}/MyInterface.java (79%) rename pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/{typeresolution/xml => strictexception}/MyTestCase.java (72%) delete mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/SignatureDeclareThrowsExceptionTest.java delete mode 100644 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/SignatureDeclareThrowsException.xml diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 81874dc828..d608b2bf1c 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -180,6 +180,10 @@ Notice this last scenario is slightly different to the Java syntax. This is due * The rule `UnusedImports` (ruleset `java-imports`) has been replaced by the typeresolution-based implementation and is now able to detect unused on-demand imports. +* The rule `SignatureDeclareThrowsException` (ruleset 'java-strictexception') has been replaced by the + typeresolution-based implementation. It has a new property `IgnoreJUnitCompletely`, which allows all + methods in a JUnit testcase to throws exceptions. + #### Deprecated Rules * The rules `NcssConstructorCount`, `NcssMethodCount`, and `NcssTypeCount` (ruleset `java-codesize`) have been @@ -194,6 +198,9 @@ Notice this last scenario is slightly different to the Java syntax. This is due * The rule `UnusedImports` in ruleset `java-typeresolution` is deprecated. Use the rule with the same name from ruleset `java-imports` instead. +* The rule `SignatureDeclareThrowsException` in ruleset `java-typeresolution` is deprecated. Use the rule + with the same name from ruleset `java-strictexception` instead. + #### Removed Rules * The deprecated rule `UseSingleton` has been removed from the ruleset `java-design`. The rule has been renamed diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java index 5754de36c3..6ba101a194 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java @@ -74,6 +74,7 @@ public class RuleSetFactoryCompatibility { addFilterRuleMoved("java", "typeresolution", "coupling", "LooseCoupling"); addFilterRuleMoved("java", "typeresolution", "clone", "CloneMethodMustImplementCloneable"); addFilterRuleMoved("java", "typeresolution", "imports", "UnusedImports"); + addFilterRuleMoved("java", "typeresolution", "strictexception", "SignatureDeclareThrowsException"); } void addFilterRuleRenamed(String language, String ruleset, String oldName, String newName) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strictexception/SignatureDeclareThrowsExceptionRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strictexception/SignatureDeclareThrowsExceptionRule.java index 8b22268cce..24873fb7d4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strictexception/SignatureDeclareThrowsExceptionRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strictexception/SignatureDeclareThrowsExceptionRule.java @@ -9,15 +9,27 @@ import java.util.List; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTExtendsList; +import net.sourceforge.pmd.lang.java.ast.ASTImplementsList; import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTNameList; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.properties.BooleanProperty; /** + * A method/constructor shouldn't explicitly throw java.lang.Exception, since it + * is unclear which exceptions that can be thrown from the methods. It might be + * difficult to document and understand such vague interfaces. Use either a class + * derived from RuntimeException or a checked exception. + * + *

This rule uses PMD's type resolution facilities, and can detect + * if the class implements or extends TestCase class * * @author Trond Andersen * @version 1.0 @@ -26,7 +38,15 @@ import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; public class SignatureDeclareThrowsExceptionRule extends AbstractJavaRule { - private boolean junitImported; + private static final BooleanProperty IGNORE_JUNIT_COMPLETELY_DESCRIPTOR = new BooleanProperty( + "IgnoreJUnitCompletely", "Allow all methods in a JUnit testcase to throw Exceptions", false, 1.0f); + + // Set to true when the class is determined to be a JUnit testcase + private boolean junitImported = false; + + public SignatureDeclareThrowsExceptionRule() { + definePropertyDescriptor(IGNORE_JUNIT_COMPLETELY_DESCRIPTOR); + } @Override public Object visit(ASTCompilationUnit node, Object o) { @@ -34,6 +54,64 @@ public class SignatureDeclareThrowsExceptionRule extends AbstractJavaRule { return super.visit(node, o); } + @Override + public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { + if (junitImported) { + return super.visit(node, data); + } + + ASTImplementsList impl = node.getFirstChildOfType(ASTImplementsList.class); + if (impl != null && impl.jjtGetParent().equals(node)) { + for (int ix = 0; ix < impl.jjtGetNumChildren(); ix++) { + Node child = impl.jjtGetChild(ix); + + if (child.getClass() != ASTClassOrInterfaceType.class) { + continue; + } + + ASTClassOrInterfaceType type = (ASTClassOrInterfaceType) child; + if (isJUnitTest(type)) { + junitImported = true; + return super.visit(node, data); + } + } + } + if (node.jjtGetNumChildren() != 0 && node.jjtGetChild(0) instanceof ASTExtendsList) { + ASTClassOrInterfaceType type = (ASTClassOrInterfaceType) node.jjtGetChild(0).jjtGetChild(0); + if (isJUnitTest(type)) { + junitImported = true; + return super.visit(node, data); + } + } + + return super.visit(node, data); + } + + private boolean isJUnitTest(ASTClassOrInterfaceType type) { + Class clazz = type.getType(); + if (clazz == null) { + if ("junit.framework.Test".equals(type.getImage())) { + return true; + } + } else if (isJUnitTest(clazz)) { + return true; + } else { + while (clazz != null && !Object.class.equals(clazz)) { + for (Class intf : clazz.getInterfaces()) { + if (isJUnitTest(intf)) { + return true; + } + } + clazz = clazz.getSuperclass(); + } + } + return false; + } + + private boolean isJUnitTest(Class clazz) { + return clazz.getName().equals("junit.framework.Test"); + } + @Override public Object visit(ASTImportDeclaration node, Object o) { if (node.getImportedName().indexOf("junit") != -1) { @@ -44,8 +122,7 @@ public class SignatureDeclareThrowsExceptionRule extends AbstractJavaRule { @Override public Object visit(ASTMethodDeclaration methodDeclaration, Object o) { - if ((methodDeclaration.getMethodName().equals("setUp") || methodDeclaration.getMethodName().equals("tearDown")) - && junitImported) { + if (junitImported && isAllowedMethod(methodDeclaration)) { return super.visit(methodDeclaration, o); } @@ -62,24 +139,39 @@ public class SignatureDeclareThrowsExceptionRule extends AbstractJavaRule { } } + checkExceptions(methodDeclaration, o); + + return super.visit(methodDeclaration, o); + } + + private boolean isAllowedMethod(ASTMethodDeclaration methodDeclaration) { + if (getProperty(IGNORE_JUNIT_COMPLETELY_DESCRIPTOR)) { + return true; + } else { + return methodDeclaration.getMethodName().equals("setUp") + || methodDeclaration.getMethodName().equals("tearDown"); + } + } + + @Override + public Object visit(ASTConstructorDeclaration constructorDeclaration, Object o) { + checkExceptions(constructorDeclaration, o); + + return super.visit(constructorDeclaration, o); + } + + /** + * Search the list of thrown exceptions for Exception + */ + private void checkExceptions(Node method, Object o) { List exceptionList = Collections.emptyList(); - ASTNameList nameList = methodDeclaration.getFirstChildOfType(ASTNameList.class); + ASTNameList nameList = method.getFirstChildOfType(ASTNameList.class); if (nameList != null) { exceptionList = nameList.findDescendantsOfType(ASTName.class); } if (!exceptionList.isEmpty()) { evaluateExceptions(exceptionList, o); } - return super.visit(methodDeclaration, o); - } - - @Override - public Object visit(ASTConstructorDeclaration constructorDeclaration, Object o) { - List exceptionList = constructorDeclaration.findDescendantsOfType(ASTName.class); - if (!exceptionList.isEmpty()) { - evaluateExceptions(exceptionList, o); - } - return super.visit(constructorDeclaration, o); } /** diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/SignatureDeclareThrowsException.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/SignatureDeclareThrowsException.java deleted file mode 100644 index e63f304285..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/rules/SignatureDeclareThrowsException.java +++ /dev/null @@ -1,195 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.typeresolution.rules; - -import java.util.List; - -import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; -import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; -import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTExtendsList; -import net.sourceforge.pmd.lang.java.ast.ASTImplementsList; -import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTName; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; -import net.sourceforge.pmd.properties.BooleanProperty; - -/** - * A method/constructor shouldn't explicitly throw java.lang.Exception, since it - * is unclear which exceptions that can be thrown from the methods. It might be - * difficult to document and understand the vague interfaces. Use either a class - * derived from RuntimeException or a checked exception. This version uses PMD's - * type resolution facilities, and can detect if the class implements or extends - * TestCase class - * - * @author Trond Andersen - * @author acaplan - * @author Wouter Zelle - */ -public class SignatureDeclareThrowsException extends AbstractJavaRule { - - private static final BooleanProperty IGNORE_JUNIT_COMPLETELY_DESCRIPTOR = new BooleanProperty( - "IgnoreJUnitCompletely", "Allow all methods in a JUnit testcase to throw Exceptions", false, 1.0f); - - // Set to true when the class is determined to be a JUnit testcase - private boolean junitImported = false; - - public SignatureDeclareThrowsException() { - definePropertyDescriptor(IGNORE_JUNIT_COMPLETELY_DESCRIPTOR); - } - - @Override - public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { - if (junitImported) { - return super.visit(node, data); - } - - ASTImplementsList impl = node.getFirstChildOfType(ASTImplementsList.class); - if (impl != null && impl.jjtGetParent().equals(node)) { - for (int ix = 0; ix < impl.jjtGetNumChildren(); ix++) { - Node child = impl.jjtGetChild(ix); - - if (child.getClass() != ASTClassOrInterfaceType.class) { - continue; - } - - ASTClassOrInterfaceType type = (ASTClassOrInterfaceType) child; - if (isJUnitTest(type)) { - junitImported = true; - return super.visit(node, data); - } - } - } - if (node.jjtGetNumChildren() != 0 && node.jjtGetChild(0) instanceof ASTExtendsList) { - ASTClassOrInterfaceType type = (ASTClassOrInterfaceType) node.jjtGetChild(0).jjtGetChild(0); - if (isJUnitTest(type)) { - junitImported = true; - return super.visit(node, data); - } - } - - return super.visit(node, data); - } - - private boolean isJUnitTest(ASTClassOrInterfaceType type) { - Class clazz = type.getType(); - if (clazz == null) { - if ("junit.framework.Test".equals(type.getImage())) { - return true; - } - } else if (isJUnitTest(clazz)) { - return true; - } else { - while (clazz != null && !Object.class.equals(clazz)) { - for (Class intf : clazz.getInterfaces()) { - if (isJUnitTest(intf)) { - return true; - } - } - clazz = clazz.getSuperclass(); - } - } - return false; - } - - private boolean isJUnitTest(Class clazz) { - return clazz.getName().equals("junit.framework.Test"); - } - - @Override - public Object visit(ASTImportDeclaration node, Object o) { - if (node.getImportedName().indexOf("junit") != -1) { - junitImported = true; - } - return super.visit(node, o); - } - - @Override - public Object visit(ASTMethodDeclaration methodDeclaration, Object o) { - if (junitImported && isAllowedMethod(methodDeclaration)) { - return super.visit(methodDeclaration, o); - } - - // Ignore overridden methods, the issue should be marked on the method definition - final List methodAnnotations = methodDeclaration.jjtGetParent().findChildrenOfType(ASTAnnotation.class); - for (final ASTAnnotation annotation : methodAnnotations) { - final ASTName annotationName = annotation.getFirstDescendantOfType(ASTName.class); - if (annotationName.hasImageEqualTo("Override") || annotationName.hasImageEqualTo("java.lang.Override")) { - return super.visit(methodDeclaration, o); - } - } - - checkExceptions(methodDeclaration, o); - - return super.visit(methodDeclaration, o); - } - - private boolean isAllowedMethod(ASTMethodDeclaration methodDeclaration) { - if (getProperty(IGNORE_JUNIT_COMPLETELY_DESCRIPTOR)) { - return true; - } else { - return methodDeclaration.getMethodName().equals("setUp") - || methodDeclaration.getMethodName().equals("tearDown"); - } - } - - @Override - public Object visit(ASTConstructorDeclaration constructorDeclaration, Object o) { - checkExceptions(constructorDeclaration, o); - - return super.visit(constructorDeclaration, o); - } - - /** - * Search the list of thrown exceptions for Exception - */ - private void checkExceptions(Node method, Object o) { - List exceptionList = method.findDescendantsOfType(ASTName.class); - if (!exceptionList.isEmpty()) { - evaluateExceptions(exceptionList, o); - } - } - - /** - * Checks all exceptions for possible violation on the exception - * declaration. - * - * @param exceptionList - * containing all exception for declaration - * @param context - */ - private void evaluateExceptions(List exceptionList, Object context) { - for (ASTName exception : exceptionList) { - if (hasDeclaredExceptionInSignature(exception)) { - addViolation(context, exception); - } - } - } - - /** - * Checks if the given value is defined as Exception and the - * parent is either a method or constructor declaration. - * - * @param exception - * to evaluate - * @return true if Exception is declared and has proper parents - */ - private boolean hasDeclaredExceptionInSignature(ASTName exception) { - return exception.hasImageEqualTo("Exception") && isParentSignatureDeclaration(exception); - } - - /** - * @param exception - * to evaluate - * @return true if parent node is either a method or constructor declaration - */ - private boolean isParentSignatureDeclaration(ASTName exception) { - Node parent = exception.jjtGetParent().jjtGetParent(); - return parent instanceof ASTMethodDeclaration || parent instanceof ASTConstructorDeclaration; - } -} diff --git a/pmd-java/src/main/resources/rulesets/java/strictexception.xml b/pmd-java/src/main/resources/rulesets/java/strictexception.xml index 7efab2baf1..f80c074abc 100644 --- a/pmd-java/src/main/resources/rulesets/java/strictexception.xml +++ b/pmd-java/src/main/resources/rulesets/java/strictexception.xml @@ -33,12 +33,15 @@ public void bar() { -Methods that declare the generic Exception as a possible throwable are not very helpful since their -failure modes are unclear. Use a class derived from RuntimeException or a more specific checked exception. +A method/constructor shouldn't explicitly throw the generic java.lang.Exception, since it +is unclear which exceptions that can be thrown from the methods. It might be +difficult to document and understand such vague interfaces. Use either a class +derived from RuntimeException or a checked exception. 3 diff --git a/pmd-java/src/main/resources/rulesets/java/typeresolution.xml b/pmd-java/src/main/resources/rulesets/java/typeresolution.xml index fb457abdc8..d4c8f9b29b 100644 --- a/pmd-java/src/main/resources/rulesets/java/typeresolution.xml +++ b/pmd-java/src/main/resources/rulesets/java/typeresolution.xml @@ -6,37 +6,13 @@ xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd"> -These are rules which resolve java Class files for comparison, as opposed to a String +All rules in this ruleset have been moved to other rulesets. Please use the other rules +directly and don't use this ruleset anymore. - - - -It is unclear which exceptions that can be thrown from the methods. -It might be difficult to document and understand the vague interfaces. -Use either a class derived from RuntimeException or a checked exception. - -JUnit classes are excluded. - - 3 - - - - - - - + diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/MyInterface.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/MyInterface.java similarity index 79% rename from pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/MyInterface.java rename to pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/MyInterface.java index 179be18d62..36143f2ad8 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/MyInterface.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/MyInterface.java @@ -2,7 +2,7 @@ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ -package net.sourceforge.pmd.lang.java.rule.typeresolution.xml; +package net.sourceforge.pmd.lang.java.rule.strictexception; /** * Warning, this class ARE not useless. It is used by the some of regression diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/MyTestCase.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/MyTestCase.java similarity index 72% rename from pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/MyTestCase.java rename to pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/MyTestCase.java index 24847e5172..32d2a096cf 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/MyTestCase.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/MyTestCase.java @@ -2,7 +2,7 @@ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ -package net.sourceforge.pmd.lang.java.rule.typeresolution.xml; +package net.sourceforge.pmd.lang.java.rule.strictexception; import org.junit.Ignore; @@ -12,10 +12,8 @@ import junit.framework.TestCase; * Warning, this class IS NOT useless. It is used by the some regression tests. * * See file: SignatureDeclareThrowsException.xml - * - * The file is already excluded from maven/surefire. */ -@Ignore +@Ignore("not a test case") public class MyTestCase extends TestCase { } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/SignatureDeclareThrowsExceptionTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/SignatureDeclareThrowsExceptionTest.java deleted file mode 100644 index f38e0598d7..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/SignatureDeclareThrowsExceptionTest.java +++ /dev/null @@ -1,17 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.typeresolution; - -import net.sourceforge.pmd.testframework.SimpleAggregatorTst; - -public class SignatureDeclareThrowsExceptionTest extends SimpleAggregatorTst { - - private static final String RULESET = "java-typeresolution"; - - @Override - public void setUp() { - addRule(RULESET, "SignatureDeclareThrowsException"); - } -} diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/strictexception/xml/SignatureDeclareThrowsException.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/strictexception/xml/SignatureDeclareThrowsException.xml index cef45981b4..909ebf7dce 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/strictexception/xml/SignatureDeclareThrowsException.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/strictexception/xml/SignatureDeclareThrowsException.xml @@ -62,6 +62,104 @@ public class Foo { + 0 + + + + + 0 + + + + + 1 + + + + + true + 0 + + + + + 0 + + + + + 0 + + + + + 0 + + + + + 0 + + + + 0 diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/SignatureDeclareThrowsException.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/SignatureDeclareThrowsException.xml deleted file mode 100644 index 0e273096b8..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/SignatureDeclareThrowsException.xml +++ /dev/null @@ -1,169 +0,0 @@ - - - - - 1 - - - - - 0 - - - - - 1 - - - - - 0 - - - - - 0 - - - - - 1 - - - - - true - 0 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 0 - Bar foo() { /* blah */} -} - ]]> - java 1.5 - - - - #1535 [java] SignatureDeclareThrowsException: ClassCastException with Annotation - 0 - implements @Readonly List<@Readonly T> {} - ]]> - - - - #350 allow throws exception when overriding a method defined elsewhere - 0 - - - From 77968550ef8f9d050f96f56ac1028a51957c8c15 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 13 Oct 2017 14:09:47 +0200 Subject: [PATCH 08/62] [java] Fix unit tests --- .../java/rule/{strictexception => clone}/MyInterface.java | 3 ++- .../rule/clone/xml/CloneMethodMustImplementCloneable.xml | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) rename pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/{strictexception => clone}/MyInterface.java (69%) diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/MyInterface.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/clone/MyInterface.java similarity index 69% rename from pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/MyInterface.java rename to pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/clone/MyInterface.java index 36143f2ad8..301d60cb87 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/MyInterface.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/clone/MyInterface.java @@ -2,11 +2,12 @@ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ -package net.sourceforge.pmd.lang.java.rule.strictexception; +package net.sourceforge.pmd.lang.java.rule.clone; /** * Warning, this class ARE not useless. It is used by the some of regression * tests. + * See file "CloneMethodMustImplementCloneable.xml" */ public interface MyInterface extends Cloneable { diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/clone/xml/CloneMethodMustImplementCloneable.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/clone/xml/CloneMethodMustImplementCloneable.xml index 391b3457c9..0fb1522e9d 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/clone/xml/CloneMethodMustImplementCloneable.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/clone/xml/CloneMethodMustImplementCloneable.xml @@ -66,7 +66,7 @@ ok, implements interface in same package which extends Cloneable ]]> 0 0 0 From 82b219428bfbd92bf843ac07a76577bfb41d7f48 Mon Sep 17 00:00:00 2001 From: Robert Painsi Date: Sat, 14 Oct 2017 15:59:14 +0200 Subject: [PATCH 09/62] [java] Add DoNotExtendJavaLangThrowable rule Copy and pasted code from DoNotExtendJavaLangError rule. Resolves #367 https://github.com/pmd/pmd/issues/367 --- docs/pages/pmd/rules/java.md | 1 + docs/pages/pmd/rules/java/strictexception.md | 26 +++++++++++++- .../rulesets/java/strictexception.xml | 25 +++++++++++++ .../StrictExceptionRulesTest.java | 1 + .../xml/DoNotExtendJavaLangThrowable.xml | 36 +++++++++++++++++++ 5 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/strictexception/xml/DoNotExtendJavaLangThrowable.xml diff --git a/docs/pages/pmd/rules/java.md b/docs/pages/pmd/rules/java.md index 92c8d37e80..d3e3e463c0 100644 --- a/docs/pages/pmd/rules/java.md +++ b/docs/pages/pmd/rules/java.md @@ -326,6 +326,7 @@ List of rulesets and rules contained in each ruleset. * [AvoidThrowingNullPointerException](pmd_rules_java_strictexception.html#avoidthrowingnullpointerexception): Avoid throwing NullPointerExceptions. These are confusing because most people will assume that th... * [AvoidThrowingRawExceptionTypes](pmd_rules_java_strictexception.html#avoidthrowingrawexceptiontypes): Avoid throwing certain exception types. Rather than throw a raw RuntimeException, Throwable,Excep... * [DoNotExtendJavaLangError](pmd_rules_java_strictexception.html#donotextendjavalangerror): Errors are system exceptions. Do not extend them. +* [DoNotExtendJavaLangThrowable](pmd_rules_java_strictexception.html#donotextendjavalangthrowable): Extend Exception or RuntimeException instead of Throwable. * [DoNotThrowExceptionInFinally](pmd_rules_java_strictexception.html#donotthrowexceptioninfinally): Throwing exceptions within a 'finally' block is confusing since they may mask other exceptions or... * [ExceptionAsFlowControl](pmd_rules_java_strictexception.html#exceptionasflowcontrol): Using Exceptions as form of flow control is not recommended as they obscure true exceptions when ... * [SignatureDeclareThrowsException](pmd_rules_java_strictexception.html#signaturedeclarethrowsexception): Methods that declare the generic Exception as a possible throwable are not very helpful since the... diff --git a/docs/pages/pmd/rules/java/strictexception.md b/docs/pages/pmd/rules/java/strictexception.md index bb3ee1265e..fd7ec02504 100644 --- a/docs/pages/pmd/rules/java/strictexception.md +++ b/docs/pages/pmd/rules/java/strictexception.md @@ -5,7 +5,7 @@ permalink: pmd_rules_java_strictexception.html folder: pmd/rules/java sidebaractiveurl: /pmd_rules_java.html editmepath: ../pmd-java/src/main/resources/rulesets/java/strictexception.xml -keywords: Strict Exceptions, AvoidCatchingThrowable, SignatureDeclareThrowsException, ExceptionAsFlowControl, AvoidCatchingNPE, AvoidThrowingRawExceptionTypes, AvoidThrowingNullPointerException, AvoidRethrowingException, DoNotExtendJavaLangError, DoNotThrowExceptionInFinally, AvoidThrowingNewInstanceOfSameException, AvoidCatchingGenericException, AvoidLosingExceptionInformation +keywords: Strict Exceptions, AvoidCatchingThrowable, SignatureDeclareThrowsException, ExceptionAsFlowControl, AvoidCatchingNPE, AvoidThrowingRawExceptionTypes, AvoidThrowingNullPointerException, AvoidRethrowingException, DoNotExtendJavaLangError, DoNotExtendJavaLangThrowable, DoNotThrowExceptionInFinally, AvoidThrowingNewInstanceOfSameException, AvoidCatchingGenericException, AvoidLosingExceptionInformation --- ## AvoidCatchingGenericException @@ -310,6 +310,30 @@ public class Foo extends Error { } ``` +## DoNotExtendJavaLangThrowable + +**Since:** PMD 6.0 + +**Priority:** Medium (3) + +Extend Exception or RuntimeException instead of Throwable. + +``` +//ClassOrInterfaceDeclaration/ExtendsList/ClassOrInterfaceType + [@Image="Throwable" or @Image="java.lang.Throwable"] +``` + +**Example(s):** + +``` java +public class Foo extends Throwable { } +``` + +**Use this rule by referencing it:** +``` xml + +``` + ## DoNotThrowExceptionInFinally **Since:** PMD 4.2 diff --git a/pmd-java/src/main/resources/rulesets/java/strictexception.xml b/pmd-java/src/main/resources/rulesets/java/strictexception.xml index 7efab2baf1..b129dd5718 100644 --- a/pmd-java/src/main/resources/rulesets/java/strictexception.xml +++ b/pmd-java/src/main/resources/rulesets/java/strictexception.xml @@ -243,6 +243,31 @@ public class Foo extends Error { } ]]> + + +Extend Exception or RuntimeException instead of Throwable. + + 3 + + + + + + + + + + + + + + 1 + + + + + 1 + + + + + 0 + + + From d891fe048afe0a748ed7f23a1ccbac4c403d5e5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 15 Oct 2017 22:07:34 +0200 Subject: [PATCH 10/62] Fix javadoc warnings on pmd-core --- .../pmd/properties/DoubleMultiProperty.java | 4 ++-- .../pmd/properties/DoubleProperty.java | 4 ++-- .../pmd/properties/FloatMultiProperty.java | 4 ++-- .../pmd/properties/FloatProperty.java | 4 ++-- .../pmd/properties/IntegerMultiProperty.java | 4 ++-- .../pmd/properties/IntegerProperty.java | 2 +- .../pmd/properties/LongMultiProperty.java | 4 ++-- .../pmd/properties/LongProperty.java | 4 ++-- .../sourceforge/pmd/util/CollectionUtil.java | 9 ++++---- .../net/sourceforge/pmd/util/StringUtil.java | 23 +++++++++---------- 10 files changed, 31 insertions(+), 31 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/DoubleMultiProperty.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/DoubleMultiProperty.java index dd48d54cb1..4b6004e132 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/DoubleMultiProperty.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/DoubleMultiProperty.java @@ -49,7 +49,7 @@ public final class DoubleMultiProperty extends AbstractMultiNumericProperty max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public DoubleMultiProperty(String theName, String theDescription, Double min, Double max, Double[] defaultValues, float theUIOrder) { @@ -74,7 +74,7 @@ public final class DoubleMultiProperty extends AbstractMultiNumericProperty max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public DoubleMultiProperty(String theName, String theDescription, Double min, Double max, List defaultValues, float theUIOrder) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/DoubleProperty.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/DoubleProperty.java index 5a0d374745..1bddf4169a 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/DoubleProperty.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/DoubleProperty.java @@ -46,7 +46,7 @@ public final class DoubleProperty extends AbstractNumericProperty { * @param defaultStr Default value * @param theUIOrder UI order * - * @throws IllegalArgumentException if min > max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds * @deprecated will be removed in 7.0.0 */ public DoubleProperty(String theName, String theDescription, String minStr, String maxStr, String defaultStr, @@ -84,7 +84,7 @@ public final class DoubleProperty extends AbstractNumericProperty { * @param theDefault Default value * @param theUIOrder UI order * - * @throws IllegalArgumentException if min > max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public DoubleProperty(String theName, String theDescription, Double min, Double max, Double theDefault, float theUIOrder) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/FloatMultiProperty.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/FloatMultiProperty.java index c5b60f3ecb..d0d199afbe 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/FloatMultiProperty.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/FloatMultiProperty.java @@ -48,7 +48,7 @@ public final class FloatMultiProperty extends AbstractMultiNumericProperty max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public FloatMultiProperty(String theName, String theDescription, Float min, Float max, Float[] defaultValues, float theUIOrder) { @@ -73,7 +73,7 @@ public final class FloatMultiProperty extends AbstractMultiNumericProperty max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public FloatMultiProperty(String theName, String theDescription, Float min, Float max, List defaultValues, float theUIOrder) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/FloatProperty.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/FloatProperty.java index 17ecfa812c..6657a04c09 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/FloatProperty.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/FloatProperty.java @@ -44,7 +44,7 @@ public final class FloatProperty extends AbstractNumericProperty { * @param defaultStr Default value * @param theUIOrder UI order * - * @throws IllegalArgumentException if min > max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds * @deprecated will be removed in 7.0.0 */ public FloatProperty(String theName, String theDescription, String minStr, String maxStr, String defaultStr, @@ -71,7 +71,7 @@ public final class FloatProperty extends AbstractNumericProperty { * @param theDefault Default value * @param theUIOrder UI order * - * @throws IllegalArgumentException if min > max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public FloatProperty(String theName, String theDescription, Float min, Float max, Float theDefault, float theUIOrder) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/IntegerMultiProperty.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/IntegerMultiProperty.java index 59e9ba298a..55b3d963dc 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/IntegerMultiProperty.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/IntegerMultiProperty.java @@ -48,7 +48,7 @@ public final class IntegerMultiProperty extends AbstractMultiNumericProperty max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public IntegerMultiProperty(String theName, String theDescription, Integer min, Integer max, Integer[] defaultValues, float theUIOrder) { @@ -74,7 +74,7 @@ public final class IntegerMultiProperty extends AbstractMultiNumericProperty max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public IntegerMultiProperty(String theName, String theDescription, Integer min, Integer max, List defaultValues, float theUIOrder) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/IntegerProperty.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/IntegerProperty.java index bc03221c33..ade84c258b 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/IntegerProperty.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/IntegerProperty.java @@ -44,7 +44,7 @@ public final class IntegerProperty extends AbstractNumericProperty { * @param theDefault Default value * @param theUIOrder UI order * - * @throws IllegalArgumentException if min > max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public IntegerProperty(String theName, String theDescription, Integer min, Integer max, Integer theDefault, float theUIOrder) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/LongMultiProperty.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/LongMultiProperty.java index cbc4974600..c0409b8107 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/LongMultiProperty.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/LongMultiProperty.java @@ -47,7 +47,7 @@ public final class LongMultiProperty extends AbstractMultiNumericProperty * @param defaultValues Array of defaults * @param theUIOrder UI order * - * @throws IllegalArgumentException if min > max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public LongMultiProperty(String theName, String theDescription, Long min, Long max, Long[] defaultValues, float theUIOrder) { @@ -72,7 +72,7 @@ public final class LongMultiProperty extends AbstractMultiNumericProperty * @param defaultValues List of defaults * @param theUIOrder UI order * - * @throws IllegalArgumentException if min > max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public LongMultiProperty(String theName, String theDescription, Long min, Long max, List defaultValues, float theUIOrder) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/LongProperty.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/LongProperty.java index 144123953e..63f02ea542 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/LongProperty.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/LongProperty.java @@ -45,7 +45,7 @@ public final class LongProperty extends AbstractNumericProperty { * @param defaultStr Default value * @param theUIOrder UI order * - * @throws IllegalArgumentException if min > max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds * @deprecated will be removed in 7.0.0 */ public LongProperty(String theName, String theDescription, String minStr, String maxStr, String defaultStr, @@ -72,7 +72,7 @@ public final class LongProperty extends AbstractNumericProperty { * @param theDefault Default value * @param theUIOrder UI order * - * @throws IllegalArgumentException if min > max or one of the defaults is not between the bounds + * @throws IllegalArgumentException if {@literal min > max} or one of the defaults is not between the bounds */ public LongProperty(String theName, String theDescription, Long min, Long max, Long theDefault, float theUIOrder) { this(theName, theDescription, min, max, theDefault, theUIOrder, false); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/CollectionUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/CollectionUtil.java index 3bf817ca6a..563bd7743e 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/CollectionUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/CollectionUtil.java @@ -12,6 +12,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; /** @@ -170,7 +171,7 @@ public final class CollectionUtil { * @param otherValue * Object * @return boolean - * @deprecated {@see Objects#deepEquals(Object, Object)} + * @deprecated {@link Objects#deepEquals(Object, Object)} */ @Deprecated public static boolean arraysAreEqual(Object value, Object otherValue) { @@ -192,7 +193,7 @@ public final class CollectionUtil { * @param thatArray * Object[] * @return boolean - * @deprecated {@see Arrays#deepEquals(Object[], Object[])} + * @deprecated {@link Arrays#deepEquals(Object[], Object[])} */ @Deprecated public static boolean valuesAreTransitivelyEqual(Object[] thisArray, Object[] thatArray) { @@ -221,7 +222,7 @@ public final class CollectionUtil { * @param otherValue * Object * @return boolean - * @deprecated {@see Objects#deepEquals(Object, Object)} + * @deprecated {@link Objects#deepEquals(Object, Object)} */ @Deprecated @SuppressWarnings("PMD.CompareObjectsWithEquals") @@ -271,7 +272,7 @@ public final class CollectionUtil { * @param a * @param b * @return boolean - * @deprecated {@see Arrays#deepEquals(Object[], Object[])} + * @deprecated {@link Arrays#deepEquals(Object[], Object[])} */ @Deprecated public static boolean areSemanticEquals(T[] a, T[] b) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java index 57bc4666bc..e96e737d16 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java @@ -52,7 +52,7 @@ public final class StringUtil { * @param prefixes * * @return boolean - * @deprecated {@see StringUtils#startsWithAny(CharSequence, CharSequence...)} + * @deprecated {@link StringUtils#startsWithAny(CharSequence, CharSequence...)} */ @Deprecated public static boolean startsWithAny(String text, String... prefixes) { @@ -112,7 +112,7 @@ public final class StringUtil { * @param value String * * @return boolean - * @deprecated {@see StringUtils#isNotBlank(CharSequence)} + * @deprecated {@link StringUtils#isNotBlank(CharSequence)} */ @Deprecated public static boolean isNotEmpty(String value) { @@ -128,7 +128,7 @@ public final class StringUtil { * @param value String to test * * @return true if the value is empty, false otherwise. - * @deprecated {@see StringUtils#isBlank(CharSequence)} + * @deprecated {@link StringUtils#isBlank(CharSequence)} */ @Deprecated public static boolean isEmpty(String value) { @@ -142,7 +142,7 @@ public final class StringUtil { * @param value String to test * * @return True if the argument is null or the empty string - * @deprecated {@see StringUtils#isEmpty(CharSequence)} + * @deprecated {@link StringUtils#isEmpty(CharSequence)} */ @Deprecated public static boolean isMissing(String value) { @@ -179,7 +179,7 @@ public final class StringUtil { * @param newString String * * @return String - * @deprecated {@see StringUtils#replace(String, String, String)} + * @deprecated {@link StringUtils#replace(String, String, String)} */ @Deprecated public static String replaceString(final String original, final String oldString, final String newString) { @@ -207,7 +207,6 @@ public final class StringUtil { * @param supportUTF8 override the default setting, whether special characters should be replaced with entities ( * false) or should be included as is ( true). * - * @see #appendXmlEscaped(StringBuilder, String) */ public static void appendXmlEscaped(StringBuilder buf, String src, boolean supportUTF8) { char c; @@ -267,7 +266,7 @@ public final class StringUtil { * @param newString String * * @return String - * @deprecated {@see StringUtils#replace(String, String, String)} or {@see StringUtils#replaceChars(String, char, char)} + * @deprecated {@link StringUtils#replace(String, String, String)} or {@link StringUtils#replaceChars(String, char, char)} */ @Deprecated public static String replaceString(final String original, char oldChar, final String newString) { @@ -300,7 +299,7 @@ public final class StringUtil { * @param delimiter char * * @return String[] - * @deprecated {@see StringUtils#split(String, char)} + * @deprecated {@link StringUtils#split(String, char)} */ @Deprecated public static String[] substringsOf(String source, char delimiter) { @@ -348,7 +347,7 @@ public final class StringUtil { * @param separator char * * @return String[] - * @deprecated {@see StringUtils#split(String, String)} + * @deprecated {@link StringUtils#split(String, String)} */ @Deprecated public static String[] substringsOf(String str, String separator) { @@ -382,7 +381,7 @@ public final class StringUtil { * @param sb StringBuffer * @param iter Iterator * @param separator String - * @deprecated {@see StringUtils#join(Iterator, String)} + * @deprecated {@link StringUtils#join(Iterator, String)} */ @Deprecated public static void asStringOn(StringBuffer sb, Iterator iter, String separator) { @@ -407,7 +406,7 @@ public final class StringUtil { * @param sb StringBuilder * @param items Object[] * @param separator String - * @deprecated {@see StringUtils#join(Iterable, String)} + * @deprecated {@link StringUtils#join(Iterable, String)} */ @Deprecated public static void asStringOn(StringBuilder sb, Object[] items, String separator) { @@ -519,7 +518,7 @@ public final class StringUtil { * @param length The desired minimum length of the resulting padded String * * @return The resulting left padded String - * @deprecated {@see StringUtils#leftPad(String, int)} + * @deprecated {@link StringUtils#leftPad(String, int)} */ @Deprecated public static String lpad(String s, int length) { From 4f2c7960e642e3f1a438162976fa8d510a7323f7 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 16 Oct 2017 21:18:06 +0200 Subject: [PATCH 11/62] Update changelog, refs #668 --- docs/pages/release_notes.md | 1 + .../net/sourceforge/pmd/properties/PropertyDescriptorField.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 14e22053a1..90d41d9996 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -383,4 +383,5 @@ a warning will now be produced suggesting users to adopt it for better performan * [#632](https://github.com/pmd/pmd/pull/632): \[apex] Add AvoidDirectAccessTriggerMap rule to the style set - [Jan Aertgeerts](https://github.com/JAertgeerts) * [#644](https://github.com/pmd/pmd/pull/644): \[core] Prevent internal dev-properties from being displayed on CodeClimate renderer - [Filipe Esperandio](https://github.com/filipesperandio) * [#660](https://github.com/pmd/pmd/pull/660): \[apex] avoid sosl in loops - [Jan Aertgeerts](https://github.com/JAertgeerts) +* [#668](https://github.com/pmd/pmd/pull/668): \[core] Fix javadoc warnings on pmd-core - [Clément Fournier](https://github.com/oowekyala) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorField.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorField.java index 745771bdb2..268db660f5 100755 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorField.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/PropertyDescriptorField.java @@ -32,7 +32,7 @@ public enum PropertyDescriptorField { MAX("max"), /** * To limit the range of valid values, package names. - * @see PackagedPro + * @see PackagedPropertyDescriptor */ LEGAL_PACKAGES("legalPackages"), /** Labels for enumerated properties. */ From ffcf5cb704149efbb826abcc72903c39ed68ad0b Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 16 Oct 2017 20:47:09 +0200 Subject: [PATCH 12/62] [apex] Remove shading for the apex modules - not needed anymore Fixes #605. The apex modules do not contain any non-apex code anymore (not an uber-jar anymore). Explicitely defining asm as runtime dependency. --- pmd-apex/pom.xml | 43 +++++------------------------- pmd-dist/pom.xml | 6 ----- pmd-dist/src/main/assembly/bin.xml | 4 --- 3 files changed, 6 insertions(+), 47 deletions(-) diff --git a/pmd-apex/pom.xml b/pmd-apex/pom.xml index 5e5394daf1..a504ffc2d2 100644 --- a/pmd-apex/pom.xml +++ b/pmd-apex/pom.xml @@ -59,43 +59,6 @@ - - - org.apache.maven.plugins - maven-shade-plugin - - - package - - shade - - - - - apex - - - ${project.groupId} - - - - - - org.objectweb.asm - shaded.org.objectweb.asm - - - - com.fasterxml.jackson - shaded.com.fasterxml.jackson - - - true - apex-jorje-shaded - - - - @@ -167,6 +130,12 @@ saxon + + org.ow2.asm + asm + runtime + + junit junit diff --git a/pmd-dist/pom.xml b/pmd-dist/pom.xml index 85d183f441..cd98712317 100644 --- a/pmd-dist/pom.xml +++ b/pmd-dist/pom.xml @@ -207,12 +207,6 @@ pmd-apex ${project.version} - - net.sourceforge.pmd - pmd-apex - ${project.version} - apex-jorje-shaded - net.sourceforge.pmd pmd-ui diff --git a/pmd-dist/src/main/assembly/bin.xml b/pmd-dist/src/main/assembly/bin.xml index 153f25e2b9..2b44241aa9 100644 --- a/pmd-dist/src/main/assembly/bin.xml +++ b/pmd-dist/src/main/assembly/bin.xml @@ -52,10 +52,6 @@ 0755 0644 false - - - apex:* - From 2b60ecaead7b85a91791484db50fec30e6e7156f Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 16 Oct 2017 21:11:38 +0200 Subject: [PATCH 13/62] Update release notes, refs #605 --- 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 90d41d9996..0497c73b62 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -233,6 +233,7 @@ a warning will now be produced suggesting users to adopt it for better performan * [#488](https://github.com/pmd/pmd/pull/488): \[apex] Use Apex lexer for CPD * [#489](https://github.com/pmd/pmd/pull/489): \[apex] Update Apex compiler * [#500](https://github.com/pmd/pmd/issues/500): \[apex] Running through CLI shows jorje optimization messages + * [#605](https://github.com/pmd/pmd/issues/605): \[apex] java.lang.NoClassDefFoundError in the latest build * [#637](https://github.com/pmd/pmd/issues/637): \[apex] Avoid SOSL in loops * cpp * [#448](https://github.com/pmd/pmd/issues/448): \[cpp] Write custom CharStream to handle continuation characters From 30aa7fca11cdc5fecdb6a768ecdf947d469dee08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 17 Oct 2017 13:35:45 -0300 Subject: [PATCH 14/62] Add DoNotExtendJavaLangThrowable to release ruleset --- pmd-core/src/main/resources/rulesets/releases/600.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pmd-core/src/main/resources/rulesets/releases/600.xml b/pmd-core/src/main/resources/rulesets/releases/600.xml index 878f268efc..59010a3370 100644 --- a/pmd-core/src/main/resources/rulesets/releases/600.xml +++ b/pmd-core/src/main/resources/rulesets/releases/600.xml @@ -10,6 +10,7 @@ This ruleset contains links to rules that are new in PMD v6.0.0 + From 92f1cdd923b85e46c130e101bf61fdecd3b486cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 17 Oct 2017 13:42:28 -0300 Subject: [PATCH 15/62] Update changelog, refs #666 --- docs/pages/release_notes.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 14e22053a1..5751b488e6 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -136,6 +136,9 @@ Notice this last scenario is slightly different to the Java syntax. This is due and "NcssTypeCount". The new rule uses the metrics framework to achieve the same. It has two properties, to define the report level for method and class sizes separately. Constructors and methods are considered the same. +* The new rule `DoNotExtendJavaLangThrowable` (ruleset `java-strictexception`) is a companion for the + `java-strictexception.xml/DoNotExtendJavaLangError`, detecting direct extensions of `java.lang.Throwable`. + * The new rule `ForLoopCanBeForeach` (ruleset `java-migration`) helps to identify those for-loops that can be safely refactored into for-each-loops available since java 1.5. @@ -383,4 +386,5 @@ a warning will now be produced suggesting users to adopt it for better performan * [#632](https://github.com/pmd/pmd/pull/632): \[apex] Add AvoidDirectAccessTriggerMap rule to the style set - [Jan Aertgeerts](https://github.com/JAertgeerts) * [#644](https://github.com/pmd/pmd/pull/644): \[core] Prevent internal dev-properties from being displayed on CodeClimate renderer - [Filipe Esperandio](https://github.com/filipesperandio) * [#660](https://github.com/pmd/pmd/pull/660): \[apex] avoid sosl in loops - [Jan Aertgeerts](https://github.com/JAertgeerts) +* [#666](https://github.com/pmd/pmd/pull/666): \[java] Add DoNotExtendJavaLangThrowable rule - [Robert Painsi](https://github.com/robertpainsi) From 53210842c884bb91b58d2669a10e51d6d9611e53 Mon Sep 17 00:00:00 2001 From: "Travis CI (pmd-bot)" Date: Tue, 17 Oct 2017 16:59:20 +0000 Subject: [PATCH 16/62] Update documentation --- docs/pages/pmd/rules/java.md | 6 ++-- docs/pages/pmd/rules/java/clone.md | 20 ++++---------- docs/pages/pmd/rules/java/coupling.md | 29 ++++++++++++-------- docs/pages/pmd/rules/java/imports.md | 8 ++++-- docs/pages/pmd/rules/java/strictexception.md | 12 ++++++-- 5 files changed, 40 insertions(+), 35 deletions(-) diff --git a/docs/pages/pmd/rules/java.md b/docs/pages/pmd/rules/java.md index d3e3e463c0..7c06662b1c 100644 --- a/docs/pages/pmd/rules/java.md +++ b/docs/pages/pmd/rules/java.md @@ -125,7 +125,7 @@ List of rulesets and rules contained in each ruleset. * [CouplingBetweenObjects](pmd_rules_java_coupling.html#couplingbetweenobjects): This rule counts the number of unique attributes, local variables, and return types within an obj... * [ExcessiveImports](pmd_rules_java_coupling.html#excessiveimports): A high number of imports can indicate a high degree of coupling within an object. This rule count... * [LawOfDemeter](pmd_rules_java_coupling.html#lawofdemeter): The Law of Demeter is a simple rule, that says "only talk to friends". It helps to reduce couplin... -* [LooseCoupling](pmd_rules_java_coupling.html#loosecoupling): The use of implementation types as object references limits your ability to use alternateimplemen... +* [LooseCoupling](pmd_rules_java_coupling.html#loosecoupling): The use of implementation types (i.e., HashSet) as object references limits your ability to use a... * [LoosePackageCoupling](pmd_rules_java_coupling.html#loosepackagecoupling): Avoid using classes from the configured package hierarchy outside of the package hierarchy, excep... ## Design @@ -214,7 +214,7 @@ List of rulesets and rules contained in each ruleset. * [ImportFromSamePackage](pmd_rules_java_imports.html#importfromsamepackage): There is no need to import a type that lives in the same package. * [TooManyStaticImports](pmd_rules_java_imports.html#toomanystaticimports): If you overuse the static import feature, it can make your program unreadable and unmaintainable,... * [UnnecessaryFullyQualifiedName](pmd_rules_java_imports.html#unnecessaryfullyqualifiedname): Import statements allow the use of non-fully qualified names. The use of a fully qualified namew... -* [UnusedImports](pmd_rules_java_imports.html#unusedimports): Avoid the use of unused import statements to prevent unwanted dependencies. +* [UnusedImports](pmd_rules_java_imports.html#unusedimports): Avoid unused import statements to prevent unwanted dependencies.This rule will also find unused o... ## J2EE * [DoNotCallSystemExit](pmd_rules_java_j2ee.html#donotcallsystemexit): Web applications should not call System.exit(), since only the web container or theapplication se... @@ -329,7 +329,7 @@ List of rulesets and rules contained in each ruleset. * [DoNotExtendJavaLangThrowable](pmd_rules_java_strictexception.html#donotextendjavalangthrowable): Extend Exception or RuntimeException instead of Throwable. * [DoNotThrowExceptionInFinally](pmd_rules_java_strictexception.html#donotthrowexceptioninfinally): Throwing exceptions within a 'finally' block is confusing since they may mask other exceptions or... * [ExceptionAsFlowControl](pmd_rules_java_strictexception.html#exceptionasflowcontrol): Using Exceptions as form of flow control is not recommended as they obscure true exceptions when ... -* [SignatureDeclareThrowsException](pmd_rules_java_strictexception.html#signaturedeclarethrowsexception): Methods that declare the generic Exception as a possible throwable are not very helpful since the... +* [SignatureDeclareThrowsException](pmd_rules_java_strictexception.html#signaturedeclarethrowsexception): A method/constructor shouldn't explicitly throw the generic java.lang.Exception, since itis uncle... ## String and StringBuffer * [AppendCharacterWithChar](pmd_rules_java_strings.html#appendcharacterwithchar): Avoid concatenating characters as strings in StringBuffer/StringBuilder.append methods. diff --git a/docs/pages/pmd/rules/java/clone.md b/docs/pages/pmd/rules/java/clone.md index 7091400db3..e3e5654198 100644 --- a/docs/pages/pmd/rules/java/clone.md +++ b/docs/pages/pmd/rules/java/clone.md @@ -54,22 +54,12 @@ public class Foo implements Cloneable { **Priority:** Medium (3) -The method clone() should only be implemented if the class implements the Cloneable interface with the exception of a final method that only throws CloneNotSupportedException. +The method clone() should only be implemented if the class implements the Cloneable interface with the exception of +a final method that only throws CloneNotSupportedException. -``` -//ClassOrInterfaceDeclaration -[not(./ExtendsList/ClassOrInterfaceType[@Image='Cloneable'])] -[not(./ImplementsList/ClassOrInterfaceType[@Image='Cloneable'])] -/ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration -[MethodDeclaration -[MethodDeclarator[@Image -= 'clone' and count(FormalParameters/*) = 0]] -[not((../MethodDeclaration[@Final = 'true'] or ancestor::ClassOrInterfaceDeclaration[1][@Final = 'true']) -and Block[count(BlockStatement)=1] -/BlockStatement/Statement/ThrowStatement/Expression -/PrimaryExpression/PrimaryPrefix/AllocationExpression -/ClassOrInterfaceType[@Image = 'CloneNotSupportedException'])]] -``` +The rule can also detect, if the class implements or extends a Cloneable class. + +**This rule is defined by the following Java class:** [net.sourceforge.pmd.lang.java.rule.clone.CloneMethodMustImplementCloneableRule](https://github.com/pmd/pmd/blob/master/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/clone/CloneMethodMustImplementCloneableRule.java) **Example(s):** diff --git a/docs/pages/pmd/rules/java/coupling.md b/docs/pages/pmd/rules/java/coupling.md index 2139ff4786..f59c6491d5 100644 --- a/docs/pages/pmd/rules/java/coupling.md +++ b/docs/pages/pmd/rules/java/coupling.md @@ -141,27 +141,32 @@ public class Foo { **Priority:** Medium (3) -The use of implementation types as object references limits your ability to use alternate -implementations in the future as requirements change. Whenever available, referencing objects -by their interface types provides much more flexibility. +The use of implementation types (i.e., HashSet) as object references limits your ability to use alternate +implementations in the future as requirements change. Whenever available, referencing objects +by their interface types (i.e, Set) provides much more flexibility. **This rule is defined by the following Java class:** [net.sourceforge.pmd.lang.java.rule.coupling.LooseCouplingRule](https://github.com/pmd/pmd/blob/master/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/coupling/LooseCouplingRule.java) **Example(s):** ``` java -// sub-optimal approach -private ArrayList list = new ArrayList<>(); +import java.util.ArrayList; +import java.util.HashSet; -public HashSet getFoo() { - return new HashSet(); -} +public class Bar { + // sub-optimal approach + private ArrayList list = new ArrayList<>(); -// preferred approach -private List list = new ArrayList<>(); + public HashSet getFoo() { + return new HashSet(); + } -public Set getFoo() { - return new HashSet(); + // preferred approach + private List list = new ArrayList<>(); + + public Set getFoo() { + return new HashSet(); + } } ``` diff --git a/docs/pages/pmd/rules/java/imports.md b/docs/pages/pmd/rules/java/imports.md index d9fb36d2ef..681a7d4060 100644 --- a/docs/pages/pmd/rules/java/imports.md +++ b/docs/pages/pmd/rules/java/imports.md @@ -154,15 +154,17 @@ public class Foo { **Priority:** Medium Low (4) -Avoid the use of unused import statements to prevent unwanted dependencies. +Avoid unused import statements to prevent unwanted dependencies. +This rule will also find unused on demand imports, i.e. import com.foo.*. **This rule is defined by the following Java class:** [net.sourceforge.pmd.lang.java.rule.imports.UnusedImportsRule](https://github.com/pmd/pmd/blob/master/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/imports/UnusedImportsRule.java) **Example(s):** ``` java -// this is bad -import java.io.File; +import java.io.File; // not referenced or required +import java.util.*; // not referenced or required + public class Foo {} ``` diff --git a/docs/pages/pmd/rules/java/strictexception.md b/docs/pages/pmd/rules/java/strictexception.md index fd7ec02504..be524a7194 100644 --- a/docs/pages/pmd/rules/java/strictexception.md +++ b/docs/pages/pmd/rules/java/strictexception.md @@ -408,8 +408,10 @@ public void bar() { **Priority:** Medium (3) -Methods that declare the generic Exception as a possible throwable are not very helpful since their -failure modes are unclear. Use a class derived from RuntimeException or a more specific checked exception. +A method/constructor shouldn't explicitly throw the generic java.lang.Exception, since it +is unclear which exceptions that can be thrown from the methods. It might be +difficult to document and understand such vague interfaces. Use either a class +derived from RuntimeException or a checked exception. **This rule is defined by the following Java class:** [net.sourceforge.pmd.lang.java.rule.strictexception.SignatureDeclareThrowsExceptionRule](https://github.com/pmd/pmd/blob/master/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/strictexception/SignatureDeclareThrowsExceptionRule.java) @@ -420,6 +422,12 @@ public void foo() throws Exception { } ``` +**This rule has the following properties:** + +|Name|Default Value|Description| +|----|-------------|-----------| +|IgnoreJUnitCompletely|false|Allow all methods in a JUnit testcase to throw Exceptions| + **Use this rule by referencing it:** ``` xml From c00400ef77487661fc559ca758568405838b337b Mon Sep 17 00:00:00 2001 From: gonzalo Date: Wed, 18 Oct 2017 08:50:06 -0300 Subject: [PATCH 17/62] Fix try with final resource --- pmd-java/etc/grammar/Java.jjt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 5f15749f5e..28f615cf39 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -2253,7 +2253,7 @@ void Resources() : void Resource() : {} { - LOOKAHEAD(2) ( ( "final" | Annotation() )* Type() VariableDeclaratorId() "=" Expression() ) + LOOKAHEAD(2) ( ( "final" {jjtThis.setFinal(true);} | Annotation() )* Type() VariableDeclaratorId() "=" Expression() ) | Name() {checkForBadConciseTryWithResourcesUsage();} } From fded4f140fdc24d78945a9e5c525cee9f143dc73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 18 Oct 2017 11:45:26 -0300 Subject: [PATCH 18/62] Update changelog, refs #675 --- 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 331df17eb5..6b4ac967b3 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -413,4 +413,5 @@ a warning will now be produced suggesting users to adopt it for better performan * [#660](https://github.com/pmd/pmd/pull/660): \[apex] avoid sosl in loops - [Jan Aertgeerts](https://github.com/JAertgeerts) * [#666](https://github.com/pmd/pmd/pull/666): \[java] Add DoNotExtendJavaLangThrowable rule - [Robert Painsi](https://github.com/robertpainsi) * [#668](https://github.com/pmd/pmd/pull/668): \[core] Fix javadoc warnings on pmd-core - [Clément Fournier](https://github.com/oowekyala) +* [#675](https://github.com/pmd/pmd/pull/675): \[java] Fix in Java grammar: Try with final resource node error - [Gonzalo Ibars Ingman](https://github.com/gibarsin) From e3f9c728d7bec8f81744aff36e5d1357fb36a007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 18 Oct 2017 23:51:01 -0300 Subject: [PATCH 19/62] content update --- .../lang/apex/rule/{security => style}/AvoidHardcodingIdRule.java | 0 .../lang/apex/rule/{security => style}/xml/AvoidHardcodingId.xml | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/{security => style}/AvoidHardcodingIdRule.java (100%) rename pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/{security => style}/xml/AvoidHardcodingId.xml (100%) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/AvoidHardcodingIdRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/style/AvoidHardcodingIdRule.java similarity index 100% rename from pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/AvoidHardcodingIdRule.java rename to pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/style/AvoidHardcodingIdRule.java diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/AvoidHardcodingId.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/style/xml/AvoidHardcodingId.xml similarity index 100% rename from pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/AvoidHardcodingId.xml rename to pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/style/xml/AvoidHardcodingId.xml From 8ef0d6989ca9537a5df6f57cc5c7f5930718bae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 18 Oct 2017 23:53:34 -0300 Subject: [PATCH 20/62] Move AvoidHardcodingIdRule to style ruleset --- .../rule/style/AvoidHardcodingIdRule.java | 4 +-- .../main/resources/rulesets/apex/ruleset.xml | 18 ++++++------- .../main/resources/rulesets/apex/security.xml | 27 ------------------- .../main/resources/rulesets/apex/style.xml | 26 ++++++++++++++++++ .../apex/rule/security/SecurityRulesTest.java | 1 - .../lang/apex/rule/style/StyleRulesTest.java | 2 +- 6 files changed, 38 insertions(+), 40 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/style/AvoidHardcodingIdRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/style/AvoidHardcodingIdRule.java index 97799220ce..4a90ca2c58 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/style/AvoidHardcodingIdRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/style/AvoidHardcodingIdRule.java @@ -2,7 +2,7 @@ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ -package net.sourceforge.pmd.lang.apex.rule.security; +package net.sourceforge.pmd.lang.apex.rule.style; import java.util.regex.Pattern; @@ -13,7 +13,7 @@ public class AvoidHardcodingIdRule extends AbstractApexRule { private static final Pattern PATTERN = Pattern.compile("^[a-zA-Z0-9]{5}[0][a-zA-Z0-9]{9,12}$", Pattern.CASE_INSENSITIVE); public AvoidHardcodingIdRule() { - setProperty(CODECLIMATE_CATEGORIES, "Security"); + setProperty(CODECLIMATE_CATEGORIES, "Style"); setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); } diff --git a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml index 074e35a711..7494211bf2 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml @@ -131,6 +131,15 @@ + + 3 + + + + + + + 3 @@ -287,15 +296,6 @@ - - 3 - - - - - - - 3 diff --git a/pmd-apex/src/main/resources/rulesets/apex/security.xml b/pmd-apex/src/main/resources/rulesets/apex/security.xml index 8dc07c0bca..8f111016ac 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/security.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/security.xml @@ -272,31 +272,4 @@ public class Foo { - - -When deploying Apex code between sandbox and production environments, or installing Force.com AppExchange packages, -it is essential to avoid hardcoding IDs in the Apex code. By doing so, if the record IDs change between environments, -the logic can dynamically identify the proper data to operate against and not fail. - - 3 - - - - - diff --git a/pmd-apex/src/main/resources/rulesets/apex/style.xml b/pmd-apex/src/main/resources/rulesets/apex/style.xml index 7c86b03a41..276580d5b5 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/style.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/style.xml @@ -138,4 +138,30 @@ global class Unchangeable { + + +When deploying Apex code between sandbox and production environments, or installing Force.com AppExchange packages, +it is essential to avoid hardcoding IDs in the Apex code. By doing so, if the record IDs change between environments, +the logic can dynamically identify the proper data to operate against and not fail. + + 3 + + + + diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java index 7d2e8cb89e..703f112d86 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java @@ -23,6 +23,5 @@ public class SecurityRulesTest extends SimpleAggregatorTst { addRule(RULESET, "ApexCRUDViolation"); addRule(RULESET, "ApexDangerousMethods"); addRule(RULESET, "ApexSuggestUsingNamedCred"); - addRule(RULESET, "AvoidHardcodingId"); } } diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/style/StyleRulesTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/style/StyleRulesTest.java index f16234d462..b8125314d4 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/style/StyleRulesTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/style/StyleRulesTest.java @@ -18,6 +18,6 @@ public class StyleRulesTest extends SimpleAggregatorTst { addRule(RULESET, "MethodNamingConventions"); addRule(RULESET, "VariableNamingConventions"); addRule(RULESET, "MethodWithSameNameAsEnclosingClass"); - + addRule(RULESET, "AvoidHardcodingId"); } } From 91bf92fe26cc29821659b08ef33a6386e7252e93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Wed, 18 Oct 2017 23:55:25 -0300 Subject: [PATCH 21/62] Update changelog, refs #661 --- docs/pages/release_notes.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 9db55ecea6..26486b187b 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -105,6 +105,10 @@ and include them to such reports. * The new rule `ForLoopCanBeForeach` (ruleset `java-migration`) helps to identify those for-loops that can be safely refactored into for-each-loops available since java 1.5. +* The new rule `AvoidHardcodingId` (ruleset `apex-style`) detects hardcoded strings that look like identifiers + and flags them. Record IDs change between environments, meaning hardcoded ids are bound to fail under a different + setup. + #### Modified Rules * The rule `UnnecessaryFinalModifier` (ruleset `java-unnecessarycode`) has been revamped to detect more cases. @@ -292,4 +296,5 @@ All existing rules have been updated to reflect these changes. If you have custo * [#588](https://github.com/pmd/pmd/pull/588): \[java] XPath function to compute metrics - [Clément Fournier](https://github.com/oowekyala) * [#598](https://github.com/pmd/pmd/pull/598): \[java] Fix #388: controversial.AvoidLiteralsInIfCondition 0.0 false positive - [Clément Fournier](https://github.com/oowekyala) * [#620](https://github.com/pmd/pmd/pull/620): \[core] Moved properties to n.s.pmd.properties - [Clément Fournier](https://github.com/oowekyala) +* [#661](https://github.com/pmd/pmd/pull/661): \[apex] avoid hardcoding id's - [Jan Aertgeerts](https://github.com/JAertgeerts) From 96bb1f4bc7d5033cc414da95ed23036beae0a8fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Mon, 23 Oct 2017 14:23:05 -0300 Subject: [PATCH 22/62] [core] Support parent-last classloading - We will now always load classes from auxclasspath during typeresolution, even if we have a class with the same name in PMD's classpath - This prevents conflicts when using different versions of the same dependencies --- .../net/sourceforge/pmd/RuleSetFactory.java | 2 +- .../pmd/util/ClasspathClassLoader.java | 25 ++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java index d49b2bbaea..2257cff659 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -543,7 +543,7 @@ public class RuleSetFactory { if (attribute == null || "".equals(attribute)) { throw new IllegalArgumentException("The 'class' field of rule can't be null, nor empty."); } - Rule rule = (Rule) classLoader.loadClass(attribute).newInstance(); + Rule rule = (Rule) RuleSetFactory.class.getClassLoader().loadClass(attribute).newInstance(); rule.setName(ruleElement.getAttribute("name")); if (ruleElement.hasAttribute("language")) { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java index fd9d057e0b..5598e1a86c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java @@ -34,7 +34,7 @@ public class ClasspathClassLoader extends URLClassLoader { static { registerAsParallelCapable(); } - + public ClasspathClassLoader(String classpath, ClassLoader parent) throws IOException { super(initURLs(classpath), parent); } @@ -96,4 +96,27 @@ public class ClasspathClassLoader extends URLClassLoader { .append(StringUtils.join(getURLs(), ":")) .append("] parent: ").append(getParent()).append(']').toString(); } + + @Override + protected Class loadClass(final String name, final boolean resolve) throws ClassNotFoundException { + // First, check if the class has already been loaded + Class c = findLoadedClass(name); + if (c == null) { + if (c == null) { + try { + // checking local + c = findClass(name); + } catch (final ClassNotFoundException e) { + // checking parent + // This call to loadClass may eventually call findClass again, in case the parent doesn't find anything. + c = super.loadClass(name, resolve); + } + } + } + + if (resolve) { + resolveClass(c); + } + return c; + } } From 42d965844f9c30a6f3aafda0866b540366530acb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Mon, 23 Oct 2017 17:46:21 -0300 Subject: [PATCH 23/62] Avoid duplicate code --- .../pmd/util/ClasspathClassLoader.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java index 5598e1a86c..fbc8bb88a3 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java @@ -102,15 +102,13 @@ public class ClasspathClassLoader extends URLClassLoader { // First, check if the class has already been loaded Class c = findLoadedClass(name); if (c == null) { - if (c == null) { - try { - // checking local - c = findClass(name); - } catch (final ClassNotFoundException e) { - // checking parent - // This call to loadClass may eventually call findClass again, in case the parent doesn't find anything. - c = super.loadClass(name, resolve); - } + try { + // checking local + c = findClass(name); + } catch (final ClassNotFoundException e) { + // checking parent + // This call to loadClass may eventually call findClass again, in case the parent doesn't find anything. + c = super.loadClass(name, resolve); } } From 042b1bda0299a28a7fddfb877a5d6015c55f7583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Mon, 23 Oct 2017 17:36:59 -0300 Subject: [PATCH 24/62] [java] Handle properly reduction incompatibilities - When a reduce operation return null (it's incompatible), we should handle it properly instead of producing a NPE. --- .../typeresolution/MethodTypeResolution.java | 12 +- .../typeinference/InferenceRuleType.java | 7 +- .../typeinference/TypeInferenceResolver.java | 5 + .../typeresolution/ClassTypeResolverTest.java | 2 +- .../pmd/typeresolution/TypeInferenceTest.java | 110 +++++++++--------- .../testdata/MethodFirstPhase.java | 8 +- 6 files changed, 82 insertions(+), 62 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java index faa593bf2b..ef2dbd9c42 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java @@ -140,8 +140,14 @@ public final class MethodTypeResolution { } methodType = parameterizeInvocation(context, methodType.getMethod(), argList); + + // May be null if the method call is not applicable + if (methodType == null) { + continue; + } } + // TODO : Is this needed? parameterizeInvocation already performs inference to check applicability... // check subtypeability of each argument to the corresponding parameter boolean methodIsApplicable = true; @@ -166,7 +172,6 @@ public final class MethodTypeResolution { return selectedMethods; } - public static MethodType parameterizeInvocation(JavaTypeDefinition context, Method method, ASTArgumentList argList) { @@ -178,6 +183,11 @@ public final class MethodTypeResolution { List resolvedTypeParameters = TypeInferenceResolver .inferTypes(produceInitialConstraints(method, argList, variables), initialBounds, variables); + // Is the method applicable? + if (resolvedTypeParameters == null) { + return null; + } + return getTypeDefOfMethod(context, method, resolvedTypeParameters); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/InferenceRuleType.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/InferenceRuleType.java index 5fc2d675d4..b2488d84ee 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/InferenceRuleType.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/InferenceRuleType.java @@ -128,7 +128,8 @@ public enum InferenceRuleType { // Otherwise, the constraint is reduced according to the form of T: TODO - throw new IllegalStateException("Reduce method is flawed! " + val.toString()); + return null; + //throw new IllegalStateException("Reduce method is flawed! " + val.toString()); } }, @@ -257,7 +258,5 @@ public enum InferenceRuleType { } } - public List reduce(BoundOrConstraint constraint) { - return null; - } + public abstract List reduce(BoundOrConstraint constraint); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/TypeInferenceResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/TypeInferenceResolver.java index d6dd482807..1a6d2e71ae 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/TypeInferenceResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/TypeInferenceResolver.java @@ -38,6 +38,11 @@ public final class TypeInferenceResolver { List newBounds = new ArrayList<>(); while (!constraints.isEmpty()) { List reduceResult = constraints.remove(constraints.size() - 1).reduce(); + + // If null, the types are incompatible + if (reduceResult == null) { + return null; + } for (BoundOrConstraint boundOrConstraint : reduceResult) { if (boundOrConstraint instanceof Bound) { 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 8cf55f6788..871f7984c2 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 @@ -1328,7 +1328,7 @@ public class ClassTypeResolverTest { int index = 0; - // int a = subtype(10, 'a', null, new Integer[0]); + // int a = subtype(10, 'a', ""); assertEquals(int.class, expressions.get(index).getType()); assertEquals(int.class, getChildType(expressions.get(index), 0)); assertEquals(int.class, getChildType(expressions.get(index++), 1)); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/TypeInferenceTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/TypeInferenceTest.java index aeb5b8c782..693d83865b 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/TypeInferenceTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/TypeInferenceTest.java @@ -10,6 +10,8 @@ import static net.sourceforge.pmd.lang.java.typeresolution.typeinference.Inferen import static net.sourceforge.pmd.lang.java.typeresolution.typeinference.InferenceRuleType.SUBTYPE; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import java.util.ArrayList; @@ -41,15 +43,15 @@ public class TypeInferenceTest { private JavaTypeDefinition generic = JavaTypeDefinition.forClass(Map.class, number, integer); private Variable alpha = new Variable(); private Variable beta = new Variable(); - JavaTypeDefinition s = JavaTypeDefinition.forClass(int.class); - JavaTypeDefinition t = JavaTypeDefinition.forClass(double.class); + private JavaTypeDefinition s = JavaTypeDefinition.forClass(int.class); + private JavaTypeDefinition t = JavaTypeDefinition.forClass(double.class); @Test public void testEqualityReduceProperVsProper() { // If S and T are proper types, the constraint reduces to true if S is the same as T (§4.3.4), and false // otherwise. assertTrue(new Constraint(number, number, EQUALITY).reduce().isEmpty()); - assertEquals(new Constraint(number, integer, EQUALITY).reduce(), null); + assertNull(new Constraint(number, integer, EQUALITY).reduce()); // Otherwise, if S or T is the null type, the constraint reduces to false. TODO } @@ -59,7 +61,7 @@ public class TypeInferenceTest { // Otherwise, if S is an inference variable, α, and T is not a primitive type, the constraint reduces to // the bound α = T. List result = new Constraint(alpha, number, EQUALITY).reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), alpha, number, EQUALITY, Bound.class); } @@ -68,11 +70,11 @@ public class TypeInferenceTest { // Otherwise, if T is an inference variable, α, and S is not a primitive type, the constraint reduces // to the bound S = α. List result = new Constraint(number, alpha, EQUALITY).reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), number, alpha, EQUALITY, Bound.class); result = new Constraint(alpha, beta, EQUALITY).reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), alpha, beta, EQUALITY, Bound.class); } @@ -82,7 +84,7 @@ public class TypeInferenceTest { // arguments B1, ..., Bn and T has type arguments A1, ..., An, the constraint reduces to the // following new constraints: for all i (1 ≤ i ≤ n), ‹Bi = Ai›. List result = new Constraint(generic, generic, EQUALITY).reduce(); - assertEquals(result.size(), 2); + assertEquals(2, result.size()); testBoundOrConstraint(result.get(0), number, number, EQUALITY, Constraint.class); testBoundOrConstraint(result.get(1), integer, integer, EQUALITY, Constraint.class); } @@ -93,7 +95,7 @@ public class TypeInferenceTest { List result = new Constraint(JavaTypeDefinition.forClass(Number[].class), JavaTypeDefinition.forClass(Integer[].class), EQUALITY) .reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), number, integer, EQUALITY, Constraint.class); } @@ -104,9 +106,9 @@ public class TypeInferenceTest { // If S and T are proper types, the constraint reduces to true if S is a subtype of T (§4.10), // and false otherwise. List result = new Constraint(integer, number, SUBTYPE).reduce(); - assertEquals(result.size(), 0); + assertEquals(0, result.size()); result = new Constraint(number, integer, SUBTYPE).reduce(); - assertEquals(result, null); + assertNull(result); // Otherwise, if S is the null type, the constraint reduces to true. TODO @@ -118,7 +120,7 @@ public class TypeInferenceTest { public void testSubtypeReduceVariableVsAny() { // Otherwise, if S is an inference variable, α, the constraint reduces to the bound α <: T. List result = new Constraint(alpha, integer, SUBTYPE).reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), alpha, integer, SUBTYPE, Bound.class); } @@ -126,11 +128,11 @@ public class TypeInferenceTest { public void testSubtypeReduceAnyVsVariable() { // Otherwise, if T is an inference variable, α, the constraint reduces to the bound S <: α. List result = new Constraint(integer, alpha, SUBTYPE).reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), integer, alpha, SUBTYPE, Bound.class); result = new Constraint(alpha, beta, SUBTYPE).reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), alpha, beta, SUBTYPE, Bound.class); } @@ -142,10 +144,10 @@ public class TypeInferenceTest { // If S and T are proper types, the constraint reduces to true if S is compatible in a loose invocation // context with T (§5.3), and false otherwise. List result = new Constraint(number, integer, LOOSE_INVOCATION).reduce(); - assertEquals(result, null); + assertNull(result); result = new Constraint(integer, number, LOOSE_INVOCATION).reduce(); - assertEquals(result.size(), 0); + assertEquals(0, result.size()); } @Test @@ -153,7 +155,7 @@ public class TypeInferenceTest { // Otherwise, if S is a primitive type, let S' be the result of applying boxing conversion (§5.1.7) to S. // Then the constraint reduces to ‹S' → T›. List result = new Constraint(primitiveInt, number, LOOSE_INVOCATION).reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), integer, number, LOOSE_INVOCATION, Constraint.class); } @@ -163,7 +165,7 @@ public class TypeInferenceTest { // Otherwise, if T is a primitive type, let T' be the result of applying boxing conversion (§5.1.7) to T. // Then the constraint reduces to ‹S = T'›. List result = new Constraint(number, primitiveInt, LOOSE_INVOCATION).reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), number, integer, EQUALITY, Constraint.class); // Otherwise, if T is a parameterized type of the form G, and there exists no type of the @@ -180,11 +182,11 @@ public class TypeInferenceTest { public void testLooseInvocationAnythingElse() { // Otherwise, the constraint reduces to ‹S<:T›. List result = new Constraint(number, alpha, LOOSE_INVOCATION).reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), number, alpha, SUBTYPE, Constraint.class); result = new Constraint(alpha, number, LOOSE_INVOCATION).reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), alpha, number, SUBTYPE, Constraint.class); } @@ -195,7 +197,7 @@ public class TypeInferenceTest { // If T is a type: // If S is a type, the constraint reduces to ‹S = T›. List result = new Constraint(number, integer, CONTAINS).reduce(); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), number, integer, EQUALITY, Constraint.class); // If T is a type: // If S is a wildcard, the constraint reduces to false. TODO @@ -215,22 +217,22 @@ public class TypeInferenceTest { // ### Original rule 1. : α = S and α = T imply ‹S = T› result = incorporationResult(new Bound(alpha, s, EQUALITY), new Bound(alpha, t, EQUALITY)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), s, t, EQUALITY, Constraint.class); // α = S and T = α imply ‹S = T› result = incorporationResult(new Bound(alpha, s, EQUALITY), new Bound(t, alpha, EQUALITY)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), s, t, EQUALITY, Constraint.class); // S = α and α = T imply ‹S = T› result = incorporationResult(new Bound(s, alpha, EQUALITY), new Bound(alpha, t, EQUALITY)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), s, t, EQUALITY, Constraint.class); // S = α and T = α imply ‹S = T› result = incorporationResult(new Bound(s, alpha, EQUALITY), new Bound(t, alpha, EQUALITY)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), s, t, EQUALITY, Constraint.class); } @@ -240,22 +242,22 @@ public class TypeInferenceTest { // ### Original rule 2. : α = S and α <: T imply ‹S <: T› result = incorporationResult(new Bound(alpha, s, EQUALITY), new Bound(alpha, t, SUBTYPE)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), s, t, SUBTYPE, Constraint.class); // S = α and α <: T imply ‹S <: T› result = incorporationResult(new Bound(s, alpha, EQUALITY), new Bound(alpha, t, SUBTYPE)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), s, t, SUBTYPE, Constraint.class); // α <: T and α = S imply ‹S <: T› result = incorporationResult(new Bound(alpha, t, SUBTYPE), new Bound(alpha, s, EQUALITY)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), s, t, SUBTYPE, Constraint.class); // α <: T and S = α imply ‹S <: T› result = incorporationResult(new Bound(alpha, t, SUBTYPE), new Bound(s, alpha, EQUALITY)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), s, t, SUBTYPE, Constraint.class); } @@ -265,22 +267,22 @@ public class TypeInferenceTest { // ### Original rule 3. : α = S and T <: α imply ‹T <: S› result = incorporationResult(new Bound(alpha, s, EQUALITY), new Bound(t, alpha, SUBTYPE)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), t, s, SUBTYPE, Constraint.class); // S = α and T <: α imply ‹T <: S› result = incorporationResult(new Bound(s, alpha, EQUALITY), new Bound(t, alpha, SUBTYPE)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), t, s, SUBTYPE, Constraint.class); // T <: α and α = S imply ‹T <: S› result = incorporationResult(new Bound(t, alpha, SUBTYPE), new Bound(alpha, s, EQUALITY)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), t, s, SUBTYPE, Constraint.class); // T <: α and S = α imply ‹T <: S› result = incorporationResult(new Bound(t, alpha, SUBTYPE), new Bound(s, alpha, EQUALITY)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), t, s, SUBTYPE, Constraint.class); } @@ -290,12 +292,12 @@ public class TypeInferenceTest { // ### Original rule 4. : S <: α and α <: T imply ‹S <: T› result = incorporationResult(new Bound(s, alpha, EQUALITY), new Bound(alpha, t, SUBTYPE)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), s, t, SUBTYPE, Constraint.class); // α <: T and S <: α imply ‹S <: T› result = incorporationResult(new Bound(alpha, t, SUBTYPE), new Bound(s, alpha, EQUALITY)); - assertEquals(result.size(), 1); + assertEquals(1, result.size()); testBoundOrConstraint(result.get(0), s, t, SUBTYPE, Constraint.class); } @@ -323,7 +325,7 @@ public class TypeInferenceTest { Set> minimalSet = TypeInferenceResolver.getMinimalErasedCandidateSet( JavaTypeDefinition.forClass(List.class).getErasedSuperTypeSet()); - assertEquals(minimalSet.size(), 1); + assertEquals(1, minimalSet.size()); assertTrue(minimalSet.contains(List.class)); } @@ -334,7 +336,7 @@ public class TypeInferenceTest { lowerBounds.add(JavaTypeDefinition.forClass(SuperClassAOther.class)); lowerBounds.add(JavaTypeDefinition.forClass(SuperClassAOther2.class)); - assertEquals(TypeInferenceResolver.lub(lowerBounds), JavaTypeDefinition.forClass(SuperClassA2.class)); + assertEquals(JavaTypeDefinition.forClass(SuperClassA2.class), TypeInferenceResolver.lub(lowerBounds)); } @Test @@ -343,8 +345,8 @@ public class TypeInferenceTest { bounds.add(new Bound(JavaTypeDefinition.forClass(SuperClassA.class), alpha, SUBTYPE)); bounds.add(new Bound(JavaTypeDefinition.forClass(SuperClassAOther.class), alpha, SUBTYPE)); Map result = TypeInferenceResolver.resolveVariables(bounds); - assertEquals(result.size(), 1); - assertEquals(result.get(alpha), JavaTypeDefinition.forClass(SuperClassA2.class)); + assertEquals(1, result.size()); + assertEquals(JavaTypeDefinition.forClass(SuperClassA2.class), result.get(alpha)); } private List incorporationResult(Bound firstBound, Bound secondBound) { @@ -358,34 +360,34 @@ public class TypeInferenceTest { private void testBoundOrConstraint(BoundOrConstraint val, JavaTypeDefinition left, JavaTypeDefinition right, InferenceRuleType rule, Class type) { - assertTrue(val.getClass() == type); - assertEquals(val.leftProper(), left); - assertEquals(val.rightProper(), right); - assertEquals(val.ruleType(), rule); + assertSame(type, val.getClass()); + assertEquals(left, val.leftProper()); + assertEquals(right, val.rightProper()); + assertEquals(rule, val.ruleType()); } private void testBoundOrConstraint(BoundOrConstraint val, JavaTypeDefinition left, Variable right, InferenceRuleType rule, Class type) { - assertTrue(val.getClass() == type); - assertEquals(val.leftProper(), left); - assertEquals(val.rightVariable(), right); - assertEquals(val.ruleType(), rule); + assertSame(type, val.getClass()); + assertEquals(left, val.leftProper()); + assertEquals(right, val.rightVariable()); + assertEquals(rule, val.ruleType()); } private void testBoundOrConstraint(BoundOrConstraint val, Variable left, JavaTypeDefinition right, InferenceRuleType rule, Class type) { - assertTrue(val.getClass() == type); - assertEquals(val.leftVariable(), left); - assertEquals(val.rightProper(), right); - assertEquals(val.ruleType(), rule); + assertSame(type, val.getClass()); + assertEquals(left, val.leftVariable()); + assertEquals(right, val.rightProper()); + assertEquals(rule, val.ruleType()); } private void testBoundOrConstraint(BoundOrConstraint val, Variable left, Variable right, InferenceRuleType rule, Class type) { - assertTrue(val.getClass() == type); - assertEquals(val.leftVariable(), left); - assertEquals(val.rightVariable(), right); - assertEquals(val.ruleType(), rule); + assertSame(type, val.getClass()); + assertEquals(left, val.leftVariable()); + assertEquals(right, val.rightVariable()); + assertEquals(rule, val.ruleType()); } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/MethodFirstPhase.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/MethodFirstPhase.java index 98f5ec3ba2..8e2408af2f 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/MethodFirstPhase.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/MethodFirstPhase.java @@ -14,7 +14,7 @@ import java.util.Set; public class MethodFirstPhase { void test() { // primitive, char, simple - int a = subtype(10, 'a', ""); + int a = subtype(Long.valueOf(10), 'a', ""); // TODO: add null, array types Exception b = vararg((Number) null); @@ -46,7 +46,11 @@ public class MethodFirstPhase { return null; } - int subtype(long a, int b, String c) { + int subtype(T a, int b, String c) { + return 0; + } + + int subtype(Long a, int b, String c) { return 0; } From 20093c809a63d0b97277670e3a69285b5dfe9938 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 24 Oct 2017 18:07:09 -0300 Subject: [PATCH 25/62] Rework ruleset loading - ResourceLoader is now instantiable, and we can tell which classloader to use to get resources - We will always use the execution classloader, or just add the paths added by ant, but *never* the auxclasspath - The classpath added by Ant won't get into the auxclasspath either --- .../main/java/net/sourceforge/pmd/PMD.java | 3 +- .../net/sourceforge/pmd/RuleSetFactory.java | 36 +++--- .../sourceforge/pmd/RuleSetReferenceId.java | 28 ++-- .../sourceforge/pmd/RulesetsFactoryUtils.java | 6 +- .../pmd/ant/internal/PMDTaskImpl.java | 46 ++++--- .../sourceforge/pmd/util/ResourceLoader.java | 121 +++++++++--------- .../pmd/RuleSetFactoryCompatibilityTest.java | 4 +- .../sourceforge/pmd/RuleSetFactoryTest.java | 23 ++-- .../pmd/RuleSetReferenceIdTest.java | 6 +- .../pmd/AbstractLanguageVersionTest.java | 5 +- .../pmd/AbstractRuleSetFactoryTest.java | 13 +- 11 files changed, 149 insertions(+), 142 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java b/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java index db91c9eed6..b0ad4a55bc 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/PMD.java @@ -42,6 +42,7 @@ import net.sourceforge.pmd.stat.Metric; import net.sourceforge.pmd.util.ClasspathClassLoader; import net.sourceforge.pmd.util.FileUtil; import net.sourceforge.pmd.util.IOUtil; +import net.sourceforge.pmd.util.ResourceLoader; import net.sourceforge.pmd.util.database.DBMSMetadata; import net.sourceforge.pmd.util.database.DBURI; import net.sourceforge.pmd.util.database.SourceObject; @@ -198,7 +199,7 @@ public class PMD { public static int doPMD(PMDConfiguration configuration) { // Load the RuleSets - RuleSetFactory ruleSetFactory = RulesetsFactoryUtils.getRulesetFactory(configuration); + RuleSetFactory ruleSetFactory = RulesetsFactoryUtils.getRulesetFactory(configuration, new ResourceLoader()); RuleSets ruleSets = RulesetsFactoryUtils.getRuleSetsWithBenchmark(configuration.getRuleSets(), ruleSetFactory); if (ruleSets == null) { return 0; diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java index 2257cff659..0c6563add0 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -49,9 +49,9 @@ import net.sourceforge.pmd.util.ResourceLoader; /** * RuleSetFactory is responsible for creating RuleSet instances from XML - * content. By default Rules will be loaded using the ClassLoader for this - * class, using the {@link RulePriority#LOW} priority, with Rule deprecation - * warnings off. By default, the ruleset compatibility filter is active, too. + * content. By default Rules will be loaded using the {@link RulePriority#LOW} priority, + * with Rule deprecation warnings off. + * By default, the ruleset compatibility filter is active, too. * See {@link RuleSetFactoryCompatibility}. */ public class RuleSetFactory { @@ -65,18 +65,18 @@ public class RuleSetFactory { private static final String MESSAGE = "message"; private static final String EXTERNAL_INFO_URL = "externalInfoUrl"; - private final ClassLoader classLoader; + private final ResourceLoader resourceLoader; private final RulePriority minimumPriority; private final boolean warnDeprecated; private final RuleSetFactoryCompatibility compatibilityFilter; public RuleSetFactory() { - this(RuleSetFactory.class.getClassLoader(), RulePriority.LOW, false, true); + this(new ResourceLoader(), RulePriority.LOW, false, true); } - public RuleSetFactory(final ClassLoader classLoader, final RulePriority minimumPriority, + public RuleSetFactory(final ResourceLoader resourceLoader, final RulePriority minimumPriority, final boolean warnDeprecated, final boolean enableCompatibility) { - this.classLoader = classLoader; + this.resourceLoader = resourceLoader; this.minimumPriority = minimumPriority; this.warnDeprecated = warnDeprecated; @@ -97,7 +97,7 @@ public class RuleSetFactory { * factory. */ public RuleSetFactory(final RuleSetFactory factory, final boolean warnDeprecated) { - this(factory.classLoader, factory.minimumPriority, warnDeprecated, factory.compatibilityFilter != null); + this(factory.resourceLoader, factory.minimumPriority, warnDeprecated, factory.compatibilityFilter != null); } /** @@ -125,7 +125,7 @@ public class RuleSetFactory { for (Language language : LanguageRegistry.findWithRuleSupport()) { Properties props = new Properties(); rulesetsProperties = "rulesets/" + language.getTerseName() + "/rulesets.properties"; - try (InputStream inputStream = ResourceLoader.loadResourceAsStream(rulesetsProperties);) { + try (InputStream inputStream = resourceLoader.loadClassPathResourceAsStreamOrThrow(rulesetsProperties)) { props.load(inputStream); } String rulesetFilenames = props.getProperty("rulesets.filenames"); @@ -134,7 +134,7 @@ public class RuleSetFactory { return createRuleSets(ruleSetReferenceIds).getRuleSetsIterator(); } catch (IOException ioe) { throw new RuntimeException("Couldn't find " + rulesetsProperties - + "; please ensure that the rulesets directory is on the classpath. The current classpath is: " + + "; please ensure that the rulesets directory is on the classpath. The current classpath is: " + System.getProperty("java.class.path")); } } @@ -143,7 +143,7 @@ public class RuleSetFactory { * Create a RuleSets from a comma separated list of RuleSet reference IDs. * This is a convenience method which calls * {@link RuleSetReferenceId#parse(String)}, and then calls - * {@link #createRuleSets(List)}. The currently configured ClassLoader is + * {@link #createRuleSets(List)}. The currently configured ResourceLoader is * used. * * @param referenceString @@ -158,7 +158,7 @@ public class RuleSetFactory { /** * Create a RuleSets from a list of RuleSetReferenceIds. The currently - * configured ClassLoader is used. + * configured ResourceLoader is used. * * @param ruleSetReferenceIds * The List of RuleSetReferenceId of the RuleSets to create. @@ -180,7 +180,7 @@ public class RuleSetFactory { * convenience method which calls {@link RuleSetReferenceId#parse(String)}, * gets the first item in the List, and then calls * {@link #createRuleSet(RuleSetReferenceId)}. The currently configured - * ClassLoader is used. + * ResourceLoader is used. * * @param referenceString * A comma separated list of RuleSet reference IDs. @@ -199,7 +199,7 @@ public class RuleSetFactory { /** * Create a RuleSet from a RuleSetReferenceId. Priority filtering is ignored - * when loading a single Rule. The currently configured ClassLoader is used. + * when loading a single Rule. The currently configured ResourceLoader is used. * * @param ruleSetReferenceId * The RuleSetReferenceId of the RuleSet to create. @@ -291,7 +291,7 @@ public class RuleSetFactory { /** * Create a Rule from a RuleSet created from a file name resource. The - * currently configured ClassLoader is used. + * currently configured ResourceLoader is used. *

* Any Rules in the RuleSet other than the one being created, are _not_ * created. Deprecated rules are _not_ ignored, so that they can be @@ -329,7 +329,7 @@ public class RuleSetFactory { private RuleSet parseRuleSetNode(RuleSetReferenceId ruleSetReferenceId, boolean withDeprecatedRuleReferences) throws RuleSetNotFoundException { try (CheckedInputStream inputStream = new CheckedInputStream( - ruleSetReferenceId.getInputStream(this.classLoader), new Adler32());) { + ruleSetReferenceId.getInputStream(resourceLoader), new Adler32());) { if (!ruleSetReferenceId.isExternal()) { throw new IllegalArgumentException( "Cannot parse a RuleSet from a non-external reference: <" + ruleSetReferenceId + ">."); @@ -543,7 +543,7 @@ public class RuleSetFactory { if (attribute == null || "".equals(attribute)) { throw new IllegalArgumentException("The 'class' field of rule can't be null, nor empty."); } - Rule rule = (Rule) RuleSetFactory.class.getClassLoader().loadClass(attribute).newInstance(); + Rule rule = (Rule) Class.forName(attribute).newInstance(); rule.setName(ruleElement.getAttribute("name")); if (ruleElement.hasAttribute("language")) { @@ -774,7 +774,7 @@ public class RuleSetFactory { */ private boolean containsRule(RuleSetReferenceId ruleSetReferenceId, String ruleName) { boolean found = false; - try (InputStream ruleSet = ruleSetReferenceId.getInputStream(classLoader)) { + try (InputStream ruleSet = ruleSetReferenceId.getInputStream(resourceLoader)) { DocumentBuilder builder = createDocumentBuilder(); Document document = builder.parse(ruleSet); Element ruleSetElement = document.getDocumentElement(); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetReferenceId.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetReferenceId.java index 74a685de99..2f49c29b50 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetReferenceId.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetReferenceId.java @@ -12,7 +12,6 @@ import java.net.URL; import java.util.ArrayList; import java.util.List; -import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import net.sourceforge.pmd.util.ResourceLoader; @@ -224,18 +223,13 @@ public class RuleSetReferenceId { * @return true if the ruleset could be loaded, * false otherwise. */ - private boolean checkRulesetExists(String name) { + private boolean checkRulesetExists(final String name) { boolean resourceFound = false; if (name != null) { - try { - InputStream resource = ResourceLoader.loadResourceAsStream(name, - RuleSetReferenceId.class.getClassLoader()); - if (resource != null) { - resourceFound = true; - IOUtils.closeQuietly(resource); - } - } catch (RuleSetNotFoundException e) { - resourceFound = false; + try (InputStream resource = new ResourceLoader().loadClassPathResourceAsStreamOrThrow(name)) { + resourceFound = true; + } catch (Exception e) { + // ignored } } return resourceFound; @@ -310,6 +304,7 @@ public class RuleSetReferenceId { if (isHttpUrl(name)) { String url = StringUtils.strip(name); try { + // FIXME : Do we really need to perform a request? if it's a url we should treat it as one even if the server is down HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection(); connection.setRequestMethod("HEAD"); connection.setConnectTimeout(ResourceLoader.TIMEOUT); @@ -393,22 +388,21 @@ public class RuleSetReferenceId { } /** - * Try to load the RuleSet resource with the specified ClassLoader. Multiple + * Try to load the RuleSet resource with the specified ResourceLoader. Multiple * attempts to get independent InputStream instances may be made, so * subclasses must ensure they support this behavior. Delegates to an * external RuleSetReferenceId if there is one associated with this * instance. * - * @param classLoader - * The ClassLoader to use. + * @param rl The {@link ResourceLoader} to use. * @return An InputStream to that resource. * @throws RuleSetNotFoundException * if unable to find a resource. */ - public InputStream getInputStream(ClassLoader classLoader) throws RuleSetNotFoundException { + public InputStream getInputStream(final ResourceLoader rl) throws RuleSetNotFoundException { if (externalRuleSetReferenceId == null) { InputStream in = StringUtils.isBlank(ruleSetFileName) ? null - : ResourceLoader.loadResourceAsStream(ruleSetFileName, classLoader); + : rl.loadResourceAsStream(ruleSetFileName); if (in == null) { throw new RuleSetNotFoundException("Can't find resource '" + ruleSetFileName + "' for rule '" + ruleName + "'" + ". Make sure the resource is a valid file or URL and is on the CLASSPATH. " @@ -416,7 +410,7 @@ public class RuleSetReferenceId { } return in; } else { - return externalRuleSetReferenceId.getInputStream(classLoader); + return externalRuleSetReferenceId.getInputStream(rl); } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RulesetsFactoryUtils.java b/pmd-core/src/main/java/net/sourceforge/pmd/RulesetsFactoryUtils.java index 2b0857a8cc..c17fd98215 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RulesetsFactoryUtils.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RulesetsFactoryUtils.java @@ -9,6 +9,7 @@ import java.util.logging.Logger; import net.sourceforge.pmd.benchmark.Benchmark; import net.sourceforge.pmd.benchmark.Benchmarker; +import net.sourceforge.pmd.util.ResourceLoader; public final class RulesetsFactoryUtils { @@ -72,8 +73,9 @@ public final class RulesetsFactoryUtils { return ruleSets; } - public static RuleSetFactory getRulesetFactory(final PMDConfiguration configuration) { - return new RuleSetFactory(configuration.getClassLoader(), configuration.getMinimumPriority(), true, + public static RuleSetFactory getRulesetFactory(final PMDConfiguration configuration, + final ResourceLoader resourceLoader) { + return new RuleSetFactory(resourceLoader, configuration.getMinimumPriority(), true, configuration.isRuleSetFactoryCompatibilityEnabled()); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java b/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java index b76a0682b3..477232d6ee 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java @@ -43,6 +43,7 @@ import net.sourceforge.pmd.lang.LanguageVersion; import net.sourceforge.pmd.renderers.AbstractRenderer; import net.sourceforge.pmd.renderers.Renderer; import net.sourceforge.pmd.util.IOUtil; +import net.sourceforge.pmd.util.ResourceLoader; import net.sourceforge.pmd.util.datasource.DataSource; import net.sourceforge.pmd.util.datasource.FileDataSource; import net.sourceforge.pmd.util.log.AntLogHandler; @@ -101,9 +102,11 @@ public class PMDTaskImpl { private void doTask() { setupClassLoader(); - + // Setup RuleSetFactory and validate RuleSets - RuleSetFactory ruleSetFactory = RulesetsFactoryUtils.getRulesetFactory(configuration); + final ResourceLoader rl = setupResourceLoader(); + RuleSetFactory ruleSetFactory = RulesetsFactoryUtils.getRulesetFactory(configuration, rl); + try { // This is just used to validate and display rules. Each thread will create its own ruleset String ruleSets = configuration.getRuleSets(); @@ -208,6 +211,27 @@ public class PMDTaskImpl { } } + private ResourceLoader setupResourceLoader() { + if (classpath == null) { + classpath = new Path(project); + } + + /* + * 'basedir' is added to the path to make sure that relative paths such + * as "resources/custom_ruleset.xml" still work when + * ant is invoked from a different directory using "-f" + */ + classpath.add(new Path(null, project.getBaseDir().toString())); + + project.log("Using the AntClassLoader: " + classpath, Project.MSG_VERBOSE); + // must be true, otherwise you'll get ClassCastExceptions as classes + // are loaded twice + // and exist in multiple class loaders + final boolean parentFirst = true; + return new ResourceLoader(new AntClassLoader(Thread.currentThread().getContextClassLoader(), + project, classpath, parentFirst)); + } + private void handleError(RuleContext ctx, Report errorReport, RuntimeException pmde) { pmde.printStackTrace(); @@ -234,24 +258,6 @@ public class PMDTaskImpl { } private void setupClassLoader() { - if (classpath == null) { - classpath = new Path(project); - } - /* - * 'basedir' is added to the path to make sure that relative paths such - * as "resources/custom_ruleset.xml" still work when - * ant is invoked from a different directory using "-f" - */ - classpath.add(new Path(null, project.getBaseDir().toString())); - - project.log("Using the AntClassLoader: " + classpath, Project.MSG_VERBOSE); - // must be true, otherwise you'll get ClassCastExceptions as classes - // are loaded twice - // and exist in multiple class loaders - boolean parentFirst = true; - configuration.setClassLoader( - new AntClassLoader(Thread.currentThread().getContextClassLoader(), project, classpath, parentFirst)); - try { if (auxClasspath != null) { project.log("Using auxclasspath: " + auxClasspath, Project.MSG_VERBOSE); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/ResourceLoader.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/ResourceLoader.java index 87748290ae..eaa107e536 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/ResourceLoader.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/ResourceLoader.java @@ -12,12 +12,11 @@ import java.io.InputStream; import java.net.HttpURLConnection; import java.net.URL; import java.net.URLConnection; +import java.util.Objects; import net.sourceforge.pmd.RuleSetNotFoundException; -/** - */ -public final class ResourceLoader { +public class ResourceLoader { public static final int TIMEOUT; @@ -31,86 +30,92 @@ public final class ResourceLoader { TIMEOUT = timeoutProperty; } - // Only static methods, so we shouldn't allow an instance to be created + private final ClassLoader classLoader; + /** * Constructor for ResourceLoader. */ - private ResourceLoader() { + public ResourceLoader() { + this(ResourceLoader.class.getClassLoader()); } /** - * Method to find a file, first by finding it as a file (either by the - * absolute or relative path), then as a URL, and then finally seeing if it - * is on the classpath. - *

- * Caller is responsible for closing the {@link InputStream}. - * - * @param name - * String - * @return InputStream - * @throws RuleSetNotFoundException + * Constructor for ResourceLoader. */ - public static InputStream loadResourceAsStream(String name) throws RuleSetNotFoundException { - InputStream stream = loadResourceAsStream(name, ResourceLoader.class.getClassLoader()); - if (stream == null) { - throw new RuleSetNotFoundException("Can't find resource " + name - + ". Make sure the resource is a valid file or URL or is on the CLASSPATH"); - } - return stream; + public ResourceLoader(final ClassLoader cl) { + this.classLoader = Objects.requireNonNull(cl); } /** - * Uses the ClassLoader passed in to attempt to load the resource if it's - * not a File or a URL + * Attempts to load the resource from file, a URL or the claspath *

* Caller is responsible for closing the {@link InputStream}. * - * @param name - * String - * @param loader - * ClassLoader + * @param name The resource to attempt and load * @return InputStream * @throws RuleSetNotFoundException */ - public static InputStream loadResourceAsStream(String name, ClassLoader loader) throws RuleSetNotFoundException { - File file = new File(name); + public InputStream loadResourceAsStream(final String name) throws RuleSetNotFoundException { + // Search file locations first + final File file = new File(name); if (file.exists()) { try { return new FileInputStream(file); - } catch (FileNotFoundException e) { + } catch (final FileNotFoundException e) { // if the file didn't exist, we wouldn't be here } - } else { + } + + // Maybe it's a url? + try { + final HttpURLConnection connection = (HttpURLConnection) new URL(name).openConnection(); + connection.setConnectTimeout(TIMEOUT); + connection.setReadTimeout(TIMEOUT); + return connection.getInputStream(); + } catch (final Exception e) { try { - HttpURLConnection connection = (HttpURLConnection) new URL(name).openConnection(); - connection.setConnectTimeout(TIMEOUT); - connection.setReadTimeout(TIMEOUT); - return connection.getInputStream(); - } catch (Exception e) { - try { - /* - * Don't use getResourceAsStream to void reusing connections between threads - * See https://github.com/pmd/pmd/issues/234 - */ - URL resource = loader.getResource(name); - if (resource == null) { - // Don't throw RuleSetNotFoundException, keep API compatibility - return null; - } else { - final URLConnection connection = resource.openConnection(); - // This avoids reusing the underlaying file, if the resource is loaded from a Jar file. - // The file is closed with the input stream then thus not leaving a leaked resource behind. - // See https://github.com/pmd/pmd/issues/364 and https://github.com/pmd/pmd/issues/337 - connection.setUseCaches(false); - final InputStream inputStream = connection.getInputStream(); - return inputStream; - } - } catch (IOException e1) { - // Ignored - } + return loadClassPathResourceAsStream(name); + } catch (final IOException ignored) { + // We will throw out own exception, with a different message } } + throw new RuleSetNotFoundException("Can't find resource " + name + ". Make sure the resource is a valid file or URL or is on the CLASSPATH"); } + + public InputStream loadClassPathResourceAsStream(final String name) throws IOException { + /* + * Don't use getResourceAsStream to avoid reusing connections between threads + * See https://github.com/pmd/pmd/issues/234 + */ + final URL resource = classLoader.getResource(name); + if (resource == null) { + // Don't throw RuleSetNotFoundException, keep API compatibility + return null; + } else { + final URLConnection connection = resource.openConnection(); + // This avoids reusing the underlying file, if the resource is loaded from a Jar file. + // The file is closed with the input stream then thus not leaving a leaked resource behind. + // See https://github.com/pmd/pmd/issues/364 and https://github.com/pmd/pmd/issues/337 + connection.setUseCaches(false); + return connection.getInputStream(); + } + } + + public InputStream loadClassPathResourceAsStreamOrThrow(final String name) throws RuleSetNotFoundException { + InputStream is = null; + try { + is = loadClassPathResourceAsStream(name); + } catch (final IOException ignored) { + // ignored + } + + if (is == null) { + throw new RuleSetNotFoundException("Can't find resource " + name + + ". Make sure the resource is on the CLASSPATH"); + } + + return is; + } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryCompatibilityTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryCompatibilityTest.java index 7bcb877f88..d09407c8ba 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryCompatibilityTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryCompatibilityTest.java @@ -13,6 +13,8 @@ import org.apache.commons.io.IOUtils; import org.junit.Assert; import org.junit.Test; +import net.sourceforge.pmd.util.ResourceLoader; + public class RuleSetFactoryCompatibilityTest { private static final Charset ISO_8859_1 = Charset.forName("ISO-8859-1"); private static final Charset UTF_8 = Charset.forName("UTF-8"); @@ -136,7 +138,7 @@ public class RuleSetFactoryCompatibilityTest { throws RuleSetNotFoundException { return factory.createRuleSet(new RuleSetReferenceId(null) { @Override - public InputStream getInputStream(ClassLoader classLoader) throws RuleSetNotFoundException { + public InputStream getInputStream(ResourceLoader resourceLoader) throws RuleSetNotFoundException { return new ByteArrayInputStream(ruleset.getBytes(UTF_8)); } }); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java index 966ecb6ffa..5278b74f9e 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetFactoryTest.java @@ -55,8 +55,7 @@ public class RuleSetFactoryTest { @Test public void testExtendedReferences() throws Exception { - InputStream in = ResourceLoader.loadResourceAsStream("net/sourceforge/pmd/rulesets/reference-ruleset.xml", - this.getClass().getClassLoader()); + InputStream in = new ResourceLoader().loadClassPathResourceAsStream("net/sourceforge/pmd/rulesets/reference-ruleset.xml"); assertNotNull("Test ruleset not found - can't continue with test!", in); in.close(); @@ -325,7 +324,8 @@ public class RuleSetFactoryTest { @Test public void testReferencePriority() throws RuleSetNotFoundException { - RuleSetFactory rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.LOW, false, true); + ResourceLoader rl = new ResourceLoader(); + RuleSetFactory rsf = new RuleSetFactory(rl, RulePriority.LOW, false, true); RuleSet ruleSet = rsf.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_INTERNAL_CHAIN)); assertEquals("Number of Rules", 3, ruleSet.getRules().size()); @@ -333,31 +333,31 @@ public class RuleSetFactoryTest { assertNotNull(ruleSet.getRuleByName("MockRuleNameRef")); assertNotNull(ruleSet.getRuleByName("MockRuleNameRefRef")); - rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.MEDIUM_HIGH, false, true); + rsf = new RuleSetFactory(rl, RulePriority.MEDIUM_HIGH, false, true); ruleSet = rsf.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_INTERNAL_CHAIN)); assertEquals("Number of Rules", 2, ruleSet.getRules().size()); assertNotNull(ruleSet.getRuleByName("MockRuleNameRef")); assertNotNull(ruleSet.getRuleByName("MockRuleNameRefRef")); - rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.HIGH, false, true); + rsf = new RuleSetFactory(rl, RulePriority.HIGH, false, true); ruleSet = rsf.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_INTERNAL_CHAIN)); assertEquals("Number of Rules", 1, ruleSet.getRules().size()); assertNotNull(ruleSet.getRuleByName("MockRuleNameRefRef")); - rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.LOW, false, true); + rsf = new RuleSetFactory(rl, RulePriority.LOW, false, true); ruleSet = rsf.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_EXTERNAL_CHAIN)); assertEquals("Number of Rules", 3, ruleSet.getRules().size()); assertNotNull(ruleSet.getRuleByName("ExternalRefRuleName")); assertNotNull(ruleSet.getRuleByName("ExternalRefRuleNameRef")); assertNotNull(ruleSet.getRuleByName("ExternalRefRuleNameRefRef")); - rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.MEDIUM_HIGH, false, true); + rsf = new RuleSetFactory(rl, RulePriority.MEDIUM_HIGH, false, true); ruleSet = rsf.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_EXTERNAL_CHAIN)); assertEquals("Number of Rules", 2, ruleSet.getRules().size()); assertNotNull(ruleSet.getRuleByName("ExternalRefRuleNameRef")); assertNotNull(ruleSet.getRuleByName("ExternalRefRuleNameRefRef")); - rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.HIGH, false, true); + rsf = new RuleSetFactory(rl, RulePriority.HIGH, false, true); ruleSet = rsf.createRuleSet(createRuleSetReferenceId(REF_INTERNAL_TO_EXTERNAL_CHAIN)); assertEquals("Number of Rules", 1, ruleSet.getRules().size()); assertNotNull(ruleSet.getRuleByName("ExternalRefRuleNameRefRef")); @@ -382,9 +382,10 @@ public class RuleSetFactoryTest { @Test public void testSetPriority() throws RuleSetNotFoundException { - RuleSetFactory rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.MEDIUM_HIGH, false, true); + ResourceLoader rl = new ResourceLoader(); + RuleSetFactory rsf = new RuleSetFactory(rl, RulePriority.MEDIUM_HIGH, false, true); assertEquals(0, rsf.createRuleSet(createRuleSetReferenceId(SINGLE_RULE)).size()); - rsf = new RuleSetFactory(getClass().getClassLoader(), RulePriority.MEDIUM_LOW, false, true); + rsf = new RuleSetFactory(rl, RulePriority.MEDIUM_LOW, false, true); assertEquals(1, rsf.createRuleSet(createRuleSetReferenceId(SINGLE_RULE)).size()); } @@ -862,7 +863,7 @@ public class RuleSetFactoryTest { private static RuleSetReferenceId createRuleSetReferenceId(final String ruleSetXml) { return new RuleSetReferenceId(null) { @Override - public InputStream getInputStream(ClassLoader classLoader) throws RuleSetNotFoundException { + public InputStream getInputStream(ResourceLoader resourceLoader) throws RuleSetNotFoundException { try { return new ByteArrayInputStream(ruleSetXml.getBytes("UTF-8")); } catch (UnsupportedEncodingException e) { diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetReferenceIdTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetReferenceIdTest.java index 8d56f85578..c258792cc8 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetReferenceIdTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetReferenceIdTest.java @@ -24,6 +24,8 @@ import java.util.List; import org.apache.commons.io.IOUtils; import org.junit.Test; +import net.sourceforge.pmd.util.ResourceLoader; + import com.github.tomakehurst.wiremock.junit.WireMockRule; public class RuleSetReferenceIdTest { @@ -119,7 +121,7 @@ public class RuleSetReferenceIdTest { RuleSetReferenceId ruleSetReferenceId = new RuleSetReferenceId(" " + rulesetUrl + " "); assertRuleSetReferenceId(true, rulesetUrl, true, null, rulesetUrl, ruleSetReferenceId); - try (InputStream inputStream = ruleSetReferenceId.getInputStream(RuleSetReferenceIdTest.class.getClassLoader());) { + try (InputStream inputStream = ruleSetReferenceId.getInputStream(new ResourceLoader())) { String loaded = IOUtils.toString(inputStream, "UTF-8"); assertEquals("xyz", loaded); } @@ -148,7 +150,7 @@ public class RuleSetReferenceIdTest { assertRuleSetReferenceId(true, hostpart + path, false, "DummyBasicMockRule", hostpart + completePath, ruleSetReferenceId); - try (InputStream inputStream = ruleSetReferenceId.getInputStream(RuleSetReferenceIdTest.class.getClassLoader());) { + try (InputStream inputStream = ruleSetReferenceId.getInputStream(new ResourceLoader())) { String loaded = IOUtils.toString(inputStream, "UTF-8"); assertEquals(basicRuleSet, loaded); } diff --git a/pmd-test/src/main/java/net/sourceforge/pmd/AbstractLanguageVersionTest.java b/pmd-test/src/main/java/net/sourceforge/pmd/AbstractLanguageVersionTest.java index b5636a18af..faa31940fd 100644 --- a/pmd-test/src/main/java/net/sourceforge/pmd/AbstractLanguageVersionTest.java +++ b/pmd-test/src/main/java/net/sourceforge/pmd/AbstractLanguageVersionTest.java @@ -115,9 +115,10 @@ public class AbstractLanguageVersionTest { return; } + ResourceLoader rl = new ResourceLoader(); Properties props = new Properties(); String rulesetsProperties = "rulesets/" + simpleTerseName + "/rulesets.properties"; - try (InputStream inputStream = ResourceLoader.loadResourceAsStream(rulesetsProperties);) { + try (InputStream inputStream = rl.loadClassPathResourceAsStreamOrThrow(rulesetsProperties)) { props.load(inputStream); } String rulesetFilenames = props.getProperty("rulesets.filenames"); @@ -131,7 +132,7 @@ public class AbstractLanguageVersionTest { String[] rulesets = rulesetFilenames.split(","); for (String r : rulesets) { - InputStream stream = ResourceLoader.loadResourceAsStream(r); + InputStream stream = rl.loadClassPathResourceAsStream(r); assertNotNull(stream); stream.close(); RuleSet ruleset = factory.createRuleSet(r); diff --git a/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java b/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java index 8226ff67b2..ed50f63189 100644 --- a/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java +++ b/pmd-test/src/main/java/net/sourceforge/pmd/AbstractRuleSetFactoryTest.java @@ -243,7 +243,7 @@ public abstract class AbstractRuleSetFactoryTest { List ruleSetFileNames = new ArrayList<>(); try { Properties properties = new Properties(); - try (InputStream is = ResourceLoader.loadResourceAsStream("rulesets/" + language + "/rulesets.properties")) { + try (InputStream is = new ResourceLoader().loadClassPathResourceAsStreamOrThrow("rulesets/" + language + "/rulesets.properties")) { properties.load(is); } String fileNames = properties.getProperty("rulesets.filenames"); @@ -335,14 +335,7 @@ public abstract class AbstractRuleSetFactoryTest { } private static InputStream loadResourceAsStream(String resource) throws RuleSetNotFoundException { - InputStream inputStream = ResourceLoader.loadResourceAsStream(resource, - AbstractRuleSetFactoryTest.class.getClassLoader()); - if (inputStream == null) { - throw new RuleSetNotFoundException("Can't find resource " + resource - + " Make sure the resource is a valid file or URL or is on the CLASSPATH. Here's the current classpath: " - + System.getProperty("java.class.path")); - } - return inputStream; + return new ResourceLoader().loadClassPathResourceAsStreamOrThrow(resource); } private void testRuleSet(String fileName) @@ -477,7 +470,7 @@ public abstract class AbstractRuleSetFactoryTest { protected static RuleSetReferenceId createRuleSetReferenceId(final String ruleSetXml) { return new RuleSetReferenceId(null) { @Override - public InputStream getInputStream(ClassLoader classLoader) throws RuleSetNotFoundException { + public InputStream getInputStream(ResourceLoader resourceLoader) throws RuleSetNotFoundException { try { return new ByteArrayInputStream(ruleSetXml.getBytes("UTF-8")); } catch (UnsupportedEncodingException e) { From 9d59c0317c6c68f67c37dac45e5eb1db62b61c84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Thu, 26 Oct 2017 00:53:09 -0300 Subject: [PATCH 26/62] [java] Fix boxing rules - Boolean types were not considered since we were using the subtypability set which deals exclusively with numeric types - Fixes #650 --- .../typeresolution/MethodTypeResolution.java | 27 ++++++++++++++----- .../MethodTypeResolutionTest.java | 24 +++++++++++++++++ 2 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/MethodTypeResolutionTest.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java index faa593bf2b..0e085a2816 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java @@ -13,7 +13,9 @@ import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; @@ -38,9 +40,10 @@ public final class MethodTypeResolution { private static final List> PRIMITIVE_SUBTYPE_ORDER; private static final List> BOXED_PRIMITIVE_SUBTYPE_ORDER; + private static final Map, Class> PRIMITIVE_BOXING_RULES; static { - List> primitiveList = new ArrayList<>(); + final List> primitiveList = new ArrayList<>(); primitiveList.add(double.class); primitiveList.add(float.class); @@ -52,7 +55,7 @@ public final class MethodTypeResolution { PRIMITIVE_SUBTYPE_ORDER = Collections.unmodifiableList(primitiveList); - List> boxedList = new ArrayList<>(); + final List> boxedList = new ArrayList<>(); boxedList.add(Double.class); boxedList.add(Float.class); @@ -63,6 +66,20 @@ public final class MethodTypeResolution { boxedList.add(Character.class); BOXED_PRIMITIVE_SUBTYPE_ORDER = Collections.unmodifiableList(boxedList); + + final Map, Class> boxingRules = new HashMap<>(); + + boxingRules.put(double.class, Double.class); + boxingRules.put(float.class, Float.class); + boxingRules.put(long.class, Long.class); + boxingRules.put(int.class, Integer.class); + boxingRules.put(short.class, Short.class); + boxingRules.put(byte.class, Byte.class); + boxingRules.put(char.class, Character.class); + boxingRules.put(boolean.class, Boolean.class); + boxingRules.put(void.class, Void.class); + + PRIMITIVE_BOXING_RULES = Collections.unmodifiableMap(boxingRules); } public static boolean checkSubtypeability(MethodType method, MethodType subtypeableMethod) { @@ -686,11 +703,7 @@ public final class MethodTypeResolution { } public static JavaTypeDefinition boxPrimitive(JavaTypeDefinition def) { - if (!def.isPrimitive()) { - return null; - } - - return JavaTypeDefinition.forClass(BOXED_PRIMITIVE_SUBTYPE_ORDER.get(PRIMITIVE_SUBTYPE_ORDER.indexOf(def.getType()))); + return JavaTypeDefinition.forClass(PRIMITIVE_BOXING_RULES.get(def.getType())); } public static List getMethodExplicitTypeArugments(Node node) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/MethodTypeResolutionTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/MethodTypeResolutionTest.java new file mode 100644 index 0000000000..4da51b52d9 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/MethodTypeResolutionTest.java @@ -0,0 +1,24 @@ +package net.sourceforge.pmd.typeresolution; + +import static org.junit.Assert.assertSame; + +import org.junit.Test; + +import net.sourceforge.pmd.lang.java.typeresolution.MethodTypeResolution; +import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition; + +public class MethodTypeResolutionTest { + + @Test + public void testBoxingRules() { + assertSame(Boolean.class, MethodTypeResolution.boxPrimitive(JavaTypeDefinition.forClass(boolean.class)).getType()); + assertSame(Double.class, MethodTypeResolution.boxPrimitive(JavaTypeDefinition.forClass(double.class)).getType()); + assertSame(Float.class, MethodTypeResolution.boxPrimitive(JavaTypeDefinition.forClass(float.class)).getType()); + assertSame(Long.class, MethodTypeResolution.boxPrimitive(JavaTypeDefinition.forClass(long.class)).getType()); + assertSame(Integer.class, MethodTypeResolution.boxPrimitive(JavaTypeDefinition.forClass(int.class)).getType()); + assertSame(Character.class, MethodTypeResolution.boxPrimitive(JavaTypeDefinition.forClass(char.class)).getType()); + assertSame(Short.class, MethodTypeResolution.boxPrimitive(JavaTypeDefinition.forClass(short.class)).getType()); + assertSame(Byte.class, MethodTypeResolution.boxPrimitive(JavaTypeDefinition.forClass(byte.class)).getType()); + assertSame(Void.class, MethodTypeResolution.boxPrimitive(JavaTypeDefinition.forClass(void.class)).getType()); + } +} From 92cc66a8cdb81a9de2ae091e425b4e28fb9e7db9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Thu, 26 Oct 2017 01:13:04 -0300 Subject: [PATCH 27/62] Checkstyle fixes --- .../pmd/typeresolution/MethodTypeResolutionTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/MethodTypeResolutionTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/MethodTypeResolutionTest.java index 4da51b52d9..7a9a5bd015 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/MethodTypeResolutionTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/MethodTypeResolutionTest.java @@ -1,3 +1,7 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + package net.sourceforge.pmd.typeresolution; import static org.junit.Assert.assertSame; From 71200e6a67afa79525f3930f3f1a32cda528412a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Thu, 26 Oct 2017 10:06:18 -0300 Subject: [PATCH 28/62] Default to reduce to false for the time being --- .../typeresolution/typeinference/InferenceRuleType.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/InferenceRuleType.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/InferenceRuleType.java index b2488d84ee..b88258dc70 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/InferenceRuleType.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typeinference/InferenceRuleType.java @@ -83,7 +83,9 @@ public enum InferenceRuleType { // A constraint formula of the form ‹S = T›, where S and T are type arguments (§4.5.1), is reduced as // follows: TODO - throw new IllegalStateException("Reduce method is flawed! " + val.toString()); + // TODO: Reduce to false for the time being, reduction is still incomplete + return null; + //throw new IllegalStateException("Reduce method is flawed! " + val.toString()); } }, @@ -128,6 +130,7 @@ public enum InferenceRuleType { // Otherwise, the constraint is reduced according to the form of T: TODO + // TODO: Reduce to false for the time being, reduction is still incomplete return null; //throw new IllegalStateException("Reduce method is flawed! " + val.toString()); } @@ -221,7 +224,9 @@ public enum InferenceRuleType { // If T is a wildcard of the form ? super T': TODO - throw new IllegalStateException("Reduce method is flawed! " + val.toString()); + // TODO: Reduce to false for the time being, reduction is still incomplete + return null; + //throw new IllegalStateException("Reduce method is flawed! " + val.toString()); } }; From ae2898e23809ca0644bf94f3be103941cd48df80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Thu, 26 Oct 2017 12:18:00 -0300 Subject: [PATCH 29/62] [java] Properly handle 0-arg calls - We no longer take all methods as applicable for a 0-arg method call during type resolution. --- .../typeresolution/MethodTypeResolution.java | 66 +++++++++---------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java index faa593bf2b..8e0b0c31f9 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java @@ -115,11 +115,10 @@ public final class MethodTypeResolution { for (int methodIndex = 0; methodIndex < methodsToSearch.size(); ++methodIndex) { MethodType methodType = methodsToSearch.get(methodIndex); - if (argList == null) { - selectedMethods.add(methodType); + final int argCount = argList == null ? 0 : argList.jjtGetNumChildren(); - // vararg methods are considered fixed arity here - varargs are dealt with in the 3rd phase - } else if (getArity(methodType.getMethod()) == argList.jjtGetNumChildren()) { + // vararg methods are considered fixed arity here, see 3rd phase + if (getArity(methodType.getMethod()) == argCount) { if (!methodType.isParameterized()) { // https://docs.oracle.com/javase/specs/jls/se8/html/jls-18.html#jls-18.5.1 // ... @@ -129,7 +128,7 @@ public final class MethodTypeResolution { // reference type, or ii) Fi is a primitive type but ei is not a standalone expression of a // primitive type; then the method is not applicable and there is no need to proceed with inference. Class[] methodParameterTypes = methodType.getMethod().getParameterTypes(); - for (int argIndex = 0; argIndex < argList.jjtGetNumChildren(); ++argIndex) { + for (int argIndex = 0; argIndex < argCount; ++argIndex) { if (((ASTExpression) argList.jjtGetChild(argIndex)).isStandAlonePrimitive()) { if (!methodParameterTypes[argIndex].isPrimitive()) { continue outter; // this method is not applicable @@ -146,7 +145,7 @@ public final class MethodTypeResolution { boolean methodIsApplicable = true; // try each arguments if it's subtypeable - for (int argIndex = 0; argIndex < argList.jjtGetNumChildren(); ++argIndex) { + for (int argIndex = 0; argIndex < argCount; ++argIndex) { if (!isSubtypeable(methodType.getParameterTypes().get(argIndex), (ASTExpression) argList.jjtGetChild(argIndex))) { methodIsApplicable = false; @@ -193,7 +192,7 @@ public final class MethodTypeResolution { int typeParamIndex = -1; if (methodParameters[i] instanceof TypeVariable) { typeParamIndex = JavaTypeDefinition - .getGenericTypeIndex(methodTypeParameters, ((TypeVariable) methodParameters[i]).getName()); + .getGenericTypeIndex(methodTypeParameters, ((TypeVariable) methodParameters[i]).getName()); } if (typeParamIndex != -1) { @@ -233,7 +232,7 @@ public final class MethodTypeResolution { int boundVarIndex = -1; if (bound instanceof TypeVariable) { boundVarIndex = - JavaTypeDefinition.getGenericTypeIndex(typeVariables, ((TypeVariable) bound).getName()); + JavaTypeDefinition.getGenericTypeIndex(typeVariables, ((TypeVariable) bound).getName()); } if (boundVarIndex != -1) { @@ -268,16 +267,15 @@ public final class MethodTypeResolution { throw new ResolutionFailedException(); } - if (argList == null) { - selectedMethods.add(methodType); + final int argCount = argList == null ? 0 : argList.jjtGetNumChildren(); - // vararg methods are considered fixed arity here, see 3rd phase - } else if (getArity(methodType.getMethod()) == argList.jjtGetNumChildren()) { + // vararg methods are considered fixed arity here, see 3rd phase + if (getArity(methodType.getMethod()) == argCount) { // check method convertability of each argument to the corresponding parameter boolean methodIsApplicable = true; // try each arguments if it's method convertible - for (int argIndex = 0; argIndex < argList.jjtGetNumChildren(); ++argIndex) { + for (int argIndex = 0; argIndex < argCount; ++argIndex) { if (!isMethodConvertible(methodType.getParameterTypes().get(argIndex), (ASTExpression) argList.jjtGetChild(argIndex))) { methodIsApplicable = false; @@ -311,37 +309,39 @@ public final class MethodTypeResolution { throw new ResolutionFailedException(); } - if (argList == null) { - selectedMethods.add(methodType); - - // now we consider varargs as not fixed arity - // if we reach here and the method is not a vararg, then we didn't find a resolution in earlier phases - } else if (methodType.isVararg()) { // check subtypeability of each argument to the corresponding parameter + // now we consider varargs as not fixed arity + // if we reach here and the method is not a vararg, then we didn't find a resolution in earlier phases + if (methodType.isVararg()) { // check subtypeability of each argument to the corresponding parameter boolean methodIsApplicable = true; List methodParameters = methodType.getParameterTypes(); JavaTypeDefinition varargComponentType = methodType.getVarargComponentType(); - // try each arguments if it's method convertible - for (int argIndex = 0; argIndex < argList.jjtGetNumChildren(); ++argIndex) { - JavaTypeDefinition parameterType = argIndex < methodParameters.size() - 1 - ? methodParameters.get(argIndex) : varargComponentType; - - if (!isMethodConvertible(parameterType, (ASTExpression) argList.jjtGetChild(argIndex))) { - methodIsApplicable = false; - break; + if (argList == null) { + // There are no arguments, make sure the method has only a vararg + methodIsApplicable = getArity(methodType.getMethod()) == 1; + } else { + // try each arguments if it's method convertible + for (int argIndex = 0; argIndex < argList.jjtGetNumChildren(); ++argIndex) { + JavaTypeDefinition parameterType = argIndex < methodParameters.size() - 1 + ? methodParameters.get(argIndex) : varargComponentType; + + if (!isMethodConvertible(parameterType, (ASTExpression) argList.jjtGetChild(argIndex))) { + methodIsApplicable = false; + break; + } + + // TODO: If k != n, or if k = n and An cannot be converted by method invocation conversion to + // Sn[], then the type which is the erasure (§4.6) of Sn is accessible at the point of invocation. + + // TODO: add unchecked conversion in an else if branch } - - // TODO: If k != n, or if k = n and An cannot be converted by method invocation conversion to - // Sn[], then the type which is the erasure (§4.6) of Sn is accessible at the point of invocation. - - // TODO: add unchecked conversion in an else if branch } if (methodIsApplicable) { selectedMethods.add(methodType); } - } else if (!methodType.isVararg()) { + } else { // TODO: Remove check for vararg here, once we can detect and use return types of method calls LOG.log(Level.FINE, "Method {0} couldn't be resolved", String.valueOf(methodType)); } From 8fe2befbbc3f229af38023fae996fefa8285b77c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Thu, 26 Oct 2017 13:11:39 -0300 Subject: [PATCH 30/62] Add a proper test case --- .../typeresolution/ClassTypeResolverTest.java | 21 +++++++++++++++++++ .../testdata/VarargsZeroArity.java | 17 +++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsZeroArity.java 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 8cf55f6788..40df739b28 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 @@ -105,6 +105,7 @@ import net.sourceforge.pmd.typeresolution.testdata.SubTypeUsage; import net.sourceforge.pmd.typeresolution.testdata.SuperExpression; import net.sourceforge.pmd.typeresolution.testdata.ThisExpression; import net.sourceforge.pmd.typeresolution.testdata.VarArgsMethodUseCase; +import net.sourceforge.pmd.typeresolution.testdata.VarargsZeroArity; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.Converter; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.JavaTypeDefinitionEquals; @@ -1521,6 +1522,26 @@ public class ClassTypeResolverTest { // Make sure we got them all assertEquals("All expressions not tested", index, expressions.size()); } + + @Test + public void testMethodTypeInferenceVarargsZeroArity() throws JaxenException { + ASTCompilationUnit acu = parseAndTypeResolveForClass15(VarargsZeroArity.class); + + List expressions = convertList( + acu.findChildNodesWithXPath("//VariableInitializer/Expression/PrimaryExpression"), + AbstractJavaTypeNode.class); + + int index = 0; + + // int var = aMethod(); + assertEquals(int.class, expressions.get(index++).getType()); + + //String var2 = aMethod(""); + assertEquals(String.class, expressions.get(index++).getType()); + + // Make sure we got them all + assertEquals("All expressions not tested", index, expressions.size()); + } @Test public void testJavaTypeDefinitionEquals() { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsZeroArity.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsZeroArity.java new file mode 100644 index 0000000000..7f1551a2a1 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsZeroArity.java @@ -0,0 +1,17 @@ +package net.sourceforge.pmd.typeresolution.testdata; + +public class VarargsZeroArity { + + public void tester() { + int var = aMethod(); + String var2 = aMethod(""); + } + + public int aMethod() { + return 0; + } + + public String aMethod(String... args) { + return null; + } +} From d8f203a859100f04c90a3b1f18917c3d2f714bed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Thu, 26 Oct 2017 18:03:54 -0300 Subject: [PATCH 31/62] Chesktyle --- .../pmd/typeresolution/testdata/VarargsZeroArity.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsZeroArity.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsZeroArity.java index 7f1551a2a1..835fde9bab 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsZeroArity.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsZeroArity.java @@ -1,3 +1,7 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + package net.sourceforge.pmd.typeresolution.testdata; public class VarargsZeroArity { From 05927af5743ceec6305d9eeb93f72fac0127e398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Thu, 26 Oct 2017 22:30:14 -0300 Subject: [PATCH 32/62] [java] Properly resolve array types - Honor dimensions of arrays - Resolve types for allocations as well as declarations of arrays --- pmd-java/etc/grammar/Java.jjt | 4 +- .../lang/java/ast/ASTArrayDimsAndInits.java | 10 ++++ .../java/ast/ASTClassOrInterfaceType.java | 8 ++++ .../symboltable/VariableNameDeclaration.java | 10 ++++ .../typeresolution/ClassTypeResolver.java | 46 ++++++------------- .../typeresolution/MethodTypeResolution.java | 1 - .../typeresolution/ClassTypeResolverTest.java | 27 +++++++++++ .../typeresolution/testdata/ArrayTypes.java | 15 ++++++ 8 files changed, 85 insertions(+), 36 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/ArrayTypes.java diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 28f615cf39..71548d28e7 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -2024,9 +2024,9 @@ void ArrayDimsAndInits() : { LOOKAHEAD(2) - ( LOOKAHEAD(2) (Annotation() {checkForBadTypeAnnotations();})* "[" Expression() "]" )+ ( LOOKAHEAD(2) "[" "]" )* + ( LOOKAHEAD(2) (Annotation() {checkForBadTypeAnnotations();})* "[" Expression() "]" {jjtThis.bumpArrayDepth();})+ ( LOOKAHEAD(2) "[" "]" {jjtThis.bumpArrayDepth();} )* | - ( "[" "]" )+ ArrayInitializer() + ( "[" "]" {jjtThis.bumpArrayDepth();})+ ArrayInitializer() } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java index e3986bc2d1..d40639bc6a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java @@ -6,6 +6,8 @@ package net.sourceforge.pmd.lang.java.ast; public class ASTArrayDimsAndInits extends AbstractJavaNode { + private int dimensions; + public ASTArrayDimsAndInits(int id) { super(id); } @@ -20,4 +22,12 @@ public class ASTArrayDimsAndInits extends AbstractJavaNode { public Object jjtAccept(JavaParserVisitor visitor, Object data) { return visitor.visit(this, data); } + + public void bumpArrayDepth() { + dimensions++; + } + + public int getDimensions() { + return dimensions; + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java index 057f2d3600..7f68048d53 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceType.java @@ -61,4 +61,12 @@ public class ASTClassOrInterfaceType extends AbstractJavaTypeNode { } return false; } + + public int getArrayDepth() { + Node p = jjtGetParent(); + if (p instanceof ASTReferenceType) { + return ((ASTReferenceType) p).getArrayDepth(); + } + return 0; + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/VariableNameDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/VariableNameDeclaration.java index b2d71756e7..3a58bd4f5c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/VariableNameDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/VariableNameDeclaration.java @@ -36,6 +36,16 @@ public class VariableNameDeclaration extends AbstractNameDeclaration implements return false; } } + + public int getArrayDepth() { + ASTVariableDeclaratorId astVariableDeclaratorId = (ASTVariableDeclaratorId) node; + ASTType typeNode = astVariableDeclaratorId.getTypeNode(); + if (typeNode != null) { + return ((Dimensionable) typeNode.jjtGetParent()).getArrayDepth(); + } else { + return 0; + } + } public boolean isVarargs() { ASTVariableDeclaratorId astVariableDeclaratorId = (ASTVariableDeclaratorId) node; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index c5206c56bf..abb23eb4b0 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -268,7 +268,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } } - populateType(node, typeName, node.isArray()); + populateType(node, typeName, node.getArrayDepth()); ASTTypeArguments typeArguments = node.getFirstChildOfType(ASTTypeArguments.class); @@ -546,14 +546,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (prefix instanceof ASTPrimaryPrefix && prefix.jjtGetParent().jjtGetNumChildren() >= 2) { - ASTArguments args = prefix.jjtGetParent().jjtGetChild(1).getFirstChildOfType(ASTArguments.class); - return args; + return prefix.jjtGetParent().jjtGetChild(1).getFirstChildOfType(ASTArguments.class); } return null; } - /** * Searches a JavaTypeDefinition and it's superclasses until a field with name {@code fieldImage} that * is visible from the {@code accessingClass} class. Once it's found, it's possibly generic type is @@ -684,7 +682,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } String name = node.getNameDeclaration().getTypeImage(); if (name != null) { - populateType(node, name, node.getNameDeclaration().isArray()); + populateType(node, name, node.getNameDeclaration().getArrayDepth()); } return super.visit(node, data); } @@ -1137,8 +1135,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { public Object visit(ASTAllocationExpression node, Object data) { super.visit(node, data); - if (node.jjtGetNumChildren() >= 2 && node.jjtGetChild(1) instanceof ASTArrayDimsAndInits - || node.jjtGetNumChildren() >= 3 && node.jjtGetChild(2) instanceof ASTArrayDimsAndInits) { + if (node.hasDescendantOfType(ASTArrayDimsAndInits.class)) { // // Classes for Array types cannot be found directly using // reflection. @@ -1147,28 +1144,11 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // dimensionality, and then ask for the type from the instance. OMFG // that's ugly. // - - // TODO Need to create utility method to allow array type creation - // which will use - // caching to avoid repeated object creation. - // TODO Modify Parser to tell us array dimensions count. - // TODO Parser seems to do some work to handle arrays in certain - // case already. - // Examine those to figure out what's going on, make sure _all_ - // array scenarios - // are ultimately covered. Appears to use a Dimensionable interface - // to handle - // only a part of the APIs (not bump), but is implemented several - // times, so - // look at refactoring to eliminate duplication. Dimensionable is - // also used - // on AccessNodes for some scenarios, need to account for that. - // Might be - // missing some TypeNode candidates we can add to the AST and have - // to deal - // with here (e.g. FormalParameter)? Plus some existing usages may - // be - // incorrect. + final Class arrayType = ((TypeNode) node.jjtGetChild(0)).getType(); + if (arrayType != null) { + final ASTArrayDimsAndInits dims = node.getFirstChildOfType(ASTArrayDimsAndInits.class); + node.setType(Array.newInstance(arrayType, (int[]) Array.newInstance(int.class, dims.getDimensions())).getClass()); + } } else { rollupTypeUnary(node); } @@ -1275,10 +1255,10 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } private void populateType(TypeNode node, String className) { - populateType(node, className, false); + populateType(node, className, 0); } - private void populateType(TypeNode node, String className, boolean isArray) { + private void populateType(TypeNode node, String className, int arrayDimens) { String qualifiedName = className; Class myType = PRIMITIVE_TYPES.get(className); @@ -1330,8 +1310,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { node.setTypeDefinition(parameter.getTypeDefinition()); } } else { - if (isArray) { - myType = Array.newInstance(myType, 0).getClass(); + if (arrayDimens > 0) { + myType = Array.newInstance(myType, (int[]) Array.newInstance(int.class, arrayDimens)).getClass(); } node.setType(myType); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java index faa593bf2b..4393a24124 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java @@ -622,7 +622,6 @@ public final class MethodTypeResolution { return false; } - public static boolean isSubtypeable(JavaTypeDefinition parameter, ASTExpression argument) { if (argument.getTypeDefinition() == null) { LOG.log(Level.FINE, "No type information for node {0}", argument.toString()); 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 8cf55f6788..608267ca84 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 @@ -71,6 +71,7 @@ import net.sourceforge.pmd.typeresolution.testdata.AnonymousClassFromInterface; import net.sourceforge.pmd.typeresolution.testdata.AnonymousInnerClass; import net.sourceforge.pmd.typeresolution.testdata.AnoymousExtendingObject; import net.sourceforge.pmd.typeresolution.testdata.ArrayListFound; +import net.sourceforge.pmd.typeresolution.testdata.ArrayTypes; import net.sourceforge.pmd.typeresolution.testdata.DefaultJavaLangImport; import net.sourceforge.pmd.typeresolution.testdata.EnumWithAnonymousInnerClass; import net.sourceforge.pmd.typeresolution.testdata.ExtraTopLevelClass; @@ -748,6 +749,32 @@ public class ClassTypeResolverTest { assertEquals("All expressions not tested", index, expressions.size()); } + @Test + public void testArrayTypes() throws JaxenException { + ASTCompilationUnit acu = parseAndTypeResolveForClass15(ArrayTypes.class); + + List expressions = convertList( + acu.findChildNodesWithXPath("//VariableDeclarator"), + AbstractJavaTypeNode.class); + + int index = 0; + + // int[] a = new int[1]; + AbstractJavaTypeNode typeNode = expressions.get(index++); + assertEquals(int[].class, typeNode.getType()); + for (AbstractJavaTypeNode n : typeNode.findDescendantsOfType(AbstractJavaTypeNode.class)) { + assertEquals(int[].class, n.getType()); + } + + // Object[][] b = new Object[1][0]; + assertEquals(Object[][].class, expressions.get(index++).getType()); + + // ArrayTypes[][][] c = new ArrayTypes[][][] { new ArrayTypes[1][2] }; + assertEquals(ArrayTypes[][][].class, expressions.get(index++).getType()); + + // Make sure we got them all + assertEquals("All expressions not tested", index, expressions.size()); + } @Test public void testFieldAccess() throws JaxenException { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/ArrayTypes.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/ArrayTypes.java new file mode 100644 index 0000000000..15dcda6bbf --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/ArrayTypes.java @@ -0,0 +1,15 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.typeresolution.testdata; + +public class ArrayTypes { + + @SuppressWarnings("unused") + public void test() { + int[] a = new int[1]; + Object[][] b = new Object[1][0]; + ArrayTypes[][][] c = new ArrayTypes[][][] { new ArrayTypes[1][2] }; + } +} From b76945bbbb1a44ca3aae986a70ec2dd36f49c226 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Thu, 26 Oct 2017 22:33:31 -0300 Subject: [PATCH 33/62] Be consistent on naming --- .../pmd/lang/java/ast/ASTArrayDimsAndInits.java | 8 ++++---- .../pmd/lang/java/typeresolution/ClassTypeResolver.java | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java index d40639bc6a..29a5eb2e57 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java @@ -6,7 +6,7 @@ package net.sourceforge.pmd.lang.java.ast; public class ASTArrayDimsAndInits extends AbstractJavaNode { - private int dimensions; + private int arrayDepth; public ASTArrayDimsAndInits(int id) { super(id); @@ -24,10 +24,10 @@ public class ASTArrayDimsAndInits extends AbstractJavaNode { } public void bumpArrayDepth() { - dimensions++; + arrayDepth++; } - public int getDimensions() { - return dimensions; + public int getArrayDepth() { + return arrayDepth; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index abb23eb4b0..192c01c850 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -1147,7 +1147,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { final Class arrayType = ((TypeNode) node.jjtGetChild(0)).getType(); if (arrayType != null) { final ASTArrayDimsAndInits dims = node.getFirstChildOfType(ASTArrayDimsAndInits.class); - node.setType(Array.newInstance(arrayType, (int[]) Array.newInstance(int.class, dims.getDimensions())).getClass()); + node.setType(Array.newInstance(arrayType, (int[]) Array.newInstance(int.class, dims.getArrayDepth())).getClass()); } } else { rollupTypeUnary(node); From b3dea3240c2c06bcbd2b1bd77ac41f3a332735b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Thu, 26 Oct 2017 22:38:40 -0300 Subject: [PATCH 34/62] Generalize test --- .../typeresolution/ClassTypeResolverTest.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) 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 608267ca84..4cf7c959f5 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 @@ -760,21 +760,25 @@ public class ClassTypeResolverTest { int index = 0; // int[] a = new int[1]; - AbstractJavaTypeNode typeNode = expressions.get(index++); - assertEquals(int[].class, typeNode.getType()); - for (AbstractJavaTypeNode n : typeNode.findDescendantsOfType(AbstractJavaTypeNode.class)) { - assertEquals(int[].class, n.getType()); - } + testSubtreeNodeTypes(expressions.get(index++), int[].class); // Object[][] b = new Object[1][0]; - assertEquals(Object[][].class, expressions.get(index++).getType()); + testSubtreeNodeTypes(expressions.get(index++), Object[][].class); // ArrayTypes[][][] c = new ArrayTypes[][][] { new ArrayTypes[1][2] }; - assertEquals(ArrayTypes[][][].class, expressions.get(index++).getType()); + testSubtreeNodeTypes(expressions.get(index++), ArrayTypes[][][].class); // Make sure we got them all assertEquals("All expressions not tested", index, expressions.size()); } + + private void testSubtreeNodeTypes(final AbstractJavaTypeNode node, final Class expectedType) { + assertEquals(expectedType, node.getType()); + // Check all typeable nodes in the tree + for (AbstractJavaTypeNode n : node.findDescendantsOfType(AbstractJavaTypeNode.class)) { + assertEquals(expectedType, n.getType()); + } + } @Test public void testFieldAccess() throws JaxenException { From 1d02c24b347b2b78222d009651da0dd6eda8a1c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 27 Oct 2017 00:08:26 -0300 Subject: [PATCH 35/62] Consider only direct children --- .../lang/java/typeresolution/ClassTypeResolver.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 192c01c850..2904621937 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -1135,18 +1135,10 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { public Object visit(ASTAllocationExpression node, Object data) { super.visit(node, data); - if (node.hasDescendantOfType(ASTArrayDimsAndInits.class)) { - // - // Classes for Array types cannot be found directly using - // reflection. - // As far as I can tell you have to create an array instance of the - // necessary - // dimensionality, and then ask for the type from the instance. OMFG - // that's ugly. - // + final ASTArrayDimsAndInits dims = node.getFirstChildOfType(ASTArrayDimsAndInits.class); + if (dims != null) { final Class arrayType = ((TypeNode) node.jjtGetChild(0)).getType(); if (arrayType != null) { - final ASTArrayDimsAndInits dims = node.getFirstChildOfType(ASTArrayDimsAndInits.class); node.setType(Array.newInstance(arrayType, (int[]) Array.newInstance(int.class, dims.getArrayDepth())).getClass()); } } else { From 83d513edcabbbf5a01476d702c7798ee45ae65b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 27 Oct 2017 00:55:33 -0300 Subject: [PATCH 36/62] Fix specificity test for varargs vs fixed - We used to fail upon mixing varargs with fixed arity instead of treating the vararg as a fixed arity as per the spec --- .../typeresolution/MethodTypeResolution.java | 35 ++++++------------- .../typeresolution/ClassTypeResolverTest.java | 30 ++++++++++++++++ .../testdata/VarargsAsFixedArity.java | 23 ++++++++++++ 3 files changed, 63 insertions(+), 25 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsAsFixedArity.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java index 8e0b0c31f9..d0f9567bc1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java @@ -69,34 +69,21 @@ public final class MethodTypeResolution { List subtypeableParams = subtypeableMethod.getParameterTypes(); List methodParams = method.getParameterTypes(); - - if (!method.getMethod().isVarArgs() && !subtypeableMethod.getMethod().isVarArgs()) { + // If we come from third-phase, both are varargs, otherwhise, treat all as fixed-arity + if (!method.getMethod().isVarArgs() || !subtypeableMethod.getMethod().isVarArgs()) { for (int index = 0; index < subtypeableParams.size(); ++index) { if (!isSubtypeable(methodParams.get(index), subtypeableParams.get(index))) { return false; } } - } else if (method.getMethod().isVarArgs() && subtypeableMethod.getMethod().isVarArgs()) { - - if (methodParams.size() < subtypeableParams.size()) { - for (int index = 0; index < subtypeableParams.size(); ++index) { - if (!isSubtypeable(method.getArgTypeIncludingVararg(index), - subtypeableMethod.getArgTypeIncludingVararg(index))) { - return false; - } - } - } else { - for (int index = 0; index < methodParams.size(); ++index) { - if (!isSubtypeable(method.getArgTypeIncludingVararg(index), - subtypeableMethod.getArgTypeIncludingVararg(index))) { - return false; - } + } else { + final int maxSize = Math.max(subtypeableParams.size(), methodParams.size()); + for (int index = 0; index < maxSize; ++index) { + if (!isSubtypeable(method.getArgTypeIncludingVararg(index), + subtypeableMethod.getArgTypeIncludingVararg(index))) { + return false; } } - - } else { - throw new IllegalStateException("These methods can only be vararg at the same time:\n" - + method.toString() + "\n" + subtypeableMethod.toString()); } return true; @@ -110,13 +97,12 @@ public final class MethodTypeResolution { List methodsToSearch, ASTArgumentList argList) { // TODO: check if explicit type arguments are applicable to the type parameter bounds List selectedMethods = new ArrayList<>(); + final int argCount = argList == null ? 0 : argList.jjtGetNumChildren(); outter: for (int methodIndex = 0; methodIndex < methodsToSearch.size(); ++methodIndex) { MethodType methodType = methodsToSearch.get(methodIndex); - final int argCount = argList == null ? 0 : argList.jjtGetNumChildren(); - // vararg methods are considered fixed arity here, see 3rd phase if (getArity(methodType.getMethod()) == argCount) { if (!methodType.isParameterized()) { @@ -260,6 +246,7 @@ public final class MethodTypeResolution { public static List selectMethodsSecondPhase(List methodsToSearch, ASTArgumentList argList) { // TODO: check if explicit type arguments are applicable to the type parameter bounds List selectedMethods = new ArrayList<>(); + final int argCount = argList == null ? 0 : argList.jjtGetNumChildren(); for (int methodIndex = 0; methodIndex < methodsToSearch.size(); ++methodIndex) { MethodType methodType = methodsToSearch.get(methodIndex); @@ -267,8 +254,6 @@ public final class MethodTypeResolution { throw new ResolutionFailedException(); } - final int argCount = argList == null ? 0 : argList.jjtGetNumChildren(); - // vararg methods are considered fixed arity here, see 3rd phase if (getArity(methodType.getMethod()) == argCount) { // check method convertability of each argument to the corresponding parameter 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 40df739b28..3ed5700ea6 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 @@ -105,6 +105,7 @@ import net.sourceforge.pmd.typeresolution.testdata.SubTypeUsage; import net.sourceforge.pmd.typeresolution.testdata.SuperExpression; import net.sourceforge.pmd.typeresolution.testdata.ThisExpression; import net.sourceforge.pmd.typeresolution.testdata.VarArgsMethodUseCase; +import net.sourceforge.pmd.typeresolution.testdata.VarargsAsFixedArity; import net.sourceforge.pmd.typeresolution.testdata.VarargsZeroArity; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.Converter; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.GenericClass; @@ -1542,6 +1543,35 @@ public class ClassTypeResolverTest { // Make sure we got them all assertEquals("All expressions not tested", index, expressions.size()); } + + @Test + public void testMethodTypeInferenceVarargsAsFixedArity() throws JaxenException { + ASTCompilationUnit acu = parseAndTypeResolveForClass15(VarargsAsFixedArity.class); + + List expressions = convertList( + acu.findChildNodesWithXPath("//VariableInitializer/Expression/PrimaryExpression"), + AbstractJavaTypeNode.class); + + int index = 0; + + // int var = aMethod(""); + assertEquals(int.class, expressions.get(index++).getType()); + + // String var2 = aMethod(); + assertEquals(String.class, expressions.get(index++).getType()); + + // String var3 = aMethod("", ""); + assertEquals(String.class, expressions.get(index++).getType()); + + // String var4 = aMethod(new Object[] { null }); + assertEquals(String.class, expressions.get(index++).getType()); + + // null literal has null type + assertNull(expressions.get(index++).getType()); + + // Make sure we got them all + assertEquals("All expressions not tested", index, expressions.size()); + } @Test public void testJavaTypeDefinitionEquals() { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsAsFixedArity.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsAsFixedArity.java new file mode 100644 index 0000000000..11df97d3dc --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsAsFixedArity.java @@ -0,0 +1,23 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.typeresolution.testdata; + +public class VarargsAsFixedArity { + + public void tester() { + int var = aMethod(""); + String var2 = aMethod(); + String var3 = aMethod("", ""); + String var4 = aMethod(new Object[] { null }); + } + + public int aMethod(Object s) { + return 0; + } + + public String aMethod(Object... s) { + return null; + } +} From 71d94b4ccd0a1f2b6897580f9ad2d6fad49ac340 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 27 Oct 2017 02:26:23 -0300 Subject: [PATCH 37/62] [java] Avoid runtime errors on incomplete classpath --- .../lang/java/typeresolution/MethodTypeResolution.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java index faa593bf2b..4bcfcdcf24 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java @@ -450,10 +450,14 @@ public final class MethodTypeResolution { Class contextClass = context.getType(); // search the class - for (Method method : contextClass.getDeclaredMethods()) { - if (isMethodApplicable(method, methodName, argArity, accessingClass, typeArguments)) { - result.add(getTypeDefOfMethod(context, method, typeArguments)); + try { + for (Method method : contextClass.getDeclaredMethods()) { + if (isMethodApplicable(method, methodName, argArity, accessingClass, typeArguments)) { + result.add(getTypeDefOfMethod(context, method, typeArguments)); + } } + } catch (final LinkageError ignored) { + // TODO : This is an incomplete classpath, report the missing class } // search it's supertype From d1d9c6f3c6259a8a7bdf4d9f6b6a6c47d2554151 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 27 Oct 2017 23:40:47 -0300 Subject: [PATCH 38/62] [java] Fix NPE in type resolution - The resolution of `GenericArrayType` was broken, so we caused NPEs. - Those types are now properly resolved, but the test case is still incomplete due to missing pieces in type inference --- .../typeresolution/ClassTypeResolver.java | 2 +- .../JavaTypeDefinitionSimple.java | 8 ++--- .../typeresolution/ClassTypeResolverTest.java | 31 +++++++++++++++++++ .../testdata/GenericsArrays.java | 18 +++++++++++ 4 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericsArrays.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index c5206c56bf..62a3a64b44 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -419,7 +419,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (node.getType() != null) { // static field or method // node.getType() has been set by the call to searchNodeNameForClass above // node.getType() will have the value equal to the Class found by that method - previousType = JavaTypeDefinition.forClass(node.getType()); + previousType = node.getTypeDefinition(); } else { // non-static field or method if (dotSplitImage.length == 1 && astArguments != null) { // method List methods = getLocalApplicableMethods(node, dotSplitImage[0], diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java index c534513de3..9696ed9503 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java @@ -85,11 +85,11 @@ import java.util.logging.Logger; } private JavaTypeDefinition getGenericType(final String parameterName, Method method, - List methodTypeArgumens) { - if (method != null && methodTypeArgumens != null) { + List methodTypeArguments) { + if (method != null && methodTypeArguments != null) { int paramIndex = getGenericTypeIndex(method.getTypeParameters(), parameterName); if (paramIndex != -1) { - return methodTypeArgumens.get(paramIndex); + return methodTypeArguments.get(paramIndex); } } @@ -190,7 +190,7 @@ import java.util.logging.Logger; return forClass(UPPER_WILDCARD, resolveTypeDefinition(wildcardUpperBounds[0], method, methodTypeArgs)); } } else if (type instanceof GenericArrayType) { - JavaTypeDefinition component = resolveTypeDefinition(((GenericArrayType) type).getGenericComponentType()); + JavaTypeDefinition component = resolveTypeDefinition(((GenericArrayType) type).getGenericComponentType(), method, methodTypeArgs); // TODO: retain the generic types of the array component... return forClass(Array.newInstance(component.getType(), 0).getClass()); } 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 8cf55f6788..5b0a3918bd 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 @@ -86,6 +86,7 @@ import net.sourceforge.pmd.typeresolution.testdata.FieldAccessShadow; import net.sourceforge.pmd.typeresolution.testdata.FieldAccessStatic; import net.sourceforge.pmd.typeresolution.testdata.FieldAccessSuper; import net.sourceforge.pmd.typeresolution.testdata.GenericMethodsImplicit; +import net.sourceforge.pmd.typeresolution.testdata.GenericsArrays; import net.sourceforge.pmd.typeresolution.testdata.InnerClass; import net.sourceforge.pmd.typeresolution.testdata.Literals; import net.sourceforge.pmd.typeresolution.testdata.MethodAccessibility; @@ -1503,6 +1504,36 @@ public class ClassTypeResolverTest { } + @Test + public void testGenericArrays() throws JaxenException { + ASTCompilationUnit acu = parseAndTypeResolveForClass15(GenericsArrays.class); + + List expressions = convertList( + acu.findChildNodesWithXPath("//VariableInitializer/Expression/PrimaryExpression"), + AbstractJavaTypeNode.class); + + int index = 0; + + // List var = Arrays.asList(params); + AbstractJavaTypeNode expression = expressions.get(index++); + // TODO : Type inference is still incomplete, we fail to detect the return type of the method + //assertEquals(List.class, expression.getTypeDefinition().getType()); + //assertEquals(String.class, expression.getTypeDefinition().getGenericType(0).getType()); + + // List var2 = Arrays.asList(params); + AbstractJavaTypeNode expression2 = expressions.get(index++); + assertEquals(List.class, expression2.getTypeDefinition().getType()); + assertEquals(String.class, expression2.getTypeDefinition().getGenericType(0).getType()); + + // List var3 = Arrays.asList(params); + AbstractJavaTypeNode expression3 = expressions.get(index++); + assertEquals(List.class, expression3.getTypeDefinition().getType()); + assertEquals(String[].class, expression3.getTypeDefinition().getGenericType(0).getType()); + + // Make sure we got them all + assertEquals("All expressions not tested", index, expressions.size()); + } + @Test public void testMethodTypeInference() throws JaxenException { ASTCompilationUnit acu = parseAndTypeResolveForClass15(GenericMethodsImplicit.class); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericsArrays.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericsArrays.java new file mode 100644 index 0000000000..096092dea1 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/GenericsArrays.java @@ -0,0 +1,18 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.typeresolution.testdata; + +import java.util.Arrays; +import java.util.List; + +public class GenericsArrays { + + @SuppressWarnings("unused") + public void test(String[] params) { + List var = Arrays.asList(params); + List var2 = Arrays.asList(params); + List var3 = Arrays.asList(params); + } +} From 20f16762f271e8c9275a19ad52931375089af3af Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 28 Oct 2017 19:19:27 +0200 Subject: [PATCH 39/62] Update release notes, refs #680 --- 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 c402b1dd2e..1ec21978cb 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -259,6 +259,7 @@ a warning will now be produced suggesting users to adopt it for better performan * [#608](https://github.com/pmd/pmd/issues/608): \[core] Add DEBUG log when applying incremental analysis * [#618](https://github.com/pmd/pmd/issues/618): \[core] Incremental Analysis doesn't close file correctly on Windows upon a cache hit * [#643](https://github.com/pmd/pmd/issues/643): \[core] PMD Properties (dev-properties) breaks markup on CodeClimateRenderer + * [#680](https://github.com/pmd/pmd/pull/680): \[core] Isolate classloaders for runtime and auxclasspath * apex * [#265](https://github.com/pmd/pmd/issues/265): \[apex] Make Rule suppression work * [#488](https://github.com/pmd/pmd/pull/488): \[apex] Use Apex lexer for CPD From 2d0133263583985e85504a4264c7e47312e22af9 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Fri, 20 Oct 2017 19:35:23 -0300 Subject: [PATCH 40/62] Convert GenericToken from class to interface && Add RegionByLine --- .../pmd/lang/ast/GenericToken.java | 29 +++++++- .../pmd/lang/ast/RegionByLine.java | 35 +++++++++ .../pmd/lang/ast/RegionByLineImpl.java | 72 +++++++++++++++++++ 3 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLine.java create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java index 8750e01ae4..d9211d3f67 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java @@ -4,6 +4,33 @@ package net.sourceforge.pmd.lang.ast; -public class GenericToken { +/** + * Represents a language-independent token such as constants, values language reserved keywords, or comments. + */ +public interface GenericToken { + /** + * Obtain the next generic token according to the input stream which generated the instance of this token. + * @return the next generic token if it exists; null if it does not exist + */ + GenericToken getNextGenericToken(); + + /** + * Obtain a special generic token which, according to the input stream which generated the instance of this token, + * precedes this instance token and succeeds the previous generic token (if there is any). + * @return the special token if it exists; null if it does not exist + */ + GenericToken getPreviousSpecialGenericToken(); + + /** + * Obtain the region where the token occupies in the source file. + * @return the region + */ + RegionByLine getLineRegion(); + + /** + * Gets the string representation of the instance of this token. + * @return the string representing this token + */ + String getImage(); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLine.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLine.java new file mode 100644 index 0000000000..e0418b3993 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLine.java @@ -0,0 +1,35 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.ast; + +/** + * Represents a region in a source file bounded by a (beginLine, beginColumn) and (endLine, endColumn). + */ +public interface RegionByLine { + + /** + * Gets the line where the region begins + * @return a non-negative integer containing the begin line + */ + int getBeginLine(); + + /** + * Gets the line where the region ends + * @return a non-negative integer containing the end line + */ + int getEndLine(); + + /** + * Gets the column offset from the start of the begin line where the region begins + * @return a non-negative integer containing the begin column + */ + int getBeginColumn(); + + /** + * Gets the column offset from the start of the end line where the region ends + * @return a non-negative integer containing the begin column + */ + int getEndColumn(); +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java new file mode 100644 index 0000000000..79435a42f5 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java @@ -0,0 +1,72 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.ast; + +/** + * Default implementation for the RegionByLine interface which is used by {@link GenericToken} + */ +public class RegionByLineImpl implements RegionByLine { + private int beginLine; + private int endLine; + private int beginColumn; + private int endColumn; + + /** + * + * @param beginLine + * @param endLine + * @param beginColumn + * @param endColumn + */ + public RegionByLineImpl(final int beginLine, final int endLine, final int beginColumn, final int endColumn) { + setBeginLine(beginLine); + setEndLine(endLine); + setBeginColumn(beginColumn); + setEndColumn(endColumn); + } + + private void setBeginLine(final int beginLine) { + this.beginLine = requireNonNegative(beginLine); + } + + private void setEndLine(final int endLine) { + this.endLine = requireNonNegative(endLine); + } + + private void setBeginColumn(final int beginColumn) { + this.beginColumn = requireNonNegative(beginColumn); + } + + private void setEndColumn(final int endColumn) { + this.endColumn = requireNonNegative(endColumn); + } + + private int requireNonNegative(final int value) { + if (value < 0) { + throw new IllegalArgumentException("value = " + value + " must be non-negative"); + } + return value; + } + + @Override + public int getBeginLine() { + return beginLine; + } + + @Override + public int getEndLine() { + return endLine; + } + + @Override + public int getBeginColumn() { + return beginColumn; + } + + @Override + public int getEndColumn() { + return endColumn; + } +} From 490b34fea3aabd98e9408beb6c02c6c2d3a245e2 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Fri, 20 Oct 2017 19:36:19 -0300 Subject: [PATCH 41/62] Change ant tasks over (Java) Token class --- pmd-java/src/main/ant/alljavacc.xml | 41 +++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/pmd-java/src/main/ant/alljavacc.xml b/pmd-java/src/main/ant/alljavacc.xml index 45c05dac70..7e3407c074 100644 --- a/pmd-java/src/main/ant/alljavacc.xml +++ b/pmd-java/src/main/ant/alljavacc.xml @@ -60,6 +60,7 @@ public class]]> + } ]]> - - public class Token - - + + public class Token implements java.io.Serializable + + + + + + specialToken; + + From 020abafeab558e881e6c7fb76830152362b448c0 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Fri, 20 Oct 2017 19:36:29 -0300 Subject: [PATCH 42/62] Change ant tasks over (JSP) Token class --- pmd-jsp/src/main/ant/alljavacc.xml | 35 +++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/pmd-jsp/src/main/ant/alljavacc.xml b/pmd-jsp/src/main/ant/alljavacc.xml index da338412b9..5990d959e6 100644 --- a/pmd-jsp/src/main/ant/alljavacc.xml +++ b/pmd-jsp/src/main/ant/alljavacc.xml @@ -62,12 +62,41 @@ public class]]> - public class Token - public class Token implements java.io.Serializable + +public class Token implements GenericToken, java.io.Serializable]]> + + + specialToken; + + + From 00ce16c177ac749723a34cd6b364393378074ce1 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Sat, 21 Oct 2017 14:00:19 -0300 Subject: [PATCH 43/62] [Not Working] Change ant tasks over (VisualForce) Token class --- pmd-visualforce/src/main/ant/alljavacc.xml | 31 ++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/pmd-visualforce/src/main/ant/alljavacc.xml b/pmd-visualforce/src/main/ant/alljavacc.xml index 9637b35151..2e577eca67 100644 --- a/pmd-visualforce/src/main/ant/alljavacc.xml +++ b/pmd-visualforce/src/main/ant/alljavacc.xml @@ -63,12 +63,39 @@ public class]]> - public class Token + public class Token implements java.io.Serializable +public class Token implements GenericToken, java.io.Serializable]]> + + specialToken; + + + From 00ed0a1eabb01116755ed16d37f24f09db34fdf2 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Sat, 21 Oct 2017 19:16:38 -0300 Subject: [PATCH 44/62] Add RegionByLineImpl javadoc --- .../sourceforge/pmd/lang/ast/RegionByLineImpl.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java index 79435a42f5..190b8be540 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java @@ -5,7 +5,7 @@ package net.sourceforge.pmd.lang.ast; /** - * Default implementation for the RegionByLine interface which is used by {@link GenericToken} + * Default immutable implementation for the RegionByLine interface which is used by {@link GenericToken} */ public class RegionByLineImpl implements RegionByLine { private int beginLine; @@ -14,11 +14,11 @@ public class RegionByLineImpl implements RegionByLine { private int endColumn; /** - * - * @param beginLine - * @param endLine - * @param beginColumn - * @param endColumn + * Create an immutable instance with all the corresponding fields. Every field requires to be non-negative + * @param beginLine the line where the region begins + * @param endLine the line where the region ends + * @param beginColumn the column offset from the start of the begin line where the region begins + * @param endColumn the column offset from the start of the end line where the region ends */ public RegionByLineImpl(final int beginLine, final int endLine, final int beginColumn, final int endColumn) { setBeginLine(beginLine); From 95835b813447fb5d42a284082a159266aba6c6e8 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Sat, 21 Oct 2017 19:51:30 -0300 Subject: [PATCH 45/62] Update getter in GenericToken --- .../main/java/net/sourceforge/pmd/lang/ast/GenericToken.java | 2 +- pmd-java/src/main/ant/alljavacc.xml | 2 +- pmd-jsp/src/main/ant/alljavacc.xml | 2 +- pmd-visualforce/src/main/ant/alljavacc.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java index d9211d3f67..5bac220dc2 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java @@ -26,7 +26,7 @@ public interface GenericToken { * Obtain the region where the token occupies in the source file. * @return the region */ - RegionByLine getLineRegion(); + RegionByLine getRegionByLine(); /** * Gets the string representation of the instance of this token. diff --git a/pmd-java/src/main/ant/alljavacc.xml b/pmd-java/src/main/ant/alljavacc.xml index 7e3407c074..0526d77079 100644 --- a/pmd-java/src/main/ant/alljavacc.xml +++ b/pmd-java/src/main/ant/alljavacc.xml @@ -108,7 +108,7 @@ public class Token implements GenericToken, java.io.Serializable]]> Date: Sat, 21 Oct 2017 23:07:06 -0300 Subject: [PATCH 46/62] Simplify method name in GenericToken && improve ant task replacetoken --- .../java/net/sourceforge/pmd/lang/ast/GenericToken.java | 6 +++--- pmd-java/src/main/ant/alljavacc.xml | 6 +++--- pmd-jsp/src/main/ant/alljavacc.xml | 6 +++--- pmd-visualforce/src/main/ant/alljavacc.xml | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java index 5bac220dc2..9d4c4a6497 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java @@ -13,7 +13,7 @@ public interface GenericToken { * Obtain the next generic token according to the input stream which generated the instance of this token. * @return the next generic token if it exists; null if it does not exist */ - GenericToken getNextGenericToken(); + GenericToken getNext(); /** * Obtain a special generic token which, according to the input stream which generated the instance of this token, @@ -29,8 +29,8 @@ public interface GenericToken { RegionByLine getRegionByLine(); /** - * Gets the string representation of the instance of this token. - * @return the string representing this token + * Gets the token's text. + * @return the token's text */ String getImage(); } diff --git a/pmd-java/src/main/ant/alljavacc.xml b/pmd-java/src/main/ant/alljavacc.xml index 0526d77079..2e6c73c31c 100644 --- a/pmd-java/src/main/ant/alljavacc.xml +++ b/pmd-java/src/main/ant/alljavacc.xml @@ -94,11 +94,11 @@ public class Token implements GenericToken, java.io.Serializable]]> - specialToken; - public Token specialToken; + - specialToken; - public Token specialToken; + - specialToken; - public Token specialToken; + Date: Sun, 22 Oct 2017 12:41:09 -0300 Subject: [PATCH 47/62] Change ant tasks over (Ecmascript5) Token class --- pmd-javascript/src/main/ant/alljavacc.xml | 36 +++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/pmd-javascript/src/main/ant/alljavacc.xml b/pmd-javascript/src/main/ant/alljavacc.xml index 15cf978038..1399f40f68 100644 --- a/pmd-javascript/src/main/ant/alljavacc.xml +++ b/pmd-javascript/src/main/ant/alljavacc.xml @@ -40,5 +40,41 @@ + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + From f29e3f0408eb89da7aa093f7625ef4fba021e42f Mon Sep 17 00:00:00 2001 From: gonzalo Date: Sun, 22 Oct 2017 12:44:14 -0300 Subject: [PATCH 48/62] Change ant tasks over (CPP) Token class --- pmd-cpp/src/main/ant/alljavacc.xml | 37 ++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/pmd-cpp/src/main/ant/alljavacc.xml b/pmd-cpp/src/main/ant/alljavacc.xml index 4066ee3a6f..de7a5f0955 100644 --- a/pmd-cpp/src/main/ant/alljavacc.xml +++ b/pmd-cpp/src/main/ant/alljavacc.xml @@ -40,6 +40,43 @@ + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + + From 026576e2666d77166bb99b5589c4d3f226e64201 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Sun, 22 Oct 2017 12:45:15 -0300 Subject: [PATCH 49/62] Change ant tasks over (Matlab) Token class --- pmd-matlab/src/main/ant/alljavacc.xml | 36 +++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/pmd-matlab/src/main/ant/alljavacc.xml b/pmd-matlab/src/main/ant/alljavacc.xml index e17d98fb20..76819e4e64 100644 --- a/pmd-matlab/src/main/ant/alljavacc.xml +++ b/pmd-matlab/src/main/ant/alljavacc.xml @@ -40,6 +40,42 @@ + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + From da902d658f18490a6158266af80bd91965e9c763 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Sun, 22 Oct 2017 12:47:11 -0300 Subject: [PATCH 50/62] Change ant tasks over (Objective-C) Token class --- pmd-objectivec/src/main/ant/alljavacc.xml | 36 +++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/pmd-objectivec/src/main/ant/alljavacc.xml b/pmd-objectivec/src/main/ant/alljavacc.xml index 5396813b9a..b2cddbfd39 100644 --- a/pmd-objectivec/src/main/ant/alljavacc.xml +++ b/pmd-objectivec/src/main/ant/alljavacc.xml @@ -40,6 +40,42 @@ + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + From 1c0d762a372895adec2ab83fe7d029b2241aa12f Mon Sep 17 00:00:00 2001 From: gonzalo Date: Sun, 22 Oct 2017 12:50:25 -0300 Subject: [PATCH 51/62] Change ant tasks over (PL/SQL) Token class --- pmd-plsql/src/main/ant/alljavacc.xml | 36 ++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/pmd-plsql/src/main/ant/alljavacc.xml b/pmd-plsql/src/main/ant/alljavacc.xml index 2cb7679a0f..62bcd25daa 100644 --- a/pmd-plsql/src/main/ant/alljavacc.xml +++ b/pmd-plsql/src/main/ant/alljavacc.xml @@ -78,5 +78,41 @@ public class]]> token="extends Exception" value="extends net.sourceforge.pmd.lang.ast.ParseException" /> + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + + From c606d409f44dc9ace9742112fd9721e24b7b0cc3 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Sun, 22 Oct 2017 12:51:49 -0300 Subject: [PATCH 52/62] Change ant tasks over (Python) Token class --- pmd-python/src/main/ant/alljavacc.xml | 36 +++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/pmd-python/src/main/ant/alljavacc.xml b/pmd-python/src/main/ant/alljavacc.xml index 06b4bfddcc..4e1df8fd97 100644 --- a/pmd-python/src/main/ant/alljavacc.xml +++ b/pmd-python/src/main/ant/alljavacc.xml @@ -40,6 +40,42 @@ + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + From 0e011767ce509e1766143cfc2ff995034c7c5920 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Sun, 22 Oct 2017 12:53:23 -0300 Subject: [PATCH 53/62] Change ant tasks over (VM) Token class --- pmd-vm/src/main/ant/alljavacc.xml | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/pmd-vm/src/main/ant/alljavacc.xml b/pmd-vm/src/main/ant/alljavacc.xml index cab38ed72f..2878476951 100644 --- a/pmd-vm/src/main/ant/alljavacc.xml +++ b/pmd-vm/src/main/ant/alljavacc.xml @@ -73,5 +73,41 @@ public class]]> + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + From e8fdbdbca4797568e69e5de37a330fb22f5bfea7 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Sun, 22 Oct 2017 21:10:43 -0300 Subject: [PATCH 54/62] Update GenericToken interface to expose directly region methods --- .../pmd/lang/ast/GenericToken.java | 30 ++++++-- .../pmd/lang/ast/RegionByLine.java | 35 --------- .../pmd/lang/ast/RegionByLineImpl.java | 72 ------------------- pmd-cpp/src/main/ant/alljavacc.xml | 28 ++++++-- pmd-java/src/main/ant/alljavacc.xml | 28 ++++++-- pmd-javascript/src/main/ant/alljavacc.xml | 28 ++++++-- pmd-jsp/src/main/ant/alljavacc.xml | 28 ++++++-- pmd-matlab/src/main/ant/alljavacc.xml | 28 ++++++-- pmd-objectivec/src/main/ant/alljavacc.xml | 28 ++++++-- pmd-plsql/src/main/ant/alljavacc.xml | 28 ++++++-- pmd-python/src/main/ant/alljavacc.xml | 28 ++++++-- pmd-visualforce/src/main/ant/alljavacc.xml | 29 ++++++-- pmd-vm/src/main/ant/alljavacc.xml | 28 ++++++-- 13 files changed, 235 insertions(+), 183 deletions(-) delete mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLine.java delete mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java index 9d4c4a6497..859cb1e1f8 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java @@ -22,15 +22,33 @@ public interface GenericToken { */ GenericToken getPreviousSpecialGenericToken(); - /** - * Obtain the region where the token occupies in the source file. - * @return the region - */ - RegionByLine getRegionByLine(); - /** * Gets the token's text. * @return the token's text */ String getImage(); + + /** + * Gets the line where the token's region begins + * @return a non-negative integer containing the begin line + */ + int getBeginLine(); + + /** + * Gets the line where the token's region ends + * @return a non-negative integer containing the end line + */ + int getEndLine(); + + /** + * Gets the column offset from the start of the begin line where the token's region begins + * @return a non-negative integer containing the begin column + */ + int getBeginColumn(); + + /** + * Gets the column offset from the start of the end line where the token's region ends + * @return a non-negative integer containing the begin column + */ + int getEndColumn(); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLine.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLine.java deleted file mode 100644 index e0418b3993..0000000000 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLine.java +++ /dev/null @@ -1,35 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.ast; - -/** - * Represents a region in a source file bounded by a (beginLine, beginColumn) and (endLine, endColumn). - */ -public interface RegionByLine { - - /** - * Gets the line where the region begins - * @return a non-negative integer containing the begin line - */ - int getBeginLine(); - - /** - * Gets the line where the region ends - * @return a non-negative integer containing the end line - */ - int getEndLine(); - - /** - * Gets the column offset from the start of the begin line where the region begins - * @return a non-negative integer containing the begin column - */ - int getBeginColumn(); - - /** - * Gets the column offset from the start of the end line where the region ends - * @return a non-negative integer containing the begin column - */ - int getEndColumn(); -} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java deleted file mode 100644 index 190b8be540..0000000000 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/RegionByLineImpl.java +++ /dev/null @@ -1,72 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.ast; - -/** - * Default immutable implementation for the RegionByLine interface which is used by {@link GenericToken} - */ -public class RegionByLineImpl implements RegionByLine { - private int beginLine; - private int endLine; - private int beginColumn; - private int endColumn; - - /** - * Create an immutable instance with all the corresponding fields. Every field requires to be non-negative - * @param beginLine the line where the region begins - * @param endLine the line where the region ends - * @param beginColumn the column offset from the start of the begin line where the region begins - * @param endColumn the column offset from the start of the end line where the region ends - */ - public RegionByLineImpl(final int beginLine, final int endLine, final int beginColumn, final int endColumn) { - setBeginLine(beginLine); - setEndLine(endLine); - setBeginColumn(beginColumn); - setEndColumn(endColumn); - } - - private void setBeginLine(final int beginLine) { - this.beginLine = requireNonNegative(beginLine); - } - - private void setEndLine(final int endLine) { - this.endLine = requireNonNegative(endLine); - } - - private void setBeginColumn(final int beginColumn) { - this.beginColumn = requireNonNegative(beginColumn); - } - - private void setEndColumn(final int endColumn) { - this.endColumn = requireNonNegative(endColumn); - } - - private int requireNonNegative(final int value) { - if (value < 0) { - throw new IllegalArgumentException("value = " + value + " must be non-negative"); - } - return value; - } - - @Override - public int getBeginLine() { - return beginLine; - } - - @Override - public int getEndLine() { - return endLine; - } - - @Override - public int getBeginColumn() { - return beginColumn; - } - - @Override - public int getEndColumn() { - return endColumn; - } -} diff --git a/pmd-cpp/src/main/ant/alljavacc.xml b/pmd-cpp/src/main/ant/alljavacc.xml index de7a5f0955..6092c621e1 100644 --- a/pmd-cpp/src/main/ant/alljavacc.xml +++ b/pmd-cpp/src/main/ant/alljavacc.xml @@ -44,8 +44,6 @@ public class Token implements java.io.Serializable @@ -65,15 +63,31 @@ public class Token implements GenericToken, java.io.Serializable]]> diff --git a/pmd-java/src/main/ant/alljavacc.xml b/pmd-java/src/main/ant/alljavacc.xml index 2e6c73c31c..df3c24e40f 100644 --- a/pmd-java/src/main/ant/alljavacc.xml +++ b/pmd-java/src/main/ant/alljavacc.xml @@ -86,8 +86,6 @@ public class]]> public class Token implements java.io.Serializable @@ -107,15 +105,31 @@ public class Token implements GenericToken, java.io.Serializable]]> diff --git a/pmd-javascript/src/main/ant/alljavacc.xml b/pmd-javascript/src/main/ant/alljavacc.xml index 1399f40f68..98266c3118 100644 --- a/pmd-javascript/src/main/ant/alljavacc.xml +++ b/pmd-javascript/src/main/ant/alljavacc.xml @@ -44,8 +44,6 @@ public class Token implements java.io.Serializable @@ -65,15 +63,31 @@ public class Token implements GenericToken, java.io.Serializable]]> diff --git a/pmd-jsp/src/main/ant/alljavacc.xml b/pmd-jsp/src/main/ant/alljavacc.xml index fb65d96335..677ed0ebeb 100644 --- a/pmd-jsp/src/main/ant/alljavacc.xml +++ b/pmd-jsp/src/main/ant/alljavacc.xml @@ -64,8 +64,6 @@ public class]]> public class Token implements java.io.Serializable @@ -85,15 +83,31 @@ public class Token implements GenericToken, java.io.Serializable]]> diff --git a/pmd-matlab/src/main/ant/alljavacc.xml b/pmd-matlab/src/main/ant/alljavacc.xml index 76819e4e64..2841465fab 100644 --- a/pmd-matlab/src/main/ant/alljavacc.xml +++ b/pmd-matlab/src/main/ant/alljavacc.xml @@ -44,8 +44,6 @@ public class Token implements java.io.Serializable @@ -65,15 +63,31 @@ public class Token implements GenericToken, java.io.Serializable]]> diff --git a/pmd-objectivec/src/main/ant/alljavacc.xml b/pmd-objectivec/src/main/ant/alljavacc.xml index b2cddbfd39..60108b51e1 100644 --- a/pmd-objectivec/src/main/ant/alljavacc.xml +++ b/pmd-objectivec/src/main/ant/alljavacc.xml @@ -44,8 +44,6 @@ public class Token implements java.io.Serializable @@ -65,15 +63,31 @@ public class Token implements GenericToken, java.io.Serializable]]> diff --git a/pmd-plsql/src/main/ant/alljavacc.xml b/pmd-plsql/src/main/ant/alljavacc.xml index 62bcd25daa..e8eb9381cb 100644 --- a/pmd-plsql/src/main/ant/alljavacc.xml +++ b/pmd-plsql/src/main/ant/alljavacc.xml @@ -81,8 +81,6 @@ public class]]> public class Token implements java.io.Serializable @@ -102,15 +100,31 @@ public class Token implements GenericToken, java.io.Serializable]]> diff --git a/pmd-python/src/main/ant/alljavacc.xml b/pmd-python/src/main/ant/alljavacc.xml index 4e1df8fd97..22cc3f93ac 100644 --- a/pmd-python/src/main/ant/alljavacc.xml +++ b/pmd-python/src/main/ant/alljavacc.xml @@ -44,8 +44,6 @@ public class Token implements java.io.Serializable @@ -65,15 +63,31 @@ public class Token implements GenericToken, java.io.Serializable]]> diff --git a/pmd-visualforce/src/main/ant/alljavacc.xml b/pmd-visualforce/src/main/ant/alljavacc.xml index 6bf811ea9d..f22d9bb72d 100644 --- a/pmd-visualforce/src/main/ant/alljavacc.xml +++ b/pmd-visualforce/src/main/ant/alljavacc.xml @@ -65,8 +65,6 @@ public class]]> public class Token implements java.io.Serializable @@ -86,14 +84,31 @@ public class Token implements GenericToken, java.io.Serializable]]> + public int getEndLine() { + return endLine; + } + + @Override + public int getBeginColumn() { + return beginColumn; + } + + @Override + public int getEndColumn() { + return endColumn; + } + +]]> diff --git a/pmd-vm/src/main/ant/alljavacc.xml b/pmd-vm/src/main/ant/alljavacc.xml index 2878476951..9a144d8703 100644 --- a/pmd-vm/src/main/ant/alljavacc.xml +++ b/pmd-vm/src/main/ant/alljavacc.xml @@ -77,8 +77,6 @@ public class]]> public class Token implements java.io.Serializable @@ -98,15 +96,31 @@ public class Token implements GenericToken, java.io.Serializable]]> From 91b8a22b42d71a2279239c1e363cab9f62fd6fa6 Mon Sep 17 00:00:00 2001 From: gonzalo Date: Sun, 22 Oct 2017 21:16:27 -0300 Subject: [PATCH 55/62] Update GenericToken specialToken method to obtain only comment tokens --- .../java/net/sourceforge/pmd/lang/ast/GenericToken.java | 6 +++--- pmd-cpp/src/main/ant/alljavacc.xml | 2 +- pmd-java/src/main/ant/alljavacc.xml | 2 +- pmd-javascript/src/main/ant/alljavacc.xml | 2 +- pmd-jsp/src/main/ant/alljavacc.xml | 2 +- pmd-matlab/src/main/ant/alljavacc.xml | 2 +- pmd-objectivec/src/main/ant/alljavacc.xml | 2 +- pmd-plsql/src/main/ant/alljavacc.xml | 2 +- pmd-python/src/main/ant/alljavacc.xml | 2 +- pmd-visualforce/src/main/ant/alljavacc.xml | 2 +- pmd-vm/src/main/ant/alljavacc.xml | 2 +- 11 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java index 859cb1e1f8..18798da868 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java @@ -16,11 +16,11 @@ public interface GenericToken { GenericToken getNext(); /** - * Obtain a special generic token which, according to the input stream which generated the instance of this token, + * Obtain a comment-type token which, according to the input stream which generated the instance of this token, * precedes this instance token and succeeds the previous generic token (if there is any). - * @return the special token if it exists; null if it does not exist + * @return the comment-type token if it exists; null if it does not exist */ - GenericToken getPreviousSpecialGenericToken(); + GenericToken getPreviousComment(); /** * Gets the token's text. diff --git a/pmd-cpp/src/main/ant/alljavacc.xml b/pmd-cpp/src/main/ant/alljavacc.xml index 6092c621e1..9347d126e4 100644 --- a/pmd-cpp/src/main/ant/alljavacc.xml +++ b/pmd-cpp/src/main/ant/alljavacc.xml @@ -59,7 +59,7 @@ public class Token implements GenericToken, java.io.Serializable]]> Date: Sat, 28 Oct 2017 20:01:07 +0200 Subject: [PATCH 56/62] [java] Mark ASTArrayDimsAndInits and ASTVariableDeclaratorId as Dimensionable --- .../pmd/lang/java/ast/ASTArrayDimsAndInits.java | 10 ++++++++-- .../pmd/lang/java/ast/ASTVariableDeclaratorId.java | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java index 29a5eb2e57..0f1fa3fa38 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTArrayDimsAndInits.java @@ -5,7 +5,7 @@ package net.sourceforge.pmd.lang.java.ast; -public class ASTArrayDimsAndInits extends AbstractJavaNode { +public class ASTArrayDimsAndInits extends AbstractJavaNode implements Dimensionable { private int arrayDepth; public ASTArrayDimsAndInits(int id) { @@ -26,8 +26,14 @@ public class ASTArrayDimsAndInits extends AbstractJavaNode { public void bumpArrayDepth() { arrayDepth++; } - + + @Override public int getArrayDepth() { return arrayDepth; } + + @Override + public boolean isArray() { + return arrayDepth > 0; // should always be true... + } } 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 445a59d046..5d6a83e208 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 @@ -11,7 +11,7 @@ import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; -public class ASTVariableDeclaratorId extends AbstractJavaTypeNode { +public class ASTVariableDeclaratorId extends AbstractJavaTypeNode implements Dimensionable { private int arrayDepth; private VariableNameDeclaration nameDeclaration; From d21be5ddc1861db9ed89b59b19a588de6bacd3c0 Mon Sep 17 00:00:00 2001 From: Matias Comercio Date: Sat, 28 Oct 2017 17:08:33 -0300 Subject: [PATCH 57/62] Add minor fixes to root pom --- pom.xml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 6dfdbd519c..19a01e76fe 100644 --- a/pom.xml +++ b/pom.xml @@ -291,7 +291,7 @@ Additionally it includes CPD, the copy-paste-detector. CPD finds duplicated code antlr4-maven-plugin ${antlr.version} - ${project.build.sourceEncoding} + ${project.build.sourceEncoding} @@ -340,6 +340,8 @@ Additionally it includes CPD, the copy-paste-detector. CPD finds duplicated code maven-compiler-plugin 3.7.0 + ${java.version} + ${java.version} ${java.version} @@ -645,7 +647,7 @@ Additionally it includes CPD, the copy-paste-detector. CPD finds duplicated code maven-pmd-plugin ${pmd.plugin.version} - true + true 100 1.${java.version} From 6072b50f11f490c3ce4e1d3268e8ef4c43084168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Sat, 28 Oct 2017 18:14:16 -0300 Subject: [PATCH 58/62] Update changelog, refs #694 --- 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 1ec21978cb..e49ce7a9e8 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -420,4 +420,5 @@ a warning will now be produced suggesting users to adopt it for better performan * [#666](https://github.com/pmd/pmd/pull/666): \[java] Add DoNotExtendJavaLangThrowable rule - [Robert Painsi](https://github.com/robertpainsi) * [#668](https://github.com/pmd/pmd/pull/668): \[core] Fix javadoc warnings on pmd-core - [Clément Fournier](https://github.com/oowekyala) * [#675](https://github.com/pmd/pmd/pull/675): \[java] Fix in Java grammar: Try with final resource node error - [Gonzalo Ibars Ingman](https://github.com/gibarsin) +* [#694](https://github.com/pmd/pmd/pull/694): \[core] Add minor fixes to root pom - [Matias Comercio](https://github.com/MatiasComercio) From 67f0fbbbfeff849df774f4a4be5a658d1ef9b05f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Mon, 30 Oct 2017 00:26:42 -0300 Subject: [PATCH 59/62] Add missing `@Override`s --- .../sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java | 2 ++ 1 file changed, 2 insertions(+) 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 5d6a83e208..8476660e31 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 @@ -49,10 +49,12 @@ public class ASTVariableDeclaratorId extends AbstractJavaTypeNode implements Dim arrayDepth++; } + @Override public int getArrayDepth() { return arrayDepth; } + @Override public boolean isArray() { return arrayDepth > 0; } From 91d40710dc987bf66096f4de94f199c30819f425 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Mon, 30 Oct 2017 16:54:38 -0300 Subject: [PATCH 60/62] Temporarily disable PMD checks on Travis --- .travis/build-deploy.sh | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.travis/build-deploy.sh b/.travis/build-deploy.sh index b0a8513d27..b555215991 100755 --- a/.travis/build-deploy.sh +++ b/.travis/build-deploy.sh @@ -25,24 +25,27 @@ function push_docs() { VERSION=$(./mvnw -q -Dexec.executable="echo" -Dexec.args='${project.version}' --non-recursive org.codehaus.mojo:exec-maven-plugin:1.5.0:exec | tail -1) echo "Building PMD ${VERSION} on branch ${TRAVIS_BRANCH}" +# TODO : Once we release PMD 6.0.0 and have a compatible PMD plugin, enable PMD once again +MVN_BUILD_FLAGS="-B -V -Dpmd.skip=true" + if travis_isPullRequest; then echo "This is a pull-request build" - ./mvnw verify -B -V + ./mvnw verify $MVN_BUILD_FLAGS elif travis_isPush; then if [[ "${VERSION}" != *-SNAPSHOT && "${TRAVIS_TAG}" != "" ]]; then echo "This is a release build for tag ${TRAVIS_TAG}" - ./mvnw deploy -Possrh,pmd-release -B -V + ./mvnw deploy -Possrh,pmd-release $MVN_BUILD_FLAGS elif [[ "${VERSION}" == *-SNAPSHOT ]]; then echo "This is a snapshot build" - ./mvnw deploy -Possrh -B -V + ./mvnw deploy -Possrh $MVN_BUILD_FLAGS push_docs else # other build. Can happen during release: the commit with a non snapshot version is built, but not from the tag. echo "This is some other build, probably during release: commit with a non-snapshot version on branch master..." - ./mvnw verify -Possrh -B -V + ./mvnw verify -Possrh $MVN_BUILD_FLAGS # we stop here - no need to execute further steps exit 0 fi From eb526f55acdf08b04ed9a7ad6ea9dbf471abc20c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Mon, 30 Oct 2017 16:55:40 -0300 Subject: [PATCH 61/62] Update changelog, refs #679 --- 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 1ec21978cb..64a1fa1e2d 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -420,4 +420,5 @@ a warning will now be produced suggesting users to adopt it for better performan * [#666](https://github.com/pmd/pmd/pull/666): \[java] Add DoNotExtendJavaLangThrowable rule - [Robert Painsi](https://github.com/robertpainsi) * [#668](https://github.com/pmd/pmd/pull/668): \[core] Fix javadoc warnings on pmd-core - [Clément Fournier](https://github.com/oowekyala) * [#675](https://github.com/pmd/pmd/pull/675): \[java] Fix in Java grammar: Try with final resource node error - [Gonzalo Ibars Ingman](https://github.com/gibarsin) +* [#679](https://github.com/pmd/pmd/pull/679): \[core] Token scheme generalization - [Gonzalo Ibars Ingman](https://github.com/gibarsin) From c466179c2ac1293664b7a49df02efe0f065263e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 31 Oct 2017 01:53:03 -0300 Subject: [PATCH 62/62] Handle SecurityErrors on bad auxclasspath entries --- .../java/net/sourceforge/pmd/util/ClasspathClassLoader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java index fbc8bb88a3..29dfeccfa2 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java @@ -105,7 +105,7 @@ public class ClasspathClassLoader extends URLClassLoader { try { // checking local c = findClass(name); - } catch (final ClassNotFoundException e) { + } catch (final ClassNotFoundException | SecurityException e) { // checking parent // This call to loadClass may eventually call findClass again, in case the parent doesn't find anything. c = super.loadClass(name, resolve);