From a9df539b7e2ced08f43c848913ae862ef900b16e Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 12 Feb 2015 16:37:07 -0800 Subject: [PATCH] added check that var and let\const cannot share scope, added check that var is not shadowed by the let\const from the inner scope --- src/compiler/binder.ts | 10 ++ src/compiler/checker.ts | 51 ++++++-- .../diagnosticInformationMap.generated.ts | 1 + src/compiler/diagnosticMessages.json | 4 + ...arationShadowedByVarDeclaration.errors.txt | 12 +- .../letAndVarRedeclaration.errors.txt | 118 ++++++++++++++++++ .../reference/letAndVarRedeclaration.js | 102 +++++++++++++++ .../shadowingViaLocalValue.errors.txt | 30 +++++ .../reference/shadowingViaLocalValue.js | 31 +++++ .../cases/compiler/letAndVarRedeclaration.ts | 53 ++++++++ .../cases/compiler/shadowingViaLocalValue.ts | 14 +++ 11 files changed, 410 insertions(+), 16 deletions(-) create mode 100644 tests/baselines/reference/letAndVarRedeclaration.errors.txt create mode 100644 tests/baselines/reference/letAndVarRedeclaration.js create mode 100644 tests/baselines/reference/shadowingViaLocalValue.errors.txt create mode 100644 tests/baselines/reference/shadowingViaLocalValue.js create mode 100644 tests/cases/compiler/letAndVarRedeclaration.ts create mode 100644 tests/cases/compiler/shadowingViaLocalValue.ts diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index c1b9d76f1b5..be9e9a525c0 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -482,6 +482,16 @@ module ts { break; } case SyntaxKind.Block: + // do not treat function block a block-scope container + // all block-scope locals that reside in this block should go to the function locals. + // Otherwise this won't be considered as redeclaration of a block scoped local: + // function foo() { + // let x; + // var x; + // } + // 'var x' will be placed into the function locals and 'let x' - into the locals of the block + bindChildren(node, 0, /*isBlockScopeContainer*/ !isAnyFunction(node.parent)); + break; case SyntaxKind.CatchClause: case SyntaxKind.ForStatement: case SyntaxKind.ForInStatement: diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index cccd74e3907..91537818175 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -8198,19 +8198,26 @@ module ts { } } - function checkCollisionWithConstDeclarations(node: VariableLikeDeclaration) { + function checkVarDeclaredNamesNotShadowed(node: VariableDeclaration | BindingElement) { + // - ScriptBody : StatementList + // It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList + // also occurs in the VarDeclaredNames of StatementList. + + // - Block : { StatementList } + // It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList + // also occurs in the VarDeclaredNames of StatementList. + // Variable declarations are hoisted to the top of their function scope. They can shadow // block scoped declarations, which bind tighter. this will not be flagged as duplicate definition // by the binder as the declaration scope is different. // A non-initialized declaration is a no-op as the block declaration will resolve before the var // declaration. the problem is if the declaration has an initializer. this will act as a write to the // block declared value. this is fine for let, but not const. - // // Only consider declarations with initializers, uninitialized var declarations will not - // step on a const variable. + // step on a let\const variable. // Do not consider let and const declarations, as duplicate block-scoped declarations - // are handled by the binder. - // We are only looking for var declarations that step on const declarations from a + // are handled by the binder. + // We are only looking for var declarations that step on let\const declarations from a // different scope. e.g.: // var x = 0; // { @@ -8219,11 +8226,33 @@ module ts { // } if (node.initializer && (getCombinedNodeFlags(node) & NodeFlags.BlockScoped) === 0) { var symbol = getSymbolOfNode(node); - if (symbol.flags & SymbolFlags.FunctionScopedVariable) { + if (symbol.flags & (SymbolFlags.FunctionScopedVariable)) { var localDeclarationSymbol = resolveName(node, (node.name).text, SymbolFlags.Variable, /*nodeNotFoundErrorMessage*/ undefined, /*nameArg*/ undefined); - if (localDeclarationSymbol && localDeclarationSymbol !== symbol && localDeclarationSymbol.flags & SymbolFlags.BlockScopedVariable) { - if (getDeclarationFlagsFromSymbol(localDeclarationSymbol) & NodeFlags.Const) { - error(node, Diagnostics.Cannot_redeclare_block_scoped_variable_0, symbolToString(localDeclarationSymbol)); + if (localDeclarationSymbol && + localDeclarationSymbol !== symbol && + localDeclarationSymbol.flags & SymbolFlags.BlockScopedVariable) { + if (getDeclarationFlagsFromSymbol(localDeclarationSymbol) & (NodeFlags.Let | NodeFlags.Const)) { + // here we know that function scoped variable is shadowed by block scoped one + // if they are defined in the same scope - binder has already reported redeclaration error + // otherwise if variable has an initializer - show error that initialization will fail + // since LHS will be block scoped name instead of function scoped + + var localVarDeclList = getAncestor(localDeclarationSymbol.valueDeclaration, SyntaxKind.VariableDeclarationList); + var localContainer = + localVarDeclList.parent.kind === SyntaxKind.VariableStatement && + localVarDeclList.parent.parent; + + // if block scoped variable is defined in the function\module\source file scope + // then since function scoped variable is hoised their names will collide + var namesShareScope = + localContainer && + (localContainer.kind === SyntaxKind.Block && isAnyFunction(localContainer.parent) || + (localContainer.kind === SyntaxKind.ModuleBlock && localContainer.kind === SyntaxKind.ModuleDeclaration) || + localContainer.kind === SyntaxKind.SourceFile); + + if (!namesShareScope) { + error(getErrorSpanForNode(node), Diagnostics.Cannot_initialize_outer_scope_variable_0_when_having_block_scoped_variable_with_the_same_name, symbolToString(localDeclarationSymbol)); + } } } } @@ -8320,7 +8349,9 @@ module ts { if (node.kind !== SyntaxKind.PropertyDeclaration && node.kind !== SyntaxKind.PropertySignature) { // We know we don't have a binding pattern or computed name here checkExportsOnMergedDeclarations(node); - checkCollisionWithConstDeclarations(node); + if (node.kind === SyntaxKind.VariableDeclaration || node.kind === SyntaxKind.BindingElement) { + checkVarDeclaredNamesNotShadowed(node); + } checkCollisionWithCapturedSuperVariable(node, node.name); checkCollisionWithCapturedThisVariable(node, node.name); checkCollisionWithRequireExportsInGeneratedCode(node, node.name); diff --git a/src/compiler/diagnosticInformationMap.generated.ts b/src/compiler/diagnosticInformationMap.generated.ts index 6c79578e41c..06ee032a498 100644 --- a/src/compiler/diagnosticInformationMap.generated.ts +++ b/src/compiler/diagnosticInformationMap.generated.ts @@ -381,6 +381,7 @@ module ts { 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'." }, Property_0_does_not_exist_on_const_enum_1: { code: 4088, category: DiagnosticCategory.Error, key: "Property '{0}' does not exist on 'const' enum '{1}'." }, 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." }, + Cannot_initialize_outer_scope_variable_0_when_having_block_scoped_variable_with_the_same_name: { code: 4090, category: DiagnosticCategory.Error, key: "Cannot initialize outer scope variable '{0}' when having block-scoped variable with the same name." }, The_current_host_does_not_support_the_0_option: { code: 5001, category: DiagnosticCategory.Error, key: "The current host does not support the '{0}' option." }, 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." }, Cannot_read_file_0_Colon_1: { code: 5012, category: DiagnosticCategory.Error, key: "Cannot read file '{0}': {1}" }, diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 8a5d8d80427..efe2a63f911 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -1517,6 +1517,10 @@ "category": "Error", "code": 4089 }, + "Cannot initialize outer scope variable '{0}' when having block-scoped variable with the same name.": { + "category": "Error", + "code": 4090 + }, "The current host does not support the '{0}' option.": { "category": "Error", "code": 5001 diff --git a/tests/baselines/reference/constDeclarationShadowedByVarDeclaration.errors.txt b/tests/baselines/reference/constDeclarationShadowedByVarDeclaration.errors.txt index 629cf7f75af..529964a6a85 100644 --- a/tests/baselines/reference/constDeclarationShadowedByVarDeclaration.errors.txt +++ b/tests/baselines/reference/constDeclarationShadowedByVarDeclaration.errors.txt @@ -1,6 +1,6 @@ -tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(7,9): error TS2451: Cannot redeclare block-scoped variable 'x'. -tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(15,13): error TS2451: Cannot redeclare block-scoped variable 'y'. -tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(22,7): error TS2451: Cannot redeclare block-scoped variable 'z'. +tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(7,9): error TS4090: Cannot initialize outer scope variable 'x' when having block-scoped variable with the same name. +tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(15,13): error TS4090: Cannot initialize outer scope variable 'y' when having block-scoped variable with the same name. +tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(22,7): error TS4090: Cannot initialize outer scope variable 'z' when having block-scoped variable with the same name. ==== tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts (3 errors) ==== @@ -12,7 +12,7 @@ tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(22,7): error TS var x = 0; ~ -!!! error TS2451: Cannot redeclare block-scoped variable 'x'. +!!! error TS4090: Cannot initialize outer scope variable 'x' when having block-scoped variable with the same name. } @@ -22,7 +22,7 @@ tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(22,7): error TS { var y = 0; ~ -!!! error TS2451: Cannot redeclare block-scoped variable 'y'. +!!! error TS4090: Cannot initialize outer scope variable 'y' when having block-scoped variable with the same name. } } @@ -31,5 +31,5 @@ tests/cases/compiler/constDeclarationShadowedByVarDeclaration.ts(22,7): error TS const z = 0; var z = 0 ~ -!!! error TS2451: Cannot redeclare block-scoped variable 'z'. +!!! error TS4090: Cannot initialize outer scope variable 'z' when having block-scoped variable with the same name. } \ No newline at end of file diff --git a/tests/baselines/reference/letAndVarRedeclaration.errors.txt b/tests/baselines/reference/letAndVarRedeclaration.errors.txt new file mode 100644 index 00000000000..48704f61040 --- /dev/null +++ b/tests/baselines/reference/letAndVarRedeclaration.errors.txt @@ -0,0 +1,118 @@ +tests/cases/compiler/letAndVarRedeclaration.ts(2,5): error TS2451: Cannot redeclare block-scoped variable 'e0'. +tests/cases/compiler/letAndVarRedeclaration.ts(3,5): error TS2451: Cannot redeclare block-scoped variable 'e0'. +tests/cases/compiler/letAndVarRedeclaration.ts(4,10): error TS2451: Cannot redeclare block-scoped variable 'e0'. +tests/cases/compiler/letAndVarRedeclaration.ts(7,9): error TS2451: Cannot redeclare block-scoped variable 'x1'. +tests/cases/compiler/letAndVarRedeclaration.ts(8,9): error TS2451: Cannot redeclare block-scoped variable 'x1'. +tests/cases/compiler/letAndVarRedeclaration.ts(9,14): error TS2451: Cannot redeclare block-scoped variable 'x1'. +tests/cases/compiler/letAndVarRedeclaration.ts(13,9): error TS2451: Cannot redeclare block-scoped variable 'x'. +tests/cases/compiler/letAndVarRedeclaration.ts(15,13): error TS2451: Cannot redeclare block-scoped variable 'x'. +tests/cases/compiler/letAndVarRedeclaration.ts(18,18): error TS2451: Cannot redeclare block-scoped variable 'x'. +tests/cases/compiler/letAndVarRedeclaration.ts(23,9): error TS2451: Cannot redeclare block-scoped variable 'x2'. +tests/cases/compiler/letAndVarRedeclaration.ts(24,9): error TS2451: Cannot redeclare block-scoped variable 'x2'. +tests/cases/compiler/letAndVarRedeclaration.ts(25,14): error TS2451: Cannot redeclare block-scoped variable 'x2'. +tests/cases/compiler/letAndVarRedeclaration.ts(29,9): error TS2451: Cannot redeclare block-scoped variable 'x2'. +tests/cases/compiler/letAndVarRedeclaration.ts(31,13): error TS2451: Cannot redeclare block-scoped variable 'x2'. +tests/cases/compiler/letAndVarRedeclaration.ts(34,18): error TS2451: Cannot redeclare block-scoped variable 'x2'. +tests/cases/compiler/letAndVarRedeclaration.ts(38,5): error TS2451: Cannot redeclare block-scoped variable 'x11'. +tests/cases/compiler/letAndVarRedeclaration.ts(39,10): error TS2451: Cannot redeclare block-scoped variable 'x11'. +tests/cases/compiler/letAndVarRedeclaration.ts(43,9): error TS2451: Cannot redeclare block-scoped variable 'x11'. +tests/cases/compiler/letAndVarRedeclaration.ts(44,14): error TS2451: Cannot redeclare block-scoped variable 'x11'. +tests/cases/compiler/letAndVarRedeclaration.ts(49,9): error TS2451: Cannot redeclare block-scoped variable 'x11'. +tests/cases/compiler/letAndVarRedeclaration.ts(50,14): error TS2451: Cannot redeclare block-scoped variable 'x11'. + + +==== tests/cases/compiler/letAndVarRedeclaration.ts (21 errors) ==== + + let e0 + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'e0'. + var e0; + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'e0'. + function e0() { } + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'e0'. + + function f0() { + let x1; + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x1'. + var x1; + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x1'. + function x1() { } + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x1'. + } + + function f1() { + let x; + ~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x'. + { + var x; + ~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x'. + } + { + function x() { } + ~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x'. + } + } + + module M0 { + let x2; + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x2'. + var x2; + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x2'. + function x2() { } + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x2'. + } + + module M1 { + let x2; + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x2'. + { + var x2; + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x2'. + } + { + function x2() { } + ~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x2'. + } + } + + let x11; + ~~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x11'. + for (var x11; ;) { + ~~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x11'. + } + + function f2() { + let x11; + ~~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x11'. + for (var x11; ;) { + ~~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x11'. + } + } + + module M2 { + let x11; + ~~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x11'. + for (var x11; ;) { + ~~~ +!!! error TS2451: Cannot redeclare block-scoped variable 'x11'. + } + } \ No newline at end of file diff --git a/tests/baselines/reference/letAndVarRedeclaration.js b/tests/baselines/reference/letAndVarRedeclaration.js new file mode 100644 index 00000000000..fd4bfe67cac --- /dev/null +++ b/tests/baselines/reference/letAndVarRedeclaration.js @@ -0,0 +1,102 @@ +//// [letAndVarRedeclaration.ts] + +let e0 +var e0; +function e0() { } + +function f0() { + let x1; + var x1; + function x1() { } +} + +function f1() { + let x; + { + var x; + } + { + function x() { } + } +} + +module M0 { + let x2; + var x2; + function x2() { } +} + +module M1 { + let x2; + { + var x2; + } + { + function x2() { } + } +} + +let x11; +for (var x11; ;) { +} + +function f2() { + let x11; + for (var x11; ;) { + } +} + +module M2 { + let x11; + for (var x11; ;) { + } +} + +//// [letAndVarRedeclaration.js] +let e0; +var e0; +function e0() { } +function f0() { + let x1; + var x1; + function x1() { } +} +function f1() { + let x; + { + var x; + } + { + function x() { } + } +} +var M0; +(function (M0) { + let x2; + var x2; + function x2() { } +})(M0 || (M0 = {})); +var M1; +(function (M1) { + let x2; + { + var x2; + } + { + function x2() { } + } +})(M1 || (M1 = {})); +let x11; +for (var x11;;) { +} +function f2() { + let x11; + for (var x11;;) { + } +} +var M2; +(function (M2) { + let x11; + for (var x11;;) { + } +})(M2 || (M2 = {})); diff --git a/tests/baselines/reference/shadowingViaLocalValue.errors.txt b/tests/baselines/reference/shadowingViaLocalValue.errors.txt new file mode 100644 index 00000000000..a4a8d9a05d2 --- /dev/null +++ b/tests/baselines/reference/shadowingViaLocalValue.errors.txt @@ -0,0 +1,30 @@ +tests/cases/compiler/shadowingViaLocalValue.ts(2,5): error TS1153: 'let' declarations are only available when targeting ECMAScript 6 and higher. +tests/cases/compiler/shadowingViaLocalValue.ts(4,13): error TS4090: Cannot initialize outer scope variable 'x' when having block-scoped variable with the same name. +tests/cases/compiler/shadowingViaLocalValue.ts(9,5): error TS1153: 'let' declarations are only available when targeting ECMAScript 6 and higher. +tests/cases/compiler/shadowingViaLocalValue.ts(11,18): error TS4090: Cannot initialize outer scope variable 'x1' when having block-scoped variable with the same name. + + +==== tests/cases/compiler/shadowingViaLocalValue.ts (4 errors) ==== + { + let x; + ~~~ +!!! error TS1153: 'let' declarations are only available when targeting ECMAScript 6 and higher. + { + var x = 1; + ~ +!!! error TS4090: Cannot initialize outer scope variable 'x' when having block-scoped variable with the same name. + } + } + + { + let x1; + ~~~ +!!! error TS1153: 'let' declarations are only available when targeting ECMAScript 6 and higher. + { + for (var x1 = 0; ;); + ~~ +!!! error TS4090: Cannot initialize outer scope variable 'x1' when having block-scoped variable with the same name. + } + } + + \ No newline at end of file diff --git a/tests/baselines/reference/shadowingViaLocalValue.js b/tests/baselines/reference/shadowingViaLocalValue.js new file mode 100644 index 00000000000..f99a209f0de --- /dev/null +++ b/tests/baselines/reference/shadowingViaLocalValue.js @@ -0,0 +1,31 @@ +//// [shadowingViaLocalValue.ts] +{ + let x; + { + var x = 1; + } +} + +{ + let x1; + { + for (var x1 = 0; ;); + } +} + + + +//// [shadowingViaLocalValue.js] +{ + let x; + { + var x = 1; + } +} +{ + let x1; + { + for (var x1 = 0;;) + ; + } +} diff --git a/tests/cases/compiler/letAndVarRedeclaration.ts b/tests/cases/compiler/letAndVarRedeclaration.ts new file mode 100644 index 00000000000..1f901ded520 --- /dev/null +++ b/tests/cases/compiler/letAndVarRedeclaration.ts @@ -0,0 +1,53 @@ +// @target: es6 + +let e0 +var e0; +function e0() { } + +function f0() { + let x1; + var x1; + function x1() { } +} + +function f1() { + let x; + { + var x; + } + { + function x() { } + } +} + +module M0 { + let x2; + var x2; + function x2() { } +} + +module M1 { + let x2; + { + var x2; + } + { + function x2() { } + } +} + +let x11; +for (var x11; ;) { +} + +function f2() { + let x11; + for (var x11; ;) { + } +} + +module M2 { + let x11; + for (var x11; ;) { + } +} \ No newline at end of file diff --git a/tests/cases/compiler/shadowingViaLocalValue.ts b/tests/cases/compiler/shadowingViaLocalValue.ts new file mode 100644 index 00000000000..72bc0f0e33e --- /dev/null +++ b/tests/cases/compiler/shadowingViaLocalValue.ts @@ -0,0 +1,14 @@ +{ + let x; + { + var x = 1; + } +} + +{ + let x1; + { + for (var x1 = 0; ;); + } +} +