Issue checkstyle#3496: Enforce ReturnCount max=1 in com.puppycrawl.to…

…ols.checkstyle.checks.javadoc
timurt · May 6, 2017 · f2f16af · f2f16af
1 parent 663b64b
commit f2f16af
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 79 deletions.
diff --git a/config/suppressions.xml b/config/suppressions.xml
@@ -92,5 +92,5 @@
 
     <!-- Until https://github.com/checkstyle/checkstyle/issues/3496 -->
     <suppress id="returnCountMaxOne" files=".*[\\/]ant[\\/]|.*[\\/]filters[\\/]|.*[\\/]api[\\/]|.*[\\/]annotation[\\/]|
-        |.*[\\/]coding[\\/]|.*[\\/]indentation[\\/]|.*[\\/]javadoc[\\/]"/>
+        |.*[\\/]coding[\\/]|.*[\\/]indentation[\\/]"/>
 </suppressions>
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocMethodCheck.java
@@ -365,6 +365,7 @@ protected final void processAST(DetailAST ast) {
      * @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) {
@@ -374,11 +375,12 @@ private boolean hasAllowedAnnotations(DetailAST methodDef) {
                     .findFirstToken(TokenTypes.IDENT);
             }
             if (allowedAnnotations.contains(identNode.getText())) {
-                return true;
+                result = true;
+                break;
             }
             annotationNode = annotationNode.getNextSibling();
         }
-        return false;
+        return result;
     }
 
     /**
@@ -445,16 +447,17 @@ private boolean isContentsAllowMissingJavadoc(DetailAST ast) {
      * @return true if given method name matches the regex.
      */
     private boolean matchesSkipRegex(DetailAST methodDef) {
+        boolean result = false;
         if (ignoreMethodNamesRegex != null) {
             final DetailAST ident = methodDef.findFirstToken(TokenTypes.IDENT);
             final String methodName = ident.getText();
 
             final Matcher matcher = ignoreMethodNamesRegex.matcher(methodName);
             if (matcher.matches()) {
-                return true;
+                result = true;
             }
         }
-        return false;
+        return result;
     }
 
     /**
@@ -517,20 +520,20 @@ private void checkComment(DetailAST ast, TextBlock comment) {
      * @param tags the list of Javadoc tags associated with the construct
      * @return true if the construct has a short circuit tag.
      */
