Add a new @closurePrimitive annotation that is propagated to every Fu…

…nctionType

proof of concept - DefaultCodingConvention now recognizes any function annotated with @closurePrimitive {asserts.fail} as a function that always throws.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=234884424

src/com/google/javascript/jscomp/CheckJSDoc.java

@@ -155,6 +155,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
     validateDefinesDeclaration(n, info);
     validateSuppress(n, info);
     validateImplicitCast(n, info);
+    validateClosurePrimitive(n, info);
   }
 
   private void validateSuppress(Node n, JSDocInfo info) {
@@ -415,44 +416,57 @@ private void validateFunctionJsDoc(Node n, JSDocInfo info) {
       return;
     }
 
-    if (info.containsFunctionDeclaration() && !info.hasType()) {
+    if (info.containsFunctionDeclaration() && !info.hasType() && !isJSDocOnFunctionNode(n, info)) {
       // This JSDoc should be attached to a FUNCTION node, or an assignment
       // with a function as the RHS, etc.
-      switch (n.getToken()) {
-        case FUNCTION:
-        case GETTER_DEF:
-        case SETTER_DEF:
-        case MEMBER_FUNCTION_DEF:
-        case STRING_KEY:
-        case COMPUTED_PROP:
-        case EXPORT:
-          return;
-        case GETELEM:
-        case GETPROP:
-          if (n.getFirstChild().isQualifiedName()) {
-            return;
-          }
-          break;
-        case VAR:
-        case LET:
-        case CONST:
-        case ASSIGN: {
+
+      reportMisplaced(
+          n,
+          "function",
+          "This JSDoc is not attached to a function node. " + "Are you missing parentheses?");
+    }
+  }
+
+  /**
+   * Whether this node's JSDoc may apply to a function
+   *
+   * <p>This has some false positive cases, to allow for patterns like goog.abstractMethod.
+   */
+  private boolean isJSDocOnFunctionNode(Node n, JSDocInfo info) {
+    switch (n.getToken()) {
+      case FUNCTION:
+      case GETTER_DEF:
+      case SETTER_DEF:
+      case MEMBER_FUNCTION_DEF:
+      case STRING_KEY:
+      case COMPUTED_PROP:
+      case EXPORT:
+        return true;
+      case GETELEM:
+      case GETPROP:
+        if (n.getFirstChild().isQualifiedName()) {
+          // assume qualified names may be function declarations
+          return true;
+        }
+        return false;
+      case VAR:
+      case LET:
+      case CONST:
+      case ASSIGN:
+        {
           Node lhs = n.getFirstChild();
           Node rhs = NodeUtil.getRValueOfLValue(lhs);
           if (rhs != null && isClass(rhs) && !info.isConstructor()) {
-            break;
+            return false;
           }
-          // TODO(tbreisacher): Check that the RHS of the assignment is a
+
+          // TODO(b/124081098): 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;
+          return true;
         }
-        default:
-          break;
-      }
-      reportMisplaced(n,
-          "function", "This JSDoc is not attached to a function node. "
-              + "Are you missing parentheses?");
+      default:
+        return false;
     }
   }
 
@@ -668,4 +682,15 @@ private void validateImplicitCast(Node n, JSDocInfo info) {
       report(n, TypeCheck.ILLEGAL_IMPLICIT_CAST);
     }
   }
+
+  /** Checks that a @closurePrimitive {id} is on a function */
+  private void validateClosurePrimitive(Node n, JSDocInfo info) {
+    if (info == null || !info.hasClosurePrimitiveId()) {
+      return;
+    }
+
+    if (!isJSDocOnFunctionNode(n, info)) {
+      report(n, MISPLACED_ANNOTATION, "closurePrimitive", "must be on a function node");
+    }
+  }
 }

src/com/google/javascript/jscomp/ClosureCodingConvention.java

