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 diff --git a/docs/pages/pmd/rules/apex.md b/docs/pages/pmd/rules/apex.md index a8ced732f2..2d2ca1a4f2 100644 --- a/docs/pages/pmd/rules/apex.md +++ b/docs/pages/pmd/rules/apex.md @@ -62,6 +62,7 @@ List of rulesets and rules contained in each ruleset. ## Style * [AvoidDirectAccessTriggerMap](pmd_rules_apex_style.html#avoiddirectaccesstriggermap): Avoid directly accessing Trigger.old and Trigger.new as it can lead to a bug. Triggers should be ... * [AvoidGlobalModifier](pmd_rules_apex_style.html#avoidglobalmodifier): Global classes should be avoided (especially in managed packages) as they can never be deleted or... +* [AvoidHardcodingId](pmd_rules_apex_style.html#avoidhardcodingid): When deploying Apex code between sandbox and production environments, or installing Force.com App... * [AvoidLogicInTrigger](pmd_rules_apex_style.html#avoidlogicintrigger): As triggers do not allow methods like regular classes they are less flexible and suited to apply ... * [ClassNamingConventions](pmd_rules_apex_style.html#classnamingconventions): Class names should always begin with an upper case character. * [MethodNamingConventions](pmd_rules_apex_style.html#methodnamingconventions): Method names should always begin with a lower case character, and should not contain underscores. diff --git a/docs/pages/pmd/rules/apex/style.md b/docs/pages/pmd/rules/apex/style.md index 1fa90e7979..d7c7323bb4 100644 --- a/docs/pages/pmd/rules/apex/style.md +++ b/docs/pages/pmd/rules/apex/style.md @@ -5,7 +5,7 @@ permalink: pmd_rules_apex_style.html folder: pmd/rules/apex sidebaractiveurl: /pmd_rules_apex.html editmepath: ../pmd-apex/src/main/resources/rulesets/apex/style.xml -keywords: Style, VariableNamingConventions, MethodNamingConventions, ClassNamingConventions, MethodWithSameNameAsEnclosingClass, AvoidLogicInTrigger, AvoidGlobalModifier, AvoidDirectAccessTriggerMap +keywords: Style, VariableNamingConventions, MethodNamingConventions, ClassNamingConventions, MethodWithSameNameAsEnclosingClass, AvoidLogicInTrigger, AvoidGlobalModifier, AvoidDirectAccessTriggerMap, AvoidHardcodingId --- ## AvoidDirectAccessTriggerMap @@ -78,6 +78,46 @@ global class Unchangeable { ``` +## 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.style.AvoidHardcodingIdRule](https://github.com/pmd/pmd/blob/master/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/style/AvoidHardcodingIdRule.java) + +**Example(s):** + +``` java +public without sharing class Foo { + void foo() { + //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 + +``` + ## AvoidLogicInTrigger **Since:** PMD 5.5.0 diff --git a/docs/pages/pmd/rules/java.md b/docs/pages/pmd/rules/java.md index 92c8d37e80..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... @@ -326,9 +326,10 @@ 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... +* [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 bb3ee1265e..be524a7194 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 @@ -384,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) @@ -396,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 diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 14e22053a1..332d6a6166 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 @@ -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. @@ -143,6 +146,10 @@ Notice this last scenario is slightly different to the Java syntax. This is due which can produce bugs by iether accessing non-existing indexes, or them leaving out. You should use for-each-loops instead. +* 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. + * A whole new ruleset has been added to Apex, `apex-empty`. It currently migrates 5 rules from the equivalent `java-empty` ruleset for Apex. The ruleset includes: * `EmptyCatchBlock` to detect catch blocks completely ignoring exceptions. @@ -172,11 +179,35 @@ 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. + +* 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. + +* 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 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 `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. + +* 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 @@ -228,11 +259,13 @@ 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 * [#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 @@ -383,4 +416,10 @@ 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) +* [#661](https://github.com/pmd/pmd/pull/661): \[apex] avoid hardcoding id's - [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) +* [#679](https://github.com/pmd/pmd/pull/679): \[core] Token scheme generalization - [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) 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-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 new file mode 100644 index 0000000000..4a90ca2c58 --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/style/AvoidHardcodingIdRule.java @@ -0,0 +1,32 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.style; + +import java.util.regex.Pattern; + +import net.sourceforge.pmd.lang.apex.ast.ASTLiteralExpression; +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, "Style"); + setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); + setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); + } + + @Override + 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; + } +} diff --git a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml index d7a1eb485e..1a02d6ca6a 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml @@ -149,6 +149,15 @@ + + 3 + + + + + + + 3 diff --git a/pmd-apex/src/main/resources/rulesets/apex/style.xml b/pmd-apex/src/main/resources/rulesets/apex/style.xml index d6f5b9347b..f816c072bd 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/style.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/style.xml @@ -138,7 +138,6 @@ 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/style/StyleRulesTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/style/StyleRulesTest.java index 88495c67a3..1fd495648f 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 @@ -19,6 +19,6 @@ public class StyleRulesTest extends SimpleAggregatorTst { addRule(RULESET, "MethodNamingConventions"); addRule(RULESET, "VariableNamingConventions"); addRule(RULESET, "MethodWithSameNameAsEnclosingClass"); - + addRule(RULESET, "AvoidHardcodingId"); } } diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/style/xml/AvoidHardcodingId.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/style/xml/AvoidHardcodingId.xml new file mode 100644 index 0000000000..b7d77b9f05 --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/style/xml/AvoidHardcodingId.xml @@ -0,0 +1,53 @@ + + + + + + Non compliant scenario: Hardcoded Id + 1 + + + + + Compliant scenario, getting ID dynamically + 0 + + + + + Test for random string combinations + 0 + + + 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 471cb7f1d0..228a4ee3ee 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactory.java @@ -50,9 +50,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 { @@ -66,18 +66,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; @@ -98,7 +98,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); } /** @@ -126,7 +126,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"); @@ -135,7 +135,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")); } } @@ -144,7 +144,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 @@ -159,7 +159,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. @@ -181,7 +181,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. @@ -200,7 +200,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. @@ -292,7 +292,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 @@ -330,7 +330,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 + ">."); @@ -544,7 +544,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) Class.forName(attribute).newInstance(); rule.setName(ruleElement.getAttribute("name")); if (ruleElement.hasAttribute("language")) { @@ -775,7 +775,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/RuleSetFactoryCompatibility.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleSetFactoryCompatibility.java index af39be8cf4..6ba101a194 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,10 @@ public class RuleSetFactoryCompatibility { // PMD 6.0.0 addFilterRuleMoved("java", "controversial", "unnecessary", "UnnecessaryParentheses"); addFilterRuleRenamed("java", "unnecessary", "UnnecessaryParentheses", "UselessParentheses"); - + 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-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/lang/ast/GenericToken.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/GenericToken.java index 8750e01ae4..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 @@ -4,6 +4,51 @@ 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 getNext(); + + /** + * 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 comment-type token if it exists; null if it does not exist + */ + GenericToken getPreviousComment(); + + /** + * 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/properties/DoubleMultiProperty.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/DoubleMultiProperty.java index 4408deae94..4226366677 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 @@ -29,7 +29,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) { @@ -54,7 +54,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 46e834dae5..3c18c0d408 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 @@ -30,7 +30,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, @@ -56,7 +56,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 cf28feb715..e963e32369 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 @@ -30,7 +30,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) { @@ -55,7 +55,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 352d5cb81b..bc2a2521e2 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 @@ -29,7 +29,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, @@ -56,7 +56,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 9453d9079a..93eb63a373 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 @@ -30,7 +30,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) { @@ -56,7 +56,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 5797c82b3b..06b6396d1a 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 @@ -28,7 +28,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 f70814f0d0..556ea0a410 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 @@ -30,7 +30,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) { @@ -55,7 +55,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 ad3751bb80..599b6c8390 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 @@ -29,7 +29,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, @@ -56,7 +56,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/ClasspathClassLoader.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/ClasspathClassLoader.java index fd9d057e0b..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 @@ -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,25 @@ 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) { + try { + // checking local + c = findClass(name); + } 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); + } + } + + if (resolve) { + resolveClass(c); + } + return c; + } } 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/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/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) { 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 + 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-cpp/src/main/ant/alljavacc.xml b/pmd-cpp/src/main/ant/alljavacc.xml index 4066ee3a6f..9347d126e4 100644 --- a/pmd-cpp/src/main/ant/alljavacc.xml +++ b/pmd-cpp/src/main/ant/alljavacc.xml @@ -40,6 +40,57 @@ + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + + 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:* - diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 5f15749f5e..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() } @@ -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();} } diff --git a/pmd-java/src/main/ant/alljavacc.xml b/pmd-java/src/main/ant/alljavacc.xml index 45c05dac70..e1a80620f1 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 + + + + + + public Token specialToken; + + 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..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,9 @@ 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) { super(id); } @@ -20,4 +22,18 @@ public class ASTArrayDimsAndInits extends AbstractJavaNode { public Object jjtAccept(JavaParserVisitor visitor, Object data) { return visitor.visit(this, data); } + + 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/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/ast/ASTVariableDeclaratorId.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclaratorId.java index 445a59d046..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 @@ -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; @@ -49,10 +49,12 @@ public class ASTVariableDeclaratorId extends AbstractJavaTypeNode { arrayDepth++; } + @Override public int getArrayDepth() { return arrayDepth; } + @Override public boolean isArray() { return arrayDepth > 0; } 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/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/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/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/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..7c357aaaf8 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); @@ -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], @@ -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,38 +1135,12 @@ 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) { - // - // 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. - // - - // 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 ASTArrayDimsAndInits dims = node.getFirstChildOfType(ASTArrayDimsAndInits.class); + if (dims != null) { + final Class arrayType = ((TypeNode) node.jjtGetChild(0)).getType(); + if (arrayType != null) { + node.setType(Array.newInstance(arrayType, (int[]) Array.newInstance(int.class, dims.getArrayDepth())).getClass()); + } } else { rollupTypeUnary(node); } @@ -1275,10 +1247,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 +1302,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..bc2591f746 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,40 +66,41 @@ 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) { 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,16 +114,14 @@ 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); - if (argList == null) { - selectedMethods.add(methodType); - - // 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 +131,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 @@ -140,13 +142,19 @@ 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; // 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; @@ -166,7 +174,6 @@ public final class MethodTypeResolution { return selectedMethods; } - public static MethodType parameterizeInvocation(JavaTypeDefinition context, Method method, ASTArgumentList argList) { @@ -178,6 +185,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); } @@ -193,7 +205,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 +245,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) { @@ -261,6 +273,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); @@ -268,16 +281,13 @@ public final class MethodTypeResolution { throw new ResolutionFailedException(); } - if (argList == null) { - selectedMethods.add(methodType); - - // 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 +321,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)); } @@ -450,10 +462,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 @@ -622,7 +638,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()); @@ -686,11 +701,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/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/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/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/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/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..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,7 +130,9 @@ public enum InferenceRuleType { // Otherwise, the constraint is reduced according to the form of 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()); } }, @@ -220,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()); } }; @@ -257,7 +263,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/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 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/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/strictexception.xml b/pmd-java/src/main/resources/rulesets/java/strictexception.xml index 7efab2baf1..2eaea11f10 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 @@ -243,6 +246,31 @@ public class Foo extends Error { } ]]> + + +Extend Exception or RuntimeException instead of Throwable. + + 3 + + + + + + + + + + -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. - - - -Avoid using implementation types (i.e., HashSet); use the interface (i.e, Set) instead - - 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 - - - - - - - -Avoid unused import statements. This rule will find unused on demand imports, i.e. import com.foo.*. - - 4 - - - - - - - -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/clone/MyInterface.java similarity index 69% 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/clone/MyInterface.java index 179be18d62..301d60cb87 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/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.typeresolution.xml; +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/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/strictexception/StrictExceptionRulesTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/StrictExceptionRulesTest.java index 608f6e8970..182d0be1ac 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/StrictExceptionRulesTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/strictexception/StrictExceptionRulesTest.java @@ -21,6 +21,7 @@ public class StrictExceptionRulesTest extends SimpleAggregatorTst { addRule(RULESET, "AvoidThrowingNullPointerException"); addRule(RULESET, "AvoidThrowingRawExceptionTypes"); addRule(RULESET, "DoNotExtendJavaLangError"); + addRule(RULESET, "DoNotExtendJavaLangThrowable"); addRule(RULESET, "ExceptionAsFlowControl"); addRule(RULESET, "SignatureDeclareThrowsException"); addRule(RULESET, "DoNotThrowExceptionInFinally"); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/CloneMethodMustImplementCloneableTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/CloneMethodMustImplementCloneableTest.java deleted file mode 100644 index 46f4e0ae5c..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/CloneMethodMustImplementCloneableTest.java +++ /dev/null @@ -1,16 +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 CloneMethodMustImplementCloneableTest extends SimpleAggregatorTst { - private static final String RULESET = "java-typeresolution"; - - @Override - public void setUp() { - addRule(RULESET, "CloneMethodMustImplementCloneable"); - } -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/LooseCouplingTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/LooseCouplingTest.java deleted file mode 100644 index a3865d3ad0..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/LooseCouplingTest.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 LooseCouplingTest extends SimpleAggregatorTst { - - private static final String RULESET = "java-typeresolution"; - - @Override - public void setUp() { - addRule(RULESET, "LooseCoupling"); - } -} 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/java/net/sourceforge/pmd/lang/java/rule/typeresolution/UnusedImportsTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/UnusedImportsTest.java deleted file mode 100644 index 6b0e1ce82f..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/typeresolution/UnusedImportsTest.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 UnusedImportsTest extends SimpleAggregatorTst { - - private static final String RULESET = "java-typeresolution"; - - @Override - public void setUp() { - addRule(RULESET, "UnusedImports"); - } -} 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..7d5f3c3891 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; @@ -86,6 +87,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; @@ -105,6 +107,8 @@ 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; import net.sourceforge.pmd.typeresolution.testdata.dummytypes.JavaTypeDefinitionEquals; @@ -748,6 +752,36 @@ 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]; + testSubtreeNodeTypes(expressions.get(index++), int[].class); + + // Object[][] b = new Object[1][0]; + testSubtreeNodeTypes(expressions.get(index++), Object[][].class); + + // ArrayTypes[][][] c = new ArrayTypes[][][] { new ArrayTypes[1][2] }; + 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 { @@ -1328,7 +1362,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)); @@ -1503,6 +1537,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); @@ -1521,6 +1585,55 @@ 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 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/MethodTypeResolutionTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/MethodTypeResolutionTest.java new file mode 100644 index 0000000000..7a9a5bd015 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/MethodTypeResolutionTest.java @@ -0,0 +1,28 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +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()); + } +} 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/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] }; + } +} 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); + } +} 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; } 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; + } +} 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..835fde9bab --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/testdata/VarargsZeroArity.java @@ -0,0 +1,21 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +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; + } +} 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 1aa299c261..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 @@ -62,6 +62,52 @@ public class Bar { + 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/coupling/xml/LooseCoupling.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/coupling/xml/LooseCoupling.xml index 49ee78f298..513f46a7d2 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/coupling/xml/LooseCoupling.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/coupling/xml/LooseCoupling.xml @@ -9,6 +9,7 @@ returning a HashSet, bad ]]> 1 0 1 2 2 1 #938 False positive on LooseCoupling for overriding methods 0 + 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/strictexception/xml/DoNotExtendJavaLangThrowable.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/strictexception/xml/DoNotExtendJavaLangThrowable.xml new file mode 100644 index 0000000000..b2ee04a52b --- /dev/null +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/strictexception/xml/DoNotExtendJavaLangThrowable.xml @@ -0,0 +1,36 @@ + + + + + 1 + + + + + 1 + + + + + 0 + + + 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/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 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/LooseCoupling.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/LooseCoupling.xml deleted file mode 100644 index 784e2a2fe4..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/typeresolution/xml/LooseCoupling.xml +++ /dev/null @@ -1,164 +0,0 @@ - - - - - 1 - - - - - 0 - - - - - 0 - - - - - 0 - - - - - 1 - - - - - 2 - - - - - 2 - - - - - 1 - - - - - 1 - - - - - 1 - - - - - 1 - - - - - #938 False positive on LooseCoupling for overriding methods - 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 - - - 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 - - - diff --git a/pmd-javascript/src/main/ant/alljavacc.xml b/pmd-javascript/src/main/ant/alljavacc.xml index 15cf978038..6f7a0a7bc4 100644 --- a/pmd-javascript/src/main/ant/alljavacc.xml +++ b/pmd-javascript/src/main/ant/alljavacc.xml @@ -40,5 +40,55 @@ + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + diff --git a/pmd-jsp/src/main/ant/alljavacc.xml b/pmd-jsp/src/main/ant/alljavacc.xml index da338412b9..34207aa4a6 100644 --- a/pmd-jsp/src/main/ant/alljavacc.xml +++ b/pmd-jsp/src/main/ant/alljavacc.xml @@ -62,12 +62,55 @@ public class]]> - public class Token - public class Token implements java.io.Serializable + +public class Token implements GenericToken, java.io.Serializable]]> + + + public Token specialToken; + + + diff --git a/pmd-matlab/src/main/ant/alljavacc.xml b/pmd-matlab/src/main/ant/alljavacc.xml index e17d98fb20..ecf431f7b6 100644 --- a/pmd-matlab/src/main/ant/alljavacc.xml +++ b/pmd-matlab/src/main/ant/alljavacc.xml @@ -40,6 +40,56 @@ + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + diff --git a/pmd-objectivec/src/main/ant/alljavacc.xml b/pmd-objectivec/src/main/ant/alljavacc.xml index 5396813b9a..26ab8cd3d9 100644 --- a/pmd-objectivec/src/main/ant/alljavacc.xml +++ b/pmd-objectivec/src/main/ant/alljavacc.xml @@ -40,6 +40,56 @@ + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + diff --git a/pmd-plsql/src/main/ant/alljavacc.xml b/pmd-plsql/src/main/ant/alljavacc.xml index 2cb7679a0f..d111dbb946 100644 --- a/pmd-plsql/src/main/ant/alljavacc.xml +++ b/pmd-plsql/src/main/ant/alljavacc.xml @@ -78,5 +78,55 @@ public class]]> token="extends Exception" value="extends net.sourceforge.pmd.lang.ast.ParseException" /> + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + + diff --git a/pmd-python/src/main/ant/alljavacc.xml b/pmd-python/src/main/ant/alljavacc.xml index 06b4bfddcc..b876617263 100644 --- a/pmd-python/src/main/ant/alljavacc.xml +++ b/pmd-python/src/main/ant/alljavacc.xml @@ -40,6 +40,56 @@ + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + 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) { diff --git a/pmd-visualforce/src/main/ant/alljavacc.xml b/pmd-visualforce/src/main/ant/alljavacc.xml index 9637b35151..b2463ff956 100644 --- a/pmd-visualforce/src/main/ant/alljavacc.xml +++ b/pmd-visualforce/src/main/ant/alljavacc.xml @@ -63,12 +63,54 @@ public class]]> - public class Token + public class Token implements java.io.Serializable +public class Token implements GenericToken, java.io.Serializable]]> + + public Token specialToken; + + + diff --git a/pmd-vm/src/main/ant/alljavacc.xml b/pmd-vm/src/main/ant/alljavacc.xml index cab38ed72f..8f07d54ade 100644 --- a/pmd-vm/src/main/ant/alljavacc.xml +++ b/pmd-vm/src/main/ant/alljavacc.xml @@ -73,5 +73,55 @@ public class]]> + + + public class Token implements java.io.Serializable + + + + + + public Token specialToken; + + 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}