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

Merge branch 'master' into deprecate-DuplicateImports-ImportsFromSamePackage

Browse Source
This commit is contained in:
Clément Fournier
2021-04-12 17:12:29 +02:00
parent 0726ec9673 92e4631fc2
commit 99cc370a96
12 changed files with 453 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
docs/pages/release_notes.md
View File

@ -14,6 +14,13 @@ 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.
#### Deprecated rules
* java-codestyle
@ -25,9 +32,19 @@ This is a {{ site.pmd.release_type }} release.
### 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 %}

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

@ -11,6 +11,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;
@ -19,6 +20,14 @@ import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
* Warn users when code that could trigger governor limits is executing within a looping construct.
*/
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" };
public OperationWithLimitsInLoopRule() {
setProperty(CODECLIMATE_CATEGORIES, "Performance");
// Note: Often more complicated as just moving a few lines.
@ -33,12 +42,13 @@ public class OperationWithLimitsInLoopRule extends AbstractAvoidNodeInLoopsRule
addRuleChainVisit(ASTDmlUndeleteStatement.class);
addRuleChainVisit(ASTDmlUpdateStatement.class);
addRuleChainVisit(ASTDmlUpsertStatement.class);
// Database methods
addRuleChainVisit(ASTMethodCallExpression.class);
// SOQL
addRuleChainVisit(ASTSoqlExpression.class);
// SOSL
addRuleChainVisit(ASTSoslExpression.class);
// Other limit consuming methods
addRuleChainVisit(ASTRunAsBlockStatement.class);
addRuleChainVisit(ASTMethodCallExpression.class);
}
// Begin DML Statements
@ -73,17 +83,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) {
@ -97,4 +96,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>

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

@ -5,6 +5,7 @@
package net.sourceforge.pmd.lang.java.ast;
import java.util.Iterator;
import java.util.List;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.annotation.InternalApi;
@ -45,6 +46,11 @@ public class ASTLocalVariableDeclaration extends AbstractJavaAccessNode implemen
return visitor.visit(this, data);
}
@Override
public List<ASTAnnotation> getDeclaredAnnotations() {
return findChildrenOfType(ASTAnnotation.class);
}
@Override
public boolean hasSuppressWarningsAnnotationFor(Rule rule) {
for (int i = 0; i < getNumChildren(); i++) {

5
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloseResourceRule.java
View File

@ -45,6 +45,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression;
import net.sourceforge.pmd.lang.java.ast.ASTTryStatement;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.Annotatable;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.ast.TypeNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
@ -187,6 +188,10 @@ public class CloseResourceRule extends AbstractJavaRule {
List<ASTVariableDeclarator> vars = method.findDescendantsOfType(ASTVariableDeclarator.class);
Map<ASTVariableDeclarator, TypeNode> resVars = new HashMap<>();
for (ASTVariableDeclarator var : vars) {
if (var.getParent() instanceof Annotatable
&& ((Annotatable) var.getParent()).isAnnotationPresent("lombok.Cleanup")) {
continue; // auto cleaned up
}
TypeNode varType = getTypeOfVariable(var);
if (varType != null && isResourceTypeOrSubtype(varType)) {
resVars.put(var, wrappedResourceTypeOrReturn(var, varType));

45
pmd-java/src/main/resources/category/java/bestpractices.xml
View File

@ -1832,6 +1832,51 @@ public class Foo {
</example>
</rule>
<rule name="UseStandardCharsets"
language="java"
since="6.34.0"
minimumLanguageVersion="1.7"
message="Please use StandardCharsets constants"
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#usestandardcharsets">
<description>
Starting with Java 7, StandardCharsets provides constants for common Charset objects, such as UTF-8.
Using the constants is less error prone, and can provide a small performance advantage compared to `Charset.forName(...)`
since no scan across the internal `Charset` caches is needed.
</description>
<priority>3</priority>
<properties>
<property name="version" value="2.0"/>
<property name="xpath">
<value>
<![CDATA[
//PrimaryExpression[PrimaryPrefix/Name/@Image = 'Charset.forName']
/PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix
/Literal[@Image = ('"US-ASCII"', '"ISO-8859-1"', '"UTF-8"', '"UTF-16BE"', '"UTF-16LE"', '"UTF-16"')]
]]>
</value>
</property>
</properties>
<example>
<![CDATA[
public class UseStandardCharsets {
public void run() {
// looking up the charset dynamically
try (OutputStreamWriter osw = new OutputStreamWriter(out, Charset.forName("UTF-8"))) {
osw.write("test");
}
// best to use StandardCharsets
try (OutputStreamWriter osw = new OutputStreamWriter(out, StandardCharsets.UTF_8)) {
osw.write("test");
}
}
}
]]>
</example>
</rule>
<rule name="UseTryWithResources"
language="java"
minimumLanguageVersion="1.7"

1
pmd-java/src/main/resources/rulesets/java/quickstart.xml
View File

@ -53,6 +53,7 @@
<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/UseStandardCharsets" />
<!-- <rule ref="category/java/bestpractices.xml/UseTryWithResources" /> -->
<!-- <rule ref="category/java/bestpractices.xml/UseVarargs" /> -->
<!-- <rule ref="category/java/bestpractices.xml/WhileLoopWithLiteralBoolean" /> -->

11
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseStandardCharsetsTest.java Normal file
View File

@ -0,0 +1,11 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.bestpractices;
import net.sourceforge.pmd.testframework.PmdRuleTst;
public class UseStandardCharsetsTest extends PmdRuleTst {
// no additional unit tests
}

23
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/closeresource/FakeContext.java Normal file
View File

@ -0,0 +1,23 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.errorprone.closeresource;
import java.io.Closeable;
import java.io.IOException;
/**
* Helper class for #2757
*/
public class FakeContext implements Closeable, AutoCloseable {
public <T> T getBean(Class<T> klass) {
return null;
}
@Override
public void close() throws IOException {
}
}

158
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseStandardCharsets.xml Normal file
View File

@ -0,0 +1,158 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data
xmlns="http://pmd.sourceforge.net/rule-tests"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
<test-code>
<description>fail, US-ASCII</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
import java.io.OutputStreamWriter;
import java.nio.charset.Charset;
public class Foo {
public static void charset() {
// looking up the charset dynamically
try (OutputStreamWriter osw = new OutputStreamWriter(out, Charset.forName("US-ASCII"))) {
osw.write("test");
}
}
}
]]></code>
</test-code>
<test-code>
<description>fail, ISO-8859-1</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
import java.io.OutputStreamWriter;
import java.nio.charset.Charset;
public class Foo {
public static void charset() {
// looking up the charset dynamically
try (OutputStreamWriter osw = new OutputStreamWriter(out, Charset.forName("ISO-8859-1"))) {
osw.write("test");
}
}
}
]]></code>
</test-code>
<test-code>
<description>fail, UTF-8</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
import java.io.OutputStreamWriter;
import java.nio.charset.Charset;
public class Foo {
public static void charset() {
// looking up the charset dynamically
try (OutputStreamWriter osw = new OutputStreamWriter(out, Charset.forName("UTF-8"))) {
osw.write("test");
}
}
}
]]></code>
</test-code>
<test-code>
<description>fail, UTF-16BE</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
import java.io.OutputStreamWriter;
import java.nio.charset.Charset;
public class Foo {
public static void charset() {
// looking up the charset dynamically
try (OutputStreamWriter osw = new OutputStreamWriter(out, Charset.forName("UTF-16BE"))) {
osw.write("test");
}
}
}
]]></code>
</test-code>
<test-code>
<description>fail, UTF-16LE</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
import java.io.OutputStreamWriter;
import java.nio.charset.Charset;
public class Foo {
public static void charset() {
// looking up the charset dynamically
try (OutputStreamWriter osw = new OutputStreamWriter(out, Charset.forName("UTF-16LE"))) {
osw.write("test");
}
}
}
]]></code>
</test-code>
<test-code>
<description>fail, UTF-16</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
import java.io.OutputStreamWriter;
import java.nio.charset.Charset;
public class Foo {
public static void charset() {
// looking up the charset dynamically
try (OutputStreamWriter osw = new OutputStreamWriter(out, Charset.forName("UTF-16"))) {
osw.write("test");
}
}
}
]]></code>
</test-code>
<test-code>
<description>pass, ISO-8859-2, no constant for it</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.io.OutputStreamWriter;
import java.nio.charset.Charset;
public class Foo {
public static void charset() {
// looking up the charset dynamically
try (OutputStreamWriter osw = new OutputStreamWriter(out, Charset.forName("ISO-8859-2"))) {
osw.write("test");
}
}
}
]]></code>
</test-code>
<test-code>
<description>pass, UTF-8 from StandardCharsets</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.io.OutputStreamWriter;
import java.nio.charset.StandardCharsets;
public class Foo {
public static void charset() {
// looking up the charset dynamically
try (OutputStreamWriter osw = new OutputStreamWriter(out, StandardCharsets.UTF_8)) {
osw.write("test");
}
}
}
]]></code>
</test-code>
</test-data>

21
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloseResource.xml
View File

@ -1659,4 +1659,25 @@ public class FalsePositive {
}
]]></code>
</test-code>
<test-code>
<description>#2757 support @lombok.Cleanup annotation</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
package net.sourceforge.pmd.lang.java.rule.errorprone.closeresource;
import lombok.Cleanup;
import lombok.val;
public class Mwe2757 {
private static SomeClass getCreator() {
@Cleanup val context = new FakeContext(); // FakeContext is is the package
return context.getBean(SomeClass.class);
}
public Mwe2757() {
}
static class SomeClass {}
}
]]></code>
</test-code>
</test-data>