phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch '7.0.x' into apexlink

Browse Source
This commit is contained in:
Clément Fournier
2021-04-16 15:24:00 +02:00
parent 8bc57f349a d33c517476
commit 73f86cefb8
102 changed files with 3369 additions and 1666 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
.ci/files/all-java.xml
View File

@ -57,8 +57,9 @@
<rule ref="category/java/bestpractices.xml/UseAssertSameInsteadOfAssertTrue"/>
<rule ref="category/java/bestpractices.xml/UseAssertTrueInsteadOfAssertEquals"/>
<rule ref="category/java/bestpractices.xml/UseCollectionIsEmpty"/>
<!-- <rule ref="category/java/bestpractices.xml/UseTryWithResources"/> -->
<!-- <rule ref="category/java/bestpractices.xml/UseVarargs"/> -->
<rule ref="category/java/bestpractices.xml/UseStandardCharsets" />
<rule ref="category/java/bestpractices.xml/UseTryWithResources"/>
<rule ref="category/java/bestpractices.xml/UseVarargs"/>
<rule ref="category/java/bestpractices.xml/WhileLoopWithLiteralBoolean"/>
<!-- codestyle.xml -->
@ -71,7 +72,7 @@
<rule ref="category/java/codestyle.xml/BooleanGetMethodName"/>
<rule ref="category/java/codestyle.xml/CallSuperInConstructor"/>
<rule ref="category/java/codestyle.xml/ClassNamingConventions"/>
<!-- <rule ref="category/java/codestyle.xml/CommentDefaultAccessModifier"/> -->
<rule ref="category/java/codestyle.xml/CommentDefaultAccessModifier"/>
<rule ref="category/java/codestyle.xml/ConfusingTernary"/>
<rule ref="category/java/codestyle.xml/ControlStatementBraces"/>
<rule ref="category/java/codestyle.xml/DefaultPackage"/>
@ -85,7 +86,7 @@
<rule ref="category/java/codestyle.xml/FormalParameterNamingConventions"/>
<rule ref="category/java/codestyle.xml/GenericsNaming"/>
<rule ref="category/java/codestyle.xml/IdenticalCatchBranches"/>
<!-- <rule ref="category/java/codestyle.xml/LinguisticNaming"/> -->
<rule ref="category/java/codestyle.xml/LinguisticNaming"/>
<rule ref="category/java/codestyle.xml/LocalHomeNamingConvention"/>
<rule ref="category/java/codestyle.xml/LocalInterfaceSessionNamingConvention"/>
<rule ref="category/java/codestyle.xml/LocalVariableCouldBeFinal"/>
@ -97,7 +98,7 @@
<rule ref="category/java/codestyle.xml/NoPackage"/>
<!-- <rule ref="category/java/codestyle.xml/OnlyOneReturn"/> -->
<rule ref="category/java/codestyle.xml/PackageCase"/>
<!-- <rule ref="category/java/codestyle.xml/PrematureDeclaration"/> -->
<rule ref="category/java/codestyle.xml/PrematureDeclaration"/>
<rule ref="category/java/codestyle.xml/RemoteInterfaceNamingConvention"/>
<rule ref="category/java/codestyle.xml/RemoteSessionInterfaceNamingConvention"/>
<rule ref="category/java/codestyle.xml/ShortClassName"/>
@ -109,8 +110,8 @@
<rule ref="category/java/codestyle.xml/UnnecessaryConstructor"/>
<rule ref="category/java/codestyle.xml/UnnecessaryFullyQualifiedName"/>
<rule ref="category/java/codestyle.xml/UnnecessaryLocalBeforeReturn"/>
<!-- <rule ref="category/java/codestyle.xml/UnnecessaryModifier"/> -->
<!-- <rule ref="category/java/codestyle.xml/UnnecessaryReturn"/> -->
<rule ref="category/java/codestyle.xml/UnnecessaryModifier"/>
<rule ref="category/java/codestyle.xml/UnnecessaryReturn"/>
<!-- <rule ref="category/java/codestyle.xml/UseDiamondOperator"/> -->
<rule ref="category/java/codestyle.xml/UseShortArrayInitializer"/>
<rule ref="category/java/codestyle.xml/UseUnderscoresInNumericLiterals"/>
@ -190,7 +191,7 @@
<!-- <rule ref="category/java/errorprone.xml/AvoidUsingOctalValues"/> -->
<!-- <rule ref="category/java/errorprone.xml/BadComparison"/> -->
<!-- <rule ref="category/java/errorprone.xml/BeanMembersShouldSerialize"/> -->
<!-- <rule ref="category/java/errorprone.xml/BrokenNullCheck"/> -->
<rule ref="category/java/errorprone.xml/BrokenNullCheck"/>
<!-- <rule ref="category/java/errorprone.xml/CallSuperFirst"/> -->
<!-- <rule ref="category/java/errorprone.xml/CallSuperLast"/> -->
<!-- <rule ref="category/java/errorprone.xml/CheckSkipResult"/> -->

10
Gemfile.lock
View File

@ -9,7 +9,7 @@ GEM
nap
open4 (~> 1.3)
colored2 (3.1.2)
concurrent-ruby (1.1.7)
concurrent-ruby (1.1.8)
cork (0.3.0)
colored2 (~> 3.1)
danger (5.16.1)
@ -27,23 +27,23 @@ GEM
differ (0.1.2)
et-orbi (1.2.4)
tzinfo
faraday (0.17.3)
faraday (0.17.4)
multipart-post (>= 1.2, < 3)
faraday-http-cache (1.3.1)
faraday (~> 0.8)
fugit (1.4.2)
fugit (1.4.4)
et-orbi (~> 1.1, >= 1.1.8)
raabro (~> 1.4)
git (1.8.1)
rchardet (~> 1.8)
kramdown (1.17.0)
liquid (5.0.0)
liquid (5.0.1)
logger-colors (1.0.0)
mini_portile2 (2.5.0)
multipart-post (2.1.1)
nap (1.1.0)
no_proxy_fix (0.1.2)
nokogiri (1.11.1)
nokogiri (1.11.2)
mini_portile2 (~> 2.5.0)
racc (~> 1.4)
octokit (4.20.0)

48
docs/Gemfile.lock
View File

@ -1,7 +1,7 @@
GEM
remote: https://rubygems.org/
specs:
activesupport (6.0.3.4)
activesupport (6.0.3.6)
concurrent-ruby (~> 1.0, >= 1.0.2)
i18n (>= 0.7, < 2)
minitest (~> 5.1)
@ -16,7 +16,7 @@ GEM
colorator (1.1.0)
commonmarker (0.17.13)
ruby-enum (~> 0.5)
concurrent-ruby (1.1.7)
concurrent-ruby (1.1.8)
dnsruby (1.61.5)
simpleidn (~> 0.1)
em-websocket (0.5.2)
@ -30,12 +30,12 @@ GEM
faraday-net_http (~> 1.0)
multipart-post (>= 1.2, < 3)
ruby2_keywords
faraday-net_http (1.0.0)
ffi (1.14.2)
faraday-net_http (1.0.1)
ffi (1.15.0)
forwardable-extended (2.6.0)
gemoji (3.0.1)
github-pages (209)
github-pages-health-check (= 1.16.1)
github-pages (214)
github-pages-health-check (= 1.17.0)
jekyll (= 3.9.0)
jekyll-avatar (= 0.7.0)
jekyll-coffeescript (= 1.1.1)
@ -50,9 +50,9 @@ GEM
jekyll-readme-index (= 0.3.0)
jekyll-redirect-from (= 0.16.0)
jekyll-relative-links (= 0.6.1)
jekyll-remote-theme (= 0.4.2)
jekyll-remote-theme (= 0.4.3)
jekyll-sass-converter (= 1.5.2)
jekyll-seo-tag (= 2.6.1)
jekyll-seo-tag (= 2.7.1)
jekyll-sitemap (= 1.4.0)
jekyll-swiss (= 1.0.0)
jekyll-theme-architect (= 0.1.1)
@ -70,19 +70,19 @@ GEM
jekyll-theme-time-machine (= 0.1.1)
jekyll-titles-from-headings (= 0.5.3)
jemoji (= 0.12.0)
kramdown (= 2.3.0)
kramdown (= 2.3.1)
kramdown-parser-gfm (= 1.1.0)
liquid (= 4.0.3)
mercenary (~> 0.3)
minima (= 2.5.1)
nokogiri (>= 1.10.4, < 2.0)
rouge (= 3.23.0)
rouge (= 3.26.0)
terminal-table (~> 1.4)
github-pages-health-check (1.16.1)
github-pages-health-check (1.17.0)
addressable (~> 2.3)
dnsruby (~> 1.60)
octokit (~> 4.0)
public_suffix (~> 3.0)
public_suffix (>= 2.0.2, < 5.0)
typhoeus (~> 1.3)
html-pipeline (2.14.0)
activesupport (>= 2)
@ -136,15 +136,15 @@ GEM
jekyll (>= 3.3, < 5.0)
jekyll-relative-links (0.6.1)
jekyll (>= 3.3, < 5.0)
jekyll-remote-theme (0.4.2)
jekyll-remote-theme (0.4.3)
addressable (~> 2.0)
jekyll (>= 3.5, < 5.0)
jekyll-sass-converter (>= 1.0, <= 3.0.0, != 2.0.0)
rubyzip (>= 1.3.0, < 3.0)
jekyll-sass-converter (1.5.2)
sass (~> 3.4)
jekyll-seo-tag (2.6.1)
jekyll (>= 3.3, < 5.0)
jekyll-seo-tag (2.7.1)
jekyll (>= 3.8, < 5.0)
jekyll-sitemap (1.4.0)
jekyll (>= 3.7, < 5.0)
jekyll-swiss (1.0.0)
@ -196,12 +196,12 @@ GEM
gemoji (~> 3.0)
html-pipeline (~> 2.2)
jekyll (>= 3.0, < 5.0)
kramdown (2.3.0)
kramdown (2.3.1)
rexml
kramdown-parser-gfm (1.1.0)
kramdown (~> 2.0)
liquid (4.0.3)
listen (3.4.0)
listen (3.5.1)
rb-fsevent (~> 0.10, >= 0.10.3)
rb-inotify (~> 0.9, >= 0.9.10)
mercenary (0.3.6)
@ -210,9 +210,9 @@ GEM
jekyll (>= 3.5, < 5.0)
jekyll-feed (~> 0.9)
jekyll-seo-tag (~> 2.1)
minitest (5.14.3)
minitest (5.14.4)
multipart-post (2.1.1)
nokogiri (1.11.1)
nokogiri (1.11.2)
mini_portile2 (~> 2.5.0)
racc (~> 1.4)
octokit (4.20.0)
@ -220,16 +220,16 @@ GEM
sawyer (~> 0.8.0, >= 0.5.3)
pathutil (0.16.2)
forwardable-extended (~> 2.6)
public_suffix (3.1.1)
public_suffix (4.0.6)
racc (1.5.2)
rb-fsevent (0.10.4)
rb-inotify (0.10.1)
ffi (~> 1.0)
rexml (3.2.4)
rouge (3.23.0)
ruby-enum (0.8.0)
rouge (3.26.0)
ruby-enum (0.9.0)
i18n
ruby2_keywords (0.0.2)
ruby2_keywords (0.0.4)
rubyzip (2.3.0)
safe_yaml (1.0.5)
sass (3.7.4)
@ -240,7 +240,7 @@ GEM
sawyer (0.8.2)
addressable (>= 2.3.5)
faraday (> 0.8, < 2.0)
simpleidn (0.1.1)
simpleidn (0.2.1)
unf (~> 0.1.4)
terminal-table (1.8.0)
unicode-display_width (~> 1.1, >= 1.1.1)

