diff --git a/src/com/google/javascript/jscomp/ReplaceMessages.java b/src/com/google/javascript/jscomp/ReplaceMessages.java index 029025804cd..db3ff82196e 100644 --- a/src/com/google/javascript/jscomp/ReplaceMessages.java +++ b/src/com/google/javascript/jscomp/ReplaceMessages.java @@ -183,9 +183,12 @@ private void updateFunctionNode(JsMessage message, Node functionNode) Node valueNode = constructAddOrStringNode(iterator, argListNode); Node newBlockNode = IR.block(IR.returnNode(valueNode)); - // TODO(user): checkTreeEqual is overkill. I am in process of rewriting - // these functions. - if (newBlockNode.checkTreeEquals(oldBlockNode) != null) { + if (!newBlockNode.isEquivalentTo( + oldBlockNode, + /* compareType= */ false, + /* recurse= */ true, + /* jsDoc= */ false, + /* sideEffect= */ false)) { newBlockNode.useSourceInfoIfMissingFromForTree(oldBlockNode); functionNode.replaceChild(oldBlockNode, newBlockNode); compiler.reportChangeToEnclosingScope(newBlockNode); diff --git a/src/com/google/javascript/rhino/Node.java b/src/com/google/javascript/rhino/Node.java index a7738126c8b..f6531de4f6f 100644 --- a/src/com/google/javascript/rhino/Node.java +++ b/src/com/google/javascript/rhino/Node.java @@ -1880,99 +1880,6 @@ public final boolean hasChild(Node child) { return false; } - /** - * Checks if the subtree under this node is the same as another subtree. Returns null if it's - * equal, or a message describing the differences. Should be called with {@code this} as the - * "expected" node and {@code actual} as the "actual" node. - */ - @VisibleForTesting - @Nullable - public final String checkTreeEquals(Node actual) { - NodeMismatch diff = checkTreeEqualsImpl(actual); - if (diff != null) { - return "Node tree inequality:" + - "\nTree1:\n" + toStringTree() + - "\n\nTree2:\n" + actual.toStringTree() + - "\n\nSubtree1: " + diff.nodeExpected.toStringTree() + - "\n\nSubtree2: " + diff.nodeActual.toStringTree(); - } - return null; - } - - /** - * Checks if the subtree under this node is the same as another subtree. Returns null if it's - * equal, or a message describing the differences. Considers two nodes to be unequal if their - * JSDocInfo doesn't match. Should be called with {@code this} as the "expected" node and {@code - * actual} as the "actual" node. - * - * @see JSDocInfo#equals(Object) - */ - @VisibleForTesting - @Nullable - public final String checkTreeEqualsIncludingJsDoc(Node actual) { - NodeMismatch diff = checkTreeEqualsImpl(actual, true); - if (diff != null) { - if (diff.nodeActual.isEquivalentTo(diff.nodeExpected, false, true, false)) { - // The only difference is that the JSDoc is different on - // the subtree. - String jsDocActual = diff.nodeActual.getJSDocInfo() == null ? - "(none)" : - diff.nodeActual.getJSDocInfo().toStringVerbose(); - - String jsDocExpected = diff.nodeExpected.getJSDocInfo() == null ? - "(none)" : - diff.nodeExpected.getJSDocInfo().toStringVerbose(); - - return "Node tree inequality:" + - "\nTree:\n" + toStringTree() + - "\n\nJSDoc differs on subtree: " + diff.nodeExpected + - "\nExpected JSDoc: " + jsDocExpected + - "\nActual JSDoc : " + jsDocActual; - } - return "Node tree inequality:" + - "\nExpected tree:\n" + toStringTree() + - "\n\nActual tree:\n" + actual.toStringTree() + - "\n\nExpected subtree: " + diff.nodeExpected.toStringTree() + - "\n\nActual subtree: " + diff.nodeActual.toStringTree(); - } - return null; - } - - /** - * Compare this node to the given node recursively and return the first pair of nodes that differs - * doing a preorder depth-first traversal. Package private for testing. Returns null if the nodes - * are equivalent. Should be called with {@code this} as the "expected" node and {@code actual} as - * the "actual" node. - */ - @Nullable - final NodeMismatch checkTreeEqualsImpl(Node actual) { - return checkTreeEqualsImpl(actual, false); - } - - /** - * Compare this node to the given node recursively and return the first pair of nodes that differs - * doing a preorder depth-first traversal. Should be called with {@code this} as the "expected" - * node and {@code actual} as the "actual" node. - * - * @param jsDoc Whether to check for differences in JSDoc. - */ - @Nullable - private NodeMismatch checkTreeEqualsImpl(Node actual, boolean jsDoc) { - if (!isEquivalentTo(actual, false, false, jsDoc)) { - return new NodeMismatch(this, actual); - } - - for (Node expectedChild = first, actualChild = actual.first; - expectedChild != null; - expectedChild = expectedChild.next, actualChild = actualChild.next) { - NodeMismatch res = expectedChild.checkTreeEqualsImpl(actualChild, jsDoc); - if (res != null) { - return res; - } - } - return null; - } - /** Checks equivalence without going into child nodes */ public final boolean isEquivalentToShallow(Node node) { return isEquivalentTo(node, false, false, false, false); @@ -3081,31 +2988,6 @@ public void setQuotedString() { throw new IllegalStateException(this + " is not a StringNode"); } - static final class NodeMismatch { - final Node nodeExpected; - final Node nodeActual; - - NodeMismatch(Node nodeExpected, Node nodeActual) { - this.nodeExpected = nodeExpected; - this.nodeActual = nodeActual; - } - - @Override - public boolean equals(@Nullable Object object) { - if (object instanceof NodeMismatch) { - NodeMismatch that = (NodeMismatch) object; - return that.nodeExpected.equals(this.nodeExpected) - && that.nodeActual.equals(this.nodeActual); - } - return false; - } - - @Override - public int hashCode() { - return Objects.hashCode(nodeExpected, nodeActual); - } - } - /*** AST type check methods ***/ diff --git a/src/com/google/javascript/rhino/testing/Asserts.java b/src/com/google/javascript/rhino/testing/Asserts.java index 6a4259dbb9b..eaff13d5983 100644 --- a/src/com/google/javascript/rhino/testing/Asserts.java +++ b/src/com/google/javascript/rhino/testing/Asserts.java @@ -118,14 +118,18 @@ public static void assertEquivalenceOperations(JSType a, JSType b) { * similar API in JUnit 4.13, but the compiler is currently pinned on 4.12, which doesn't include * it. */ - public static void assertThrows( - Class exceptionClass, ThrowingRunnable runnable) { + @SuppressWarnings("unchecked") + public static T assertThrows( + Class exceptionClass, ThrowingRunnable runnable) { try { runnable.run(); - assertWithMessage("Did not get expected exception: %s", exceptionClass).fail(); } catch (Throwable expectedException) { assertThat(expectedException).isInstanceOf(exceptionClass); + return (T) expectedException; // Unchecked cast. J2CL doesn't support `Class::cast`. } + + assertWithMessage("Did not get expected exception: %s", exceptionClass).fail(); + throw new AssertionError("Impossible"); } /** Functional interface for use with {@link #assertThrows}. */ diff --git a/src/com/google/javascript/rhino/testing/NodeSubject.java b/src/com/google/javascript/rhino/testing/NodeSubject.java index 94b0c4d7f7f..95a40e55e9c 100644 --- a/src/com/google/javascript/rhino/testing/NodeSubject.java +++ b/src/com/google/javascript/rhino/testing/NodeSubject.java @@ -39,14 +39,24 @@ package com.google.javascript.rhino.testing; +import static com.google.common.collect.Streams.stream; +import static com.google.common.truth.Fact.fact; +import static com.google.common.truth.Fact.simpleFact; import static com.google.common.truth.Truth.assertAbout; +import com.google.common.collect.Streams; +import com.google.common.truth.Fact; import com.google.common.truth.FailureMetadata; import com.google.common.truth.StringSubject; import com.google.common.truth.Subject; import com.google.errorprone.annotations.CheckReturnValue; +import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Token; +import java.util.ArrayList; +import java.util.Optional; +import java.util.function.Function; +import javax.annotation.Nullable; /** * A Truth Subject for the Node class. Usage: @@ -59,6 +69,9 @@ * */ public final class NodeSubject extends Subject { + + private Function serializer; + @CheckReturnValue public static NodeSubject assertNode(Node node) { return assertAbout(nodes()).that(node); @@ -72,12 +85,79 @@ private NodeSubject(FailureMetadata failureMetadata, Node node) { super(failureMetadata, node); } - @Override // TODO(nickreid): This isn't really equality based. Use a different name. - public void isEqualTo(Object o) { - check().that(o).isInstanceOf(Node.class); - Node node = (Node) o; + /** + * Specify a function to use when rendering {@code Node}s into assertion failure messages. + * + *

