Repository: google/closure-compiler
Pull request #1006 (2015-06-16 14:40:15)

Add CheckJSDoc pass to verify annotations are used appropriately
Moves most JSDoc warnings out of IRFactory and makes them supressible.

It seems like the diagnostic groups related to JSDocs should be cleaned up:

* deprecatedAnnotations
* misplacedTypeAnnotation

Perhaps combine to `invalidJsDoc`?



ChadKillingsworth: Fixes #984 
dimvar: Looks good. I left some comments.
ChadKillingsworth: Fixed most of these, but had some followup questions.

Also, what do you think about combining the diagnostic groups to simply `invalidJsDoc`?
dimvar: > what do you think about combining the diagnostic groups to simply invalidJsDoc?

No, let's leave them separate. I can see people wanting to ignore the deprecation warnings, but keep the misplaced jsdoc warnings.

The misplaced jsdoc warnings are our version of parse errors, for the syntax we'd like to have but doesn't exist in JS.

Patch:
diff --git a/src/com/google/javascript/jscomp/CheckJSDoc.java b/src/com/google/javascript/jscomp/CheckJSDoc.java
new file mode 100644
index 000000000..12f2a863e
--- /dev/null
+++ b/src/com/google/javascript/jscomp/CheckJSDoc.java
@@ -0,0 +1,253 @@
+/*
+ * Copyright 2015 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.javascript.jscomp;
+
+import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
+import com.google.javascript.rhino.JSDocInfo;
+import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.Token;
+
+/**
+ * Checks for misplaced, misused or deprecated JSDoc annotations.
+ *
+ * @author chadkillingsworth@gmail.com (Chad Killingsworth)
+ */
+final class CheckJSDoc implements CompilerPass {
+
+  public static final DiagnosticType MISPLACED_ANNOTATION =
+      DiagnosticType.warning("JSC_MISPLACED_ANNOTATION",
+          "Misplaced {0} annotation.{1}");
+
+  public static final DiagnosticType ANNOTATION_DEPRECATED =
+      DiagnosticType.warning("JSC_ANNOTATION_DEPRECATED",
+          "The {0} annotation is deprecated.{1}");
+
+  private final AbstractCompiler compiler;
+
+  CheckJSDoc(AbstractCompiler compiler) {
+    this.compiler = compiler;
+  }
+
+  @Override
+  public void process(Node externs, Node root) {
+    NodeTraversal.traverse(compiler, externs, new ProcessNodes());
+    NodeTraversal.traverse(compiler, root, new ProcessNodes());
+  }
+
+  final class ProcessNodes extends AbstractPostOrderCallback {
+
+    @Override
+    public void visit(NodeTraversal t, Node n, Node parent) {
+      JSDocInfo info = n.getJSDocInfo();
+      validateTypeAnnotations(t, n, info);
+      validateFunctionJsDoc(t, n, info);
+      validateMsgJsDoc(t, n, info);
+      validateDeprecatedJsDoc(t, n, info);
+      validateNoCollapse(t, n, info);
+    }
+
+    /**
+     * Checks that deprecated annotations such as @expose are not present
+     */
+    private void validateDeprecatedJsDoc(NodeTraversal t, Node n,
+        JSDocInfo info) {
+      if (info != null && info.isExpose()) {
+        t.getCompiler().report(
+            t.makeError(n, ANNOTATION_DEPRECATED, "@expose",
+                " Use @nocollapse or @export instead."));
+      }
+    }
+
+    /**
+     * Warns when nocollapse annotations are pressent on nodes
+     * which are not eligible for property collapsing .
+     */
+    private void validateNoCollapse(NodeTraversal t, Node n,
+        JSDocInfo info) {
+      if (n.isFromExterns()) {
+        if (info != null && info.isNoCollapse()) {
+          // @nocollapse has no effect in externs
+          t.getCompiler().report(t.makeError(n, MISPLACED_ANNOTATION,
+              "@nocollapse", "This JSDoc has no effect in externs."));
+        }
+        return;
+      }
+      JSDocInfo bestInfo = NodeUtil.getBestJSDocInfo(n);
+      if (bestInfo != null && bestInfo.isNoCollapse() &&
+          (n.getType() == Token.GETELEM || n.getType() == Token.GETPROP) &&
+          NodeUtil.isPrototypeProperty(n)) {
+        t.getCompiler().report(t.makeError(n, MISPLACED_ANNOTATION,
+            "@nocollapse", "This JSDoc has no effect on prototype"
+                + "properties."));
+      }
+    }
+
+    /**
+     * Checks that JSDoc intended for a function is actually attached to a
+     * function.
+     */
+    private void validateFunctionJsDoc(NodeTraversal t, Node n,
+        JSDocInfo info) {
+      if (info == null) {
+        return;
+      }
+
+      if (info.containsFunctionDeclaration() && !info.hasType()) {
+        // This JSDoc should be attached to a FUNCTION node, or an assignment
+        // with a function as the RHS, etc.
+        switch (n.getType()) {
+          case Token.FUNCTION:
+          case Token.VAR:
+          case Token.LET:
+          case Token.CONST:
+          case Token.GETTER_DEF:
+          case Token.SETTER_DEF:
+          case Token.MEMBER_FUNCTION_DEF:
+          case Token.STRING_KEY:
+          case Token.EXPORT:
+            return;
+          case Token.GETELEM:
+          case Token.GETPROP:
+            if (n.getFirstChild().isQualifiedName()) {
+              return;
+            }
+            break;
+          case Token.ASSIGN: {
+            // TODO(tbreisacher): Check that the RHS of the assignment is a
+            // function. Note that it can be a FUNCTION node, but it can also be
+            // a call to goog.abstractMethod, goog.functions.constant, etc.
+            return;
+          }
+        }
+        t.getCompiler().report(t.makeError(n, MISPLACED_ANNOTATION,
+            "function", "This JSDoc is not attached to a function node. "
+                + "Are you missing parentheses?"));
+      }
+    }
+
+    /**
+     * Checks that annotations for messages ({@code @desc}, {@code @hidden},
+     * and {@code @meaning})
+     * are in the proper place, namely on names starting with MSG_ which
+     * indicates they should be
+     * extracted for translation. A later pass checks that the right side is
+     * a call to goog.getMsg.
+     */
+    private void validateMsgJsDoc(NodeTraversal t, Node n,
+        JSDocInfo info) {
+      if (info == null) {
+        return;
+      }
+
+      if (info.getDescription() != null || info.isHidden() || info.getMeaning() != null) {
+        boolean descOkay = false;
+        switch (n.getType()) {
+          case Token.ASSIGN: {
+            Node lhs = n.getFirstChild();
+            if (lhs.isName()) {
+              descOkay = lhs.getString().startsWith("MSG_");
+            } else if (lhs.isQualifiedName()) {
+              descOkay = lhs.getLastChild().getString().startsWith("MSG_");
+            }
+            break;
+          }
+          case Token.VAR:
+          case Token.LET:
+          case Token.CONST:
+            descOkay = n.getFirstChild().getString().startsWith("MSG_");
+            break;
+          case Token.STRING_KEY:
+            descOkay = n.getString().startsWith("MSG_");
+            break;
+        }
+        if (!descOkay) {
+          t.getCompiler().report(t.makeError(n, MISPLACED_ANNOTATION,
+              "message", " @desc, @hidden, and @meaning annotations should only"
+                  + "be on message nodes."));
+        }
+      }
+    }
+
+    /**
+     * Check that JSDoc with a {@code @type} annotation is in a valid place.
+     */
+    private void validateTypeAnnotations(NodeTraversal t, Node n,
+        JSDocInfo info) {
+      if (info != null && info.hasType()) {
+        boolean valid = false;
+        switch (n.getType()) {
+          // Casts, variable declarations, and exports are valid.
+          case Token.CAST:
+          case Token.VAR:
+          case Token.LET:
+          case Token.CONST:
+          case Token.EXPORT:
+            valid = true;
+            break;
+          // Function declarations are valid
+          case Token.FUNCTION:
+            valid = NodeUtil.isFunctionDeclaration(n);
+            break;
+          // Object literal properties, catch declarations and variable
+          // initializers are valid.
+          case Token.NAME:
+          case Token.DEFAULT_VALUE:
+            Node parent = n.getParent();
+            switch (parent.getType()) {
+              case Token.STRING_KEY:
+              case Token.GETTER_DEF:
+              case Token.SETTER_DEF:
+              case Token.CATCH:
+              case Token.FUNCTION:
+              case Token.VAR:
+              case Token.LET:
+              case Token.CONST:
+              case Token.PARAM_LIST:
+                valid = true;
+                break;
+            }
+            break;
+          // Object literal properties are valid
+          case Token.STRING_KEY:
+          case Token.GETTER_DEF:
+          case Token.SETTER_DEF:
+            valid = true;
+            break;
+          // Property assignments are valid, if at the root of an expression.
+          case Token.ASSIGN:
+            valid = n.getParent().isExprResult()
+                && (n.getFirstChild().isGetProp() || n.getFirstChild().isGetElem());
+            break;
+          case Token.GETPROP:
+            valid = n.getParent().isExprResult() && n.isQualifiedName();
+            break;
+          case Token.CALL:
+            valid = info.isDefine();
+            break;
+          default:
+            break;
+        }
+
+        if (!valid) {
+          t.getCompiler().report(t.makeError(n, MISPLACED_ANNOTATION,
+              "type", "Type annotations are not allowed here. "
+                  + "Are you missing parentheses?"));
+        }
+      }
+    }
+  }
+}
diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java
index 4a3d8e228..9d8223ae9 100644
--- a/src/com/google/javascript/jscomp/DefaultPassConfig.java
+++ b/src/com/google/javascript/jscomp/DefaultPassConfig.java
@@ -32,7 +32,6 @@ import com.google.javascript.jscomp.ExtractPrototypeMemberDeclarations.Pattern;
 import com.google.javascript.jscomp.NodeTraversal.Callback;
 import com.google.javascript.jscomp.lint.CheckEnums;
 import com.google.javascript.jscomp.lint.CheckInterfaces;
