Instead of using @noinline to tell OptimizeParameters to back off, create a new jsdoc annotation that is more precise @usedViaDotConstructor

This should be more intuitive and doesn't confuse the linter.

PiperOrigin-RevId: 704434645

Author: lukesandberg

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

@@ -160,6 +160,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
     validateReturnJsDoc(n, info);
     validateTsType(n, info);
     validateJsDocTypeNames(info);
+    validateIsUsedViaDotConstructor(n, info);
   }
 
   private void validateSuppress(Node n, JSDocInfo info) {
@@ -837,4 +838,15 @@ public void visit(Node typeRefNode) {
       }
     }
   }
+
+  /** Checks that @usedViaDotConstructor is only used on constructors. */
+  private void validateIsUsedViaDotConstructor(Node n, JSDocInfo info) {
+    if (info == null || !info.isUsedViaDotConstructor()) {
+      return;
+    }
+    if (!(n.isFunction() && info.isConstructor())
+        && !NodeUtil.isEs6ConstructorMemberFunctionDef(n)) {
+      report(n, MISPLACED_ANNOTATION, "usedViaDotConstructor", "must be on a constructor");
+    }
+  }
 }

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

@@ -671,8 +671,8 @@ private boolean allDefinitionsAreCandidateFunctions(Node n) {
                 // In `function f(a, b = a) { ... }` it's very difficult to determine if `a` is
                 // movable.
                 && !mayReferenceParamBeforeBody(functionNode)
-                // `function f(/** @noinline */ a) {...}` means back off.
-                && !mayHaveNoInlineParameters(functionNode);
+                // `/** @usedViaDotConstructor */ function f(a) {...}` means back off.
+                && !isUsedViaDotConstructor(functionNode);
           }
         }
       case FUNCTION:
