-
Notifications
You must be signed in to change notification settings - Fork 12.8k
disallow recursive references for block-scoped bindings #2309
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
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 |
---|---|---|
|
@@ -203,6 +203,33 @@ module ts { | |
isCatchClauseVariableDeclaration(declaration); | ||
} | ||
|
||
export function getEnclosingBlockScopeContainer(node: Node): Node { | ||
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. Did anything change here (can't tell cuz it moved)? 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. no, code was just moved from emitter to utilities |
||
var current = node; | ||
while (current) { | ||
if (isFunctionLike(current)) { | ||
return current; | ||
} | ||
switch (current.kind) { | ||
case SyntaxKind.SourceFile: | ||
case SyntaxKind.CaseBlock: | ||
case SyntaxKind.CatchClause: | ||
case SyntaxKind.ModuleDeclaration: | ||
case SyntaxKind.ForStatement: | ||
case SyntaxKind.ForInStatement: | ||
case SyntaxKind.ForOfStatement: | ||
return current; | ||
case SyntaxKind.Block: | ||
// function block is not considered block-scope container | ||
// see comment in binder.ts: bind(...), case for SyntaxKind.Block | ||
if (!isFunctionLike(current.parent)) { | ||
return current; | ||
} | ||
} | ||
|
||
current = current.parent; | ||
} | ||
} | ||
|
||
export function isCatchClauseVariableDeclaration(declaration: Declaration) { | ||
return declaration && | ||
declaration.kind === SyntaxKind.VariableDeclaration && | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
tests/cases/conformance/es6/for-ofStatements/for-of55.ts(2,15): error TS2448: Block-scoped variable 'v' used before its declaration. | ||
|
||
|
||
==== tests/cases/conformance/es6/for-ofStatements/for-of55.ts (1 errors) ==== | ||
let v = [1]; | ||
for (let v of v) { | ||
~ | ||
!!! error TS2448: Block-scoped variable 'v' used before its declaration. | ||
v; | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
tests/cases/compiler/recursiveLetConst.ts(2,9): error TS2448: Block-scoped variable 'x' used before its declaration. | ||
tests/cases/compiler/recursiveLetConst.ts(3,12): error TS2448: Block-scoped variable 'x1' used before its declaration. | ||
tests/cases/compiler/recursiveLetConst.ts(4,11): error TS2448: Block-scoped variable 'y' used before its declaration. | ||
tests/cases/compiler/recursiveLetConst.ts(5,14): error TS2448: Block-scoped variable 'y1' used before its declaration. | ||
tests/cases/compiler/recursiveLetConst.ts(6,14): error TS2448: Block-scoped variable 'v' used before its declaration. | ||
tests/cases/compiler/recursiveLetConst.ts(7,16): error TS2448: Block-scoped variable 'v' used before its declaration. | ||
tests/cases/compiler/recursiveLetConst.ts(8,15): error TS2448: Block-scoped variable 'v' used before its declaration. | ||
tests/cases/compiler/recursiveLetConst.ts(9,15): error TS2448: Block-scoped variable 'v' used before its declaration. | ||
tests/cases/compiler/recursiveLetConst.ts(10,17): error TS2448: Block-scoped variable 'v' used before its declaration. | ||
tests/cases/compiler/recursiveLetConst.ts(11,11): error TS2448: Block-scoped variable 'x2' used before its declaration. | ||
|
||
|
||
==== tests/cases/compiler/recursiveLetConst.ts (10 errors) ==== | ||
'use strict' | ||
let x = x + 1; | ||
~ | ||
!!! error TS2448: Block-scoped variable 'x' used before its declaration. | ||
let [x1] = x1 + 1; | ||
~~ | ||
!!! error TS2448: Block-scoped variable 'x1' used before its declaration. | ||
const y = y + 2; | ||
~ | ||
!!! error TS2448: Block-scoped variable 'y' used before its declaration. | ||
const [y1] = y1 + 1; | ||
~~ | ||
!!! error TS2448: Block-scoped variable 'y1' used before its declaration. | ||
for (let v = v; ; ) { } | ||
~ | ||
!!! error TS2448: Block-scoped variable 'v' used before its declaration. | ||
for (let [v] = v; ;) { } | ||
~ | ||
!!! error TS2448: Block-scoped variable 'v' used before its declaration. | ||
for (let v in v) { } | ||
~ | ||
!!! error TS2448: Block-scoped variable 'v' used before its declaration. | ||
for (let v of v) { } | ||
~ | ||
!!! error TS2448: Block-scoped variable 'v' used before its declaration. | ||
for (let [v] of v) { } | ||
~ | ||
!!! error TS2448: Block-scoped variable 'v' used before its declaration. | ||
let [x2 = x2] = [] | ||
~~ | ||
!!! error TS2448: Block-scoped variable 'x2' used before its declaration. | ||
let z0 = () => z0; | ||
let z1 = function () { return z1; } | ||
let z2 = { f() { return z2;}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
//// [recursiveLetConst.ts] | ||
'use strict' | ||
let x = x + 1; | ||
let [x1] = x1 + 1; | ||
const y = y + 2; | ||
const [y1] = y1 + 1; | ||
for (let v = v; ; ) { } | ||
for (let [v] = v; ;) { } | ||
for (let v in v) { } | ||
for (let v of v) { } | ||
for (let [v] of v) { } | ||
let [x2 = x2] = [] | ||
let z0 = () => z0; | ||
let z1 = function () { return z1; } | ||
let z2 = { f() { return z2;}} | ||
|
||
//// [recursiveLetConst.js] | ||
'use strict'; | ||
let x = x + 1; | ||
let [x1] = x1 + 1; | ||
const y = y + 2; | ||
const [y1] = y1 + 1; | ||
for (let v = v;;) { | ||
} | ||
for (let [v] = v;;) { | ||
} | ||
for (let v in v) { | ||
} | ||
for (let v of v) { | ||
} | ||
for (let [v] of v) { | ||
} | ||
let [x2 = x2] = []; | ||
let z0 = () => z0; | ||
let z1 = function () { | ||
return z1; | ||
}; | ||
let z2 = { | ||
f() { | ||
return z2; | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// @target:es6 | ||
'use strict' | ||
let x = x + 1; | ||
let [x1] = x1 + 1; | ||
const y = y + 2; | ||
const [y1] = y1 + 1; | ||
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. What about: This should be fine. 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. good catch, thanks 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. How about That is a false negative case: it generates no TS error, but it does crash at runtime. That false negative is expected, and therefore it is important to test that it is indeed happening as expected. |
||
for (let v = v; ; ) { } | ||
for (let [v] = v; ;) { } | ||
for (let v in v) { } | ||
for (let v of v) { } | ||
for (let [v] of v) { } | ||
let [x2 = x2] = [] | ||
let z0 = () => z0; | ||
let z1 = function () { return z1; } | ||
let z2 = { f() { return z2;}} |
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.
Can you move everything guarded by
if (nameNotFoundMessage)
into another function?