-import com.google.javascript.jscomp.lint.CheckJSDoc;
 import com.google.javascript.jscomp.lint.CheckNullableReturn;
 import com.google.javascript.jscomp.lint.CheckPrototypeProperties;
 import com.google.javascript.jscomp.parsing.ParserRunner;
@@ -198,6 +197,9 @@ public final class DefaultPassConfig extends PassConfig {
 
     checks.add(createEmptyPass("beforeStandardChecks"));
 
+    // Verify JsDoc annotations
+    checks.add(checkJsDoc);
+
     if (options.closurePass) {
       checks.add(closureRewriteModule);
     }
@@ -944,6 +946,14 @@ public final class DefaultPassConfig extends PassConfig {
           "Exports can only be generated if export symbol/property " +
           "functions are set.");
 
+  /** Verifies JSDoc annotations are used properly. */
+  private final PassFactory checkJsDoc = new PassFactory("checkJsDoc", true) {
+    @Override
+    protected CompilerPass create(AbstractCompiler compiler) {
+      return new CheckJSDoc(compiler);
+    }
+  };
+
   /** Generates exports for @export annotations. */
   private final PassFactory generateExports = new PassFactory("generateExports", true) {
     @Override
@@ -1512,7 +1522,7 @@ public final class DefaultPassConfig extends PassConfig {
       return combineChecks(compiler, ImmutableList.<Callback>of(
           new CheckEnums(compiler),
           new CheckInterfaces(compiler),
-          new CheckJSDoc(compiler),
+          new com.google.javascript.jscomp.lint.CheckJSDoc(compiler),
           new CheckNullableReturn(compiler),
           new CheckPrototypeProperties(compiler),
           new ImplicitNullabilityCheck(compiler)));
diff --git a/src/com/google/javascript/jscomp/DiagnosticGroups.java b/src/com/google/javascript/jscomp/DiagnosticGroups.java
index 11123f969..046623550 100644
--- a/src/com/google/javascript/jscomp/DiagnosticGroups.java
+++ b/src/com/google/javascript/jscomp/DiagnosticGroups.java
@@ -20,7 +20,6 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.javascript.jscomp.lint.CheckEnums;
 import com.google.javascript.jscomp.lint.CheckInterfaces;
-import com.google.javascript.jscomp.lint.CheckJSDoc;
 import com.google.javascript.jscomp.lint.CheckNullableReturn;
 import com.google.javascript.jscomp.lint.CheckPrototypeProperties;
 import com.google.javascript.jscomp.newtypes.JSTypeCreatorFromJSDoc;
@@ -396,11 +395,9 @@ public class DiagnosticGroups {
       DiagnosticGroups.registerGroup("duplicateMessage",
           JsMessageVisitor.MESSAGE_DUPLICATE_KEY);
 
-  public static final DiagnosticGroup MISPLACED_TYPE_ANNOTATION =
-      DiagnosticGroups.registerGroup("misplacedTypeAnnotation",
-          RhinoErrorReporter.MISPLACED_TYPE_ANNOTATION,
-          RhinoErrorReporter.MISPLACED_FUNCTION_ANNOTATION,
-          RhinoErrorReporter.MISPLACED_MSG_ANNOTATION);
+  public static final DiagnosticGroup MISPLACED_ANNOTATION =
+      DiagnosticGroups.registerGroup("misplacedAnnotation",
+          CheckJSDoc.MISPLACED_ANNOTATION);
 
   public static final DiagnosticGroup SUSPICIOUS_CODE =
       DiagnosticGroups.registerGroup("suspiciousCode",
@@ -411,7 +408,7 @@ public class DiagnosticGroups {
 
   public static final DiagnosticGroup DEPRECATED_ANNOTATIONS =
       DiagnosticGroups.registerGroup("deprecatedAnnotations",
-          RhinoErrorReporter.ANNOTATION_DEPRECATED);
+          CheckJSDoc.ANNOTATION_DEPRECATED);
 
   // These checks are not intended to be enabled as errors. It is
   // recommended that you think of them as "linter" warnings that
@@ -423,10 +420,10 @@ public class DiagnosticGroups {
           // checkTypes DiagnosticGroup
           CheckInterfaces.INTERFACE_FUNCTION_NOT_EMPTY,
           CheckInterfaces.INTERFACE_SHOULD_NOT_TAKE_ARGS,
-          CheckJSDoc.MISSING_PARAM_JSDOC,
-          CheckJSDoc.MUST_BE_PRIVATE,
-          CheckJSDoc.OPTIONAL_NAME_NOT_MARKED_OPTIONAL,
-          CheckJSDoc.OPTIONAL_TYPE_NOT_USING_OPTIONAL_NAME,
+          com.google.javascript.jscomp.lint.CheckJSDoc.MISSING_PARAM_JSDOC,
+          com.google.javascript.jscomp.lint.CheckJSDoc.MUST_BE_PRIVATE,
+          com.google.javascript.jscomp.lint.CheckJSDoc.OPTIONAL_NAME_NOT_MARKED_OPTIONAL,
+          com.google.javascript.jscomp.lint.CheckJSDoc.OPTIONAL_TYPE_NOT_USING_OPTIONAL_NAME,
           CheckNullableReturn.NULLABLE_RETURN,
           CheckNullableReturn.NULLABLE_RETURN_WITH_NAME,
           CheckPrototypeProperties.ILLEGAL_PROTOTYPE_MEMBER,
diff --git a/src/com/google/javascript/jscomp/RhinoErrorReporter.java b/src/com/google/javascript/jscomp/RhinoErrorReporter.java
index 79a44abfe..1f6bfdfe5 100644
--- a/src/com/google/javascript/jscomp/RhinoErrorReporter.java
+++ b/src/com/google/javascript/jscomp/RhinoErrorReporter.java
@@ -57,19 +57,6 @@ class RhinoErrorReporter {
   static final DiagnosticType JSDOC_IN_BLOCK_COMMENT =
       DiagnosticType.warning("JSC_JSDOC_IN_BLOCK_COMMENT", "Parse error. {0}");
 
-  static final DiagnosticType MISPLACED_TYPE_ANNOTATION =
-      DiagnosticType.warning("JSC_MISPLACED_TYPE_ANNOTATION",
-          "Type annotations are not allowed here. " +
-          "Are you missing parentheses?");
-
-  static final DiagnosticType MISPLACED_FUNCTION_ANNOTATION =
-      DiagnosticType.warning("JSC_MISPLACED_FUNCTION_ANNOTATION",
-          "Misplaced function annotation.");
-
-  static final DiagnosticType MISPLACED_MSG_ANNOTATION =
-      DiagnosticType.disabled("JSC_MISPLACED_MSG_ANNOTATION",
-          "Misplaced message annotation.");
-
   static final DiagnosticType INVALID_ES3_PROP_NAME = DiagnosticType.warning(
       "JSC_INVALID_ES3_PROP_NAME",
       "Keywords and reserved words are not allowed as unquoted property " +
@@ -95,9 +82,6 @@ class RhinoErrorReporter {
           "{0}. Use --language_in=ECMASCRIPT6_TYPED " +
           "to enable ES6 typed features.");
 
-  static final DiagnosticType ANNOTATION_DEPRECATED =
-      DiagnosticType.warning("JSC_ANNOTATION_DEPRECATED", "{0}");
-
   // A map of Rhino messages to their DiagnosticType.
   private final Map<Pattern, DiagnosticType> typeMap;
 
@@ -138,18 +122,6 @@ class RhinoErrorReporter {
             "Did you mean to start it with '/**'?\\E"),
             JSDOC_IN_BLOCK_COMMENT)
 
-        // Unexpected @type annotations
-        .put(Pattern.compile("^Type annotations are not allowed here.*"),
-            MISPLACED_TYPE_ANNOTATION)
-
-        // Unexpected function JsDoc
-        .put(Pattern.compile("^This JSDoc is not attached to a function node.*"),
-            MISPLACED_FUNCTION_ANNOTATION)
-
-        // Unexpected @desc JsDoc
-        .put(Pattern.compile("^@desc, @hidden, and @meaning annotations.*"),
-            MISPLACED_MSG_ANNOTATION)
-
         .put(Pattern.compile("^Keywords and reserved words" +
             " are not allowed as unquoted property.*"),
             INVALID_ES3_PROP_NAME)
@@ -173,10 +145,6 @@ class RhinoErrorReporter {
         .put(Pattern.compile("^type syntax is only supported in ES6 typed mode.*"),
             ES6_TYPED)
 
-        // Deprecated annotations
-        .put(Pattern.compile("^The @[a-z]+ annotation is deprecated\\..*"),
-            ANNOTATION_DEPRECATED)
-
         .build();
   }
 
diff --git a/src/com/google/javascript/jscomp/parsing/IRFactory.java b/src/com/google/javascript/jscomp/parsing/IRFactory.java
index 8607d7e66..728ee6cd0 100644
--- a/src/com/google/javascript/jscomp/parsing/IRFactory.java
+++ b/src/com/google/javascript/jscomp/parsing/IRFactory.java
@@ -161,15 +161,6 @@ class IRFactory {
       "Non-JSDoc comment has annotations. " +
       "Did you mean to start it with '/**'?";
 
-  static final String MISPLACED_TYPE_ANNOTATION =
-      "Type annotations are not allowed here. Are you missing parentheses?";
-
-  static final String MISPLACED_FUNCTION_ANNOTATION =
-      "This JSDoc is not attached to a function node. Are you missing parentheses?";
-
-  static final String MISPLACED_MSG_ANNOTATION =
-      "@desc, @hidden, and @meaning annotations should only be on message nodes.";
-
   static final String INVALID_ES3_PROP_NAME =
       "Keywords and reserved words are not allowed as unquoted property " +
       "names in older versions of JavaScript. " +
@@ -216,9 +207,6 @@ class IRFactory {
 
   static final String UNDEFINED_LABEL = "undefined label \"%s\"";
 
-  static final String ANNOTATION_DEPRECATED =
-      "The %s annotation is deprecated.%s";
-
   private final String sourceString;
   private final List<Integer> newlines;
   private final StaticSourceFile sourceFile;
@@ -358,7 +346,6 @@ class IRFactory {
   }
 
   private void validate(Node n) {
-    validateJsDoc(n);
     validateParameters(n);
     validateBreakContinue(n);
     validateReturn(n);
@@ -483,68 +470,6 @@ class IRFactory {
     }
   }
 
-  private void validateJsDoc(Node n) {
-    validateTypeAnnotations(n);
-    validateFunctionJsDoc(n);
-    validateMsgJsDoc(n);
-    validateDeprecatedJsDoc(n);
-  }
-
-  /**
-   * Checks that deprecated annotations such as @expose are not present
-   */
-  private void validateDeprecatedJsDoc(Node n) {
-    JSDocInfo info = n.getJSDocInfo();
-    if (info == null) {
-      return;
-    }
-    if (info.isExpose()) {
-      errorReporter.warning(
-          String.format(ANNOTATION_DEPRECATED, "@expose",
-              " Use @nocollapse or @export instead."),
-          sourceName,
-          n.getLineno(), n.getCharno());
-    }
-  }
-
-  /**
-   * Checks that annotations for messages ({@code @desc}, {@code @hidden}, and {@code @meaning})
-   * are in the proper place, namely on names starting with MSG_ which indicates they should be
-   * extracted for translation. A later pass checks that the right side is a call to goog.getMsg.
-   */
-  private void validateMsgJsDoc(Node n) {
-    JSDocInfo info = n.getJSDocInfo();
-    if (info == null) {
-      return;
-    }
-    if (info.getDescription() != null || info.isHidden() || info.getMeaning() != null) {
-      boolean descOkay = false;
-      switch (n.getType()) {
-        case Token.ASSIGN: {
-          Node lhs = n.getFirstChild();
-          if (lhs.isName()) {
-            descOkay = lhs.getString().startsWith("MSG_");
-          } else if (lhs.isQualifiedName()) {
-            descOkay = lhs.getLastChild().getString().startsWith("MSG_");
-          }
-          break;
-        }
-        case Token.VAR:
-        case Token.LET:
-        case Token.CONST:
-          descOkay = n.getFirstChild().getString().startsWith("MSG_");
-          break;
-        case Token.STRING_KEY:
-          descOkay = n.getString().startsWith("MSG_");
-          break;
-      }
-      if (!descOkay) {
-        errorReporter.warning(MISPLACED_MSG_ANNOTATION,
-            sourceName, n.getLineno(), n.getCharno());
-      }
-    }
-  }
-
   private JSDocInfo recordJsDoc(SourceRange location, JSDocInfo info) {
     if (info != null && info.hasTypeInformation()) {
       hasJsDocTypeAnnotations = true;
@@ -566,111 +491,6 @@ class IRFactory {
     }
   }
 
-  /**
-   * Checks that JSDoc intended for a function is actually attached to a
-   * function.
-   */
-  private void validateFunctionJsDoc(Node n) {
-    JSDocInfo info = n.getJSDocInfo();
-    if (info == null) {
-      return;
-    }
-    if (info.containsFunctionDeclaration() && !info.hasType()) {
-      // This JSDoc should be attached to a FUNCTION node, or an assignment
-      // with a function as the RHS, etc.
-      switch (n.getType()) {
-        case Token.FUNCTION:
-        case Token.VAR:
-        case Token.LET:
-        case Token.CONST:
-        case Token.GETTER_DEF:
-        case Token.SETTER_DEF:
-        case Token.MEMBER_FUNCTION_DEF:
-        case Token.STRING_KEY:
-        case Token.EXPORT:
-          return;
-        case Token.GETELEM:
-        case Token.GETPROP:
-          if (n.getFirstChild().isQualifiedName()) {
-            return;
-          }
-          break;
-        case Token.ASSIGN: {
-          // TODO(tbreisacher): Check that the RHS of the assignment is a
-          // function. Note that it can be a FUNCTION node, but it can also be
-          // a call to goog.abstractMethod, goog.functions.constant, etc.
-          return;
-        }
-      }
-      errorReporter.warning(MISPLACED_FUNCTION_ANNOTATION,
-          sourceName,
-          n.getLineno(), n.getCharno());
-    }
-  }
-
-  /**
-   * Check that JSDoc with a {@code @type} annotation is in a valid place.
-   */
-  @SuppressWarnings("incomplete-switch")
-  private void validateTypeAnnotations(Node n) {
-    JSDocInfo info = n.getJSDocInfo();
-    if (info != null && info.hasType()) {
-      boolean valid = false;
-      switch (n.getType()) {
-        // Function declarations are valid
-        case Token.FUNCTION:
-          valid = isFunctionDeclaration(n);
-          break;
-        // Object literal properties, catch declarations and variable
-        // initializers are valid.
-        case Token.NAME:
-        case Token.DEFAULT_VALUE:
-          Node parent = n.getParent();
-          switch (parent.getType()) {
-            case Token.STRING_KEY:
-            case Token.GETTER_DEF:
-            case Token.SETTER_DEF:
-            case Token.CATCH:
-            case Token.FUNCTION:
-            case Token.VAR:
-            case Token.LET:
-            case Token.CONST:
-            case Token.PARAM_LIST:
-              valid = true;
-              break;
-          }
-          break;
-        // Casts, variable declarations, exports, and object literal properties are all valid.
-        case Token.CAST:
-        case Token.VAR:
-        case Token.LET:
-        case Token.CONST:
-        case Token.EXPORT:
-        case Token.STRING_KEY:
-        case Token.GETTER_DEF:
-        case Token.SETTER_DEF:
-          valid = true;
-          break;
-        // Property assignments are valid, if at the root of an expression.
-        case Token.ASSIGN:
-          valid =
-              n.getParent().isExprResult()
-                  && (n.getFirstChild().isGetProp() || n.getFirstChild().isGetElem());
-          break;
-        case Token.GETPROP:
-          valid = n.getParent().isExprResult() && n.isQualifiedName();
-          break;
-        case Token.CALL:
-          valid = info.isDefine();
-          break;
-      }
-
-      if (!valid) {
-        errorReporter.warning(MISPLACED_TYPE_ANNOTATION, sourceName, n.getLineno(), n.getCharno());
-      }
-    }
-  }
-
   private void validateLabel(Node n) {
     if (n.isLabel()) {
       Node labelName = n.getFirstChild();


Test patch:
diff --git a/test/com/google/javascript/jscomp/CheckJsDocTest.java b/test/com/google/javascript/jscomp/CheckJsDocTest.java
new file mode 100644
index 000000000..366557fda
--- /dev/null
+++ b/test/com/google/javascript/jscomp/CheckJsDocTest.java
@@ -0,0 +1,140 @@
+/*
+ * Copyright 2015 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.javascript.jscomp;
+
+import static com.google.javascript.jscomp.CheckJSDoc.ANNOTATION_DEPRECATED;
+import static com.google.javascript.jscomp.CheckJSDoc.MISPLACED_ANNOTATION;
+
+/**
+ * Tests for {@link CheckJSDoc}.
+ *
+ * @author chadkillingsworth@gmail.com (Chad Killingsworth)
+ */
+
+public final class CheckJsDocTest extends CompilerTestCase {
+
+  @Override
+  protected CompilerPass getProcessor(final Compiler compiler) {
+    return new CheckJSDoc(compiler);
+  }
+
+  public void testSameWithWarning(String src, DiagnosticType warning) {
+    test(src, src, null, warning);
+  }
+
+  public void testExposeDeprecated() {
+    testSameWithWarning("/** @expose */ var x = 0;", ANNOTATION_DEPRECATED);
+  }
+
+  public void testJSDocFunctionNodeAttachment() {
+    testSameWithWarning("var a = /** @param {number} index */5;"
+        + "/** @return boolean */function f(index){}", MISPLACED_ANNOTATION);
+  }
+
+  public void testJSDocDescAttachment() {
+    testSameWithWarning("function f() { " +
+            "  return /** @type {string} */ (g(1 /** @desc x */));" +
+            "};", MISPLACED_ANNOTATION);
+
+    testSameWithWarning("/** @desc Foo. */ var bar = goog.getMsg('hello');",
+        MISPLACED_ANNOTATION);
+    testSameWithWarning("/** @desc Foo. */ x.y.z.bar = goog.getMsg('hello');",
+        MISPLACED_ANNOTATION);
+    testSameWithWarning("var msgs = {/** @desc x */ x: goog.getMsg('x')}",
+        MISPLACED_ANNOTATION);
+    testSameWithWarning("/** @desc Foo. */ bar = goog.getMsg('x');",
+        MISPLACED_ANNOTATION);
+  }
+
+  public void testJSDocTypeAttachment() {
+    testSameWithWarning("function f() { " +
+        "  /** @type {string} */" +
+        "  if (true) return;" +
+        "};", MISPLACED_ANNOTATION);
+
+    testSameWithWarning("function f() { " +
+        "  /** @type {string} */" +
+        "  return;" +
+        "};", MISPLACED_ANNOTATION);
+  }
+
+
+  public void testMisplacedTypeAnnotation1() {
+    // misuse with COMMA
+    testSameWithWarning(
+        "var o = {};" +
+            "/** @type {string} */ o.prop1 = 1, o.prop2 = 2;",
+        MISPLACED_ANNOTATION);
+  }
+
+  public void testMisplacedTypeAnnotation2() {
+    // missing parentheses for the cast.
+    testSameWithWarning(
+        "var o = /** @type {string} */ getValue();",
+        MISPLACED_ANNOTATION);
+  }
+
+  public void testMisplacedTypeAnnotation3() {
+    // missing parentheses for the cast.
+    testSameWithWarning(
+        "var o = 1 + /** @type {string} */ value;",
+        MISPLACED_ANNOTATION);
+  }
+
+  public void testMisplacedTypeAnnotation4() {
+    // missing parentheses for the cast.
+    testSameWithWarning(
+        "var o = /** @type {!Array.<string>} */ ['hello', 'you'];",
+        MISPLACED_ANNOTATION);
+  }
+
+  public void testMisplacedTypeAnnotation5() {
+    // missing parentheses for the cast.
+    testSameWithWarning(
+        "var o = (/** @type {!Foo} */ {});",
+        MISPLACED_ANNOTATION);
+  }
+
+  public void testMisplacedTypeAnnotation6() {
+    testSameWithWarning(
+        "var o = /** @type {function():string} */ function() {return 'str';}",
+        MISPLACED_ANNOTATION);
+  }
+
+  public void testMisplacedTypeAnnotation7() {
+    testSameWithWarning(
+        "var x = /** @type {string} */ y;",
+        MISPLACED_ANNOTATION);
+  }
+
+  public void testAllowedNocollapseAnnotation() {
+    testSame("var foo = {}; /** @nocollapse */ foo.bar = true;");
+  }
+
+  public void testMisplacedNocollapseAnnotation1() {
+    testSameWithWarning(
+        "/** @constructor */ function foo() {};"
+            + "/** @nocollapse */ foo.prototype.bar = function() {};",
+        MISPLACED_ANNOTATION);
+  }
+
+  public void testNocollapseInExterns() {
+    test("var foo = {}; /** @nocollapse */ foo.bar = true;",
+        "foo.bar;", "foo.bar;", null,
+        MISPLACED_ANNOTATION);
+  }
+}
diff --git a/test/com/google/javascript/jscomp/CompilerTypeTestCase.java b/test/com/google/javascript/jscomp/CompilerTypeTestCase.java
index 16b851fc9..8c5cbb498 100644
--- a/test/com/google/javascript/jscomp/CompilerTypeTestCase.java
+++ b/test/com/google/javascript/jscomp/CompilerTypeTestCase.java
@@ -136,7 +136,7 @@ abstract class CompilerTypeTestCase extends BaseJSTypeTestCase {
     options.setWarningLevel(
         DiagnosticGroups.MISSING_PROPERTIES, CheckLevel.WARNING);
     options.setWarningLevel(
-        DiagnosticGroups.MISPLACED_TYPE_ANNOTATION, CheckLevel.WARNING);
+        DiagnosticGroups.MISPLACED_ANNOTATION, CheckLevel.WARNING);
     options.setWarningLevel(
         DiagnosticGroups.INVALID_CASTS, CheckLevel.WARNING);
     options.setWarningLevel(DiagnosticGroups.LINT_CHECKS, CheckLevel.WARNING);
diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java
index 3a628ea9a..777bc311b 100644
--- a/test/com/google/javascript/jscomp/IntegrationTest.java
+++ b/test/com/google/javascript/jscomp/IntegrationTest.java
@@ -418,9 +418,9 @@ public final class IntegrationTest extends IntegrationTestCase {
             "  return b.meeny+c.miny+b.tiger" +
             "}"},
         new DiagnosticType[]{
-            RhinoErrorReporter.PARSE_ERROR,
-            RhinoErrorReporter.PARSE_ERROR,
-            RhinoErrorReporter.PARSE_ERROR});
+            CheckJSDoc.ANNOTATION_DEPRECATED,
+            CheckJSDoc.ANNOTATION_DEPRECATED,
+            CheckJSDoc.ANNOTATION_DEPRECATED});
   }
 
   public void testCheckSymbolsOff() {
diff --git a/test/com/google/javascript/jscomp/NormalizeTest.java b/test/com/google/javascript/jscomp/NormalizeTest.java
index 3d74bb26c..a2d72e1ae 100644
--- a/test/com/google/javascript/jscomp/NormalizeTest.java
+++ b/test/com/google/javascript/jscomp/NormalizeTest.java
@@ -461,13 +461,11 @@ public final class NormalizeTest extends CompilerTestCase {
   }
 
   public void testExposeSimple() {
-    setExpectParseWarningsThisTest();
     test("var x = {}; /** @expose */ x.y = 3; x.y = 5;",
          "var x = {}; x['y'] = 3; x['y'] = 5;");
   }
 
   public void testExposeComplex() {
-    setExpectParseWarningsThisTest();
     test(
         "var x = {/** @expose */ a: 1, b: 2};"
         + "x.a = 3; /** @expose */ x.b = 5;",
diff --git a/test/com/google/javascript/jscomp/RhinoErrorReporterTest.java b/test/com/google/javascript/jscomp/RhinoErrorReporterTest.java
index 97da1b9e8..c08e0e2e5 100644
--- a/test/com/google/javascript/jscomp/RhinoErrorReporterTest.java
+++ b/test/com/google/javascript/jscomp/RhinoErrorReporterTest.java
@@ -29,12 +29,10 @@ import java.util.List;
  */
 public final class RhinoErrorReporterTest extends TestCase {
 
-  private boolean reportMisplacedTypeAnnotations;
   private boolean reportEs3Props;
 
   @Override
   protected void setUp() throws Exception {
-    reportMisplacedTypeAnnotations = false;
     reportEs3Props = true;
     super.setUp();
   }
@@ -60,25 +58,6 @@ public final class RhinoErrorReporterTest extends TestCase {
     assertEquals(8, error.getCharno());
   }
 
-  public void testMisplacedTypeAnnotation() throws Exception {
-    reportMisplacedTypeAnnotations = false;
-
-    assertNoWarningOrError("var x = /** @type {string} */ y;");
-
-    reportMisplacedTypeAnnotations = true;
-
-    String message =
-        "Type annotations are not allowed here. " +
-        "Are you missing parentheses?";
-    JSError error = assertWarning(
-        "var x = /** @type {string} */ y;",
-        RhinoErrorReporter.MISPLACED_TYPE_ANNOTATION,
-        message);
-
-    assertEquals(1, error.getLineNumber());
-    assertEquals(30, error.getCharno());
-  }
-
   public void testInvalidEs3Prop() throws Exception {
     reportEs3Props = false;
 
@@ -100,19 +79,6 @@ public final class RhinoErrorReporterTest extends TestCase {
     assertEquals(10, error.getCharno());
   }
 
-  public void testAnnotationDeprecated() throws Exception {
-    String message =
-        "The @expose annotation is deprecated. Use @nocollapse or @export "
-        + "instead.";
-    JSError error = assertWarning(
-        "/** @expose */ var x = 1;",
-        RhinoErrorReporter.ANNOTATION_DEPRECATED,
-        message);
-
-    assertEquals(1, error.getLineNumber());
-    assertEquals(15, error.getCharno());
-  }
-
   /**
    * Verifies that the compiler emits an error for the given code.
    */
@@ -155,11 +121,6 @@ public final class RhinoErrorReporterTest extends TestCase {
   private Compiler parseCode(String code) {
     Compiler compiler = new Compiler();
     CompilerOptions options = new CompilerOptions();
-    if (reportMisplacedTypeAnnotations) {
-      options.setWarningLevel(
-          DiagnosticGroups.MISPLACED_TYPE_ANNOTATION,
-          CheckLevel.WARNING);
-    }
 
     if (!reportEs3Props) {
       options.setWarningLevel(
diff --git a/test/com/google/javascript/jscomp/TypeCheckTest.java b/test/com/google/javascript/jscomp/TypeCheckTest.java
index 76abbdab4..cdb95d154 100644
--- a/test/com/google/javascript/jscomp/TypeCheckTest.java
+++ b/test/com/google/javascript/jscomp/TypeCheckTest.java
@@ -6705,10 +6705,7 @@ public final class TypeCheckTest extends CompilerTypeTestCase {
         "document.getElementById;\n" +
         "var list = /** @type {!Array<string>} */ ['hello', 'you'];\n" +
         "list.push('?');\n" +
-        "document.getElementById('node').innerHTML = list.toString();",
-        // Parse warning, but still applied.
-        "Type annotations are not allowed here. " +
-        "Are you missing parentheses?");
+        "document.getElementById('node').innerHTML = list.toString();");
   }
 
   public void testIssue483() throws Exception {
@@ -7191,8 +7188,7 @@ public final class TypeCheckTest extends CompilerTypeTestCase {
         "function foo(opt_f) {" +
         "  /** @type {Function} */" +
         "  return opt_f || function () {};" +
-        "}",
-        "Type annotations are not allowed here. Are you missing parentheses?");
+        "}");
   }
 
   /**
diff --git a/test/com/google/javascript/jscomp/parsing/NewParserTest.java b/test/com/google/javascript/jscomp/parsing/NewParserTest.java
index 736fd4623..2bf37092a 100644
--- a/test/com/google/javascript/jscomp/parsing/NewParserTest.java
+++ b/test/com/google/javascript/jscomp/parsing/NewParserTest.java
@@ -17,9 +17,6 @@
 package com.google.javascript.jscomp.parsing;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.javascript.jscomp.parsing.IRFactory.MISPLACED_FUNCTION_ANNOTATION;
-import static com.google.javascript.jscomp.parsing.IRFactory.MISPLACED_MSG_ANNOTATION;
-import static com.google.javascript.jscomp.parsing.IRFactory.MISPLACED_TYPE_ANNOTATION;
 import static com.google.javascript.jscomp.testing.NodeSubject.assertNode;
 
 import com.google.common.base.Joiner;
@@ -65,9 +62,6 @@ public final class NewParserTest extends BaseJSTypeTestCase {
       "for legacy reasons. Removing this from your code is " +
       "safe for all browsers currently in use.";
 
-  private static final String ANNOTATION_DEPRECATED_WARNING =
-      "The %s annotation is deprecated.%s";
-
   private Config.LanguageMode mode;
   private boolean isIdeMode = false;
 
@@ -538,10 +532,9 @@ public final class NewParserTest extends BaseJSTypeTestCase {
    * propagate to following code due to {@link JSDocInfo} aggregation.
    */
   public void testJSDocAttachment6() throws Exception {
-    Node functionNode = parseWarning(
+    Node functionNode = parse(
         "var a = /** @param {number} index */5;"
-        + "/** @return boolean */function f(index){}",
-        MISPLACED_FUNCTION_ANNOTATION)
+        + "/** @return boolean */function f(index){}")
         .getFirstChild().getNext();
 
     assertThat(functionNode.getType()).isEqualTo(Token.FUNCTION);
@@ -658,10 +651,10 @@ public final class NewParserTest extends BaseJSTypeTestCase {
 
   public void testJSDocAttachment17() {
     Node fn =
-        parseWarning(
+        parse(
             "function f() { " +
             "  return /** @type {string} */ (g(1 /** @desc x */));" +
-            "};", MISPLACED_MSG_ANNOTATION).getFirstChild();
+            "};").getFirstChild();
     assertThat(fn.getType()).isEqualTo(Token.FUNCTION);
     Node cast = fn.getLastChild().getFirstChild().getFirstChild();
     assertThat(cast.getType()).isEqualTo(Token.CAST);
@@ -680,12 +673,11 @@ public final class NewParserTest extends BaseJSTypeTestCase {
 
   public void testJSDocAttachment19() {
     Node fn =
-        parseWarning(
+        parse(
             "function f() { " +
             "  /** @type {string} */" +
             "  return;" +
-            "};",
-            MISPLACED_TYPE_ANNOTATION).getFirstChild();
+            "};").getFirstChild();
     assertThat(fn.getType()).isEqualTo(Token.FUNCTION);
 
     Node ret = fn.getLastChild().getFirstChild();
@@ -695,11 +687,11 @@ public final class NewParserTest extends BaseJSTypeTestCase {
 
   public void testJSDocAttachment20() {
     Node fn =
-        parseWarning(
+        parse(
             "function f() { " +
             "  /** @type {string} */" +
             "  if (true) return;" +
-            "};", MISPLACED_TYPE_ANNOTATION).getFirstChild();
+            "};").getFirstChild();
     assertThat(fn.getType()).isEqualTo(Token.FUNCTION);
 
     Node ret = fn.getLastChild().getFirstChild();
@@ -875,17 +867,6 @@ public final class NewParserTest extends BaseJSTypeTestCase {
     parse("var msgs = {/** @desc x */ MSG_X: goog.getMsg('x')}");
   }
 
-  public void testMisplacedDescAnnotation() {
-    parseWarning("/** @desc Foo. */ var bar = goog.getMsg('hello');",
-        MISPLACED_MSG_ANNOTATION);
-    parseWarning("/** @desc Foo. */ x.y.z.bar = goog.getMsg('hello');",
-        MISPLACED_MSG_ANNOTATION);
-    parseWarning("var msgs = {/** @desc x */ x: goog.getMsg('x')}",
-        MISPLACED_MSG_ANNOTATION);
-    parseWarning("/** @desc Foo. */ bar = goog.getMsg('x');",
-        MISPLACED_MSG_ANNOTATION);
-  }
-
   public void testUnescapedSlashInRegexpCharClass() {
     parse("var foo = /[/]/;");
     parse("var foo = /[hi there/]/;");
@@ -2156,47 +2137,6 @@ public final class NewParserTest extends BaseJSTypeTestCase {
         "'(' expected");
   }
 
-  public void testMisplacedTypeAnnotation1() {
-    // misuse with COMMA
-    parseWarning(
-        "var o = {};" +
-        "/** @type {string} */ o.prop1 = 1, o.prop2 = 2;",
-        MISPLACED_TYPE_ANNOTATION);
-  }
-
-  public void testMisplacedTypeAnnotation2() {
-    // missing parentheses for the cast.
-    parseWarning(
-        "var o = /** @type {string} */ getValue();",
-        MISPLACED_TYPE_ANNOTATION);
-  }
-
-  public void testMisplacedTypeAnnotation3() {
-    // missing parentheses for the cast.
-    parseWarning(
-        "var o = 1 + /** @type {string} */ value;",
-        MISPLACED_TYPE_ANNOTATION);
-  }
-
-  public void testMisplacedTypeAnnotation4() {
-    // missing parentheses for the cast.
-    parseWarning(
-        "var o = /** @type {!Array.<string>} */ ['hello', 'you'];",
-        MISPLACED_TYPE_ANNOTATION);
-  }
-
-  public void testMisplacedTypeAnnotation5() {
-    // missing parentheses for the cast.
-    parseWarning(
-        "var o = (/** @type {!Foo} */ {});",
-        MISPLACED_TYPE_ANNOTATION);
-  }
-
-  public void testMisplacedTypeAnnotation6() {
-    parseWarning("var o = /** @type {function():string} */ function() {return 'str';}",
-        MISPLACED_TYPE_ANNOTATION);
-  }
-
   public void testValidTypeAnnotation1() {
     parse("/** @type {string} */ var o = 'str';");
     parse("var /** @type {string} */ o = 'str', /** @type {number} */ p = 0;");
@@ -2704,12 +2644,6 @@ public final class NewParserTest extends BaseJSTypeTestCase {
         + "");
   }
 
-  public void testExposeDeprecated() {
-    parseWarning("/** @expose */ var x = 0;",
-        String.format(ANNOTATION_DEPRECATED_WARNING, "@expose",
-            " Use @nocollapse or @export instead."));
-  }
-
   private Node script(Node stmt) {
     Node n = new Node(Token.SCRIPT, stmt);
     n.setIsSyntheticBlock(true);