Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixes rename for destructuring, named imports and default imports #7945

Merged
merged 17 commits into from
Apr 13, 2016
Merged

Fixes rename for destructuring, named imports and default imports #7945

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
ace2285
Destructuring of object binding pattern element without property name…
sheetalkamat Apr 5, 2016
01ca100
Include the target symbol in search if location is propertyName or th…
sheetalkamat Apr 6, 2016
168d106
Test cases for destructuring declarations in "for" and "for of" loops
sheetalkamat Apr 7, 2016
6d43c02
Test cases when var is renamed and used in destructuring
sheetalkamat Apr 7, 2016
ad916ab
Handles when property is renamed and is also part of destructuring as…
sheetalkamat Apr 7, 2016
2e44bcc
Test case for nested binding element's rename
sheetalkamat Apr 7, 2016
a405f28
Merge branch 'master' into renameAndFindRef
sheetalkamat Apr 7, 2016
92622bf
Fix the git ignore so that browser test server is ignored
sheetalkamat Apr 8, 2016
f7ca439
Handle the rename locations for default import
sheetalkamat Apr 8, 2016
9e77730
Rename function
sheetalkamat Apr 8, 2016
9e82646
Another test case for rename in destructuring assignment
sheetalkamat Apr 8, 2016
7a09e2f
PR feedback
sheetalkamat Apr 11, 2016
529bdd4
PR feedback
sheetalkamat Apr 11, 2016
9496146
Rename of getPropertySymbolIfObjectBindingPatternWithoutPropertyName …
sheetalkamat Apr 11, 2016
edd0989
Removed debugger; statement
sheetalkamat Apr 11, 2016
958a6a4
Some restructuring according to PR feedback
sheetalkamat Apr 11, 2016
c492fc6
Update the entry point to return property symbol of destructuring ass…
sheetalkamat Apr 13, 2016
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
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ rwc-report.html
*.swp
build.json
*.actual
tests/webTestServer.js
tests/webTestServer.js.map
tests/webhost/*.d.ts
tests/webhost/webtsc.js
tests/cases/**/*.js
Expand Down
150 changes: 98 additions & 52 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ namespace ts {
getShorthandAssignmentValueSymbol,
getExportSpecifierLocalTargetSymbol,
getTypeAtLocation: getTypeOfNode,
getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment,
typeToString,
getSymbolDisplayBuilder,
symbolToString,
Expand Down Expand Up @@ -11797,39 +11798,43 @@ namespace ts {
function checkObjectLiteralAssignment(node: ObjectLiteralExpression, sourceType: Type, contextualMapper?: TypeMapper): Type {
const properties = node.properties;
for (const p of properties) {
if (p.kind === SyntaxKind.PropertyAssignment || p.kind === SyntaxKind.ShorthandPropertyAssignment) {
const name = <PropertyName>(<PropertyAssignment>p).name;
if (name.kind === SyntaxKind.ComputedPropertyName) {
checkComputedPropertyName(<ComputedPropertyName>name);
}
if (isComputedNonLiteralName(name)) {
continue;
}
checkObjectLiteralDestructuringPropertyAssignment(sourceType, p, contextualMapper);
}
return sourceType;
}

const text = getTextOfPropertyName(name);
const type = isTypeAny(sourceType)
? sourceType
: getTypeOfPropertyOfType(sourceType, text) ||
isNumericLiteralName(text) && getIndexTypeOfType(sourceType, IndexKind.Number) ||
getIndexTypeOfType(sourceType, IndexKind.String);
if (type) {
if (p.kind === SyntaxKind.ShorthandPropertyAssignment) {
checkDestructuringAssignment(<ShorthandPropertyAssignment>p, type);
}
else {
// non-shorthand property assignments should always have initializers
checkDestructuringAssignment((<PropertyAssignment>p).initializer, type);
}
function checkObjectLiteralDestructuringPropertyAssignment(objectLiteralType: Type, property: ObjectLiteralElement, contextualMapper?: TypeMapper) {
if (property.kind === SyntaxKind.PropertyAssignment || property.kind === SyntaxKind.ShorthandPropertyAssignment) {
const name = <PropertyName>(<PropertyAssignment>property).name;
if (name.kind === SyntaxKind.ComputedPropertyName) {
checkComputedPropertyName(<ComputedPropertyName>name);
}
if (isComputedNonLiteralName(name)) {
return undefined;
}

const text = getTextOfPropertyName(name);
const type = isTypeAny(objectLiteralType)
? objectLiteralType
: getTypeOfPropertyOfType(objectLiteralType, text) ||
isNumericLiteralName(text) && getIndexTypeOfType(objectLiteralType, IndexKind.Number) ||
getIndexTypeOfType(objectLiteralType, IndexKind.String);
if (type) {
if (property.kind === SyntaxKind.ShorthandPropertyAssignment) {
return checkDestructuringAssignment(<ShorthandPropertyAssignment>property, type);
}
else {
error(name, Diagnostics.Type_0_has_no_property_1_and_no_string_index_signature, typeToString(sourceType), declarationNameToString(name));
// non-shorthand property assignments should always have initializers
return checkDestructuringAssignment((<PropertyAssignment>property).initializer, type);
}
}
else {
error(p, Diagnostics.Property_assignment_expected);
error(name, Diagnostics.Type_0_has_no_property_1_and_no_string_index_signature, typeToString(objectLiteralType), declarationNameToString(name));
}
}
return sourceType;
else {
error(property, Diagnostics.Property_assignment_expected);
}
}

function checkArrayLiteralAssignment(node: ArrayLiteralExpression, sourceType: Type, contextualMapper?: TypeMapper): Type {
Expand All @@ -11839,44 +11844,51 @@ namespace ts {
const elementType = checkIteratedTypeOrElementType(sourceType, node, /*allowStringInput*/ false) || unknownType;
const elements = node.elements;
for (let i = 0; i < elements.length; i++) {
const e = elements[i];
if (e.kind !== SyntaxKind.OmittedExpression) {
if (e.kind !== SyntaxKind.SpreadElementExpression) {
const propName = "" + i;
const type = isTypeAny(sourceType)
? sourceType
: isTupleLikeType(sourceType)
? getTypeOfPropertyOfType(sourceType, propName)
: elementType;
if (type) {
checkDestructuringAssignment(e, type, contextualMapper);
checkArrayLiteralDestructuringElementAssignment(node, sourceType, i, elementType, contextualMapper);
}
return sourceType;
}

function checkArrayLiteralDestructuringElementAssignment(node: ArrayLiteralExpression, sourceType: Type,
elementIndex: number, elementType: Type, contextualMapper?: TypeMapper) {
const elements = node.elements;
const element = elements[elementIndex];
if (element.kind !== SyntaxKind.OmittedExpression) {
Copy link
Member

Choose a reason for hiding this comment

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

Does this implicitly return undefined? Invert the conditions here to reduce nesting and make the returns explicit.

if (element.kind !== SyntaxKind.SpreadElementExpression) {
const propName = "" + elementIndex;
const type = isTypeAny(sourceType)
? sourceType
: isTupleLikeType(sourceType)
? getTypeOfPropertyOfType(sourceType, propName)
: elementType;
if (type) {
return checkDestructuringAssignment(element, type, contextualMapper);
}
else {
if (isTupleType(sourceType)) {
error(element, Diagnostics.Tuple_type_0_with_length_1_cannot_be_assigned_to_tuple_with_length_2, typeToString(sourceType), (<TupleType>sourceType).elementTypes.length, elements.length);
}
else {
if (isTupleType(sourceType)) {
error(e, Diagnostics.Tuple_type_0_with_length_1_cannot_be_assigned_to_tuple_with_length_2, typeToString(sourceType), (<TupleType>sourceType).elementTypes.length, elements.length);
}
else {
error(e, Diagnostics.Type_0_has_no_property_1, typeToString(sourceType), propName);
}
error(element, Diagnostics.Type_0_has_no_property_1, typeToString(sourceType), propName);
}
}
}
else {
if (elementIndex < elements.length - 1) {
error(element, Diagnostics.A_rest_element_must_be_last_in_an_array_destructuring_pattern);
}
else {
if (i < elements.length - 1) {
error(e, Diagnostics.A_rest_element_must_be_last_in_an_array_destructuring_pattern);
const restExpression = (<SpreadElementExpression>element).expression;
if (restExpression.kind === SyntaxKind.BinaryExpression && (<BinaryExpression>restExpression).operatorToken.kind === SyntaxKind.EqualsToken) {
error((<BinaryExpression>restExpression).operatorToken, Diagnostics.A_rest_element_cannot_have_an_initializer);
}
else {
const restExpression = (<SpreadElementExpression>e).expression;
if (restExpression.kind === SyntaxKind.BinaryExpression && (<BinaryExpression>restExpression).operatorToken.kind === SyntaxKind.EqualsToken) {
error((<BinaryExpression>restExpression).operatorToken, Diagnostics.A_rest_element_cannot_have_an_initializer);
}
else {
checkDestructuringAssignment(restExpression, createArrayType(elementType), contextualMapper);
}
return checkDestructuringAssignment(restExpression, createArrayType(elementType), contextualMapper);
}
}
}
}
return sourceType;
return undefined;
}

function checkDestructuringAssignment(exprOrAssignment: Expression | ShorthandPropertyAssignment, sourceType: Type, contextualMapper?: TypeMapper): Type {
Expand Down Expand Up @@ -16549,6 +16561,40 @@ namespace ts {
return unknownType;
}

// Gets the type of object literal or array literal of destructuring assignment.
// { a } from
// for ( { a } of elems) {
// }
// [ a ] from
// [a] = [ some array ...]
function getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(expr: Expression): Type {
Copy link
Member

Choose a reason for hiding this comment

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

Is expr the initializer? Or the assignment expression? Can you document that explicit with a comment and potentially using a slightly more precise type than Expression?

Copy link
Contributor

Choose a reason for hiding this comment

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

is not this the same as getTypeFromBindingPattern

Copy link
Member Author

Choose a reason for hiding this comment

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

Its kind of higher level and gets the type of binding patterns object literal or array literal from binding pattern assignment. GetTypeFromBindingPattern needs BindingPattern. These are object literals and array literals from assignment so different node kinds

Copy link
Contributor

Choose a reason for hiding this comment

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

we should then change the entry point to return a targetSymbol for a bindingPattern instead of a type.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mhegazy done.

// If this is from "for of"
// for ( { a } of elems) {
// }
if (expr.parent.kind === SyntaxKind.ForOfStatement) {
const iteratedType = checkRightHandSideOfForOf((<ForOfStatement>expr.parent).expression);
return checkDestructuringAssignment(expr, iteratedType || unknownType);
}
// If this is from "for" initializer
// for ({a } = elems[0];.....) { }
if (expr.parent.kind === SyntaxKind.BinaryExpression) {
const iteratedType = checkExpression((<BinaryExpression>expr.parent).right);
return checkDestructuringAssignment(expr, iteratedType || unknownType);
}
// If this is from nested object binding pattern
// for ({ skills: { primary, secondary } } = multiRobot, i = 0; i < 1; i++) {
if (expr.parent.kind === SyntaxKind.PropertyAssignment) {
const typeOfParentObjectLiteral = getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(<Expression>expr.parent.parent);
return checkObjectLiteralDestructuringPropertyAssignment(typeOfParentObjectLiteral || unknownType, <ObjectLiteralElement>expr.parent);
}
// Array literal assignment - array destructuring pattern
Debug.assert(expr.parent.kind === SyntaxKind.ArrayLiteralExpression);
// [{ property1: p1, property2 }] = elems;
const typeOfArrayLiteral = getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(<Expression>expr.parent);
const elementType = checkIteratedTypeOrElementType(typeOfArrayLiteral || unknownType, expr.parent, /*allowStringInput*/ false) || unknownType;
return checkArrayLiteralDestructuringElementAssignment(<ArrayLiteralExpression>expr.parent, typeOfArrayLiteral,
indexOf((<ArrayLiteralExpression>expr.parent).elements, expr), elementType || unknownType);
}

function getTypeOfExpression(expr: Expression): Type {
if (isRightSideOfQualifiedNameOrPropertyAccess(expr)) {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1735,6 +1735,7 @@ namespace ts {
getSymbolsOfParameterPropertyDeclaration(parameter: ParameterDeclaration, parameterName: string): Symbol[];
getShorthandAssignmentValueSymbol(location: Node): Symbol;
getExportSpecifierLocalTargetSymbol(location: ExportSpecifier): Symbol;
getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(expression: Expression): Type;
getTypeAtLocation(node: Node): Type;
typeToString(type: Type, enclosingDeclaration?: Node, flags?: TypeFormatFlags): string;
symbolToString(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags): string;
Expand Down
Loading