From ace2285ac2913a4544512058e829973734d32f63 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 5 Apr 2016 16:16:54 -0700 Subject: [PATCH 01/16] Destructuring of object binding pattern element without property name should reference property Fixes #6312 --- src/services/services.ts | 36 +++++++++++++++++++ ...lRefsObjectBindingElementPropertyName03.ts | 2 +- ...lRefsObjectBindingElementPropertyName04.ts | 6 ++-- ...lRefsObjectBindingElementPropertyName06.ts | 5 +-- ...lRefsObjectBindingElementPropertyName09.ts | 6 ++-- .../renameDestructuringClassProperty.ts | 23 ++++++++++++ .../renameDestructuringFunctionParameter.ts | 10 ++++++ 7 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 tests/cases/fourslash/renameDestructuringClassProperty.ts create mode 100644 tests/cases/fourslash/renameDestructuringFunctionParameter.ts diff --git a/src/services/services.ts b/src/services/services.ts index a634b2baf1a49..731db03628972 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -5615,6 +5615,21 @@ namespace ts { return (symbol.flags & SymbolFlags.Alias) && !!getDeclarationOfKind(symbol, SyntaxKind.ImportSpecifier); } + function isObjectBindingPatternElementWithoutPropertyName(symbol: Symbol) { + const bindingElement = getDeclarationOfKind(symbol, SyntaxKind.BindingElement); + return bindingElement && + bindingElement.parent.kind === SyntaxKind.ObjectBindingPattern && + !bindingElement.propertyName; + } + + function getPropertySymbolIfObjectBindingPatternWithoutPropertyName(symbol: Symbol) { + if (isObjectBindingPatternElementWithoutPropertyName(symbol)) { + const bindingElement = getDeclarationOfKind(symbol, SyntaxKind.BindingElement); + const typeOfPattern = typeChecker.getTypeAtLocation(bindingElement.parent); + return typeOfPattern && typeChecker.getPropertyOfType(typeOfPattern, (bindingElement.name).text); + } + } + function getInternedName(symbol: Symbol, location: Node, declarations: Declaration[]): string { // If this is an export or import specifier it could have been renamed using the 'as' syntax. // If so we want to search for whatever under the cursor. @@ -5660,6 +5675,12 @@ namespace ts { return undefined; } + // If symbol is of object binding pattern element without property name we would want to + // look for property too and that could be anywhere + if (isObjectBindingPatternElementWithoutPropertyName(symbol)) { + return undefined; + } + // if this symbol is visible from its parent container, e.g. exported, then bail out // if symbol correspond to the union property - bail out if (symbol.parent || (symbol.flags & SymbolFlags.SyntheticProperty)) { @@ -6103,6 +6124,13 @@ namespace ts { result = result.concat(typeChecker.getSymbolsOfParameterPropertyDeclaration(symbol.valueDeclaration, symbol.name)); } + // If this is symbol of binding element without propertyName declaration in Object binding pattern + // Include the property in the search + const bindingElementPropertySymbol = getPropertySymbolIfObjectBindingPatternWithoutPropertyName(symbol); + if (bindingElementPropertySymbol) { + result.push(bindingElementPropertySymbol); + } + // If this is a union property, add all the symbols from all its source symbols in all unioned types. // If the symbol is an instantiation from a another symbol (e.g. widened symbol) , add the root the list forEach(typeChecker.getRootSymbols(symbol), rootSymbol => { @@ -6212,6 +6240,14 @@ namespace ts { }); } + // If the reference location is the binding element and doesn't have property name + // then include the binding element in the related symbols + // let { a } : { a }; + const bindingElementPropertySymbol = getPropertySymbolIfObjectBindingPatternWithoutPropertyName(referenceSymbol); + if (bindingElementPropertySymbol && searchSymbols.indexOf(bindingElementPropertySymbol) >= 0) { + return bindingElementPropertySymbol; + } + // Unwrap symbols to get to the root (e.g. transient symbols as a result of widening) // Or a union property, use its underlying unioned symbols return forEach(typeChecker.getRootSymbols(referenceSymbol), rootSymbol => { diff --git a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName03.ts b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName03.ts index 304fa9e42e9ad..8dc2b1e7bb851 100644 --- a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName03.ts +++ b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName03.ts @@ -6,7 +6,7 @@ ////} //// ////var foo: I; -////var [{ [|property1|]: prop1 }, { property1, property2 } ] = [foo, foo]; +////var [{ [|property1|]: prop1 }, { [|property1|], property2 } ] = [foo, foo]; let ranges = test.ranges(); for (let range of ranges) { diff --git a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName04.ts b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName04.ts index bdb37525f7189..dfa0997774ef0 100644 --- a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName04.ts +++ b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName04.ts @@ -6,14 +6,12 @@ ////} //// ////function f({ /**/[|property1|]: p1 }: I, -//// { /*SHOULD_BE_A_REFERENCE*/property1 }: I, +//// { [|property1|] }: I, //// { property1: p2 }) { //// -//// return property1 + 1; +//// return [|property1|] + 1; ////} -// NOTE: In the future, the identifier at -// SHOULD_BE_A_REFERENCE should be in the set of ranges. goTo.marker(); let ranges = test.ranges(); diff --git a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts index 67c7029861ecb..d416b22027b7d 100644 --- a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts +++ b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts @@ -8,12 +8,12 @@ ////var elems: I[]; ////for (let { [|property1|]: p } of elems) { ////} -////for (let { property1 } of elems) { +////for (let { [|property1|] } of elems) { ////} ////for (var { [|property1|]: p1 } of elems) { ////} ////var p2; -////for ({ property1 : p2 } of elems) { +////for ({ /*This should be referenced too*/property1 : p2 } of elems) { ////} // Note: if this test ever changes, consider updating @@ -23,6 +23,7 @@ let ranges = test.ranges(); for (let range of ranges) { goTo.position(range.start); + debugger; verify.referencesCountIs(ranges.length); for (let expectedRange of ranges) { verify.referencesAtPositionContains(expectedRange); diff --git a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName09.ts b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName09.ts index e45359bcf1cc8..0b82c73e31d51 100644 --- a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName09.ts +++ b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName09.ts @@ -1,19 +1,17 @@ /// ////interface I { -//// /*SHOULD_BE_A_REFERENCE1*/property1: number; +//// [|property1|]: number; //// property2: string; ////} //// -////function f({ /*SHOULD_BE_A_REFERENCE2*/property1: p1 }: I, +////function f({ [|property1|]: p1 }: I, //// { /**/[|property1|] }: I, //// { property1: p2 }) { //// //// return [|property1|] + 1; ////} -// NOTE: In the future, the identifiers at -// SHOULD_BE_A_REFERENCE[1/2] should be in the set of ranges. goTo.marker(); let ranges = test.ranges(); diff --git a/tests/cases/fourslash/renameDestructuringClassProperty.ts b/tests/cases/fourslash/renameDestructuringClassProperty.ts new file mode 100644 index 0000000000000..acc58f999c4be --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringClassProperty.ts @@ -0,0 +1,23 @@ +/// + +////class A { +//// [|foo|]: string; +////} +////class B { +//// syntax1(a: A): void { +//// let { [|foo|] } = a; +//// } +//// syntax2(a: A): void { +//// let { [|foo|]: foo } = a; +//// } +//// syntax11(a: A): void { +//// let { [|foo|] } = a; +//// [|foo|] = "newString"; +//// } +////} + +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/renameDestructuringFunctionParameter.ts b/tests/cases/fourslash/renameDestructuringFunctionParameter.ts new file mode 100644 index 0000000000000..d1df0f5827531 --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringFunctionParameter.ts @@ -0,0 +1,10 @@ +/// + +////function f({[|a|]}: {[|a|]}) { +//// f({[|a|]}); +////} +let ranges = test.ranges(); +for (let range of ranges) { + goTo.position(range.start); + verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); +} From 01ca100d1652f94face35edcdfa4b2193380eef2 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 6 Apr 2016 14:24:20 -0700 Subject: [PATCH 02/16] Include the target symbol in search if location is propertyName or the import or export specifier dont specify "as" clause Handles #7708 --- src/services/services.ts | 61 ++++++++++--------- .../renameImportAndExportInDiffFiles.ts | 18 ++++++ 2 files changed, 50 insertions(+), 29 deletions(-) create mode 100644 tests/cases/fourslash/renameImportAndExportInDiffFiles.ts diff --git a/src/services/services.ts b/src/services/services.ts index 731db03628972..05d44a0b3d58b 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -5611,8 +5611,19 @@ namespace ts { }; } - function isImportSpecifierSymbol(symbol: Symbol) { - return (symbol.flags & SymbolFlags.Alias) && !!getDeclarationOfKind(symbol, SyntaxKind.ImportSpecifier); + function getImportOrExportSpecifierPropertyNameSymbolSpecifier(symbol: Symbol, location: Node): ImportOrExportSpecifier { + if (symbol.flags & SymbolFlags.Alias) { + const importOrExportSpecifier = forEach(symbol.declarations, + declaration => (declaration.kind === SyntaxKind.ImportSpecifier || + declaration.kind === SyntaxKind.ExportSpecifier) ? declaration : undefined); + if (importOrExportSpecifier && + // export { a } + (!importOrExportSpecifier.propertyName || + // export {a as class } where a is location + importOrExportSpecifier.propertyName === location)) { + return importOrExportSpecifier; + } + } } function isObjectBindingPatternElementWithoutPropertyName(symbol: Symbol) { @@ -6077,17 +6088,19 @@ namespace ts { let result = [symbol]; // If the symbol is an alias, add what it aliases to the list - if (isImportSpecifierSymbol(symbol)) { - result.push(typeChecker.getAliasedSymbol(symbol)); - } - - // For export specifiers, the exported name can be referring to a local symbol, e.g.: // import {a} from "mod"; - // export {a as somethingElse} - // We want the *local* declaration of 'a' as declared in the import, - // *not* as declared within "mod" (or farther) - if (location.parent.kind === SyntaxKind.ExportSpecifier) { - result.push(typeChecker.getExportSpecifierLocalTargetSymbol(location.parent)); + // export {a} + //// For export specifiers, the exported name can be referring to a local symbol, e.g.: + //// import {a} from "mod"; + //// export {a as somethingElse} + //// We want the *local* declaration of 'a' as declared in the import, + //// *not* as declared within "mod" (or farther) + const importOrExportSpecifier = getImportOrExportSpecifierPropertyNameSymbolSpecifier(symbol, location); + if (importOrExportSpecifier) { + result = result.concat(populateSearchSymbolSet( + importOrExportSpecifier.kind === SyntaxKind.ImportSpecifier ? + typeChecker.getAliasedSymbol(symbol) : + typeChecker.getExportSpecifierLocalTargetSymbol(importOrExportSpecifier), location)); } // If the location is in a context sensitive location (i.e. in an object literal) try @@ -6212,23 +6225,13 @@ namespace ts { } // If the reference symbol is an alias, check if what it is aliasing is one of the search - // symbols. - if (isImportSpecifierSymbol(referenceSymbol)) { - const aliasedSymbol = typeChecker.getAliasedSymbol(referenceSymbol); - if (searchSymbols.indexOf(aliasedSymbol) >= 0) { - return aliasedSymbol; - } - } - - // For export specifiers, it can be a local symbol, e.g. - // import {a} from "mod"; - // export {a as somethingElse} - // We want the local target of the export (i.e. the import symbol) and not the final target (i.e. "mod".a) - if (referenceLocation.parent.kind === SyntaxKind.ExportSpecifier) { - const aliasedSymbol = typeChecker.getExportSpecifierLocalTargetSymbol(referenceLocation.parent); - if (searchSymbols.indexOf(aliasedSymbol) >= 0) { - return aliasedSymbol; - } + // symbols but by looking up for related symbol of this alias so it can handle multiple level of indirectness. + const importOrExportSpecifier = getImportOrExportSpecifierPropertyNameSymbolSpecifier(referenceSymbol, referenceLocation); + if (importOrExportSpecifier) { + const aliasedSymbol = importOrExportSpecifier.kind === SyntaxKind.ImportSpecifier ? + typeChecker.getAliasedSymbol(referenceSymbol) : + typeChecker.getExportSpecifierLocalTargetSymbol(importOrExportSpecifier); + return getRelatedSymbol(searchSymbols, aliasedSymbol, referenceLocation); } // If the reference location is in an object literal, try to get the contextual type for the diff --git a/tests/cases/fourslash/renameImportAndExportInDiffFiles.ts b/tests/cases/fourslash/renameImportAndExportInDiffFiles.ts new file mode 100644 index 0000000000000..7b09872119695 --- /dev/null +++ b/tests/cases/fourslash/renameImportAndExportInDiffFiles.ts @@ -0,0 +1,18 @@ +/// + +// @Filename: a.ts +////export var /*1*/a; + +// @Filename: b.ts +////import { /*2*/a } from './a'; +////export { /*3*/a }; + +goTo.file("a.ts"); +goTo.marker("1"); + +goTo.file("b.ts"); +goTo.marker("2"); +verify.referencesCountIs(3); + +goTo.marker("3"); +verify.referencesCountIs(3); \ No newline at end of file From 168d10642155c3b4bab86a8311322a049e60cd0d Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 7 Apr 2016 10:47:57 -0700 Subject: [PATCH 03/16] Test cases for destructuring declarations in "for" and "for of" loops --- .../renameDestructuringDeclarationInFor.ts | 20 +++++++++++++++++++ .../renameDestructuringDeclarationInForOf.ts | 19 ++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 tests/cases/fourslash/renameDestructuringDeclarationInFor.ts create mode 100644 tests/cases/fourslash/renameDestructuringDeclarationInForOf.ts diff --git a/tests/cases/fourslash/renameDestructuringDeclarationInFor.ts b/tests/cases/fourslash/renameDestructuringDeclarationInFor.ts new file mode 100644 index 0000000000000..0edf90850924c --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringDeclarationInFor.ts @@ -0,0 +1,20 @@ +/// + +////interface I { +//// [|property1|]: number; +//// property2: string; +////} +////var elems: I[]; +//// +////var p2: number, property1: number; +////for (let { [|property1|]: p2 } = elems[0]; p2 < 100; p2++) { +////} +////for (let { [|property1|] } = elems[0]; p2 < 100; p2++) { +//// [|property1|] = p2; +////} + +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/renameDestructuringDeclarationInForOf.ts b/tests/cases/fourslash/renameDestructuringDeclarationInForOf.ts new file mode 100644 index 0000000000000..1c2b04b7ab59d --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringDeclarationInForOf.ts @@ -0,0 +1,19 @@ +/// + +////interface I { +//// [|property1|]: number; +//// property2: string; +////} +////var elems: I[]; +//// +////for (let { [|property1|] } of elems) { +//// [|property1|]++; +////} +////for (let { [|property1|]: p2 } of elems) { +////} + +let ranges = test.ranges() +for (let range of ranges) { + goTo.position(range.start); + verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); +} From 6d43c0279611cfdae00fa75448426a51d8f74071 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 7 Apr 2016 10:48:21 -0700 Subject: [PATCH 04/16] Test cases when var is renamed and used in destructuring --- .../renameDestructuringAssignmentInFor2.ts | 20 +++++++++++++++++++ .../renameDestructuringAssignmentInForOf2.ts | 20 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 tests/cases/fourslash/renameDestructuringAssignmentInFor2.ts create mode 100644 tests/cases/fourslash/renameDestructuringAssignmentInForOf2.ts diff --git a/tests/cases/fourslash/renameDestructuringAssignmentInFor2.ts b/tests/cases/fourslash/renameDestructuringAssignmentInFor2.ts new file mode 100644 index 0000000000000..ca75e42394b71 --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringAssignmentInFor2.ts @@ -0,0 +1,20 @@ +/// + +////interface I { +//// property1: number; +//// property2: string; +////} +////var elems: I[]; +//// +////var p2: number, [|property1|]: number; +////for ({ [|property1|] } = elems[0]; p2 < 100; p2++) { +//// p2 = [|property1|]++; +////} +////for ({ property1: p2 } = elems[0]; p2 < 100; p2++) { +////} + +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/renameDestructuringAssignmentInForOf2.ts b/tests/cases/fourslash/renameDestructuringAssignmentInForOf2.ts new file mode 100644 index 0000000000000..401b6776d2ec3 --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringAssignmentInForOf2.ts @@ -0,0 +1,20 @@ +/// + +////interface I { +//// property1: number; +//// property2: string; +////} +////var elems: I[]; +//// +////var [|property1|]: number, p2: number; +////for ({ [|property1|] } of elems) { +//// [|property1|]++; +////} +////for ({ property1: p2 } of elems) { +////} + +let ranges = test.ranges() +for (let range of ranges) { + goTo.position(range.start); + verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); +} From ad916ab05dcab990388bbc2f322eae7ed26fdbc0 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 7 Apr 2016 15:41:42 -0700 Subject: [PATCH 05/16] Handles when property is renamed and is also part of destructuring assignment Handles destructuring assignment part of #6312 --- src/compiler/checker.ts | 142 +++++++++++------- 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, 285 insertions(+), 54 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 6d21a9b5e3ffc..0597fd7893fa0 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); + 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 (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 { - 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); - } + 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 { - if (i < elements.length - 1) { - error(e, Diagnostics.A_rest_element_must_be_last_in_an_array_destructuring_pattern); + 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 { - 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); - } + 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 3bc3d1ad6a178..8807495675e77 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 05d44a0b3d58b..af597b2141bdf 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 d416b22027b7d..c0b940d43804f 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 0000000000000..6be57b81fa281 --- /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 0000000000000..d965875d259d1 --- /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 0000000000000..9d5d2a041e501 --- /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 0000000000000..c4db2de512f37 --- /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 0000000000000..e603c82dbd710 --- /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 0000000000000..89dc899c5bf15 --- /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 0000000000000..bef88d201d493 --- /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 0000000000000..b684e6b6a8174 --- /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); +} From 2e44bccb36566596a15a7c1b9763ddb1b88afc7b Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 7 Apr 2016 15:45:45 -0700 Subject: [PATCH 06/16] Test case for nested binding element's rename --- ...renameDestructuringNestedBindingElement.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 tests/cases/fourslash/renameDestructuringNestedBindingElement.ts diff --git a/tests/cases/fourslash/renameDestructuringNestedBindingElement.ts b/tests/cases/fourslash/renameDestructuringNestedBindingElement.ts new file mode 100644 index 0000000000000..1c4254b14aef0 --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringNestedBindingElement.ts @@ -0,0 +1,22 @@ +/// + +////interface MultiRobot { +//// name: string; +//// skills: { +//// [|primary|]: string; +//// secondary: string; +//// }; +////} +////let multiRobots: MultiRobot[]; +////for (let { skills: {[|primary|]: primaryA, secondary: secondaryA } } of multiRobots) { +//// console.log(primaryA); +////} +////for (let { 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); +} From 92622bf715cea81f0a67df58c36b7e75d4d934a5 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 8 Apr 2016 10:05:22 -0700 Subject: [PATCH 07/16] Fix the git ignore so that browser test server is ignored --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index db8e29d903a1c..e1492c2cd73e8 100644 --- a/.gitignore +++ b/.gitignore @@ -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 From f7ca43917fe4739a0f48d8ea74a3904864aa5945 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 8 Apr 2016 10:19:02 -0700 Subject: [PATCH 08/16] Handle the rename locations for default import Handles #7024 --- src/services/services.ts | 11 +++++---- tests/cases/fourslash/renameDefaultImport.ts | 19 +++++++++++++++ .../renameDefaultImportDifferentName.ts | 23 +++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 tests/cases/fourslash/renameDefaultImport.ts create mode 100644 tests/cases/fourslash/renameDefaultImportDifferentName.ts diff --git a/src/services/services.ts b/src/services/services.ts index 2d27516956e91..cb062a0fc9f36 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -6116,15 +6116,18 @@ namespace ts { // If the symbol is an alias, add what it aliases to the list // import {a} from "mod"; // export {a} + // If the symbol is an alias to default declaration, add what it aliases to the list + // declare "mod" { export default class B { } } + // import B from "mod"; //// For export specifiers, the exported name can be referring to a local symbol, e.g.: //// import {a} from "mod"; //// export {a as somethingElse} //// We want the *local* declaration of 'a' as declared in the import, //// *not* as declared within "mod" (or farther) const importOrExportSpecifier = getImportOrExportSpecifierPropertyNameSymbolSpecifier(symbol, location); - if (importOrExportSpecifier) { + if (importOrExportSpecifier || getDeclarationOfKind(symbol, SyntaxKind.ImportClause)) { result = result.concat(populateSearchSymbolSet( - importOrExportSpecifier.kind === SyntaxKind.ImportSpecifier ? + !importOrExportSpecifier || importOrExportSpecifier.kind === SyntaxKind.ImportSpecifier ? typeChecker.getAliasedSymbol(symbol) : typeChecker.getExportSpecifierLocalTargetSymbol(importOrExportSpecifier), location)); } @@ -6253,8 +6256,8 @@ namespace ts { // If the reference symbol is an alias, check if what it is aliasing is one of the search // symbols but by looking up for related symbol of this alias so it can handle multiple level of indirectness. const importOrExportSpecifier = getImportOrExportSpecifierPropertyNameSymbolSpecifier(referenceSymbol, referenceLocation); - if (importOrExportSpecifier) { - const aliasedSymbol = importOrExportSpecifier.kind === SyntaxKind.ImportSpecifier ? + if (importOrExportSpecifier || getDeclarationOfKind(referenceSymbol, SyntaxKind.ImportClause)) { + const aliasedSymbol = !importOrExportSpecifier || importOrExportSpecifier.kind === SyntaxKind.ImportSpecifier ? typeChecker.getAliasedSymbol(referenceSymbol) : typeChecker.getExportSpecifierLocalTargetSymbol(importOrExportSpecifier); return getRelatedSymbol(searchSymbols, aliasedSymbol, referenceLocation); diff --git a/tests/cases/fourslash/renameDefaultImport.ts b/tests/cases/fourslash/renameDefaultImport.ts new file mode 100644 index 0000000000000..fd9534e65f706 --- /dev/null +++ b/tests/cases/fourslash/renameDefaultImport.ts @@ -0,0 +1,19 @@ +/// + +// @Filename: B.ts +////export default class [|B|] { +//// test() { +//// } +////} + +// @Filename: A.ts +////import [|B|] from "./B"; +////let b = new [|B|](); +////b.test(); + +let ranges = test.ranges() +for (let range of ranges) { + goTo.file(range.fileName); + goTo.position(range.start); + verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); +} diff --git a/tests/cases/fourslash/renameDefaultImportDifferentName.ts b/tests/cases/fourslash/renameDefaultImportDifferentName.ts new file mode 100644 index 0000000000000..5965f1a63e99e --- /dev/null +++ b/tests/cases/fourslash/renameDefaultImportDifferentName.ts @@ -0,0 +1,23 @@ +/// + +// @Filename: B.ts +////export default class /*1*/C { +//// test() { +//// } +////} + +// @Filename: A.ts +////import [|B|] from "./B"; +////let b = new [|B|](); +////b.test(); + +goTo.file("B.ts"); +goTo.marker("1"); +verify.occurrencesAtPositionCount(1); + +goTo.file("A.ts"); +let ranges = test.ranges() +for (let range of ranges) { + goTo.position(range.start); + verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); +} From 9e777306c7b53f304dee0a94635ce686b5f2fb8a Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 8 Apr 2016 13:36:18 -0700 Subject: [PATCH 09/16] Rename function --- src/services/services.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index cb062a0fc9f36..940e7df3b2bcb 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -5624,7 +5624,7 @@ namespace ts { }; } - function getImportOrExportSpecifierPropertyNameSymbolSpecifier(symbol: Symbol, location: Node): ImportOrExportSpecifier { + function getImportOrExportSpecifierForPropertyNameSymbol(symbol: Symbol, location: Node): ImportOrExportSpecifier { if (symbol.flags & SymbolFlags.Alias) { const importOrExportSpecifier = forEach(symbol.declarations, declaration => (declaration.kind === SyntaxKind.ImportSpecifier || @@ -6124,7 +6124,7 @@ namespace ts { //// export {a as somethingElse} //// We want the *local* declaration of 'a' as declared in the import, //// *not* as declared within "mod" (or farther) - const importOrExportSpecifier = getImportOrExportSpecifierPropertyNameSymbolSpecifier(symbol, location); + const importOrExportSpecifier = getImportOrExportSpecifierForPropertyNameSymbol(symbol, location); if (importOrExportSpecifier || getDeclarationOfKind(symbol, SyntaxKind.ImportClause)) { result = result.concat(populateSearchSymbolSet( !importOrExportSpecifier || importOrExportSpecifier.kind === SyntaxKind.ImportSpecifier ? @@ -6255,7 +6255,7 @@ namespace ts { // If the reference symbol is an alias, check if what it is aliasing is one of the search // symbols but by looking up for related symbol of this alias so it can handle multiple level of indirectness. - const importOrExportSpecifier = getImportOrExportSpecifierPropertyNameSymbolSpecifier(referenceSymbol, referenceLocation); + const importOrExportSpecifier = getImportOrExportSpecifierForPropertyNameSymbol(referenceSymbol, referenceLocation); if (importOrExportSpecifier || getDeclarationOfKind(referenceSymbol, SyntaxKind.ImportClause)) { const aliasedSymbol = !importOrExportSpecifier || importOrExportSpecifier.kind === SyntaxKind.ImportSpecifier ? typeChecker.getAliasedSymbol(referenceSymbol) : From 9e82646ac39aa7d83274cc41937e41affe03d100 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 8 Apr 2016 13:41:16 -0700 Subject: [PATCH 10/16] Another test case for rename in destructuring assignment --- .../fourslash/renameDestructuringAssignment.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 tests/cases/fourslash/renameDestructuringAssignment.ts diff --git a/tests/cases/fourslash/renameDestructuringAssignment.ts b/tests/cases/fourslash/renameDestructuringAssignment.ts new file mode 100644 index 0000000000000..b7b18f661d2a4 --- /dev/null +++ b/tests/cases/fourslash/renameDestructuringAssignment.ts @@ -0,0 +1,14 @@ +/// + +////interface I { +//// [|x|]: number; +////} +////var a: I; +////var x; +////({ [|x|]: x } = a); + +let ranges = test.ranges() +for (let range of ranges) { + goTo.position(range.start); + verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false); +} From 7a09e2f0e974d76db0a5079342597df887ccf6cf Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 11 Apr 2016 14:31:15 -0700 Subject: [PATCH 11/16] PR feedback --- src/compiler/checker.ts | 24 ++++++++++++++++-------- src/services/services.ts | 2 ++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index f8647007bdd6c..97749c780b0e2 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11844,17 +11844,18 @@ namespace ts { const elementType = checkIteratedTypeOrElementType(sourceType, node, /*allowStringInput*/ false) || unknownType; const elements = node.elements; for (let i = 0; i < elements.length; i++) { - checkArrayLiteralDestructuringElementAssignment(node, sourceType, elements[i], i, elementType, contextualMapper); + checkArrayLiteralDestructuringElementAssignment(node, sourceType, i, elementType, contextualMapper); } return sourceType; } function checkArrayLiteralDestructuringElementAssignment(node: ArrayLiteralExpression, sourceType: Type, - element: Expression, index: number, elementType: Type, contextualMapper?: TypeMapper) { + elementIndex: number, elementType: Type, contextualMapper?: TypeMapper) { const elements = node.elements; + const element = elements[elementIndex]; if (element.kind !== SyntaxKind.OmittedExpression) { if (element.kind !== SyntaxKind.SpreadElementExpression) { - const propName = "" + index; + const propName = "" + elementIndex; const type = isTypeAny(sourceType) ? sourceType : isTupleLikeType(sourceType) @@ -11873,7 +11874,7 @@ namespace ts { } } else { - if (index < elements.length - 1) { + if (elementIndex < elements.length - 1) { error(element, Diagnostics.A_rest_element_must_be_last_in_an_array_destructuring_pattern); } else { @@ -11887,6 +11888,7 @@ namespace ts { } } } + return undefined; } function checkDestructuringAssignment(exprOrAssignment: Expression | ShorthandPropertyAssignment, sourceType: Type, contextualMapper?: TypeMapper): Type { @@ -16559,9 +16561,15 @@ 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 { // If this is from "for of" - // for ( { a } of elemns) { + // for ( { a } of elems) { // } if (expr.parent.kind === SyntaxKind.ForOfStatement) { const iteratedType = checkRightHandSideOfForOf((expr.parent).expression); @@ -16577,15 +16585,15 @@ namespace ts { // 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); + return checkObjectLiteralDestructuringPropertyAssignment(typeOfParentObjectLiteral || unknownType, 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; + const elementType = checkIteratedTypeOrElementType(typeOfArrayLiteral || unknownType, expr.parent, /*allowStringInput*/ false) || unknownType; return checkArrayLiteralDestructuringElementAssignment(expr.parent, typeOfArrayLiteral, - expr, indexOf((expr.parent).elements, expr), elementType); + indexOf((expr.parent).elements, expr), elementType || unknownType); } function getTypeOfExpression(expr: Expression): Type { diff --git a/src/services/services.ts b/src/services/services.ts index 940e7df3b2bcb..2ebe2535246a6 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -5637,6 +5637,7 @@ namespace ts { return importOrExportSpecifier; } } + return undefined; } function isObjectBindingPatternElementWithoutPropertyName(symbol: Symbol) { @@ -5652,6 +5653,7 @@ namespace ts { const typeOfPattern = typeChecker.getTypeAtLocation(bindingElement.parent); return typeOfPattern && typeChecker.getPropertyOfType(typeOfPattern, (bindingElement.name).text); } + return undefined; } function getInternedName(symbol: Symbol, location: Node, declarations: Declaration[]): string { From 529bdd4ea66dd8ad0c0494a58e36a74cbb4cf266 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 11 Apr 2016 14:56:12 -0700 Subject: [PATCH 12/16] PR feedback --- src/services/services.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 2ebe2535246a6..c665fb2278ff2 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -6270,19 +6270,20 @@ namespace ts { // object literal, lookup the property symbol in the contextual type, and use this symbol to // compare to our searchSymbol if (isNameOfPropertyAssignment(referenceLocation)) { - const contexualSymbol = forEach(getPropertySymbolsFromContextualType(referenceLocation), contextualSymbol => { + const contextualSymbol = forEach(getPropertySymbolsFromContextualType(referenceLocation), contextualSymbol => { return forEach(typeChecker.getRootSymbols(contextualSymbol), s => searchSymbols.indexOf(s) >= 0 ? s : undefined); }); - if (contexualSymbol) { - return contexualSymbol; + if (contextualSymbol) { + return contextualSymbol; } - // If the reference location is name of property symbol from object literal destructuring pattern - // Compare it to search symbol something similar to 'property' from + // If the reference location is the name of property from object literal destructuring pattern + // Get the property symbol from the object literal's type and look if thats the search symbol + // In below eg. get 'property' from type of elems iterating type // for ( { property: p2 } of elems) { } if (isArrayLiteralOrObjectLiteralDestructuringPattern(referenceLocation.parent.parent)) { - // Do work to determine if this is property symbol corresponding to the search symbol + // Do work to determine if this is a property symbol corresponding to the search symbol const typeOfObjectLiteral = typeChecker.getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(referenceLocation.parent.parent); if (typeOfObjectLiteral) { const propertySymbol = typeChecker.getPropertyOfType(typeOfObjectLiteral, (referenceLocation).text); From 94961463fc423e47228d4fbe8e0c5b40642b80f1 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 11 Apr 2016 15:25:25 -0700 Subject: [PATCH 13/16] Rename of getPropertySymbolIfObjectBindingPatternWithoutPropertyName to getPropertySymbolOfObjectBindingPatternWithoutPropertyName --- src/services/services.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index c665fb2278ff2..af3f1666e1de6 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -5647,7 +5647,7 @@ namespace ts { !bindingElement.propertyName; } - function getPropertySymbolIfObjectBindingPatternWithoutPropertyName(symbol: Symbol) { + function getPropertySymbolOfObjectBindingPatternWithoutPropertyName(symbol: Symbol) { if (isObjectBindingPatternElementWithoutPropertyName(symbol)) { const bindingElement = getDeclarationOfKind(symbol, SyntaxKind.BindingElement); const typeOfPattern = typeChecker.getTypeAtLocation(bindingElement.parent); @@ -6170,7 +6170,7 @@ namespace ts { // If this is symbol of binding element without propertyName declaration in Object binding pattern // Include the property in the search - const bindingElementPropertySymbol = getPropertySymbolIfObjectBindingPatternWithoutPropertyName(symbol); + const bindingElementPropertySymbol = getPropertySymbolOfObjectBindingPatternWithoutPropertyName(symbol); if (bindingElementPropertySymbol) { result.push(bindingElementPropertySymbol); } @@ -6297,7 +6297,7 @@ namespace ts { // If the reference location is the binding element and doesn't have property name // then include the binding element in the related symbols // let { a } : { a }; - const bindingElementPropertySymbol = getPropertySymbolIfObjectBindingPatternWithoutPropertyName(referenceSymbol); + const bindingElementPropertySymbol = getPropertySymbolOfObjectBindingPatternWithoutPropertyName(referenceSymbol); if (bindingElementPropertySymbol && searchSymbols.indexOf(bindingElementPropertySymbol) >= 0) { return bindingElementPropertySymbol; } From edd098990b9a4ef3101e7b9824b9096cc5a0099e Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 11 Apr 2016 16:05:36 -0700 Subject: [PATCH 14/16] Removed debugger; statement --- .../fourslash/findAllRefsObjectBindingElementPropertyName06.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts index c0b940d43804f..379d1d4d5f59b 100644 --- a/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts +++ b/tests/cases/fourslash/findAllRefsObjectBindingElementPropertyName06.ts @@ -23,7 +23,6 @@ let ranges = test.ranges(); for (let range of ranges) { goTo.position(range.start); - debugger; verify.referencesCountIs(ranges.length); for (let expectedRange of ranges) { verify.referencesAtPositionContains(expectedRange); From 958a6a41ade51e8876e4cfa2087bcbc94fc616fe Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 11 Apr 2016 16:53:49 -0700 Subject: [PATCH 15/16] Some restructuring according to PR feedback --- src/services/services.ts | 65 +++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index af3f1666e1de6..d79e2b94a74ec 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -5624,8 +5624,14 @@ namespace ts { }; } - function getImportOrExportSpecifierForPropertyNameSymbol(symbol: Symbol, location: Node): ImportOrExportSpecifier { + function getAliasSymbolForPropertyNameSymbol(symbol: Symbol, location: Node): Symbol { if (symbol.flags & SymbolFlags.Alias) { + // Default import get alias + const defaultImport = getDeclarationOfKind(symbol, SyntaxKind.ImportClause); + if (defaultImport) { + return typeChecker.getAliasedSymbol(symbol); + } + const importOrExportSpecifier = forEach(symbol.declarations, declaration => (declaration.kind === SyntaxKind.ImportSpecifier || declaration.kind === SyntaxKind.ExportSpecifier) ? declaration : undefined); @@ -5634,7 +5640,22 @@ namespace ts { (!importOrExportSpecifier.propertyName || // export {a as class } where a is location importOrExportSpecifier.propertyName === location)) { - return importOrExportSpecifier; + // If Import specifier -> get alias + // else Export specifier -> get local target + return importOrExportSpecifier.kind === SyntaxKind.ImportSpecifier ? + typeChecker.getAliasedSymbol(symbol) : + typeChecker.getExportSpecifierLocalTargetSymbol(importOrExportSpecifier); + } + } + return undefined; + } + + function getPropertySymbolOfDestructuringAssignment(location: Node) { + if (isArrayLiteralOrObjectLiteralDestructuringPattern(location.parent.parent)) { + // Do work to determine if this is a property symbol corresponding to the search symbol + const typeOfObjectLiteral = typeChecker.getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(location.parent.parent); + if (typeOfObjectLiteral) { + return typeChecker.getPropertyOfType(typeOfObjectLiteral, (location).text); } } return undefined; @@ -6105,12 +6126,9 @@ namespace ts { // 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); + if (isNameOfPropertyAssignment(location) && location.parent.kind !== SyntaxKind.ShorthandPropertyAssignment) { + const propertySymbol = getPropertySymbolOfDestructuringAssignment(location); + if (propertySymbol) { result.push(propertySymbol); } } @@ -6126,12 +6144,9 @@ namespace ts { //// export {a as somethingElse} //// We want the *local* declaration of 'a' as declared in the import, //// *not* as declared within "mod" (or farther) - const importOrExportSpecifier = getImportOrExportSpecifierForPropertyNameSymbol(symbol, location); - if (importOrExportSpecifier || getDeclarationOfKind(symbol, SyntaxKind.ImportClause)) { - result = result.concat(populateSearchSymbolSet( - !importOrExportSpecifier || importOrExportSpecifier.kind === SyntaxKind.ImportSpecifier ? - typeChecker.getAliasedSymbol(symbol) : - typeChecker.getExportSpecifierLocalTargetSymbol(importOrExportSpecifier), location)); + const aliasSymbol = getAliasSymbolForPropertyNameSymbol(symbol, location); + if (aliasSymbol) { + result = result.concat(populateSearchSymbolSet(aliasSymbol, location)); } // If the location is in a context sensitive location (i.e. in an object literal) try @@ -6257,15 +6272,11 @@ namespace ts { // If the reference symbol is an alias, check if what it is aliasing is one of the search // symbols but by looking up for related symbol of this alias so it can handle multiple level of indirectness. - const importOrExportSpecifier = getImportOrExportSpecifierForPropertyNameSymbol(referenceSymbol, referenceLocation); - if (importOrExportSpecifier || getDeclarationOfKind(referenceSymbol, SyntaxKind.ImportClause)) { - const aliasedSymbol = !importOrExportSpecifier || importOrExportSpecifier.kind === SyntaxKind.ImportSpecifier ? - typeChecker.getAliasedSymbol(referenceSymbol) : - typeChecker.getExportSpecifierLocalTargetSymbol(importOrExportSpecifier); - return getRelatedSymbol(searchSymbols, aliasedSymbol, referenceLocation); + const aliasSymbol = getAliasSymbolForPropertyNameSymbol(referenceSymbol, referenceLocation); + if (aliasSymbol) { + return getRelatedSymbol(searchSymbols, aliasSymbol, 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 @@ -6282,15 +6293,9 @@ namespace ts { // Get the property symbol from the object literal's type and look if thats the search symbol // In below eg. get 'property' from type of elems iterating type // for ( { property: p2 } of elems) { } - if (isArrayLiteralOrObjectLiteralDestructuringPattern(referenceLocation.parent.parent)) { - // Do work to determine if this is a 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; - } - } + const propertySymbol = getPropertySymbolOfDestructuringAssignment(referenceLocation); + if (propertySymbol && searchSymbols.indexOf(propertySymbol) >= 0) { + return propertySymbol; } } From c492fc6369f1319003695f8c0d8302fb3bc39f1d Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 13 Apr 2016 14:50:41 -0700 Subject: [PATCH 16/16] Update the entry point to return property symbol of destructuring assignment --- src/compiler/checker.ts | 15 ++++++++++++++- src/compiler/types.ts | 2 +- src/services/services.ts | 10 ++-------- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 97749c780b0e2..c346dc97f76cf 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -83,7 +83,7 @@ namespace ts { getShorthandAssignmentValueSymbol, getExportSpecifierLocalTargetSymbol, getTypeAtLocation: getTypeOfNode, - getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment, + getPropertySymbolOfDestructuringAssignment, typeToString, getSymbolDisplayBuilder, symbolToString, @@ -16568,6 +16568,7 @@ namespace ts { // [ a ] from // [a] = [ some array ...] function getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(expr: Expression): Type { + Debug.assert(expr.kind === SyntaxKind.ObjectLiteralExpression || expr.kind === SyntaxKind.ArrayLiteralExpression); // If this is from "for of" // for ( { a } of elems) { // } @@ -16596,6 +16597,18 @@ namespace ts { indexOf((expr.parent).elements, expr), elementType || unknownType); } + // Gets the property symbol corresponding to the property in destructuring assignment + // 'property1' from + // for ( { property1: a } of elems) { + // } + // 'property1' at location 'a' from: + // [a] = [ property1, property2 ] + function getPropertySymbolOfDestructuringAssignment(location: Identifier) { + // Get the type of the object or array literal and then look for property of given name in the type + const typeOfObjectLiteral = getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(location.parent.parent); + return typeOfObjectLiteral && getPropertyOfType(typeOfObjectLiteral, location.text); + } + function getTypeOfExpression(expr: Expression): Type { if (isRightSideOfQualifiedNameOrPropertyAccess(expr)) { expr = expr.parent; diff --git a/src/compiler/types.ts b/src/compiler/types.ts index d5b9ab6d14326..17351babc7849 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -1735,7 +1735,7 @@ namespace ts { getSymbolsOfParameterPropertyDeclaration(parameter: ParameterDeclaration, parameterName: string): Symbol[]; getShorthandAssignmentValueSymbol(location: Node): Symbol; getExportSpecifierLocalTargetSymbol(location: ExportSpecifier): Symbol; - getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(expression: Expression): Type; + getPropertySymbolOfDestructuringAssignment(location: Identifier): Symbol; 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 d79e2b94a74ec..a30bc47b83c50 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -5651,14 +5651,8 @@ namespace ts { } function getPropertySymbolOfDestructuringAssignment(location: Node) { - if (isArrayLiteralOrObjectLiteralDestructuringPattern(location.parent.parent)) { - // Do work to determine if this is a property symbol corresponding to the search symbol - const typeOfObjectLiteral = typeChecker.getTypeOfArrayLiteralOrObjectLiteralDestructuringAssignment(location.parent.parent); - if (typeOfObjectLiteral) { - return typeChecker.getPropertyOfType(typeOfObjectLiteral, (location).text); - } - } - return undefined; + return isArrayLiteralOrObjectLiteralDestructuringPattern(location.parent.parent) && + typeChecker.getPropertySymbolOfDestructuringAssignment(location); } function isObjectBindingPatternElementWithoutPropertyName(symbol: Symbol) {