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

Several improvements after final review

Browse Source
This commit is contained in:
Juan Martín Sotuyo Dodero
2018-09-23 00:55:04 -03:00
parent 86dc44de3e
commit 29c5d5b49b
6 changed files with 79 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
docs/pages/pmd/rules/apex.md
View File

@ -113,7 +113,7 @@ folder: pmd/rules
It contains the following rules:
[ApexBadCrypto](pmd_rules_apex_security.html#apexbadcrypto), [ApexCRUDViolation](pmd_rules_apex_security.html#apexcrudviolation), [ApexCSRF](pmd_rules_apex_security.html#apexcsrf), [ApexDangerousMethods](pmd_rules_apex_security.html#apexdangerousmethods), [ApexInsecureEndpoint](pmd_rules_apex_security.html#apexinsecureendpoint), [ApexOpenRedirect](pmd_rules_apex_security.html#apexopenredirect), [ApexSharingViolations](pmd_rules_apex_security.html#apexsharingviolations), [ApexSOQLInjection](pmd_rules_apex_security.html#apexsoqlinjection), [ApexSuggestUsingNamedCred](pmd_rules_apex_security.html#apexsuggestusingnamedcred), [ApexUnitTestClassShouldHaveAsserts](pmd_rules_apex_bestpractices.html#apexunittestclassshouldhaveasserts), [ApexUnitTestShouldNotUseSeeAllDataTrue](pmd_rules_apex_bestpractices.html#apexunittestshouldnotuseseealldatatrue), [ApexXSSFromEscapeFalse](pmd_rules_apex_security.html#apexxssfromescapefalse), [ApexXSSFromURLParam](pmd_rules_apex_security.html#apexxssfromurlparam), [AvoidDeeplyNestedIfStmts](pmd_rules_apex_design.html#avoiddeeplynestedifstmts), [AvoidDirectAccessTriggerMap](pmd_rules_apex_errorprone.html#avoiddirectaccesstriggermap), [AvoidDmlStatementsInLoops](pmd_rules_apex_performance.html#avoiddmlstatementsinloops), [AvoidGlobalModifier](pmd_rules_apex_bestpractices.html#avoidglobalmodifier), [AvoidHardcodingId](pmd_rules_apex_errorprone.html#avoidhardcodingid), [AvoidLogicInTrigger](pmd_rules_apex_bestpractices.html#avoidlogicintrigger), [AvoidNonExistentAnnotations](pmd_rules_apex_errorprone.html#avoidnonexistentannotations), [AvoidSoqlInLoops](pmd_rules_apex_performance.html#avoidsoqlinloops), [AvoidSoslInLoops](pmd_rules_apex_performance.html#avoidsoslinloops), [ClassNamingConventions](pmd_rules_apex_codestyle.html#classnamingconventions), [CyclomaticComplexity](pmd_rules_apex_design.html#cyclomaticcomplexity), [EmptyCatchBlock](pmd_rules_apex_errorprone.html#emptycatchblock), [EmptyIfStmt](pmd_rules_apex_errorprone.html#emptyifstmt), [EmptyStatementBlock](pmd_rules_apex_errorprone.html#emptystatementblock), [EmptyTryOrFinallyBlock](pmd_rules_apex_errorprone.html#emptytryorfinallyblock), [EmptyWhileStmt](pmd_rules_apex_errorprone.html#emptywhilestmt), [ExcessiveClassLength](pmd_rules_apex_design.html#excessiveclasslength), [ExcessiveParameterList](pmd_rules_apex_design.html#excessiveparameterlist), [ExcessivePublicCount](pmd_rules_apex_design.html#excessivepubliccount), [ForLoopsMustUseBraces](pmd_rules_apex_codestyle.html#forloopsmustusebraces), [IfElseStmtsMustUseBraces](pmd_rules_apex_codestyle.html#ifelsestmtsmustusebraces), [IfStmtsMustUseBraces](pmd_rules_apex_codestyle.html#ifstmtsmustusebraces), [MethodNamingConventions](pmd_rules_apex_codestyle.html#methodnamingconventions), [MethodWithSameNameAsEnclosingClass](pmd_rules_apex_errorprone.html#methodwithsamenameasenclosingclass), [NcssConstructorCount](pmd_rules_apex_design.html#ncssconstructorcount), [NcssMethodCount](pmd_rules_apex_design.html#ncssmethodcount), [NcssTypeCount](pmd_rules_apex_design.html#ncsstypecount), [OneDeclarationPerLine](pmd_rules_apex_codestyle.html#onedeclarationperline), [StdCyclomaticComplexity](pmd_rules_apex_design.html#stdcyclomaticcomplexity), [TooManyFields](pmd_rules_apex_design.html#toomanyfields), [VariableNamingConventions](pmd_rules_apex_codestyle.html#variablenamingconventions), [WhileLoopsMustUseBraces](pmd_rules_apex_codestyle.html#whileloopsmustusebraces)
[ApexBadCrypto](pmd_rules_apex_security.html#apexbadcrypto), [ApexCRUDViolation](pmd_rules_apex_security.html#apexcrudviolation), [ApexCSRF](pmd_rules_apex_security.html#apexcsrf), [ApexDangerousMethods](pmd_rules_apex_security.html#apexdangerousmethods), [ApexDoc](pmd_rules_apex_documentation.html#apexdoc), [ApexInsecureEndpoint](pmd_rules_apex_security.html#apexinsecureendpoint), [ApexOpenRedirect](pmd_rules_apex_security.html#apexopenredirect), [ApexSharingViolations](pmd_rules_apex_security.html#apexsharingviolations), [ApexSOQLInjection](pmd_rules_apex_security.html#apexsoqlinjection), [ApexSuggestUsingNamedCred](pmd_rules_apex_security.html#apexsuggestusingnamedcred), [ApexUnitTestClassShouldHaveAsserts](pmd_rules_apex_bestpractices.html#apexunittestclassshouldhaveasserts), [ApexUnitTestShouldNotUseSeeAllDataTrue](pmd_rules_apex_bestpractices.html#apexunittestshouldnotuseseealldatatrue), [ApexXSSFromEscapeFalse](pmd_rules_apex_security.html#apexxssfromescapefalse), [ApexXSSFromURLParam](pmd_rules_apex_security.html#apexxssfromurlparam), [AvoidDeeplyNestedIfStmts](pmd_rules_apex_design.html#avoiddeeplynestedifstmts), [AvoidDirectAccessTriggerMap](pmd_rules_apex_errorprone.html#avoiddirectaccesstriggermap), [AvoidDmlStatementsInLoops](pmd_rules_apex_performance.html#avoiddmlstatementsinloops), [AvoidGlobalModifier](pmd_rules_apex_bestpractices.html#avoidglobalmodifier), [AvoidHardcodingId](pmd_rules_apex_errorprone.html#avoidhardcodingid), [AvoidLogicInTrigger](pmd_rules_apex_bestpractices.html#avoidlogicintrigger), [AvoidNonExistentAnnotations](pmd_rules_apex_errorprone.html#avoidnonexistentannotations), [AvoidSoqlInLoops](pmd_rules_apex_performance.html#avoidsoqlinloops), [AvoidSoslInLoops](pmd_rules_apex_performance.html#avoidsoslinloops), [ClassNamingConventions](pmd_rules_apex_codestyle.html#classnamingconventions), [CyclomaticComplexity](pmd_rules_apex_design.html#cyclomaticcomplexity), [EmptyCatchBlock](pmd_rules_apex_errorprone.html#emptycatchblock), [EmptyIfStmt](pmd_rules_apex_errorprone.html#emptyifstmt), [EmptyStatementBlock](pmd_rules_apex_errorprone.html#emptystatementblock), [EmptyTryOrFinallyBlock](pmd_rules_apex_errorprone.html#emptytryorfinallyblock), [EmptyWhileStmt](pmd_rules_apex_errorprone.html#emptywhilestmt), [ExcessiveClassLength](pmd_rules_apex_design.html#excessiveclasslength), [ExcessiveParameterList](pmd_rules_apex_design.html#excessiveparameterlist), [ExcessivePublicCount](pmd_rules_apex_design.html#excessivepubliccount), [ForLoopsMustUseBraces](pmd_rules_apex_codestyle.html#forloopsmustusebraces), [IfElseStmtsMustUseBraces](pmd_rules_apex_codestyle.html#ifelsestmtsmustusebraces), [IfStmtsMustUseBraces](pmd_rules_apex_codestyle.html#ifstmtsmustusebraces), [MethodNamingConventions](pmd_rules_apex_codestyle.html#methodnamingconventions), [MethodWithSameNameAsEnclosingClass](pmd_rules_apex_errorprone.html#methodwithsamenameasenclosingclass), [NcssConstructorCount](pmd_rules_apex_design.html#ncssconstructorcount), [NcssMethodCount](pmd_rules_apex_design.html#ncssmethodcount), [NcssTypeCount](pmd_rules_apex_design.html#ncsstypecount), [OneDeclarationPerLine](pmd_rules_apex_codestyle.html#onedeclarationperline), [StdCyclomaticComplexity](pmd_rules_apex_design.html#stdcyclomaticcomplexity), [TooManyFields](pmd_rules_apex_design.html#toomanyfields), [VariableNamingConventions](pmd_rules_apex_codestyle.html#variablenamingconventions), [WhileLoopsMustUseBraces](pmd_rules_apex_codestyle.html#whileloopsmustusebraces)
* Empty Code (`rulesets/apex/empty.xml`):

2
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNode.java
View File

@ -16,7 +16,7 @@ public abstract class AbstractApexNode<T extends AstNode> extends AbstractApexNo
protected final T node;
public AbstractApexNode(T node) {
super(node.getClass().hashCode());
super(node.getClass());
this.node = node;
}

7
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/AbstractApexNodeBase.java
View File

@ -10,15 +10,11 @@ import net.sourceforge.pmd.lang.ast.SourceCodePositioner;
public abstract class AbstractApexNodeBase extends AbstractNode {
public AbstractApexNodeBase(int id) {
super(id);
}
public AbstractApexNodeBase(Class<?> klass) {
super(klass.hashCode());
}
public void calculateLineNumbers(SourceCodePositioner positioner, int startOffset, int endOffset) {
/* package */ void calculateLineNumbers(SourceCodePositioner positioner, int startOffset, int endOffset) {
// end column will be interpreted as inclusive, while endOffset/endIndex
// is exclusive
endOffset -= 1;
@ -44,7 +40,6 @@ public abstract class AbstractApexNodeBase extends AbstractNode {
public Object childrenAccept(ApexParserVisitor visitor, Object data) {
if (children != null) {
for (int i = 0; i < children.length; ++i) {
@SuppressWarnings("unchecked")
// we know that the children here are all ApexNodes
AbstractApexNodeBase apexNode = (AbstractApexNodeBase) children[i];
apexNode.jjtAccept(visitor, data);

66
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeBuilder.java
View File

@ -9,6 +9,7 @@ import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Stack;
@ -222,19 +223,19 @@ public final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
// The Apex nodes with children to build.
private Stack<AstNode> parents = new Stack<>();
private AdditionalPassScope scope = new AdditionalPassScope(Errors.createErrors());
private final SourceCodePositioner sourceCodePositioner;
private final String sourceCode;
private List<TokenLocation> tokenLocations;
private ListIterator<TokenLocation> apexDocTokenLocations;
public ApexTreeBuilder(String sourceCode) {
this.sourceCode = sourceCode;
sourceCodePositioner = new SourceCodePositioner(sourceCode);
tokenLocations = buildTokens(sourceCode);
apexDocTokenLocations = buildApexDocTokenLocations(sourceCode).listIterator();
}
AdditionalPassScope scope = new AdditionalPassScope(Errors.createErrors());
static <T extends AstNode> AbstractApexNode<T> createNodeAdapter(T node) {
try {
@SuppressWarnings("unchecked")
@ -294,38 +295,45 @@ public final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
}
private int getApexDocIndex(ApexNode<?> node) {
ASTAnnotation annotation = node.getFirstDescendantOfType(ASTAnnotation.class);
ApexNode<?> firstNode = annotation == null ? node : annotation;
int index = firstNode.getNode().getLoc().getStartIndex();
final int index = node.getNode().getLoc().getStartIndex();
return sourceCode.lastIndexOf('\n', index);
}
private TokenLocation getApexDocTokenLocation(int index) {
TokenLocation last = null;
for (TokenLocation location : tokenLocations) {
while (apexDocTokenLocations.hasNext()) {
final TokenLocation location = apexDocTokenLocations.next();
if (location.index >= index) {
if (last != null && last.token.startsWith("/**")) {
// rollback, the next token corresponds to a different node
apexDocTokenLocations.previous();
if (last != null) {
return last;
}
return null;
}
last = location;
}
return null;
return last;
}
private static List<TokenLocation> buildTokens(String source) {
private static List<TokenLocation> buildApexDocTokenLocations(String source) {
ANTLRStringStream stream = new ANTLRStringStream(source);
ApexLexer lexer = new ApexLexer(stream);
ArrayList<TokenLocation> tokenLocations = new ArrayList<>();
Integer startIndex = 0;
int startIndex = 0;
Token token = lexer.nextToken();
Integer endIndex = lexer.getCharIndex();
int endIndex = lexer.getCharIndex();
while (token.getType() != Token.EOF) {
if (token.getType() != ApexLexer.WS) {
tokenLocations.add(new TokenLocation(startIndex, token.getText()));
if (token.getType() == ApexLexer.BLOCK_COMMENT) {
// Filter only block comments starting with "/**"
if (token.getText().startsWith("/**")) {
tokenLocations.add(new TokenLocation(startIndex, token.getText()));
}
}
// TODO : Check other non-doc comments and tokens of type ApexLexer.EOL_COMMENT for "NOPMD" suppressions
startIndex = endIndex;
token = lexer.nextToken();
endIndex = lexer.getCharIndex();
@ -360,12 +368,9 @@ public final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
@Override
public boolean visit(UserInterface node, AdditionalPassScope scope) {
return visit(node);
}
@Override
public void visitEnd(UserInterface node, AdditionalPassScope scope) {
final boolean ret = visit(node);
buildFormalComment(node);
return ret;
}
@Override
@ -650,12 +655,9 @@ public final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
@Override
public boolean visit(Property node, AdditionalPassScope scope) {
return visit(node);
}
@Override
public void visitEnd(Property node, AdditionalPassScope scope) {
final boolean ret = visit(node);
buildFormalComment(node);
return ret;
}
@Override
@ -720,22 +722,16 @@ public final class ApexTreeBuilder extends AstVisitor<AdditionalPassScope> {
@Override
public boolean visit(UserClass node, AdditionalPassScope scope) {
return visit(node);
}
@Override
public void visitEnd(UserClass node, AdditionalPassScope scope) {
final boolean ret = visit(node);
buildFormalComment(node);
return ret;
}
@Override
public boolean visit(Method node, AdditionalPassScope scope) {
return visit(node);
}
@Override
public void visitEnd(Method node, AdditionalPassScope scope) {
final boolean ret = visit(node);
buildFormalComment(node);
return ret;
}
@Override

46
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/documentation/ApexDocRule.java
View File

@ -11,6 +11,7 @@ import java.util.ArrayList;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import net.sourceforge.pmd.lang.apex.ast.ASTAnnotation;
import net.sourceforge.pmd.lang.apex.ast.ASTFormalComment;
@ -23,7 +24,6 @@ import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
import apex.jorje.data.Locations;
import apex.jorje.semantic.ast.member.Parameter;
import apex.jorje.semantic.ast.modifier.ModifierGroup;
public class ApexDocRule extends AbstractApexRule {
@ -37,32 +37,32 @@ public class ApexDocRule extends AbstractApexRule {
private static final String UNEXPECTED_RETURN_MESSAGE = "Unexpected ApexDoc @return";
private static final String MISMATCHED_PARAM_MESSAGE = "Missing or mismatched ApexDoc @param";
private boolean inTestClass;
public ApexDocRule() {
addRuleChainVisit(ASTUserClass.class);
addRuleChainVisit(ASTUserInterface.class);
addRuleChainVisit(ASTMethod.class);
addRuleChainVisit(ASTProperty.class);
}
@Override
public Object visit(ASTUserClass node, Object data) {
super.visit(node, data);
handleClassOrInterface(node, data);
return data;
}
@Override
public Object visit(ASTUserInterface node, Object data) {
super.visit(node, data);
handleClassOrInterface(node, data);
return data;
}
@Override
public Object visit(ASTAnnotation node, Object data) {
if (node.getImage().equals("IsTest")) {
inTestClass = true;
}
return data;
}
@Override
public Object visit(ASTMethod node, Object data) {
if (node.jjtGetParent() instanceof ASTProperty) {
// Skip property methods, doc is required on the property itself
return data;
}
ApexDocComment comment = getApexDocComment(node);
if (comment == null) {
if (shouldHaveApexDocs(node)) {
@ -83,11 +83,9 @@ public class ApexDocRule extends AbstractApexRule {
}
}
ArrayList<String> params = new ArrayList<>();
for (Parameter x : node.getNode().getMethodInfo().getParameters()) {
String value = x.getName().getValue();
params.add(value);
}
// Collect parameter names in order
final List<String> params = node.getNode().getMethodInfo().getParameters()
.stream().map(p -> p.getName().getValue()).collect(Collectors.toList());
if (!comment.params.equals(params)) {
addViolationWithMessage(data, node, MISMATCHED_PARAM_MESSAGE);
@ -127,9 +125,17 @@ public class ApexDocRule extends AbstractApexRule {
}
private boolean shouldHaveApexDocs(ApexNode<?> node) {
if (inTestClass || node.getNode().getLoc() == Locations.NONE) {
if (node.getNode().getLoc() == Locations.NONE) {
return false;
}
// is this a test?
for (final ASTAnnotation annotation : node.findDescendantsOfType(ASTAnnotation.class)) {
if (annotation.getImage().equals("IsTest")) {
return false;
}
}
ASTModifierNode modifier = node.getFirstChildOfType(ASTModifierNode.class);
if (modifier != null) {
boolean isPublic = modifier.isPublic();
@ -160,7 +166,7 @@ public class ApexDocRule extends AbstractApexRule {
return null;
}
private class ApexDocComment {
private static class ApexDocComment {
boolean hasDescription;
boolean hasReturn;
List<String> params;

21
pmd-apex/src/main/resources/category/apex/documentation.xml
View File

@ -10,12 +10,22 @@ Rules that are related to code documentation.
</description>
<rule name="ApexDoc"
since="6.7.0"
since="6.8.0"
message="ApexDoc comment is missing or incorrect"
class="net.sourceforge.pmd.lang.apex.rule.documentation.ApexDocRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_documentation.html#apexdoc">
<description>
Identifies missing or incorrect ApexDoc comments.
This rule validates that:
* ApexDoc comments are present for classes, methods, and properties that are public or global, excluding
overrides and test classes (as well as the contents of test classes).
* ApexDoc comments should contain @description.
* ApexDoc comments on non-void, non-constructor methods should contain @return.
* ApexDoc comments on void or constructor methods should not contain @return.
* ApexDoc comments on methods with parameters should contain @param for each parameter, in the same
order as the method signature.
Method overrides and tests are both exempted from having ApexDoc.
</description>
<priority>3</priority>
<example>
@ -23,6 +33,13 @@ Identifies missing or incorrect ApexDoc comments.
/**
* @description Hello World
*/
public class HelloWorld {
/**
* @description Bar
* @return Bar
*/
public Object bar() { return null; }
}
]]>
</example>
</rule>