From a5d411a9b471e770d9d35b38dbe5ff18349c1c80 Mon Sep 17 00:00:00 2001 From: James Wright Date: Mon, 27 Jan 2025 09:50:53 -0800 Subject: [PATCH] Use shadow nodes instead of printing and re-parsing closure-unaware JS in the ManageClosureUnawareCode passes. PiperOrigin-RevId: 720206818 --- .../javascript/jscomp/DiagnosticGroups.java | 1 + .../jscomp/ManageClosureUnawareCode.java | 431 ++++++++++++------ .../serialization/ScriptNodeDeserializer.java | 21 +- .../jscomp/ManageClosureUnawareCodeTest.java | 180 ++------ 4 files changed, 327 insertions(+), 306 deletions(-) diff --git a/src/com/google/javascript/jscomp/DiagnosticGroups.java b/src/com/google/javascript/jscomp/DiagnosticGroups.java index 850a8a64dc4..330b8b92a7a 100644 --- a/src/com/google/javascript/jscomp/DiagnosticGroups.java +++ b/src/com/google/javascript/jscomp/DiagnosticGroups.java @@ -671,6 +671,7 @@ public static DiagnosticGroup forName(String name) { public static final DiagnosticGroup INVALID_CLOSURE_UNAWARE_ANNOTATED_CODE = DiagnosticGroups.registerUnsuppressibleGroup( + ManageClosureUnawareCode.UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_PRESERVE, ManageClosureUnawareCode.UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_CODE); public static final DiagnosticGroup CANNOT_TRANSPILE_FEATURE = diff --git a/src/com/google/javascript/jscomp/ManageClosureUnawareCode.java b/src/com/google/javascript/jscomp/ManageClosureUnawareCode.java index f0922553a8c..8b2edb49faa 100644 --- a/src/com/google/javascript/jscomp/ManageClosureUnawareCode.java +++ b/src/com/google/javascript/jscomp/ManageClosureUnawareCode.java @@ -15,12 +15,9 @@ */ package com.google.javascript.jscomp; -import static com.google.common.base.Preconditions.checkArgument; - -import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; -import com.google.javascript.rhino.IR; -import com.google.javascript.rhino.JSDocInfo; +import com.google.javascript.jscomp.colors.StandardColors; import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.jstype.JSTypeNative; import org.jspecify.annotations.Nullable; /** @@ -52,6 +49,11 @@ */ final class ManageClosureUnawareCode implements CompilerPass { + public static final DiagnosticType UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_PRESERVE = + DiagnosticType.error( + "JSC_UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_PRESERVE", + "This reference to $jscomp_wrap_closure_unaware_code is not expected."); + public static final DiagnosticType UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_CODE = DiagnosticType.error( "JSC_UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_CODE", @@ -59,18 +61,18 @@ final class ManageClosureUnawareCode implements CompilerPass { + " @closureUnaware."); private final AbstractCompiler compiler; - private static final String JSCOMP_CLOSURE_UNAWARE_CODE_SHADOW_HOST_NAME = + private final AstFactory astFactory; + private static final String JSCOMP_CLOSURE_UNAWARE_CODE_PRESERVE_FN = "$jscomp_wrap_closure_unaware_code"; private final boolean isUnwrapping; - /** - * Whether the synthetic extern for JSCOMP_CLOSURE_UNAWARE_CODE_SHADOW_HOST_NAME has been injected - */ - private boolean shadowHostNameExternInjected = false; + /** Whether the synthetic extern for JSCOMP_CLOSURE_UNAWARE_CODE_PRESERVE_FN has been injected */ + private boolean preserveFunctionInjected = false; private ManageClosureUnawareCode(AbstractCompiler compiler, final boolean unwrapPhase) { this.compiler = compiler; + this.astFactory = compiler.createAstFactory(); this.isUnwrapping = unwrapPhase; } @@ -89,54 +91,7 @@ public void process(Node externs, Node root) { return; } // wrapping mode - new ValidateAndWrapClosureUnawareCode(compiler).process(externs, root); - } - - private final class ValidateAndWrapClosureUnawareCode implements CompilerPass { - - private final AbstractCompiler compiler; - - private ValidateAndWrapClosureUnawareCode(AbstractCompiler compiler) { - this.compiler = compiler; - } - - @Override - public void process(Node externs, Node root) { - - for (Node script = root.getFirstChild(); script != null; script = script.getNext()) { - if (script.isClosureUnawareCode()) { - - ValidateAndWrapClosureUnawareScript validateAndWrapClosureUnawareScript = - new ValidateAndWrapClosureUnawareScript(compiler); - NodeTraversal.traverse(compiler, script, validateAndWrapClosureUnawareScript); - // If the file was marked as @closureUnaware, but we didn't actually wrap anything, that - // means that the file was not in the expected shape. - boolean hasWrappedSomething = - validateAndWrapClosureUnawareScript.hasSeenClosureUnawareAnnotation(); - if (!hasWrappedSomething) { - reportUnexpectedClosureUnawareCode(compiler, script); - } - } else { - // This would occur if the annotation was present in a non-fileoverview comment and the - // fileoverview did not contain this annotation. - NodeUtil.visitPreOrder( - script, - new NodeUtil.Visitor() { - @Override - public void visit(Node n) { - JSDocInfo jsDocInfo = n.getJSDocInfo(); - if (jsDocInfo != null && jsDocInfo.isClosureUnawareCode()) { - reportUnexpectedClosureUnawareCode(compiler, n); - } - } - }); - } - } - } - } - - private static final void reportUnexpectedClosureUnawareCode(AbstractCompiler compiler, Node n) { - compiler.report(JSError.make(n, UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_CODE)); + NodeTraversal.traverse(compiler, root, new ValidateAndWrapGlobalIifeCode()); } /** @@ -163,103 +118,250 @@ private static final void reportUnexpectedClosureUnawareCode(AbstractCompiler co * *

exports assignment statements */ - private final class ValidateAndWrapClosureUnawareScript extends AbstractPostOrderCallback { - - private final AbstractCompiler compiler; - private final AstFactory astFactory; + private final class ValidateAndWrapGlobalIifeCode implements NodeTraversal.Callback { - ValidateAndWrapClosureUnawareScript(AbstractCompiler compiler) { - this.compiler = compiler; - this.astFactory = compiler.createAstFactory(); + private void reportUnexpectedCode(Node n) { + compiler.report(JSError.make(n, UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_CODE)); } - private boolean seenClosureUnawareAnnotation = false; + @Override + public final boolean shouldTraverse(NodeTraversal t, Node n, @Nullable Node parent) { + return parent == null || !parent.isScript(); // Don't traverse children of scripts + } @Override public final void visit(NodeTraversal t, Node n, @Nullable Node parent) { - checkArgument( - !n.isRoot(), - "ValidateAndWrapClosureUnawareScript should be run directly on each SCRIPT node" - + " individually, and should not be re-used across SCRIPT nodes."); - - JSDocInfo jsDocInfo = n.getJSDocInfo(); - if (jsDocInfo == null || !jsDocInfo.isClosureUnawareCode()) { + if (n == null || !n.isScript()) { return; } - - if (n.isScript()) { - // This is a file-overview comment, which should not trigger shadowing. + if (!n.isClosureUnawareCode()) { return; } - seenClosureUnawareAnnotation = true; - - if (!isValidClosureUnawareAnnotatedNode(n)) { - reportUnexpectedClosureUnawareCode(compiler, n); + // We want to look at: + // top-level CALLs + // (SCRIPT -> (MODULE_BODY -> )? EXPR_RESULT -> CALL) + // second-level CALLS inside IF + // (SCRIPT -> (MODULE_BODY -> )? EXPR_RESULT -> IF -> BLOCK -> EXPR_RESULT -> CALL) + // calls inside nested IF + // (SCRIPT -> (MODULE_BODY -> )? EXPR_RESULT -> IF -> BLOCK -> IF -> BLOCK -> EXPR_RESULT -> + // CALL) + + // We are going to manually find the relevant nodes here, and anything else we will report an + // error for. + Node script = n; + + Node exprParent = script.getFirstChild(); + if (exprParent == null) { + reportUnexpectedCode(script); return; } + if (!exprParent.isModuleBody()) { + if (exprParent.isExprResult()) { + if (NodeUtil.isExprCall(exprParent) + && NodeUtil.isBundledGoogModuleCall(exprParent.getFirstChild())) { + // EXPR_RESULT -> CALL -> FUNCTION -> BLOCK -> EXPR_RESULT(s) + // We want the BLOCK node, as we will iterate over it to get EXPR_RESULTs + exprParent = exprParent.getFirstChild().getSecondChild().getChildAtIndex(2); + } else { + // Sometimes, there is no MODULE_BODY node in the AST, just SCRIPT -> EXPR_RESULT(s). + // Assume that if we didn't find the MODULE_BODY as the first child of the SCRIPT that + // all the EXPR_RESULT are direct children of the SCRIPT. + exprParent = script; + } + } else { + reportUnexpectedCode(script); + return; + } + } - hideClosureUnawareCodeRoot(t, n); - } + // MODULE_BODY has a list of statement kinds. We want to validate that they are all "known" + for (Node child = exprParent.getFirstChild(); child != null; child = child.getNext()) { + // Allowed statement types: + if (child.isExprResult() + && child.getFirstChild().isString() + && child.getFirstChild().getString().equals("use strict")) { + // "use strict" pragmas, likely added by the whitespace-wrapping of Closure modules + continue; + } + + // -- calls to goog.module + if (NodeUtil.isGoogModuleCall(child)) { + continue; + } + + // -- direct calls to goog.require (no assignments) + if (NodeUtil.isExprCall(child) && NodeUtil.isGoogRequireCall(child.getFirstChild())) { + continue; + } + + if (child.isConst()) { + if (child.getFirstChild().isDestructuringLhs()) { + Node rhs = child.getFirstChild().getSecondChild(); + if (rhs != null && NodeUtil.isGoogRequireCall(rhs)) { + continue; + } + reportUnexpectedCode(child); + continue; + } + // get RHS of this const declaration and see if it is a goog.require + Node rhs = child.getSecondChild(); + if (rhs != null && NodeUtil.isGoogRequireCall(rhs)) { + continue; + } + reportUnexpectedCode(child); + continue; + } + + if (NodeUtil.isExprCall(child)) { + maybeRewriteCall(t, child.getFirstChild()); + continue; + } + + if (child.isIf()) { + visitIf(t, child, 0); + continue; + } - private final boolean hasSeenClosureUnawareAnnotation() { - return seenClosureUnawareAnnotation; + // exports = + if (child.isExprResult() && child.getFirstChild().isAssign()) { + Node lhs = child.getFirstFirstChild(); + if (lhs != null && lhs.matchesName("exports")) { + continue; + } + reportUnexpectedCode(child); + continue; + } + + if (child.isReturn() && child.getFirstChild().matchesName("exports")) { + // return exports; + // Likely added by whitespace-wrapping of Closure modules + continue; + } + + // Fallthrough: we don't know why this statement is here? + reportUnexpectedCode(child); + } } - private final boolean isValidClosureUnawareAnnotatedNode(Node n) { - // We only intend to support hiding FUNCTION nodes that are children under a CALL node - // as this generally matches the mental model of what we are rewriting it with. - if (!n.isFunction()) { - return false; + private void visitIfBlockStmt(NodeTraversal t, Node n, int depth) { + Node child = n.getFirstChild(); + if (NodeUtil.isExprCall(child)) { + maybeRewriteCall(t, child.getFirstChild()); + return; } - Node parent = n.getParent(); - if (parent == null) { - return false; + // Could be a nested if + if (child.isIf()) { + if (depth > 1) { + // This nested if statement is too deep - we only expect at most two layers of nested IF + reportUnexpectedCode(child); + return; + } else { + visitIf(t, child, depth + 1); + return; + } } - // We only support: + // We don't know what this is. + reportUnexpectedCode(child); + } - // - FUNCTION nodes that are direct CALL callees - if (parent.isCall()) { - return true; + private void visitIf(NodeTraversal t, Node n, int depth) { + Node trueBlock = n.getSecondChild(); + @Nullable Node falseBlock = null; + if (n.getChildCount() > 2) { + falseBlock = n.getChildAtIndex(2); } - // - FUNCTION nodes that are children of a GETPROP node that is a direct CALL callee (e.g. - // fn.call or fn.apply can be the CALL callee) - Node grandparent = parent.getParent(); - if (grandparent == null) { - return false; + boolean hasProblems = false; + if (trueBlock.getChildCount() > 1) { + reportUnexpectedCode(trueBlock); + hasProblems = true; + } + if (falseBlock != null && falseBlock.getChildCount() > 1) { + reportUnexpectedCode(falseBlock); + hasProblems = true; } - if (parent.isGetProp() && grandparent.isCall()) { - String calledFn = parent.getString(); - return calledFn.equals("call") || calledFn.equals("apply"); + if (hasProblems) { + return; } - return false; + // This IF has a true (and possibly a false) block, both with one child. + visitIfBlockStmt(t, trueBlock, depth); + if (falseBlock != null) { + visitIfBlockStmt(t, falseBlock, depth); + } } - private final void hideClosureUnawareCodeRoot(NodeTraversal t, Node n) { - if (!shadowHostNameExternInjected) { - NodeUtil.createSynthesizedExternsSymbol( - compiler, JSCOMP_CLOSURE_UNAWARE_CODE_SHADOW_HOST_NAME); - shadowHostNameExternInjected = true; + private void maybeRewriteCall(NodeTraversal t, Node n) { + if (!n.isCall() || n.getChildCount() != 2) { + reportUnexpectedCode(n); + return; } - Node shadowNameNode = - astFactory.createNameWithUnknownType(JSCOMP_CLOSURE_UNAWARE_CODE_SHADOW_HOST_NAME); - shadowNameNode.putBooleanProp(Node.IS_CONSTANT_NAME, true); - n.replaceWith(shadowNameNode); + Node prop = n.getFirstChild(); + // <>.call() + if (!prop.isGetProp() || !prop.getString().equals("call") || prop.getChildCount() != 1) { + reportUnexpectedCode(prop); + return; + } + Node globalThisArg = n.getSecondChild(); + if (globalThisArg == null || !globalThisArg.getString().equals("globalThis")) { + reportUnexpectedCode(globalThisArg); + return; + } - Node shadowJsRoot = IR.root(IR.script(astFactory.exprResult(n))); + Node evalFn = prop.getFirstChild(); + if (!evalFn.isFunction() || evalFn.getSecondChild().hasChildren()) { + // not a function, or a function with parameters + reportUnexpectedCode(evalFn); + return; + } + + Node closureUnawareCodeBlock = evalFn.getChildAtIndex(2); + if (!closureUnawareCodeBlock.isBlock()) { + reportUnexpectedCode(closureUnawareCodeBlock); + return; + } - shadowNameNode.setClosureUnawareShadow(shadowJsRoot); - t.reportCodeChange(shadowNameNode); + wrapClosureUnawareCode(t, n); } } - // TODO jameswr: Given that the CodePrinter supports printing out the shadow instead of the shadow - // host node, do we even need to revert the AST back to the original form at the end of - // compilation? - private static final class UnwrapConcealedClosureUnawareCode implements NodeTraversal.Callback { + private void wrapClosureUnawareCode(NodeTraversal t, Node iifeNode) { + // These code blocks should be in the form of: + // CALL + // GETPROP + // FUNCTION + // NAME + // PARAM_LIST + // BLOCK + // ... closure unaware code here ... + // NAME globalThis + + if (!preserveFunctionInjected) { + NodeUtil.createSynthesizedExternsSymbol(compiler, JSCOMP_CLOSURE_UNAWARE_CODE_PRESERVE_FN); + preserveFunctionInjected = true; + } + + Node codeBlock = iifeNode.getFirstFirstChild().getChildAtIndex(2); + String stringifiedCode = compiler.toSource(codeBlock); + + Node wrappedReplacement = + astFactory + .createCall( + astFactory.createNameWithUnknownType(JSCOMP_CLOSURE_UNAWARE_CODE_PRESERVE_FN), + AstFactory.type(JSTypeNative.UNKNOWN_TYPE, StandardColors.UNKNOWN), + astFactory.createString(stringifiedCode)) + .srcrefTree(iifeNode); + + wrappedReplacement.getFirstChild().putBooleanProp(Node.IS_CONSTANT_NAME, true); + + iifeNode.replaceWith(wrappedReplacement); + NodeUtil.markFunctionsDeleted(iifeNode.getFirstFirstChild(), compiler); + t.reportCodeChange(); + } + + private final class UnwrapConcealedClosureUnawareCode implements NodeTraversal.Callback { @Override public boolean shouldTraverse(NodeTraversal t, Node n, @Nullable Node parent) { @@ -272,43 +374,72 @@ public boolean shouldTraverse(NodeTraversal t, Node n, @Nullable Node parent) { } // Once inside a closureUnaware script, we want to traverse the entire thing to make sure we - // find all the nodes marked as closure-unaware shadows. + // find all the calls to $jscomp_wrap_closure_unaware_code. return true; } @Override public void visit(NodeTraversal t, Node n, @Nullable Node parent) { - Node shadowAstRoot = n.getClosureUnawareShadow(); - if (shadowAstRoot == null) { + if (!n.isCall()) { + // We only expect free calls to the preserve function - anything else is probably a bug. + if (n.matchesName(JSCOMP_CLOSURE_UNAWARE_CODE_PRESERVE_FN) && !parent.isCall()) { + compiler.report(JSError.make(n, UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_PRESERVE)); + } return; } - // ROOT -> SCRIPT -> EXPR_RESULT -> FUNCTION - Node originalCodeFunction = shadowAstRoot.getFirstFirstChild().getFirstChild(); - originalCodeFunction.detach(); - n.replaceWith(originalCodeFunction); - t.reportCodeChange(originalCodeFunction); - // Note: you might be tempted to mark this new FUNCTION as "newly created scope". - // This is probably wrong - the scope never actually disappeared from the perspective of the - // compiler, and ChangeVerifier will complain (correctly) if that was done. - - // With the shadowed code detached, we are just iterating over all the synthetic nodes created - // above and marking them as deleted to avoid confusing various compiler verification checks. - NodeUtil.visitPreOrder( - shadowAstRoot, - new NodeUtil.Visitor() { - @Override - public void visit(Node n) { - n.setDeleted(true); - } - }); - // Also mark the NAME node that was hosting the shadow as deleted, in case anything is looking - // for it. - n.setDeleted(true); - - // This ROOT -> SCRIPT -> EXPR_RESULT node structure that was enclosed in the shadow is now - // dangling and we need to inform the compiler that we've removed it. - t.reportCodeChange(shadowAstRoot.getFirstChild()); + Node call = n; + Node callee = call.getFirstChild(); + // <>.call() + if (!callee.matchesName(JSCOMP_CLOSURE_UNAWARE_CODE_PRESERVE_FN)) { + return; + } + + if (call.getChildCount() != 2) { + compiler.report(JSError.make(call, UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_PRESERVE)); + return; // callee and a single arg + } + + Node stringifiedSource = call.getSecondChild(); + if (stringifiedSource == null || !stringifiedSource.isString()) { + compiler.report(JSError.make(call, UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_PRESERVE)); + return; + } + String wrappedSrc = stringifiedSource.getString(); + + // We add the @closureUnaware annotation to the wrapped block so that the parsed SCRIPT node + // will have JSDoc with isClosureUnawareCode returning true. + // We need to do this because this block of code is parsed in isolation and there is no other + // signal present to prevent emitting invalid JSDoc parse errors when the wrapped code + // contains + // "invalid" (non-closure?) JSDoc. + // This annotation is not attached to the CALL node which we will attach to the AST later, and + // that is intentional. + String unwrappedCode = + "/**\n" + + " * @fileoverview\n" + + " * @closureUnaware\n" + + " */\n" + + "(/** @closureUnaware */ function() " + + wrappedSrc + + ").call(globalThis);"; + + try { + Node parsedCodeRoot = + compiler.parseSyntheticCode( + // The wrappedSrc is the entire BLOCK node from the previous function. + call.getSourceFileName(), unwrappedCode); + Node parsedCode = parsedCodeRoot.getOnlyChild().getOnlyChild(); + parsedCode.srcrefTree(call); // The original src info should have been stashed here + parsedCode.detach(); + + call.replaceWith(parsedCode); + NodeUtil.markNewScopesChanged(parsedCode.getFirstFirstChild(), compiler); + t.reportCodeChange(); + } catch (RuntimeException e) { + System.err.println(e); + throw e; + } } } } diff --git a/src/com/google/javascript/jscomp/serialization/ScriptNodeDeserializer.java b/src/com/google/javascript/jscomp/serialization/ScriptNodeDeserializer.java index b78342ecaeb..fe25a09cb1f 100644 --- a/src/com/google/javascript/jscomp/serialization/ScriptNodeDeserializer.java +++ b/src/com/google/javascript/jscomp/serialization/ScriptNodeDeserializer.java @@ -89,8 +89,7 @@ private ScriptNodeDeserializer owner() { return ScriptNodeDeserializer.this; } - private Node visit( - AstNode astNode, @Nullable FeatureContext context, @Nullable Node sourceFileTemplate) { + private Node visit(AstNode astNode, FeatureContext context, @Nullable Node sourceFileTemplate) { if (sourceFileTemplate == null || astNode.getSourceFile() != 0) { // 0 == 'not set' sourceFileTemplate = @@ -114,18 +113,12 @@ private Node visit( n.setLinenoCharno(currentLine, currentColumn); this.previousLine = currentLine; this.previousColumn = currentColumn; - if (context != null) { - this.recordScriptFeatures(context, n); - } + this.recordScriptFeatures(context, n); - @Nullable FeatureContext newContext = contextFor(context, n); + FeatureContext newContext = contextFor(context, n); if (Node.hasBitSet(properties, NodeProperty.CLOSURE_UNAWARE_SHADOW.getNumber())) { AstNode serializedShadowChild = astNode.getChild(0); - // Unlike normal deserialization, we want to avoid recording features for code within the - // shadow because we don't run transpilation passes over it and later stages of the compiler - // attempt to validate that transpilation has successfully run over the entire AST and no - // features remain that won't work in the given language output level. - Node shadowedCode = this.visit(serializedShadowChild, null, sourceFileTemplate); + Node shadowedCode = this.visit(serializedShadowChild, newContext, sourceFileTemplate); this.owner().setOriginalNameIfPresent(serializedShadowChild, shadowedCode); // The shadowed code is only the "source" parts of the shadow structure, and does not // include the synthetic code that is needed for the compiler to consider it a valid @@ -706,11 +699,7 @@ private enum FeatureContext { NONE; } - private static @Nullable FeatureContext contextFor( - @Nullable FeatureContext parentContext, Node node) { - if (parentContext == null) { - return null; - } + private static FeatureContext contextFor(FeatureContext parentContext, Node node) { switch (node.getToken()) { case PARAM_LIST: return FeatureContext.PARAM_LIST; diff --git a/test/com/google/javascript/jscomp/ManageClosureUnawareCodeTest.java b/test/com/google/javascript/jscomp/ManageClosureUnawareCodeTest.java index 574c20ccd2b..df63ac7ec30 100644 --- a/test/com/google/javascript/jscomp/ManageClosureUnawareCodeTest.java +++ b/test/com/google/javascript/jscomp/ManageClosureUnawareCodeTest.java @@ -15,11 +15,6 @@ */ package com.google.javascript.jscomp; -import static com.google.javascript.rhino.testing.NodeSubject.assertNode; - -import com.google.common.collect.ImmutableList; -import com.google.javascript.rhino.IR; -import com.google.javascript.rhino.Node; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -53,13 +48,11 @@ public void tearDown() throws Exception { super.tearDown(); runWrapPass = true; runUnwrapPass = true; - gatheredShadowNodeRoot = IR.root(); + languageInOverride = Optional.empty(); } - private boolean runWrapPass = true; - private boolean runUnwrapPass = true; - private boolean gatherShadowNodesBeforeUnwrapping = true; - private Node gatheredShadowNodeRoot = IR.root(); + public boolean runWrapPass = true; + public boolean runUnwrapPass = true; public Optional languageInOverride = Optional.empty(); @Override @@ -82,28 +75,6 @@ protected CompilerPass getProcessor(Compiler compiler) { .setInternalFactory(ManageClosureUnawareCode::wrap) .build()); } - if (gatherShadowNodesBeforeUnwrapping) { - passes.add( - PassFactory.builder() - .setName("gatherShadowNodes") - .setInternalFactory( - (c) -> - new CompilerPass() { - - @Override - public void process(Node externs, Node root) { - NodeUtil.visitPreOrder( - root, - (Node node) -> { - Node shadow = node.getClosureUnawareShadow(); - if (shadow != null) { - gatheredShadowNodeRoot.addChildToBack(shadow.cloneTree()); - } - }); - } - }) - .build()); - } if (runUnwrapPass) { passes.add( PassFactory.builder() @@ -122,25 +93,10 @@ public void process(Node externs, Node root) { return phaseopt; } - private void doTest(String js, String expectedWrapped, List expectedShadowNodeContents) { + private void doTest(String js, String expectedWrapped) { runWrapPass = true; runUnwrapPass = false; // Validate that only wrapping results in the expected wrapped contents. - gatherShadowNodesBeforeUnwrapping = !expectedShadowNodeContents.isEmpty(); test(js, expectedWrapped); - if (!expectedShadowNodeContents.isEmpty()) { - Node expectedShadowNodeRoot = - expectedShadowNodeContents.stream() - .map(s -> parseExpectedJs(s).detach()) - .collect(IR::root, Node::addChildToBack, Node::addChildToBack); - assertNode(gatheredShadowNodeRoot) - .usingSerializer( - (n) -> - new CodePrinter.Builder(n) - .setCompilerOptions(getLastCompiler().getOptions()) - .setPrettyPrint(true) - .build()) - .isEqualIncludingJsDocTo(expectedShadowNodeRoot); - } // now test with unwrapping enabled so it is a no-op runWrapPass = true; @@ -164,9 +120,7 @@ public void testDirectLoad() { lines( "/** @fileoverview @closureUnaware */", "goog.module('foo.bar.baz_raw');", - "$jscomp_wrap_closure_unaware_code.call(globalThis)"), - ImmutableList.of( - lines("/** @closureUnaware */", "(function() {", " window['foo'] = 5;", "})"))); + "$jscomp_wrap_closure_unaware_code('{window[\"foo\"]=5}')")); } @Test @@ -193,10 +147,8 @@ public void testDirectLoadWithRequireAndExports() { "goog.module('foo.bar.baz_raw');", "goog.require('foo.bar');", "const {a} = goog.require('foo.baz');", - "$jscomp_wrap_closure_unaware_code.call(globalThis)", - "exports = globalThis['foo'];"), - ImmutableList.of( - lines("/** @closureUnaware */", "(function() {", " window['foo'] = 5;", "})"))); + "$jscomp_wrap_closure_unaware_code('{window[\"foo\"]=5}')", + "exports = globalThis['foo'];")); } @Test @@ -221,10 +173,8 @@ public void testConditionalLoad() { " */", "goog.module('foo.bar.baz_raw');", "if (!window['foo']) {", - " $jscomp_wrap_closure_unaware_code.call(globalThis)", - "}"), - ImmutableList.of( - lines("/** @closureUnaware */", "(function() {", " window['foo'] = 5;", "})"))); + " $jscomp_wrap_closure_unaware_code('{window[\"foo\"]=5}');", + "}")); } @Test @@ -254,13 +204,10 @@ public void testDebugSrcLoad() { " */", "goog.module('foo.bar.baz_raw');", "if (goog.DEBUG) {", - " $jscomp_wrap_closure_unaware_code.call(globalThis)", + " $jscomp_wrap_closure_unaware_code('{window[\"foo\"]=5}');", "} else {", - " $jscomp_wrap_closure_unaware_code.call(globalThis)", - "}"), - ImmutableList.of( - lines("/** @closureUnaware */", "(function() {", " window['foo'] = 5;", "})"), - lines("/** @closureUnaware */", "(function() {", " window['foo'] = 10;", "})"))); + " $jscomp_wrap_closure_unaware_code('{window[\"foo\"]=10}');", + "}")); } @Test @@ -293,14 +240,11 @@ public void testConditionalAndDebugSrcLoad() { "goog.module('foo.bar.baz_raw');", "if (!window['foo']) {", " if (goog.DEBUG) {", - " $jscomp_wrap_closure_unaware_code.call(globalThis)", + " $jscomp_wrap_closure_unaware_code('{window[\"foo\"]=5}');", " } else {", - " $jscomp_wrap_closure_unaware_code.call(globalThis)", + " $jscomp_wrap_closure_unaware_code('{window[\"foo\"]=10}');", " }", - "}"), - ImmutableList.of( - lines("/** @closureUnaware */", "(function() {", " window['foo'] = 5;", "})"), - lines("/** @closureUnaware */", "(function() {", " window['foo'] = 10;", "})"))); + "}")); } @Test @@ -328,14 +272,8 @@ public void testDirectLoad_nestedChangeScopes() { " */", "goog.module('foo.bar.baz_raw');", "if (goog.DEBUG) {", - " $jscomp_wrap_closure_unaware_code.call(globalThis)", - "}"), - ImmutableList.of( - lines( - "/** @closureUnaware */", - "(function() {", - " function bar() { window['foo'] = 5;} bar();", - "})"))); + " $jscomp_wrap_closure_unaware_code('{function" + " bar(){window[\"foo\"]=5}bar()}');", + "}")); } @Test @@ -362,16 +300,8 @@ public void testDirectLoad_nestedGlobalThisIIFEIsNotRewritten() { " */", "goog.module('foo.bar.baz_raw');", "if (goog.DEBUG) {", - " $jscomp_wrap_closure_unaware_code.call(globalThis)", - "}"), - ImmutableList.of( - lines( - "/** @closureUnaware */", - "(function() {", - " (function() {", - " window['foo'] = 10;", - " }).call(globalThis);", - "})"))); + " $jscomp_wrap_closure_unaware_code('{(function(){window[\"foo\"]=10}).call(globalThis)}');", + "}")); } @Test @@ -388,7 +318,8 @@ public void testUnwrap_doesNotEmitParseErrorsThatShouldBeSuppressed() { } @Test - public void testErrorsOnWrapping_invalidAnnotation_statement() { + public void testErrorsOnUnwrapping_nonCallRef() { + runWrapPass = false; // This error only occurs when unwrapping testError( lines( "/**", @@ -396,77 +327,50 @@ public void testErrorsOnWrapping_invalidAnnotation_statement() { " * @closureUnaware", " */", "goog.module('foo.bar.baz_raw');", - "/** @closureUnaware */", - "exports.bar = 10;"), - ManageClosureUnawareCode.UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_CODE); + "var x = $jscomp_wrap_closure_unaware_code;", + "x('window[\"foo\"]=5')"), + ManageClosureUnawareCode.UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_PRESERVE); } @Test - public void testErrorsForScriptsWithoutFileoverviewClosureUnawareAnnotation() { + public void testErrorsOnUnwrapping_invalidCallRef_tooManyArgs() { + runWrapPass = false; // This error only occurs when unwrapping testError( lines( "/**", " * @fileoverview", + " * @closureUnaware", " */", "goog.module('foo.bar.baz_raw');", - "/** @closureUnaware */", - "(function() {", - " window['foo'] = 10;", - "}).call(globalThis);"), - ManageClosureUnawareCode.UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_CODE); + "$jscomp_wrap_closure_unaware_code(this, 'window[\"foo\"]=5')"), + ManageClosureUnawareCode.UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_PRESERVE); } @Test - public void testErrorsIfMissingNodeLevelAnnotation() { - // It is an error to have a file-level @closureUnaware annotation, but no annotation on some - // other node to indicate the start of the closure-unaware sub-AST. - testError( + public void testErrorsOnUnwrapping_invalidCallRef_ignoresOtherCalls() { + runWrapPass = false; // This error only occurs when unwrapping + testNoWarning( lines( "/**", " * @fileoverview", " * @closureUnaware", " */", "goog.module('foo.bar.baz_raw');", - "$jscomp_wrap_closure_unaware_code(5)"), - ManageClosureUnawareCode.UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_CODE); + "eval(this, 'window[\"foo\"]=5')")); } @Test - public void testNoErrorsForConfusingNestedAnnotations() { - doTest( - lines( - "/**", - " * @fileoverview", - " * @closureUnaware", - " */", - "goog.module('foo.bar.baz_raw');", - "(/** @closureUnaware */ function() {", - // This nested usage of the @closureUnaware annotation is not a parse error, but it - // could be confusing if a human author expected there to be multiple shadowed ASTs / - // sections of closure-aware and closure-unaware code. - // This test-case is just codifying that it is always the top-most AST node that is - // converted into a shadow AST node, and that there won't be nested shadow ASTs. - " (/** @closureUnaware */ function() {", - " window['foo'] = 10;", - " }).call(globalThis);", - "}).call(globalThis);"), + public void testErrorsOnUnwrapping_invalidCallRef_wrongArgType() { + runWrapPass = false; // This error only occurs when unwrapping + testError( lines( "/**", " * @fileoverview", " * @closureUnaware", " */", "goog.module('foo.bar.baz_raw');", - "$jscomp_wrap_closure_unaware_code.call(globalThis)"), - ImmutableList.of( - lines( - "/** @closureUnaware */", - "(function() {", - // Note that there isn't a JSDoc closureUnaware annotation on the inner function, as - // it was never created during parsing of the AST. - " (function() {", - " window['foo'] = 10;", - " }).call(globalThis);", - "})"))); + "$jscomp_wrap_closure_unaware_code(5)"), + ManageClosureUnawareCode.UNEXPECTED_JSCOMPILER_CLOSURE_UNAWARE_PRESERVE); } @Test @@ -488,9 +392,7 @@ public void testAllowsSpecifyingAnnotationOnIIFE() { " * @closureUnaware", " */", "goog.module('foo.bar.baz_raw');", - "$jscomp_wrap_closure_unaware_code.call(globalThis)"), - ImmutableList.of( - lines("/** @closureUnaware */", "(function() {", " window['foo'] = 5;", "})"))); + "$jscomp_wrap_closure_unaware_code('{window[\"foo\"]=5}')")); } @Test @@ -515,8 +417,6 @@ public void testReparseWithInvalidJSDocTags() { " * @closureUnaware", " */", "goog.module('foo.bar.baz_raw');", - "$jscomp_wrap_closure_unaware_code.call(globalThis)"), - ImmutableList.of( - lines("/** @closureUnaware */", "(function() {", " window['foo'] = 5;", "})"))); + "$jscomp_wrap_closure_unaware_code('{window[\"foo\"]=5}')")); } }