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

Merge branch 'feature-1496'

Browse Source
This commit is contained in:
Andreas Dangel
2017-02-13 21:13:56 +01:00
parent e864ad2740 983a0dc9b8
commit 4e04e62374
6 changed files with 326 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
pmd-core/src/main/resources/rulesets/releases/554.xml Normal file
View File

@ -0,0 +1,14 @@
<?xml version="1.0"?>
<ruleset name="554"
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 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
<description>
This ruleset contains links to rules that are new in PMD v5.5.4
</description>
<rule ref="rulesets/java/design.xml/AccessorMethodGeneration"/>
</ruleset>

74
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/AccessorMethodGenerationRule.java Normal file
View File

@ -0,0 +1,74 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.design;
import java.util.List;
import java.util.Map;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
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.AbstractJavaAccessNode;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.symboltable.AbstractJavaScope;
import net.sourceforge.pmd.lang.java.symboltable.ClassNameDeclaration;
import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
import net.sourceforge.pmd.lang.java.symboltable.MethodNameDeclaration;
import net.sourceforge.pmd.lang.java.symboltable.SourceFileScope;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
public class AccessorMethodGenerationRule extends AbstractJavaRule {
public Object visit(final ASTCompilationUnit node, final Object data) {
final SourceFileScope file = node.getScope().getEnclosingScope(SourceFileScope.class);
analyzeScope(file, data);
return data; // Stop tree navigation
}
private void analyzeScope(final AbstractJavaScope file, final Object data) {
for (final ClassNameDeclaration classDecl : file.getDeclarations(ClassNameDeclaration.class).keySet()) {
final ClassScope classScope = (ClassScope) classDecl.getScope();
// Check fields
for (final Map.Entry<VariableNameDeclaration, List<NameOccurrence>> varDecl : classScope.getVariableDeclarations().entrySet()) {
final ASTFieldDeclaration field = varDecl.getKey().getNode().getFirstParentOfType(ASTFieldDeclaration.class);
analyzeMember(field, varDecl.getValue(), classScope, data);
}
// Check methods
for (final Map.Entry<MethodNameDeclaration, List<NameOccurrence>> methodDecl : classScope.getMethodDeclarations().entrySet()) {
final ASTMethodDeclaration method = methodDecl.getKey().getNode().getFirstParentOfType(ASTMethodDeclaration.class);
analyzeMember(method, methodDecl.getValue(), classScope, data);
}
// Check inner classes
analyzeScope(classScope, data);
}
}
public void analyzeMember(final AbstractJavaAccessNode node, final List<NameOccurrence> occurrences,
final ClassScope classScope, final Object data) {
if (!node.isPrivate()) {
return;
}
for (final NameOccurrence no : occurrences) {
Node n = no.getLocation();
while (n != null && !(n instanceof ASTClassOrInterfaceDeclaration) && !(n instanceof ASTEnumDeclaration)) {
n = n.jjtGetParent();
}
// Are we within the same class that defines the field / method?
if (!n.getImage().equals(classScope.getClassName())) {
addViolation(data, no.getLocation());
}
}
}
}

32
pmd-java/src/main/resources/rulesets/java/design.xml
View File

@ -1967,6 +1967,38 @@ public interface YetAnotherConstantInterface {
public static final int CONST1 = 1; // no violation
int anyMethod();
}
]]>
</example>
</rule>
<rule name="AccessorMethodGeneration"
language="java"
since="5.5.4"
message="Avoid autogenerated methods to access private fields and methods of inner / outer classes"
class="net.sourceforge.pmd.lang.java.rule.design.AccessorMethodGenerationRule"
externalInfoUrl="${pmd.website.baseurl}/rules/java/design.html#AccessorMethodGeneration">
<description>
When accessing a private field / method from another class, the Java compiler will generate a accessor methods
with package-private visibility. This adds overhead, and to the dex method count on Android. This situation can
be avoided by changing the visibility of the field / method from private to package-private.
</description>
<priority>3</priority>
<example>
<![CDATA[
public class OuterClass {
private int counter;
/* package */ int id;
public class InnerClass {
InnerClass() {
OuterClass.this.counter++; // wrong accessor method will be generated
}
public int getOuterClassId() {
return OuterClass.this.id; // id is package-private, no accessor method needed
}
}
}
]]>
</example>

1
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/DesignRulesTest.java
View File

