From ad916ab05dcab990388bbc2f322eae7ed26fdbc0 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 7 Apr 2016 15:41:42 -0700 Subject: [PATCH] Handles when property is renamed and is also part of destructuring assignment Handles destructuring assignment part of #6312 --- src/compiler/checker.ts | 148 +++++++++++------- src/compiler/types.ts | 1 + src/services/services.ts | 34 +++- ...lRefsObjectBindingElementPropertyName06.ts | 2 +- .../renameDestructuringAssignmentInFor.ts | 20 +++ .../renameDestructuringAssignmentInForOf.ts | 20 +++ ...ructuringAssignmentNestedInArrayLiteral.ts | 15 ++ ...ucturingAssignmentNestedInArrayLiteral2.ts | 15 ++ ...enameDestructuringAssignmentNestedInFor.ts | 22 +++ ...nameDestructuringAssignmentNestedInFor2.ts | 23 +++ ...ameDestructuringAssignmentNestedInForOf.ts | 22 +++ ...meDestructuringAssignmentNestedInForOf2.ts | 23 +++ 12 files changed, 288 insertions(+), 57 deletions(-) create mode 100644 tests/cases/fourslash/renameDestructuringAssignmentInFor.ts create mode 100644 tests/cases/fourslash/renameDestructuringAssignmentInForOf.ts create mode 100644 tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral.ts create mode 100644 tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral2.ts create mode 100644 tests/cases/fourslash/renameDestructuringAssignmentNestedInFor.ts create mode 100644 tests/cases/fourslash/renameDestructuringAssignmentNestedInFor2.ts create mode 100644 tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf.ts create mode 100644 tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf2.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 6d21a9b5e3f..0597fd7893f 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -83,6 +83,7 @@ namespace ts { getShorthandAssignmentValueSymbol, getExportSpecifierLocalTargetSymbol, getTypeAtLocation: getTypeOfNode, + getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment, typeToString, getSymbolDisplayBuilder, symbolToString, @@ -11690,39 +11691,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 = (p).name; - if (name.kind === SyntaxKind.ComputedPropertyName) { - checkComputedPropertyName(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(p, type); - } - else { - // non-shorthand property assignments should always have initializers - checkDestructuringAssignment((p).initializer, type); - } + function checkObjectLiteralDestructuringPropertyAssignment(objectLiteralType: Type, property: ObjectLiteralElement, contextualMapper?: TypeMapper) { + if (property.kind === SyntaxKind.PropertyAssignment || property.kind === SyntaxKind.ShorthandPropertyAssignment) { + const name = (property).name; + if (name.kind === SyntaxKind.ComputedPropertyName) { + checkComputedPropertyName(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(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((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 { @@ -11732,44 +11737,49 @@ 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); - } - else { - if (isTupleType(sourceType)) { - error(e, Diagnostics.Tuple_type_0_with_length_1_cannot_be_assigned_to_tuple_with_length_2, typeToString(sourceType), (sourceType).elementTypes.length, elements.length); - } - else { - error(e, Diagnostics.Type_0_has_no_property_1, typeToString(sourceType), propName); - } - } + checkArrayLiteralDestructuringElementAssignment(node, sourceType, elements[i], i, elementType, contextualMapper); + } + return sourceType; + } + + function checkArrayLiteralDestructuringElementAssignment(node: ArrayLiteralExpression, sourceType: Type, + element: Expression, index: number, elementType: Type, contextualMapper?: TypeMapper) { + const elements = node.elements; + if (element.kind !== SyntaxKind.OmittedExpression) { + if (element.kind !== SyntaxKind.SpreadElementExpression) { + const propName = "" + index; + const type = isTypeAny(sourceType) + ? sourceType + : isTupleLikeType(sourceType) + ? getTypeOfPropertyOfType(sourceType, propName) + : elementType; + if (type) { + return checkDestructuringAssignment(element, type, contextualMapper); } else { - if (i < elements.length - 1) { - error(e, Diagnostics.A_rest_element_must_be_last_in_an_array_destructuring_pattern); + if (isTupleType(sourceType)) { + error(element, Diagnostics.Tuple_type_0_with_length_1_cannot_be_assigned_to_tuple_with_length_2, typeToString(sourceType), (sourceType).elementTypes.length, elements.length); } else { - const restExpression = (e).expression; - if (restExpression.kind === SyntaxKind.BinaryExpression && (restExpression).operatorToken.kind === SyntaxKind.EqualsToken) { - error((restExpression).operatorToken, Diagnostics.A_rest_element_cannot_have_an_initializer); - } - else { - checkDestructuringAssignment(restExpression, createArrayType(elementType), contextualMapper); - } + error(element, Diagnostics.Type_0_has_no_property_1, typeToString(sourceType), propName); + } + } + } + else { + if (index < elements.length - 1) { + error(element, Diagnostics.A_rest_element_must_be_last_in_an_array_destructuring_pattern); + } + else { + const restExpression = (element).expression; + if (restExpression.kind === SyntaxKind.BinaryExpression && (restExpression).operatorToken.kind === SyntaxKind.EqualsToken) { + error((restExpression).operatorToken, Diagnostics.A_rest_element_cannot_have_an_initializer); + } + else { + return checkDestructuringAssignment(restExpression, createArrayType(elementType), contextualMapper); } } } } - return sourceType; } function checkDestructuringAssignment(exprOrAssignment: Expression | ShorthandPropertyAssignment, sourceType: Type, contextualMapper?: TypeMapper): Type { @@ -16431,6 +16441,34 @@ namespace ts { return unknownType; } + function getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(expr: Expression): Type { + // If this is from "for of" + // for ( { a } of elemns) { + // } + if (expr.parent.kind === SyntaxKind.ForOfStatement) { + const iteratedType = checkRightHandSideOfForOf((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((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(expr.parent.parent); + return checkObjectLiteralDestructuringPropertyAssignment(typeOfParentObjectLiteral, expr.parent); + } + // Array literal assignment - array destructuring pattern + Debug.assert(expr.parent.kind === SyntaxKind.ArrayLiteralExpression); + // [{ property1: p1, property2 }] = elems; + const typeOfArrayLiteral = getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(expr.parent); + const elementType = checkIteratedTypeOrElementType(typeOfArrayLiteral, expr.parent, /*allowStringInput*/ false) || unknownType; + return checkArrayLiteralDestructuringElementAssignment(expr.parent, typeOfArrayLiteral, + expr, indexOf((expr.parent).elements, expr), elementType); + } function getTypeOfExpression(expr: Expression): Type { if (isRightSideOfQualifiedNameOrPropertyAccess(expr)) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 3bc3d1ad6a1..8807495675e 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -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; diff --git a/src/services/services.ts b/src/services/services.ts index 05d44a0b3d5..af597b2141b 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -6087,6 +6087,19 @@ namespace ts { // The search set contains at least the current symbol let result = [symbol]; + // If the location is name of property symbol from object literal destructuring pattern + // Search the property symbol + // for ( { property: p2 } of elems) { } + if (isNameOfPropertyAssignment(location) && + location.parent.kind !== SyntaxKind.ShorthandPropertyAssignment && + isArrayLiteralOrObjectLiteralDestructuringPattern(location.parent.parent)) { + const typeOfObjectLiteral = typeChecker.getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(location.parent.parent); + if (typeOfObjectLiteral) { + const propertySymbol = typeChecker.getPropertyOfType(typeOfObjectLiteral, (location).text); + result.push(propertySymbol); + } + } + // If the symbol is an alias, add what it aliases to the list // import {a} from "mod"; // export {a} @@ -6234,13 +6247,32 @@ namespace ts { return getRelatedSymbol(searchSymbols, aliasedSymbol, referenceLocation); } + // If the reference location is in an object literal, try to get the contextual type for the // object literal, lookup the property symbol in the contextual type, and use this symbol to // compare to our searchSymbol if (isNameOfPropertyAssignment(referenceLocation)) { - return forEach(getPropertySymbolsFromContextualType(referenceLocation), contextualSymbol => { + const contexualSymbol = forEach(getPropertySymbolsFromContextualType(referenceLocation), contextualSymbol => { return forEach(typeChecker.getRootSymbols(contextualSymbol), s => searchSymbols.indexOf(s) >= 0 ? s : undefined); }); + + if (contexualSymbol) { + return contexualSymbol; + } + + // If the reference location is name of property symbol from object literal destructuring pattern + // Compare it to search symbol something similar to 'property' from + // for ( { property: p2 } of elems) { } + if (isArrayLiteralOrObjectLiteralDestructuringPattern(referenceLocation.parent.parent)) { + // Do work to determine if this is property symbol corresponding to the search symbol + const typeOfObjectLiteral = typeChecker.getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(referenceLocation.parent.parent); + if (typeOfObjectLiteral) { + const propertySymbol = typeChecker.getPropertyOfType(typeOfObjectLiteral, (referenceLocation).text); + if (searchSymbols.indexOf(propertySymbol) >= 0) { + return propertySymbol; + } + } + } } // If the reference location is the binding element and doesn't have property name diff --git a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts index d416b22027b..c0b940d4380 100644 --- a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts +++ b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts @@ -13,7 +13,7 @@ ////for (var { [|property1|]: p1 } of elems) { ////} ////var p2; -////for ({ /*This should be referenced too*/property1 : p2 } of elems) { +////for ({ [|property1|] : p2 } of elems) { ////} // Note: if this test ever changes, consider updating diff --git a/tests/cases/fourslash/renameDestructuringAssignmentInFor.ts b/tests/cases/fourslash/renameDestructuringAssignmentInFor.ts new file mode 100644 index 00000000000..6be57b81fa2 --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringAssignmentInFor.ts @@ -0,0 +1,20 @@ +/// + +////interface I { +//// /*1*/[|property1|]: number; +//// property2: string; +////} +////var elems: I[]; +//// +////var p2: number, property1: number; +////for ({ [|property1|] } = elems[0]; p2 < 100; p2++) { +//// p2 = property1++; +////} +////for ({ /*2*/[|property1|]: p2 } = elems[0]; p2 < 100; p2++) { +////} + +goTo.marker("1"); +verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); + +goTo.marker("2"); +verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); diff --git a/tests/cases/fourslash/renameDestructuringAssignmentInForOf.ts b/tests/cases/fourslash/renameDestructuringAssignmentInForOf.ts new file mode 100644 index 00000000000..d965875d259 --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringAssignmentInForOf.ts @@ -0,0 +1,20 @@ +/// + +////interface I { +//// /*1*/[|property1|]: number; +//// property2: string; +////} +////var elems: I[]; +//// +////var property1: number, p2: number; +////for ({ [|property1|] } of elems) { +//// property1++; +////} +////for ({ /*2*/[|property1|]: p2 } of elems) { +////} + +goTo.marker("1"); +verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); + +goTo.marker("2"); +verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); \ No newline at end of file diff --git a/tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral.ts b/tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral.ts new file mode 100644 index 00000000000..9d5d2a041e5 --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral.ts @@ -0,0 +1,15 @@ +/// + +////interface I { +//// /*1*/[|property1|]: number; +//// property2: string; +////} +////var elems: I[], p1: number, property1: number; +////[{ /*2*/[|property1|]: p1 }] = elems; +////[{ [|property1|] }] = elems; + +goTo.marker("1"); +verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); + +goTo.marker("2"); +verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); diff --git a/tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral2.ts b/tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral2.ts new file mode 100644 index 00000000000..c4db2de512f --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringAssignmentNestedInArrayLiteral2.ts @@ -0,0 +1,15 @@ +/// + +////interface I { +//// property1: number; +//// property2: string; +////} +////var elems: I[], p1: number, [|property1|]: number; +////[{ property1: p1 }] = elems; +////[{ [|property1|] }] = elems; + +let ranges = test.ranges() +for (let range of ranges) { + goTo.position(range.start); + verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); +} diff --git a/tests/cases/fourslash/renameDestructuringAssignmentNestedInFor.ts b/tests/cases/fourslash/renameDestructuringAssignmentNestedInFor.ts new file mode 100644 index 00000000000..e603c82dbd7 --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringAssignmentNestedInFor.ts @@ -0,0 +1,22 @@ +/// + +////interface MultiRobot { +//// name: string; +//// skills: { +//// /*1*/[|primary|]: string; +//// secondary: string; +//// }; +////} +////let multiRobot: MultiRobot; +////for ({ skills: { /*2*/[|primary|]: primaryA, secondary: secondaryA } } = multiRobot, i = 0; i < 1; i++) { +//// console.log(primaryA); +////} +////for ({ skills: { [|primary|], secondary } } = multiRobot, i = 0; i < 1; i++) { +//// console.log(primary); +////} + +goTo.marker("1"); +verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); + +goTo.marker("2"); +verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); diff --git a/tests/cases/fourslash/renameDestructuringAssignmentNestedInFor2.ts b/tests/cases/fourslash/renameDestructuringAssignmentNestedInFor2.ts new file mode 100644 index 00000000000..89dc899c5bf --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringAssignmentNestedInFor2.ts @@ -0,0 +1,23 @@ +/// + +////interface MultiRobot { +//// name: string; +//// skills: { +//// primary: string; +//// secondary: string; +//// }; +////} +////let multiRobot: MultiRobot, [|primary|]: string; +////for ({ skills: { primary: primaryA, secondary: secondaryA } } = multiRobot, i = 0; i < 1; i++) { +//// console.log(primaryA); +////} +////for ({ skills: { [|primary|], secondary } } = multiRobot, i = 0; i < 1; i++) { +//// console.log([|primary|]); +////} + +let ranges = test.ranges() +for (let range of ranges) { + goTo.position(range.start); + verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); +} + diff --git a/tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf.ts b/tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf.ts new file mode 100644 index 00000000000..bef88d201d4 --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf.ts @@ -0,0 +1,22 @@ +/// + +////interface MultiRobot { +//// name: string; +//// skills: { +//// /*1*/[|primary|]: string; +//// secondary: string; +//// }; +////} +////let multiRobots: MultiRobot[]; +////for ({ skills: { /*2*/[|primary|]: primaryA, secondary: secondaryA } } of multiRobots) { +//// console.log(primaryA); +////} +////for ({ skills: { [|primary|], secondary } } of multiRobots) { +//// console.log(primary); +////} + +goTo.marker("1"); +verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); + +goTo.marker("2"); +verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); diff --git a/tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf2.ts b/tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf2.ts new file mode 100644 index 00000000000..b684e6b6a81 --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringAssignmentNestedInForOf2.ts @@ -0,0 +1,23 @@ +/// + +////interface MultiRobot { +//// name: string; +//// skills: { +//// primary: string; +//// secondary: string; +//// }; +////} +////let multiRobots: MultiRobot[], [|primary|]: string; +////for ({ skills: { primary: primaryA, secondary: secondaryA } } of multiRobots) { +//// console.log(primaryA); +////} +////for ({ skills: { [|primary|], secondary } } of multiRobots) { +//// console.log([|primary|]); +////} + + +let ranges = test.ranges() +for (let range of ranges) { + goTo.position(range.start); + verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); +}