diff --git a/src/com/google/javascript/jscomp/ChangeVerifier.java b/src/com/google/javascript/jscomp/ChangeVerifier.java new file mode 100644 index 00000000000..6fffd69d77c --- /dev/null +++ b/src/com/google/javascript/jscomp/ChangeVerifier.java @@ -0,0 +1,169 @@ +/* + * Copyright 2017 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.common.base.Preconditions; +import com.google.common.base.Predicates; +import com.google.javascript.jscomp.NodeUtil.Visitor; +import com.google.javascript.rhino.Node; +import java.util.HashMap; +import java.util.Map; + +/** + * A Class to assist in AST change tracking verification. To validate a "snapshot" is taken + * before and "checkRecordedChanges" at the desired check point. + */ +public class ChangeVerifier { + private final AbstractCompiler compiler; + private final Map map = new HashMap<>(); + private int snapshotChange; + + ChangeVerifier(AbstractCompiler compiler) { + this.compiler = compiler; + } + + ChangeVerifier snapshot(Node root) { + // remove any existing snapshot data. + map.clear(); + snapshotChange = compiler.getChangeStamp(); + + Node snapshot = root.cloneTree(); + associateClones(root, snapshot); + return this; + } + + void checkRecordedChanges(Node current) { + checkRecordedChanges("", current); + } + + void checkRecordedChanges(String passName, Node root) { + verifyScopeChangesHaveBeenRecorded(passName, root); + } + + /** + * Given an AST and its copy, map the root node of each scope of main to the + * corresponding root node of clone + */ + private void associateClones(Node n, Node snapshot) { + // TODO(johnlenz): determine if MODULE_BODY is useful here. + if (NodeUtil.isChangeScopeRoot(n)) { + map.put(n, snapshot); + } + + Node child = n.getFirstChild(); + Node snapshotChild = snapshot.getFirstChild(); + while (child != null) { + associateClones(child, snapshotChild); + child = child.getNext(); + snapshotChild = snapshotChild.getNext(); + } + } + + /** Checks that the scope roots marked as changed have indeed changed */ + private void verifyScopeChangesHaveBeenRecorded( + String passName, Node root) { + final String passNameMsg = passName.isEmpty() ? "" : passName + ": "; + + NodeUtil.visitPreOrder(root, + new Visitor() { + @Override + public void visit(Node n) { + if (n.isRoot()) { + verifyRoot(n); + } else if (NodeUtil.isChangeScopeRoot(n)) { + Node clone = map.get(n); + if (clone == null) { + verifyNewNode(n); + } else { + verifyNodeChange(passNameMsg, n, clone); + } + } + } + }, + Predicates.alwaysTrue()); + } + + private void verifyNewNode(Node n) { + // TODO(johnlenz): Verify the new nodes are properly tagged. + } + + private void verifyRoot(Node root) { + Preconditions.checkState(root.isRoot()); + if (root.getChangeTime() != 0) { + throw new IllegalStateException("Root nodes should never be marked as changed."); + } + } + + private void verifyNodeChange(final String passNameMsg, Node n, Node snapshot) { + if (n.isRoot()) { + return; + } + if (n.getChangeTime() > snapshot.getChangeTime()) { + if (isEquivalentToExcludingFunctions(n, snapshot)) { + throw new IllegalStateException( + passNameMsg + "unchanged scope marked as changed: " + n.toStringTree()); + } + } else { + if (!isEquivalentToExcludingFunctions(n, snapshot)) { + throw new IllegalStateException( + passNameMsg + "change scope not marked as changed: " + n.toStringTree()); + } + } + } + + /** + * @return Whether the two node are equivalent while ignoring + * differences any descendant functions differences. + */ + private static boolean isEquivalentToExcludingFunctions( + Node thisNode, Node thatNode) { + if (thisNode == null || thatNode == null) { + return thisNode == null && thatNode == null; + } + if (!thisNode.isEquivalentWithSideEffectsToShallow(thatNode)) { + return false; + } + if (thisNode.getChildCount() != thatNode.getChildCount()) { + return false; + } + Node thisChild = thisNode.getFirstChild(); + Node thatChild = thatNode.getFirstChild(); + while (thisChild != null && thatChild != null) { + if (thisChild.isFunction() || thisChild.isScript()) { + // Don't compare function expression name, parameters or bodies. + // But do check that that the node is there. + if (thatChild.getToken() != thisChild.getToken()) { + return false; + } + // Only compare function names for function declarations (not function expressions) + // as they change the outer scope definition. + if (thisChild.isFunction() && NodeUtil.isFunctionDeclaration(thisChild)) { + String thisName = thisChild.getFirstChild().getString(); + String thatName = thatChild.getFirstChild().getString(); + if (!thisName.equals(thatName)) { + return false; + } + } + } else if (!isEquivalentToExcludingFunctions(thisChild, thatChild)) { + return false; + } + thisChild = thisChild.getNext(); + thatChild = thatChild.getNext(); + } + return true; + } +} + diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index 3402c2c2f4c..3b6bf405952 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -40,7 +40,6 @@ import java.util.Collection; import java.util.Collections; import java.util.EnumSet; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -4726,68 +4725,6 @@ static Node getEnclosingChangeScopeRoot(Node n) { return n; } - /** - * Given an AST and its copy, map the root node of each scope of main to the - * corresponding root node of clone - */ - public static Map mapMainToClone(Node main, Node clone) { - Preconditions.checkState(main.isEquivalentTo(clone)); - Map mtoc = new HashMap<>(); - mtoc.put(main, clone); - mtocHelper(mtoc, main, clone); - return mtoc; - } - - private static void mtocHelper(Map map, Node main, Node clone) { - // TODO(johnlenz): determine if MODULE_BODY is useful here. - if (main.isFunction() || main.isScript()) { - map.put(main, clone); - } - Node mchild = main.getFirstChild(); - Node cchild = clone.getFirstChild(); - while (mchild != null) { - mtocHelper(map, mchild, cchild); - mchild = mchild.getNext(); - cchild = cchild.getNext(); - } - } - - /** Checks that the scope roots marked as changed have indeed changed */ - public static void verifyScopeChanges( - String passName, final Map mtoc, Node main) { - final String passNameMsg = passName.isEmpty() ? "" : passName + ": "; - - Node clone = mtoc.get(main); - verifyNodeChange(passNameMsg, main, clone); - visitPreOrder(main, - new Visitor() { - @Override - public void visit(Node n) { - if ((n.isScript() || n.isFunction()) && mtoc.containsKey(n)) { - Node clone = mtoc.get(n); - verifyNodeChange(passNameMsg, n, clone); - } - } - }, - Predicates.alwaysTrue()); - } - - static void verifyNodeChange(final String passNameMsg, Node n, Node clone) { - if (n.isRoot() && n.getChangeTime() != 0) { - throw new IllegalStateException("Root nodes should never be marked as changed."); - } - if (n.getChangeTime() > clone.getChangeTime()) { - if (isEquivalentToExcludingFunctions(n, clone)) { - throw new IllegalStateException( - passNameMsg + "unchanged scope marked as changed: " + n.toStringTree()); - } - } else { - if (!isEquivalentToExcludingFunctions(n, clone)) { - throw new IllegalStateException( - passNameMsg + "change scope not marked as changed: " + n.toStringTree()); - } - } - } static int countAstSizeUpToLimit(Node n, final int limit) { // Java doesn't allow accessing mutable local variables from another class. @@ -4813,47 +4750,6 @@ static int countAstSize(Node n) { return countAstSizeUpToLimit(n, Integer.MAX_VALUE); } - /** - * @return Whether the two node are equivalent while ignoring - * differences any descendant functions differences. - */ - private static boolean isEquivalentToExcludingFunctions( - Node thisNode, Node thatNode) { - if (thisNode == null || thatNode == null) { - return thisNode == null && thatNode == null; - } - if (!thisNode.isEquivalentWithSideEffectsToShallow(thatNode)) { - return false; - } - if (thisNode.getChildCount() != thatNode.getChildCount()) { - return false; - } - Node thisChild = thisNode.getFirstChild(); - Node thatChild = thatNode.getFirstChild(); - while (thisChild != null && thatChild != null) { - if (thisChild.isFunction() || thisChild.isScript()) { - // Don't compare function expression name, parameters or bodies. - // But do check that that the node is there. - if (thatChild.getToken() != thisChild.getToken()) { - return false; - } - // Only compare function names for function declarations (not function expressions) - // as they change the outer scope definition. - if (thisChild.isFunction() && NodeUtil.isFunctionDeclaration(thisChild)) { - String thisName = thisChild.getFirstChild().getString(); - String thatName = thatChild.getFirstChild().getString(); - if (!thisName.equals(thatName)) { - return false; - } - } - } else if (!isEquivalentToExcludingFunctions(thisChild, thatChild)) { - return false; - } - thisChild = thisChild.getNext(); - thatChild = thatChild.getNext(); - } - return true; - } static JSDocInfo createConstantJsDoc() { JSDocInfoBuilder builder = new JSDocInfoBuilder(false); diff --git a/src/com/google/javascript/jscomp/PhaseOptimizer.java b/src/com/google/javascript/jscomp/PhaseOptimizer.java index 509730027e7..be6fc801eac 100644 --- a/src/com/google/javascript/jscomp/PhaseOptimizer.java +++ b/src/com/google/javascript/jscomp/PhaseOptimizer.java @@ -63,9 +63,8 @@ class PhaseOptimizer implements CompilerPass { private final boolean useSizeHeuristicToStopOptimizationLoop; - // Used for sanity checks between loopable passes - private Node lastAst; - private Map mtoc; // Stands for "main to clone" + // Used for sanity checking passes + private ChangeVerifier changeVerifier; /** * When processing loopable passes in order, the PhaseOptimizer can be in one @@ -189,12 +188,7 @@ Loop addFixedPointLoop() { */ void setSanityCheck(PassFactory sanityCheck) { this.sanityCheck = sanityCheck; - setSanityCheckState(); - } - - private void setSanityCheckState() { - lastAst = jsRoot.cloneTree(); - mtoc = NodeUtil.mapMainToClone(jsRoot, lastAst); + this.changeVerifier = new ChangeVerifier(compiler).snapshot(jsRoot); } /** @@ -248,7 +242,7 @@ private void maybePrintAstHashcodes(String passName, Node root) { private void maybeSanityCheck(String passName, Node externs, Node root) { if (sanityCheck != null) { sanityCheck.create(compiler).process(externs, root); - NodeUtil.verifyScopeChanges(passName, mtoc, jsRoot); + changeVerifier.checkRecordedChanges(passName, jsRoot); } } @@ -275,7 +269,7 @@ public void process(Node externs, Node root) { if (sanityCheck != null) { // Before running the pass, clone the AST so you can sanity-check the // changed AST against the clone after the pass finishes. - setSanityCheckState(); + changeVerifier = new ChangeVerifier(compiler).snapshot(jsRoot); } if (tracker != null) { tracker.recordPassStart(name, factory.isOneTimePass()); diff --git a/test/com/google/javascript/jscomp/ChangeVerifierTest.java b/test/com/google/javascript/jscomp/ChangeVerifierTest.java new file mode 100644 index 00000000000..cb884f836a8 --- /dev/null +++ b/test/com/google/javascript/jscomp/ChangeVerifierTest.java @@ -0,0 +1,111 @@ +/* + * Copyright 2017 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.common.truth.Truth.assertThat; + +import com.google.common.base.Preconditions; +import com.google.javascript.rhino.IR; +import com.google.javascript.rhino.Node; +import junit.framework.TestCase; + +public final class ChangeVerifierTest extends TestCase { + + public void testCorrectValidationOfScriptWithChangeAfterFunction() { + Node script = parse("function A() {} if (0) { A(); }"); + Preconditions.checkState(script.isScript()); + + Compiler compiler = new Compiler(); + compiler.incrementChangeStamp(); + ChangeVerifier verifier = new ChangeVerifier(compiler).snapshot(script); + + // Here we make a change in that doesn't change the script node + // child count. + getCallNode(script).detach(); + + // Mark the script as changed + compiler.incrementChangeStamp(); + script.setChangeTime(compiler.getChangeStamp()); + + // will throw if no change is detected. + verifier.checkRecordedChanges("test1", script); + } + + public void testChangeToScriptNotReported() { + Node script = parse("function A() {} if (0) { A(); }"); + Preconditions.checkState(script.isScript()); + + Compiler compiler = new Compiler(); + compiler.incrementChangeStamp(); + ChangeVerifier verifier = new ChangeVerifier(compiler).snapshot(script); + + // no change + verifier.checkRecordedChanges("test1", script); + + // add a statement, but don't report the change. + script.addChildToBack(IR.exprResult(IR.nullNode())); + + try { + verifier.checkRecordedChanges("test2", script); + fail("exception expected"); + } catch (IllegalStateException e) { + // TODO(johnlenz): use this when we upgrade Trush: + // assertThat(e).hasMessageThat().contains("change scope not marked as changed"); + assertTrue(e.getMessage().contains("change scope not marked as changed")); + } + } + + + public static void testChangeVerification() { + Compiler compiler = new Compiler(); + + Node mainScript = IR.script(); + Node main = IR.root(mainScript); + + ChangeVerifier verifier = new ChangeVerifier(compiler).snapshot(main); + + verifier.checkRecordedChanges(main); + + mainScript.addChildToFront( + IR.function(IR.name("A"), IR.paramList(), IR.block())); + compiler.incrementChangeStamp(); + mainScript.setChangeTime(compiler.getChangeStamp()); + + verifier.checkRecordedChanges(main); + } + + private static Node parse(String js) { + Compiler compiler = new Compiler(); + compiler.initCompilerOptionsIfTesting(); + Node n = compiler.parseTestCode(js); + assertThat(compiler.getErrors()).isEmpty(); + return n; + } + + private static Node getCallNode(Node n) { + if (n.isCall()) { + return n; + } + for (Node c : n.children()) { + Node result = getCallNode(c); + if (result != null) { + return result; + } + } + return null; + } +} diff --git a/test/com/google/javascript/jscomp/CompilerTestCase.java b/test/com/google/javascript/jscomp/CompilerTestCase.java index cd24388ae61..d319309e2a6 100644 --- a/test/com/google/javascript/jscomp/CompilerTestCase.java +++ b/test/com/google/javascript/jscomp/CompilerTestCase.java @@ -38,7 +38,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import junit.framework.TestCase; @@ -1427,9 +1426,10 @@ private void test( recentChange.reset(); - Map mtoc = null; + ChangeVerifier changeVerifier = null; if (checkAstChangeMarking) { - mtoc = NodeUtil.mapMainToClone(mainRoot, mainRoot.cloneTree()); + changeVerifier = new ChangeVerifier(compiler); + changeVerifier.snapshot(mainRoot); } getProcessor(compiler).process(externsRoot, mainRoot); @@ -1438,7 +1438,7 @@ private void test( // TODO(johnlenz): add support for multiple passes in getProcessor so that we can // check the AST marking after each pass runs. // Verify that changes to the AST are properly marked on the AST. - NodeUtil.verifyScopeChanges("", mtoc, mainRoot); + changeVerifier.checkRecordedChanges(mainRoot); } if (astValidationEnabled) { diff --git a/test/com/google/javascript/jscomp/NodeTraversalTest.java b/test/com/google/javascript/jscomp/NodeTraversalTest.java index 360cb6e6bfb..2e45a488605 100644 --- a/test/com/google/javascript/jscomp/NodeTraversalTest.java +++ b/test/com/google/javascript/jscomp/NodeTraversalTest.java @@ -28,7 +28,6 @@ import com.google.javascript.rhino.Token; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.Set; import junit.framework.TestCase; @@ -211,17 +210,16 @@ public void testReportChange4() { "}"); assertChangesRecorded(code, new NameChangingCallback()); } - + private void assertChangesRecorded(String code, NodeTraversal.Callback callback) { final String externs = ""; Compiler compiler = new Compiler(); Node tree = parseRoots(compiler, externs, code); - System.out.println("parse done"); - Node clone = tree.cloneTree(); - Map map = NodeUtil.mapMainToClone(tree, clone); + + ChangeVerifier changeVerifier = new ChangeVerifier(compiler).snapshot(tree); NodeTraversal.traverseRootsEs6( compiler, callback, tree.getFirstChild(), tree.getSecondChild()); - NodeUtil.verifyScopeChanges("test", map, tree); + changeVerifier.checkRecordedChanges(tree); } diff --git a/test/com/google/javascript/jscomp/NodeUtilTest.java b/test/com/google/javascript/jscomp/NodeUtilTest.java index 80d56a81edf..d25c6cfc3bb 100644 --- a/test/com/google/javascript/jscomp/NodeUtilTest.java +++ b/test/com/google/javascript/jscomp/NodeUtilTest.java @@ -32,7 +32,6 @@ import com.google.javascript.rhino.jstype.TernaryValue; import java.util.Collection; import java.util.HashSet; -import java.util.Map; import java.util.Set; import junit.framework.TestCase; @@ -50,7 +49,7 @@ private static Node parse(String js) { return n; } - static Node getNode(String js) { + private static Node getNode(String js) { Node root = parse("var a=(" + js + ");"); Node expr = root.getFirstChild(); Node var = expr.getFirstChild(); @@ -116,19 +115,19 @@ public void testIsLiteralOrConstValue() { assertNotLiteral(getNode("void foo()")); } - public void assertLiteralAndImmutable(Node n) { + private void assertLiteralAndImmutable(Node n) { assertTrue(NodeUtil.isLiteralValue(n, true)); assertTrue(NodeUtil.isLiteralValue(n, false)); assertTrue(NodeUtil.isImmutableValue(n)); } - public void assertLiteralButNotImmutable(Node n) { + private void assertLiteralButNotImmutable(Node n) { assertTrue(NodeUtil.isLiteralValue(n, true)); assertTrue(NodeUtil.isLiteralValue(n, false)); assertFalse(NodeUtil.isImmutableValue(n)); } - public void assertNotLiteral(Node n) { + private void assertNotLiteral(Node n) { assertFalse(NodeUtil.isLiteralValue(n, true)); assertFalse(NodeUtil.isLiteralValue(n, false)); assertFalse(NodeUtil.isImmutableValue(n)); @@ -296,21 +295,21 @@ public void testGetArrayStringValue() { } public void testIsObjectLiteralKey1() throws Exception { - testIsObjectLiteralKey( + assertIsObjectLiteralKey( parseExpr("({})"), false); - testIsObjectLiteralKey( + assertIsObjectLiteralKey( parseExpr("a"), false); - testIsObjectLiteralKey( + assertIsObjectLiteralKey( parseExpr("'a'"), false); - testIsObjectLiteralKey( + assertIsObjectLiteralKey( parseExpr("1"), false); - testIsObjectLiteralKey( + assertIsObjectLiteralKey( parseExpr("({a: 1})").getFirstChild(), true); - testIsObjectLiteralKey( + assertIsObjectLiteralKey( parseExpr("({1: 1})").getFirstChild(), true); - testIsObjectLiteralKey( + assertIsObjectLiteralKey( parseExpr("({get a(){}})").getFirstChild(), true); - testIsObjectLiteralKey( + assertIsObjectLiteralKey( parseExpr("({set a(b){}})").getFirstChild(), true); } @@ -319,41 +318,41 @@ private Node parseExpr(String js) { return root.getFirstFirstChild(); } - private void testIsObjectLiteralKey(Node node, boolean expected) { + private void assertIsObjectLiteralKey(Node node, boolean expected) { assertEquals(expected, NodeUtil.isObjectLitKey(node)); } public void testGetFunctionName1() throws Exception { Node parent = parse("function name(){}"); - testGetFunctionName(parent.getFirstChild(), "name"); + assertGetNameResult(parent.getFirstChild(), "name"); } public void testGetFunctionName2() throws Exception { Node parent = parse("var name = function(){}") .getFirstFirstChild(); - testGetFunctionName(parent.getFirstChild(), "name"); + assertGetNameResult(parent.getFirstChild(), "name"); } public void testGetFunctionName3() throws Exception { Node parent = parse("qualified.name = function(){}") .getFirstFirstChild(); - testGetFunctionName(parent.getLastChild(), "qualified.name"); + assertGetNameResult(parent.getLastChild(), "qualified.name"); } public void testGetFunctionName4() throws Exception { Node parent = parse("var name2 = function name1(){}") .getFirstFirstChild(); - testGetFunctionName(parent.getFirstChild(), "name2"); + assertGetNameResult(parent.getFirstChild(), "name2"); } public void testGetFunctionName5() throws Exception { Node n = parse("qualified.name2 = function name1(){}"); Node parent = n.getFirstFirstChild(); - testGetFunctionName(parent.getLastChild(), "qualified.name2"); + assertGetNameResult(parent.getLastChild(), "qualified.name2"); } public void testGetBestFunctionName1() throws Exception { @@ -371,7 +370,7 @@ public void testGetBestFunctionName2() throws Exception { NodeUtil.getNearestFunctionName(parent.getLastChild())); } - private void testGetFunctionName(Node function, String name) { + private void assertGetNameResult(Node function, String name) { assertEquals(Token.FUNCTION, function.getToken()); assertEquals(name, NodeUtil.getName(function)); } @@ -1166,109 +1165,109 @@ public void testGetSourceName() { public void testLocalValue1() throws Exception { // Names are not known to be local. - assertFalse(testLocalValue("x")); - assertFalse(testLocalValue("x()")); - assertFalse(testLocalValue("this")); - assertFalse(testLocalValue("arguments")); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("x"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("x()"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("this"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("arguments"))); // We can't know if new objects are local unless we know // that they don't alias themselves. // TODO(tdeegan): Revisit this. - assertFalse(testLocalValue("new x()")); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("new x()"))); // property references are assumed to be non-local - assertFalse(testLocalValue("(new x()).y")); - assertFalse(testLocalValue("(new x())['y']")); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("(new x()).y"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("(new x())['y']"))); // Primitive values are local - assertTrue(testLocalValue("null")); - assertTrue(testLocalValue("undefined")); - assertTrue(testLocalValue("Infinity")); - assertTrue(testLocalValue("NaN")); - assertTrue(testLocalValue("1")); - assertTrue(testLocalValue("'a'")); - assertTrue(testLocalValue("true")); - assertTrue(testLocalValue("false")); - assertTrue(testLocalValue("[]")); - assertTrue(testLocalValue("{}")); - - assertFalse(testLocalValue("[x]")); - assertFalse(testLocalValue("{'a':x}")); - assertTrue(testLocalValue("{'a': {'b': 2}}")); - assertFalse(testLocalValue("{'a': {'b': global}}")); - assertTrue(testLocalValue("{get someGetter() { return 1; }}")); - assertTrue(testLocalValue("{get someGetter() { return global; }}")); - assertTrue(testLocalValue("{set someSetter(value) {}}")); - assertTrue(testLocalValue("{[someComputedProperty]: {}}")); - assertFalse(testLocalValue("{[someComputedProperty]: global}")); - assertFalse(testLocalValue("{[someComputedProperty]: {'a':x}}")); - assertTrue(testLocalValue("{[someComputedProperty]: {'a':1}}")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("null"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("undefined"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("Infinity"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("NaN"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("1"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("'a'"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("true"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("false"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("[]"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("{}"))); + + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("[x]"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("{'a':x}"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("{'a': {'b': 2}}"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("{'a': {'b': global}}"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("{get someGetter() { return 1; }}"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("{get someGetter() { return global; }}"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("{set someSetter(value) {}}"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("{[someComputedProperty]: {}}"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("{[someComputedProperty]: global}"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("{[someComputedProperty]: {'a':x}}"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("{[someComputedProperty]: {'a':1}}"))); // increment/decrement results in primitive number, the previous value is // always coersed to a number (even in the post. - assertTrue(testLocalValue("++x")); - assertTrue(testLocalValue("--x")); - assertTrue(testLocalValue("x++")); - assertTrue(testLocalValue("x--")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("++x"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("--x"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("x++"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("x--"))); // The left side of an only assign matters if it is an alias or mutable. - assertTrue(testLocalValue("x=1")); - assertFalse(testLocalValue("x=[]")); - assertFalse(testLocalValue("x=y")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("x=1"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("x=[]"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("x=y"))); // The right hand side of assignment opts don't matter, as they force // a local result. - assertTrue(testLocalValue("x+=y")); - assertTrue(testLocalValue("x*=y")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("x+=y"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("x*=y"))); // Comparisons always result in locals, as they force a local boolean // result. - assertTrue(testLocalValue("x==y")); - assertTrue(testLocalValue("x!=y")); - assertTrue(testLocalValue("x>y")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("x==y"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("x!=y"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("x>y"))); // Only the right side of a comma matters - assertTrue(testLocalValue("(1,2)")); - assertTrue(testLocalValue("(x,1)")); - assertFalse(testLocalValue("(x,y)")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("(1,2)"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("(x,1)"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("(x,y)"))); // Both the operands of OR matter - assertTrue(testLocalValue("1||2")); - assertFalse(testLocalValue("x||1")); - assertFalse(testLocalValue("x||y")); - assertFalse(testLocalValue("1||y")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("1||2"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("x||1"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("x||y"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("1||y"))); // Both the operands of AND matter - assertTrue(testLocalValue("1&&2")); - assertFalse(testLocalValue("x&&1")); - assertFalse(testLocalValue("x&&y")); - assertFalse(testLocalValue("1&&y")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("1&&2"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("x&&1"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("x&&y"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("1&&y"))); // Only the results of HOOK matter - assertTrue(testLocalValue("x?1:2")); - assertFalse(testLocalValue("x?x:2")); - assertFalse(testLocalValue("x?1:x")); - assertFalse(testLocalValue("x?x:y")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("x?1:2"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("x?x:2"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("x?1:x"))); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("x?x:y"))); // Results of ops are local values - assertTrue(testLocalValue("!y")); - assertTrue(testLocalValue("~y")); - assertTrue(testLocalValue("y + 1")); - assertTrue(testLocalValue("y + z")); - assertTrue(testLocalValue("y * z")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("!y"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("~y"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("y + 1"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("y + z"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("y * z"))); - assertTrue(testLocalValue("'a' in x")); - assertTrue(testLocalValue("typeof x")); - assertTrue(testLocalValue("x instanceof y")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("'a' in x"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("typeof x"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("x instanceof y"))); - assertTrue(testLocalValue("void x")); - assertTrue(testLocalValue("void 0")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("void x"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("void 0"))); - assertFalse(testLocalValue("{}.x")); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("{}.x"))); - assertTrue(testLocalValue("{}.toString()")); - assertTrue(testLocalValue("o.toString()")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("{}.toString()"))); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("o.toString()"))); - assertFalse(testLocalValue("o.valueOf()")); + assertFalse(NodeUtil.evaluatesToLocalValue(getNode("o.valueOf()"))); - assertTrue(testLocalValue("delete a.b")); + assertTrue(NodeUtil.evaluatesToLocalValue(getNode("delete a.b"))); } public void testLocalValue2() { @@ -1379,34 +1378,30 @@ public void testCallSideEffects() { assertTrue(NodeUtil.mayHaveSideEffects(callExpr)); } - private boolean testLocalValue(String js) { - return NodeUtil.evaluatesToLocalValue(getNode(js)); - } - public void testValidDefine() { - assertTrue(testValidDefineValue("1")); - assertTrue(testValidDefineValue("-3")); - assertTrue(testValidDefineValue("true")); - assertTrue(testValidDefineValue("false")); - assertTrue(testValidDefineValue("'foo'")); + assertTrue(getIsValidDefineValueResultFor("1")); + assertTrue(getIsValidDefineValueResultFor("-3")); + assertTrue(getIsValidDefineValueResultFor("true")); + assertTrue(getIsValidDefineValueResultFor("false")); + assertTrue(getIsValidDefineValueResultFor("'foo'")); - assertFalse(testValidDefineValue("x")); - assertFalse(testValidDefineValue("null")); - assertFalse(testValidDefineValue("undefined")); - assertFalse(testValidDefineValue("NaN")); + assertFalse(getIsValidDefineValueResultFor("x")); + assertFalse(getIsValidDefineValueResultFor("null")); + assertFalse(getIsValidDefineValueResultFor("undefined")); + assertFalse(getIsValidDefineValueResultFor("NaN")); - assertTrue(testValidDefineValue("!true")); - assertTrue(testValidDefineValue("-true")); - assertTrue(testValidDefineValue("1 & 8")); - assertTrue(testValidDefineValue("1 + 8")); - assertTrue(testValidDefineValue("'a' + 'b'")); - assertTrue(testValidDefineValue("true ? 'a' : 'b'")); + assertTrue(getIsValidDefineValueResultFor("!true")); + assertTrue(getIsValidDefineValueResultFor("-true")); + assertTrue(getIsValidDefineValueResultFor("1 & 8")); + assertTrue(getIsValidDefineValueResultFor("1 + 8")); + assertTrue(getIsValidDefineValueResultFor("'a' + 'b'")); + assertTrue(getIsValidDefineValueResultFor("true ? 'a' : 'b'")); - assertFalse(testValidDefineValue("1 & foo")); - assertFalse(testValidDefineValue("foo ? 'a' : 'b'")); + assertFalse(getIsValidDefineValueResultFor("1 & foo")); + assertFalse(getIsValidDefineValueResultFor("foo ? 'a' : 'b'")); } - private boolean testValidDefineValue(String js) { + private boolean getIsValidDefineValueResultFor(String js) { Node script = parse("var test = " + js + ";"); Node var = script.getFirstChild(); Node name = var.getFirstChild(); @@ -1691,10 +1686,6 @@ public void testMayBeString() { assertTrue(NodeUtil.mayBeString(getNode("a += 1"))); } - public void test1() { - assertFalse(NodeUtil.isStringResult(getNode("a+b"))); - } - public void testIsStringResult() { assertFalse(NodeUtil.isStringResult(getNode("1"))); assertFalse(NodeUtil.isStringResult(getNode("true"))); @@ -2327,11 +2318,11 @@ public void testLhsByDestructuring7e() { assertLhsByDestructuring(nameNodeA); } - static void assertLhsByDestructuring(Node n) { + private static void assertLhsByDestructuring(Node n) { assertTrue(NodeUtil.isLhsByDestructuring(n)); } - static void assertNotLhsByDestructuring(Node n) { + private static void assertNotLhsByDestructuring(Node n) { assertFalse(NodeUtil.isLhsByDestructuring(n)); } @@ -2491,24 +2482,6 @@ public void testIsObjectDefinePropertyDefinition() { getCallNode("Object.defineProperty();"))); } - public void testCorrectValidationOfScriptWithChangeAfterFunction() { - Node script = parse("function A() {} if (0) { A(); }"); - Preconditions.checkState(script.isScript()); - - Node clone = script.cloneTree(); - Map mtoc = NodeUtil.mapMainToClone(script, clone); - - // Here we make a change in that doesn't change the script node - // child count. - getCallNode(script).detach(); - - // Mark the script as changed - script.setChangeTime(100); - - // will throw if no change is detected. - NodeUtil.verifyScopeChanges("test", mtoc, script); - } - private boolean executedOnceTestCase(String code) { Node ast = parse(code); Node nameNode = getNameNode(ast, "x"); @@ -2534,40 +2507,23 @@ private void assertNodeTreesEqual( assertNull(error, error); } - static void testFunctionName(String js, String expected) { + private static void testFunctionName(String js, String expected) { assertEquals( expected, NodeUtil.getNearestFunctionName(getFunctionNode(js))); } - public static void testChangeVerification() { - Node mainScript = IR.script(); - Node main = IR.root(mainScript); - - Node clone = main.cloneTree(); - - Map map = NodeUtil.mapMainToClone(main, clone); - - NodeUtil.verifyScopeChanges("", map, main); - - mainScript.addChildToFront( - IR.function(IR.name("A"), IR.paramList(), IR.block())); - mainScript.setChangeTime(100); - - NodeUtil.verifyScopeChanges("", map, main); - } - - static Node getClassNode(String js) { + private static Node getClassNode(String js) { Node root = parse(js); return getClassNode(root); } - static Iterable getLhsNodesOfDeclaration(String js) { + private static Iterable getLhsNodesOfDeclaration(String js) { Node root = parse(js); return NodeUtil.getLhsNodesOfDeclaration(root.getFirstChild()); } - static Node getClassNode(Node n) { + private static Node getClassNode(Node n) { if (n.isClass()) { return n; } @@ -2580,12 +2536,12 @@ static Node getClassNode(Node n) { return null; } - static Node getCallNode(String js) { + private static Node getCallNode(String js) { Node root = parse(js); return getCallNode(root); } - static Node getCallNode(Node n) { + private static Node getCallNode(Node n) { if (n.isCall()) { return n; } @@ -2598,12 +2554,12 @@ static Node getCallNode(Node n) { return null; } - static Node getFunctionNode(String js) { + private static Node getFunctionNode(String js) { Node root = parse(js); return getFunctionNode(root); } - static Node getFunctionNode(Node n) { + private static Node getFunctionNode(Node n) { if (n.isFunction()) { return n; } @@ -2616,7 +2572,7 @@ static Node getFunctionNode(Node n) { return null; } - static Node getNameNode(Node n, String name) { + private static Node getNameNode(Node n, String name) { if (n.isName() && n.getString().equals(name)) { return n; } @@ -2629,11 +2585,11 @@ static Node getNameNode(Node n, String name) { return null; } - static boolean isValidPropertyName(String s) { + private static boolean isValidPropertyName(String s) { return NodeUtil.isValidPropertyName(LanguageMode.ECMASCRIPT3, s); } - static boolean isValidQualifiedName(String s) { + private static boolean isValidQualifiedName(String s) { return NodeUtil.isValidQualifiedName(LanguageMode.ECMASCRIPT3, s); } }