Skip to content

Declaration emit fixes for binding pattern in variable statements #2025

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

Merged
merged 2 commits into from
Mar 18, 2015
Merged
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
9 changes: 8 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1581,8 +1581,15 @@ module ts {

function determineIfDeclarationIsVisible() {
switch (node.kind) {
case SyntaxKind.VariableDeclaration:
case SyntaxKind.BindingElement:
return isDeclarationVisible(<Declaration>node.parent.parent);
case SyntaxKind.VariableDeclaration:
if (isBindingPattern(node.name) &&
!(<BindingPattern>node.name).elements.length) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer you just compared to 0.

// If the binding pattern is empty, this variable declaration is not visible
return false;
}
// Otherwise fall through
case SyntaxKind.ModuleDeclaration:
case SyntaxKind.ClassDeclaration:
case SyntaxKind.InterfaceDeclaration:
Expand Down
96 changes: 64 additions & 32 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1024,62 +1024,94 @@ module ts {
// If we are emitting property it isn't moduleElement and hence we already know it needs to be emitted
// so there is no check needed to see if declaration is visible
if (node.kind !== SyntaxKind.VariableDeclaration || resolver.isDeclarationVisible(node)) {
// If this node is a computed name, it can only be a symbol, because we've already skipped
// it if it's not a well known symbol. In that case, the text of the name will be exactly
// what we want, namely the name expression enclosed in brackets.
writeTextOfNode(currentSourceFile, node.name);
// If optional property emit ?
if ((node.kind === SyntaxKind.PropertyDeclaration || node.kind === SyntaxKind.PropertySignature) && hasQuestionToken(node)) {
write("?");
}
if ((node.kind === SyntaxKind.PropertyDeclaration || node.kind === SyntaxKind.PropertySignature) && node.parent.kind === SyntaxKind.TypeLiteral) {
emitTypeOfVariableDeclarationFromTypeLiteral(node);
if (isBindingPattern(node.name)) {
emitBindingPattern(<BindingPattern>node.name);
}
else if (!(node.flags & NodeFlags.Private)) {
writeTypeOfDeclaration(node, node.type, getVariableDeclarationTypeVisibilityError);
else {
// If this node is a computed name, it can only be a symbol, because we've already skipped
// it if it's not a well known symbol. In that case, the text of the name will be exactly
// what we want, namely the name expression enclosed in brackets.
writeTextOfNode(currentSourceFile, node.name);
// If optional property emit ?
if ((node.kind === SyntaxKind.PropertyDeclaration || node.kind === SyntaxKind.PropertySignature) && hasQuestionToken(node)) {
write("?");
}
if ((node.kind === SyntaxKind.PropertyDeclaration || node.kind === SyntaxKind.PropertySignature) && node.parent.kind === SyntaxKind.TypeLiteral) {
emitTypeOfVariableDeclarationFromTypeLiteral(node);
}
else if (!(node.flags & NodeFlags.Private)) {
writeTypeOfDeclaration(node, node.type, getVariableDeclarationTypeVisibilityError);
}
}
}

function getVariableDeclarationTypeVisibilityError(symbolAccesibilityResult: SymbolAccessiblityResult): SymbolAccessibilityDiagnostic {
var diagnosticMessage: DiagnosticMessage;
function getVariableDeclarationTypeVisibilityDiagnosticMessage(symbolAccesibilityResult: SymbolAccessiblityResult) {
if (node.kind === SyntaxKind.VariableDeclaration) {
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
symbolAccesibilityResult.accessibility === SymbolAccessibility.CannotBeNamed ?
Diagnostics.Exported_variable_0_has_or_is_using_name_1_from_external_module_2_but_cannot_be_named :
Diagnostics.Exported_variable_0_has_or_is_using_name_1_from_private_module_2 :
Diagnostics.Exported_variable_0_has_or_is_using_private_name_1;
return symbolAccesibilityResult.errorModuleName ?
symbolAccesibilityResult.accessibility === SymbolAccessibility.CannotBeNamed ?
Diagnostics.Exported_variable_0_has_or_is_using_name_1_from_external_module_2_but_cannot_be_named :
Diagnostics.Exported_variable_0_has_or_is_using_name_1_from_private_module_2 :
Diagnostics.Exported_variable_0_has_or_is_using_private_name_1;
}
// This check is to ensure we don't report error on constructor parameter property as that error would be reported during parameter emit
else if (node.kind === SyntaxKind.PropertyDeclaration || node.kind === SyntaxKind.PropertySignature) {
// TODO(jfreeman): Deal with computed properties in error reporting.
if (node.flags & NodeFlags.Static) {
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
symbolAccesibilityResult.accessibility === SymbolAccessibility.CannotBeNamed ?
Diagnostics.Public_static_property_0_of_exported_class_has_or_is_using_name_1_from_external_module_2_but_cannot_be_named :
Diagnostics.Public_static_property_0_of_exported_class_has_or_is_using_name_1_from_private_module_2 :
Diagnostics.Public_static_property_0_of_exported_class_has_or_is_using_private_name_1;
return symbolAccesibilityResult.errorModuleName ?
symbolAccesibilityResult.accessibility === SymbolAccessibility.CannotBeNamed ?
Diagnostics.Public_static_property_0_of_exported_class_has_or_is_using_name_1_from_external_module_2_but_cannot_be_named :
Diagnostics.Public_static_property_0_of_exported_class_has_or_is_using_name_1_from_private_module_2 :
Diagnostics.Public_static_property_0_of_exported_class_has_or_is_using_private_name_1;
}
else if (node.parent.kind === SyntaxKind.ClassDeclaration) {
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
symbolAccesibilityResult.accessibility === SymbolAccessibility.CannotBeNamed ?
Diagnostics.Public_property_0_of_exported_class_has_or_is_using_name_1_from_external_module_2_but_cannot_be_named :
Diagnostics.Public_property_0_of_exported_class_has_or_is_using_name_1_from_private_module_2 :
Diagnostics.Public_property_0_of_exported_class_has_or_is_using_private_name_1;
return symbolAccesibilityResult.errorModuleName ?
symbolAccesibilityResult.accessibility === SymbolAccessibility.CannotBeNamed ?
Diagnostics.Public_property_0_of_exported_class_has_or_is_using_name_1_from_external_module_2_but_cannot_be_named :
Diagnostics.Public_property_0_of_exported_class_has_or_is_using_name_1_from_private_module_2 :
Diagnostics.Public_property_0_of_exported_class_has_or_is_using_private_name_1;
}
else {
// Interfaces cannot have types that cannot be named
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
Diagnostics.Property_0_of_exported_interface_has_or_is_using_name_1_from_private_module_2 :
Diagnostics.Property_0_of_exported_interface_has_or_is_using_private_name_1;
return symbolAccesibilityResult.errorModuleName ?
Diagnostics.Property_0_of_exported_interface_has_or_is_using_name_1_from_private_module_2 :
Diagnostics.Property_0_of_exported_interface_has_or_is_using_private_name_1;
}
}
}

function getVariableDeclarationTypeVisibilityError(symbolAccesibilityResult: SymbolAccessiblityResult): SymbolAccessibilityDiagnostic {
var diagnosticMessage = getVariableDeclarationTypeVisibilityDiagnosticMessage(symbolAccesibilityResult);
return diagnosticMessage !== undefined ? {
diagnosticMessage,
errorNode: node,
typeName: node.name
} : undefined;
}

function emitBindingPattern(bindingPattern: BindingPattern) {
emitCommaList(bindingPattern.elements, emitBindingElement);
}

function emitBindingElement(bindingElement: BindingElement) {
function getBindingElementTypeVisibilityError(symbolAccesibilityResult: SymbolAccessiblityResult): SymbolAccessibilityDiagnostic {
var diagnosticMessage = getVariableDeclarationTypeVisibilityDiagnosticMessage(symbolAccesibilityResult);
return diagnosticMessage !== undefined ? {
diagnosticMessage,
errorNode: bindingElement,
typeName: bindingElement.name
} : undefined;
}

if (bindingElement.name) {
Copy link
Member

Choose a reason for hiding this comment

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

When would the name be undefined?

Copy link
Member

Choose a reason for hiding this comment

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

Never mind, I see that the reason is that you can have an omitted element such as in the following:

var [a, , c] = foo();

But please put a comment with an example.

Also, I believe that @JsonFreeman mentioned that for a well-formed tree, it is more appropriate to check the syntax kind.

Copy link
Contributor

Choose a reason for hiding this comment

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

Namely, when bindingElement.name is undefined, it's because the syntax kind is OmittedExpression and not BindingElement

if (isBindingPattern(bindingElement.name)) {
emitBindingPattern(<BindingPattern>bindingElement.name);
}
else {
writeTextOfNode(currentSourceFile, bindingElement.name);
writeTypeOfDeclaration(bindingElement, /*type*/ undefined, getBindingElementTypeVisibilityError);
}
}
}
}

function emitTypeOfVariableDeclarationFromTypeLiteral(node: VariableLikeDeclaration) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//// [declarationEmitDestructuringArrayPattern1.ts]

var [] = [1, "hello"]; // Dont emit anything
var [x] = [1, "hello"]; // emit x: number
var [x1, y1] = [1, "hello"]; // emit x1: number, y1: string
var [, , z1] = [0, 1, 2]; // emit z1: number

var a = [1, "hello"];
var [x2] = a; // emit x2: number | string
var [x3, y3, z3] = a; // emit x3, y3, z3

//// [declarationEmitDestructuringArrayPattern1.js]
var _a = [1, "hello"]; // Dont emit anything
var x = ([1, "hello"])[0]; // emit x: number
var _b = [1, "hello"], x1 = _b[0], y1 = _b[1]; // emit x1: number, y1: string
var _c = [0, 1, 2], z1 = _c[2]; // emit z1: number
var a = [1, "hello"];
var x2 = a[0]; // emit x2: number | string
var x3 = a[0], y3 = a[1], z3 = a[2]; // emit x3, y3, z3


//// [declarationEmitDestructuringArrayPattern1.d.ts]
declare var x: number;
declare var x1: number, y1: string;
declare var z1: number;
declare var a: (string | number)[];
declare var x2: string | number;
declare var x3: string | number, y3: string | number, z3: string | number;
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
=== tests/cases/compiler/declarationEmitDestructuringArrayPattern1.ts ===

var [] = [1, "hello"]; // Dont emit anything
>[1, "hello"] : (string | number)[]

var [x] = [1, "hello"]; // emit x: number
>x : number
>[1, "hello"] : [number, string]

var [x1, y1] = [1, "hello"]; // emit x1: number, y1: string
>x1 : number
>y1 : string
>[1, "hello"] : [number, string]

var [, , z1] = [0, 1, 2]; // emit z1: number
>z1 : number
>[0, 1, 2] : [number, number, number]

var a = [1, "hello"];
>a : (string | number)[]
>[1, "hello"] : (string | number)[]

var [x2] = a; // emit x2: number | string
>x2 : string | number
>a : (string | number)[]

var [x3, y3, z3] = a; // emit x3, y3, z3
>x3 : string | number
>y3 : string | number
>z3 : string | number
>a : (string | number)[]

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//// [declarationEmitDestructuringArrayPattern2.ts]
var [x10, [y10, [z10]]] = [1, ["hello", [true]]];

var [x11 = 0, y11 = ""] = [1, "hello"];
var [a11, b11, c11] = [];

var [a2, [b2, { x12, y12: c2 }]=["abc", { x12: 10, y12: false }]] = [1, ["hello", { x12: 5, y12: true }]];

var [x13, y13] = [1, "hello"];
var [a3, b3] = [[x13, y13], { x: x13, y: y13 }];


//// [declarationEmitDestructuringArrayPattern2.js]
var _a = [1, ["hello", [true]]], x10 = _a[0], _b = _a[1], y10 = _b[0], z10 = _b[1][0];
var _c = [1, "hello"], _d = _c[0], x11 = _d === void 0 ? 0 : _d, _e = _c[1], y11 = _e === void 0 ? "" : _e;
var _f = [], a11 = _f[0], b11 = _f[1], c11 = _f[2];
var _g = [1, ["hello", { x12: 5, y12: true }]], a2 = _g[0], _h = _g[1], _j = _h === void 0 ? ["abc", { x12: 10, y12: false }] : _h, b2 = _j[0], _k = _j[1], x12 = _k.x12, c2 = _k.y12;
var _l = [1, "hello"], x13 = _l[0], y13 = _l[1];
var _m = [[x13, y13], { x: x13, y: y13 }], a3 = _m[0], b3 = _m[1];


//// [declarationEmitDestructuringArrayPattern2.d.ts]
declare var x10: number, y10: string, z10: boolean;
declare var x11: number, y11: string;
declare var a11: any, b11: any, c11: any;
declare var a2: number, b2: string, x12: number, c2: boolean;
declare var x13: number, y13: string;
declare var a3: (string | number)[], b3: {
x: number;
y: string;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
=== tests/cases/compiler/declarationEmitDestructuringArrayPattern2.ts ===
var [x10, [y10, [z10]]] = [1, ["hello", [true]]];
>x10 : number
>y10 : string
>z10 : boolean
>[1, ["hello", [true]]] : [number, [string, [boolean]]]
>["hello", [true]] : [string, [boolean]]
>[true] : [boolean]

var [x11 = 0, y11 = ""] = [1, "hello"];
>x11 : number
>y11 : string
>[1, "hello"] : [number, string]

var [a11, b11, c11] = [];
>a11 : any
>b11 : any
>c11 : any
>[] : undefined[]

var [a2, [b2, { x12, y12: c2 }]=["abc", { x12: 10, y12: false }]] = [1, ["hello", { x12: 5, y12: true }]];
>a2 : number
>b2 : string
>x12 : number
>y12 : unknown
>c2 : boolean
>["abc", { x12: 10, y12: false }] : [string, { x12: number; y12: boolean; }]
>{ x12: 10, y12: false } : { x12: number; y12: boolean; }
>x12 : number
>y12 : boolean
>[1, ["hello", { x12: 5, y12: true }]] : [number, [string, { x12: number; y12: boolean; }]]
>["hello", { x12: 5, y12: true }] : [string, { x12: number; y12: boolean; }]
>{ x12: 5, y12: true } : { x12: number; y12: boolean; }
>x12 : number
>y12 : boolean

var [x13, y13] = [1, "hello"];
>x13 : number
>y13 : string
>[1, "hello"] : [number, string]

var [a3, b3] = [[x13, y13], { x: x13, y: y13 }];
>a3 : (string | number)[]
>b3 : { x: number; y: string; }
>[[x13, y13], { x: x13, y: y13 }] : [(string | number)[], { x: number; y: string; }]
>[x13, y13] : (string | number)[]
>x13 : number
>y13 : string
>{ x: x13, y: y13 } : { x: number; y: string; }
>x : number
>x13 : number
>y : string
>y13 : string

Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//// [declarationEmitDestructuringArrayPattern3.ts]
module M {
export var [a, b] = [1, 2];
}

//// [declarationEmitDestructuringArrayPattern3.js]
var M;
(function (M) {
_a = [1, 2], M.a = _a[0], M.b = _a[1];
var _a;
})(M || (M = {}));


//// [declarationEmitDestructuringArrayPattern3.d.ts]
declare module M {
var a: number, b: number;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
=== tests/cases/compiler/declarationEmitDestructuringArrayPattern3.ts ===
module M {
>M : typeof M

export var [a, b] = [1, 2];
>a : number
>b : number
>[1, 2] : [number, number]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//// [declarationEmitDestructuringArrayPattern4.ts]
var [...a5] = [1, 2, 3];
var [x14, ...a6] = [1, 2, 3];
var [x15, y15, ...a7] = [1, 2, 3];
var [x16, y16, z16, ...a8] = [1, 2, 3];

var [...a9] = [1, "hello", true];
var [x17, ...a10] = [1, "hello", true];
var [x18, y18, ...a12] = [1, "hello", true];
var [x19, y19, z19, ...a13] = [1, "hello", true];

//// [declarationEmitDestructuringArrayPattern4.js]
var _a = [1, 2, 3], a5 = _a.slice(0);
var _b = [1, 2, 3], x14 = _b[0], a6 = _b.slice(1);
var _c = [1, 2, 3], x15 = _c[0], y15 = _c[1], a7 = _c.slice(2);
var _d = [1, 2, 3], x16 = _d[0], y16 = _d[1], z16 = _d[2], a8 = _d.slice(3);
var _e = [1, "hello", true], a9 = _e.slice(0);
var _f = [1, "hello", true], x17 = _f[0], a10 = _f.slice(1);
var _g = [1, "hello", true], x18 = _g[0], y18 = _g[1], a12 = _g.slice(2);
var _h = [1, "hello", true], x19 = _h[0], y19 = _h[1], z19 = _h[2], a13 = _h.slice(3);


//// [declarationEmitDestructuringArrayPattern4.d.ts]
declare var a5: number[];
declare var x14: number, a6: number[];
declare var x15: number, y15: number, a7: number[];
declare var x16: number, y16: number, z16: number, a8: number[];
declare var a9: (string | number | boolean)[];
declare var x17: string | number | boolean, a10: (string | number | boolean)[];
declare var x18: string | number | boolean, y18: string | number | boolean, a12: (string | number | boolean)[];
declare var x19: string | number | boolean, y19: string | number | boolean, z19: string | number | boolean, a13: (string | number | boolean)[];
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
=== tests/cases/compiler/declarationEmitDestructuringArrayPattern4.ts ===
var [...a5] = [1, 2, 3];
>a5 : number[]
>[1, 2, 3] : number[]

var [x14, ...a6] = [1, 2, 3];
>x14 : number
>a6 : number[]
>[1, 2, 3] : number[]

var [x15, y15, ...a7] = [1, 2, 3];
>x15 : number
>y15 : number
>a7 : number[]
>[1, 2, 3] : number[]

var [x16, y16, z16, ...a8] = [1, 2, 3];
>x16 : number
>y16 : number
>z16 : number
>a8 : number[]
>[1, 2, 3] : number[]

var [...a9] = [1, "hello", true];
>a9 : (string | number | boolean)[]
>[1, "hello", true] : (string | number | boolean)[]

var [x17, ...a10] = [1, "hello", true];
>x17 : string | number | boolean
>a10 : (string | number | boolean)[]
>[1, "hello", true] : (string | number | boolean)[]

var [x18, y18, ...a12] = [1, "hello", true];
>x18 : string | number | boolean
>y18 : string | number | boolean
>a12 : (string | number | boolean)[]
>[1, "hello", true] : (string | number | boolean)[]

var [x19, y19, z19, ...a13] = [1, "hello", true];
>x19 : string | number | boolean
>y19 : string | number | boolean
>z19 : string | number | boolean
>a13 : (string | number | boolean)[]
>[1, "hello", true] : (string | number | boolean)[]

Loading