@@ -345,8 +345,8 @@ public boolean isPropertyRenameFunction(String name) {
 
   @Override
   public boolean isFunctionCallThatAlwaysThrows(Node n) {
-    return CodingConventions.defaultIsFunctionCallThatAlwaysThrows(
-        n, "goog.asserts.fail");
+    return super.isFunctionCallThatAlwaysThrows(n)
+        || CodingConventions.defaultIsFunctionCallThatAlwaysThrows(n, "goog.asserts.fail");
   }
 
   @Override

src/com/google/javascript/jscomp/CodingConventions.java

@@ -20,6 +20,7 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.errorprone.annotations.Immutable;
+import com.google.javascript.rhino.ClosurePrimitive;
 import com.google.javascript.rhino.Node;
 import com.google.javascript.rhino.NominalTypeBuilder;
 import com.google.javascript.rhino.StaticSourceFile;
@@ -358,6 +359,10 @@ public boolean isVarArgsParameter(Node parameter) {
 
     @Override
     public boolean isFunctionCallThatAlwaysThrows(Node n) {
+      if (NodeUtil.isExprCall(n)) {
+        FunctionType fnType = FunctionType.toMaybeFunctionType(n.getFirstFirstChild().getJSType());
+        return fnType != null && ClosurePrimitive.ASSERTS_FAIL == fnType.getClosurePrimitive();
+      }
       return false;
     }
 

src/com/google/javascript/jscomp/FunctionTypeBuilder.java

@@ -32,6 +32,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
+import com.google.javascript.rhino.ClosurePrimitive;
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.JSDocInfo;
 import com.google.javascript.rhino.JSTypeExpression;
@@ -92,6 +93,7 @@ final class FunctionTypeBuilder {
   private boolean isRecord = false;
   private boolean isAbstract = false;
   private Node parametersNode = null;
+  private ClosurePrimitive closurePrimitiveId = null;
   private ImmutableList<TemplateType> templateTypeNames = ImmutableList.of();
   private ImmutableList<TemplateType> constructorTemplateTypeNames = ImmutableList.of();
   private TypedScope declarationScope = null;
@@ -702,6 +704,13 @@ FunctionTypeBuilder inferImplicitConstructorParameters(Node parametersNode) {
     return this;
   }
 
+  FunctionTypeBuilder inferClosurePrimitive(@Nullable JSDocInfo info) {
+    if (info != null && info.hasClosurePrimitiveId()) {
+      this.closurePrimitiveId = ClosurePrimitive.fromStringId(info.getClosurePrimitiveId());
+    }
+    return this;
+  }
+
   private void setConstructorTemplateTypeNames(List<TemplateType> templates, @Nullable Node ctor) {
     if (!templates.isEmpty()) {
       this.constructorTemplateTypeNames = ImmutableList.copyOf(templates);
@@ -906,6 +915,7 @@ FunctionType buildAndRegister() {
               .withTypeOfThis(thisType)
               .withTemplateKeys(templateTypeNames)
               .withIsAbstract(isAbstract)
+              .withClosurePrimitiveId(closurePrimitiveId)
               .build();
       maybeSetBaseType(fnType);
     }

src/com/google/javascript/jscomp/TypedScopeCreator.java

@@ -1360,6 +1360,7 @@ private FunctionType createFunctionTypeFromNodes(
               .setDeclarationScope(lvalueNode != null ? getLValueRootScope(lvalueNode) : null)
               .inferFromOverriddenFunction(overriddenType, parametersNode)
               .inferKind(info)
+              .inferClosurePrimitive(info)
               .inferTemplateTypeName(info, prototypeOwner)
               .inferInheritance(info, null);
 

src/com/google/javascript/jscomp/parsing/Annotation.java

@@ -27,10 +27,11 @@ enum Annotation {
   NG_INJECT,
   ABSTRACT,
   AUTHOR,
-  CUSTOM_ELEMENT,
+  CLOSURE_PRIMITIVE,
   CONSISTENTIDGENERATOR,
   CONSTANT,
   CONSTRUCTOR,
+  CUSTOM_ELEMENT,
   RECORD,
   DEFINE,
   DEPRECATED,
@@ -91,6 +92,7 @@ enum Annotation {
           .put("abstract", Annotation.ABSTRACT)
           .put("argument", Annotation.PARAM)
           .put("author", Annotation.AUTHOR)
+          .put("closurePrimitive", Annotation.CLOSURE_PRIMITIVE)
           .put("consistentIdGenerator", Annotation.CONSISTENTIDGENERATOR)
           .put("const", Annotation.CONSTANT)
           .put("constant", Annotation.CONSTANT)

src/com/google/javascript/jscomp/parsing/Config.java

@@ -135,6 +135,9 @@ public enum RunMode {
   /** Set of recognized names in a {@code @suppress} tag. */
   abstract ImmutableSet<String> suppressionNames();
 
+  /** Set of recognized names in a {@code @closurePrimitive} tag. */
+  abstract ImmutableSet<String> closurePrimitiveNames();
+
   /** Whether to parse inline source maps (//# sourceMappingURL=data:...). */
   abstract boolean parseInlineSourceMaps();
 
@@ -150,6 +153,7 @@ static Builder builder() {
         .setRunMode(RunMode.STOP_AFTER_ERROR)
         .setExtraAnnotationNames(ImmutableSet.<String>of())
         .setSuppressionNames(ImmutableSet.<String>of())
+        .setClosurePrimitiveNames(ImmutableSet.of())
         .setParseInlineSourceMaps(false);
   }
 
@@ -167,6 +171,8 @@ abstract static class Builder {
 
     abstract Builder setSuppressionNames(Iterable<String> names);
 
+    abstract Builder setClosurePrimitiveNames(Iterable<String> names);
+
     final Builder setExtraAnnotationNames(Iterable<String> names) {
       return setAnnotations(buildAnnotations(names));
     }

src/com/google/javascript/jscomp/parsing/JsDocInfoParser.java

@@ -119,6 +119,7 @@ private void addMissingTypeWarning(int lineno, int charno) {
 
   private final Map<String, Annotation> annotations;
   private final Set<String> suppressionNames;
+  private final Set<String> closurePrimitiveNames;
   private final boolean preserveWhitespace;
   private static final Set<String> modifiesAnnotationKeywords =
       ImmutableSet.of("this", "arguments");
@@ -176,6 +177,7 @@ private enum State {
     }
     this.annotations = config.annotations();
     this.suppressionNames = config.suppressionNames();
+    this.closurePrimitiveNames = config.closurePrimitiveNames();
     this.preserveWhitespace = config.jsDocParsingMode().shouldPreserveWhitespace();
 
     this.errorReporter = errorReporter;
@@ -676,6 +678,10 @@ private JsDocToken parseAnnotation(JsDocToken token,
           }
           return token;
 
+        case CLOSURE_PRIMITIVE:
+          skipEOLs();
+          return parseClosurePrimitiveTag(next());
+
         case NO_COMPILE:
           if (!jsdocBuilder.recordNoCompile()) {
             addParserWarning("msg.jsdoc.nocompile");
@@ -1346,6 +1352,36 @@ private JsDocToken parseSuppressTag(JsDocToken token) {
     }
   }
 
+  /**
+   * Parse a {@code @closurePrimitive} tag
+   *
+   * @param token The current token.
+   */
+  private JsDocToken parseClosurePrimitiveTag(JsDocToken token) {
+    if (token != JsDocToken.LEFT_CURLY) {
+      addParserWarning("msg.jsdoc.missing.lc");
+      return token;
+    } else if (match(JsDocToken.STRING)) {
+      String name = stream.getString();
+      if (!closurePrimitiveNames.contains(name)) {
+        addParserWarning("msg.jsdoc.closurePrimitive.invalid", name);
+      } else if (!jsdocBuilder.recordClosurePrimitiveId(name)) {
+        addParserWarning("msg.jsdoc.closurePrimitive.extra");
+      }
+      token = next();
+    } else {
+      addParserWarning("msg.jsdoc.closurePrimitive.missing");
+      return token;
+    }
+
+    if (!match(JsDocToken.RIGHT_CURLY)) {
+      addParserWarning("msg.jsdoc.missing.rc");
+    } else {
+      token = next();
+    }
+    return eatUntilEOLIfNotAnnotation();
+  }
+
   /**
    * Parse a {@code @modifies} tag of the form
    * {@code @modifies&#123;this|arguments|param&#125;}.

src/com/google/javascript/jscomp/parsing/ParserConfig.properties

@@ -43,6 +43,7 @@ jsdoc.annotations =\
     channel, \
     class,\
     classdesc,\
+    closurePrimitive,\
     codepen,\
     config,\
     consistentIdGenerator,\
@@ -213,6 +214,11 @@ jsdoc.suppressions =\
     visibility,\
     with
 
+# A comma-delimited list of valid closure primitive ids.
+# This correspond to the ClosurePrimitive enum once normalized (see ClosurePrimitive.fromStringId)
+jsdoc.primitives =\
+   asserts.fail,\
+
 # A comma-delimited list of reserved words that we should not rename variables
 # to. Used when an extension is released that steps on globals.
 # This prevents the compiler from renaming variables to these names, but not

src/com/google/javascript/jscomp/parsing/ParserRunner.java

@@ -51,6 +51,7 @@ public final class ParserRunner {
 
   private static Set<String> suppressionNames = null;
   private static Set<String> reservedVars = null;
+  private static Set<String> closurePrimitiveNames = null;
 
   // Should never need to instantiate class of static methods.
   private ParserRunner() {}
@@ -87,6 +88,7 @@ public static Config createConfig(
         .setJsDocParsingMode(jsdocParsingMode)
         .setRunMode(runMode)
         .setSuppressionNames(suppressionNames)
+        .setClosurePrimitiveNames(closurePrimitiveNames)
         .setLanguageMode(languageMode)
         .setParseInlineSourceMaps(parseInlineSourceMaps)
         .setStrictMode(strictMode)
@@ -106,6 +108,7 @@ private static synchronized void initResourceConfig() {
     ResourceBundle config = ResourceBundle.getBundle(CONFIG_RESOURCE);
     annotationNames = extractList(config.getString("jsdoc.annotations"));
     suppressionNames = extractList(config.getString("jsdoc.suppressions"));
+    closurePrimitiveNames = extractList(config.getString("jsdoc.primitives"));
     reservedVars = extractList(config.getString("compiler.reserved.vars"));
   }