Skip to content

Commit 9f7c252

Browse files
vladimavladima
committed
Merge pull request #2027 from Microsoft/redefinedLetConst
Disallow let\const declarations in the same scope with var declarations.
2 parents 5047d2d + 941728d commit 9f7c252

11 files changed

+411
-17
lines changed

‎src/compiler/binder.ts

+10
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,16 @@ module ts {
482482
break;
483483
}
484484
case SyntaxKind.Block:
485+
// do not treat function block a block-scope container
486+
// all block-scope locals that reside in this block should go to the function locals.
487+
// Otherwise this won't be considered as redeclaration of a block scoped local:
488+
// function foo() {
489+
// let x;
490+
// var x;
491+
// }
492+
// 'var x' will be placed into the function locals and 'let x' - into the locals of the block
493+
bindChildren(node, 0, /*isBlockScopeContainer*/ !isAnyFunction(node.parent));
494+
break;
485495
case SyntaxKind.CatchClause:
486496
case SyntaxKind.ForStatement:
487497
case SyntaxKind.ForInStatement:

‎src/compiler/checker.ts

+42-11
Original file line numberDiff line numberDiff line change
@@ -8198,32 +8198,61 @@ module ts {
81988198
}
81998199
}
82008200

8201-
function checkCollisionWithConstDeclarations(node: VariableLikeDeclaration) {
8201+
function checkVarDeclaredNamesNotShadowed(node: VariableDeclaration | BindingElement) {
8202+
// - ScriptBody : StatementList
8203+
// It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList
8204+
// also occurs in the VarDeclaredNames of StatementList.
8205+
8206+
// - Block : { StatementList }
8207+
// It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList
8208+
// also occurs in the VarDeclaredNames of StatementList.
8209+
82028210
// Variable declarations are hoisted to the top of their function scope. They can shadow
82038211
// block scoped declarations, which bind tighter. this will not be flagged as duplicate definition
82048212
// by the binder as the declaration scope is different.
82058213
// A non-initialized declaration is a no-op as the block declaration will resolve before the var
82068214
// declaration. the problem is if the declaration has an initializer. this will act as a write to the
82078215
// block declared value. this is fine for let, but not const.
8208-
//
82098216
// Only consider declarations with initializers, uninitialized var declarations will not
8210-
// step on a const variable.
8217+
// step on a let/const variable.
82118218
// Do not consider let and const declarations, as duplicate block-scoped declarations
82128219
// are handled by the binder.
8213-
// We are only looking for var declarations that step on const declarations from a
8220+
// We are only looking for var declarations that step on let\const declarations from a
82148221
// different scope. e.g.:
8215-
// var x = 0;
82168222
// {
8217-
// const x = 0;
8218-
// var x = 0;
8223+
// const x = 0; // localDeclarationSymbol obtained after name resolution will correspond to this declaration
8224+
// var x = 0; // symbol for this declaration will be 'symbol'
82198225
// }
82208226
if (node.initializer && (getCombinedNodeFlags(node) & NodeFlags.BlockScoped) === 0) {
82218227
var symbol = getSymbolOfNode(node);
82228228
if (symbol.flags & SymbolFlags.FunctionScopedVariable) {
82238229
var localDeclarationSymbol = resolveName(node, (<Identifier>node.name).text, SymbolFlags.Variable, /*nodeNotFoundErrorMessage*/ undefined, /*nameArg*/ undefined);
8224-
if (localDeclarationSymbol && localDeclarationSymbol !== symbol && localDeclarationSymbol.flags & SymbolFlags.BlockScopedVariable) {
8225-
if (getDeclarationFlagsFromSymbol(localDeclarationSymbol) & NodeFlags.Const) {
8226-
error(node, Diagnostics.Cannot_redeclare_block_scoped_variable_0, symbolToString(localDeclarationSymbol));
8230+
if (localDeclarationSymbol &&
8231+
localDeclarationSymbol !== symbol &&
8232+
localDeclarationSymbol.flags & SymbolFlags.BlockScopedVariable) {
8233+
if (getDeclarationFlagsFromSymbol(localDeclarationSymbol) & NodeFlags.BlockScoped) {
8234+
8235+
var varDeclList = getAncestor(localDeclarationSymbol.valueDeclaration, SyntaxKind.VariableDeclarationList);
8236+
var container =
8237+
varDeclList.parent.kind === SyntaxKind.VariableStatement &&
8238+
varDeclList.parent.parent;
8239+
8240+
// names of block-scoped and function scoped variables can collide only
8241+
// if block scoped variable is defined in the function\module\source file scope (because of variable hoisting)
8242+
var namesShareScope =
8243+
container &&
8244+
(container.kind === SyntaxKind.Block && isAnyFunction(container.parent) ||
8245+
(container.kind === SyntaxKind.ModuleBlock && container.kind === SyntaxKind.ModuleDeclaration) ||
8246+
container.kind === SyntaxKind.SourceFile);
8247+
8248+
// here we know that function scoped variable is shadowed by block scoped one
8249+
// if they are defined in the same scope - binder has already reported redeclaration error
8250+
// otherwise if variable has an initializer - show error that initialization will fail
8251+
// since LHS will be block scoped name instead of function scoped
8252+
if (!namesShareScope) {
8253+
var name = symbolToString(localDeclarationSymbol);
8254+
error(getErrorSpanForNode(node), Diagnostics.Cannot_initialize_outer_scoped_variable_0_in_the_same_scope_as_block_scoped_declaration_1, name, name);
8255+
}
82278256
}
82288257
}
82298258
}
@@ -8320,7 +8349,9 @@ module ts {
83208349
if (node.kind !== SyntaxKind.PropertyDeclaration && node.kind !== SyntaxKind.PropertySignature) {
83218350
// We know we don't have a binding pattern or computed name here
83228351
checkExportsOnMergedDeclarations(node);
8323-
checkCollisionWithConstDeclarations(node);
8352+
if (node.kind === SyntaxKind.VariableDeclaration || node.kind === SyntaxKind.BindingElement) {
8353+
checkVarDeclaredNamesNotShadowed(<VariableDeclaration | BindingElement>node);
8354+
}
83248355
checkCollisionWithCapturedSuperVariable(node, <Identifier>node.name);
83258356
checkCollisionWithCapturedThisVariable(node, <Identifier>node.name);
83268357
checkCollisionWithRequireExportsInGeneratedCode(node, <Identifier>node.name);

‎src/compiler/diagnosticInformationMap.generated.ts

+1
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ module ts {
381381
const_enum_member_initializer_was_evaluated_to_disallowed_value_NaN: { code: 4087, category: DiagnosticCategory.Error, key: "'const' enum member initializer was evaluated to disallowed value 'NaN'." },
382382
Property_0_does_not_exist_on_const_enum_1: { code: 4088, category: DiagnosticCategory.Error, key: "Property '{0}' does not exist on 'const' enum '{1}'." },
383383
let_is_not_allowed_to_be_used_as_a_name_in_let_or_const_declarations: { code: 4089, category: DiagnosticCategory.Error, key: "'let' is not allowed to be used as a name in 'let' or 'const' declarations." },
384+
Cannot_initialize_outer_scoped_variable_0_in_the_same_scope_as_block_scoped_declaration_1: { code: 4090, category: DiagnosticCategory.Error, key: "Cannot initialize outer scoped variable '{0}' in the same scope as block scoped declaration '{1}'." },
384385
The_current_host_does_not_support_the_0_option: { code: 5001, category: DiagnosticCategory.Error, key: "The current host does not support the '{0}' option." },
385386
Cannot_find_the_common_subdirectory_path_for_the_input_files: { code: 5009, category: DiagnosticCategory.Error, key: "Cannot find the common subdirectory path for the input files." },
386387
Cannot_read_file_0_Colon_1: { code: 5012, category: DiagnosticCategory.Error, key: "Cannot read file '{0}': {1}" },

‎src/compiler/diagnosticMessages.json

+4
Original file line numberDiff line numberDiff line change
@@ -1517,6 +1517,10 @@
15171517
"category": "Error",
15181518
"code": 4089
15191519
},
1520+
"Cannot initialize outer scoped variable '{0}' in the same scope as block scoped declaration '{1}'.": {
1521+
"category": "Error",
1522+
"code": 4090
1523+
},
15201524
"The current host does not support the '{0}' option.": {
15211525
"category": "Error",
15221526
"code": 5001

‎tests/baselines/reference/constDeclarationShadowedByVarDeclaration.errors.txt

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(7,9): error TS2451: Cannot redeclare block-scoped variable 'x'.
2-
tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(15,13): error TS2451: Cannot redeclare block-scoped variable 'y'.
3-
tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(22,7): error TS2451: Cannot redeclare block-scoped variable 'z'.
1+
tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(7,9): error TS4090: Cannot initialize outer scoped variable 'x' in the same scope as block scoped declaration 'x'.
2+
tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(15,13): error TS4090: Cannot initialize outer scoped variable 'y' in the same scope as block scoped declaration 'y'.
3+
tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(22,7): error TS4090: Cannot initialize outer scoped variable 'z' in the same scope as block scoped declaration 'z'.
44

55

66
==== tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts (3 errors) ====
@@ -12,7 +12,7 @@ tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(22,7): error TS
1212

1313
var x = 0;
1414
~
15-
!!! error TS2451: Cannot redeclare block-scoped variable 'x'.
15+
!!! error TS4090: Cannot initialize outer scoped variable 'x' in the same scope as block scoped declaration 'x'.
1616
}
1717

1818

@@ -22,7 +22,7 @@ tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(22,7): error TS
2222
{
2323
var y = 0;
2424
~
25-
!!! error TS2451: Cannot redeclare block-scoped variable 'y'.
25+
!!! error TS4090: Cannot initialize outer scoped variable 'y' in the same scope as block scoped declaration 'y'.
2626
}
2727
}
2828

@@ -31,5 +31,5 @@ tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(22,7): error TS
3131
const z = 0;
3232
var z = 0
3333
~
34-
!!! error TS2451: Cannot redeclare block-scoped variable 'z'.
34+
!!! error TS4090: Cannot initialize outer scoped variable 'z' in the same scope as block scoped declaration 'z'.
3535
}

‎tests/baselines/reference/letAndVarRedeclaration.errors.txt

+118
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
tests/cases/compiler/letAndVarRedeclaration.ts(2,5): error TS2451: Cannot redeclare block-scoped variable 'e0'.
2+
tests/cases/compiler/letAndVarRedeclaration.ts(3,5): error TS2451: Cannot redeclare block-scoped variable 'e0'.
3+
tests/cases/compiler/letAndVarRedeclaration.ts(4,10): error TS2451: Cannot redeclare block-scoped variable 'e0'.
4+
tests/cases/compiler/letAndVarRedeclaration.ts(7,9): error TS2451: Cannot redeclare block-scoped variable 'x1'.
5+
tests/cases/compiler/letAndVarRedeclaration.ts(8,9): error TS2451: Cannot redeclare block-scoped variable 'x1'.
6+
tests/cases/compiler/letAndVarRedeclaration.ts(9,14): error TS2451: Cannot redeclare block-scoped variable 'x1'.
7+
tests/cases/compiler/letAndVarRedeclaration.ts(13,9): error TS2451: Cannot redeclare block-scoped variable 'x'.
8+
tests/cases/compiler/letAndVarRedeclaration.ts(15,13): error TS2451: Cannot redeclare block-scoped variable 'x'.
9+
tests/cases/compiler/letAndVarRedeclaration.ts(18,18): error TS2451: Cannot redeclare block-scoped variable 'x'.
10+
tests/cases/compiler/letAndVarRedeclaration.ts(23,9): error TS2451: Cannot redeclare block-scoped variable 'x2'.
11+
tests/cases/compiler/letAndVarRedeclaration.ts(24,9): error TS2451: Cannot redeclare block-scoped variable 'x2'.
12+
tests/cases/compiler/letAndVarRedeclaration.ts(25,14): error TS2451: Cannot redeclare block-scoped variable 'x2'.
13+
tests/cases/compiler/letAndVarRedeclaration.ts(29,9): error TS2451: Cannot redeclare block-scoped variable 'x2'.
14+
tests/cases/compiler/letAndVarRedeclaration.ts(31,13): error TS2451: Cannot redeclare block-scoped variable 'x2'.
15+
tests/cases/compiler/letAndVarRedeclaration.ts(34,18): error TS2451: Cannot redeclare block-scoped variable 'x2'.
16+
tests/cases/compiler/letAndVarRedeclaration.ts(38,5): error TS2451: Cannot redeclare block-scoped variable 'x11'.
17+
tests/cases/compiler/letAndVarRedeclaration.ts(39,10): error TS2451: Cannot redeclare block-scoped variable 'x11'.
18+
tests/cases/compiler/letAndVarRedeclaration.ts(43,9): error TS2451: Cannot redeclare block-scoped variable 'x11'.
19+
tests/cases/compiler/letAndVarRedeclaration.ts(44,14): error TS2451: Cannot redeclare block-scoped variable 'x11'.
20+
tests/cases/compiler/letAndVarRedeclaration.ts(49,9): error TS2451: Cannot redeclare block-scoped variable 'x11'.
21+
tests/cases/compiler/letAndVarRedeclaration.ts(50,14): error TS2451: Cannot redeclare block-scoped variable 'x11'.
22+
23+
24+
==== tests/cases/compiler/letAndVarRedeclaration.ts (21 errors) ====
25+
26+
let e0
27+
~~
28+
!!! error TS2451: Cannot redeclare block-scoped variable 'e0'.
29+
var e0;
30+
~~
31+
!!! error TS2451: Cannot redeclare block-scoped variable 'e0'.
32+
function e0() { }
33+
~~
34+
!!! error TS2451: Cannot redeclare block-scoped variable 'e0'.
35+
36+
function f0() {
37+
let x1;
38+
~~
39+
!!! error TS2451: Cannot redeclare block-scoped variable 'x1'.
40+
var x1;
41+
~~
42+
!!! error TS2451: Cannot redeclare block-scoped variable 'x1'.
43+
function x1() { }
44+
~~
45+
!!! error TS2451: Cannot redeclare block-scoped variable 'x1'.
46+
}
47+
48+
function f1() {
49+
let x;
50+
~
51+
!!! error TS2451: Cannot redeclare block-scoped variable 'x'.
52+
{
53+
var x;
54+
~
55+
!!! error TS2451: Cannot redeclare block-scoped variable 'x'.
56+
}
57+
{
58+
function x() { }
59+
~
60+
!!! error TS2451: Cannot redeclare block-scoped variable 'x'.
61+
}
62+
}
63+
64+
module M0 {
65+
let x2;
66+
~~
67+
!!! error TS2451: Cannot redeclare block-scoped variable 'x2'.
68+
var x2;
69+
~~
70+
!!! error TS2451: Cannot redeclare block-scoped variable 'x2'.
71+
function x2() { }
72+
~~
73+
!!! error TS2451: Cannot redeclare block-scoped variable 'x2'.
74+
}
75+
76+
module M1 {
77+
let x2;
78+
~~
79+
!!! error TS2451: Cannot redeclare block-scoped variable 'x2'.
80+
{
81+
var x2;
82+
~~
83+
!!! error TS2451: Cannot redeclare block-scoped variable 'x2'.
84+
}
85+
{
86+
function x2() { }
87+
~~
88+
!!! error TS2451: Cannot redeclare block-scoped variable 'x2'.
89+
}
90+
}
91+
92+
let x11;
93+
~~~
94+
!!! error TS2451: Cannot redeclare block-scoped variable 'x11'.
95+
for (var x11; ;) {
96+
~~~
97+
!!! error TS2451: Cannot redeclare block-scoped variable 'x11'.
98+
}
99+
100+
function f2() {
101+
let x11;
102+
~~~
103+
!!! error TS2451: Cannot redeclare block-scoped variable 'x11'.
104+
for (var x11; ;) {
105+
~~~
106+
!!! error TS2451: Cannot redeclare block-scoped variable 'x11'.
107+
}
108+
}
109+
110+
module M2 {
111+
let x11;
112+
~~~
113+
!!! error TS2451: Cannot redeclare block-scoped variable 'x11'.
114+
for (var x11; ;) {
115+
~~~
116+
!!! error TS2451: Cannot redeclare block-scoped variable 'x11'.
117+
}
118+
}

‎tests/baselines/reference/letAndVarRedeclaration.js

+102
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
//// [letAndVarRedeclaration.ts]
2+
3+
let e0
4+
var e0;
5+
function e0() { }
6+
7+
function f0() {
8+
let x1;
9+
var x1;
10+
function x1() { }
11+
}
12+
13+
function f1() {
14+
let x;
15+
{
16+
var x;
17+
}
18+
{
19+
function x() { }
20+
}
21+
}
22+
23+
module M0 {
24+
let x2;
25+
var x2;
26+
function x2() { }
27+
}
28+
29+
module M1 {
30+
let x2;
31+
{
32+
var x2;
33+
}
34+
{
35+
function x2() { }
36+
}
37+
}
38+
39+
let x11;
40+
for (var x11; ;) {
41+
}
42+
43+
function f2() {
44+
let x11;
45+
for (var x11; ;) {
46+
}
47+
}
48+
49+
module M2 {
50+
let x11;
51+
for (var x11; ;) {
52+
}
53+
}
54+
55+
//// [letAndVarRedeclaration.js]
56+
let e0;
57+
var e0;
58+
function e0() { }
59+
function f0() {
60+
let x1;
61+
var x1;
62+
function x1() { }
63+
}
64+
function f1() {
65+
let x;
66+
{
67+
var x;
68+
}
69+
{
70+
function x() { }
71+
}
72+
}
73+
var M0;
74+
(function (M0) {
75+
let x2;
76+
var x2;
77+
function x2() { }
78+
})(M0 || (M0 = {}));
79+
var M1;
80+
(function (M1) {
81+
let x2;
82+
{
83+
var x2;
84+
}
85+
{
86+
function x2() { }
87+
}
88+
})(M1 || (M1 = {}));
89+
let x11;
90+
for (var x11;;) {
91+
}
92+
function f2() {
93+
let x11;
94+
for (var x11;;) {
95+
}
96+
}
97+
var M2;
98+
(function (M2) {
99+
let x11;
100+
for (var x11;;) {
101+
}
102+
})(M2 || (M2 = {}));

0 commit comments

Comments
 (0)