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

pmd: fix #1115 commentRequiredRule in pmd 5.1 is not working properly

Browse Source
This commit is contained in:
Andreas Dangel committed 2013-08-03 15:47:38 +02:00
1 parent 9255e26c70
commit 98e27294f0
4 files changed
+130 -49
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

pmd/etc/changelog.txt
+1
View File
@@ -1,6 +1,7 @@
????? ??, 2013 - 5.1.0:
Fixed bug 1059: Change rule name "Use Singleton" should be "Use Utility class"
Fixed bug 1115: commentRequiredRule in pmd 5.1 is not working properly
New EcmaScript rules and ruleset:
Controversial ruleset, featuring
pmd/src/main/java/net/sourceforge/pmd/lang/java/rule/comments/AbstractCommentRule.java
+76 -48
View File
@@ -3,19 +3,21 @@
*/
package net.sourceforge.pmd.lang.java.rule.comments;
import java.util.List;
import java.util.Map.Entry;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map.Entry;
import java.util.SortedMap;
import java.util.TreeMap;
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.ASTConstructorDeclaration;
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.ast.AbstractJavaAccessTypeNode;
import net.sourceforge.pmd.lang.java.ast.Comment;
import net.sourceforge.pmd.lang.java.ast.FormalComment;
import net.sourceforge.pmd.lang.java.ast.MultiLineComment;
@@ -37,7 +39,7 @@ public abstract class AbstractCommentRule extends AbstractJavaRule {
int atPos = comments.indexOf('@');
if (atPos < 0)
return Collections.EMPTY_LIST;
return Collections.emptyList();
List<Integer> ints = new ArrayList<Integer>();
ints.add(atPos);
@@ -146,63 +148,89 @@ public abstract class AbstractCommentRule extends AbstractJavaRule {
}
protected void assignCommentsToDeclarations(ASTCompilationUnit cUnit) {
SortedMap<Integer, Object> itemsByLineNumber = orderedCommentsAndDeclarations(cUnit);
SortedMap<Integer, Node> itemsByLineNumber = orderedCommentsAndDeclarations(cUnit);
FormalComment lastComment = null;
AbstractJavaAccessNode lastNode = null;
for (Entry<Integer, Node> entry : itemsByLineNumber.entrySet()) {
Node value = entry.getValue();
for (Entry<Integer, Object> entry : itemsByLineNumber.entrySet()) {
Object value = entry.getValue();
if (lastComment == null) {
if (value instanceof FormalComment) {
lastComment = (FormalComment) value;
} else {
// else this is declaration without comment
if (!(value instanceof AbstractJavaAccessTypeNode)) {
lastNode = (AbstractJavaAccessNode) value;
}
}
// else this is declaration without comment
} else if (value instanceof AbstractJavaAccessNode) {
AbstractJavaAccessNode node = (AbstractJavaAccessNode) value;
node.comment(lastComment);
lastComment = null;
// maybe the last comment is within the last node
if (isCommentNotWithin(lastComment, lastNode) && isCommentBefore(lastComment, node)) {
node.comment(lastComment);
lastComment = null;
}
if (!(node instanceof AbstractJavaAccessTypeNode)) {
lastNode = node;
}
}
}
}
/**
*
* @since
* @param cUnit
* @return bla
*/
protected SortedMap<Integer, Object> orderedCommentsAndDeclarations(
ASTCompilationUnit cUnit) {
private boolean isCommentNotWithin(FormalComment n1, Node n2) {
if (n1 == null || n2 == null) {
return true;
}
if ((n1.getEndLine() < n2.getEndLine())
|| (n1.getEndLine() == n2.getEndLine() && n1.getEndColumn() < n2.getEndColumn())) {
return false;
} else {
return true;
}
}
SortedMap<Integer, Object> itemsByLineNumber = new TreeMap<Integer, Object>();
List<ASTClassOrInterfaceDeclaration> packageDecl = cUnit
.findDescendantsOfType(ASTClassOrInterfaceDeclaration.class);
for (ASTClassOrInterfaceDeclaration decl : packageDecl) {
itemsByLineNumber.put(decl.getBeginLine(), decl);
}
for (Comment comment : cUnit.getComments()) {
itemsByLineNumber.put(comment.getBeginLine(), comment);
}
List<ASTFieldDeclaration> fields = cUnit
.findDescendantsOfType(ASTFieldDeclaration.class);
for (ASTFieldDeclaration fieldDecl : fields) {
itemsByLineNumber.put(fieldDecl.getBeginLine(), fieldDecl);
}
List<ASTMethodDeclaration> methods = cUnit
.findDescendantsOfType(ASTMethodDeclaration.class);
for (ASTMethodDeclaration methodDecl : methods) {
itemsByLineNumber.put(methodDecl.getBeginLine(), methodDecl);
}
List<ASTConstructorDeclaration> constructors = cUnit
.findDescendantsOfType(ASTConstructorDeclaration.class);
for (ASTConstructorDeclaration constructorDecl : constructors) {
itemsByLineNumber.put(constructorDecl.getBeginLine(), constructorDecl);
}
return itemsByLineNumber;
private boolean isCommentBefore(FormalComment n1, Node n2) {
if ((n1.getEndLine() < n2.getBeginLine())
|| (n1.getEndLine() == n2.getBeginLine() && n1.getEndColumn() < n2.getBeginColumn())) {
return true;
} else {
return false;
}
}
/**
*
* @since
* @param cUnit
* @return bla
*/
protected SortedMap<Integer, Node> orderedCommentsAndDeclarations(ASTCompilationUnit cUnit) {
SortedMap<Integer, Node> itemsByLineNumber = new TreeMap<Integer, Node>();
List<ASTClassOrInterfaceDeclaration> packageDecl = cUnit
.findDescendantsOfType(ASTClassOrInterfaceDeclaration.class);
addDeclarations(itemsByLineNumber, packageDecl);
addDeclarations(itemsByLineNumber, cUnit.getComments());
List<ASTFieldDeclaration> fields = cUnit.findDescendantsOfType(ASTFieldDeclaration.class);
addDeclarations(itemsByLineNumber, fields);
List<ASTMethodDeclaration> methods = cUnit.findDescendantsOfType(ASTMethodDeclaration.class);
addDeclarations(itemsByLineNumber, methods);
List<ASTConstructorDeclaration> constructors = cUnit.findDescendantsOfType(ASTConstructorDeclaration.class);
addDeclarations(itemsByLineNumber, constructors);
return itemsByLineNumber;
}
private void addDeclarations(SortedMap<Integer, Node> map, List<? extends Node> nodes) {
for (Node node : nodes) {
map.put((node.getBeginLine() << 16) + node.getBeginColumn(), node);
}
}
}
pmd/src/test/java/net/sourceforge/pmd/testframework/RuleTst.java
+27 -1
View File
@@ -9,6 +9,7 @@ import static org.junit.Assert.fail;
import java.io.IOException;
import java.io.InputStream;
import java.io.StringReader;
import java.io.StringWriter;
import java.util.Map;
import java.util.Properties;
@@ -29,6 +30,7 @@ import net.sourceforge.pmd.RuleSetNotFoundException;
import net.sourceforge.pmd.RuleSets;
import net.sourceforge.pmd.lang.Language;
import net.sourceforge.pmd.lang.LanguageVersion;
import net.sourceforge.pmd.renderers.TextRenderer;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
@@ -75,6 +77,7 @@ public abstract class RuleTst {
Map<PropertyDescriptor<?>, Object> oldProperties = rule.getPropertiesByPropertyDescriptor();
try {
int res;
Report report;
try {
// Set test specific properties onto the Rule
if (test.getProperties() != null) {
@@ -90,11 +93,13 @@ public abstract class RuleTst {
}
}
res = processUsingStringReader(test.getCode(), rule, test.getLanguageVersion()).size();
report = processUsingStringReader(test.getCode(), rule, test.getLanguageVersion());
res = report.size();
} catch (Throwable t) {
t.printStackTrace();
throw new RuntimeException('"' + test.getDescription() + "\" failed", t);
}
printReport(test, report);
assertEquals('"' + test.getDescription() + "\" resulted in wrong number of failures,",
test.getNumberOfProblemsExpected(), res);
} finally {
@@ -107,6 +112,27 @@ public abstract class RuleTst {
}
}
private void printReport(TestDescriptor test, Report report) {
if (test.getNumberOfProblemsExpected() != report.size()) {
System.out.println("--------------------------------------------------------------");
System.out.println("Test Failure: " + test.getDescription());
System.out.println(" -> Expected " + test.getNumberOfProblemsExpected() + " problem(s), but "
+ report.size() + " problem(s) found.");
System.out.println();
TextRenderer renderer = new TextRenderer();
renderer.setWriter(new StringWriter());
try {
renderer.start();
renderer.renderFileReport(report);
renderer.end();
} catch (IOException e) {
throw new RuntimeException(e);
}
System.out.println(renderer.getWriter().toString());
System.out.println("--------------------------------------------------------------");
}
}
private Report processUsingStringReader(String code, Rule rule,
LanguageVersion languageVersion) throws PMDException {
Report report = new Report();
pmd/src/test/resources/net/sourceforge/pmd/lang/java/rule/comments/xml/CommentRequired.xml
+26
View File
@@ -61,4 +61,30 @@ public class Foo {
<rule-property name="protectedMethodCommentRequirement">Unwanted</rule-property>
<code-ref id="with-all-comments"/>
</test-code>
<test-code>
<description>#1115 commentRequiredRule in pmd 5.1 is not working properly</description>
<expected-problems>2</expected-problems>
<code><![CDATA[
/**
* Test class
*/
public class Test {
public void method1() {
/**
* comment here
*/
}
public void method2() {
}
}
]]></code>
</test-code>
<test-code>
<description>#1115 commentRequiredRule in pmd 5.1 is not working properly - without new lines</description>
<expected-problems>2</expected-problems>
<code><![CDATA[
/** Test class */ public class Test { public void method1() { /** comment here */ } public void method2() { } }
]]></code>
</test-code>
</test-data>