hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

Make TCC consider local classes.

Browse Source 
If a local class is declared inside a method of the outer class,
then attribute accesses made by the local class count as if they
are made by the enclosing method.
This commit is contained in:
Clément Fournier
2018-01-16 23:42:29 +01:00
parent 8cfc073b8c
commit 2ca4b347c3
10 changed files with 229 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -95,6 +95,12 @@ public final class JavaQualifiedName implements QualifiedName {
localIndices[0] = NOTLOCAL_PLACEHOLDER;
}
/* default, test only */ static void resetLocalIndicesCounter() {
LOCAL_INDICES.clear();
}
/**
* Builds the qualified name of a method declaration.
*

5
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/TccMetric.java
View File

@ -11,7 +11,7 @@ import java.util.Map;
import java.util.Set;
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
import net.sourceforge.pmd.lang.java.metrics.impl.visitors.TccMethodPairVisitor;
import net.sourceforge.pmd.lang.java.metrics.impl.visitors.TccAttributeAccessCollector;
import net.sourceforge.pmd.lang.metrics.MetricOptions;
/**
@ -25,8 +25,7 @@ public class TccMetric extends AbstractJavaClassMetric {
@Override
public double computeFor(ASTAnyTypeDeclaration node, MetricOptions options) {
@SuppressWarnings("unchecked")
Map<String, Set<String>> usagesByMethod = (Map<String, Set<String>>) node.jjtAccept(new TccMethodPairVisitor(), null);
Map<String, Set<String>> usagesByMethod = new TccAttributeAccessCollector(node).start();
int numPairs = numMethodsRelatedByAttributeAccess(usagesByMethod);
int maxPairs = maxMethodPairs(usagesByMethod.size());

86
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/TccMethodPairVisitor.java → pmd-java/src/main/java/net/sourceforge/pmd/lang/java/metrics/impl/visitors/TccAttributeAccessCollector.java
View File

@ -6,44 +6,65 @@ package net.sourceforge.pmd.lang.java.metrics.impl.visitors;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.Stack;
import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
import net.sourceforge.pmd.lang.java.ast.JavaParserVisitorReducedAdapter;
import net.sourceforge.pmd.lang.java.symboltable.ClassScope;
import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration;
import net.sourceforge.pmd.lang.symboltable.Scope;
/**
* Returns the map of method names to the set local attributes accessed when visiting a class.
* Returns the map of method names to the set of local attributes accessed when visiting a class.
*
* @author Clément Fournier
* @since 6.0.0
*/
public class TccMethodPairVisitor extends JavaParserVisitorReducedAdapter {
public class TccAttributeAccessCollector extends JavaParserVisitorReducedAdapter {
private final ASTAnyTypeDeclaration exploredClass;
/**
* Collects for each method of the current class, which local attributes are accessed.
*/
Stack<Map<String, Set<String>>> methodAttributeAccess = new Stack<>();
/** The name of the current method. */
private String currentMethodName;
private Map<String, Set<String>> methodAttributeAccess;
public TccAttributeAccessCollector(ASTAnyTypeDeclaration exploredClass) {
this.exploredClass = exploredClass;
}
/**
* Collects the attribute accesses by method into a map.
*/
@SuppressWarnings("unchecked")
public Map<String, Set<String>> start() {
return (Map<String, Set<String>>) this.visit(exploredClass, new HashMap<String, Set<String>>());
}
@Override
public Object visit(ASTAnyTypeDeclaration node, Object data) {
methodAttributeAccess.push(new HashMap<String, Set<String>>());
super.visit(node, data);
methodAttributeAccess.peek().remove(null);
return methodAttributeAccess.pop();
if (node == exploredClass) {
methodAttributeAccess = new HashMap<>();
super.visit(node, data);
} else if (node instanceof ASTClassOrInterfaceDeclaration
&& ((ASTClassOrInterfaceDeclaration) node).isLocal()) {
super.visit(node, data);
}
return methodAttributeAccess;
}
@ -51,26 +72,37 @@ public class TccMethodPairVisitor extends JavaParserVisitorReducedAdapter {
public Object visit(ASTMethodDeclaration node, Object data) {
if (!node.isAbstract()) {
currentMethodName = node.getQualifiedName().getOperation();
methodAttributeAccess.peek().put(currentMethodName, new HashSet<String>());
if (node.getFirstParentOfType(ASTAnyTypeDeclaration.class) == exploredClass) {
currentMethodName = node.getQualifiedName().getOperation();
methodAttributeAccess.put(currentMethodName, new HashSet<String>());
super.visit(node, data);
currentMethodName = null;
super.visit(node, data);
currentMethodName = null;
} else {
super.visit(node, data);
}
}
return null;
}
@Override
public Object visit(ASTConstructorDeclaration node, Object data) {
return data; // we're only looking for method pairs
}
/**
* The primary expression node is used to detect access to attributes and method calls. If the access is not for a
* foreign class, then the {@link #methodAttributeAccess} map is updated for the current method.
* The primary expression node is used to detect access
* to attributes and method calls. If the access is not for a
* foreign class, then the {@link #methodAttributeAccess}
* map is updated for the current method.
*/
@Override
public Object visit(ASTPrimaryExpression node, Object data) {
if (currentMethodName != null) {
Set<String> methodAccess = methodAttributeAccess.peek().get(currentMethodName);
Set<String> methodAccess = methodAttributeAccess.get(currentMethodName);
String variableName = getVariableName(node);
if (isLocalAttributeAccess(variableName, node.getScope())) {
methodAccess.add(variableName);
@ -81,9 +113,18 @@ public class TccMethodPairVisitor extends JavaParserVisitorReducedAdapter {
return super.visit(node, data);
}
private String getVariableName(ASTPrimaryExpression node) {
ASTPrimaryPrefix prefix = node.getFirstDescendantOfType(ASTPrimaryPrefix.class);
if (prefix.usesThisModifier()) {
List<ASTPrimarySuffix> suffixes = node.findChildrenOfType(ASTPrimarySuffix.class);
if (suffixes.size() > 1) {
if (!suffixes.get(1).isArguments()) { // not a method call
return suffixes.get(0).getImage();
}
}
}
ASTName name = prefix.getFirstDescendantOfType(ASTName.class);
String variableName = null;
@ -107,7 +148,8 @@ public class TccMethodPairVisitor extends JavaParserVisitorReducedAdapter {
while (currentScope != null) {
for (VariableNameDeclaration decl : currentScope.getDeclarations(VariableNameDeclaration.class).keySet()) {
if (decl.getImage().equals(varName)) {
if (currentScope instanceof ClassScope) {
if (currentScope instanceof ClassScope
&& ((ClassScope) currentScope).getClassDeclaration().getNode() == exploredClass) {
return true;
}
}

5
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/QualifiedNameTest.java
View File

@ -25,6 +25,11 @@ import net.sourceforge.pmd.lang.java.ParserTstUtil;
*/
public class QualifiedNameTest {
/** Provides a hook into the package-private reset method for the local indices counter. */
public static void resetLocalIndicesCounterHook() {
JavaQualifiedName.resetLocalIndicesCounter();
}
@Test
public void testEmptyPackage() {

4
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/AbstractMetricTestRule.java
View File

@ -132,7 +132,7 @@ public abstract class AbstractMetricTestRule extends AbstractJavaMetricsRule {
if (val == (int) val) {
return String.valueOf((int) val);
} else {
return String.format(Locale.ROOT, "%." + 4 + "f", val);
return String.format(Locale.ROOT, "%.4f", val);
}
}
@ -165,6 +165,6 @@ public abstract class AbstractMetricTestRule extends AbstractJavaMetricsRule {
"" + niceDoubleString(methodValue), });
}
}
return data;
return super.visit(node, data);
}
}

2
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/impl/AllMetricsTest.java
View File

@ -5,6 +5,7 @@
package net.sourceforge.pmd.lang.java.metrics.impl;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.lang.java.ast.QualifiedNameTest;
import net.sourceforge.pmd.lang.java.metrics.MetricsHook;
import net.sourceforge.pmd.testframework.SimpleAggregatorTst;
@ -22,6 +23,7 @@ public class AllMetricsTest extends SimpleAggregatorTst {
@Override
protected Rule reinitializeRule(Rule rule) {
MetricsHook.reset();
QualifiedNameTest.resetLocalIndicesCounterHook();
return rule;
}

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

@ -5,6 +5,7 @@
package net.sourceforge.pmd.lang.java.rule.design;
import net.sourceforge.pmd.Rule;
import net.sourceforge.pmd.lang.java.ast.QualifiedNameTest;
import net.sourceforge.pmd.lang.java.metrics.MetricsHook;
import net.sourceforge.pmd.testframework.SimpleAggregatorTst;
@ -18,6 +19,7 @@ public class DesignRulesTest extends SimpleAggregatorTst {
@Override
protected Rule reinitializeRule(Rule rule) {
MetricsHook.reset();
QualifiedNameTest.resetLocalIndicesCounterHook();
return rule;
}

105
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/metrics/impl/xml/TccTest.xml
View File

@ -26,11 +26,11 @@ this(name, valueType, initialValue, null);
public Property(String name, Class valueType, Object initialValue, Object[] values) {
_name = name;
_valueType = valueType;
_initialValue = initialValue;
_availableValues = values;
_currentValue = _initialValue;
_name = name;
_valueType = valueType;
_initialValue = initialValue;
_availableValues = values;
_currentValue = _initialValue;
}
public String getName() {
@ -97,4 +97,99 @@ return _name.compareTo(((Property) o)._name);
]]></code>
</test-code>
<test-code>
<description>Do not crash on local class, refs #827</description>
<expected-problems>1</expected-problems>
<expected-messages>
<message>'com.pack.Pack$1Inner' has value 0.</message>
</expected-messages>
<code><![CDATA[
package com.pack;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class Pack implements IPack {
@Override
public Map<String, String> get() {
class Inner implements IInner {
private Map<String, String> results;
public Inner(Map<String, String> results) {
this.results = results;
}
public void method() {
this.results = new HashMap<String, String>();
}
private void otherMethod() {
this.results.clear();
}
}
return null;
}
}
]]></code>
</test-code>
<test-code>
<description>Attribute accesses in local class count as accesses of the method</description>
<expected-problems>2</expected-problems>
<expected-messages>
<message>'com.pack.Pack' has value 1.</message>
<message>'com.pack.Pack$1Inner' has value 0.</message>
</expected-messages>
<code><![CDATA[
package com.pack;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class Pack implements IPack {
// the attribute is in the outer class
private Map<String, String> results;
public void paired() {
results.clear();
}
@Override
public Map<String, String> get() {
class Inner implements IInner {
public Inner(Map<String, String> results) {
this.results = results;
}
public void method() {
this.results = new HashMap<String, String>();
}
private void otherMethod() {
this.results.clear();
}
}
return null;
}
}
]]></code>
</test-code>
</test-data>

53
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/GodClass.xml
View File

@ -133,15 +133,52 @@ public class Foo {
<description>#1085 NullPointerException by at net.sourceforge.pmd.lang.java.rule.design.GodClassRule.visit(GodClassRule.java:313)</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public enum Color {
BLUE,
RED;
public int toHex() {
return 0;
}
}
public enum Color {
BLUE,
RED;
public int toHex() {
return 0;
}
}
]]>
</code>
</test-code>
<test-code>
<description>GodClass crashes with java.lang.NullPointerException, refs #827</description>
<expected-problems>0</expected-problems>
<code>
<![CDATA[
package com.pack;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class Pack implements IPack {
@Override
public Map<String, String> get() {
class Inner implements IInner {
private Map<String, String> results;
public Inner(Map<String, String> results) {
this.results = results;
}
}
return null;
}
}
]]>
</code>
</test-code>
</test-data>

2
pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java
View File

@ -163,7 +163,7 @@ public abstract class RuleTst {
*
* @param rule The rule to reinitialise
*
* @return The rule once it has be reinitialised
* @return The rule once it has been reinitialised
*/
protected Rule reinitializeRule(Rule rule) {
return findRule(rule.getRuleSetName(), rule.getName());