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 c68a0b475e..df7f0db197 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 @@ -4,8 +4,16 @@ package net.sourceforge.pmd.lang.java.rule.documentation; +import static net.sourceforge.pmd.lang.java.rule.documentation.CommentRequiredRule.CommentRequirement.Ignored; +import static net.sourceforge.pmd.lang.java.rule.documentation.CommentRequiredRule.CommentRequirement.Required; +import static net.sourceforge.pmd.lang.java.rule.documentation.CommentRequiredRule.CommentRequirement.mappings; +import static net.sourceforge.pmd.lang.java.rule.documentation.CommentRequiredRule.CommentRequirement.values; + import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; @@ -16,70 +24,36 @@ 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.AbstractJavaAccessNode; -import net.sourceforge.pmd.properties.BooleanProperty; +import net.sourceforge.pmd.lang.java.ast.AbstractJavaNode; import net.sourceforge.pmd.properties.EnumeratedProperty; -import net.sourceforge.pmd.properties.PropertySource; +import net.sourceforge.pmd.properties.EnumeratedProperty.EnumPBuilder; + /** * @author Brian Remedios */ public class CommentRequiredRule extends AbstractCommentRule { - enum CommentRequirement { - Required("Required"), Ignored("Ignored"), Unwanted("Unwanted"); - - private final String label; - - CommentRequirement(String theLabel) { - label = theLabel; - } - - public static String[] labels() { - String[] labels = new String[values().length]; - int i = 0; - for (CommentRequirement requirement : values()) { - labels[i++] = requirement.label; - } - return labels; - } - } - + public static final EnumeratedProperty IGNORE_OVERRIDE_DESCRIPTOR + = requirementPropertyBuilder("methodWithOverrideRequirement", "Comments on @Override methods") + .defaultValue(Ignored).build(); + public static final EnumeratedProperty HEADER_CMT_REQUIREMENT_DESCRIPTOR + = requirementPropertyBuilder("headerCommentRequirement", "Header comments").uiOrder(1.0f).build(); + public static final EnumeratedProperty FIELD_CMT_REQUIREMENT_DESCRIPTOR + = requirementPropertyBuilder("fieldCommentRequirement", "Field comments").uiOrder(2.0f).build(); + public static final EnumeratedProperty PUB_METHOD_CMT_REQUIREMENT_DESCRIPTOR + = requirementPropertyBuilder("publicMethodCommentRequirement", "Public method and constructor comments") + .uiOrder(3.0f).build(); + public static final EnumeratedProperty PROT_METHOD_CMT_REQUIREMENT_DESCRIPTOR + = requirementPropertyBuilder("protectedMethodCommentRequirement", "Protected method constructor comments") + .uiOrder(4.0f).build(); + public static final EnumeratedProperty ENUM_CMT_REQUIREMENT_DESCRIPTOR + = requirementPropertyBuilder("enumCommentRequirement", "Enum comments").uiOrder(5.0f).build(); + public static final EnumeratedProperty SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR + = requirementPropertyBuilder("serialVersionUIDCommentRequired", "Serial version UID comments") + .defaultValue(Ignored).uiOrder(6.0f).build(); private boolean ignoreOverrideMethods = true; - public static final BooleanProperty IGNORE_OVERRIDE_DESCRIPTOR = - BooleanProperty.named("ignoreOverride") - .desc("Ignore methods marked with @Override") - .defaultValue(true) - .build(); - - public static final EnumeratedProperty HEADER_CMT_REQUIREMENT_DESCRIPTOR = new EnumeratedProperty<>( - "headerCommentRequirement", - "Header comments. Possible values: " + Arrays.toString(CommentRequirement.values()), - CommentRequirement.labels(), CommentRequirement.values(), 0, CommentRequirement.class, 1.0f); - - public static final EnumeratedProperty FIELD_CMT_REQUIREMENT_DESCRIPTOR = new EnumeratedProperty<>( - "fieldCommentRequirement", - "Field comments. Possible values: " + Arrays.toString(CommentRequirement.values()), - CommentRequirement.labels(), CommentRequirement.values(), 0, CommentRequirement.class, 2.0f); - - public static final EnumeratedProperty PUB_METHOD_CMT_REQUIREMENT_DESCRIPTOR = new EnumeratedProperty<>( - "publicMethodCommentRequirement", - "Public method and constructor comments. Possible values: " + Arrays.toString(CommentRequirement.values()), - CommentRequirement.labels(), CommentRequirement.values(), 0, CommentRequirement.class, 3.0f); - - public static final EnumeratedProperty PROT_METHOD_CMT_REQUIREMENT_DESCRIPTOR = new EnumeratedProperty<>( - "protectedMethodCommentRequirement", - "Protected method constructor comments. Possible values: " + Arrays.toString(CommentRequirement.values()), - CommentRequirement.labels(), CommentRequirement.values(), 0, CommentRequirement.class, 4.0f); - - public static final EnumeratedProperty ENUM_CMT_REQUIREMENT_DESCRIPTOR = new EnumeratedProperty<>( - "enumCommentRequirement", "Enum comments. Possible values: " + Arrays.toString(CommentRequirement.values()), - CommentRequirement.labels(), CommentRequirement.values(), 0, CommentRequirement.class, 5.0f); - - public static final EnumeratedProperty SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR = new EnumeratedProperty<>( - "serialVersionUIDCommentRequired", - "serial version UID commts. Possible values: " + Arrays.toString(CommentRequirement.values()), - CommentRequirement.labels(), CommentRequirement.values(), 1, CommentRequirement.class, 6.0f); public CommentRequiredRule() { definePropertyDescriptor(IGNORE_OVERRIDE_DESCRIPTOR); @@ -91,205 +65,157 @@ public class CommentRequiredRule extends AbstractCommentRule { definePropertyDescriptor(SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR); } - private CommentRequirement getCommentRequirement(String label) { - if (CommentRequirement.Ignored.label.equals(label)) { - return CommentRequirement.Ignored; - } else if (CommentRequirement.Required.label.equals(label)) { - return CommentRequirement.Required; - } else if (CommentRequirement.Unwanted.label.equals(label)) { - return CommentRequirement.Unwanted; - } else { - return null; + + private void checkCommentMeetsRequirement(Object data, AbstractJavaNode node, + EnumeratedProperty descriptor) { + switch (getProperty(descriptor)) { + case Ignored: + break; + case Required: + if (node.comment() == null) { + commentRequiredViolation(data, node, descriptor); + } + break; + case Unwanted: + if (node.comment() != null) { + commentRequiredViolation(data, node, descriptor); + } + break; + default: + break; } } + // Adds a violation + private void commentRequiredViolation(Object data, AbstractJavaNode node, + EnumeratedProperty descriptor) { + addViolationWithMessage(data, node, descriptor.name() + " " + getProperty(descriptor)); + } + + @Override public Object visit(ASTClassOrInterfaceDeclaration decl, Object data) { - CommentRequirement headerRequirement = getCommentRequirement( - getProperty(HEADER_CMT_REQUIREMENT_DESCRIPTOR).toString()); - - if (headerRequirement != CommentRequirement.Ignored) { - if (headerRequirement == CommentRequirement.Required) { - if (decl.comment() == null) { - addViolationWithMessage(data, decl, - HEADER_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Required, - decl.getBeginLine(), decl.getEndLine()); - } - } else { - if (decl.comment() != null) { - addViolationWithMessage(data, decl, - HEADER_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Unwanted, - decl.getBeginLine(), decl.getEndLine()); - } - } - } - + checkCommentMeetsRequirement(data, decl, HEADER_CMT_REQUIREMENT_DESCRIPTOR); return super.visit(decl, data); } + @Override public Object visit(ASTConstructorDeclaration decl, Object data) { - checkComment(decl, data); + checkMethodOrConstructorComment(decl, data); return super.visit(decl, data); } + @Override public Object visit(ASTMethodDeclaration decl, Object data) { if (ignoreOverrideMethods) { List annotations = decl.jjtGetParent().findDescendantsOfType(ASTMarkerAnnotation.class); - for (ASTMarkerAnnotation ann : annotations) { + for (ASTMarkerAnnotation ann : annotations) { // TODO consider making a method to get the annotations of a method if (ann.getFirstChildOfType(ASTName.class).getImage().equals("Override")) { return super.visit(decl, data); } } } - checkComment(decl, data); + checkMethodOrConstructorComment(decl, data); return super.visit(decl, data); } - private void checkComment(AbstractJavaAccessNode decl, Object data) { - CommentRequirement pubMethodRequirement = getCommentRequirement( - getProperty(PUB_METHOD_CMT_REQUIREMENT_DESCRIPTOR).toString()); - CommentRequirement protMethodRequirement = getCommentRequirement( - getProperty(PROT_METHOD_CMT_REQUIREMENT_DESCRIPTOR).toString()); + private void checkMethodOrConstructorComment(AbstractJavaAccessNode decl, Object data) { if (decl.isPublic()) { - if (pubMethodRequirement != CommentRequirement.Ignored) { - if (pubMethodRequirement == CommentRequirement.Required) { - if (decl.comment() == null) { - addViolationWithMessage(data, decl, - PUB_METHOD_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Required, - decl.getBeginLine(), decl.getEndLine()); - } - } else { - if (decl.comment() != null) { - addViolationWithMessage(data, decl, - PUB_METHOD_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Unwanted, - decl.getBeginLine(), decl.getEndLine()); - } - } - } + checkCommentMeetsRequirement(data, decl, PUB_METHOD_CMT_REQUIREMENT_DESCRIPTOR); } else if (decl.isProtected()) { - if (protMethodRequirement != CommentRequirement.Ignored) { - if (protMethodRequirement == CommentRequirement.Required) { - if (decl.comment() == null) { - addViolationWithMessage(data, decl, - PROT_METHOD_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Required, - decl.getBeginLine(), decl.getEndLine()); - } - } else { - if (decl.comment() != null) { - addViolationWithMessage(data, decl, - PROT_METHOD_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Unwanted, - decl.getBeginLine(), decl.getEndLine()); - } - } - } + checkCommentMeetsRequirement(data, decl, PROT_METHOD_CMT_REQUIREMENT_DESCRIPTOR); } } + @Override public Object visit(ASTFieldDeclaration decl, Object data) { - CommentRequirement fieldRequirement = getCommentRequirement( - getProperty(FIELD_CMT_REQUIREMENT_DESCRIPTOR).toString()); - - if (fieldRequirement != CommentRequirement.Ignored) { - if (isSerialVersionUID(decl)) { - checkSerialVersionUID(decl, data, fieldRequirement); - } else if (fieldRequirement == CommentRequirement.Required) { - if (decl.comment() == null) { - addViolationWithMessage(data, decl, - FIELD_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Required, - decl.getBeginLine(), decl.getEndLine()); - } - } else { - if (decl.comment() != null) { - addViolationWithMessage(data, decl, - FIELD_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Unwanted, - decl.getBeginLine(), decl.getEndLine()); - } - } + if (isSerialVersionUID(decl)) { + checkCommentMeetsRequirement(data, decl, SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR); + } else { + checkCommentMeetsRequirement(data, decl, FIELD_CMT_REQUIREMENT_DESCRIPTOR); } return super.visit(decl, data); } - private void checkSerialVersionUID(ASTFieldDeclaration decl, Object data, CommentRequirement fieldRequirement) { - CommentRequirement serialVersionUIDReq = getCommentRequirement( - getProperty(SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR).toString()); - if (serialVersionUIDReq != CommentRequirement.Ignored) { - if (fieldRequirement == CommentRequirement.Required) { - if (decl.comment() == null) { - addViolationWithMessage(data, decl, - SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Required, - decl.getBeginLine(), decl.getEndLine()); - } - } else { - if (decl.comment() != null) { - addViolationWithMessage(data, decl, - SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Unwanted, - decl.getBeginLine(), decl.getEndLine()); - } - } - } - } private boolean isSerialVersionUID(ASTFieldDeclaration field) { - if ("serialVersionUID".equals(field.getVariableName()) && field.isStatic() && field.isFinal() - && field.getType() == long.class) { - return true; - } - return false; + return "serialVersionUID".equals(field.getVariableName()) + && field.isStatic() + && field.isFinal() + && field.getType() == long.class; } + @Override public Object visit(ASTEnumDeclaration decl, Object data) { - - CommentRequirement enumRequirement = getCommentRequirement( - getProperty(ENUM_CMT_REQUIREMENT_DESCRIPTOR).toString()); - - if (enumRequirement != CommentRequirement.Ignored) { - if (enumRequirement == CommentRequirement.Required) { - if (decl.comment() == null) { - addViolationWithMessage(data, decl, - ENUM_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Required, - decl.getBeginLine(), decl.getEndLine()); - } - } else { - if (decl.comment() != null) { - addViolationWithMessage(data, decl, - ENUM_CMT_REQUIREMENT_DESCRIPTOR.name() + " " + CommentRequirement.Unwanted, - decl.getBeginLine(), decl.getEndLine()); - } - } - } - + checkCommentMeetsRequirement(data, decl, ENUM_CMT_REQUIREMENT_DESCRIPTOR); return super.visit(decl, data); } + @Override public Object visit(ASTCompilationUnit cUnit, Object data) { assignCommentsToDeclarations(cUnit); - ignoreOverrideMethods = getProperty(IGNORE_OVERRIDE_DESCRIPTOR); + // ignoreOverrideMethods = getProperty(IGNORE_OVERRIDE_DESCRIPTOR); return super.visit(cUnit, data); } + public boolean allCommentsAreIgnored() { - return 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 - && getProperty(SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR) == CommentRequirement.Ignored; + return getProperty(HEADER_CMT_REQUIREMENT_DESCRIPTOR) == Ignored + && getProperty(FIELD_CMT_REQUIREMENT_DESCRIPTOR) == Ignored + && getProperty(PUB_METHOD_CMT_REQUIREMENT_DESCRIPTOR) == Ignored + && getProperty(PROT_METHOD_CMT_REQUIREMENT_DESCRIPTOR) == Ignored + && getProperty(SERIAL_VERSION_UID_CMT_REQUIREMENT_DESCRIPTOR) == Ignored; } - /** - * @see PropertySource#dysfunctionReason() - */ + @Override public String dysfunctionReason() { return allCommentsAreIgnored() ? "All comment types are ignored" : null; } + + + enum CommentRequirement { + Required("Required"), Ignored("Ignored"), Unwanted("Unwanted"); + + private static final Map MAPPINGS; + private final String label; + + static { + Map tmp = new HashMap<>(); + for (CommentRequirement r : values()) { + tmp.put(r.label, r); + } + MAPPINGS = Collections.unmodifiableMap(tmp); + } + + + CommentRequirement(String theLabel) { + label = theLabel; + } + + + public static Map mappings() { + return MAPPINGS; + } + } + + + // pre-filled builder + private static EnumPBuilder requirementPropertyBuilder(String name, String commentType) { + return EnumeratedProperty.named(name) + .desc(commentType + ". Possible values: " + Arrays.toString(values())) + .mappings(mappings()) + .defaultValue(Required) + .type(CommentRequirement.class); + } }