Skip to content

Commit

Permalink
Enforce TDZ within initializer of lexical declaration (refactored) (#…
Browse files Browse the repository at this point in the history 
…2733)

* [[FIX]] Enforce TDZ within initializer of lexical declaration

Fixes #2637

* [[FIX]] Enforce TDZ within class heritage definition

* [[FIX]] Enforce TDZ within for in/of head

Fixes gh-2693

* [[CHORE]] Refactor var initialization tracking

Conceptually, the initialization state of a given variable is a property
of that variable. Model that relationship in the code organization by
tracking variable initialization using a dedicated property on the
representation of the variable itself. Beyond improving code clarity
(the "label" objects remain the single source of truth for the state of
each variable), this also reduces memory allocation costs (no new
objects need to be created).
  • Loading branch information
@jugglinmike @rwaldron
jugglinmike authored and rwaldron committed Oct 18, 2016
1 parent f7eb3d7 commit 8e9d406
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 18 deletions.
28 changes: 24 additions & 4 deletions src/jshint.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3568,10 +3568,6 @@ var JSHINT = (function() {
type: type,
token: t.token });
names.push(t.token);

if (lone && inexport) {
state.funct["(scope)"].setExported(t.token.value, t.token);
}
}
}
}
Expand All @@ -3594,6 +3590,19 @@ var JSHINT = (function() {
}
}

if (!prefix) {
for (t in tokens) {
if (tokens.hasOwnProperty(t)) {
t = tokens[t];
state.funct["(scope)"].initialize(t.id);

if (lone && inexport) {
state.funct["(scope)"].setExported(t.token.value, t.token);
}
}
}
}

statement.first = statement.first.concat(names);

