New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed generateUid creating clashing ids after scope re-crawling #11375
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -155,6 +155,7 @@ const collectorVisitor = { | |
For(path) { | ||
for (const key of (t.FOR_INIT_KEYS: Array)) { | ||
const declar = path.get(key); | ||
// delegate block scope handling to the `BlockScoped` method | ||
if (declar.isVar()) { | ||
const parentScope = | ||
path.scope.getFunctionParent() || path.scope.getProgramParent(); | ||
|
@@ -253,6 +254,31 @@ const collectorVisitor = { | |
} | ||
} | ||
}, | ||
|
||
CatchClause(path) { | ||
path.scope.registerBinding("let", path); | ||
}, | ||
|
||
Function(path) { | ||
if ( | ||
path.isFunctionExpression() && | ||
path.has("id") && | ||
!path.get("id").node[t.NOT_LOCAL_BINDING] | ||
) { | ||
path.scope.registerBinding("local", path.get("id"), path); | ||
} | ||
|
||
const params: Array<NodePath> = path.get("params"); | ||
for (const param of params) { | ||
path.scope.registerBinding("param", param); | ||
} | ||
}, | ||
|
||
ClassExpression(path) { | ||
if (path.has("id") && !path.get("id").node[t.NOT_LOCAL_BINDING]) { | ||
path.scope.registerBinding("local", path); | ||
} | ||
}, | ||
}; | ||
|
||
let uid = 0; | ||
|
@@ -279,6 +305,7 @@ export default class Scope { | |
this.path = path; | ||
|
||
this.labels = new Map(); | ||
this.inited = false; | ||
} | ||
|
||
/** | ||
|
@@ -761,7 +788,10 @@ export default class Scope { | |
} | ||
|
||
init() { | ||
if (!this.references) this.crawl(); | ||
if (!this.inited) { | ||
this.inited = true; | ||
this.crawl(); | ||
} | ||
} | ||
|
||
crawl() { | ||
|
@@ -773,50 +803,24 @@ export default class Scope { | |
this.uids = Object.create(null); | ||
this.data = Object.create(null); | ||
|
||
// ForStatement - left, init | ||
|
||
if (path.isLoop()) { | ||
for (const key of (t.FOR_INIT_KEYS: Array<string>)) { | ||
const node = path.get(key); | ||
if (node.isBlockScoped()) this.registerBinding(node.node.kind, node); | ||
} | ||
} | ||
|
||
// FunctionExpression - id | ||
|
||
if (path.isFunctionExpression() && path.has("id")) { | ||
if (!path.get("id").node[t.NOT_LOCAL_BINDING]) { | ||
// TODO: explore removing this as it should be covered by collectorVisitor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. attempt to remove this has resulted in 14~ failing tests, most of the updated snapshots seemed OK, but some didn't. It requires more investigation and I'm afraid that I won't have time to do this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
if (path.isFunction()) { | ||
if ( | ||
path.isFunctionExpression() && | ||
path.has("id") && | ||
!path.get("id").node[t.NOT_LOCAL_BINDING] | ||
) { | ||
this.registerBinding("local", path.get("id"), path); | ||
} | ||
} | ||
|
||
// Class | ||
|
||
if (path.isClassExpression() && path.has("id")) { | ||
if (!path.get("id").node[t.NOT_LOCAL_BINDING]) { | ||
this.registerBinding("local", path); | ||
} | ||
} | ||
|
||
// Function - params, rest | ||
|
||
if (path.isFunction()) { | ||
const params: Array<NodePath> = path.get("params"); | ||
for (const param of params) { | ||
this.registerBinding("param", param); | ||
} | ||
} | ||
|
||
// CatchClause - param | ||
|
||
if (path.isCatchClause()) { | ||
this.registerBinding("let", path); | ||
} | ||
|
||
// Program | ||
|
||
const parent = this.getProgramParent(); | ||
if (parent.crawling) return; | ||
const programParent = this.getProgramParent(); | ||
if (programParent.crawling) return; | ||
|
||
const state = { | ||
references: [], | ||
|
@@ -832,11 +836,8 @@ export default class Scope { | |
for (const path of state.assignments) { | ||
// register undeclared bindings as globals | ||
const ids = path.getBindingIdentifiers(); | ||
let programParent; | ||
for (const name of Object.keys(ids)) { | ||
if (path.scope.getBinding(name)) continue; | ||
|
||
programParent = programParent || path.scope.getProgramParent(); | ||
programParent.addGlobal(ids[name]); | ||
} | ||
|
||
|
@@ -850,7 +851,7 @@ export default class Scope { | |
if (binding) { | ||
binding.reference(ref); | ||
} else { | ||
ref.scope.getProgramParent().addGlobal(ref.node); | ||
programParent.addGlobal(ref.node); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this check was super implicit and confusing (nested scopes don't even gather
references
so this object stays always empty for them, but is initialized in.crawl
)