Skip to content

Commit

Permalink
Issue #5608: Adding support for allowedAnnotations in javadocTypeCheck
Browse files Browse the repository at this point in the history
  • Loading branch information
ngeor committed Oct 15, 2018
1 parent fdf3438 commit fb5fff4
Show file tree
Hide file tree
Showing 7 changed files with 240 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import com.puppycrawl.tools.checkstyle.api.Scope;
import com.puppycrawl.tools.checkstyle.api.TextBlock;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.utils.AnnotationUtil;
import com.puppycrawl.tools.checkstyle.utils.CheckUtil;
import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
import com.puppycrawl.tools.checkstyle.utils.ScopeUtil;
Expand Down Expand Up @@ -185,7 +186,7 @@ public class JavadocMethodCheck extends AbstractTypeAwareCheck {
*/
private boolean allowMissingPropertyJavadoc;

/** List of annotations that could allow missed documentation. */
/** List of annotations that allow missed documentation. */
private List<String> allowedAnnotations = Collections.singletonList("Override");

/** Method names that match this pattern do not require javadoc blocks. */
Expand Down Expand Up @@ -356,30 +357,6 @@ protected final void processAST(DetailAST ast) {
}
}

/**
* Some javadoc.
* @param methodDef Some javadoc.
* @return Some javadoc.
*/
private boolean hasAllowedAnnotations(DetailAST methodDef) {
boolean result = false;
final DetailAST modifiersNode = methodDef.findFirstToken(TokenTypes.MODIFIERS);
DetailAST annotationNode = modifiersNode.findFirstToken(TokenTypes.ANNOTATION);
while (annotationNode != null && annotationNode.getType() == TokenTypes.ANNOTATION) {
DetailAST identNode = annotationNode.findFirstToken(TokenTypes.IDENT);
if (identNode == null) {
identNode = annotationNode.findFirstToken(TokenTypes.DOT)
.findFirstToken(TokenTypes.IDENT);
}
if (allowedAnnotations.contains(identNode.getText())) {
result = true;
break;
}
annotationNode = annotationNode.getNextSibling();
}
return result;
}

/**
* Some javadoc.
* @param methodDef Some javadoc.
Expand Down Expand Up @@ -428,7 +405,8 @@ private boolean isMissingJavadocAllowed(final DetailAST ast) {
*/
private boolean isContentsAllowMissingJavadoc(DetailAST ast) {
return (ast.getType() == TokenTypes.METHOD_DEF || ast.getType() == TokenTypes.CTOR_DEF)
&& (getMethodsNumberOfLine(ast) <= minLineCount || hasAllowedAnnotations(ast));
&& (getMethodsNumberOfLine(ast) <= minLineCount
|| AnnotationUtil.containsAnnotation(ast, allowedAnnotations));
}

/**
Expand Down Expand Up @@ -488,7 +466,8 @@ private void checkComment(DetailAST ast, TextBlock comment) {
while (!hasInheritDocTag && it.hasNext()) {
hasInheritDocTag = it.next().isInheritDocTag();
}
final boolean reportExpectedTags = !hasInheritDocTag && !hasAllowedAnnotations(ast);
final boolean reportExpectedTags = !hasInheritDocTag
&& !AnnotationUtil.containsAnnotation(ast, allowedAnnotations);

checkParamTags(tags, ast, reportExpectedTags);
checkThrowsTags(tags, getThrows(ast), reportExpectedTags);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package com.puppycrawl.tools.checkstyle.checks.javadoc;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -30,6 +32,7 @@
import com.puppycrawl.tools.checkstyle.api.Scope;
import com.puppycrawl.tools.checkstyle.api.TextBlock;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.utils.AnnotationUtil;
import com.puppycrawl.tools.checkstyle.utils.CheckUtil;
import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
import com.puppycrawl.tools.checkstyle.utils.JavadocUtil;
Expand Down Expand Up @@ -112,6 +115,9 @@ public class JavadocTypeCheck
/** Controls whether to flag errors for unknown tags. Defaults to false. */
private boolean allowUnknownTags;

/** List of annotations that allow missed documentation. */
private List<String> allowedAnnotations = new ArrayList<>();

/**
* Sets the scope to check.
* @param scope a scope.
Expand Down Expand Up @@ -162,6 +168,14 @@ public void setAllowUnknownTags(boolean flag) {
allowUnknownTags = flag;
}

/**
* Sets list of annotations.
* @param userAnnotations user's value.
*/
public void setAllowedAnnotations(String... userAnnotations) {
allowedAnnotations = Arrays.asList(userAnnotations);
}

