Skip to content

Commit 17efd88

Browse files
lauraharkercopybara-github
authored andcommitted
Fix incorrect change scope reporting in Es6RewriteDestructuring & IsolatePolyfills
Es6RewriteDestructuring was reporting nested scopes were changed, when they were not; IsolatedPolyfills was failing to report adding a new temporary variable to the initial script. PiperOrigin-RevId: 830553255
1 parent 1e8e543 commit 17efd88

File tree

4 files changed

+56
-3
lines changed

4 files changed

+56
-3
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,6 @@ private void visitDestructuringPatternInVanillaForInnerVars(NodeTraversal t, Nod
889889
}
890890

891891
this.compiler.reportChangeToEnclosingScope(insertionPoint);
892-
NodeUtil.markNewScopesChanged(insertionPoint, compiler);
893892
}
894893

895894
/** for (const [a, b, c] of arr) */

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ private Node createTempName(Node srcref) {
304304
isTempVarInitialized = true;
305305
Node decl = IR.var(IR.name(POLYFILL_TEMP)).srcrefTree(srcref);
306306
compiler.getNodeForCodeInsertion(null).addChildToFront(decl);
307+
compiler.reportChangeToEnclosingScope(decl);
307308
}
308309
// The same temporary variable is always used for every polyfill invocation. This is believed
309310
// to be safe and makes the code easier to generate and smaller. There's a change it will make
@@ -354,7 +355,7 @@ private void cleanUpJscompLookupPolyfilledValue() {
354355

355356
Scope syntheticCodeScope =
356357
new SyntacticScopeCreator(compiler)
357-
.createScope(compiler.getNodeForCodeInsertion(/* module= */ null), /* parent= */ null);
358+
.createScope(compiler.getNodeForCodeInsertion(/* chunk= */ null), /* parent= */ null);
358359
Var syntheticVar = syntheticCodeScope.getVar(jscompLookupMethod.getString());
359360
// Don't error if we can't find a definition for jscompLookupMethod. It's possible that we are
360361
// running in transpileOnly mode and are not injecting runtime libraries.

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,37 @@ protected CompilerPass getProcessor(Compiler compiler) {
8080
.build();
8181
}
8282

83+
@Test
84+
public void testForLoopInitializer_let_accessedInClosure() {
85+
test(
86+
"""
87+
const aClosures = [];
88+
const bClosures = [];
89+
for (let a = 0, [b, c] = [a, 3]; b < c; a--, b++) {
90+
aClosures.push(() => a);
91+
bClosures.push(() => b);
92+
}
93+
""",
94+
"""
95+
const aClosures = [];
96+
const bClosures = [];
97+
for (let a = 0, b, c, $jscomp$destructuring$var0$unused = (() => {
98+
let $jscomp$destructuring$var1 = [a, 3];
99+
var $jscomp$destructuring$var2 = (0,$jscomp.makeIterator)($jscomp$destructuring$var1);
100+
b = $jscomp$destructuring$var2.next().value;
101+
c = $jscomp$destructuring$var2.next().value;
102+
return $jscomp$destructuring$var1;
103+
})(); b < c; a--, b++) {
104+
aClosures.push(() => {
105+
return a;
106+
});
107+
bClosures.push(() => {
108+
return b;
109+
});
110+
}
111+
""");
112+
}
113+
83114
@Test
84115
public void testObjectDestructuring_forLoopInitializer_doesNotCrash() {
85116
test(

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ protected CompilerPass getProcessor(Compiler compiler) {
7171
.getSynthesizedExternsInput()
7272
.getAstRoot(compiler)
7373
.addChildToBack(IR.var(IR.name("$jscomp$lookupPolyfilledValue")));
74-
addPolyfillInjection(compiler.getNodeForCodeInsertion(/* module= */ null), compiler);
74+
addPolyfillInjection(compiler.getNodeForCodeInsertion(/* chunk= */ null), compiler);
7575
new IsolatePolyfills(compiler, Polyfills.fromTable(Joiner.on("\n").join(polyfillTable)))
7676
.process(externs, root);
7777
};
@@ -479,4 +479,26 @@ public void testJscompLookupPolyfillKeptIfUsed() {
479479
.call($jscomp$polyfillTmp, cb);
480480
""");
481481
}
482+
483+
@Test
484+
public void testPolyfilTemp_multipleFiles_insertedInFirstFile() {
485+
// Regression test for failure to report code change when adding "var $jscomp$polyfillTmp;"
486+
// Don't call addLibrary() for this test: it reports a code change regardless & make the
487+
// test incorrectly pass.
488+
polyfillTable.add("String.prototype.endsWith es6 es5 es6/string/endswith");
489+
setLanguage(ES6, ES5);
490+
491+
test(
492+
srcs("$jscomp.polyfill('String.prototype.endsWith', 'es6', 'es5');", "x().endsWith(y);"),
493+
expected(
494+
"""
495+
var $jscomp$polyfillTmp;
496+
$jscomp.polyfill('String.prototype.endsWith', 'es6', 'es5');
497+
""",
498+
"""
499+
($jscomp$polyfillTmp = x(),
500+
$jscomp$lookupPolyfilledValue($jscomp$polyfillTmp, 'endsWith'))
501+
.call($jscomp$polyfillTmp, y);
502+
"""));
503+
}
482504
}

0 commit comments

Comments
 (0)