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

Merge branch 'pr-1252'

Browse Source
This commit is contained in:
Clément Fournier
2018-08-12 19:27:20 +02:00
parent 7bbb688859 c3ea6de8e0
commit 0ae88f7cd0
7 changed files with 680 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
docs/pages/release_notes.md
View File

@ -22,6 +22,11 @@ This is a minor release.
#### New Rules
* The new Java rule [`LinguisticNaming`](pmd_rules_java_codestyle.html#linguisticnaming) (`java-codestyle`)
detects cases, when a method name indicates it returns a boolean (such as `isSmall()`) but it doesn't.
Besides method names, the rule also checks field and variable names. It also checks, that getters return
something but setters won't. The rule has several properties with which it can be customized.
* The new PL/SQL rule [`ForLoopNaming`](pmd_rules_plsql_codestyle.html#forloopnaming) (`plsql-codestyle`)
enforces a naming convention for "for loops". Both "cursor for loops" and "index for loops" are covered.
The rule can be customized via patterns. By default, short variable names are reported.
@ -48,6 +53,7 @@ This is a minor release.
### External Contributions
* [#109](https://github.com/pmd/pmd/pull/109): \[java] Add two linguistics rules under naming - [Arda Aslan](https://github.com/ardaasln)
* [#1254](https://github.com/pmd/pmd/pull/1254): \[ci] \[GSoC] Integrating the danger and pmdtester to travis CI - [BBG](https://github.com/djydewang)
* [#1258](https://github.com/pmd/pmd/pull/1258): \[java] Use typeof in MissingSerialVersionUID - [krichter722](https://github.com/krichter722)
* [#1264](https://github.com/pmd/pmd/pull/1264): \[cpp] Fix NullPointerException in CPPTokenizer:99 - [Rafael Cortês](https://github.com/mrfyda)

2
pmd-core/src/main/resources/rulesets/releases/670.xml
View File

@ -9,5 +9,7 @@ This ruleset contains links to rules that are new in PMD v6.7.0
</description>
<rule ref="category/plsql/codestyle.xml/ForLoopNaming"/>
<rule ref="category/java/codestyle.xml/LinguisticNaming"/>
</ruleset>

5
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTVariableDeclarator.java
View File

@ -21,4 +21,9 @@ public class ASTVariableDeclarator extends AbstractJavaTypeNode {
public Object jjtAccept(JavaParserVisitor visitor, Object data) {
return visitor.visit(this, data);
}
public String getName() {
// first child will be VariableDeclaratorId
return jjtGetChild(0).getImage();
}
}

186
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LinguisticNamingRule.java Normal file
View File

@ -0,0 +1,186 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.rule.codestyle;
import java.util.List;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTResultType;
import net.sourceforge.pmd.lang.java.ast.ASTType;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.properties.BooleanProperty;
import net.sourceforge.pmd.properties.StringMultiProperty;
public class LinguisticNamingRule extends AbstractJavaRule {
private static final BooleanProperty CHECK_BOOLEAN_METHODS = BooleanProperty.named("checkBooleanMethod")
.defaultValue(true).desc("Check method names and types for inconsistent naming").uiOrder(1.0f).build();
private static final BooleanProperty CHECK_GETTERS = BooleanProperty.named("checkGetters").defaultValue(true)
.desc("Check return type of getters").uiOrder(2.0f).build();
private static final BooleanProperty CHECK_SETTERS = BooleanProperty.named("checkSetters").defaultValue(true)
.desc("Check return type of setters").uiOrder(3.0f).build();
private static final BooleanProperty CHECK_PREFIXED_TRANSFORM_METHODS = BooleanProperty
.named("checkPrefixedTransformMethods")
.defaultValue(true).desc("Check return type of methods whose names start with 'to'").uiOrder(4.0f).build();
private static final BooleanProperty CHECK_TRANSFORM_METHODS = BooleanProperty.named("checkTransformMethods")
.defaultValue(false).desc("Check return type of methods which contain 'To' in their name").uiOrder(4.0f).build();
private static final StringMultiProperty BOOLEAN_METHOD_PREFIXES_PROPERTY = StringMultiProperty
.named("booleanMethodPrefixes").defaultValues("is", "has", "can", "have", "will", "should")
.desc("the prefixes of methods that return boolean").uiOrder(5.0f).build();
private static final BooleanProperty CHECK_FIELDS = BooleanProperty.named("checkFields").defaultValue(true)
.desc("Check field names and types for inconsistent naming").uiOrder(6.0f).build();
private static final BooleanProperty CHECK_VARIABLES = BooleanProperty.named("checkVariables").defaultValue(true)
.desc("Check local variable names and types for inconsistent naming").uiOrder(7.0f).build();
private static final StringMultiProperty BOOLEAN_FIELD_PREFIXES_PROPERTY = StringMultiProperty
.named("booleanFieldPrefixes").defaultValues("is", "has", "can", "have", "will", "should")
.desc("the prefixes of fields and variables that indicate boolean").uiOrder(8.0f).build();
public LinguisticNamingRule() {
definePropertyDescriptor(CHECK_BOOLEAN_METHODS);
definePropertyDescriptor(CHECK_GETTERS);
definePropertyDescriptor(CHECK_SETTERS);
definePropertyDescriptor(CHECK_PREFIXED_TRANSFORM_METHODS);
definePropertyDescriptor(CHECK_TRANSFORM_METHODS);
definePropertyDescriptor(BOOLEAN_METHOD_PREFIXES_PROPERTY);
definePropertyDescriptor(CHECK_FIELDS);
definePropertyDescriptor(CHECK_VARIABLES);
definePropertyDescriptor(BOOLEAN_FIELD_PREFIXES_PROPERTY);
addRuleChainVisit(ASTMethodDeclaration.class);
addRuleChainVisit(ASTFieldDeclaration.class);
addRuleChainVisit(ASTLocalVariableDeclaration.class);
}
@Override
public Object visit(ASTMethodDeclaration node, Object data) {
String nameOfMethod = node.getMethodName();
if (getProperty(CHECK_BOOLEAN_METHODS)) {
checkBooleanMethods(node, data, nameOfMethod);
}
if (getProperty(CHECK_SETTERS)) {
checkSetters(node, data, nameOfMethod);
}
if (getProperty(CHECK_GETTERS)) {
checkGetters(node, data, nameOfMethod);
}
if (getProperty(CHECK_PREFIXED_TRANSFORM_METHODS)) {
checkPrefixedTransformMethods(node, data, nameOfMethod);
}
if (getProperty(CHECK_TRANSFORM_METHODS)) {
checkTransformMethods(node, data, nameOfMethod);
}
return data;
}
private void checkPrefixedTransformMethods(ASTMethodDeclaration node, Object data, String nameOfMethod) {
ASTResultType resultType = node.getResultType();
if (resultType.isVoid() && hasPrefix(nameOfMethod, "to")) {
// To as prefix
addViolationWithMessage(data, node, "Linguistics Antipattern - The transform method ''{0}'' should not return void linguistically",
new Object[] { nameOfMethod });
}
}
private void checkTransformMethods(ASTMethodDeclaration node, Object data, String nameOfMethod) {
ASTResultType resultType = node.getResultType();
if (resultType.isVoid() && containsWord(nameOfMethod, "To")) {
// To in the middle somewhere
addViolationWithMessage(data, node, "Linguistics Antipattern - The transform method ''{0}'' should not return void linguistically",
new Object[] { nameOfMethod });
}
}
private void checkGetters(ASTMethodDeclaration node, Object data, String nameOfMethod) {
ASTResultType resultType = node.getResultType();
if (hasPrefix(nameOfMethod, "get") && resultType.isVoid()) {
addViolationWithMessage(data, node, "Linguistics Antipattern - The getter ''{0}'' should not return void linguistically",
new Object[] { nameOfMethod });
}
}
private void checkSetters(ASTMethodDeclaration node, Object data, String nameOfMethod) {
ASTResultType resultType = node.getResultType();
if (hasPrefix(nameOfMethod, "set") && !resultType.isVoid()) {
addViolationWithMessage(data, node, "Linguistics Antipattern - The setter ''{0}'' should not return any type except void linguistically",
new Object[] { nameOfMethod });
}
}
private void checkBooleanMethods(ASTMethodDeclaration node, Object data, String nameOfMethod) {
ASTResultType resultType = node.getResultType();
ASTType t = node.getResultType().getFirstChildOfType(ASTType.class);
if (!resultType.isVoid() && t != null) {
for (String prefix : getProperty(BOOLEAN_METHOD_PREFIXES_PROPERTY)) {
if (hasPrefix(nameOfMethod, prefix) && !"boolean".equalsIgnoreCase(t.getTypeImage())) {
addViolationWithMessage(data, node, "Linguistics Antipattern - The method ''{0}'' indicates linguistically it returns a boolean, but it returns ''{1}''",
new Object[] { nameOfMethod, t.getTypeImage() });
}
}
}
}
private void checkField(String typeImage, ASTVariableDeclarator node, Object data) {
for (String prefix : getProperty(BOOLEAN_FIELD_PREFIXES_PROPERTY)) {
if (hasPrefix(node.getName(), prefix) && !"boolean".equalsIgnoreCase(typeImage)) {
addViolationWithMessage(data, node, "Linguistics Antipattern - The field ''{0}'' indicates linguistically it is a boolean, but it is ''{1}''",
new Object[] { node.getName(), typeImage });
}
}
}
private void checkVariable(String typeImage, ASTVariableDeclarator node, Object data) {
for (String prefix : getProperty(BOOLEAN_FIELD_PREFIXES_PROPERTY)) {
if (hasPrefix(node.getName(), prefix) && !"boolean".equalsIgnoreCase(typeImage)) {
addViolationWithMessage(data, node, "Linguistics Antipattern - The variable ''{0}'' indicates linguistically it is a boolean, but it is ''{1}''",
new Object[] { node.getName(), typeImage });
}
}
}
@Override
public Object visit(ASTFieldDeclaration node, Object data) {
ASTType type = node.getFirstChildOfType(ASTType.class);
if (type != null && getProperty(CHECK_FIELDS)) {
List<ASTVariableDeclarator> fields = node.findChildrenOfType(ASTVariableDeclarator.class);
for (ASTVariableDeclarator field : fields) {
checkField(type.getTypeImage(), field, data);
}
}
return data;
}
@Override
public Object visit(ASTLocalVariableDeclaration node, Object data) {
ASTType type = node.getFirstChildOfType(ASTType.class);
if (type != null && getProperty(CHECK_VARIABLES)) {
List<ASTVariableDeclarator> variables = node.findChildrenOfType(ASTVariableDeclarator.class);
for (ASTVariableDeclarator variable : variables) {
checkVariable(type.getTypeImage(), variable, data);
}
}
return data;
}
private static boolean hasPrefix(String name, String prefix) {
return name.startsWith(prefix) && name.length() > prefix.length()
&& Character.isUpperCase(name.charAt(prefix.length()));
}
private static boolean containsWord(String name, String word) {
int index = name.indexOf(word);
if (index >= 0 && name.length() > index + word.length()) {
return Character.isUpperCase(name.charAt(index + word.length()));
}
return false;
}
}

63
pmd-java/src/main/resources/category/java/codestyle.xml
View File

@ -938,6 +938,69 @@ if (foo) { // preferred approach
</example>
</rule>
<rule name="LinguisticNaming"
language="java"
since="6.7.0"
message="Linguistics Antipattern - Method name and return type is inconsistent linguistically"
class="net.sourceforge.pmd.lang.java.rule.codestyle.LinguisticNamingRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_codestyle.html#linguisticnaming">
<description>
This rule finds Linguistic Naming Antipatterns. It checks for fields, that are named, as if they should
be boolean but have a different type. It also checks for methods, that according to their name, should
return a boolean, but don't. Further, it checks, that getters return something and setters won't.
Finally, it checks that methods, that start with "to" - so called transform methods - actually return
something, since according to their name, they should convert or transform one object into another.
There is additionally an option, to check for methods that contain "To" in their name - which are
also transform methods. However, this is disabled by default, since this detection is prone to
false positives.
For more information, see [Linguistic Antipatterns - What They Are and How
Developers Perceive Them](https://doi.org/10.1007/s10664-014-9350-8).
</description>
<priority>3</priority>
<example>
<![CDATA[
public class LinguisticNaming {
int isValid; // the field name indicates a boolean, but it is an int.
boolean isTrue; // correct type of the field
void myMethod() {
int hasMoneyLocal; // the local variable name indicates a boolean, but it is an int.
boolean hasSalaryLocal; // correct naming and type
}
// the name of the method indicates, it is a boolean, but the method returns an int.
int isValid() {
return 1;
}
// correct naming and return type
boolean isSmall() {
return true;
}
// the name indicates, this is a setter, but it returns something
int setName() {
return 1;
}
// the name indicates, this is a getter, but it doesn't return anything
void getName() {
// nothing to return?
}
// the name indicates, it transforms an object and should return the result
void toDataType() {
// nothing to return?
}
// the name indicates, it transforms an object and should return the result
void grapeToWine() {
// nothing to return?
}
}
]]>
</example>
</rule>
<rule name="LocalHomeNamingConvention"
language="java"
since="4.0"

1
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/CodeStyleRulesTest.java
View File

@ -42,6 +42,7 @@ public class CodeStyleRulesTest extends SimpleAggregatorTst {
addRule(RULESET, "IdenticalCatchBranches");
addRule(RULESET, "IfElseStmtsMustUseBraces");
addRule(RULESET, "IfStmtsMustUseBraces");
addRule(RULESET, "LinguisticNaming");
addRule(RULESET, "LocalHomeNamingConvention");
addRule(RULESET, "LocalInterfaceSessionNamingConvention");
addRule(RULESET, "LocalVariableCouldBeFinal");

417
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/LinguisticNaming.xml Normal file
View File

File diff suppressed because it is too large Load Diff