diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredRule.java index 7d915f1511..aaf8c59e8a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredRule.java @@ -11,7 +11,9 @@ import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.logging.Logger; +import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; @@ -20,7 +22,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMarkerAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTName; -import net.sourceforge.pmd.lang.java.ast.ASTPackageDeclaration; import net.sourceforge.pmd.lang.java.ast.AbstractJavaAccessNode; import net.sourceforge.pmd.lang.java.ast.AbstractJavaNode; import net.sourceforge.pmd.lang.java.multifile.signature.JavaOperationSignature; @@ -33,11 +34,11 @@ import net.sourceforge.pmd.properties.PropertyFactory; * @author Brian Remedios */ public class CommentRequiredRule extends AbstractCommentRule { + private static final Logger LOG = Logger.getLogger(CommentRequiredRule.class.getName()); // Used to pretty print a message private static final Map DESCRIPTOR_NAME_TO_COMMENT_TYPE = new HashMap<>(); - private static final PropertyDescriptor ACCESSOR_CMT_DESCRIPTOR = requirementPropertyBuilder("accessorCommentRequirement", "Comments on getters and setters\"") .defaultValue(CommentRequirement.Ignored).build(); @@ -45,8 +46,7 @@ public class CommentRequiredRule extends AbstractCommentRule { = requirementPropertyBuilder("methodWithOverrideCommentRequirement", "Comments on @Override methods") .defaultValue(CommentRequirement.Ignored).build(); private static final PropertyDescriptor HEADER_CMT_REQUIREMENT_DESCRIPTOR - = requirementPropertyBuilder("headerCommentRequirement", "Header comments") - .defaultValue(CommentRequirement.Ignored).build(); + = requirementPropertyBuilder("headerCommentRequirement", "Deprecated! Header comments. Please use the property \"classCommentRequired\" instead.").build(); private static final PropertyDescriptor CLASS_CMT_REQUIREMENT_DESCRIPTOR = requirementPropertyBuilder("classCommentRequirement", "Class comments").build(); private static final PropertyDescriptor FIELD_CMT_REQUIREMENT_DESCRIPTOR @@ -64,6 +64,8 @@ public class CommentRequiredRule extends AbstractCommentRule { = requirementPropertyBuilder("serialPersistentFieldsCommentRequired", "Serial persistent fields comments") .defaultValue(CommentRequirement.Ignored).build(); + /** stores the resolved property values. This is necessary in order to transparently use deprecated properties. */ + private final Map, CommentRequirement> propertyValues = new HashMap<>(); public CommentRequiredRule() { definePropertyDescriptor(OVERRIDE_CMT_DESCRIPTOR); @@ -78,10 +80,37 @@ public class CommentRequiredRule extends AbstractCommentRule { definePropertyDescriptor(SERIAL_PERSISTENT_FIELDS_CMT_REQUIREMENT_DESCRIPTOR); } + @Override + public void start(RuleContext ctx) { + propertyValues.put(ACCESSOR_CMT_DESCRIPTOR, getProperty(ACCESSOR_CMT_DESCRIPTOR)); + propertyValues.put(OVERRIDE_CMT_DESCRIPTOR, getProperty(OVERRIDE_CMT_DESCRIPTOR)); + propertyValues.put(FIELD_CMT_REQUIREMENT_DESCRIPTOR, getProperty(FIELD_CMT_REQUIREMENT_DESCRIPTOR)); + propertyValues.put(PUB_METHOD_CMT_REQUIREMENT_DESCRIPTOR, getProperty(PUB_METHOD_CMT_REQUIREMENT_DESCRIPTOR)); + propertyValues.put(PROT_METHOD_CMT_REQUIREMENT_DESCRIPTOR, getProperty(PROT_METHOD_CMT_REQUIREMENT_DESCRIPTOR)); + propertyValues.put(ENUM_CMT_REQUIREMENT_DESCRIPTOR, getProperty(ENUM_CMT_REQUIREMENT_DESCRIPTOR)); + propertyValues.put(SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR, + getProperty(SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR)); + propertyValues.put(SERIAL_PERSISTENT_FIELDS_CMT_REQUIREMENT_DESCRIPTOR, + getProperty(SERIAL_PERSISTENT_FIELDS_CMT_REQUIREMENT_DESCRIPTOR)); + + CommentRequirement headerCommentRequirementValue = getProperty(HEADER_CMT_REQUIREMENT_DESCRIPTOR); + boolean headerCommentRequirementValueOverridden = headerCommentRequirementValue != CommentRequirement.Required; + CommentRequirement classCommentRequirementValue = getProperty(CLASS_CMT_REQUIREMENT_DESCRIPTOR); + boolean classCommentRequirementValueOverridden = classCommentRequirementValue != CommentRequirement.Required; + + if (headerCommentRequirementValueOverridden && !classCommentRequirementValueOverridden) { + LOG.warning("Rule CommentRequired uses deprecated property 'headerCommentRequirement'. " + + "Future versions of PMD will remove support for this property. " + + "Please use 'classCommentRequirement' instead!"); + propertyValues.put(CLASS_CMT_REQUIREMENT_DESCRIPTOR, headerCommentRequirementValue); + } else { + propertyValues.put(CLASS_CMT_REQUIREMENT_DESCRIPTOR, classCommentRequirementValue); + } + } private void checkCommentMeetsRequirement(Object data, AbstractJavaNode node, PropertyDescriptor descriptor) { - switch (getProperty(descriptor)) { + switch (propertyValues.get(descriptor)) { case Ignored: break; case Required: @@ -119,13 +148,6 @@ public class CommentRequiredRule extends AbstractCommentRule { } - @Override - public Object visit(ASTPackageDeclaration decl, Object data) { - checkCommentMeetsRequirement(data, decl, HEADER_CMT_REQUIREMENT_DESCRIPTOR); - return super.visit(decl, data); - } - - @Override public Object visit(ASTConstructorDeclaration decl, Object data) { checkMethodOrConstructorComment(decl, data); @@ -193,9 +215,9 @@ public class CommentRequiredRule extends AbstractCommentRule { * This field must be initialized with an array of ObjectStreamField objects. * The modifiers for the field are required to be private, static, and final. * - * @see Oracle docs * @param field the field, must not be null - * @return true if the field ia a serialPersistentFields variable, otherwise false + * @return true if the field is a serialPersistentFields variable, otherwise false + * @see Oracle docs */ private boolean isSerialPersistentFields(final ASTFieldDeclaration field) { return "serialPersistentFields".equals(field.getVariableName()) @@ -212,7 +234,6 @@ public class CommentRequiredRule extends AbstractCommentRule { return super.visit(decl, data); } - @Override public Object visit(ASTCompilationUnit cUnit, Object data) { assignCommentsToDeclarations(cUnit); @@ -223,8 +244,8 @@ public class CommentRequiredRule extends AbstractCommentRule { return getProperty(OVERRIDE_CMT_DESCRIPTOR) == CommentRequirement.Ignored && getProperty(ACCESSOR_CMT_DESCRIPTOR) == CommentRequirement.Ignored - && getProperty(HEADER_CMT_REQUIREMENT_DESCRIPTOR) == CommentRequirement.Ignored - && getProperty(CLASS_CMT_REQUIREMENT_DESCRIPTOR) == CommentRequirement.Ignored + && (getProperty(CLASS_CMT_REQUIREMENT_DESCRIPTOR) == CommentRequirement.Ignored + || getProperty(HEADER_CMT_REQUIREMENT_DESCRIPTOR) == CommentRequirement.Ignored) && getProperty(FIELD_CMT_REQUIREMENT_DESCRIPTOR) == CommentRequirement.Ignored && getProperty(PUB_METHOD_CMT_REQUIREMENT_DESCRIPTOR) == CommentRequirement.Ignored && getProperty(PROT_METHOD_CMT_REQUIREMENT_DESCRIPTOR) == CommentRequirement.Ignored diff --git a/pmd-java/src/main/resources/category/java/documentation.xml b/pmd-java/src/main/resources/category/java/documentation.xml index 44ca738003..db378336b3 100644 --- a/pmd-java/src/main/resources/category/java/documentation.xml +++ b/pmd-java/src/main/resources/category/java/documentation.xml @@ -31,7 +31,7 @@ A rule for the politically correct... we don't want to offend anyone. class="net.sourceforge.pmd.lang.java.rule.documentation.CommentRequiredRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_documentation.html#commentrequired"> -Denotes whether comments are required (or unwanted) for specific language elements. +Denotes whether javadoc (formal) comments are required (or unwanted) for specific language elements. 3 diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredTest.java index 6b31f2edfe..7b6d9a6eca 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/documentation/CommentRequiredTest.java @@ -8,6 +8,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import org.junit.Test; @@ -23,6 +24,14 @@ public class CommentRequiredTest extends PmdRuleTst { assertNull("By default, the rule should be functional", rule.dysfunctionReason()); List> propertyDescriptors = getProperties(rule); + // remove deprecated properties + for (Iterator> it = propertyDescriptors.iterator(); it.hasNext();) { + PropertyDescriptor property = it.next(); + if (property.description().startsWith("Deprecated!")) { + it.remove(); + } + } + for (PropertyDescriptor property : propertyDescriptors) { setPropertyValue(rule, property, "Ignored"); } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentRequired.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentRequired.xml index bc2c3c3e61..3c372bdd0a 100755 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentRequired.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/documentation/xml/CommentRequired.xml @@ -50,7 +50,6 @@ public class Foo { Missing comments - only class level required. - Required Required Ignored Ignored @@ -62,7 +61,6 @@ public class Foo { Too many comments - all comments are unwanted. - Unwanted Unwanted Unwanted Unwanted @@ -242,9 +240,6 @@ public class CommentRequired implements Serializable { #1522 [java] CommentRequired: false positive 0 - #1683 [java] CommentRequired property names are inconsistent - Required + #1683 [java] CommentRequired property names are inconsistent - use deprecated property + Unwanted 1 - #1683 [java] CommentRequired property names are inconsistent - package with comment without new lines - Required + #1683 [java] CommentRequired property names are inconsistent - use new property + Unwanted 1 - - - - #1683 [java] CommentRequired property names are inconsistent - package without comment and without new lines - Required - 2 -