@Override
public int[] getDefaultTokens() {
return getAcceptableTokens();
Expand Down Expand Up @@ -239,7 +253,8 @@ private boolean shouldCheck(final DetailAST ast) {
&& (excludeScope == null
|| !customScope.isIn(excludeScope)
|| surroundingScope != null
&& !surroundingScope.isIn(excludeScope));
&& !surroundingScope.isIn(excludeScope))
&& !AnnotationUtil.containsAnnotation(ast, allowedAnnotations);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

package com.puppycrawl.tools.checkstyle.utils;

import java.util.List;
import java.util.function.Predicate;

import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.FullIdent;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
Expand All @@ -43,8 +46,7 @@ private AnnotationUtil() {
}

/**
* Checks to see if the AST is annotated with
* the passed in annotation.
* Checks if the AST is annotated with the passed in annotation.
*
* <p>
* This method will not look for imports or package
Expand All @@ -65,18 +67,14 @@ private AnnotationUtil() {
*/
public static boolean containsAnnotation(final DetailAST ast,
String annotation) {
if (ast == null) {
throw new IllegalArgumentException(THE_AST_IS_NULL);
}
return getAnnotation(ast, annotation) != null;
}

/**
* Checks to see if the AST is annotated with
* any annotation.
* Checks if the AST is annotated with any annotation.
*
* @param ast the current node
* @return true if contains an annotation
* @return {@code true} if the AST contains at least one annotation
*/
public static boolean containsAnnotation(final DetailAST ast) {
if (ast == null) {
Expand All @@ -86,10 +84,47 @@ public static boolean containsAnnotation(final DetailAST ast) {
return holder != null && holder.findFirstToken(TokenTypes.ANNOTATION) != null;
}

/**
* Checks if the given AST element is annotated with any of the specified annotations.
*
* <p>
* This method accepts both simple and fully-qualified names,
* e.g. "Override" will match both java.lang.Override and Override.
* </p>
*
* @param ast The type or method definition.
* @param annotations A collection of annotations to look for.
* @return {@code true} if the given AST element is annotated with
* at least one of the specified annotations;
* {@code false} otherwise.
*/
public static boolean containsAnnotation(DetailAST ast, List<String> annotations) {
if (ast == null) {
throw new IllegalArgumentException(THE_AST_IS_NULL);
}

boolean result = false;

if (annotations != null && !annotations.isEmpty()) {
final DetailAST firstMatchingAnnotation = getAnnotation(ast, annotationNode -> {
DetailAST identNode = annotationNode.findFirstToken(TokenTypes.IDENT);
if (identNode == null) {
identNode = annotationNode.findFirstToken(TokenTypes.DOT)
.findFirstToken(TokenTypes.IDENT);
}

return annotations.contains(identNode.getText());
});
result = firstMatchingAnnotation != null;
}

return result;
}

/**
* Gets the AST that holds a series of annotations for the
* potentially annotated AST. Returns {@code null}
* the passed in AST is not have an Annotation Holder.
* if the passed in AST does not have an Annotation Holder.
*
* @param ast the current node
* @return the Annotation Holder
Expand All @@ -113,9 +148,8 @@ public static DetailAST getAnnotationHolder(DetailAST ast) {
}

/**
* Checks to see if the AST is annotated with
* the passed in annotation and return the AST
* representing that annotation.
* Checks if the AST is annotated with the passed in annotation
* and returns the AST representing that annotation.
*
* <p>
* This method will not look for imports or package
Expand Down Expand Up @@ -149,18 +183,37 @@ public static DetailAST getAnnotation(final DetailAST ast,
"the annotation is empty or spaces");
}

return getAnnotation(ast, annotationNode -> {
final DetailAST firstChild = annotationNode.findFirstToken(TokenTypes.AT);
final String name =
FullIdent.createFullIdent(firstChild.getNextSibling()).getText();
return annotation.equals(name);
});
}

/**
* Checks if the given AST is annotated with at least one annotation that
* matches the given predicate and returns the AST representing the first
* matching annotation.
*
* <p>
* This method will not look for imports or package
* statements to detect the passed in annotation.
* </p>
*
* @param ast the current node
* @param predicate The predicate which decides if an annotation matches
* @return the AST representing that annotation
*/
private static DetailAST getAnnotation(final DetailAST ast,
Predicate<DetailAST> predicate) {
final DetailAST holder = getAnnotationHolder(ast);
DetailAST result = null;
for (DetailAST child = holder.getFirstChild();
child != null; child = child.getNextSibling()) {
if (child.getType() == TokenTypes.ANNOTATION) {
final DetailAST firstChild = child.findFirstToken(TokenTypes.AT);
final String name =
FullIdent.createFullIdent(firstChild.getNextSibling()).getText();
if (annotation.equals(name)) {
result = child;
break;
}
if (child.getType() == TokenTypes.ANNOTATION && predicate.test(child)) {
result = child;
break;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,4 +396,44 @@ public void testBadTagSuppress() throws Exception {
expected);
}

@Test
public void testAllowedAnnotationsDefault() throws Exception {
final DefaultConfiguration checkConfig =
createModuleConfig(JavadocTypeCheck.class);

final String[] expected = {
"3: " + getCheckMessage(MSG_JAVADOC_MISSING),
"7: " + getCheckMessage(MSG_JAVADOC_MISSING),
};
verify(checkConfig,
getPath("InputJavadocTypeAllowedAnnotations.java"),
expected);
}

@Test
public void testAllowedAnnotationsAllowed() throws Exception {
final DefaultConfiguration checkConfig =
createModuleConfig(JavadocTypeCheck.class);
checkConfig.addAttribute("allowedAnnotations", "ThisIsOk");

final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verify(checkConfig,
getPath("InputJavadocTypeAllowedAnnotations.java"),
expected);
}

@Test
public void testAllowedAnnotationsNotAllowed() throws Exception {
final DefaultConfiguration checkConfig =
createModuleConfig(JavadocTypeCheck.class);
checkConfig.addAttribute("allowedAnnotations", "Override");

final String[] expected = {
"3: " + getCheckMessage(MSG_JAVADOC_MISSING),
"7: " + getCheckMessage(MSG_JAVADOC_MISSING),
};
verify(checkConfig,
getPath("InputJavadocTypeAllowedAnnotations.java"),
expected);
}
}
Loading

0 comments on commit fb5fff4

Please sign in to comment.