15
docs/pages/7_0_0_release_notes.md
View File

@ -88,6 +88,7 @@ The default version is always ES6.
The other property `ignoreBalancing` (default: true) is similar, in that it allows parentheses that help
reading and understanding the expressions.
* The rule {% rule "java/bestpractices/LooseCoupling" %} has a new property to allow some types to be coupled to (`allowedTypes`).
* {% rule "java/errorprone/EmptyCatchBlock" %}: `CloneNotSupportedException` and `InterruptedException` are not special-cased anymore. Rename the exception parameter to `ignored` to ignore them.
#### Removed Rules
@ -112,13 +113,6 @@ The following previously deprecated rules have been finally removed:
* VariableNamingConventions (java-codestyle)
* WhileLoopsMustUseBraces (java-codestyle)
#### Changed rules
##### Java
* {% rule "java/errorprone/EmptyCatchBlock" %}: `CloneNotSupportedException` and `InterruptedException` are not special-cased anymore. Rename the exception parameter to `ignored` to ignore them.
### Fixed Issues
* apex-design
@ -142,24 +136,31 @@ The following previously deprecated rules have been finally removed:
* [#2147](https://github.com/pmd/pmd/issues/2147): \[java] JUnitTestsShouldIncludeAssert - false positives with lambdas and static methods
* [#2464](https://github.com/pmd/pmd/issues/2464): \[java] LooseCoupling must ignore class literals: ArrayList.class
* [#2542](https://github.com/pmd/pmd/issues/2542): \[java] UseCollectionIsEmpty can not detect the case `foo.bar().size()`
* [#2650](https://github.com/pmd/pmd/issues/2650): \[java] UseTryWithResources false positive when AutoCloseable helper used
* [#2796](https://github.com/pmd/pmd/issue/2796): \[java] UnusedAssignment false positive with call chains
* [#2797](https://github.com/pmd/pmd/issues/2797): \[java] MissingOverride long-standing issues
* [#2806](https://github.com/pmd/pmd/issues/2806): \[java] SwitchStmtsShouldHaveDefault false-positive with Java 14 switch non-fallthrough branches
* [#2822](https://github.com/pmd/pmd/issues/2822): \[java] LooseCoupling rule: Extend to cover user defined implementations and interfaces
* [#2882](https://github.com/pmd/pmd/issues/2882): \[java] UseTryWithResources - false negative for explicit close
* [#2883](https://github.com/pmd/pmd/issues/2883): \[java] JUnitAssertionsShouldIncludeMessage false positive with method call
* [#2890](https://github.com/pmd/pmd/issues/2890): \[java] UnusedPrivateMethod false positive with generics
* java-codestyle
* [#1208](https://github.com/pmd/pmd/issues/1208): \[java] PrematureDeclaration rule false-positive on variable declared to measure time
* [#1429](https://github.com/pmd/pmd/issues/1429): \[java] PrematureDeclaration as result of method call (false positive)
* [#1673](https://github.com/pmd/pmd/issues/1673): \[java] UselessParentheses false positive with conditional operator
* [#1790](https://github.com/pmd/pmd/issues/1790): \[java] UnnecessaryFullyQualifiedName false positive with enum constant
* [#1918](https://github.com/pmd/pmd/issues/1918): \[java] UselessParentheses false positive with boolean operators
* [#2299](https://github.com/pmd/pmd/issues/2299): \[java] UnnecessaryFullyQualifiedName false positive with similar package name
* [#2528](https://github.com/pmd/pmd/issues/2528): \[java] MethodNamingConventions - JUnit 5 method naming not support ParameterizedTest
* [#2739](https://github.com/pmd/pmd/issues/2739): \[java] UselessParentheses false positive for string concatenation
* [#3195](https://github.com/pmd/pmd/pull/3195): \[java] Improve rule UnnecessaryReturn to detect more cases
* [#3221](https://github.com/pmd/pmd/issues/3221): \[java] PrematureDeclaration false positive for unused variables
* java-errorprone
* [#1005](https://github.com/pmd/pmd/issues/1005): \[java] CloneMethodMustImplementCloneable triggers for interfaces
* [#2532](https://github.com/pmd/pmd/issues/2532): \[java] AvoidDecimalLiteralsInBigDecimalConstructor can not detect the case new BigDecimal(Expression)
* [#2716](https://github.com/pmd/pmd/issues/2716): \[java] CompareObjectsWithEqualsRule: False positive with Enums
* [#2880](https://github.com/pmd/pmd/issues/2880): \[java] CompareObjectsWithEquals - false negative with type res
* [#3071](https://github.com/pmd/pmd/issues/3071): \[java] BrokenNullCheck FP with PMD 6.30.0
* java-multithreading
* [#2537](https://github.com/pmd/pmd/issues/2537): \[java] DontCallThreadRun can't detect the case that call run() in `this.run()`
* [#2538](https://github.com/pmd/pmd/issues/2538): \[java] DontCallThreadRun can't detect the case that call run() in `foo.bar.run()`

17
docs/pages/release_notes.md
View File

@ -19,11 +19,28 @@ This is a {{ site.pmd.release_type }} release.
### New and noteworthy
#### New rules
* The new Java rule {% rule "java/bestpractices/UseStandardCharsets" %} finds usages of `Charset.forName`,
where `StandardCharsets` can be used instead.
This rule is also part of the Quickstart Ruleset (`rulesets/java/quickstart.xml`) for Java.
### Fixed Issues
* apex-performance
* [#3198](https://github.com/pmd/pmd/pull/3198): \[apex] OperationWithLimitsInLoopRule: Support more limit consuming static method invocations
* java-bestpractices
* [#3190](https://github.com/pmd/pmd/issues/3190): \[java] Use StandardCharsets instead of Charset.forName
* java-errorprone
* [#2757](https://github.com/pmd/pmd/issues/2757): \[java] CloseResource: support Lombok's @Cleanup annotation
### API Changes
### External Contributions
* [#3193](https://github.com/pmd/pmd/pull/3193): \[java] New rule: UseStandardCharsets - [Andrea Aime](https://github.com/aaime)
* [#3198](https://github.com/pmd/pmd/pull/3198): \[apex] OperationWithLimitsInLoopRule: Support more limit consuming static method invocations - [Jonathan Wiesel](https://github.com/jonathanwiesel)
{% endtocmaker %}

79
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/performance/OperationWithLimitsInLoopRule.java
View File

@ -13,6 +13,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTDmlUndeleteStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTDmlUpdateStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTDmlUpsertStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTRunAsBlockStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTSoqlExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTSoslExpression;
import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
@ -23,25 +24,34 @@ import net.sourceforge.pmd.lang.rule.RuleTargetSelector;
*/
public class OperationWithLimitsInLoopRule extends AbstractAvoidNodeInLoopsRule {
private static final String APPROVAL_CLASS_NAME = "Approval";
private static final String MESSAGING_CLASS_NAME = "Messaging";
private static final String SYSTEM_CLASS_NAME = "System";
private static final String[] MESSAGING_LIMIT_METHODS = new String[] { "renderEmailTemplate", "renderStoredEmailTemplate", "sendEmail" };
private static final String[] SYSTEM_LIMIT_METHODS = new String[] { "enqueueJob", "schedule", "scheduleBatch" };
@Override
protected @NonNull RuleTargetSelector buildTargetSelector() {
return RuleTargetSelector.forTypes(
// DML
ASTDmlDeleteStatement.class,
ASTDmlInsertStatement.class,
ASTDmlMergeStatement.class,
ASTDmlUndeleteStatement.class,
ASTDmlUpdateStatement.class,
ASTDmlUpsertStatement.class,
// Database methods
ASTMethodCallExpression.class,
// SOQL
ASTSoqlExpression.class,
// SOSL
ASTSoslExpression.class
// DML
ASTDmlDeleteStatement.class,
ASTDmlInsertStatement.class,
ASTDmlMergeStatement.class,
ASTDmlUndeleteStatement.class,
ASTDmlUpdateStatement.class,
ASTDmlUpsertStatement.class,
// SOQL
ASTSoqlExpression.class,
// SOSL
ASTSoslExpression.class,
// Other limit consuming methods
ASTMethodCallExpression.class,
ASTRunAsBlockStatement.class
);
}
// Begin DML Statements
@Override
public Object visit(ASTDmlDeleteStatement node, Object data) {
@ -74,17 +84,6 @@ public class OperationWithLimitsInLoopRule extends AbstractAvoidNodeInLoopsRule
}
// End DML Statements
// Begin Database method invocations
@Override
public Object visit(ASTMethodCallExpression node, Object data) {
if (Helper.isAnyDatabaseMethodCall(node)) {
return checkForViolation(node, data);
} else {
return data;
}
}
// End Database method invocations
// Begin SOQL method invocations
@Override
public Object visit(ASTSoqlExpression node, Object data) {
@ -98,4 +97,36 @@ public class OperationWithLimitsInLoopRule extends AbstractAvoidNodeInLoopsRule
return checkForViolation(node, data);
}
// End SOSL method invocations
// Begin general method invocations
@Override
public Object visit(ASTRunAsBlockStatement node, Object data) {
return checkForViolation(node, data);
}
@Override
public Object visit(ASTMethodCallExpression node, Object data) {
if (Helper.isAnyDatabaseMethodCall(node)
|| Helper.isMethodName(node, APPROVAL_CLASS_NAME, Helper.ANY_METHOD)
|| checkLimitClassMethods(node, MESSAGING_CLASS_NAME, MESSAGING_LIMIT_METHODS)
|| checkLimitClassMethods(node, SYSTEM_CLASS_NAME, SYSTEM_LIMIT_METHODS)) {
return checkForViolation(node, data);
} else {
return data;
}
}
private boolean checkLimitClassMethods(ASTMethodCallExpression node, String className, String[] methodNames) {
for (String method : methodNames) {
if (Helper.isMethodName(node, className, method)) {
return true;
}
}
return false;
}
// End general method invocations
}

109
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/performance/xml/OperationWithLimitsInLoop.xml
View File

@ -410,4 +410,113 @@ public class Foo {
]]></code>
</test-code>
<!-- End Sosl method invocations -->
<!-- Begin approval method invocations -->
<test-code>
<description>Problematic approval actions in loop</description>
<expected-problems>3</expected-problems>
<code><![CDATA[
public class Foo {
public void test1() {
Account a = new Account();
Approval.ProcessSubmitRequest req = new Approval.ProcessSubmitRequest();
req.setObjectId(a.id);
do {
Approval.process(req);
Approval.unlock(a);
Approval.lock(a);
} while(true);
}
}
]]></code>
</test-code>
<test-code>
<description>Best Practice: Batch up data into a list and invoke your Approval actions once on that list of data.</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public void foo() {
List<Account> accounts = new List<Account>();
List<Approval.ProcessSubmitRequest> reqs = new List<Approval.ProcessSubmitRequest>();
while(true) {
Account account = new Account();
accounts.add(account);
Approval.ProcessSubmitRequest req = new Approval.ProcessSubmitRequest();
req.setObjectId(account.id);
reqs.add(req);
}
Approval.process(reqs, true);
Approval.unlock(accounts);
Approval.lock(accounts);
}
}
]]></code>
</test-code>
<!-- End approval method invocations -->
<!-- Begin messaging method invocations -->
<test-code>
<description>Problematic messaging actions in loop</description>
<expected-problems>3</expected-problems>
<code><![CDATA[
public class Foo {
public void test1() {
Contact cont = new Contact();
Account acc = new Account();
do {
Messaging.SingleEmailMessage email = new Messaging.SingleEmailMessage();
List<Messaging.RenderEmailTemplateBodyResult> renderedRes = Messaging.renderEmailTemplate(cont.Id, acc.Id, new List<String>());
Messaging.SingleEmailMessage renderedMail = Messaging.renderStoredEmailTemplate(null, cont.Id, acc.Id);
Messaging.sendEmail(new Messaging.SingleEmailMessage[]{email});
} while(true);
}
}
]]></code>
</test-code>
<!-- End messaging method invocations -->
<!-- Begin system method invocations -->
<test-code>
<description>Problematic system actions in loop</description>
<expected-problems>3</expected-problems>
<code><![CDATA[
public class Foo {
public void test1() {
do {
System.enqueueJob(new MyQueueable());
System.schedule('x', '0 0 0 1 1 ?', new MySchedule());
System.scheduleBatch(new MyBatch(), 'x', 1);
System.debug(LoggingLevel.INFO, 'X');
System.assertEquals(1, 1);
} while(true);
}
}
]]></code>
</test-code>
<test-code>
<description>Problematic system.runAs action in loop</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
public void test1() {
do {
System.runAs(new User()) {
System.debug(LoggingLevel.INFO, 'X');
System.assertEquals(1, 1);
}
} while(true);
}
}
]]></code>
</test-code>
<!-- End system method invocations -->
</test-data>

13
pmd-core/src/main/resources/rulesets/releases/6340.xml Normal file
View File

@ -0,0 +1,13 @@
<?xml version="1.0"?>
<ruleset name="6340"
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
<description>
This ruleset contains links to rules that are new in PMD v6.34.0
</description>
<rule ref="category/java/bestpractices.xml/UseStandardCharsets" />
</ruleset>

9
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnyTypeDeclaration.java
View File

@ -260,6 +260,15 @@ public interface ASTAnyTypeDeclaration
}
/**
* Returns true if this is a regular interface declaration (not an annotation).
* Note that {@link #isInterface()} counts annotations in.
*/
default boolean isRegularInterface() {
return false;
}
/** Returns true if this is an {@linkplain ASTAnnotationTypeDeclaration annotation type declaration}. */
default boolean isAnnotation() {
return this instanceof ASTAnnotationTypeDeclaration;

5
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceDeclaration.java
View File

@ -54,6 +54,11 @@ public final class ASTClassOrInterfaceDeclaration extends AbstractAnyTypeDeclara
return !isInterface;
}
@Override
public boolean isRegularInterface() {
return isInterface;
}
void setInterface() {
this.isInterface = true;
}

6
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTFieldDeclaration.java
View File

@ -60,11 +60,7 @@ public final class ASTFieldDeclaration extends AbstractJavaNode
@Deprecated
@DeprecatedAttribute(replaceWith = "VariableDeclaratorId/@Name")
public String getVariableName() {
ASTVariableDeclaratorId decl = getFirstDescendantOfType(ASTVariableDeclaratorId.class);
if (decl != null) {
return decl.getImage();
}
return null;
return getVarIds().firstOrThrow().getName();
}

11
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTMethodDeclaration.java
View File

@ -9,6 +9,7 @@ import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken;
import net.sourceforge.pmd.lang.java.symbols.JMethodSymbol;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
import net.sourceforge.pmd.lang.rule.xpath.DeprecatedAttribute;
@ -145,4 +146,14 @@ public final class ASTMethodDeclaration extends AbstractMethodOrConstructorDecla
return children(ASTArrayDimensions.class).first();
}
/**
* Returns whether this is a main method declaration.
*/
public boolean isMainMethod() {
return this.hasModifiers(JModifier.PUBLIC, JModifier.STATIC)
&& "main".equals(this.getName())
&& this.isVoid()
&& this.getArity() == 1
&& TypeTestUtil.isExactlyA(String[].class, this.getFormalParameters().get(0));
}
}

60
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/internal/PrettyPrintingUtil.java
View File

@ -9,7 +9,9 @@ import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTArrayType;
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.ASTEnumDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameters;
import net.sourceforge.pmd.lang.java.ast.ASTList;
@ -17,7 +19,10 @@ import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType;
import net.sourceforge.pmd.lang.java.ast.ASTRecordDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTResource;
import net.sourceforge.pmd.lang.java.ast.ASTType;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
/**
* @author Clément Fournier
@ -70,6 +75,12 @@ public final class PrettyPrintingUtil {
}
}
public static String prettyPrintType(ASTType t) {
StringBuilder sb = new StringBuilder();
prettyPrintTypeNode(t, sb);
return sb.toString();
}
/**
* Returns a normalized method name. This just looks at the image of the types of the parameters.
*/
@ -83,7 +94,7 @@ public final class PrettyPrintingUtil {
/**
* Returns the generic kind of declaration this is, eg "enum" or "class".
*/
public static String kindName(ASTAnyTypeDeclaration decl) {
public static String getPrintableNodeKind(ASTAnyTypeDeclaration decl) {
if (decl instanceof ASTClassOrInterfaceDeclaration && decl.isInterface()) {
return "interface";
} else if (decl instanceof ASTAnnotationTypeDeclaration) {
@ -96,4 +107,51 @@ public final class PrettyPrintingUtil {
return "class";
}
/**
* Returns the "name" of a node. For methods and constructors, this
* may return a signature with parameters.
*/
public static String getNodeName(JavaNode node) {
// constructors are differentiated by their parameters, while we only use method name for methods
if (node instanceof ASTMethodDeclaration) {
return ((ASTMethodDeclaration) node).getName();
} else if (node instanceof ASTMethodOrConstructorDeclaration) {
// constructors are differentiated by their parameters, while we only use method name for methods
return displaySignature((ASTConstructorDeclaration) node);
} else if (node instanceof ASTFieldDeclaration) {
return ((ASTFieldDeclaration) node).getVarIds().firstOrThrow().getName();
} else if (node instanceof ASTResource) {
return ((ASTResource) node).getStableName();
} else if (node instanceof ASTAnyTypeDeclaration) {
return ((ASTAnyTypeDeclaration) node).getSimpleName();
} else if (node instanceof ASTVariableDeclaratorId) {
return ((ASTVariableDeclaratorId) node).getName();
} else {
return node.getImage(); // todo get rid of this
}
}
/**
* Returns the 'kind' of node this is. For instance for a {@link ASTFieldDeclaration},
* returns "field".
*
* @throws UnsupportedOperationException If unimplemented for a node kind
* @see #getPrintableNodeKind(ASTAnyTypeDeclaration)
*/
public static String getPrintableNodeKind(JavaNode node) {
if (node instanceof ASTAnyTypeDeclaration) {
return getPrintableNodeKind((ASTAnyTypeDeclaration) node);
} else if (node instanceof ASTMethodDeclaration) {
return "method";
} else if (node instanceof ASTConstructorDeclaration) {
return "constructor";
} else if (node instanceof ASTFieldDeclaration) {
return "field";
} else if (node instanceof ASTResource) {
return "resource specification";
}
throw new UnsupportedOperationException("Node " + node + " is unaccounted for");
}
}

2
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java
View File

@ -32,8 +32,8 @@ import net.sourceforge.pmd.lang.java.ast.BinaryOp;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
import net.sourceforge.pmd.lang.java.types.InvocationMatcher;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil.InvocationMatcher;
/**
* @author Clément Fournier

20
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/GuardLogStatementRule.java
View File

@ -15,10 +15,8 @@ import java.util.logging.Level;
import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTExpressionStatement;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
@ -26,7 +24,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression;
import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
import net.sourceforge.pmd.lang.java.ast.ASTStringLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
import net.sourceforge.pmd.properties.PropertyDescriptor;
@ -39,7 +37,7 @@ import net.sourceforge.pmd.properties.PropertyDescriptor;
* @author Tammo van Lessen - provided original XPath expression
*
*/
public class GuardLogStatementRule extends AbstractJavaRule implements Rule {
public class GuardLogStatementRule extends AbstractJavaRulechainRule {
/*
* guard methods and log levels:
*
@ -84,22 +82,14 @@ public class GuardLogStatementRule extends AbstractJavaRule implements Rule {
private static final String JAVA_UTIL_LOG_GUARD_METHOD = "isLoggable";
public GuardLogStatementRule() {
super(ASTExpressionStatement.class);
definePropertyDescriptor(LOG_LEVELS);
definePropertyDescriptor(GUARD_METHODS);
}
@Override
public Object visit(ASTCompilationUnit unit, Object data) {
public void start(RuleContext ctx) {
extractProperties();
return super.visit(unit, data);
}
@Override
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
if (node.isInterface()) {
return data; // don't consider interfaces
}
return super.visit(node, data);
}
@Override

42
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnitAssertionsShouldIncludeMessageRule.java
View File

@ -4,31 +4,28 @@
package net.sourceforge.pmd.lang.java.rule.bestpractices;
import static net.sourceforge.pmd.util.CollectionUtil.listOf;
import java.util.List;
import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.rule.internal.TestFrameworksUtil;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil.InvocationMatcher;
import net.sourceforge.pmd.lang.java.types.InvocationMatcher;
import net.sourceforge.pmd.lang.java.types.InvocationMatcher.CompoundInvocationMatcher;
public class JUnitAssertionsShouldIncludeMessageRule extends AbstractJavaRulechainRule {
private final List<InvocationMatcher> checks =
listOf(
InvocationMatcher.parse("_#assertEquals(_,_)"),
InvocationMatcher.parse("_#assertTrue(_)"),
InvocationMatcher.parse("_#assertFalse(_)"),
InvocationMatcher.parse("_#assertSame(_,_)"),
InvocationMatcher.parse("_#assertNotSame(_,_)"),
InvocationMatcher.parse("_#assertNull(_)"),
InvocationMatcher.parse("_#assertNotNull(_)"),
InvocationMatcher.parse("_#assertArrayEquals(_,_)"),
InvocationMatcher.parse("_#assertThat(_,_)"),
InvocationMatcher.parse("_#fail()"),
InvocationMatcher.parse("_#assertEquals(float,float,float)"),
InvocationMatcher.parse("_#assertEquals(double,double,double)")
private final CompoundInvocationMatcher checks =
InvocationMatcher.parseAll(
"_#assertEquals(_,_)",
"_#assertTrue(_)",
"_#assertFalse(_)",
"_#assertSame(_,_)",
"_#assertNotSame(_,_)",
"_#assertNull(_)",
"_#assertNotNull(_)",
"_#assertArrayEquals(_,_)",
"_#assertThat(_,_)",
"_#fail()",
"_#assertEquals(float,float,float)",
"_#assertEquals(double,double,double)"
);
public JUnitAssertionsShouldIncludeMessageRule() {
@ -38,11 +35,8 @@ public class JUnitAssertionsShouldIncludeMessageRule extends AbstractJavaRulecha
@Override
public Object visit(ASTMethodCall node, Object data) {
if (TestFrameworksUtil.isCallOnAssertionContainer(node)) {
for (InvocationMatcher check : checks) {
if (check.matchesCall(node)) {
addViolation(data, node);
break;
}
if (checks.anyMatch(node)) {
addViolation(data, node);
}
}
return null;

1
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedAssignmentRule.java
View File

@ -906,6 +906,7 @@ public class UnusedAssignmentRule extends AbstractJavaRule {
return false;
}
@SuppressWarnings("PMD.UnusedPrivateMethod")
private static JVariableSymbol getVarIfUnaryAssignment(ASTUnaryExpression node) {
ASTExpression operand = node.getOperand();
if (!node.getOperator().isPure() && operand instanceof ASTNamedReferenceExpr) {

21
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedFormalParameterRule.java
View File

@ -12,16 +12,18 @@ import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.AccessNode.Visibility;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.ast.JModifier;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.properties.PropertyDescriptor;
public class UnusedFormalParameterRule extends AbstractJavaRule {
public class UnusedFormalParameterRule extends AbstractJavaRulechainRule {
private static final PropertyDescriptor<Boolean> CHECKALL_DESCRIPTOR = booleanProperty("checkAll").desc("Check all methods, including non-private ones").defaultValue(false).build();
public UnusedFormalParameterRule() {
super(ASTConstructorDeclaration.class, ASTMethodDeclaration.class);
definePropertyDescriptor(CHECKALL_DESCRIPTOR);
}
@ -36,19 +38,20 @@ public class UnusedFormalParameterRule extends AbstractJavaRule {
if (node.getVisibility() != Visibility.V_PRIVATE && !getProperty(CHECKALL_DESCRIPTOR)) {
return data;
}
if (node.getBody() != null && !JavaRuleUtil.isSerializationReadObject(node) && !node.isOverridden()) {
if (node.getBody() != null
&& !node.hasModifiers(JModifier.DEFAULT)
&& !JavaRuleUtil.isSerializationReadObject(node)
&& !node.isOverridden()) {
check(node, data);
}
return data;
}
private void check(ASTMethodOrConstructorDeclaration node, Object data) {
if (!node.getEnclosingType().isInterface()) {
for (ASTFormalParameter formal : node.getFormalParameters()) {
ASTVariableDeclaratorId varId = formal.getVarId();
if (JavaRuleUtil.isNeverUsed(varId) && !JavaRuleUtil.isExplicitUnusedVarName(varId.getName())) {
addViolation(data, varId, new Object[] {node instanceof ASTMethodDeclaration ? "method" : "constructor", varId.getName(), });
}
for (ASTFormalParameter formal : node.getFormalParameters()) {
ASTVariableDeclaratorId varId = formal.getVarId();
if (JavaRuleUtil.isNeverUsed(varId) && !JavaRuleUtil.isExplicitUnusedVarName(varId.getName())) {
addViolation(data, varId, new Object[] { node instanceof ASTMethodDeclaration ? "method" : "constructor", varId.getName(), });
}
}
}

2
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ClassNamingConventionsRule.java
View File

@ -91,6 +91,6 @@ public class ClassNamingConventionsRule extends AbstractNamingConventionRule<AST
@Override
String kindDisplayName(ASTAnyTypeDeclaration node, PropertyDescriptor<Pattern> descriptor) {
return JavaRuleUtil.isUtilityClass(node) ? "utility class" : PrettyPrintingUtil.kindName(node);
return JavaRuleUtil.isUtilityClass(node) ? "utility class" : PrettyPrintingUtil.getPrintableNodeKind(node);
}
}

101
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/CommentDefaultAccessModifierRule.java
View File

@ -4,13 +4,12 @@
package net.sourceforge.pmd.lang.java.rule.codestyle;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.regex.Pattern;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
@ -19,12 +18,12 @@ import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclarator;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.AccessNode;
import net.sourceforge.pmd.lang.java.ast.Annotatable;
import net.sourceforge.pmd.lang.java.ast.AccessNode.Visibility;
import net.sourceforge.pmd.lang.java.ast.Comment;
import net.sourceforge.pmd.lang.java.rule.AbstractIgnoredAnnotationRule;
import net.sourceforge.pmd.lang.java.ast.internal.PrettyPrintingUtil;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaPropertyUtil;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertyFactory;
@ -36,36 +35,37 @@ import net.sourceforge.pmd.properties.PropertyFactory;
*
* @author Damián Techeira
*/
public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationRule {
public class CommentDefaultAccessModifierRule extends AbstractJavaRulechainRule {
private static final PropertyDescriptor<Pattern> REGEX_DESCRIPTOR = PropertyFactory.regexProperty("regex")
.desc("Regular expression").defaultValue("\\/\\*\\s+(default|package)\\s+\\*\\/").build();
private static final PropertyDescriptor<Boolean> TOP_LEVEL_TYPES = PropertyFactory.booleanProperty("checkTopLevelTypes")
.desc("Check for default access modifier in top-level classes, annotations, and enums")
.defaultValue(false).build();
private static final String MESSAGE = "To avoid mistakes add a comment "
+ "at the beginning of the %s %s if you want a default access modifier";
private static final PropertyDescriptor<List<String>> IGNORED_ANNOTS =
JavaPropertyUtil.ignoredAnnotationsDescriptor(
"com.google.common.annotations.VisibleForTesting",
"android.support.annotation.VisibleForTesting"
);
private static final PropertyDescriptor<Pattern> REGEX_DESCRIPTOR =
PropertyFactory.regexProperty("regex")
.desc("Regular expression")
.defaultValue("\\/\\*\\s+(default|package)\\s+\\*\\/").build();
private static final PropertyDescriptor<Boolean> TOP_LEVEL_TYPES =
PropertyFactory.booleanProperty("checkTopLevelTypes")
.desc("Check for default access modifier in top-level classes, annotations, and enums")
.defaultValue(false).build();
private static final String MESSAGE = "To avoid mistakes add a comment at the beginning of the {0} {1} if you want a default access modifier";
private final Set<Integer> interestingLineNumberComments = new HashSet<>();
public CommentDefaultAccessModifierRule() {
super(ASTCompilationUnit.class, ASTMethodDeclaration.class, ASTAnyTypeDeclaration.class, ASTConstructorDeclaration.class, ASTFieldDeclaration.class);
definePropertyDescriptor(IGNORED_ANNOTS);
definePropertyDescriptor(REGEX_DESCRIPTOR);
definePropertyDescriptor(TOP_LEVEL_TYPES);
}
@Override
protected Collection<String> defaultSuppressionAnnotations() {
Collection<String> ignoredStrings = new ArrayList<>();
ignoredStrings.add("com.google.common.annotations.VisibleForTesting");
ignoredStrings.add("android.support.annotation.VisibleForTesting");
return ignoredStrings;
}
@Override
public Object visit(final ASTCompilationUnit node, final Object data) {
interestingLineNumberComments.clear();
final List<Comment> comments = node.getComments();
for (final Comment comment : comments) {
if (getProperty(REGEX_DESCRIPTOR).matcher(comment.getImage()).matches()) {
for (final Comment comment : node.getComments()) {
if (getProperty(REGEX_DESCRIPTOR).matcher(comment.getText()).matches()) {
interestingLineNumberComments.add(comment.getBeginLine());
}
}
@ -75,8 +75,7 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR
@Override
public Object visit(final ASTMethodDeclaration decl, final Object data) {
if (shouldReport(decl)) {
addViolationWithMessage(data, decl,
String.format(MESSAGE, decl.getFirstChildOfType(ASTMethodDeclarator.class).getImage(), "method"));
report((RuleContext) data, decl, "method", PrettyPrintingUtil.displaySignature(decl));
}
return super.visit(decl, data);
}
@ -84,8 +83,7 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR
@Override
public Object visit(final ASTFieldDeclaration decl, final Object data) {
if (shouldReport(decl)) {
addViolationWithMessage(data, decl, String.format(MESSAGE,
decl.getFirstDescendantOfType(ASTVariableDeclaratorId.class).getImage(), "field"));
report((RuleContext) data, decl, "field", decl.getVarIds().firstOrThrow().getName());
}
return super.visit(decl, data);
}
@ -93,7 +91,7 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR
@Override
public Object visit(final ASTAnnotationTypeDeclaration decl, final Object data) {
if (!decl.isNested() && shouldReportTypeDeclaration(decl)) { // check for top-level annotation declarations
addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "top-level annotation"));
report((RuleContext) data, decl, "top-level annotation", decl.getSimpleName());
}
return super.visit(decl, data);
}
@ -101,7 +99,7 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR
@Override
public Object visit(final ASTEnumDeclaration decl, final Object data) {
if (!decl.isNested() && shouldReportTypeDeclaration(decl)) { // check for top-level enums
addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "top-level enum"));
report((RuleContext) data, decl, "top-level enum", decl.getSimpleName());
}
return super.visit(decl, data);
}
@ -109,9 +107,9 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR
@Override
public Object visit(final ASTClassOrInterfaceDeclaration decl, final Object data) {
if (decl.isNested() && shouldReport(decl)) { // check for nested classes
addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "nested class"));
report((RuleContext) data, decl, "nested class", decl.getSimpleName());
} else if (!decl.isNested() && shouldReportTypeDeclaration(decl)) { // and for top-level ones
addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "top-level class"));
report((RuleContext) data, decl, "top-level class", decl.getSimpleName());
}
return super.visit(decl, data);
}
@ -119,11 +117,15 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR
@Override
public Object visit(final ASTConstructorDeclaration decl, Object data) {
if (shouldReport(decl)) {
addViolationWithMessage(data, decl, String.format(MESSAGE, decl.getImage(), "constructor"));
report((RuleContext) data, decl, "constructor", PrettyPrintingUtil.displaySignature(decl));
}
return super.visit(decl, data);
}
private void report(RuleContext data, AccessNode decl, String kind, String description) {
addViolationWithMessage(data, decl, MESSAGE, new String[] {kind, description, });
}
private boolean shouldReport(final AccessNode decl) {
final ASTAnyTypeDeclaration enclosing = decl.getEnclosingType();
@ -133,29 +135,26 @@ public class CommentDefaultAccessModifierRule extends AbstractIgnoredAnnotationR
return isConcreteClass && isMissingComment(decl);
}
protected boolean hasIgnoredAnnotation(AccessNode node) {
if (node instanceof Annotatable) {
return hasIgnoredAnnotation((Annotatable) node);
}
return false;
}
private boolean isMissingComment(AccessNode decl) {
// check if the class/method/field has a default access
// modifier
return decl.isPackagePrivate()
// if is a default access modifier check if there is a comment
// in this line
&& !interestingLineNumberComments.contains(decl.getBeginLine())
// that it is not annotated with e.g. @VisibleForTesting
&& !hasIgnoredAnnotation(decl);
return decl.getVisibility() == Visibility.V_PACKAGE
// if is a default access modifier check if there is a comment
// in this line
&& !interestingLineNumberComments.contains(decl.getBeginLine())
// that it is not annotated with e.g. @VisibleForTesting
&& isNotIgnored(decl);
}
private boolean isNotIgnored(AccessNode decl) {
return getProperty(IGNORED_ANNOTS).stream().noneMatch(decl::isAnnotationPresent);
}
private boolean shouldReportTypeDeclaration(ASTAnyTypeDeclaration decl) {
// don't report on interfaces
return !decl.isInterface()
&& isMissingComment(decl)
// either nested or top level and we should check it
&& (decl.isNested() || getProperty(TOP_LEVEL_TYPES));
return !decl.isRegularInterface()
&& isMissingComment(decl)
// either nested or top level and we should check it
&& (decl.isNested() || getProperty(TOP_LEVEL_TYPES));
}
}

8
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/IdenticalCatchBranchesRule.java
View File

@ -11,7 +11,7 @@ import java.util.Set;
import net.sourceforge.pmd.lang.java.ast.ASTCatchClause;
import net.sourceforge.pmd.lang.java.ast.ASTTryStatement;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
@ -21,7 +21,11 @@ import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
* @author Clément Fournier
* @since 6.4.0
*/
public class IdenticalCatchBranchesRule extends AbstractJavaRule {
public class IdenticalCatchBranchesRule extends AbstractJavaRulechainRule {
public IdenticalCatchBranchesRule() {
super(ASTTryStatement.class);
}
private boolean areEquivalent(ASTCatchClause st1, ASTCatchClause st2) {

73
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LinguisticNamingRule.java
View File

@ -1,15 +1,14 @@
/**
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.codestyle;
import static net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil.containsCamelCaseWord;
import static net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil.startsWithCamelCaseWord;
import static net.sourceforge.pmd.properties.PropertyFactory.booleanProperty;
import static net.sourceforge.pmd.properties.PropertyFactory.stringListProperty;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
@ -20,11 +19,18 @@ import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTType;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
import net.sourceforge.pmd.lang.java.rule.AbstractIgnoredAnnotationRule;
import net.sourceforge.pmd.lang.java.ast.Annotatable;
import net.sourceforge.pmd.lang.java.ast.internal.PrettyPrintingUtil;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaPropertyUtil;
import net.sourceforge.pmd.lang.java.types.JPrimitiveType.PrimitiveTypeKind;
import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
import net.sourceforge.pmd.properties.PropertyDescriptor;
public class LinguisticNamingRule extends AbstractIgnoredAnnotationRule {
public class LinguisticNamingRule extends AbstractJavaRulechainRule {
private static final PropertyDescriptor<List<String>> IGNORED_ANNOTS =
JavaPropertyUtil.ignoredAnnotationsDescriptor("java.lang.Override");
private static final PropertyDescriptor<Boolean> CHECK_BOOLEAN_METHODS =
booleanProperty("checkBooleanMethod").defaultValue(true).desc("Check method names and types for inconsistent naming.").build();
private static final PropertyDescriptor<Boolean> CHECK_GETTERS =
@ -57,6 +63,8 @@ public class LinguisticNamingRule extends AbstractIgnoredAnnotationRule {
.defaultValues("is", "has", "can", "have", "will", "should").build();
public LinguisticNamingRule() {
super(ASTMethodDeclaration.class, ASTFieldDeclaration.class, ASTLocalVariableDeclaration.class);
definePropertyDescriptor(IGNORED_ANNOTS);
definePropertyDescriptor(CHECK_BOOLEAN_METHODS);
definePropertyDescriptor(CHECK_GETTERS);
definePropertyDescriptor(CHECK_SETTERS);
@ -67,14 +75,6 @@ public class LinguisticNamingRule extends AbstractIgnoredAnnotationRule {
definePropertyDescriptor(CHECK_FIELDS);
definePropertyDescriptor(CHECK_VARIABLES);
definePropertyDescriptor(BOOLEAN_FIELD_PREFIXES_PROPERTY);
addRuleChainVisit(ASTMethodDeclaration.class);
addRuleChainVisit(ASTFieldDeclaration.class);
addRuleChainVisit(ASTLocalVariableDeclaration.class);
}
@Override
protected Collection<String> defaultSuppressionAnnotations() {
return Collections.checkedList(Arrays.asList("java.lang.Override"), String.class);
}
@Override
@ -105,6 +105,10 @@ public class LinguisticNamingRule extends AbstractIgnoredAnnotationRule {
return data;
}
private boolean hasIgnoredAnnotation(Annotatable node) {
return node.isAnyAnnotationPresent(getProperty(IGNORED_ANNOTS));
}
private void checkPrefixedTransformMethods(ASTMethodDeclaration node, Object data, String nameOfMethod) {
List<String> prefixes = getProperty(TRANSFORM_METHOD_NAMES_PROPERTY);
String[] splitMethodName = StringUtils.splitByCharacterTypeCamelCase(nameOfMethod);
@ -117,9 +121,8 @@ public class LinguisticNamingRule extends AbstractIgnoredAnnotationRule {
}
private void checkTransformMethods(ASTMethodDeclaration node, Object data, String nameOfMethod) {
List<String> infixes = getProperty(TRANSFORM_METHOD_NAMES_PROPERTY);
for (String infix : infixes) {
if (node.isVoid() && containsWord(nameOfMethod, StringUtils.capitalize(infix))) {
for (String infix : getProperty(TRANSFORM_METHOD_NAMES_PROPERTY)) {
if (node.isVoid() && containsCamelCaseWord(nameOfMethod, StringUtils.capitalize(infix))) {
// "To" or any other configured infix in the middle somewhere
addViolationWithMessage(data, node, "Linguistics Antipattern - The transform method ''{0}'' should not return void linguistically",
new Object[] { nameOfMethod });
@ -130,21 +133,21 @@ public class LinguisticNamingRule extends AbstractIgnoredAnnotationRule {
}
private void checkGetters(ASTMethodDeclaration node, Object data, String nameOfMethod) {
if (hasPrefix(nameOfMethod, "get") && node.isVoid()) {
if (startsWithCamelCaseWord(nameOfMethod, "get") && node.isVoid()) {
addViolationWithMessage(data, node, "Linguistics Antipattern - The getter ''{0}'' should not return void linguistically",
new Object[] { nameOfMethod });
}
}
private void checkSetters(ASTMethodDeclaration node, Object data, String nameOfMethod) {
if (hasPrefix(nameOfMethod, "set") && !node.isVoid()) {
if (startsWithCamelCaseWord(nameOfMethod, "set") && !node.isVoid()) {
addViolationWithMessage(data, node, "Linguistics Antipattern - The setter ''{0}'' should not return any type except void linguistically",
new Object[] { nameOfMethod });
}
}
private boolean isBooleanType(ASTType node) {
return "boolean".equalsIgnoreCase(node.getTypeImage())
return node.getTypeMirror().unbox().isPrimitive(PrimitiveTypeKind.BOOLEAN)
|| TypeTestUtil.isA("java.util.concurrent.atomic.AtomicBoolean", node)
|| TypeTestUtil.isA("java.util.function.Predicate", node);
}
@ -153,9 +156,9 @@ public class LinguisticNamingRule extends AbstractIgnoredAnnotationRule {
ASTType t = node.getResultTypeNode();
if (!t.isVoid()) {
for (String prefix : getProperty(BOOLEAN_METHOD_PREFIXES_PROPERTY)) {
if (hasPrefix(nameOfMethod, prefix) && !isBooleanType(t)) {
if (startsWithCamelCaseWord(nameOfMethod, prefix) && !isBooleanType(t)) {
addViolationWithMessage(data, node, "Linguistics Antipattern - The method ''{0}'' indicates linguistically it returns a boolean, but it returns ''{1}''",
new Object[] { nameOfMethod, t.getTypeImage() });
new Object[] {nameOfMethod, PrettyPrintingUtil.prettyPrintType(t) });
}
}
}
@ -163,18 +166,18 @@ public class LinguisticNamingRule extends AbstractIgnoredAnnotationRule {
private void checkField(ASTType typeNode, ASTVariableDeclarator node, Object data) {
for (String prefix : getProperty(BOOLEAN_FIELD_PREFIXES_PROPERTY)) {
if (hasPrefix(node.getName(), prefix) && !isBooleanType(typeNode)) {
if (startsWithCamelCaseWord(node.getName(), prefix) && !isBooleanType(typeNode)) {
addViolationWithMessage(data, node, "Linguistics Antipattern - The field ''{0}'' indicates linguistically it is a boolean, but it is ''{1}''",
new Object[] { node.getName(), typeNode.getTypeImage() });
new Object[] { node.getName(), PrettyPrintingUtil.prettyPrintType(typeNode) });
}
}
}
private void checkVariable(ASTType typeNode, ASTVariableDeclarator node, Object data) {
for (String prefix : getProperty(BOOLEAN_FIELD_PREFIXES_PROPERTY)) {
if (hasPrefix(node.getName(), prefix) && !isBooleanType(typeNode)) {
if (startsWithCamelCaseWord(node.getName(), prefix) && !isBooleanType(typeNode)) {
addViolationWithMessage(data, node, "Linguistics Antipattern - The variable ''{0}'' indicates linguistically it is a boolean, but it is ''{1}''",
new Object[] { node.getName(), typeNode.getTypeImage() });
new Object[] { node.getName(), PrettyPrintingUtil.prettyPrintType(typeNode) });
}
}
}
@ -183,8 +186,7 @@ public class LinguisticNamingRule extends AbstractIgnoredAnnotationRule {
public Object visit(ASTFieldDeclaration node, Object data) {
ASTType type = node.getTypeNode();
if (type != null && getProperty(CHECK_FIELDS)) {
List<ASTVariableDeclarator> fields = node.findChildrenOfType(ASTVariableDeclarator.class);
for (ASTVariableDeclarator field : fields) {
for (ASTVariableDeclarator field : node.children(ASTVariableDeclarator.class)) {
checkField(type, field, data);
}
}
@ -195,24 +197,11 @@ public class LinguisticNamingRule extends AbstractIgnoredAnnotationRule {
public Object visit(ASTLocalVariableDeclaration node, Object data) {
ASTType type = node.getTypeNode();
if (type != null && getProperty(CHECK_VARIABLES)) {
List<ASTVariableDeclarator> variables = node.findChildrenOfType(ASTVariableDeclarator.class);
for (ASTVariableDeclarator variable : variables) {
for (ASTVariableDeclarator variable : node.children(ASTVariableDeclarator.class)) {
checkVariable(type, variable, data);
}
}
return data;
}
private static boolean hasPrefix(String name, String prefix) {
return name.startsWith(prefix) && name.length() > prefix.length()
&& Character.isUpperCase(name.charAt(prefix.length()));
}
private static boolean containsWord(String name, String word) {
int index = name.indexOf(word);
if (index >= 0 && name.length() > index + word.length()) {
return Character.isUpperCase(name.charAt(index + word.length()));
}
return false;
}
}

138
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/PrematureDeclarationRule.java
View File

@ -4,20 +4,28 @@
package net.sourceforge.pmd.lang.java.rule.codestyle;
import static java.util.Collections.emptySet;
import static net.sourceforge.pmd.lang.ast.NodeStream.asInstanceOf;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
import net.sourceforge.pmd.lang.ast.NodeStream;
import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTForInit;
import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTResource;
import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement;
import net.sourceforge.pmd.lang.java.ast.ASTStatement;
import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement;
import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
import net.sourceforge.pmd.lang.java.types.InvocationMatcher;
import net.sourceforge.pmd.lang.java.types.InvocationMatcher.CompoundInvocationMatcher;
/**
* Checks for variables in methods that are defined before they are really
@ -27,90 +35,98 @@ import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
*
* @author Brian Remedios
*/
public class PrematureDeclarationRule extends AbstractJavaRule {
public class PrematureDeclarationRule extends AbstractJavaRulechainRule {
private static final CompoundInvocationMatcher TIME_METHODS =
InvocationMatcher.parseAll(
"java.lang.System#nanoTime()",
"java.lang.System#currentTimeMillis()"
);
public PrematureDeclarationRule() {
super(ASTLocalVariableDeclaration.class);
}
@Override
public Object visit(ASTLocalVariableDeclaration node, Object data) {
// is it part of a for-loop declaration?
if (node.getParent() instanceof ASTForInit) {
// yes, those don't count
return super.visit(node, data);
if (node.getParent() instanceof ASTForInit
|| node.getParent() instanceof ASTResource) {
// those don't count
return null;
}
for (ASTVariableDeclaratorId id : node) {
for (ASTBlockStatement block : statementsAfter(node)) {
if (hasReferencesIn(block, id.getVariableName())) {
ASTExpression initializer = id.getInitializer();
if (JavaRuleUtil.isNeverUsed(id) // avoid the duplicate with unused variables
|| cannotBeMoved(initializer)
|| JavaRuleUtil.hasSideEffect(initializer, emptySet())) {
continue;
}
Set<JVariableSymbol> refsInInitializer = getReferencedVars(initializer);
// If there's no initializer, or the initializer doesn't depend on anything (eg, a literal),
// then we don't care about side-effects
boolean hasStatefulInitializer = !refsInInitializer.isEmpty() || JavaRuleUtil.hasSideEffect(initializer, emptySet());
for (ASTStatement stmt : statementsAfter(node)) {
if (hasReferencesIn(stmt, id)
|| hasStatefulInitializer && JavaRuleUtil.hasSideEffect(stmt, refsInInitializer)) {
break;
}
if (hasExit(block)) {
addViolation(data, node);
if (hasExit(stmt)) {
addViolation(data, node, id.getName());
break;
}
}
}
return super.visit(node, data);
return null;
}
/**
* Returns the set of local variables referenced inside the expression.
*/
private static Set<JVariableSymbol> getReferencedVars(ASTExpression term) {
return term == null ? emptySet()
: term.descendantsOrSelf()
.filterIs(ASTNamedReferenceExpr.class)
.filter(it -> it.getReferencedSym() != null)
.collect(Collectors.mapping(ASTNamedReferenceExpr::getReferencedSym, Collectors.toSet()));
}
/**
* Time methods cannot be moved ever, even when there are no side-effects.
* The side effect they depend on is the program being executed. Are they
* the only methods like that?
*/
private boolean cannotBeMoved(ASTExpression initializer) {
return TIME_METHODS.anyMatch(initializer);
}
/**
* Returns whether the block contains a return call or throws an exception.
* Exclude blocks that have these things as part of an inner class.
*/
private boolean hasExit(ASTBlockStatement block) {
return block.descendants().map(asInstanceOf(ASTThrowStatement.class, ASTReturnStatement.class)).nonEmpty();
private static boolean hasExit(ASTStatement block) {
return block.descendants()
.map(asInstanceOf(ASTThrowStatement.class, ASTReturnStatement.class))
.nonEmpty();
}
/**
* Returns whether the variable is mentioned within the statement or not.
*/
private static boolean hasReferencesIn(ASTBlockStatement block, String varName) {
// allow for closures on the var
for (ASTName name : block.findDescendantsOfType(ASTName.class, true)) {
if (isReference(varName, name.getImage())) {
return true;
}
}
return false;
private static boolean hasReferencesIn(ASTStatement stmt, ASTVariableDeclaratorId var) {
return stmt.descendants(ASTVariableAccess.class)
.crossFindBoundaries()
.filterMatching(ASTNamedReferenceExpr::getReferencedSym, var.getSymbol())
.nonEmpty();
}
/**
* Return whether the shortName is part of the compound name by itself or as
* a method call receiver.
*/
private static boolean isReference(String shortName, String compoundName) {
int dotPos = compoundName.indexOf('.');
return dotPos < 0 ? shortName.equals(compoundName) : shortName.equals(compoundName.substring(0, dotPos));
}
/**
* Returns all the block statements following the given local var declaration.
*/
private static List<ASTBlockStatement> statementsAfter(ASTLocalVariableDeclaration node) {
Node blockOrSwitch = node.getParent().getParent();
int count = blockOrSwitch.getNumChildren();
int start = node.getParent().getIndexInParent() + 1;
List<ASTBlockStatement> nextBlocks = new ArrayList<>(count - start);
for (int i = start; i < count; i++) {
Node maybeBlock = blockOrSwitch.getChild(i);
if (maybeBlock instanceof ASTBlockStatement) {
nextBlocks.add((ASTBlockStatement) maybeBlock);
}
}
return nextBlocks;
/** Returns all the statements following the given local var declaration. */
private static NodeStream<ASTStatement> statementsAfter(ASTLocalVariableDeclaration node) {
return node.asStream().followingSiblings().filterIs(ASTStatement.class);
}
}

139
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryModifierRule.java
View File

@ -14,9 +14,7 @@ import java.util.EnumSet;
import java.util.Set;
import org.apache.commons.lang3.StringUtils;
import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
@ -24,83 +22,46 @@ import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTResource;
import net.sourceforge.pmd.lang.java.ast.AccessNode;
import net.sourceforge.pmd.lang.java.ast.JModifier;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.ast.internal.PrettyPrintingUtil;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
public class UnnecessaryModifierRule extends AbstractJavaRule {
public class UnnecessaryModifierRule extends AbstractJavaRulechainRule {
public UnnecessaryModifierRule() {
addRuleChainVisit(ASTEnumDeclaration.class);
addRuleChainVisit(ASTAnnotationTypeDeclaration.class);
addRuleChainVisit(ASTClassOrInterfaceDeclaration.class);
addRuleChainVisit(ASTMethodDeclaration.class);
addRuleChainVisit(ASTResource.class);
addRuleChainVisit(ASTFieldDeclaration.class);
addRuleChainVisit(ASTConstructorDeclaration.class);
super(ASTAnyTypeDeclaration.class,
ASTMethodDeclaration.class,
ASTResource.class,
ASTFieldDeclaration.class,
ASTConstructorDeclaration.class);
}
private void reportUnnecessaryModifiers(Object data, Node node,
private void reportUnnecessaryModifiers(Object data, JavaNode node,
JModifier unnecessaryModifier, String explanation) {
reportUnnecessaryModifiers(data, node, EnumSet.of(unnecessaryModifier), explanation);
}
private void reportUnnecessaryModifiers(Object data, Node node,
private void reportUnnecessaryModifiers(Object data, JavaNode node,
Set<JModifier> unnecessaryModifiers, String explanation) {
if (unnecessaryModifiers.isEmpty()) {
return;
}
super.addViolation(data, node, new String[]{
formatUnnecessaryModifiers(unnecessaryModifiers),
getPrintableNodeKind(node),
getNodeName(node),
PrettyPrintingUtil.getPrintableNodeKind(node),
PrettyPrintingUtil.getNodeName(node),
explanation.isEmpty() ? "" : ": " + explanation,
});
}
// TODO this should probably make it into a PrettyPrintingUtil or something.
private String getNodeName(Node node) {
// constructors are differentiated by their parameters, while we only use method name for methods
if (node instanceof ASTMethodDeclaration) {
return ((ASTMethodDeclaration) node).getName();
} else if (node instanceof ASTMethodOrConstructorDeclaration) {
// constructors are differentiated by their parameters, while we only use method name for methods
return PrettyPrintingUtil.displaySignature((ASTConstructorDeclaration) node);
} else if (node instanceof ASTFieldDeclaration) {
return ((ASTFieldDeclaration) node).getVariableName();
} else if (node instanceof ASTResource) {
return ((ASTResource) node).getStableName();
} else {
return node.getImage();
}
}
// TODO same here
private String getPrintableNodeKind(Node node) {
if (node instanceof ASTAnyTypeDeclaration) {
return PrettyPrintingUtil.kindName((ASTAnyTypeDeclaration) node);
} else if (node instanceof ASTMethodDeclaration) {
return "method";
} else if (node instanceof ASTConstructorDeclaration) {
return "constructor";
} else if (node instanceof ASTFieldDeclaration) {
return "field";
} else if (node instanceof ASTResource) {
return "resource specification";
}
throw new UnsupportedOperationException("Node " + node + " is unaccounted for");
}
private String formatUnnecessaryModifiers(Set<JModifier> set) {
// prints in the standard modifier order (sorted by enum constant ordinal),
// regardless of the actual order in which we checked
@ -126,7 +87,7 @@ public class UnnecessaryModifierRule extends AbstractJavaRule {
@Override
public Object visit(ASTAnnotationTypeDeclaration node, Object data) {
if (node.isAbstract()) {
if (node.hasExplicitModifiers(ABSTRACT)) {
// may have several violations, with different explanations
reportUnnecessaryModifiers(data, node, ABSTRACT, "annotations types are implicitly abstract");
@ -137,12 +98,7 @@ public class UnnecessaryModifierRule extends AbstractJavaRule {
return data;
}
// a public annotation within an interface or annotation
if (node.hasExplicitModifiers(PUBLIC) && isParentInterfaceType(node.getEnclosingType())) {
reportUnnecessaryModifiers(data, node, PUBLIC,
"members of " + getPrintableNodeKind(node.getEnclosingType())
+ " types are implicitly public");
}
checkDeclarationInInterfaceType(data, node, EnumSet.of(PUBLIC));
if (node.hasExplicitModifiers(STATIC)) {
// a static annotation
@ -153,7 +109,8 @@ public class UnnecessaryModifierRule extends AbstractJavaRule {
}
// also considers annotations, as should ASTAnyTypeDeclaration do
private boolean isParentInterfaceType(@Nullable ASTAnyTypeDeclaration enclosing) {
private boolean isParentInterfaceType(AccessNode node) {
ASTAnyTypeDeclaration enclosing = node.getEnclosingType();
return enclosing != null && enclosing.isInterface();
}
@ -170,23 +127,11 @@ public class UnnecessaryModifierRule extends AbstractJavaRule {
return data;
}
boolean isParentInterfaceOrAnnotation = isParentInterfaceType(node.getEnclosingType());
checkDeclarationInInterfaceType(data, node, EnumSet.of(PUBLIC, STATIC));
// a public class or interface within an interface or annotation
if (node.hasExplicitModifiers(PUBLIC) && isParentInterfaceOrAnnotation) {
reportUnnecessaryModifiers(data, node, PUBLIC,
"members of " + getPrintableNodeKind(node.getEnclosingType())
+ " types are implicitly public");
}
if (node.hasExplicitModifiers(STATIC)) {
if (node.isInterface()) {
// a static interface
reportUnnecessaryModifiers(data, node, STATIC, "member interfaces are implicitly static");
} else if (isParentInterfaceOrAnnotation) {
// a type nested within an interface
reportUnnecessaryModifiers(data, node, STATIC, "types nested within an interface type are implicitly static");
}
if (node.hasExplicitModifiers(STATIC) && node.isInterface() && !isParentInterfaceType(node)) {
// a static interface
reportUnnecessaryModifiers(data, node, STATIC, "member interfaces are implicitly static");
}
return data;
@ -194,16 +139,8 @@ public class UnnecessaryModifierRule extends AbstractJavaRule {
@Override
public Object visit(final ASTMethodDeclaration node, Object data) {
Set<JModifier> unnecessary = EnumSet.noneOf(JModifier.class);
if (node.hasExplicitModifiers(PUBLIC)) {
unnecessary.add(PUBLIC);
}
if (node.hasExplicitModifiers(ABSTRACT)) {
unnecessary.add(ABSTRACT);
}
checkDeclarationInInterfaceType(data, node, unnecessary);
checkDeclarationInInterfaceType(data, node, EnumSet.of(PUBLIC, ABSTRACT));
if (node.hasExplicitModifiers(FINAL)) {
// If the method is annotated by @SafeVarargs then it's ok
@ -237,27 +174,14 @@ public class UnnecessaryModifierRule extends AbstractJavaRule {
@Override
public Object visit(ASTFieldDeclaration node, Object data) {
Set<JModifier> unnecessary = EnumSet.noneOf(JModifier.class);
if (node.hasExplicitModifiers(PUBLIC)) {
unnecessary.add(PUBLIC);
}
if (node.hasExplicitModifiers(STATIC)) {
unnecessary.add(STATIC);
}
if (node.hasExplicitModifiers(FINAL)) {
unnecessary.add(FINAL);
}
checkDeclarationInInterfaceType(data, node, unnecessary);
checkDeclarationInInterfaceType(data, node, EnumSet.of(PUBLIC, STATIC, FINAL));
return data;
}
@Override
public Object visit(ASTConstructorDeclaration node, Object data) {
if (node.getEnclosingType().isEnum()) {
if (node.hasExplicitModifiers(PRIVATE)) {
reportUnnecessaryModifiers(data, node, PRIVATE, "enum constructors are implicitly private");
}
if (node.getEnclosingType().isEnum() && node.hasExplicitModifiers(PRIVATE)) {
reportUnnecessaryModifiers(data, node, PRIVATE, "enum constructors are implicitly private");
}
return data;
}
@ -268,16 +192,17 @@ public class UnnecessaryModifierRule extends AbstractJavaRule {
}
private void checkDeclarationInInterfaceType(Object data, JavaNode fieldOrMethod, Set<JModifier> unnecessary) {
private void checkDeclarationInInterfaceType(Object data, AccessNode member, Set<JModifier> unnecessary) {
// third ancestor could be an AllocationExpression
// if this is a method in an anonymous inner class
ASTAnyTypeDeclaration parent = fieldOrMethod.getEnclosingType();
if (parent.isInterface()) {
reportUnnecessaryModifiers(data, fieldOrMethod, unnecessary,
"the " + getPrintableNodeKind(fieldOrMethod) + " is declared in an "
+ getPrintableNodeKind(parent) + " type");
ASTAnyTypeDeclaration parent = member.getEnclosingType();
if (isParentInterfaceType(member)) {
unnecessary.removeIf(mod -> !member.hasExplicitModifiers(mod));
String explanation = "the " + PrettyPrintingUtil.getPrintableNodeKind(member)
+ " is declared in an " + PrettyPrintingUtil.getPrintableNodeKind(parent) + " type";
reportUnnecessaryModifiers(data, member, unnecessary, explanation);
}
}
}

84
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryReturnRule.java
View File

@ -1,34 +1,86 @@
/**
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.codestyle;
import net.sourceforge.pmd.lang.java.ast.ASTBlock;
import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.ast.NodeStream;
import net.sourceforge.pmd.lang.java.ast.ASTCompactConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
import net.sourceforge.pmd.lang.java.ast.ASTInitializer;
import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression;
import net.sourceforge.pmd.lang.java.ast.ASTLoopStatement;
import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement;
import net.sourceforge.pmd.lang.java.ast.ASTStatement;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.ast.ASTSwitchArrowBranch;
import net.sourceforge.pmd.lang.java.ast.ASTSwitchBranch;
import net.sourceforge.pmd.lang.java.ast.ASTSwitchExpression;
import net.sourceforge.pmd.lang.java.ast.ASTSwitchFallthroughBranch;
import net.sourceforge.pmd.lang.java.ast.ASTTryStatement;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
public class UnnecessaryReturnRule extends AbstractJavaRule {
public class UnnecessaryReturnRule extends AbstractJavaRulechainRule {
@Override
public Object visit(ASTMethodDeclaration node, Object data) {
if (node.isVoid()) {
super.visit(node, data);
}
return data;
public UnnecessaryReturnRule() {
super(ASTReturnStatement.class);
}
@Override
public Object visit(ASTReturnStatement node, Object data) {
if (node.getParent() instanceof ASTStatement && node.getNthParent(2) instanceof ASTBlockStatement
&& node.getNthParent(3) instanceof ASTBlock && node.getNthParent(4) instanceof ASTMethodDeclaration) {
if (node.getNumChildren() > 0) {
return null;
}
NodeStream<ASTStatement> enclosingStatements =
node.ancestorsOrSelf()
.takeWhile(it -> !isCfgLimit(it))
.filterIs(ASTStatement.class);
if (enclosingStatements.all(UnnecessaryReturnRule::isLastStatementOfParent)) {
addViolation(data, node);
}
return data;
return null;
}
private boolean isCfgLimit(JavaNode it) {
return it instanceof ASTMethodOrConstructorDeclaration
|| it instanceof ASTCompactConstructorDeclaration
|| it instanceof ASTInitializer
|| it instanceof ASTLambdaExpression;
}
/**
* Returns true if this is the last statement of the parent node,
* ie the next statement to be executed is after the parent in the
* CFG.
*/
private static boolean isLastStatementOfParent(ASTStatement it) {
// Note that local class declaration statements could be ignored
// because they don't contribute anything to control flow. But this
// is rare enough that this has not been implemented. A corresponding
// test is in the test file.
JavaNode parent = it.getParent();
if (JavaRuleUtil.isLastChild(it)) {
if (parent instanceof ASTSwitchArrowBranch) {
return !isBranchOfSwitchExpr((ASTSwitchBranch) parent);
} else if (parent instanceof ASTSwitchFallthroughBranch) {
return JavaRuleUtil.isLastChild(parent) && !isBranchOfSwitchExpr((ASTSwitchBranch) parent);
} else {
return !(parent instanceof ASTLoopStatement); // returns break the loop so are not unnecessary (though it could be replaced by break)
}
}
// so we're not the last child...
return parent instanceof ASTIfStatement // maybe we're before the else clause
|| parent instanceof ASTTryStatement; // maybe we're the body of a try
// also maybe we're the body of a do/while, but that is a loop, so it's necessary
}
private static boolean isBranchOfSwitchExpr(ASTSwitchBranch branch) {
return branch.getParent() instanceof ASTSwitchExpression;
}
}

2
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/CyclomaticComplexityRule.java
View File

@ -84,7 +84,7 @@ public class CyclomaticComplexityRule extends AbstractJavaRulechainRule {
if (classWmc >= getProperty(CLASS_LEVEL_DESCRIPTOR)) {
int classHighest = (int) MetricsUtil.computeStatistics(JavaMetrics.CYCLO, node.getOperations(), cycloOptions).getMax();
String[] messageParams = {PrettyPrintingUtil.kindName(node),
String[] messageParams = {PrettyPrintingUtil.getPrintableNodeKind(node),
node.getSimpleName(),
" total",
classWmc + " (highest " + classHighest + ")", };

9
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/GodClassRule.java
View File

@ -10,7 +10,7 @@ import static net.sourceforge.pmd.lang.java.metrics.JavaMetrics.TIGHT_CLASS_COHE
import static net.sourceforge.pmd.lang.java.metrics.JavaMetrics.WEIGHED_METHOD_COUNT;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.metrics.MetricsUtil;
import net.sourceforge.pmd.util.StringUtil;
@ -24,7 +24,7 @@ import net.sourceforge.pmd.util.StringUtil;
*
* @since 5.0
*/
public class GodClassRule extends AbstractJavaRule {
public class GodClassRule extends AbstractJavaRulechainRule {
/**
* Very high threshold for WMC (Weighted Method Count). See: Lanza. Object-Oriented Metrics in Practice. Page 16.
@ -42,6 +42,11 @@ public class GodClassRule extends AbstractJavaRule {
private static final double TCC_THRESHOLD = 1.0 / 3.0;
public GodClassRule() {
super(ASTClassOrInterfaceDeclaration.class);
}
@Override
public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
if (!MetricsUtil.supportsAll(node, WEIGHED_METHOD_COUNT, TIGHT_CLASS_COHESION, ACCESS_TO_FOREIGN_DATA)) {

2
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/NcssCountRule.java
View File

@ -97,7 +97,7 @@ public final class NcssCountRule extends AbstractJavaRulechainRule {
int classHighest = (int) MetricsUtil.computeStatistics(JavaMetrics.NCSS, node.getOperations(), ncssOptions).getMax();
if (classSize >= level) {
String[] messageParams = {PrettyPrintingUtil.kindName(node),
String[] messageParams = {PrettyPrintingUtil.getPrintableNodeKind(node),
node.getSimpleName(),
classSize + " (Highest = " + classHighest + ")", };

7
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentContentRule.java
View File

@ -19,7 +19,7 @@ import org.apache.commons.lang3.StringUtils;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.lang.java.ast.Comment;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertySource;
@ -30,7 +30,7 @@ import net.sourceforge.pmd.properties.PropertySource;
*
* @author Brian Remedios
*/
public class CommentContentRule extends AbstractJavaRule {
public class CommentContentRule extends AbstractJavaRulechainRule {
private boolean caseSensitive;
private List<String> originalBadWords;
@ -52,6 +52,7 @@ public class CommentContentRule extends AbstractJavaRule {
}
public CommentContentRule() {
super(ASTCompilationUnit.class);
definePropertyDescriptor(CASE_SENSITIVE_DESCRIPTOR);
definePropertyDescriptor(DISSALLOWED_TERMS_DESCRIPTOR);
}
@ -131,7 +132,7 @@ public class CommentContentRule extends AbstractJavaRule {
addViolationWithMessage(data, cUnit, errorMsgFor(badWords), comment.getBeginLine(), comment.getEndLine());
}
return super.visit(cUnit, data);
return null;
}
private boolean hasDisallowedTerms() {

6
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredRule.java
View File

@ -14,6 +14,7 @@ import java.util.Map;
import java.util.logging.Logger;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.java.ast.ASTBodyDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration;
@ -22,7 +23,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.ast.JavadocCommentOwner;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
import net.sourceforge.pmd.properties.PropertyBuilder.GenericPropertyBuilder;
import net.sourceforge.pmd.properties.PropertyDescriptor;
@ -32,7 +33,7 @@ import net.sourceforge.pmd.properties.PropertyFactory;
/**
* @author Brian Remedios
*/
public class CommentRequiredRule extends AbstractJavaRule {
public class CommentRequiredRule extends AbstractJavaRulechainRule {
private static final Logger LOG = Logger.getLogger(CommentRequiredRule.class.getName());
// Used to pretty print a message
@ -67,6 +68,7 @@ public class CommentRequiredRule extends AbstractJavaRule {
private final Map<PropertyDescriptor<CommentRequirement>, CommentRequirement> propertyValues = new HashMap<>();
public CommentRequiredRule() {
super(ASTBodyDeclaration.class);
definePropertyDescriptor(OVERRIDE_CMT_DESCRIPTOR);
definePropertyDescriptor(ACCESSOR_CMT_DESCRIPTOR);
definePropertyDescriptor(CLASS_CMT_REQUIREMENT_DESCRIPTOR);

Some files were not shown because too many files have changed in this diff Show More