if (state.tokens.next.id !== ",") {
Expand Down Expand Up @@ -3732,14 +3741,21 @@ var JSHINT = (function() {
state.funct["(scope)"].addlabel(this.name, {
type: "class",
token: state.tokens.curr });

} else if (state.tokens.next.identifier && state.tokens.next.value !== "extends") {
// BindingIdentifier(opt)
this.name = identifier();
this.namedExpr = true;
} else {
this.name = state.nameStack.infer();
}

classtail(this);

if (isStatement) {
state.funct["(scope)"].initialize(this.name);
}

return this;
}

Expand Down Expand Up @@ -4584,6 +4600,7 @@ var JSHINT = (function() {
// Import bindings are immutable (see ES6 8.1.1.5.5)
state.funct["(scope)"].addlabel(this.name, {
type: "const",
initialized: true,
token: state.tokens.curr });

if (state.tokens.next.value === ",") {
Expand All @@ -4610,6 +4627,7 @@ var JSHINT = (function() {
// Import bindings are immutable (see ES6 8.1.1.5.5)
state.funct["(scope)"].addlabel(this.name, {
type: "const",
initialized: true,
token: state.tokens.curr });
}
} else {
Expand All @@ -4635,6 +4653,7 @@ var JSHINT = (function() {
// Import bindings are immutable (see ES6 8.1.1.5.5)
state.funct["(scope)"].addlabel(importName, {
type: "const",
initialized: true,
token: state.tokens.curr });

if (state.tokens.next.value === ",") {
Expand Down Expand Up @@ -4699,6 +4718,7 @@ var JSHINT = (function() {
if (this.block) {
state.funct["(scope)"].addlabel(identifier, {
type: exportType,
initialized: true,
token: token });

state.funct["(scope)"].setExported(identifier, token);
Expand Down
20 changes: 18 additions & 2 deletions src/scope-manager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,12 +611,19 @@ var scopeManager = function(state, predefined, exported, declared) {
this.block.use(labelName, token);
},

initialize: function(labelName) {
if (_current["(labels)"][labelName]) {
_current["(labels)"][labelName]["(initialized)"] = true;
}
},

/**
* adds an indentifier to the relevant current scope and creates warnings/errors as necessary
* @param {string} labelName
* @param {Object} opts
* @param {String} opts.type - the type of the label e.g. "param", "var", "let, "const", "function"
* @param {Token} opts.token - the token pointing at the declaration
* @param {boolean} opts.initialized - whether the binding should be created in an "initialized" state.
*/
addlabel: function(labelName, opts) {

Expand Down Expand Up @@ -664,7 +671,9 @@ var scopeManager = function(state, predefined, exported, declared) {
}
}

scopeManagerInst.block.add(labelName, type, token, !isexported);
scopeManagerInst.block.add(
labelName, type, token, !isexported, opts.initialized
);

} else {

Expand Down Expand Up @@ -805,6 +814,12 @@ var scopeManager = function(state, predefined, exported, declared) {
token["(function)"] = _currentFunctBody;
_current["(usages)"][labelName]["(tokens)"].push(token);
}

// blockscoped vars can't be used within their initializer (TDZ)
var label = _current["(labels)"][labelName];
if (label && label["(blockscoped)"] && !label["(initialized)"]) {
error("E056", token, labelName, label["(type)"]);
}
},

reassign: function(labelName, token) {
Expand All @@ -827,10 +842,11 @@ var scopeManager = function(state, predefined, exported, declared) {
/**
* Adds a new variable
*/
add: function(labelName, type, tok, unused) {
add: function(labelName, type, tok, unused, initialized) {
_current["(labels)"][labelName] = {
"(type)" : type,
"(token)": tok,
"(initialized)": !!initialized,
"(blockscoped)": true,
"(unused)": unused };
},
Expand Down
115 changes: 108 additions & 7 deletions tests/unit/core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -925,12 +925,13 @@ exports.testConstRedeclaration = function (test) {
];

TestRun(test)
.addError(2, "'a' has already been declared.")
.addError(9, "'a' has already been declared.")
.addError(13, "'b' has already been declared.")
.test(src, {
esnext: true
});
.addError(2, "'a' has already been declared.")
.addError(6, "'a' was used before it was declared, which is illegal for 'const' variables.")
.addError(9, "'a' has already been declared.")
.addError(13, "'b' has already been declared.")
.test(src, {
esnext: true
});

test.done();
};
Expand Down Expand Up @@ -2088,4 +2089,104 @@ exports["destructuring in setter parameter"] = function (test) {
], { esversion: 6 });

test.done();
};
};

exports["TDZ within initializer of lexical declarations"] = function(test) {
var code = [
"let a = a;",
"const b = b;",
"let c = () => c;",
"const d = () => d;",
// line 5
"let e = {",
" x: e,",
" y: () => e",
"};",
"const f = {",
" x: f,",
" y: () => f",
"};",
// line 13
"let g, h = g;",
"const i = 0, j = i;",
"let [ k, l ] = l;",
"const [ m, n ] = n;",
// line 17
"let o = (() => o) + o;",
"const p = (() => p) + p;"
];

TestRun(test)
.addError(1, "'a' was used before it was declared, which is illegal for 'let' variables.")
.addError(2, "'b' was used before it was declared, which is illegal for 'const' variables.")
.addError(6, "'e' was used before it was declared, which is illegal for 'let' variables.")
.addError(10, "'f' was used before it was declared, which is illegal for 'const' variables.")
.addError(15, "'l' was used before it was declared, which is illegal for 'let' variables.")
.addError(16, "'n' was used before it was declared, which is illegal for 'const' variables.")
.addError(17, "'o' was used before it was declared, which is illegal for 'let' variables.")
.addError(18, "'p' was used before it was declared, which is illegal for 'const' variables.")
.test(code, { esversion: 6 });

test.done();
};

exports["TDZ within class heritage definition"] = function(test) {
var code = [
"let A = class extends A {};",
"let B = class extends { B } {};",
"let C = class extends { method() { return C; } } {};",
// line 4
"const D = class extends D {};",
"const E = class extends { E } {};",
"const F = class extends { method() { return F; } } {};",
// line 7
"class G extends G {}",
"class H extends { H } {}",
"class I extends { method() { return I; }} {}"
];

TestRun(test)
.addError(1, "'A' was used before it was declared, which is illegal for 'let' variables.")
.addError(2, "'B' was used before it was declared, which is illegal for 'let' variables.")
.addError(4, "'D' was used before it was declared, which is illegal for 'const' variables.")
.addError(5, "'E' was used before it was declared, which is illegal for 'const' variables.")
.addError(7, "'G' was used before it was declared, which is illegal for 'class' variables.")
.addError(8, "'H' was used before it was declared, which is illegal for 'class' variables.")
.test(code, { esversion: 6 });

test.done();
};

exports["TDZ within for in/of head"] = function(test) {
var code = [
"for (let a in a);",
"for (const b in b);",
"for (let c of c);",
"for (const d of d);",

// line 5
"for (let e in { e });",
"for (const f in { f });",
"for (let g of { g });",
"for (const h of { h });",

// line 9
"for (let i in { method() { return i; } });",
"for (const j in { method() { return j; } });",
"for (let k of { method() { return k; } });",
"for (const l of { method() { return l; } });"
];

TestRun(test)
.addError(1, "'a' was used before it was declared, which is illegal for 'let' variables.")
.addError(2, "'b' was used before it was declared, which is illegal for 'const' variables.")
.addError(3, "'c' was used before it was declared, which is illegal for 'let' variables.")
.addError(4, "'d' was used before it was declared, which is illegal for 'const' variables.")
.addError(5, "'e' was used before it was declared, which is illegal for 'let' variables.")
.addError(6, "'f' was used before it was declared, which is illegal for 'const' variables.")
.addError(7, "'g' was used before it was declared, which is illegal for 'let' variables.")
.addError(8, "'h' was used before it was declared, which is illegal for 'const' variables.")
.test(code, { esversion: 6 });

test.done();
};
9 changes: 4 additions & 5 deletions tests/unit/options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ exports.shadowEs6 = function (test) {
[344, "'zzi' has already been declared."],
[345, "'zzj' has already been declared."],
[349, "'zzl' has already been declared."],
[349, "'zzl' was used before it was declared, which is illegal for 'const' variables."],
[350, "'zzm' has already been declared."],
[350, "'zzm' was used before it was declared, which is illegal for 'let' variables."],
[364, "'zj' has already been declared."]
];

Expand Down Expand Up @@ -759,11 +761,8 @@ exports.undef = function (test) {

// block scope cannot use themselves in the declaration
TestRun(test)
// JSHint does not currently enforce the correct temporal dead zone
// semantics in this case. Once this is fixed, the following errors
// should be thrown:
//.addError(1, "'a' was used before it was declared, which is illegal for 'let' variables.")
//.addError(2, "'b' was used before it was declared, which is illegal for 'const' variables.")
.addError(1, "'a' was used before it was declared, which is illegal for 'let' variables.")
.addError(2, "'b' was used before it was declared, which is illegal for 'const' variables.")
.addError(5, "'e' is already defined.")
.test([
'let a = a;',
Expand Down

0 comments on commit 8e9d406

Please sign in to comment.