Skip to content

Commit

Permalink
Add workaround for Safari for loop lexical scope bug (#567)
Browse files Browse the repository at this point in the history
* Add workaround for Safari for loop lexical scope bug

Safari raises a syntax error for a `let` or `const` declaration in a
`for` loop initalization that shadows a parent function's parameter:

```js
function a(b) {
  for (let b;;);
}
```

This is valid code, but Safari throws a syntax error. The bug has been
[reported](https://bugs.webkit.org/show_bug.cgi?id=171041) and since
[fixed](https://trac.webkit.org/changeset/217200/webkit/trunk/Source) in
WebKit, so future versions of Safari will not have this problem.

This modifies the scope tracker's `canUseInReferencedScopes` method to
detect cases where a rename would trigger this bug and use a different
name.

Fixes #559

* More idiomatic implementation

* More extensive tests

* Better use path API
  • Loading branch information
btmills authored and boopathi committed Jun 13, 2017
1 parent 90ee63b commit cb993ed
Show file tree
Hide file tree
Showing 2 changed files with 370 additions and 0 deletions.
Expand Up @@ -1281,4 +1281,343 @@ describe("mangle-names", () => {
`);
expect(transform(source, { topLevel: true }, "module")).toBe(expected);
});

// Safari raises a syntax error for a `let` or `const` declaration in a `for`
// loop initialization that shadows a parent function's parameter.
// https://github.com/babel/babili/issues/559
// https://bugs.webkit.org/show_bug.cgi?id=171041
// https://trac.webkit.org/changeset/217200/webkit/trunk/Source
describe("Safari for loop lexical scope workaround", () => {
it("should permit shadowing in top-level for loops", () => {
const source = unpad(`
var a;
for (a = 0;;);
for (a of x);
for (x of a);
for (a in x);
for (x in a);
for (;; a++);
for (;; a = 1);
for (let b;;);
for (let b of x);
for (const b of x);
for (let b in x);
for (const b in x);
for (let [b, c] of x);
for (const [b, c] of x);
for (let [b, c] in x);
for (const [b, c] in x);
for (let { c: { b: { a } } } = x;;);
for (;; () => {
let a = 1;
});
`);
const expected = unpad(`
var a;
for (a = 0;;);
for (a of x);
for (x of a);
for (a in x);
for (x in a);
for (;; a++);
for (;; a = 1);
for (let a;;);
for (let a of x);
for (const a of x);
for (let a in x);
for (const a in x);
for (let [a, b] of x);
for (const [a, b] of x);
for (let [a, b] in x);
for (const [a, b] in x);
for (let { c: { b: { a: b } } } = x;;);
for (;; () => {
let b = 1;
});
`);
expect(transform(source)).toBe(expected);
});

it("should permit shadowing in nested for loops", () => {
const source = unpad(`
function a(a) {
{
for (a = 0;;);
for (a of x);
for (x of a);
for (a in x);
for (x in a);
for (;; a++);
for (;; a = 1);
for (let a;;);
for (let a of x);
for (const a of x);
for (let a in x);
for (const a in x);
for (let [a, b] of x);
for (const [a, b] of x);
for (let [a, b] in x);
for (const [a, b] in x);
for (let { c: { b: { a } } } = x;;);
for (;; () => {
let a = 1;
});
}
}
`);
const expected = unpad(`
function a(b) {
{
for (b = 0;;);
for (b of x);
for (x of b);
for (b in x);
for (x in b);
for (;; b++);
for (;; b = 1);
for (let b;;);
for (let b of x);
for (const b of x);
for (let b in x);
for (const b in x);
for (let [c, a] of x);
for (const [c, a] of x);
for (let [c, a] in x);
for (const [c, a] in x);
for (let { c: { b: { a: b } } } = x;;);
for (;; () => {
let b = 1;
});
}
}
`);
expect(transform(source)).toBe(expected);
});

it("should not shadow params in function declaration top-level for loops", () => {
const source = unpad(`
function a(a) {
for (a = 0;;);
for (a of x);
for (x of a);
for (a in x);
for (x in a);
for (;; a++);
for (;; a = 1);
for (let b;;);
for (let b of x);
for (const b of x);
for (let b in x);
for (const b in x);
for (let [b, c] of x);
for (const [b, c] of x);
for (let [b, c] in x);
for (const [b, c] in x);
for (let { c: { b: { a } } } = x;;);
for (;; () => {
let a = 1;
});
}
`);
const expected = unpad(`
function a(b) {
for (b = 0;;);
for (b of x);
for (x of b);
for (b in x);
for (x in b);
for (;; b++);
for (;; b = 1);
for (let a;;);
for (let a of x);
for (const a of x);
for (let a in x);
for (const a in x);
for (let [a, d] of x);
for (const [a, d] of x);
for (let [a, d] in x);
for (const [a, d] in x);
for (let { c: { b: { a: c } } } = x;;);
for (;; () => {
let b = 1;
});
}
`);
expect(transform(source)).toBe(expected);
});

it("should not shadow params in function expression top-level for loops", () => {
const source = unpad(`
var a = function (a) {
for (a = 0;;);
for (a of x);
for (x of a);
for (a in x);
for (x in a);
for (;; a++);
for (;; a = 1);
for (let b;;);
for (let b of x);
for (const b of x);
for (let b in x);
for (const b in x);
for (let [b, c] of x);
for (const [b, c] of x);
for (let [b, c] in x);
for (const [b, c] in x);
for (let { c: { b: { a } } } = x;;);
for (;; () => {
let a = 1;
});
};
`);
const expected = unpad(`
var a = function (b) {
for (b = 0;;);
for (b of x);
for (x of b);
for (b in x);
for (x in b);
for (;; b++);
for (;; b = 1);
for (let a;;);
for (let a of x);
for (const a of x);
for (let a in x);
for (const a in x);
for (let [a, d] of x);
for (const [a, d] of x);
for (let [a, d] in x);
for (const [a, d] in x);
for (let { c: { b: { a: c } } } = x;;);
for (;; () => {
let b = 1;
});
};
`);
expect(transform(source)).toBe(expected);
});

it("should not shadow params in arrow function top-level for loops", () => {
const source = unpad(`
var a = (a) => {
for (a = 0;;);
for (a of x);
for (x of a);
for (a in x);
for (x in a);
for (;; a++);
for (;; a = 1);
for (let b;;);
for (let b of x);
for (const b of x);
for (let b in x);
for (const b in x);
for (let [b, c] of x);
for (const [b, c] of x);
for (let [b, c] in x);
for (const [b, c] in x);
for (let { c: { b: { a } } } = x;;);
for (;; () => {
let a = 1;
});
};
`);
const expected = unpad(`
var a = (b) => {
for (b = 0;;);
for (b of x);
for (x of b);
for (b in x);
for (x in b);
for (;; b++);
for (;; b = 1);
for (let a;;);
for (let a of x);
for (const a of x);
for (let a in x);
for (const a in x);
for (let [a, d] of x);
for (const [a, d] of x);
for (let [a, d] in x);
for (const [a, d] in x);
for (let { c: { b: { a: c } } } = x;;);
for (;; () => {
let b = 1;
});
};
`);
expect(transform(source)).toBe(expected);
});

it("should not shadow params in class method top-level for loops", () => {
const source = unpad(`
class a {
a(a) {
for (a = 0;;);
for (a of x);
for (x of a);
for (a in x);
for (x in a);
for (;; a++);
for (;; a = 1);
for (let b;;);
for (let b of x);
for (const b of x);
for (let b in x);
for (const b in x);
for (let [b, c] of x);
for (const [b, c] of x);
for (let [b, c] in x);
for (const [b, c] in x);
for (let { c: { b: { a } } } = x;;);
for (;; () => {
let a = 1;
});
}
}
`);
const expected = unpad(`
class a {
a(b) {
for (b = 0;;);
for (b of x);
for (x of b);
for (b in x);
for (x in b);
for (;; b++);
for (;; b = 1);
for (let a;;);
for (let a of x);
for (const a of x);
for (let a in x);
for (const a in x);
for (let [a, d] of x);
for (const [a, d] of x);
for (let [a, d] in x);
for (const [a, d] in x);
for (let { c: { b: { a: c } } } = x;;);
for (;; () => {
let b = 1;
});
}
}
`);
expect(transform(source)).toBe(expected);
});
});
});
31 changes: 31 additions & 0 deletions packages/babel-plugin-minify-mangle-names/src/scope-tracker.js
Expand Up @@ -121,6 +121,37 @@ module.exports = class ScopeTracker {
return false;
}

// Safari raises a syntax error for a `let` or `const` declaration in a
// `for` loop initialization that shadows a parent function's parameter.
// https://github.com/babel/babili/issues/559
// https://bugs.webkit.org/show_bug.cgi?id=171041
// https://trac.webkit.org/changeset/217200/webkit/trunk/Source
const maybeDecl = binding.path.parentPath;
const isBlockScoped =
maybeDecl.isVariableDeclaration({ kind: "let" }) ||
maybeDecl.isVariableDeclaration({ kind: "const" });
if (isBlockScoped) {
const maybeFor = maybeDecl.parentPath;
const isForLoopBinding =
maybeFor.isForStatement({ init: maybeDecl.node }) ||
maybeFor.isForXStatement({ left: maybeDecl.node });
if (isForLoopBinding) {
const fnParent = maybeFor.getFunctionParent();
if (fnParent.isFunction({ body: maybeFor.parent })) {
const parentFunctionBinding = this.bindings
.get(fnParent.scope)
.get(next);
if (parentFunctionBinding) {
const parentFunctionHasParamBinding =
parentFunctionBinding.kind === "param";
if (parentFunctionHasParamBinding) {
return false;
}
}
}
}
}

for (let i = 0; i < binding.constantViolations.length; i++) {
const violation = binding.constantViolations[i];
if (tracker.hasBindingOrReference(violation.scope, binding, next)) {
Expand Down

0 comments on commit cb993ed

Please sign in to comment.