diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/PropertyDescriptorBuilder.java b/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/PropertyDescriptorBuilder.java index dc0cb5b4a4..784fe1ff0f 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/PropertyDescriptorBuilder.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/properties/builders/PropertyDescriptorBuilder.java @@ -75,4 +75,11 @@ public abstract class PropertyDescriptorBuilder build(); + /** + * Returns the name of the property to be built. + */ + public String getName() { + return name; + } + } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleBuilder.java b/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleBuilder.java index af015a2932..15d4bbc635 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleBuilder.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/rules/RuleBuilder.java @@ -12,6 +12,7 @@ import org.w3c.dom.Element; import net.sourceforge.pmd.Rule; import net.sourceforge.pmd.RulePriority; +import net.sourceforge.pmd.RuleSetReference; import net.sourceforge.pmd.lang.Language; import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.LanguageVersion; @@ -20,7 +21,7 @@ import net.sourceforge.pmd.properties.PropertyDescriptor; /** * Builds a rule, validating its parameters throughout. The builder can define property descriptors, but not override - * them. For that, use {@link RuleFactory#decorateRule(Rule, Element)}. + * them. For that, use {@link RuleFactory#decorateRule(Rule, RuleSetReference, Element)}. * * @author Clément Fournier * @since 6.0.0 diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/AbstractNamingConventionRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/AbstractNamingConventionRule.java new file mode 100644 index 0000000000..380c502949 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/AbstractNamingConventionRule.java @@ -0,0 +1,66 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.codestyle; + +import java.util.regex.Pattern; + +import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.properties.PropertyDescriptor; +import net.sourceforge.pmd.properties.RegexProperty; +import net.sourceforge.pmd.properties.RegexProperty.RegexPBuilder; +import net.sourceforge.pmd.util.StringUtil; + + +/** + * Base class for naming conventions rule. Not public API, but + * used to uniformize eg property names between our rules. + * + *

Protected methods may leak API because concrete classes + * are not final so they're package private instead + * + * @author Clément Fournier + * @since 6.5.0 + */ +abstract class AbstractNamingConventionRule extends AbstractJavaRule { + + + /** The argument is interpreted as the display name, and is converted to camel case to get the property name. */ + RegexPBuilder defaultProp(String name) { + return defaultProp(StringUtil.toCamelCase(name, true), name); + } + + /** Returns a pre-filled builder with the given name and display name (for the description). */ + RegexPBuilder defaultProp(String name, String displayName) { + return RegexProperty.named(name + "Pattern") + .desc("Regex which applies to " + displayName.trim() + " names") + .defaultValue(defaultConvention()); + } + + /** Default regex string for this kind of entities. */ + abstract String defaultConvention(); + + + /** Generic "kind" of node, eg "static method" or "utility class". */ + abstract String kindDisplayName(T node, PropertyDescriptor descriptor); + + /** Extracts the name that should be pattern matched. */ + String nameExtractor(T node) { + return node.getImage(); + } + + + void checkMatches(T node, PropertyDescriptor regex, Object data) { + String name = nameExtractor(node); + if (!getProperty(regex).matcher(name).matches()) { + addViolation(data, node, new Object[]{ + kindDisplayName(node, regex), + name, + getProperty(regex).toString(), + }); + } + } + +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ClassNamingConventionsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ClassNamingConventionsRule.java index a7c23bb0b7..6214a5c93d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ClassNamingConventionsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ClassNamingConventionsRule.java @@ -16,33 +16,30 @@ import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTInitializer; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.ast.AccessNode; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sourceforge.pmd.properties.RegexProperty; -import net.sourceforge.pmd.properties.RegexProperty.RegexPBuilder; -import net.sourceforge.pmd.util.StringUtil; /** * Configurable naming conventions for type declarations. */ -public class ClassNamingConventionsRule extends AbstractJavaRule { +public class ClassNamingConventionsRule extends AbstractNamingConventionRule { - private static final RegexProperty CLASS_REGEX = defaultProp("class").desc("Regex which applies to concrete class names").build(); - private static final RegexProperty ABSTRACT_CLASS_REGEX = defaultProp("abstract class").build(); - private static final RegexProperty INTERFACE_REGEX = defaultProp("interface").build(); - private static final RegexProperty ENUMERATION_REGEX = defaultProp("enum").build(); - private static final RegexProperty ANNOTATION_REGEX = defaultProp("annotation").build(); - private static final RegexProperty UTILITY_CLASS_REGEX = defaultProp("utility class").defaultValue("[A-Z][a-zA-Z0-9]+(Utils?|Helper)").build(); + private final RegexProperty classRegex = defaultProp("class", "concrete class").build(); + private final RegexProperty abstractClassRegex = defaultProp("abstract class").build(); + private final RegexProperty interfaceRegex = defaultProp("interface").build(); + private final RegexProperty enumerationRegex = defaultProp("enum").build(); + private final RegexProperty annotationRegex = defaultProp("annotation").build(); + private final RegexProperty utilityClassRegex = defaultProp("utility class").defaultValue("[A-Z][a-zA-Z0-9]+(Utils?|Helper)").build(); public ClassNamingConventionsRule() { - definePropertyDescriptor(CLASS_REGEX); - definePropertyDescriptor(ABSTRACT_CLASS_REGEX); - definePropertyDescriptor(INTERFACE_REGEX); - definePropertyDescriptor(ENUMERATION_REGEX); - definePropertyDescriptor(ANNOTATION_REGEX); - definePropertyDescriptor(UTILITY_CLASS_REGEX); + definePropertyDescriptor(classRegex); + definePropertyDescriptor(abstractClassRegex); + definePropertyDescriptor(interfaceRegex); + definePropertyDescriptor(enumerationRegex); + definePropertyDescriptor(annotationRegex); + definePropertyDescriptor(utilityClassRegex); addRuleChainVisit(ASTClassOrInterfaceDeclaration.class); addRuleChainVisit(ASTEnumDeclaration.class); @@ -50,16 +47,6 @@ public class ClassNamingConventionsRule extends AbstractJavaRule { } - private void checkMatches(ASTAnyTypeDeclaration node, PropertyDescriptor regex, Object data) { - if (!getProperty(regex).matcher(node.getImage()).matches()) { - addViolation(data, node, new Object[]{ - isUtilityClass(node) ? "utility class" : node.getTypeKind().getPrintableName(), - node.getImage(), - getProperty(regex).toString(), - }); - } - } - // This could probably be moved to ClassOrInterfaceDeclaration // to share the implementation and be used from XPath private boolean isUtilityClass(ASTAnyTypeDeclaration node) { @@ -126,13 +113,13 @@ public class ClassNamingConventionsRule extends AbstractJavaRule { public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { if (node.isAbstract()) { - checkMatches(node, ABSTRACT_CLASS_REGEX, data); + checkMatches(node, abstractClassRegex, data); } else if (isUtilityClass(node)) { - checkMatches(node, UTILITY_CLASS_REGEX, data); + checkMatches(node, utilityClassRegex, data); } else if (node.isInterface()) { - checkMatches(node, INTERFACE_REGEX, data); + checkMatches(node, interfaceRegex, data); } else { - checkMatches(node, CLASS_REGEX, data); + checkMatches(node, classRegex, data); } return data; @@ -141,22 +128,26 @@ public class ClassNamingConventionsRule extends AbstractJavaRule { @Override public Object visit(ASTEnumDeclaration node, Object data) { - checkMatches(node, ENUMERATION_REGEX, data); + checkMatches(node, enumerationRegex, data); return data; } @Override public Object visit(ASTAnnotationTypeDeclaration node, Object data) { - checkMatches(node, ANNOTATION_REGEX, data); + checkMatches(node, annotationRegex, data); return data; } - private static RegexPBuilder defaultProp(String name) { - return RegexProperty.named(StringUtil.toCamelCase(name) + "Pattern") - .desc("Regex which applies to " + name.trim() + " names") - .defaultValue("[A-Z][a-zA-Z0-9]+"); + @Override + String defaultConvention() { + return "[A-Z][a-zA-Z0-9]+"; + } + + @Override + String kindDisplayName(ASTAnyTypeDeclaration node, PropertyDescriptor descriptor) { + return isUtilityClass(node) ? "utility class" : node.getTypeKind().getPrintableName(); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodNamingConventionsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodNamingConventionsRule.java index 812c5f4b61..5fc75f6ee9 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodNamingConventionsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodNamingConventionsRule.java @@ -15,16 +15,14 @@ import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTEnumConstant; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper; import net.sourceforge.pmd.properties.BooleanProperty; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sourceforge.pmd.properties.RegexProperty; import net.sourceforge.pmd.properties.RegexProperty.RegexPBuilder; -import net.sourceforge.pmd.util.StringUtil; -public class MethodNamingConventionsRule extends AbstractJavaRule { +public class MethodNamingConventionsRule extends AbstractNamingConventionRule { private static final Map DESCRIPTOR_TO_DISPLAY_NAME = new HashMap<>(); @@ -32,34 +30,24 @@ public class MethodNamingConventionsRule extends AbstractJavaRule { private static final BooleanProperty CHECK_NATIVE_METHODS_DESCRIPTOR = new BooleanProperty("checkNativeMethods", "deprecated! Check native methods", true, 1.0f); - private static final RegexProperty INSTANCE_REGEX = defaultProp("method").desc("Regex which applies to instance method names").build(); - private static final RegexProperty STATIC_REGEX = defaultProp("static").build(); - private static final RegexProperty NATIVE_REGEX = defaultProp("native").build(); - private static final RegexProperty JUNIT3_REGEX = defaultProp("JUnit 3 test").defaultValue("test[A-Z0-9][a-zA-Z0-9]*").build(); - private static final RegexProperty JUNIT4_REGEX = defaultProp("JUnit 4 test").build(); + + private final RegexProperty instanceRegex = defaultProp("", "instance").build(); + private final RegexProperty staticRegex = defaultProp("static").build(); + private final RegexProperty nativeRegex = defaultProp("native").build(); + private final RegexProperty junit3Regex = defaultProp("JUnit 3 test").defaultValue("test[A-Z0-9][a-zA-Z0-9]*").build(); + private final RegexProperty junit4Regex = defaultProp("JUnit 4 test").build(); public MethodNamingConventionsRule() { definePropertyDescriptor(CHECK_NATIVE_METHODS_DESCRIPTOR); - definePropertyDescriptor(INSTANCE_REGEX); - definePropertyDescriptor(STATIC_REGEX); - definePropertyDescriptor(NATIVE_REGEX); - definePropertyDescriptor(JUNIT3_REGEX); - definePropertyDescriptor(JUNIT4_REGEX); + definePropertyDescriptor(instanceRegex); + definePropertyDescriptor(staticRegex); + definePropertyDescriptor(nativeRegex); + definePropertyDescriptor(junit3Regex); + definePropertyDescriptor(junit4Regex); } - private void checkMatches(ASTMethodDeclaration node, PropertyDescriptor regex, Object data) { - if (!getProperty(regex).matcher(node.getMethodName()).matches()) { - addViolation(data, node.getMethodDeclarator(), new Object[]{ - DESCRIPTOR_TO_DISPLAY_NAME.get(regex.name()) + " method", - node.getMethodName(), - getProperty(regex).toString(), - }); - } - } - - private boolean isJunit4Test(ASTMethodDeclaration node) { return node.isAnnotationPresent("org.junit.Test"); } @@ -92,31 +80,48 @@ public class MethodNamingConventionsRule extends AbstractJavaRule { if (node.isNative()) { if (getProperty(CHECK_NATIVE_METHODS_DESCRIPTOR)) { - checkMatches(node, NATIVE_REGEX, data); + checkMatches(node, nativeRegex, data); } else { return super.visit(node, data); } } else if (node.isStatic()) { - checkMatches(node, STATIC_REGEX, data); + checkMatches(node, staticRegex, data); } else if (isJunit4Test(node)) { - checkMatches(node, JUNIT4_REGEX, data); + checkMatches(node, junit4Regex, data); } else if (isJunit3Test(node)) { - checkMatches(node, JUNIT3_REGEX, data); + checkMatches(node, junit3Regex, data); } else { - checkMatches(node, INSTANCE_REGEX, data); + checkMatches(node, instanceRegex, data); } return super.visit(node, data); } - private static RegexPBuilder defaultProp(String displayName) { - String propName = StringUtil.toCamelCase(displayName, true) + "Pattern"; - DESCRIPTOR_TO_DISPLAY_NAME.put(propName, displayName); + @Override + String defaultConvention() { + return "[a-z][a-zA-Z0-9]+"; + } - return RegexProperty.named(propName) - .desc("Regex which applies to " + displayName.trim() + " method names") - .defaultValue("[a-z][a-zA-Z0-9]+"); + @Override + String nameExtractor(ASTMethodDeclaration node) { + return node.getMethodName(); + } + + @Override + RegexPBuilder defaultProp(String name, String displayName) { + String display = (displayName + " method").trim(); + RegexPBuilder prop = super.defaultProp(name.isEmpty() ? "method" : name, display); + + DESCRIPTOR_TO_DISPLAY_NAME.put(prop.getName(), display); + + return prop; + } + + + @Override + String kindDisplayName(ASTMethodDeclaration node, PropertyDescriptor descriptor) { + return DESCRIPTOR_TO_DISPLAY_NAME.get(descriptor.name()); } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodNamingConventions.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodNamingConventions.xml index c8f4c0b5ba..c54c8e4b99 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodNamingConventions.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodNamingConventions.xml @@ -15,6 +15,9 @@ public class Foo { method names should not contain underscores 1 + + The instance method name 'bar_foo' doesn't match '[a-z][a-zA-Z0-9]+' + true 1 2 + + The native method name '__surfunc__' doesn't match '[a-z][a-zA-Z0-9]+' + st_[a-z][A-Za-z]* 1 2 + + The static method name 'foo' doesn't match 'st_[a-z][A-Za-z]*' + nt_[a-z][A-Za-z]* 1 2 + + The native method name 'foo' doesn't match 'nt_[a-z][A-Za-z]*' + test_[a-z][A-Za-z]* 1 9 + + The JUnit 3 test method name 'testGetBestTeam' doesn't match 'test_[a-z][A-Za-z]*' + [a-z][A-Za-z]*Test 2 12,16 + + The JUnit 4 test method name 'testGetBestTeam' doesn't match '[a-z][A-Za-z]*Test' + The JUnit 4 test method name 'getBestTeam' doesn't match '[a-z][A-Za-z]*Test' + + + Instance method custom convention + m_[a-z][A-Za-z]* + 1 + 3 + + The instance method name 'fooBar' doesn't match 'm_[a-z][A-Za-z]*' + + + + +