Allow @nosideeffects in function jsdoc outside of externs, and use it as a signal in PureFunctionIdentifier to treat the function as pure.

PiperOrigin-RevId: 508396386

Author: 12wrigja

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

@@ -699,11 +699,9 @@ private void validateDefaultValue(Node n) {
     }
   }
 
-  /**
-   * Check that @nosideeeffects annotations are only present in externs.
-   */
+  /** Check that @modifies annotations are only present in externs. */
   private void validateNoSideEffects(Node n, JSDocInfo info) {
-    // Cannot have @modifies or @nosideeffects in regular (non externs) js. Report errors.
+    // Cannot have @modifies in regular (non externs) js. Report errors.
     if (info == null) {
       return;
     }
@@ -715,9 +713,6 @@ private void validateNoSideEffects(Node n, JSDocInfo info) {
     if (info.hasSideEffectsArgumentsAnnotation() || info.modifiesThis()) {
       report(n, INVALID_MODIFIES_ANNOTATION);
     }
-    if (info.isNoSideEffects()) {
-      report(n, INVALID_NO_SIDE_EFFECT_ANNOTATION);
-    }
   }
 
   /**

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

@@ -658,10 +658,27 @@ public void visit(NodeTraversal traversal, Node node, Node parent) {
         allFunctionCalls.add(node);
       }
 
+      if (node.isFunction()) {
+        JSDocInfo jsdoc = NodeUtil.getBestJSDocInfo(node);
+        if (jsdoc != null && jsdoc.isNoSideEffects()) {
+          // Treat all names (both local aliases and exported names) as if they don't have side
+          // effects.
+          for (AmbiguatedFunctionSummary summary : summariesForAllNamesOfFunctionByNode.get(node)) {
+            summary.setIsArtificiallyPure(true);
+            summary.bitmask = 0; // no side effects
+          }
+        }
+      }
+
       Node root = this.functionScopeStack.getLast().root;
       if (root != null) {
-        // We only need to look at nodes in function scopes.
         for (AmbiguatedFunctionSummary summary : summariesForAllNamesOfFunctionByNode.get(root)) {
+          if (summary.isArtificiallyPure()) {
+            // Ignore node side effects for summaries that have been marked as artificially pure.
+            // We can't skip traversing the nodes in case an artificially pure node contains other
+            // function definitions.
+            continue;
+          }
           updateSideEffectsForNode(checkNotNull(summary), traversal, node);
         }
       }
@@ -1197,6 +1214,12 @@ private static boolean calleeAndCallerShareThis(Node invocation) {
      * @return Returns true if the propagation changed the side effects on the caller.
      */
     boolean propagate(AmbiguatedFunctionSummary callee, AmbiguatedFunctionSummary caller) {
+      if (callee.isArtificiallyPure() || caller.isArtificiallyPure()) {
+        // Pure callees should never propagate their side effects to their callers
+        //   - This would already happen - this condition is just a shortcut.
+        // Pure callers should never receive side effects propagated from their callees
+        return false;
+      }
       int initialCallerFlags = caller.bitmask;
 
       if (callerIsAlias) {
@@ -1256,6 +1279,7 @@ private static final class AmbiguatedFunctionSummary {
     // The side effect flags for this set of functions.
     // TODO(nickreid): Replace this with a `Node.SideEffectFlags`.
     private int bitmask = 0;
+    private boolean isArtificiallyPure = false;
 
     /** Adds a new summary node to {@code graph}, storing the node and returning the summary. */
     static AmbiguatedFunctionSummary createInGraph(
@@ -1271,6 +1295,7 @@ private AmbiguatedFunctionSummary(
 
     @CanIgnoreReturnValue
     private AmbiguatedFunctionSummary setMask(int mask) {
+      checkState(!isArtificiallyPure, "Artificially pure summaries should not be modified");
       bitmask |= mask;
       return this;
     }
@@ -1325,6 +1350,14 @@ boolean hasNoFlagsSet() {
       return this.bitmask == 0;
     }
 
+    void setIsArtificiallyPure(boolean isArtificiallyPure) {
+      this.isArtificiallyPure = isArtificiallyPure;
+    }
+
+    boolean isArtificiallyPure() {
+      return isArtificiallyPure;
+    }
+
     @Override
     @DoNotCall // For debugging only.
     public String toString() {

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

@@ -21,7 +21,6 @@
 import static com.google.javascript.jscomp.CheckJSDoc.DISALLOWED_MEMBER_JSDOC;
 import static com.google.javascript.jscomp.CheckJSDoc.INVALID_DEFINE_ON_LET;
 import static com.google.javascript.jscomp.CheckJSDoc.INVALID_MODIFIES_ANNOTATION;
-import static com.google.javascript.jscomp.CheckJSDoc.INVALID_NO_SIDE_EFFECT_ANNOTATION;
 import static com.google.javascript.jscomp.CheckJSDoc.JSDOC_IN_BLOCK_COMMENT;
 import static com.google.javascript.jscomp.CheckJSDoc.JSDOC_ON_RETURN;
 import static com.google.javascript.jscomp.CheckJSDoc.MISPLACED_ANNOTATION;
@@ -957,37 +956,33 @@ public void testBadTypedef_withES6Modules() {
   }
 
   @Test
-  public void testInvalidAnnotation1() {
-    testError("/** @nosideeffects */ function foo() {}", INVALID_NO_SIDE_EFFECT_ANNOTATION);
+  public void testNoSideEffectsAnnotation1() {
+    testSame("/** @nosideeffects */ function foo() {}");
   }
 
   @Test
-  public void testInvalidAnnotation2() {
-    testError("var f = /** @nosideeffects */ function() {}", INVALID_NO_SIDE_EFFECT_ANNOTATION);
+  public void testNoSideEffectsAnnotation2() {
+    testSame("var f = /** @nosideeffects */ function() {}");
   }
 
   @Test
-  public void testInvalidAnnotation3() {
-    testError("/** @nosideeffects */ var f = function() {}", INVALID_NO_SIDE_EFFECT_ANNOTATION);
+  public void testNoSideEffectsAnnotation3() {
+    testSame("/** @nosideeffects */ var f = function() {}");
   }
 
   @Test
-  public void testInvalidAnnotation4() {
-    testError(
-        "var f = function() {};" + "/** @nosideeffects */ f.x = function() {}",
-        INVALID_NO_SIDE_EFFECT_ANNOTATION);
+  public void testNoSideEffectsAnnotation4() {
+    testSame("var f = function() {};" + "/** @nosideeffects */ f.x = function() {}");
   }
 
   @Test
-  public void testInvalidAnnotation5() {
-    testError(
-        "var f = function() {};" + "f.x = /** @nosideeffects */ function() {}",
-        INVALID_NO_SIDE_EFFECT_ANNOTATION);
+  public void testNoSideEffectsAnnotation5() {
+    testSame("var f = function() {};" + "f.x = /** @nosideeffects */ function() {}");
   }
 
   @Test
-  public void testInvalidAnnotation_withES6Modules() {
-    testError("export /** @nosideeffects */ function foo() {}", INVALID_NO_SIDE_EFFECT_ANNOTATION);
+  public void testNoSideEffectsAnnotations() {
+    testSame("export /** @nosideeffects */ function foo() {}");
   }
 
   @Test

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

@@ -547,6 +547,46 @@ public void testSharedFunctionName2() {
     }
   }
 
+  @Test
+  public void testMultipleFunctionDefinitionsOnlyOneWithNoSideEffectsJsDoc() {
+    String source =
+        lines(
+            "let b = undefined;",
+            "if (true) {",
+            "  b = /** @nosideeffects */(function(){throw 0;});",
+            "} else {",
+            "  b = () => {}",
+            "}",
+            "b()");
+    assertPureCallsMarked(source, ImmutableList.of("b"));
+  }
+
+  @Test
+  public void testNoSideEffectsNoPropagationFromOtherCalls() {
+    String source =
+        lines(
+            "var hasSideEffects = function() { throw 0; }",
+            "var foo = /** @nosideeffects */ function(){ hasSideEffects(); return 0; };",
+            "var bar = function(){ hasSideEffects(); return 0; };",
+            "foo();",
+            "bar();");
+    // bar is not marked as pure because it inherits side effects from hasSideEffects
+    assertPureCallsMarked(source, ImmutableList.of("foo"));
+  }
+
+  @Test
+  public void testNoSideEffectsNoPropagationToOtherCalls() {
+    String source =
+        lines(
+            "var liesAboutSideEffects = /** @nosideeffects */ function() { throw 0; }",
+            "var foo = function(){ liesAboutSideEffects(); return 0; };",
+            "var bar = function(){ liesAboutSideEffects(); return 0; };",
+            "foo();",
+            "bar();");
+    assertPureCallsMarked(
+        source, ImmutableList.of("liesAboutSideEffects", "liesAboutSideEffects", "foo", "bar"));
+  }
+
   @Test
   public void testAnnotationInExternStubs1() {
     // In the case where a property is defined first as a stub and then with a FUNCTION: