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

Destructuring #1346

Merged
merged 38 commits into from Dec 9, 2014

Conversation

Projects
None yet
5 participants
@ahejlsberg
Member

ahejlsberg commented Dec 3, 2014

This pull request adds support for ECMAScript 6 destructuring declarations and assignments with down-level ES3 and ES5 code generation. For example:

var [x, y] = [10, 20];
[x, y] = [y, x];

The above example generates:

var _a = [10, 20], x = _a[0], y = _a[1];
_b = [y, x], x = _b[0], y = _b[1];

Another example:

interface NameAndLocation {
    name: string;
    location: [number, number];
}

function getNameAndLocation(): NameAndLocation {
    return { name: "top-left", location: [10, 20] };
}

function displayNameAndLocation({name, location: [x, y] = [0, 0]}: NameAndLocation) {
    console.log("name=" + name + " x=" + x + " y=" + y);
}

var {name, location: [x, y]} = getNameAndLocation();
displayNameAndLocation(getNameAndLocation());

The above example generates:

function getNameAndLocation() {
    return { name: "top-left", location: [10, 20] };
}
function displayNameAndLocation(_a) {
    var name = _a.name, _b = _a.location, _c = _b === void 0 ? [0, 0] : _b, x = _c[0], y = _c[1];
    console.log("name=" + name + " x=" + x + " y=" + y);
}
var _a = getNameAndLocation(), name = _a.name, _b = _a.location, x = _b[0], y = _b[1];
displayNameAndLocation(getNameAndLocation());

Work still remaining:

  • Support for rest elements in array destructuring.
  • Pure ES6 code generation with no rewriting (under -target ES6 switch).
  • Updated spec.

ahejlsberg added some commits Nov 5, 2014

Merge branch 'master' into destructuring
Conflicts:
	src/compiler/binder.ts
	src/compiler/checker.ts
	src/compiler/diagnosticInformationMap.generated.ts
	src/compiler/diagnosticMessages.json
Allow string or numeric literal as property name of object binding
Require RHS of array destructuring to be an actual array type (i.e. assignable to any[])
Tighten test for tuple type (previously just required a "0" property)
Support exported destructuring variable declarations
Support .d.ts generation for functions with destructuring parameters
@ahejlsberg

This comment has been minimized.

Show comment
Hide comment
@ahejlsberg

ahejlsberg Dec 4, 2014

Member

@CyrusNajmabadi Not sure what you mean by the 'helicoptering' question, but certainly the code is written such that you can helicopter into something like var [[[a]]] = [[[3]]]; and ask for the type of a and we'll do the appropriate 'reaching up and over' to infer type number.

Regarding a new branch, not sure that would help any. To get an all up view of how this branch is different from the current master (without everyone's intervening edits) just use the "Files changed" tab at the top of this page (instead of the link to the commit for the merge with master).

Member

ahejlsberg commented Dec 4, 2014

@CyrusNajmabadi Not sure what you mean by the 'helicoptering' question, but certainly the code is written such that you can helicopter into something like var [[[a]]] = [[[3]]]; and ask for the type of a and we'll do the appropriate 'reaching up and over' to infer type number.

Regarding a new branch, not sure that would help any. To get an all up view of how this branch is different from the current master (without everyone's intervening edits) just use the "Files changed" tab at the top of this page (instead of the link to the commit for the merge with master).

Show outdated Hide outdated src/compiler/parser.ts Outdated
Show outdated Hide outdated src/compiler/parser.ts Outdated
Show outdated Hide outdated src/compiler/parser.ts Outdated
@CyrusNajmabadi

This comment has been minimized.

Show comment
Hide comment
@CyrusNajmabadi

CyrusNajmabadi Dec 5, 2014

Contributor

@ahejlsberg That was indeed what i was referring to when i mentioned 'helicoptering'. Does this support fall out naturally from teh existing code we have that walks upwards/outwards? Or was new code necessary to support the destructuring case? If the latter, could you point out where i should look to find it?

Thanks!

Contributor

CyrusNajmabadi commented Dec 5, 2014

@ahejlsberg That was indeed what i was referring to when i mentioned 'helicoptering'. Does this support fall out naturally from teh existing code we have that walks upwards/outwards? Or was new code necessary to support the destructuring case? If the latter, could you point out where i should look to find it?

Thanks!

@ahejlsberg

This comment has been minimized.

Show comment
Hide comment
@ahejlsberg

ahejlsberg Dec 5, 2014

Member

@CyrusNajmabadi The 'helicoptering' ability is in a sense a core requirement because we need the ability to obtain the type of any variable (including one deeply buried in a destructuring declaration) at any time (including both regular compilation and language service use). So, you could say it naturally falls out of the code, but there wasn't really a choice to do it any other way.

The interesting functions to look at are getTypeForVariableDeclaration and getTypeForBindingElement. They recursively call each other to walk up the chain of parent binding patterns to finally arrive at the top declaration. The root type obtained for that declaration is then broken down as the recursion returns to the leaf binding element.

Further nuance is added by the way in which a binding pattern can imply a type when no annotation or initializer is available from which to infer, and the effects this has on contextual typing. The newly added comments on getWidenedTypeForVariableDeclaration explain how this works.

Member

ahejlsberg commented Dec 5, 2014

@CyrusNajmabadi The 'helicoptering' ability is in a sense a core requirement because we need the ability to obtain the type of any variable (including one deeply buried in a destructuring declaration) at any time (including both regular compilation and language service use). So, you could say it naturally falls out of the code, but there wasn't really a choice to do it any other way.

The interesting functions to look at are getTypeForVariableDeclaration and getTypeForBindingElement. They recursively call each other to walk up the chain of parent binding patterns to finally arrive at the top declaration. The root type obtained for that declaration is then broken down as the recursion returns to the leaf binding element.

Further nuance is added by the way in which a binding pattern can imply a type when no annotation or initializer is available from which to infer, and the effects this has on contextual typing. The newly added comments on getWidenedTypeForVariableDeclaration explain how this works.

@CyrusNajmabadi

This comment has been minimized.

Show comment
Hide comment
@CyrusNajmabadi

CyrusNajmabadi Dec 5, 2014

Contributor

@ahejlsberg Awesome. Thanks for the explanation. It's definitely helpful to understand how all that 'helicoptering in/out' works. BTW, do you have a better term to describe that process?

Contributor

CyrusNajmabadi commented Dec 5, 2014

@ahejlsberg Awesome. Thanks for the explanation. It's definitely helpful to understand how all that 'helicoptering in/out' works. BTW, do you have a better term to describe that process?

Addressing CR feedback:
New SyntaxKind.BindingElement
Introduced new VariableLikeDeclaration and BindingElement types
Cleaned up VariableDeclaration, ParameterDeclaration, PropertyDeclaration types
Node kind of binding element is always SyntaxKind.BindingElement
Changed CheckVariableDeclaration to CheckVariableLikeDeclaration
Reorganized CheckVariableLikeDeclaration
@CyrusNajmabadi

This comment has been minimized.

Show comment
Hide comment
@CyrusNajmabadi

CyrusNajmabadi Dec 6, 2014

Contributor

getTypeForVariable_Like_Declaration

Contributor

CyrusNajmabadi commented on src/compiler/checker.ts in 05c9966 Dec 6, 2014

getTypeForVariable_Like_Declaration

@CyrusNajmabadi

This comment has been minimized.

Show comment
Hide comment
@CyrusNajmabadi

CyrusNajmabadi Dec 6, 2014

Contributor

getWidenedTypeForVariable_Like_Declaration

Contributor

CyrusNajmabadi commented on src/compiler/checker.ts in 05c9966 Dec 6, 2014

getWidenedTypeForVariable_Like_Declaration

@CyrusNajmabadi

This comment has been minimized.

Show comment
Hide comment
@CyrusNajmabadi

CyrusNajmabadi Dec 6, 2014

Contributor

=== (just for consistency)

Contributor

CyrusNajmabadi commented on src/compiler/checker.ts in 05c9966 Dec 6, 2014

=== (just for consistency)

@CyrusNajmabadi

This comment has been minimized.

Show comment
Hide comment
@CyrusNajmabadi

CyrusNajmabadi Dec 6, 2014

Contributor

seems odd for VariableDeclaration to not extend VariableLikeDeclaration. Was that intentional?

Contributor

CyrusNajmabadi commented on src/compiler/types.ts in 05c9966 Dec 6, 2014

seems odd for VariableDeclaration to not extend VariableLikeDeclaration. Was that intentional?

This comment has been minimized.

Show comment
Hide comment
@ahejlsberg

ahejlsberg Dec 6, 2014

Member

This is intentional. VariableLikeDeclaration is a type with all of the (optional) members a variable-like declaration might have (as opposed to the members they all share). All variable-like declaration types are assignable to VariableLikeDeclaration but not actually subtypes (because we want the individual types to only have those properties that could actually occur).

Member

ahejlsberg replied Dec 6, 2014

This is intentional. VariableLikeDeclaration is a type with all of the (optional) members a variable-like declaration might have (as opposed to the members they all share). All variable-like declaration types are assignable to VariableLikeDeclaration but not actually subtypes (because we want the individual types to only have those properties that could actually occur).

@CyrusNajmabadi

This comment has been minimized.

Show comment
Hide comment
@CyrusNajmabadi

CyrusNajmabadi Dec 6, 2014

Contributor

Same issue with ParameterDeclaration.

Contributor

CyrusNajmabadi commented on src/compiler/types.ts in 05c9966 Dec 6, 2014

Same issue with ParameterDeclaration.

@CyrusNajmabadi

This comment has been minimized.

Show comment
Hide comment
@CyrusNajmabadi

CyrusNajmabadi Dec 6, 2014

Contributor

Consider adding a brand to this type, and then using subclassing. There will then be stronger nominal typing in the compiler to ensure on the right types can be passed to a function that takes a VariableLikeDeclaration.

Contributor

CyrusNajmabadi commented on src/compiler/types.ts in 05c9966 Dec 6, 2014

Consider adding a brand to this type, and then using subclassing. There will then be stronger nominal typing in the compiler to ensure on the right types can be passed to a function that takes a VariableLikeDeclaration.

@CyrusNajmabadi

This comment has been minimized.

Show comment
Hide comment
@CyrusNajmabadi

CyrusNajmabadi Dec 6, 2014

Contributor

cast to ParameterDeclaration.

Contributor

CyrusNajmabadi commented on src/services/navigationBar.ts in 05c9966 Dec 6, 2014

cast to ParameterDeclaration.

@CyrusNajmabadi

This comment has been minimized.

Show comment
Hide comment
@CyrusNajmabadi

CyrusNajmabadi Dec 6, 2014

Contributor

Looks pretty good overall. Thanks for making these changes!

Contributor

CyrusNajmabadi commented on 05c9966 Dec 6, 2014

Looks pretty good overall. Thanks for making these changes!

ahejlsberg added some commits Dec 6, 2014

Merge branch 'master' into destructuring
Move downlevel vs. ES6 emit branching into individual emit functions
@mhegazy

This comment has been minimized.

Show comment
Hide comment
@mhegazy

mhegazy Dec 9, 2014

Contributor

👍

Contributor

mhegazy commented Dec 9, 2014

👍

1 similar comment
@JsonFreeman

This comment has been minimized.

Show comment
Hide comment
@JsonFreeman

JsonFreeman Dec 9, 2014

Contributor

👍

Contributor

JsonFreeman commented Dec 9, 2014

👍

Merge branch 'master' into destructuring
Conflicts:
	src/compiler/binder.ts
	src/compiler/checker.ts
	src/compiler/emitter.ts
	src/compiler/parser.ts
	src/services/services.ts
	tests/baselines/reference/parserCommaInTypeMemberList2.errors.txt

ahejlsberg added a commit that referenced this pull request Dec 9, 2014

@ahejlsberg ahejlsberg merged commit bb70e9e into master Dec 9, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@ahejlsberg ahejlsberg deleted the destructuring branch Dec 9, 2014

// If no type was specified or inferred for parent, or if the specified or inferred type is any,
// infer from the initializer of the binding element if one is present. Otherwise, go with the
// undefined or any type of the parent.
if (!parentType || parentType === anyType) {

This comment has been minimized.

@JsonFreeman

JsonFreeman Dec 9, 2014

Contributor

Shouldn't the type be any if the parent type was any?

@JsonFreeman

JsonFreeman Dec 9, 2014

Contributor

Shouldn't the type be any if the parent type was any?

This comment has been minimized.

@ahejlsberg

ahejlsberg Dec 9, 2014

Member

It is if there is not an initializer. If you're suggesting it should be any regardless of the initializer, then no. Consider:

var a: any;
var [x, y = 1] = a;

Here, a provides no clue as to the intended type of x and y, so we infer from the type of the initializer. This contrasts with:

var a: any;
var [x, y = 1] = [a, a];

Here, the type of a is the intended type for y, and the initializer is required to be assignment compatible with that type.

@ahejlsberg

ahejlsberg Dec 9, 2014

Member

It is if there is not an initializer. If you're suggesting it should be any regardless of the initializer, then no. Consider:

var a: any;
var [x, y = 1] = a;

Here, a provides no clue as to the intended type of x and y, so we infer from the type of the initializer. This contrasts with:

var a: any;
var [x, y = 1] = [a, a];

Here, the type of a is the intended type for y, and the initializer is required to be assignment compatible with that type.

This comment has been minimized.

@JsonFreeman

JsonFreeman Dec 9, 2014

Contributor

Do we do that for contextual typing? An analogous example (courtesy of @CyrusNajmabadi):

var v: (a: any) => void = (a = 1) { a.blah };

Should a in the body be of type any or number? Applying the reasoning you stated from destructuring, it seems like in this example it should be number, not any.

@JsonFreeman

JsonFreeman Dec 9, 2014

Contributor

Do we do that for contextual typing? An analogous example (courtesy of @CyrusNajmabadi):

var v: (a: any) => void = (a = 1) { a.blah };

Should a in the body be of type any or number? Applying the reasoning you stated from destructuring, it seems like in this example it should be number, not any.

This comment has been minimized.

@ahejlsberg

ahejlsberg Dec 10, 2014

Member

For a parameter the order of precedence is this:

  • Explicit type annotation.
  • Contextual parameter type.
  • Initializer type.

For a binding element it is this:

  • Deconstructed type (i.e. the type of the corresponding property or array element in the deconstructed value).
  • Type of local default value (initializer) if type of deconstructed value is unknown or any.

So, you can't really apply the reasoning of one to the other. They're different kinds of constructs.

@ahejlsberg

ahejlsberg Dec 10, 2014

Member

For a parameter the order of precedence is this:

  • Explicit type annotation.
  • Contextual parameter type.
  • Initializer type.

For a binding element it is this:

  • Deconstructed type (i.e. the type of the corresponding property or array element in the deconstructed value).
  • Type of local default value (initializer) if type of deconstructed value is unknown or any.

So, you can't really apply the reasoning of one to the other. They're different kinds of constructs.

This comment has been minimized.

@JsonFreeman

JsonFreeman Dec 10, 2014

Contributor

I understand that the way you described is how things are working, but it doesn't seem self explanatory why that is the case. Why are they different constructs?

@JsonFreeman

JsonFreeman Dec 10, 2014

Contributor

I understand that the way you described is how things are working, but it doesn't seem self explanatory why that is the case. Why are they different constructs?

This comment has been minimized.

@ahejlsberg

ahejlsberg Dec 10, 2014

Member

I assume you agree with the already existing rules for parameters, so I won't try to elaborate on those.

For a destructuring element, the high order bit is the type of the value being deconstructed. Indeed, the initializer isn't even evaluated when a deconstructed value is available. So, this establishes the intuition that the binding element type should primarily come from the corresponding property or element in the deconstructed type. Then, in cases where the type of the deconstructed value isn't known or is any, it makes sense to take a clue from the initializer. That's the next best source of information.

@ahejlsberg

ahejlsberg Dec 10, 2014

Member

I assume you agree with the already existing rules for parameters, so I won't try to elaborate on those.

For a destructuring element, the high order bit is the type of the value being deconstructed. Indeed, the initializer isn't even evaluated when a deconstructed value is available. So, this establishes the intuition that the binding element type should primarily come from the corresponding property or element in the deconstructed type. Then, in cases where the type of the deconstructed value isn't known or is any, it makes sense to take a clue from the initializer. That's the next best source of information.

This comment has been minimized.

@JsonFreeman

JsonFreeman Dec 10, 2014

Contributor

That makes a lot of sense, but why is that not the case for parameters? I don't necessarily agree with the rules for parameters. I buy your argument about the destructuring and would like to apply it to parameters

@JsonFreeman

JsonFreeman Dec 10, 2014

Contributor

That makes a lot of sense, but why is that not the case for parameters? I don't necessarily agree with the rules for parameters. I buy your argument about the destructuring and would like to apply it to parameters

}
// Return the inferred type for a binding element
function getTypeForBindingElement(declaration: BindingElement): Type {

This comment has been minimized.

@JsonFreeman

JsonFreeman Dec 9, 2014

Contributor

What is the difference between getTypeForBindingElement and getTypeFromBindingElement?

@JsonFreeman

JsonFreeman Dec 9, 2014

Contributor

What is the difference between getTypeForBindingElement and getTypeFromBindingElement?

This comment has been minimized.

@ahejlsberg

ahejlsberg Dec 9, 2014

Member

It's pretty much what the names say (and there are a number of comments in the code). Consider:

var [x, y = 1] = [10, undefined];

Here, getTypeForBindingElement returns number and undefined for x and y respectively, and getTypeFromBindingElement returns any and number for x and y respectively.

@ahejlsberg

ahejlsberg Dec 9, 2014

Member

It's pretty much what the names say (and there are a number of comments in the code). Consider:

var [x, y = 1] = [10, undefined];

Here, getTypeForBindingElement returns number and undefined for x and y respectively, and getTypeFromBindingElement returns any and number for x and y respectively.

This comment has been minimized.

@JsonFreeman

JsonFreeman Dec 9, 2014

Contributor

Maybe we should call them getTypeFromBindingElementParent and getTypeFromBindingElementInitializer

@JsonFreeman

JsonFreeman Dec 9, 2014

Contributor

Maybe we should call them getTypeFromBindingElementParent and getTypeFromBindingElementInitializer

This comment has been minimized.

@ahejlsberg

ahejlsberg Dec 10, 2014

Member

Not crazy about those. Regarding getTypeFromBindingElementParent, we're not actually getting the type from the element's parent, we're getting it from the corresponding property or element in the type of the value being destructured. Regarding getTypeFromBindingElementInitializer, the type comes from the initializer or, lacking one, if the element is itself a binding pattern, from the shape of that pattern and recursively from it's elements.

@ahejlsberg

ahejlsberg Dec 10, 2014

Member

Not crazy about those. Regarding getTypeFromBindingElementParent, we're not actually getting the type from the element's parent, we're getting it from the corresponding property or element in the type of the value being destructured. Regarding getTypeFromBindingElementInitializer, the type comes from the initializer or, lacking one, if the element is itself a binding pattern, from the shape of that pattern and recursively from it's elements.

This comment has been minimized.

@JsonFreeman

JsonFreeman Dec 10, 2014

Contributor

Maybe those names aren't very accurate, but I think we need two names where the distinction is encoded in more than a preposition.

@JsonFreeman

JsonFreeman Dec 10, 2014

Contributor

Maybe those names aren't very accurate, but I think we need two names where the distinction is encoded in more than a preposition.

@Microsoft Microsoft locked and limited conversation to collaborators Jun 18, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.