@@ -682,8 +682,8 @@ private boolean allDefinitionsAreCandidateFunctions(Node n) {
             && !NodeUtil.doesFunctionReferenceOwnArgumentsObject(n)
             // In `function f(a, b = a) { ... }` it's very difficult to determine if `a` is movable.
             && !mayReferenceParamBeforeBody(n)
-            // `function f(/** @noinline */ a) {...}` means back off.
-            && !mayHaveNoInlineParameters(n);
+            // `/** @usedViaDotConstructor */ function f(a) {...}` means back off.
+            && !isUsedViaDotConstructor(n);
       case CAST:
       case COMMA:
         return allDefinitionsAreCandidateFunctions(n.getLastChild());
@@ -701,23 +701,14 @@ private boolean allDefinitionsAreCandidateFunctions(Node n) {
   }
 
   /**
-   * Returns true if the function has any parameters tagged with @noinline.
+   * Returns true if the function is annotated with @usedViaDotConstructor
    *
-   * <p>In this case we just back off on all parameter optimizations. We could be more clever and
-   * only back off on the ones tagged @noinline, but it's not worth the complexity.
+   * <p>In this case we just back off on all parameter optimizations since we cannot detect which
+   * parameters are used via calls through {@code .constructor}.
    */
-  private static boolean mayHaveNoInlineParameters(Node function) {
-    Node paramList = function.getSecondChild();
-    if (!paramList.hasChildren()) {
-      return false; // Fast path; there can't possibly be @noinline params.
-    }
-    for (Node param = paramList.getFirstChild(); param != null; param = param.getNext()) {
-      var jsDocInfo = param.getJSDocInfo();
-      if (jsDocInfo != null && jsDocInfo.isNoInline()) {
-        return true;
-      }
-    }
-    return false;
+  private static boolean isUsedViaDotConstructor(Node function) {
+    var docInfo = NodeUtil.getBestJSDocInfo(function);
+    return docInfo != null && docInfo.isUsedViaDotConstructor();
   }
 
   /**

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

@@ -100,6 +100,7 @@ enum Annotation {
   TYPEDEF,
   TYPE_SUMMARY,
   UNRESTRICTED,
+  USED_VIA_DOT_CONSTRUCTOR,
   WIZACTION,
   TS_TYPE,
   WIZ_ANALYZER,
@@ -180,6 +181,7 @@ enum Annotation {
           .put("typedef", Annotation.TYPEDEF)
           .put("typeSummary", Annotation.TYPE_SUMMARY)
           .put("unrestricted", Annotation.UNRESTRICTED)
+          .put("usedViaDotConstructor", Annotation.USED_VIA_DOT_CONSTRUCTOR)
           .put("wizaction", Annotation.WIZACTION)
           .put("tsType", Annotation.TS_TYPE)
           .put("wizAnalyzer", Annotation.WIZ_ANALYZER)

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

@@ -773,9 +773,6 @@ private JsDocToken parseAnnotation(JsDocToken token, List<ExtendedTypeInfo> exte
           }
           return eatUntilEOLIfNotAnnotation();
 
-        case NOT_IMPLEMENTED:
-          return eatUntilEOLIfNotAnnotation();
-
         case INHERIT_DOC:
         case OVERRIDE:
           if (!jsdocBuilder.recordOverride()) {
@@ -1082,12 +1079,18 @@ private JsDocToken parseAnnotation(JsDocToken token, List<ExtendedTypeInfo> exte
         case LOG_TYPE_IN_COMPILER:
           var unused = jsdocBuilder.recordLogTypeInCompiler();
           return eatUntilEOLIfNotAnnotation();
+        case NOT_IMPLEMENTED:
         case JSX:
         case JSX_FRAGMENT:
         case SOY_MODULE:
         case SOY_TEMPLATE:
         case WIZ_ANALYZER:
           return eatUntilEOLIfNotAnnotation();
+        case USED_VIA_DOT_CONSTRUCTOR:
+          if (!jsdocBuilder.recordUsedViaDotConstructor()) {
+            addParserWarning(Msg.JSDOC_USEDVIADOTCONSTRUCTOR);
+          }
+          return eatUntilEOLIfNotAnnotation();
         case WIZACTION:
           if (!jsdocBuilder.recordWizaction()) {
             addParserWarning(Msg.JSDOC_WIZACTION);

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

@@ -150,6 +150,7 @@ jsdoc.annotations =\
     typeSummary,\
     url,\
     usage,\
+    usedViaDotConstructor,\
     version,\
     virtual,\
     visibility,\

src/com/google/javascript/jscomp/serialization/JSDocSerializer.java

@@ -170,6 +170,9 @@ public static JSDocInfo convertJSDocInfoForOptimizations(JSDocInfo jsdoc) {
     if (jsdoc.getSuppressions().contains("untranspilableFeatures")) {
       builder.addKind(JsdocTag.JSDOC_SUPPRESS_UNTRANSPILABLE_FEATURES);
     }
+    if (jsdoc.isUsedViaDotConstructor()) {
+      builder.addKind(JsdocTag.JSDOC_USED_VIA_DOT_CONSTRUCTOR);
+    }
 
     OptimizationJsdoc result = builder.build();
     if (OptimizationJsdoc.getDefaultInstance().equals(result)) {
@@ -349,7 +352,11 @@ private static JSTypeExpression createPlaceholderType() {
         case JSDOC_SASS_GENERATED_CSS_TS:
           builder.recordSassGeneratedCssTs();
           continue;
-
+        case JSDOC_USED_VIA_DOT_CONSTRUCTOR:
+          {
+            var unused = builder.recordUsedViaDotConstructor();
+            continue;
+          }
         case JSDOC_UNSPECIFIED:
         case UNRECOGNIZED:
           throw new MalformedTypedAstException(

src/com/google/javascript/rhino/JSDocInfo.java

@@ -325,6 +325,7 @@ private static enum Bit {
     NOCOMPILE,
     NODTS,
     UNRESTRICTED,
+    USED_VIA_DOT_CONSTRUCTOR,
     STRUCT,
     DICT,
     NOCOLLAPSE,
@@ -1240,6 +1241,11 @@ public boolean isClosureUnawareCode() {
     return checkBit(Bit.CLOSURE_UNAWARE_CODE);
   }
 
+  /** Returns whether JSDoc is annotated with the {@code @usedViaDotConstructor} annotation. */
+  public boolean isUsedViaDotConstructor() {
+    return checkBit(Bit.USED_VIA_DOT_CONSTRUCTOR);
+  }
+
   /** Gets the description specified by the {@code @license} annotation. */
   public String getLicense() {
     return LICENSE.get(this);
@@ -2712,6 +2718,11 @@ public boolean recordClosureUnawareCode() {
       return populateBit(Bit.CLOSURE_UNAWARE_CODE, true);
     }
 
+    /** Records that this JSDoc was annotated with the {@code @usedViaDotConstructor} annotation. */
+    public boolean recordUsedViaDotConstructor() {
+      return populateBit(Bit.USED_VIA_DOT_CONSTRUCTOR, true);
+    }
+
     // TODO(sdh): this is a new method - consider removing it in favor of recordType?
     // The main difference is that this force-sets the type, while recordType backs off.
     // This is useful for (e.g.) copyFromWithNewType.

src/com/google/javascript/rhino/Msg.java

@@ -144,6 +144,7 @@ public enum Msg {
   JSDOC_TYPETRANSFORMATION_MISSING_PARAM("Missing parameter in {0}"),
   JSDOC_TYPE_RECORD_DUPLICATE("Duplicate record field {0}."),
   JSDOC_TYPE_SYNTAX("type not recognized due to syntax error."),
+  JSDOC_USEDVIADOTCONSTRUCTOR("extra @usedViaDotConstructor tag"),
   JSDOC_UNNECESSARY_BRACES("braces are not required here"),
   JSDOC_VERSIONMISSING("@version tag missing version information"),
   JSDOC_WIZACTION("extra @wizaction tag"),

src/com/google/javascript/rhino/typed_ast/optimization_jsdoc.proto

@@ -83,4 +83,7 @@ enum JsdocTag {
   // Used to require function inlining to enable other optimizations,
   // especially constant folding of object literals.
   JSDOC_REQUIRE_INLINING = 38;
+
+  // Used by OptimizeParameters
+  JSDOC_USED_VIA_DOT_CONSTRUCTOR = 39;
 }

test/com/google/javascript/jscomp/CheckJsDocTest.java

@@ -1304,4 +1304,11 @@ public void testNameDeclarationAndAssignForAbstractClass() {
   public void testNameDeclarationAndAssignForTemplatedClass() {
     testSame(lines("/** @template T */", "let A = A_1 = class A {}"));
   }
+
+  @Test
+  public void testIsUsedViaDotConstrucctor() {
+    testSame("/** @constructor @usedViaDotConstructor */ function A() {}");
+    testSame("class Foo {/** @usedViaDotConstructor */ constructor() {} }");
+    testWarning("/** @usedViaDotConstructor */ function A() {}", MISPLACED_ANNOTATION);
+  }
 }

test/com/google/javascript/jscomp/OptimizeParametersTest.java

@@ -409,7 +409,7 @@ public void testSimpleRemoval1() {
     test(
         "const foo = (p1)=>{       }; foo(); foo()", //
         "const foo = (  )=>{var p1;}; foo(); foo()");
-    testSame("const foo = (/** @noinline */ p1)=>{}; foo(); foo()");
+    testSame("class foo { /** @usedViaDotConstructor */ constructor(p1) {}} new foo(); new foo()");
 
     // constant parameter
     test(
@@ -421,7 +421,8 @@ public void testSimpleRemoval1() {
     test(
         "const foo = (p1)=>{           }; foo(1); foo(1)",
         "const foo = (  )=>{var p1 = 1;}; foo( ); foo( )");
-    testSame("const foo = (/** @noinline */ p1)=>{}; foo(1); foo(1)");
+    testSame(
+        "class foo { /** @usedViaDotConstructor */ constructor(p1) {}} new foo(1); new foo(1)");
   }
 
   @Test
@@ -441,7 +442,7 @@ public void testSimpleRemoval2() {
     test(
         "function f(p1) {           } new f(1,x()); new f(1,y())",
         "function f(  ) {var p1 = 1;} new f(  x()); new f(  y())");
-    testSame("function f(/** @noinline */ p1) {} new f(); new f()");
+    testSame("/** @usedViaDotConstructor */ function f(p1) {} new f(); new f()");
   }
 
   @Test
@@ -469,22 +470,22 @@ public void testSimpleRemoval4() {
     test(
         "function f(p1) {       } f.prop = 1; new f(); new f()",
         "function f(  ) {var p1;} f.prop = 1; new f(); new f()");
-    testSame("function f(/** @noinline */ p1) {} f.prop = 1; new f(); new f()");
+    testSame("/** @usedViaDotConstructor */ function f(p1) {} f.prop = 1; new f(); new f()");
 
     test(
         "function f(p1) {           } f.prop = 1; new f(1); new f(1)",
         "function f(  ) {var p1 = 1;} f.prop = 1; new f( ); new f( )");
-    testSame("function f(/** @noinline */ p1) {} f.prop = 1; new f(1); new f(1)");
+    testSame("/** @usedViaDotConstructor */ function f(p1) {} f.prop = 1; new f(1); new f(1)");
 
     test(
         "function f(p1) {       } f['prop'] = 1; new f(); new f()",
         "function f(  ) {var p1;} f['prop'] = 1; new f(); new f()");
-    testSame("function f(/** @noinline */ p1) {} f['prop'] = 1; new f(); new f()");
+    testSame("/** @usedViaDotConstructor */ function f(p1) {} f['prop'] = 1; new f(); new f()");
 
     test(
         "function f(p1) {           } f['prop'] = 1; new f(1); new f(1)",
         "function f(  ) {var p1 = 1;} f['prop'] = 1; new f( ); new f( )");
-    testSame("function f(/** @noinline */ p1) {} f['prop'] = 1; new f(1); new f(1)");
+    testSame("/** @usedViaDotConstructor */ function f(p1) {} f['prop'] = 1; new f(1); new f(1)");
   }
 
   @Test