Remove NodeUtil.getPureBooleanValue()

This also allows removal of the final call to mayHaveSideEffects()
that doesn't pass in a compiler value.

Caller of getImpureBooleanValue() or getLiteralBooleanValue() is
responsible for checking for side effects.
A followup change will merge those two methods into just
getBooleanValue().

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

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

@@ -20,6 +20,7 @@
 
 import com.google.javascript.jscomp.parsing.parser.FeatureSet;
 import com.google.javascript.rhino.Node;
+import com.google.javascript.rhino.jstype.TernaryValue;
 
 /**
  * An abstract class whose implementations run peephole optimizations:
@@ -130,6 +131,24 @@ protected String getSideEffectFreeStringValue(Node n) {
     return value;
   }
 
+  /**
+   * Calculate the known boolean value for a node if possible and if it has no side effects.
+   *
+   * <p>Returns {@link TernaryValue#UNKNOWN} if the node has side effects or its value cannot be
+   * statically determined.
+   */
+  protected TernaryValue getSideEffectFreeBooleanValue(Node n) {
+    TernaryValue value = NodeUtil.getLiteralBooleanValue(n);
+    // Calculating the boolean value, if any, is likely to be faster than calculating side effects,
+    // and there are only a very few cases where we can compute a boolean value, but there could
+    // also be side effects. e.g. `void doSomething()` has value `false`, regardless of the
+    // behavior of `doSomething()`
+    if (value != TernaryValue.UNKNOWN && astAnalyzer.mayHaveSideEffects(n)) {
+      value = TernaryValue.UNKNOWN;
+    }
+    return value;
+  }
+
   /**
    * Returns true if the current node's type implies side effects.
    *

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

@@ -51,7 +51,7 @@ Node optimizeSubtree(Node n) {
         tryMinimizeExits(NodeUtil.getLoopCodeBlock(n), Token.CONTINUE, null);
 
         Node cond = NodeUtil.getConditionExpression(n);
-        if (NodeUtil.getPureBooleanValue(cond) == TernaryValue.FALSE) {
+        if (getSideEffectFreeBooleanValue(cond) == TernaryValue.FALSE) {
           // Normally, we wouldn't be able to optimize BREAKs inside a loop
           // but as we know the condition will always be false, we can treat them
           // as we would a CONTINUE.

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

@@ -85,8 +85,7 @@ private NodeUtil() {}
    * consisting of literals when possible. Use that method instead if you only want to find literal
    * values.
    *
-   * <p>This method does not consider whether the node may have side-effects. Use {@link
-   * #getPureBooleanValue(Node)} if you need to avoid side-effects.
+   * <p>This method does not consider whether the node may have side-effects.
    */
   static TernaryValue getImpureBooleanValue(Node n) {
     // This switch consists of cases that are not supported by getLiteralBooleanValue(),
@@ -141,8 +140,7 @@ static TernaryValue getImpureBooleanValue(Node n) {
    * </pre></code>
    *
    * <p>This method does not consider whether the literal might have side-effects when evaluated.
-   * This can be true for object literals, for example. Be sure to call {@link
-   * #getPureBooleanValue(Node)} if you need to care about side-effects.
+   * This can be true for object literals, for example.
    *
    * <p>Except for the cases listed above, this method also returns {@code TernaryValue.UNKNOWN} for
    * boolean expressions even when it is possible to determine their value. If you want to evaluate
@@ -203,25 +201,6 @@ static TernaryValue getLiteralBooleanValue(Node n) {
     return TernaryValue.UNKNOWN;
   }
 
-  /**
-   * If {@code n} represents a literal value without side-effects that can be interpreted statically
-   * as a known boolean value, this method returns the corresponding {@code TernaryValue}.
-   * Otherwise, it returns {@code TernaryValue.UNKNOWN}.
-   *
-   * <p>Use {@link #getLiteralBooleanValue(Node)} if you don't care about side-effects. Use {@link
-   * #getImpureBooleanValue(Node)} if you don't care about side-effects and also want to compute
-   * values for non-literal expressions.
-   */
-  static TernaryValue getPureBooleanValue(Node n) {
-    // Determining whether n is a literal boolean is likely to be faster than determining whether
-    // it has side-effects when n is a large subtree.
-    TernaryValue value = getLiteralBooleanValue(n);
-    if (value != TernaryValue.UNKNOWN && mayHaveSideEffects(n)) {
-      value = TernaryValue.UNKNOWN;
-    }
-    return value;
-  }
-
   /**
    * Gets the value of a node as a String, or null if it cannot be converted. When it returns a
    * non-null String, this method effectively emulates the <code>String()</code> JavaScript cast
@@ -1094,12 +1073,6 @@ static boolean mayEffectMutableState(Node n, AbstractCompiler compiler) {
     return checkForStateChangeHelper(n, true, compiler);
   }
 
-  // TODO(johnplaisted): All call sites should pass in the compiler.
-  @Deprecated
-  public static boolean mayHaveSideEffects(Node n) {
-    return mayHaveSideEffects(n, null);
-  }
-
   /**
    * Returns true if the node which may have side effects when executed.
    * This version default to the "safe" assumptions when the compiler object is not

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

@@ -355,7 +355,7 @@ private Node tryFoldUnaryOperator(Node n) {
       return n;
     }
 
-    TernaryValue leftVal = NodeUtil.getPureBooleanValue(left);
+    TernaryValue leftVal = getSideEffectFreeBooleanValue(left);
     if (leftVal == TernaryValue.UNKNOWN) {
       return n;
     }
@@ -1178,8 +1178,8 @@ private static TernaryValue tryStrictEqualityComparison(
           }
         case BOOLEAN:
           {
-            TernaryValue lv = NodeUtil.getPureBooleanValue(left);
-            TernaryValue rv = NodeUtil.getPureBooleanValue(right);
+            TernaryValue lv = peepholeOptimization.getSideEffectFreeBooleanValue(left);
+            TernaryValue rv = peepholeOptimization.getSideEffectFreeBooleanValue(right);
             return lv.and(rv).or(lv.not().and(rv.not()));
           }
         default: // Symbol and Object cannot be folded in the general case.

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

@@ -1033,8 +1033,8 @@ private Node performConditionSubstitutions(Node n) {
           // In the last two cases, code size may increase slightly (adding
           // some parens because the comma operator has a low precedence) but
           // the new AST is easier for other passes to handle.
-          TernaryValue rightVal = NodeUtil.getPureBooleanValue(right);
-          if (NodeUtil.getPureBooleanValue(right) != TernaryValue.UNKNOWN) {
+          TernaryValue rightVal = getSideEffectFreeBooleanValue(right);
+          if (getSideEffectFreeBooleanValue(right) != TernaryValue.UNKNOWN) {
             Token type = n.getToken();
             Node replacement = null;
             boolean rval = rightVal.toBoolean(true);
@@ -1083,8 +1083,8 @@ private Node performConditionSubstitutions(Node n) {
           //   Only when x is NAME, hence x does not have side effects
           //   x ? x : y        --> x || y
           Node replacement = null;
-          TernaryValue trueNodeVal = NodeUtil.getPureBooleanValue(trueNode);
-          TernaryValue falseNodeVal = NodeUtil.getPureBooleanValue(falseNode);
+          TernaryValue trueNodeVal = getSideEffectFreeBooleanValue(trueNode);
+          TernaryValue falseNodeVal = getSideEffectFreeBooleanValue(falseNode);
           if (trueNodeVal == TernaryValue.TRUE && falseNodeVal == TernaryValue.FALSE) {
             // Remove useless conditionals, keep the condition
             condition.detach();
@@ -1120,7 +1120,7 @@ private Node performConditionSubstitutions(Node n) {
 
       default:
         // while(true) --> while(1)
-        TernaryValue nVal = NodeUtil.getPureBooleanValue(n);
+        TernaryValue nVal = getSideEffectFreeBooleanValue(n);
         if (nVal != TernaryValue.UNKNOWN) {
           boolean result = nVal.toBoolean(true);
           int equivalentResult = result ? 1 : 0;

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

@@ -1070,22 +1070,6 @@ private Node tryFoldHook(Node n) {
     return replacement;
   }
 
-  /**
-   * Removes WHILEs that always evaluate to false.
-   */
-  Node tryFoldWhile(Node n) {
-    checkArgument(n.isWhile());
-    Node cond = NodeUtil.getConditionExpression(n);
-    if (NodeUtil.getPureBooleanValue(cond) != TernaryValue.FALSE) {
-      return n;
-    }
-    NodeUtil.redeclareVarsInsideBranch(n);
-    reportChangeToEnclosingScope(n.getParent());
-    NodeUtil.removeChild(n.getParent(), n);
-
-    return null;
-  }
-
   /**
    * Removes FORs that always evaluate to false.
    */
@@ -1265,7 +1249,7 @@ static boolean hasUnnamedBreakOrContinue(Node n) {
    * Remove always true loop conditions.
    */
   private void tryFoldForCondition(Node forCondition) {
-    if (NodeUtil.getPureBooleanValue(forCondition) == TernaryValue.TRUE) {
+    if (getSideEffectFreeBooleanValue(forCondition) == TernaryValue.TRUE) {
       reportChangeToEnclosingScope(forCondition);
       forCondition.replaceWith(IR.empty());
     }

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

@@ -198,97 +198,87 @@ public static final class BooleanValueTests {
     @Parameter(0)
     public String jsExpression;
 
-    /** Expected result of NodeUtil.getPureBooleanValue() */
-    @Parameter(1)
-    public TernaryValue expectedPureResult;
-
     /** Expected result of NodeUtil.getLiteralBooleanValue() */
-    @Parameter(2)
+    @Parameter(1)
     public TernaryValue expectedLiteralResult;
 
     /** Expected result of NodeUtil.getImpureBooleanValue() */
-    @Parameter(3)
+    @Parameter(2)
     public TernaryValue expectedImpureResult;
 
-    @Parameters(name = "\"{0}\" is pure ({1}) literal ({2}) impure ({3})")
+    @Parameters(name = "\"{0}\" is literal ({1}) impure ({2})")
     public static Iterable<Object[]> cases() {
       return ImmutableList.copyOf(
           new Object[][] {
             // truly literal, side-effect free values are always known
-            {"true", TernaryValue.TRUE, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"10", TernaryValue.TRUE, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"'0'", TernaryValue.TRUE, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"/a/", TernaryValue.TRUE, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"{}", TernaryValue.TRUE, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"[]", TernaryValue.TRUE, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"false", TernaryValue.FALSE, TernaryValue.FALSE, TernaryValue.FALSE},
-            {"null", TernaryValue.FALSE, TernaryValue.FALSE, TernaryValue.FALSE},
-            {"0", TernaryValue.FALSE, TernaryValue.FALSE, TernaryValue.FALSE},
-            {"''", TernaryValue.FALSE, TernaryValue.FALSE, TernaryValue.FALSE},
-            {"undefined", TernaryValue.FALSE, TernaryValue.FALSE, TernaryValue.FALSE},
+            {"true", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"10", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"'0'", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"/a/", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"{}", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"[]", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"false", TernaryValue.FALSE, TernaryValue.FALSE},
+            {"null", TernaryValue.FALSE, TernaryValue.FALSE},
+            {"0", TernaryValue.FALSE, TernaryValue.FALSE},
+            {"''", TernaryValue.FALSE, TernaryValue.FALSE},
+            {"undefined", TernaryValue.FALSE, TernaryValue.FALSE},
 
             // literals that have side-effects aren't pure
-            {"{a:foo()}", TernaryValue.UNKNOWN, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"[foo()]", TernaryValue.UNKNOWN, TernaryValue.TRUE, TernaryValue.TRUE},
+            {"{a:foo()}", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"[foo()]", TernaryValue.TRUE, TernaryValue.TRUE},
 
             // not really literals, but we pretend they are for our purposes
-            {"void 0", TernaryValue.FALSE, TernaryValue.FALSE, TernaryValue.FALSE},
+            {"void 0", TernaryValue.FALSE, TernaryValue.FALSE},
             // side-effect keeps this one from being pure
-            {"void foo()", TernaryValue.UNKNOWN, TernaryValue.FALSE, TernaryValue.FALSE},
-            {"!true", TernaryValue.FALSE, TernaryValue.FALSE, TernaryValue.FALSE},
-            {"!false", TernaryValue.TRUE, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"!''", TernaryValue.TRUE, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"class Klass {}", TernaryValue.TRUE, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"new Date()", TernaryValue.TRUE, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"b", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
-            {"-'0.0'", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
+            {"void foo()", TernaryValue.FALSE, TernaryValue.FALSE},
+            {"!true", TernaryValue.FALSE, TernaryValue.FALSE},
+            {"!false", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"!''", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"class Klass {}", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"new Date()", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"b", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
+            {"-'0.0'", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
 
             // template literals
-            {"``", TernaryValue.FALSE, TernaryValue.FALSE, TernaryValue.FALSE},
-            {"`definiteLength`", TernaryValue.TRUE, TernaryValue.TRUE, TernaryValue.TRUE},
-            {"`${some}str`", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
+            {"``", TernaryValue.FALSE, TernaryValue.FALSE},
+            {"`definiteLength`", TernaryValue.TRUE, TernaryValue.TRUE},
+            {"`${some}str`", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
 
             // non-literal expressions
-            {"a=true", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.TRUE},
-            {"a=false", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.FALSE},
-            {"a=(false,true)", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.TRUE},
-            {"a=(true,false)", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.FALSE},
-            {"a=(false || true)", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.TRUE},
-            {"a=(true && false)", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.FALSE},
-            {"a=!(true && false)", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.TRUE},
-            {"a,true", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.TRUE},
-            {"a,false", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.FALSE},
-            {"true||false", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.TRUE},
-            {"false||false", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.FALSE},
-            {"true&&true", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.TRUE},
-            {"true&&false", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.FALSE},
+            {"a=true", TernaryValue.UNKNOWN, TernaryValue.TRUE},
+            {"a=false", TernaryValue.UNKNOWN, TernaryValue.FALSE},
+            {"a=(false,true)", TernaryValue.UNKNOWN, TernaryValue.TRUE},
+            {"a=(true,false)", TernaryValue.UNKNOWN, TernaryValue.FALSE},
+            {"a=(false || true)", TernaryValue.UNKNOWN, TernaryValue.TRUE},
+            {"a=(true && false)", TernaryValue.UNKNOWN, TernaryValue.FALSE},
+            {"a=!(true && false)", TernaryValue.UNKNOWN, TernaryValue.TRUE},
+            {"a,true", TernaryValue.UNKNOWN, TernaryValue.TRUE},
+            {"a,false", TernaryValue.UNKNOWN, TernaryValue.FALSE},
+            {"true||false", TernaryValue.UNKNOWN, TernaryValue.TRUE},
+            {"false||false", TernaryValue.UNKNOWN, TernaryValue.FALSE},
+            {"true&&true", TernaryValue.UNKNOWN, TernaryValue.TRUE},
+            {"true&&false", TernaryValue.UNKNOWN, TernaryValue.FALSE},
 
             // Assignment ops other than ASSIGN are unknown.
-            {"a *= 2", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
+            {"a *= 2", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
 
             // Complex expressions that contain anything other then "=", ",", or "!" are
             // unknown.
-            {"2 + 2", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
+            {"2 + 2", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
 
             // assignment values are the RHS
-            {"a=1", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.TRUE},
-            {"a=/a/", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.TRUE},
-            {"a={}", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.TRUE},
+            {"a=1", TernaryValue.UNKNOWN, TernaryValue.TRUE},
+            {"a=/a/", TernaryValue.UNKNOWN, TernaryValue.TRUE},
+            {"a={}", TernaryValue.UNKNOWN, TernaryValue.TRUE},
 
             // hooks have impure boolean value if both cases have same impure boolean value
-            {"a?true:true", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.TRUE},
-            {"a?false:false", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.FALSE},
-            {"a?true:false", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
-            {"a?true:foo()", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
+            {"a?true:true", TernaryValue.UNKNOWN, TernaryValue.TRUE},
+            {"a?false:false", TernaryValue.UNKNOWN, TernaryValue.FALSE},
+            {"a?true:false", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
+            {"a?true:foo()", TernaryValue.UNKNOWN, TernaryValue.UNKNOWN},
           });
     }
 
-    @Test
-    public void getPureBooleanValue() {
-      assertThat(NodeUtil.getPureBooleanValue(parseExpr(jsExpression)))
-          .isEqualTo(expectedPureResult);
-    }
-
     @Test
     public void getLiteralBooleanValue() {
       assertThat(NodeUtil.getLiteralBooleanValue(parseExpr(jsExpression)))