Skip to content

Detect usage of block-scoped variables from earlier case blocks #47160

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25248,6 +25248,28 @@ namespace ts {
if (isCaptured) {
getNodeLinks(symbol.valueDeclaration).flags |= NodeCheckFlags.CapturedBlockScopedBinding;
}

// Check for usage of a variable from a case clause prior to the referencing one
// switch(n) {
// case 0:
// let x = 0;
// case 1:
// x; // <- bad
// if (true) x; // <- bad (note: need to walk to parent)
// }
if (container.kind === SyntaxKind.CaseBlock) {
let usageCaseClause = getAncestor(node, SyntaxKind.CaseClause);
if (usageCaseClause) {
// Walk up until we're at the same level as the declaring block
while (usageCaseClause.parent !== container) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a "'0' is declared here" related span to the valueDeclaration.

usageCaseClause = usageCaseClause.parent;
}

if (usageCaseClause.pos > symbol.valueDeclaration.pos) {
error(node, Diagnostics.Variable_0_is_declared_in_a_prior_case_block, symbolToString(symbol));
}
}
}
}

function isBindingCapturedByNode(node: Node, decl: VariableDeclaration | BindingElement) {
Expand Down Expand Up @@ -26107,6 +26129,7 @@ namespace ts {
// Don't do this for assignment declarations unless there is a type tag on the assignment, to avoid circularity from checking the right operand.
function getContextualTypeForAssignmentDeclaration(binaryExpression: BinaryExpression): Type | undefined {
const kind = getAssignmentDeclarationKind(binaryExpression);
let valueDeclaration;
switch (kind) {
case AssignmentDeclarationKind.None:
case AssignmentDeclarationKind.ThisProperty:
Expand Down Expand Up @@ -26161,7 +26184,7 @@ namespace ts {
case AssignmentDeclarationKind.ExportsProperty:
case AssignmentDeclarationKind.Prototype:
case AssignmentDeclarationKind.PrototypeProperty:
let valueDeclaration = binaryExpression.left.symbol?.valueDeclaration;
valueDeclaration = binaryExpression.left.symbol?.valueDeclaration;
// falls through
case AssignmentDeclarationKind.ModuleExports:
valueDeclaration ||= binaryExpression.symbol?.valueDeclaration;
Expand Down
5 changes: 5 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3361,6 +3361,11 @@
"category": "Error",
"code": 2836
},
"Variable '{0}' is declared in a prior case block.": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case needs to be quoted

Suggested change
"Variable '{0}' is declared in a prior case block.": {
"Variable '{0}' is declared in a prior 'case' block.": {

It's tough to give some more context, but I think it could be helpful. Maybe something like

Suggested change
"Variable '{0}' is declared in a prior case block.": {
"Variable '{0}' is declared in a prior 'case' block and cannot be referenced if that declaration does not execute.": {

"category": "Error",
"code": 2837
},


"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
135 changes: 135 additions & 0 deletions tests/baselines/reference/switchCaseTdz.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
tests/cases/compiler/switchCaseTdz.ts(3,9): error TS2448: Block-scoped variable 'x' used before its declaration.
tests/cases/compiler/switchCaseTdz.ts(4,9): error TS2448: Block-scoped variable 'y' used before its declaration.
tests/cases/compiler/switchCaseTdz.ts(5,9): error TS2448: Block-scoped variable 'z' used before its declaration.
tests/cases/compiler/switchCaseTdz.ts(19,9): error TS2837: Variable 'x' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(20,9): error TS2837: Variable 'y' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(21,9): error TS2837: Variable 'z' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(24,13): error TS2837: Variable 'x' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(25,13): error TS2837: Variable 'y' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(26,13): error TS2837: Variable 'z' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(30,13): error TS2837: Variable 'x' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(31,13): error TS2837: Variable 'y' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(32,13): error TS2837: Variable 'z' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(39,9): error TS2837: Variable 'x' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(40,9): error TS2837: Variable 'y' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(41,9): error TS2837: Variable 'z' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(44,13): error TS2837: Variable 'x' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(45,13): error TS2837: Variable 'y' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(46,13): error TS2837: Variable 'z' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(50,13): error TS2837: Variable 'x' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(51,13): error TS2837: Variable 'y' is declared in a prior case block.
tests/cases/compiler/switchCaseTdz.ts(52,13): error TS2837: Variable 'z' is declared in a prior case block.


==== tests/cases/compiler/switchCaseTdz.ts (21 errors) ====
switch (1 + 1) {
case -1:
x;
~
!!! error TS2448: Block-scoped variable 'x' used before its declaration.
!!! related TS2728 tests/cases/compiler/switchCaseTdz.ts:9:13: 'x' is declared here.
y;
~
!!! error TS2448: Block-scoped variable 'y' used before its declaration.
!!! related TS2728 tests/cases/compiler/switchCaseTdz.ts:10:15: 'y' is declared here.
z;
~
!!! error TS2448: Block-scoped variable 'z' used before its declaration.
!!! related TS2728 tests/cases/compiler/switchCaseTdz.ts:11:16: 'z' is declared here.

case 0:
var ok = 0;
let x = 0;
const y = 0;
const [z] = [0];

ok;
x;
y;
z;

case 1:
x = 0; // <- bad
~
!!! error TS2837: Variable 'x' is declared in a prior case block.
y; // <- bad
~
!!! error TS2837: Variable 'y' is declared in a prior case block.
z; // <- bad
~
!!! error TS2837: Variable 'z' is declared in a prior case block.
ok;
if (true) {
x = 0; // <- bad
~
!!! error TS2837: Variable 'x' is declared in a prior case block.
y; // <- bad
~
!!! error TS2837: Variable 'y' is declared in a prior case block.
z; // <- bad
~
!!! error TS2837: Variable 'z' is declared in a prior case block.
ok;
}
let f1 = function () {
x = 0; // <- bad
~
!!! error TS2837: Variable 'x' is declared in a prior case block.
y; // <- bad
~
!!! error TS2837: Variable 'y' is declared in a prior case block.
z; // <- bad
~
!!! error TS2837: Variable 'z' is declared in a prior case block.
ok;
}
break;

case 2:
case 3:
x; // <- bad
~
!!! error TS2837: Variable 'x' is declared in a prior case block.
y; // <- bad
~
!!! error TS2837: Variable 'y' is declared in a prior case block.
z; // <- bad
~
!!! error TS2837: Variable 'z' is declared in a prior case block.
ok;
if (true) {
x; // <- bad
~
!!! error TS2837: Variable 'x' is declared in a prior case block.
y; // <- bad
~
!!! error TS2837: Variable 'y' is declared in a prior case block.
z; // <- bad
~
!!! error TS2837: Variable 'z' is declared in a prior case block.
ok;
}
let f2 = function () {
x; // <- bad
~
!!! error TS2837: Variable 'x' is declared in a prior case block.
y; // <- bad
~
!!! error TS2837: Variable 'y' is declared in a prior case block.
z; // <- bad
~
!!! error TS2837: Variable 'z' is declared in a prior case block.
ok;
}

}

switch(2 + 2) {
case 0:
let x = 1;
switch(x + x) {
case 2:
// Legal
x;
}
}
129 changes: 129 additions & 0 deletions tests/baselines/reference/switchCaseTdz.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
//// [switchCaseTdz.ts]
switch (1 + 1) {
case -1:
x;
y;
z;

case 0:
var ok = 0;
let x = 0;
const y = 0;
const [z] = [0];

ok;
x;
y;
z;

case 1:
x = 0; // <- bad
y; // <- bad
z; // <- bad
ok;
if (true) {
x = 0; // <- bad
y; // <- bad
z; // <- bad
ok;
}
let f1 = function () {
x = 0; // <- bad
y; // <- bad
z; // <- bad
ok;
}
break;

case 2:
case 3:
x; // <- bad
y; // <- bad
z; // <- bad
ok;
if (true) {
x; // <- bad
y; // <- bad
z; // <- bad
ok;
}
let f2 = function () {
x; // <- bad
y; // <- bad
z; // <- bad
ok;
}

}

switch(2 + 2) {
case 0:
let x = 1;
switch(x + x) {
case 2:
// Legal
x;
}
}

//// [switchCaseTdz.js]
switch (1 + 1) {
case -1:
x_1;
y_1;
z_1;
case 0:
var ok = 0;
var x_1 = 0;
var y_1 = 0;
var z_1 = [0][0];
ok;
x_1;
y_1;
z_1;
case 1:
x_1 = 0; // <- bad
y_1; // <- bad
z_1; // <- bad
ok;
if (true) {
x_1 = 0; // <- bad
y_1; // <- bad
z_1; // <- bad
ok;
}
var f1 = function () {
x_1 = 0; // <- bad
y_1; // <- bad
z_1; // <- bad
ok;
};
break;
case 2:
case 3:
x_1; // <- bad
y_1; // <- bad
z_1; // <- bad
ok;
if (true) {
x_1; // <- bad
y_1; // <- bad
z_1; // <- bad
ok;
}
var f2 = function () {
x_1; // <- bad
y_1; // <- bad
z_1; // <- bad
ok;
};
}
switch (2 + 2) {
case 0:
var x = 1;
switch (x + x) {
case 2:
// Legal
x;
}
}
Loading