A common choice of serializer is {@link Compiler::toSource}, to as render JavaScript code. + */ + public NodeSubject usingSerializer(Function serializer) { + this.serializer = serializer; + return this; + } + + @Override + public void isEqualTo(Object expected) { + throw new UnsupportedOperationException("Use an overload with a declared type."); + } + + /** + * Compare the node-trees of actual and expected. + * + *

The per-node comparison ignores: + * + *

    + *
  • Types + *
  • JSDoc + *
  • Side-effects + *
+ */ + // TODO(nickreid): This isn't really equality based. Use a different name. + public void isEqualTo(Node expected) { + isEqualToInternal(expected, /* checkJsdoc= */ false); + } + + /** + * Compare the node-trees of actual and expected. + * + *

The per-node comparison ignores: + * + *

    + *
  • Types + *
  • Side-effects + *
+ */ + // TODO(nickreid): This isn't really equality based. Use a different name. + public void isEqualIncludingJsDocTo(Node expected) { + isEqualToInternal(expected, /* checkJsdoc= */ true); + } + + // TODO(nickreid): This isn't really equality based. Use a different name. + public void isEqualToInternal(Node expected, boolean checkJsdoc) { + isNotNull(); + assertNode(expected).isNotNull(); + + findFirstMismatch(actual(), expected, checkJsdoc) + .ifPresent( + (mismatch) -> { + ArrayList facts = new ArrayList<>(); + facts.add(fact("Actual", serializeNode(actual()))); + facts.add(fact("Expected", serializeNode(expected))); + + Node misActual = mismatch.actual; + facts.add(fact("Actual mismatch", serializeNode(misActual))); + if (checkJsdoc) { + facts.add(fact("Actual JSDoc", jsdocToStringNullsafe(misActual.getJSDocInfo()))); + } - check("checkTreeEquals(%s)", node).that(actual().checkTreeEquals(node)).isNull(); + Node misExpected = mismatch.expected; + facts.add(fact("Expected mismatch", serializeNode(misExpected))); + if (checkJsdoc) { + facts.add( + fact("Expected JSDoc", jsdocToStringNullsafe(misExpected.getJSDocInfo()))); + } + + failWithoutActual(simpleFact("Node tree inequality"), facts.toArray(new Fact[0])); + }); } public NodeSubject isEquivalentTo(Node other) { @@ -193,4 +273,55 @@ public NodeSubject hasChildren(boolean hasChildren) { public StringSubject hasStringThat() { return check("getString()").that(actual().getString()); } + + @Override + protected String actualCustomStringRepresentation() { + return serializeNode(actual()); + } + + /** + * Compare the given node-trees recursively and return the first pair of nodes that differs doing + * a pre-order traversal. + * + * @param jsDoc Whether to check for differences in JSDoc. + */ + private static Optional findFirstMismatch( + Node actual, Node expected, boolean jsDoc) { + if (!actual.isEquivalentTo( + expected, /* compareType= */ false, /* recurse= */ false, jsDoc, /* sideEffect= */ false)) { + return Optional.of(new NodeMismatch(actual, expected)); + } + + // `isEquivalentTo` confirms that the number of children is the same. + return Streams.zip( + stream(actual.children()), + stream(expected.children()), + (actualChild, expectedChild) -> findFirstMismatch(actualChild, expectedChild, jsDoc)) + .filter(Optional::isPresent) + .findFirst() + .orElse(Optional.empty()); + } + + /** A pair of nodes that were expected to match in some way but didn't. */ + private static final class NodeMismatch { + final Node actual; + final Node expected; + + NodeMismatch(Node actual, Node expected) { + this.actual = actual; + this.expected = expected; + } + } + + private String serializeNode(Node node) { + if (serializer != null) { + return serializer.apply(node); + } else { + return node.toStringTree(); + } + } + + private static String jsdocToStringNullsafe(@Nullable JSDocInfo jsdoc) { + return jsdoc == null ? "(null)" : jsdoc.toStringVerbose(); + } } diff --git a/test/com/google/javascript/jscomp/CodePrinterTest.java b/test/com/google/javascript/jscomp/CodePrinterTest.java index b061d9e139d..c4411c1308e 100644 --- a/test/com/google/javascript/jscomp/CodePrinterTest.java +++ b/test/com/google/javascript/jscomp/CodePrinterTest.java @@ -17,8 +17,8 @@ package com.google.javascript.jscomp; import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; import static com.google.javascript.jscomp.CompilerTestCase.lines; +import static com.google.javascript.rhino.testing.NodeSubject.assertNode; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; @@ -1999,18 +1999,9 @@ public void testParsePrintParse() { private void testReparse(String code) { Compiler compiler = new Compiler(); - Node parse1 = parse(code); - Node parse2 = parse(new CodePrinter.Builder(parse1).build()); - String explanation = parse1.checkTreeEquals(parse2); - assertWithMessage( - "\nExpected: " - + compiler.toSource(parse1) - + "\nResult: " - + compiler.toSource(parse2) - + "\n" - + explanation) - .that(explanation) - .isNull(); + Node parseOnce = parse(code); + Node parseTwice = parse(new CodePrinter.Builder(parseOnce).build()); + assertNode(parseTwice).usingSerializer(compiler::toSource).isEqualTo(parseOnce); } @Test diff --git a/test/com/google/javascript/jscomp/CommandLineRunnerTest.java b/test/com/google/javascript/jscomp/CommandLineRunnerTest.java index 95516d8c60d..91a4c0cda53 100644 --- a/test/com/google/javascript/jscomp/CommandLineRunnerTest.java +++ b/test/com/google/javascript/jscomp/CommandLineRunnerTest.java @@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.javascript.jscomp.testing.JSErrorSubject.assertError; +import static com.google.javascript.rhino.testing.NodeSubject.assertNode; import static java.nio.charset.StandardCharsets.US_ASCII; import static java.nio.charset.StandardCharsets.UTF_8; @@ -2282,14 +2283,17 @@ public void testJsonStreamSourceMap() { String output = new String(outReader.toByteArray(), UTF_8); assertThat(output) .isEqualTo( - "[{" - + "\"src\":\"function log(a){console.log(a)}log(\\\"one.js\\\");\\n\"," - + "\"path\":\"bar.js\"," - + "\"source_map\":\"{\\n\\\"version\\\":3,\\n\\\"file\\\":\\\"bar.js\\\",\\n" - + "\\\"lineCount\\\":1,\\n\\\"mappings\\\":\\\"AAAAA,QAASA,IAAG,CAACC,CAAD,CAAI,CACdC," + "[{\"src\":\"function log(a){console.log(a)}log(\\\"one.js\\\");\\n" + + "\",\"path\":\"bar.js\",\"source_map\":\"{\\n" + + "\\\"version\\\":3,\\n" + + "\\\"file\\\":\\\"bar.js\\\",\\n" + + "\\\"lineCount\\\":1,\\n" + + "\\\"mappings\\\":\\\"AAAAA,QAASA,IAAG,CAACC,CAAD,CAAI,CACdC," + "OAAAA,IAAAA,CAAYD,CAAZC,CADc,CAGhBF,GAAAA,CAAI,QAAJA;\\\",\\n" + "\\\"sources\\\":[\\\"one.js\\\"],\\n" - + "\\\"names\\\":[\\\"log\\\",\\\"a\\\",\\\"console\\\"]\\n}\\n\"}]"); + + "\\\"names\\\":[\\\"log\\\",\\\"a\\\",\\\"console\\\"]\\n" + + "}\\n" + + "\"}]"); } @Test @@ -2405,12 +2409,18 @@ public void testWebpackModuleIds() throws IOException { String output = new String(outReader.toByteArray(), UTF_8); assertThat(output) .isEqualTo( - "[{\"src\":\"var module$bar={default:{}};" - + "console.log(\\\"bar\\\");var module$foo={default:{}};\\n\",\"path\":\"out.js\"," - + "\"source_map\":\"{\\n\\\"version\\\":3,\\n\\\"file\\\":\\\"out.js\\\",\\n\\" - + "\"lineCount\\\":1,\\n\\\"mappings\\\":\\\"AAAA,IAAA,WAAA,CAAA,QAAA,EAAA,CAAAA,QAAAC," - + "IAAA,CAAY,KAAZ,C,CCAA,IAAA,WAAA,CAAA,QAAA,EAAA;\\\",\\n\\\"sources\\\":[\\\"bar.js\\\"," - + "\\\"foo.js\\\"],\\n\\\"names\\\":[\\\"console\\\",\\\"log\\\"]\\n}\\n\"}]"); + "[{\"src\":\"var module$bar={default:{}};console.log(\\\"bar\\\");var" + + " module$foo={default:{}};\\n" + + "\",\"path\":\"out.js\",\"source_map\":\"{\\n" + + "\\\"version\\\":3,\\n" + + "\\\"file\\\":\\\"out.js\\\",\\n" + + "\\\"lineCount\\\":1,\\n" + + "\\\"mappings\\\":\\\"AAAA,IAAA,WAAA,CAAA,QAAA,EAAA,CAAAA,QAAAC," + + "IAAA,CAAY,KAAZ,C,CCAA,IAAA,WAAA,CAAA,QAAA,EAAA;\\\",\\n" + + "\\\"sources\\\":[\\\"bar.js\\\",\\\"foo.js\\\"],\\n" + + "\\\"names\\\":[\\\"console\\\",\\\"log\\\"]\\n" + + "}\\n" + + "\"}]"); } @Test @@ -2491,16 +2501,35 @@ public void testOutputSourceMapContainsSourcesContent() { String output = new String(outReader.toByteArray(), UTF_8); assertThat(output) .isEqualTo( - "[{\"src\":\"\\n\",\"path\":\"./m1.js\",\"source_map\":\"{\\n\\\"version\\\":3," - + "\\n\\\"file\\\":\\\"./m1.js\\\",\\n\\\"lineCount\\\":1,\\n\\\"mappings\\\":\\\";\\\"" - + ",\\n\\\"sources\\\":[],\\n\\\"names\\\":[]\\n}\\n\"},{\"src\":\"alert(\\\"foo\\\");" - + "\\n\",\"path\":\"./m2.js\",\"source_map\":\"{\\n\\\"version\\\":3,\\n\\\"file\\\":" - + "\\\"./m2.js\\\",\\n\\\"lineCount\\\":1,\\n\\\"mappings\\\":\\\"AAAAA,KAAA,CAAM,KAAN;" - + "\\\",\\n\\\"sources\\\":[\\\"foo.js\\\"],\\n\\\"sourcesContent\\\":[\\\"" - + "alert('foo');\\\"],\\n\\\"names\\\":[\\\"alert\\\"]\\n}\\n\"}," - + "{\"src\":\"\\n\",\"path\":\"./$weak$.js\",\"source_map\":\"{\\n\\\"version\\\":3,\\n" - + "\\\"file\\\":\\\"./$weak$.js\\\",\\n\\\"lineCount\\\":1,\\n\\\"mappings\\\":\\\";" - + "\\\",\\n\\\"sources\\\":[],\\n\\\"names\\\":[]\\n}\\n\"}]"); + "[{\"src\":\"\\n" + + "\",\"path\":\"./m1.js\",\"source_map\":\"{\\n" + + "\\\"version\\\":3,\\n" + + "\\\"file\\\":\\\"./m1.js\\\",\\n" + + "\\\"lineCount\\\":1,\\n" + + "\\\"mappings\\\":\\\";\\\",\\n" + + "\\\"sources\\\":[],\\n" + + "\\\"names\\\":[]\\n" + + "}\\n" + + "\"},{\"src\":\"alert(\\\"foo\\\");\\n" + + "\",\"path\":\"./m2.js\",\"source_map\":\"{\\n" + + "\\\"version\\\":3,\\n" + + "\\\"file\\\":\\\"./m2.js\\\",\\n" + + "\\\"lineCount\\\":1,\\n" + + "\\\"mappings\\\":\\\"AAAAA,KAAA,CAAM,KAAN;\\\",\\n" + + "\\\"sources\\\":[\\\"foo.js\\\"],\\n" + + "\\\"sourcesContent\\\":[\\\"alert('foo');\\\"],\\n" + + "\\\"names\\\":[\\\"alert\\\"]\\n" + + "}\\n" + + "\"},{\"src\":\"\\n" + + "\",\"path\":\"./$weak$.js\",\"source_map\":\"{\\n" + + "\\\"version\\\":3,\\n" + + "\\\"file\\\":\\\"./$weak$.js\\\",\\n" + + "\\\"lineCount\\\":1,\\n" + + "\\\"mappings\\\":\\\";\\\",\\n" + + "\\\"sources\\\":[],\\n" + + "\\\"names\\\":[]\\n" + + "}\\n" + + "\"}]"); } @Test @@ -2566,16 +2595,7 @@ private void test(String[] original, String[] compiled, DiagnosticType warning) assertThat(compiler.toSource()).isEqualTo(Joiner.on("").join(compiled)); } else { Node expectedRoot = parse(compiled); - String explanation = expectedRoot.checkTreeEquals(root); - assertWithMessage( - "\nExpected: " - + compiler.toSource(expectedRoot) - + "\nResult: " - + compiler.toSource(root) - + "\n" - + explanation) - .that(explanation) - .isNull(); + assertNode(root).usingSerializer(compiler::toSource).isEqualTo(expectedRoot); } } diff --git a/test/com/google/javascript/jscomp/CompilerTestCase.java b/test/com/google/javascript/jscomp/CompilerTestCase.java index b2a215f81b1..13381e2c85e 100644 --- a/test/com/google/javascript/jscomp/CompilerTestCase.java +++ b/test/com/google/javascript/jscomp/CompilerTestCase.java @@ -22,6 +22,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.javascript.jscomp.testing.JSErrorSubject.assertError; +import static com.google.javascript.rhino.testing.NodeSubject.assertNode; import com.google.common.annotations.GwtIncompatible; import com.google.common.base.Joiner; @@ -1746,16 +1747,7 @@ private void testInternal( // Generally, externs should not be changed by the compiler passes. if (externsChange && !allowExternsChanges) { - String explanation = externsRootClone.checkTreeEquals(externsRoot); - assertWithMessage( - "Unexpected changes to externs" - + "\nExpected: " - + compiler.toSource(externsRootClone) - + "\nResult: " - + compiler.toSource(externsRoot) - + "\n" - + explanation) - .fail(); + assertNode(externsRootClone).usingSerializer(compiler::toSource).isEqualTo(externsRoot); } if (!codeChange && !externsChange) { @@ -1788,27 +1780,12 @@ private void testInternal( } } if (compareAsTree) { - String explanation; if (compareJsDoc) { - explanation = expectedRoot.checkTreeEqualsIncludingJsDoc(mainRoot); + assertNode(mainRoot) + .usingSerializer(compiler::toSource) + .isEqualIncludingJsDocTo(expectedRoot); } else { - explanation = expectedRoot.checkTreeEquals(mainRoot); - } - if (explanation != null) { - String expectedAsSource = compiler.toSource(expectedRoot); - String mainAsSource = compiler.toSource(mainRoot); - if (expectedAsSource.equals(mainAsSource)) { - assertWithMessage("In: " + expectedAsSource + "\n" + explanation).fail(); - } else { - assertWithMessage( - "\nExpected: " - + expectedAsSource - + "\nResult: " - + mainAsSource - + "\n" - + explanation) - .fail(); - } + assertNode(mainRoot).usingSerializer(compiler::toSource).isEqualTo(expectedRoot); } } else { String[] expectedSources = new String[expected.size()]; @@ -1828,17 +1805,10 @@ private void testInternal( Node normalizeCheckExternsRootClone = normalizeCheckRootClone.getFirstChild(); Node normalizeCheckMainRootClone = normalizeCheckRootClone.getLastChild(); new PrepareAst(compiler).process(normalizeCheckExternsRootClone, normalizeCheckMainRootClone); - String explanation = normalizeCheckMainRootClone.checkTreeEquals(mainRoot); - assertWithMessage( - "Node structure normalization invalidated." - + "\nExpected: " - + compiler.toSource(normalizeCheckMainRootClone) - + "\nResult: " - + compiler.toSource(mainRoot) - + "\n" - + explanation) - .that(explanation) - .isNull(); + + assertNode(normalizeCheckMainRootClone) + .usingSerializer(compiler::toSource) + .isEqualTo(mainRoot); // TODO(johnlenz): enable this for most test cases. // Currently, this invalidates test for while-loops, for-loop @@ -1848,17 +1818,10 @@ private void testInternal( if (normalizeEnabled) { new Normalize(compiler, true) .process(normalizeCheckExternsRootClone, normalizeCheckMainRootClone); - explanation = normalizeCheckMainRootClone.checkTreeEquals(mainRoot); - assertWithMessage( - "Normalization invalidated." - + "\nExpected: " - + compiler.toSource(normalizeCheckMainRootClone) - + "\nResult: " - + compiler.toSource(mainRoot) - + "\n" - + explanation) - .that(explanation) - .isNull(); + + assertNode(normalizeCheckMainRootClone) + .usingSerializer(compiler::toSource) + .isEqualTo(mainRoot); } } else { assertThat(compiler.getErrors()) @@ -2020,17 +1983,8 @@ protected void testExternChanges(String extern, String input, String expectedExt checkState( externs.hasOneChild(), "Compare as tree only works when output has a single script."); externs = externs.getFirstChild(); - String explanation = expected.checkTreeEqualsIncludingJsDoc(externs); - assertWithMessage( - "" - + "\nExpected: " - + compiler.toSource(expected) - + "\nResult: " - + compiler.toSource(externs) - + "\n" - + explanation) - .that(explanation) - .isNull(); + + assertNode(externs).usingSerializer(compiler::toSource).isEqualIncludingJsDocTo(externs); } else { String externsCode = compiler.toSource(externs); String expectedCode = compiler.toSource(expected); diff --git a/test/com/google/javascript/jscomp/FunctionInjectorTest.java b/test/com/google/javascript/jscomp/FunctionInjectorTest.java index 56dc221a220..9282b862351 100644 --- a/test/com/google/javascript/jscomp/FunctionInjectorTest.java +++ b/test/com/google/javascript/jscomp/FunctionInjectorTest.java @@ -20,6 +20,7 @@ import static com.google.common.truth.Truth.assertWithMessage; import static com.google.javascript.jscomp.CompilerTestCase.lines; import static com.google.javascript.jscomp.FunctionInjector.isDirectCallNodeReplacementPossible; +import static com.google.javascript.rhino.testing.NodeSubject.assertNode; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -1908,17 +1909,11 @@ public boolean call(NodeTraversal t, Node n, Node parent) { Node result = injector.inline(ref, fnName, fnNode); validateSourceInfo(compiler, result); - String explanation = expectedRoot.checkTreeEquals(tree.getFirstChild()); - assertWithMessage( - "" - + "\nExpected: " - + toSource(expectedRoot) - + "\nResult: " - + toSource(tree.getFirstChild()) - + "\n" - + explanation) - .that(explanation) - .isNull(); + + assertNode(tree.getFirstChild()) + .usingSerializer(FunctionInjectorTest::toSource) + .isEqualTo(expectedRoot); + return true; } }; diff --git a/test/com/google/javascript/jscomp/FunctionToBlockMutatorTest.java b/test/com/google/javascript/jscomp/FunctionToBlockMutatorTest.java index d79fe09073b..062caeb8bcc 100644 --- a/test/com/google/javascript/jscomp/FunctionToBlockMutatorTest.java +++ b/test/com/google/javascript/jscomp/FunctionToBlockMutatorTest.java @@ -17,8 +17,8 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; import static com.google.javascript.jscomp.CompilerTestCase.lines; +import static com.google.javascript.rhino.testing.NodeSubject.assertNode; import com.google.javascript.jscomp.AbstractCompiler.LifeCycleStage; import com.google.javascript.jscomp.NodeTraversal.Callback; @@ -302,16 +302,7 @@ public void helperMutate(String code, String expectedResult, String fnName, Stri Node result = mutator.mutate(fnName, fnNode, n, resultName, needsDefaultResult, isCallInLoop); validateSourceInfo(compiler, result); - String explanation = expected.checkTreeEquals(result); - assertWithMessage( - "\nExpected: " - + compiler.toSource(expected) - + "\nResult: " - + compiler.toSource(result) - + "\n" - + explanation) - .that(explanation) - .isNull(); + assertNode(result).usingSerializer(compiler::toSource).isEqualTo(expected); return true; }; diff --git a/test/com/google/javascript/jscomp/IntegrationTestCase.java b/test/com/google/javascript/jscomp/IntegrationTestCase.java index 12f83ebc7b8..2a47a9ae400 100644 --- a/test/com/google/javascript/jscomp/IntegrationTestCase.java +++ b/test/com/google/javascript/jscomp/IntegrationTestCase.java @@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.javascript.jscomp.testing.JSErrorSubject.assertError; +import static com.google.javascript.rhino.testing.NodeSubject.assertNode; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; @@ -367,18 +368,7 @@ protected void test(CompilerOptions options, Node root = compiler.getJsRoot(); if (compiled != null) { Node expectedRoot = parseExpectedCode(compiled, options, normalizeResults); - String explanation = expectedRoot.checkTreeEquals(root); - assertWithMessage( - "\n" - + "Expected: " - + compiler.toSource(expectedRoot) - + "\n" - + "Result: " - + compiler.toSource(root) - + "\n" - + explanation) - .that(explanation) - .isNull(); + assertNode(root).usingSerializer(compiler::toSource).isEqualTo(expectedRoot); } } @@ -422,16 +412,7 @@ protected void test(CompilerOptions options, if (compiled != null) { Node root = compiler.getRoot().getLastChild(); Node expectedRoot = parseExpectedCode(compiled, options, normalizeResults); - String explanation = expectedRoot.checkTreeEquals(root); - assertWithMessage( - "\nExpected: " - + compiler.toSource(expectedRoot) - + "\nResult: " - + compiler.toSource(root) - + "\n" - + explanation) - .that(explanation) - .isNull(); + assertNode(root).usingSerializer(compiler::toSource).isEqualTo(expectedRoot); } } @@ -444,16 +425,7 @@ protected void test( if (compiled != null) { Node root = compiler.getRoot().getLastChild(); Node expectedRoot = parseExpectedCode(compiled, options, normalizeResults); - String explanation = expectedRoot.checkTreeEquals(root); - assertWithMessage( - "\nExpected: " - + compiler.toSource(expectedRoot) - + "\nResult: " - + compiler.toSource(root) - + "\n" - + explanation) - .that(explanation) - .isNull(); + assertNode(root).usingSerializer(compiler::toSource).isEqualTo(expectedRoot); } } @@ -483,16 +455,7 @@ protected void testParseError(CompilerOptions options, Node root = compiler.getRoot().getLastChild(); Node expectedRoot = parseExpectedCode( new String[] {compiled}, options, normalizeResults); - String explanation = expectedRoot.checkTreeEquals(root); - assertWithMessage( - "\nExpected: " - + compiler.toSource(expectedRoot) - + "\nResult: " - + compiler.toSource(root) - + "\n" - + explanation) - .that(explanation) - .isNull(); + assertNode(root).usingSerializer(compiler::toSource).isEqualTo(expectedRoot); } } diff --git a/test/com/google/javascript/jscomp/NodeUtilTest.java b/test/com/google/javascript/jscomp/NodeUtilTest.java index 9e5e245b5fe..2667d640387 100644 --- a/test/com/google/javascript/jscomp/NodeUtilTest.java +++ b/test/com/google/javascript/jscomp/NodeUtilTest.java @@ -3507,7 +3507,7 @@ public void testNewQName1() { Node expected = IR.getprop( IR.name("ns"), IR.string("prop")); - assertNodeTreesEqual(expected, actual); + assertNode(actual).isEqualTo(expected); } @Test @@ -3520,7 +3520,7 @@ public void testNewQualifiedNameNode2() { Node expected = IR.getprop( IR.thisNode(), IR.string("prop")); - assertNodeTreesEqual(expected, actual); + assertNode(actual).isEqualTo(expected); } @Test @@ -4013,12 +4013,6 @@ private boolean functionIsRValueOfAssign(String js) { return funcNode == NodeUtil.getRValueOfLValue(nameNode); } - private void assertNodeTreesEqual( - Node expected, Node actual) { - String error = expected.checkTreeEquals(actual); - assertThat(error).isNull(); - } - private static void testFunctionName(String js, String expected) { assertThat(NodeUtil.getNearestFunctionName(parseFirst(FUNCTION, js))).isEqualTo(expected); } diff --git a/test/com/google/javascript/jscomp/RecoverableJsAstTest.java b/test/com/google/javascript/jscomp/RecoverableJsAstTest.java index 875380e8e7c..fa5cc73c8c4 100644 --- a/test/com/google/javascript/jscomp/RecoverableJsAstTest.java +++ b/test/com/google/javascript/jscomp/RecoverableJsAstTest.java @@ -17,7 +17,7 @@ package com.google.javascript.jscomp; import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.javascript.rhino.testing.NodeSubject.assertNode; import com.google.common.collect.ImmutableList; import com.google.common.truth.Correspondence; @@ -150,18 +150,9 @@ private void checkCompile(SourceAst realAst, RecoverableJsAst ast, String expect assertThat(mainRoot.isRoot()).isTrue(); assertThat(mainRoot.hasChildren()).isFalse(); } else { - String explanation = expectedRoot.checkTreeEqualsIncludingJsDoc(mainRoot); - if (explanation != null) { - String expectedAsSource = compiler.toSource(expectedRoot); - String mainAsSource = compiler.toSource(mainRoot); - if (expectedAsSource.equals(mainAsSource)) { - assertWithMessage("In: %s\n%s", expectedAsSource, explanation).fail(); - } else { - assertWithMessage( - "Expected: %s\nResult: %s\n%s", expectedAsSource, mainAsSource, explanation) - .fail(); - } - } + assertNode(mainRoot) + .usingSerializer(compiler::toSource) + .isEqualIncludingJsDocTo(expectedRoot); } assertThat(compiler.getResult().errors) diff --git a/test/com/google/javascript/jscomp/ScopedAliasesTest.java b/test/com/google/javascript/jscomp/ScopedAliasesTest.java index 92b7d045da4..90155bb43c3 100644 --- a/test/com/google/javascript/jscomp/ScopedAliasesTest.java +++ b/test/com/google/javascript/jscomp/ScopedAliasesTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.javascript.rhino.testing.NodeSubject.assertNode; import com.google.common.collect.ImmutableList; import com.google.common.collect.ListMultimap; @@ -1385,7 +1386,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { .that(actualTypes.size()) .isEqualTo(expectedTypes.size()); for (int i = 0; i < expectedTypes.size(); i++) { - assertThat(expectedTypes.get(i).checkTreeEquals(actualTypes.get(i))).isNull(); + assertNode(actualTypes.get(i)).isEqualTo(expectedTypes.get(i)); } } else { actualTypes = new ArrayList<>(); diff --git a/test/com/google/javascript/jscomp/parsing/ParserTest.java b/test/com/google/javascript/jscomp/parsing/ParserTest.java index e6d3a6eeb76..6d778325035 100644 --- a/test/com/google/javascript/jscomp/parsing/ParserTest.java +++ b/test/com/google/javascript/jscomp/parsing/ParserTest.java @@ -1621,10 +1621,7 @@ public void testMalformedRegexp() { } private static void assertNodeEquality(Node expected, Node found) { - String message = expected.checkTreeEquals(found); - if (message != null) { - assertWithMessage(message).fail(); - } + assertNode(found).isEqualTo(expected); } @SuppressWarnings("unchecked") @@ -2923,9 +2920,11 @@ public void testTemplateLiteralWithLineContinuation() { mode = LanguageMode.ECMASCRIPT6; strictMode = SLOPPY; expectFeatures(Feature.TEMPLATE_LITERALS); - Node n = parseWarning("`string \\\ncontinuation`", - "String continuations are not recommended. See" - + " https://google.github.io/styleguide/jsguide.html#features-strings-no-line-continuations"); + Node n = + parseWarning( + "`string \\\ncontinuation`", + "String continuations are not recommended. See" + + " https://google.github.io/styleguide/jsguide.html#features-strings-no-line-continuations"); Node templateLiteral = n.getFirstFirstChild(); Node stringNode = templateLiteral.getFirstChild(); assertNode(stringNode).hasType(Token.TEMPLATELIT_STRING); diff --git a/test/com/google/javascript/jscomp/parsing/TypeSyntaxTest.java b/test/com/google/javascript/jscomp/parsing/TypeSyntaxTest.java index 8ed2153984a..be030063e1d 100644 --- a/test/com/google/javascript/jscomp/parsing/TypeSyntaxTest.java +++ b/test/com/google/javascript/jscomp/parsing/TypeSyntaxTest.java @@ -40,6 +40,7 @@ import com.google.javascript.rhino.Node.TypeDeclarationNode; import com.google.javascript.rhino.Token; import com.google.javascript.rhino.TypeDeclarationsIR; +import com.google.javascript.rhino.testing.NodeSubject; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; @@ -860,8 +861,7 @@ private static void assertDeclaredType( } private static void assertTreeEquals(String message, Node expected, Node actual) { - String treeDiff = expected.checkTreeEquals(actual); - assertWithMessage(message + ": " + treeDiff).that(treeDiff).isNull(); + assertWithMessage(message).about(NodeSubject.nodes()).that(actual).isEqualTo(expected); } private Node parse(String source, String expected, LanguageMode languageIn) { diff --git a/test/com/google/javascript/rhino/NodeTest.java b/test/com/google/javascript/rhino/NodeTest.java index 8cfa2976ac7..9f94e51c820 100644 --- a/test/com/google/javascript/rhino/NodeTest.java +++ b/test/com/google/javascript/rhino/NodeTest.java @@ -41,7 +41,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.javascript.rhino.testing.NodeSubject.assertNode; -import com.google.javascript.rhino.Node.NodeMismatch; import com.google.javascript.rhino.jstype.JSTypeNative; import com.google.javascript.rhino.jstype.JSTypeRegistry; import com.google.javascript.rhino.testing.TestErrorReporter; @@ -73,49 +72,6 @@ public void testMergeOverflowGraciously() { assertThat(Node.extractCharno(linecharno)).isEqualTo(4095); } - @Test - public void testCheckTreeEqualsImplSame() { - Node node1 = new Node(Token.LET, new Node(Token.VAR)); - Node node2 = new Node(Token.LET, new Node(Token.VAR)); - assertThat(node1.checkTreeEqualsImpl(node2)).isNull(); - } - - @Test - public void testCheckTreeEqualsImplDifferentType() { - Node node1 = new Node(Token.LET, new Node(Token.VAR)); - Node node2 = new Node(Token.VAR, new Node(Token.VAR)); - assertThat(node1.checkTreeEqualsImpl(node2)).isEqualTo(new NodeMismatch(node1, node2)); - } - - @Test - public void testCheckTreeEqualsImplDifferentChildCount() { - Node node1 = new Node(Token.LET, new Node(Token.VAR)); - Node node2 = new Node(Token.LET); - assertThat(node1.checkTreeEqualsImpl(node2)).isEqualTo(new NodeMismatch(node1, node2)); - } - - @Test - public void testCheckTreeEqualsImplDifferentChild() { - Node child1 = new Node(Token.LET); - Node child2 = new Node(Token.VAR); - Node node1 = new Node(Token.LET, child1); - Node node2 = new Node(Token.LET, child2); - assertThat(node1.checkTreeEqualsImpl(node2)).isEqualTo(new NodeMismatch(child1, child2)); - } - - @Test - public void testCheckTreeEqualsSame() { - Node node1 = new Node(Token.LET); - assertThat(node1.checkTreeEquals(node1)).isNull(); - } - - @Test - public void testCheckTreeEqualsStringDifferent() { - Node node1 = new Node(Token.ADD); - Node node2 = new Node(Token.SUB); - assertThat(node1.checkTreeEquals(node2)).isNotNull(); - } - @Test public void isEquivalentToForFunctionsConsidersKindOfFunction() { Node normalFunction = IR.function(IR.name(""), IR.paramList(), IR.block()); @@ -160,34 +116,26 @@ public void isEquivalentToForFunctionsConsidersKindOfFunction() { } @Test - public void testCheckTreeEqualsBooleanSame() { + public void testIsEquivalentTo_withBoolean_isSame() { Node node1 = new Node(Token.LET); assertThat(node1.isEquivalentTo(node1)).isTrue(); } @Test - public void testCheckTreeEqualsBooleanDifferent() { + public void testIsEquivalentTo_withBoolean_isDifferent() { Node node1 = new Node(Token.LET); Node node2 = new Node(Token.VAR); assertThat(node1.isEquivalentTo(node2)).isFalse(); } @Test - public void testCheckTreeEqualsSlashVDifferent() { + public void testIsEquivalentTo_withSlashV_isDifferent() { Node node1 = Node.newString("\u000B"); node1.putBooleanProp(Node.SLASH_V, true); Node node2 = Node.newString("\u000B"); assertThat(node1.isEquivalentTo(node2)).isFalse(); } - @Test - public void testCheckTreeEqualsImplDifferentIncProp() { - Node node1 = new Node(Token.INC); - node1.putBooleanProp(Node.INCRDECR_PROP, true); - Node node2 = new Node(Token.INC); - assertThat(node1.checkTreeEqualsImpl(node2)).isNotNull(); - } - @Test public void testCheckTreeTypeAwareEqualsSame() { TestErrorReporter testErrorReporter = new TestErrorReporter(null, null);