DevTools: [JsDocValidator] Check validity of @param and @return annotations

Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/MethodAnnotationChecker.java

Index: Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/MethodAnnotationChecker.java
diff --git a/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/MethodAnnotationChecker.java b/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/MethodAnnotationChecker.java
index 4ec6b728b398d26a90c76d3e4adabf0d1719a3d7..ad0f5d1e075cc4b6c520517ab574d0d0bea86ddf 100644
--- a/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/MethodAnnotationChecker.java
+++ b/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/MethodAnnotationChecker.java
@@ -17,7 +17,10 @@ import java.util.regex.Pattern;
 public final class MethodAnnotationChecker extends ContextTrackingChecker {
 
     private static final Pattern PARAM_PATTERN =
-            Pattern.compile("@param\\s+\\{.+\\}\\s+([^\\s]+)(?:[^}]*)$", Pattern.MULTILINE);
+            Pattern.compile("@param\\s+(\\{.+\\}\\s+)?([^\\s]+).*$", Pattern.MULTILINE);

[aandrey, 2014/06/05 15:03:16]

\\s+  ->  \\s
(let's be more strict and allow exactly 1 space?)

[apavlov, 2014/06/05 15:31:51]

Technically, this is a valid annotation understood by Closure. My goal is to
detect annotations that look valid to the author but not to the compiler.

+
+    private static final Pattern INVALID_RETURN_PATTERN =
+            Pattern.compile("@return(?:s.*|\\s+[^{]*)$", Pattern.MULTILINE);

[aandrey, 2014/06/05 15:03:16]

"@return(?:s|\\s[^{]|\\s*$)"

this will also handle following cases:

"@return"
"@return value {number}"

[apavlov, 2014/06/05 15:31:51]

Right, this is INVALID_RETURN_PATTERN :)

 
     private final Set<FunctionRecord> valueReturningFunctions = new HashSet<>();
     private final Set<FunctionRecord> throwingFunctions = new HashSet<>();
@@ -68,8 +71,14 @@ public final class MethodAnnotationChecker extends ContextTrackingChecker {
         String jsDoc = getContext().getNodeText(jsDocNode);
         Matcher m = PARAM_PATTERN.matcher(jsDoc);
         while (m.find()) {
-            String jsDocParam = m.group(1);
-            paramNames.remove(jsDocParam);
+            String paramType = m.group(1);
+            if (paramType == null) {
+                getContext().reportErrorInNode(jsDocNode, m.start(2), String.format(
+                        "Invalid @param annotation found -"
+                        + " should be \"@param {<type>} paramName\""));
+            } else {
+                paramNames.remove(m.group(2));
+            }
         }
         return paramNames.toArray(new String[paramNames.size()]);
     }
@@ -130,13 +139,14 @@ public final class MethodAnnotationChecker extends ContextTrackingChecker {
         boolean isInterfaceFunction =
                 function.enclosingType != null && function.enclosingType.isInterface;
         int invalidAnnotationIndex =
-                invalidReturnsAnnotationIndex(getState().getNodeText(jsDocNode));
+                invalidReturnAnnotationIndex(getState().getNodeText(jsDocNode));
         if (invalidAnnotationIndex != -1) {
             String suggestedResolution = (isReturningFunction || isInterfaceFunction)
-                    ? "should be @return instead"
+                    ? "should be \"@return {<type>}\""
                     : "please remove, as function does not return value";
             getContext().reportErrorInNode(jsDocNode, invalidAnnotationIndex,
-                    String.format("invalid @returns annotation found - %s", suggestedResolution));
+                    String.format(
+                            "invalid return type annotation found - %s", suggestedResolution));
             return;
         }
         AstNode functionNameNode = getFunctionNameNode(function.functionNode);
@@ -172,8 +182,12 @@ public final class MethodAnnotationChecker extends ContextTrackingChecker {
         return nameNode == null ? null : getState().getNodeText(nameNode);
     }
 
-    private static int invalidReturnsAnnotationIndex(String jsDoc) {
-        return jsDoc == null ? -1 : jsDoc.indexOf("@returns");
+    private static int invalidReturnAnnotationIndex(String jsDoc) {
+        if (jsDoc == null) {
+            return -1;
+        }
+        Matcher m = INVALID_RETURN_PATTERN.matcher(jsDoc);
+        return m.find() ? m.start() : -1;
     }
 
     private static AstNode getFunctionNameNode(FunctionNode functionNode) {