-    private boolean hasShortCircuitTag(final DetailAST ast,
-            final List<JavadocTag> tags) {
+    private boolean hasShortCircuitTag(final DetailAST ast, final List<JavadocTag> tags) {
+        boolean result = true;
         // Check if it contains {@inheritDoc} tag
-        if (tags.size() != 1
-                || !tags.get(0).isInheritDocTag()) {
-            return false;
+        if (tags.size() == 1
+                && tags.get(0).isInheritDocTag()) {
+            // Invalid if private, a constructor, or a static method
+            if (!JavadocTagInfo.INHERIT_DOC.isValidOn(ast)) {
+                log(ast, MSG_INVALID_INHERIT_DOC);
+            }
         }
-
-        // Invalid if private, a constructor, or a static method
-        if (!JavadocTagInfo.INHERIT_DOC.isValidOn(ast)) {
-            log(ast, MSG_INVALID_INHERIT_DOC);
+        else {
+            result = false;
         }
-
-        return true;
+        return result;
     }
 
     /**
@@ -543,14 +546,15 @@ private boolean hasShortCircuitTag(final DetailAST ast,
      */
     private static Scope calculateScope(final DetailAST ast) {
         final DetailAST mods = ast.findFirstToken(TokenTypes.MODIFIERS);
-        final Scope declaredScope = ScopeUtils.getScopeFromMods(mods);
+        final Scope scope;
 
         if (ScopeUtils.isInInterfaceOrAnnotationBlock(ast)) {
-            return Scope.PUBLIC;
+            scope = Scope.PUBLIC;
         }
         else {
-            return declaredScope;
+            scope = ScopeUtils.getScopeFromMods(mods);
         }
+        return scope;
     }
 
     /**

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocNodeImpl.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocNodeImpl.java
@@ -94,12 +94,11 @@ public int getColumnNumber() {
 
     @Override
     public DetailNode[] getChildren() {
-        if (children == null) {
-            return EMPTY_DETAIL_NODE_ARRAY;
-        }
-        else {
-            return Arrays.copyOf(children, children.length);
+        DetailNode[] nodeChildren = EMPTY_DETAIL_NODE_ARRAY;
+        if (children != null) {
+            nodeChildren = Arrays.copyOf(children, children.length);
         }
+        return nodeChildren;
     }
 
     @Override

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocParagraphCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocParagraphCheck.java
@@ -194,17 +194,18 @@ private static DetailNode getNearestNode(DetailNode node) {
      * @return true, if line is empty line.
      */
     private static boolean isEmptyLine(DetailNode newLine) {
+        boolean result = false;
         DetailNode previousSibling = JavadocUtils.getPreviousSibling(newLine);
-        if (previousSibling == null
-                || previousSibling.getParent().getType() != JavadocTokenTypes.JAVADOC) {
-            return false;
-        }
-        if (previousSibling.getType() == JavadocTokenTypes.TEXT
-                && previousSibling.getText().trim().isEmpty()) {
-            previousSibling = JavadocUtils.getPreviousSibling(previousSibling);
+        if (previousSibling != null
+                && previousSibling.getParent().getType() == JavadocTokenTypes.JAVADOC) {
+            if (previousSibling.getType() == JavadocTokenTypes.TEXT
+                    && previousSibling.getText().trim().isEmpty()) {
+                previousSibling = JavadocUtils.getPreviousSibling(previousSibling);
+            }
+            result = previousSibling != null
+                    && previousSibling.getType() == JavadocTokenTypes.LEADING_ASTERISK;
         }
-        return previousSibling != null
-                && previousSibling.getType() == JavadocTokenTypes.LEADING_ASTERISK;
+        return result;
     }
 
     /**
@@ -213,18 +214,20 @@ private static boolean isEmptyLine(DetailNode newLine) {
      * @return true, if line with paragraph tag is first line in javadoc.
      */
     private static boolean isFirstParagraph(DetailNode paragraphTag) {
+        boolean result = true;
         DetailNode previousNode = JavadocUtils.getPreviousSibling(paragraphTag);
         while (previousNode != null) {
             if (previousNode.getType() == JavadocTokenTypes.TEXT
                     && !previousNode.getText().trim().isEmpty()
                 || previousNode.getType() != JavadocTokenTypes.LEADING_ASTERISK
                     && previousNode.getType() != JavadocTokenTypes.NEWLINE
                     && previousNode.getType() != JavadocTokenTypes.TEXT) {
-                return false;
+                result = false;
+                break;
             }
             previousNode = JavadocUtils.getPreviousSibling(previousNode);
         }
-        return true;
+        return result;
     }
 
     /**
@@ -250,16 +253,18 @@ private static DetailNode getNearestEmptyLine(DetailNode node) {
      * @return true, if NEWLINE node is a last node in javadoc.
      */
     private static boolean isLastEmptyLine(DetailNode newLine) {
+        boolean result = true;
         DetailNode nextNode = JavadocUtils.getNextSibling(newLine);
         while (nextNode != null && nextNode.getType() != JavadocTokenTypes.JAVADOC_TAG) {
             if (nextNode.getType() == JavadocTokenTypes.TEXT
                     && !nextNode.getText().trim().isEmpty()
                     || nextNode.getType() == JavadocTokenTypes.HTML_ELEMENT) {
-                return false;
+                result = false;
+                break;
             }
             nextNode = JavadocUtils.getNextSibling(nextNode);
         }
-        return true;
+        return result;
     }
 
     /**

diff --git a/...om/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTagContinuationIndentationCheck.java b/...om/puppycrawl/tools/checkstyle/checks/javadoc/JavadocTagContinuationIndentationCheck.java
@@ -129,13 +129,15 @@ private static List<DetailNode> getAllNewlineNodes(DetailNode descriptionNode) {
      * @return true, if description node is a description of in-line tag.
      */
     private static boolean isInlineDescription(DetailNode description) {
+        boolean isInline = false;
         DetailNode inlineTag = description.getParent();
         while (inlineTag != null) {
             if (inlineTag.getType() == JavadocTokenTypes.JAVADOC_INLINE_TAG) {
-                return true;
+                isInline = true;
+                break;
             }
             inlineTag = inlineTag.getParent();
         }
-        return false;
+        return isInline;
     }
 }
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocVariableCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocVariableCheck.java
@@ -130,31 +130,21 @@ private boolean isIgnored(DetailAST ast) {
      * @return whether we should check a given node.
      */
     private boolean shouldCheck(final DetailAST ast) {
-        if (ScopeUtils.isInCodeBlock(ast) || isIgnored(ast)) {
-            return false;
-        }
-
-        final Scope customScope;
-        if (ast.getType() == TokenTypes.ENUM_CONSTANT_DEF) {
-            customScope = Scope.PUBLIC;
-        }
-        else {
-            final DetailAST mods = ast.findFirstToken(TokenTypes.MODIFIERS);
-            final Scope declaredScope = ScopeUtils.getScopeFromMods(mods);
-
-            if (ScopeUtils.isInInterfaceOrAnnotationBlock(ast)) {
-                customScope = Scope.PUBLIC;
+        boolean result = false;
+        if (!ScopeUtils.isInCodeBlock(ast) && !isIgnored(ast)) {
+            Scope customScope = Scope.PUBLIC;
+            if (ast.getType() != TokenTypes.ENUM_CONSTANT_DEF
+                    && !ScopeUtils.isInInterfaceOrAnnotationBlock(ast)) {
+                final DetailAST mods = ast.findFirstToken(TokenTypes.MODIFIERS);
+                customScope = ScopeUtils.getScopeFromMods(mods);
             }
-            else {
-                customScope = declaredScope;
-            }
-        }
 
-        final Scope surroundingScope = ScopeUtils.getSurroundingScope(ast);
-
-        return customScope.isIn(scope) && surroundingScope.isIn(scope)
-            && (excludeScope == null
-                || !customScope.isIn(excludeScope)
-                || !surroundingScope.isIn(excludeScope));
+            final Scope surroundingScope = ScopeUtils.getSurroundingScope(ast);
+            result = customScope.isIn(scope) && surroundingScope.isIn(scope)
+                && (excludeScope == null
+                    || !customScope.isIn(excludeScope)
+                    || !surroundingScope.isIn(excludeScope));
+        }
+        return result;
     }
 }
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/TagParser.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/TagParser.java
@@ -169,28 +169,29 @@ private static boolean isTag(String[] javadocText, Point pos) {
      * @return id for given tag
      */
     private static String getTagId(String[] javadocText, Point tagStart) {
+        String tagId = "";
         int column = tagStart.getColumnNo() + 1;
         String text = javadocText[tagStart.getLineNo()];
-        if (column >= text.length()) {
-            return "";
-        }
+        if (column < text.length()) {
 
-        if (text.charAt(column) == '/') {
-            column++;
-        }
+            if (text.charAt(column) == '/') {
+                column++;
+            }
 
-        text = text.substring(column).trim();
-        int position = 0;
+            text = text.substring(column).trim();
+            int position = 0;
 
-        //Character.isJavaIdentifier... may not be a valid HTML
-        //identifier but is valid for generics
-        while (position < text.length()
-            && (Character.isJavaIdentifierStart(text.charAt(position))
-                || Character.isJavaIdentifierPart(text.charAt(position)))) {
-            position++;
-        }
+            //Character.isJavaIdentifier... may not be a valid HTML
+            //identifier but is valid for generics
+            while (position < text.length()
+                    && (Character.isJavaIdentifierStart(text.charAt(position))
+                        || Character.isJavaIdentifierPart(text.charAt(position)))) {
+                position++;
+            }
 
-        return text.substring(0, position);
+            tagId = text.substring(0, position);
+        }
+        return tagId;
     }
 
     /**