@ -18,6 +18,7 @@ public class DesignRulesTest extends SimpleAggregatorTst {
addRule(RULESET, "AbstractClassWithoutAbstractMethod");
addRule(RULESET, "AbstractClassWithoutAnyMethod");
addRule(RULESET, "AccessorClassGeneration");
addRule(RULESET, "AccessorMethodGeneration");
addRule(RULESET, "AssignmentToNonFinalStatic");
addRule(RULESET, "AvoidDeeplyNestedIfStmts");
addRule(RULESET, "AvoidInstanceofChecksInCatchClause");

170
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/AccessorMethodGeneration.xml Normal file
View File

@ -0,0 +1,170 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data>
<test-code>
<description><![CDATA[
inner class accesses private field from outer class
]]></description>
<expected-problems>1</expected-problems>
<expected-linenumbers>8</expected-linenumbers>
<code><![CDATA[
public class Foo {
private int field;
public class InnerClass {
private long innerField;
InnerClass() {
innerField = Foo.this.field; // violation, accesing a private field
}
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
inner class accesses private field from outer class unqualified
]]></description>
<expected-problems>1</expected-problems>
<expected-linenumbers>8</expected-linenumbers>
<code><![CDATA[
public class Foo {
private int field;
public class InnerClass {
private long innerField;
InnerClass() {
innerField = field; // violation, accesing a private field
}
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
outer class accesses private field from inner class
]]></description>
<expected-problems>1</expected-problems>
<expected-linenumbers>9</expected-linenumbers>
<code><![CDATA[
public class Foo {
private int field;
public class InnerClass {
private long innerField;
}
long method() {
return new InnerClass().innerField; // violation, accesing a private field
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
non private fields are ok
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public int publicField;
protected int protectedField;
/* package */ int packageField;
public class InnerClass {
private long innerField;
InnerClass() {
innerField = Foo.this.publicField; // this is ok
innerField += Foo.this.protectedField; // this is ok
innerField += Foo.this.packageField; // this is ok
}
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
inner class accesses private method of outer class, unqualified
]]></description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code><![CDATA[
public class Foo {
public class InnerClass {
private void secret() {
outerSecret(); // violation, accesing a private method
}
}
private void outerSecret() {
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
inner class accesses private method of outer class, qualified
]]></description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code><![CDATA[
public class Foo {
public class InnerClass {
private void secret() {
Foo.this.outerSecret(); // violation, accesing a private method
}
}
private void outerSecret() {
}
}
]]></code>
</test-code>
<test-code regressionTest="false">
<description><![CDATA[
IGNORED - outer class accesses private method of inner class
]]></description>
<expected-problems>1</expected-problems>
<expected-linenumbers>9</expected-linenumbers>
<code><![CDATA[
public class Foo {
public class InnerClass {
private void secret() {
}
}
private void outerSecret() {
// FIXME : This is not detected right now. Name searches are not taking qualifiers into account...
new InnerClass().secret(); // violation, accesing a private method
}
}
]]></code>
</test-code>
<test-code>
<description><![CDATA[
inner class accesses non-private methods of outer class
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public class InnerClass {
private void secret() {
outerPublic(); // this is ok
outerProtected(); // this is ok
outerPackage(); // this is ok
}
}
public void outerPublic() {
}
protected void outerProtected() {
}
/* package */ void outerPackage() {
}
}
]]></code>
</test-code>
</test-data>

38
src/site/markdown/overview/changelog.md
View File

@ -28,9 +28,10 @@ making it over 500X faster, and `PreserveStackTrace` which is now 7X faster.
### Table Of Contents
* [New and noteworthy](#New_and_noteworthy)
* [Incremental Analysis](#Incremental_Analysis)
* [Apex Security Rule Set](#Apex_Security_Rule_Set)
* [Modified Rules](#Modified_Rules)
* [Incremental Analysis](#Incremental_Analysis)
* [Apex Security Rule Set](#Apex_Security_Rule_Set)
* [New Rules](#New_Rules)
* [Modified Rules](#Modified_Rules)
* [Fixed Issues](#Fixed_Issues)
* [API Changes](#API_Changes)
* [External Contributions](#External_Contributions)
@ -214,6 +215,36 @@ Makes sure that all values obtained from URL parameters are properly escaped / s
to avoid XSS attacks.
#### New Rules
##### AccessorMethodGeneration (java-design)
When accessing a private field / method from another class, the Java compiler will generate a accessor methods
with package-private visibility. This adds overhead, and to the dex method count on Android. This situation can
be avoided by changing the visibility of the field / method from private to package-private.
For instance, it would report violations on code such as:
```
public class OuterClass {
private int counter;
/* package */ int id;
public class InnerClass {
InnerClass() {
OuterClass.this.counter++; // wrong, accessor method will be generated
}
public int getOuterClassId() {
return OuterClass.this.id; // id is package-private, no accessor method needed
}
}
}
```
This new rule is part of the `java-design` ruleset.
#### Modified Rules
* The Java rule "UseLocaleWithCaseConversions" (ruleset java-design) has been modified, to detect calls
@ -247,6 +278,7 @@ to avoid XSS attacks.
* java-design
* [#1448](https://sourceforge.net/p/pmd/bugs/1448/): \[java] ImmutableField: Private field in inner class gives false positive with lambdas
* [#1495](https://sourceforge.net/p/pmd/bugs/1495/): \[java] UnnecessaryLocalBeforeReturn with assert
* [#1496](https://sourceforge.net/p/pmd/bugs/1496/): \[java] New Rule: AccesorMethodGeneration - complements accessor class rule
* [#1512](https://sourceforge.net/p/pmd/bugs/1512/): \[java] Combine rules AvoidConstantsInInterface and ConstantsInInterface
* [#1552](https://sourceforge.net/p/pmd/bugs/1552/): \[java] MissingBreakInSwitch - False positive for continue
* [#1556](https://sourceforge.net/p/pmd/bugs/1556/): \[java] UseLocaleWithCaseConversions does not works with `ResultSet` (false negative)