From d3d4f83f89d6d407f4a5f061964b70be3dc8eeed Mon Sep 17 00:00:00 2001 From: Andy Date: Mon, 22 Oct 2018 11:44:06 -0700 Subject: [PATCH] Remove hack to get target of GetAccessor symbol (#27868) * Remove hack to get target of GetAccessor symbol * Add tests and get moveToNewFile to work with binding patterns --- src/harness/fourslash.ts | 8 +++-- src/services/refactors/moveToNewFile.ts | 27 ++++++++++++--- src/services/utilities.ts | 10 ++---- .../findAllRefsDestructureGetter2.ts | 26 +++++++++++++++ tests/cases/fourslash/fourslash.ts | 2 +- .../moveToNewFile_bindingPatterns.ts | 23 +++++++++++++ tests/cases/fourslash/moveToNewFile_getter.ts | 33 +++++++++++++++++++ 7 files changed, 113 insertions(+), 16 deletions(-) create mode 100644 tests/cases/fourslash/findAllRefsDestructureGetter2.ts create mode 100644 tests/cases/fourslash/moveToNewFile_bindingPatterns.ts create mode 100644 tests/cases/fourslash/moveToNewFile_getter.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index dc28871d1c0..a75a78e2a63 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1328,8 +1328,10 @@ Actual: ${stringify(fullActual)}`); }))); } - public verifyQuickInfoAt(markerName: string, expectedText: string, expectedDocumentation?: string) { - this.goToMarker(markerName); + public verifyQuickInfoAt(markerName: string | Range, expectedText: string, expectedDocumentation?: string) { + if (typeof markerName === "string") this.goToMarker(markerName); + else this.goToRangeStart(markerName); + this.verifyQuickInfoString(expectedText, expectedDocumentation); } @@ -4221,7 +4223,7 @@ namespace FourSlashInterface { this.state.verifyQuickInfoString(expectedText, expectedDocumentation); } - public quickInfoAt(markerName: string, expectedText: string, expectedDocumentation?: string) { + public quickInfoAt(markerName: string | FourSlash.Range, expectedText: string, expectedDocumentation?: string) { this.state.verifyQuickInfoAt(markerName, expectedText, expectedDocumentation); } diff --git a/src/services/refactors/moveToNewFile.ts b/src/services/refactors/moveToNewFile.ts index 693cdd9c6cb..757b1ab71cf 100644 --- a/src/services/refactors/moveToNewFile.ts +++ b/src/services/refactors/moveToNewFile.ts @@ -612,7 +612,7 @@ namespace ts.refactor { | ImportEqualsDeclaration; type TopLevelDeclarationStatement = NonVariableTopLevelDeclaration | VariableStatement; interface TopLevelVariableDeclaration extends VariableDeclaration { parent: VariableDeclarationList & { parent: VariableStatement; }; } - type TopLevelDeclaration = NonVariableTopLevelDeclaration | TopLevelVariableDeclaration; + type TopLevelDeclaration = NonVariableTopLevelDeclaration | TopLevelVariableDeclaration | BindingElement; function isTopLevelDeclaration(node: Node): node is TopLevelDeclaration { return isNonVariableTopLevelDeclaration(node) && isSourceFile(node.parent) || isVariableDeclaration(node) && isSourceFile(node.parent.parent.parent); } @@ -653,7 +653,7 @@ namespace ts.refactor { return cb(statement as FunctionDeclaration | ClassDeclaration | EnumDeclaration | ModuleDeclaration | TypeAliasDeclaration | InterfaceDeclaration | ImportEqualsDeclaration); case SyntaxKind.VariableStatement: - return forEach((statement as VariableStatement).declarationList.declarations as ReadonlyArray, cb); + return firstDefined((statement as VariableStatement).declarationList.declarations, decl => forEachTopLevelDeclarationInBindingName(decl.name, cb)); case SyntaxKind.ExpressionStatement: { const { expression } = statement as ExpressionStatement; @@ -663,13 +663,32 @@ namespace ts.refactor { } } } + function forEachTopLevelDeclarationInBindingName(name: BindingName, cb: (node: TopLevelDeclaration) => T): T | undefined { + switch (name.kind) { + case SyntaxKind.Identifier: + return cb(cast(name.parent, (x): x is TopLevelVariableDeclaration | BindingElement => isVariableDeclaration(x) || isBindingElement(x))); + case SyntaxKind.ArrayBindingPattern: + case SyntaxKind.ObjectBindingPattern: + return firstDefined(name.elements, em => isOmittedExpression(em) ? undefined : forEachTopLevelDeclarationInBindingName(em.name, cb)); + default: + return Debug.assertNever(name); + } + } function nameOfTopLevelDeclaration(d: TopLevelDeclaration): Identifier | undefined { - return d.kind === SyntaxKind.ExpressionStatement ? d.expression.left.name : tryCast(d.name, isIdentifier); + return isExpressionStatement(d) ? d.expression.left.name : tryCast(d.name, isIdentifier); } function getTopLevelDeclarationStatement(d: TopLevelDeclaration): TopLevelDeclarationStatement { - return isVariableDeclaration(d) ? d.parent.parent : d; + switch (d.kind) { + case SyntaxKind.VariableDeclaration: + return d.parent.parent; + case SyntaxKind.BindingElement: + return getTopLevelDeclarationStatement( + cast(d.parent.parent, (p): p is TopLevelVariableDeclaration | BindingElement => isVariableDeclaration(p) || isBindingElement(p))); + default: + return d; + } } function addExportToChanges(sourceFile: SourceFile, decl: TopLevelDeclarationStatement, changes: textChanges.ChangeTracker, useEs6Exports: boolean): void { diff --git a/src/services/utilities.ts b/src/services/utilities.ts index e53f568c0f4..f0326599ad0 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1337,15 +1337,9 @@ namespace ts { !bindingElement.propertyName; } - export function getPropertySymbolFromBindingElement(checker: TypeChecker, bindingElement: ObjectBindingElementWithoutPropertyName) { + export function getPropertySymbolFromBindingElement(checker: TypeChecker, bindingElement: ObjectBindingElementWithoutPropertyName): Symbol | undefined { const typeOfPattern = checker.getTypeAtLocation(bindingElement.parent); - const propSymbol = typeOfPattern && checker.getPropertyOfType(typeOfPattern, bindingElement.name.text); - if (propSymbol && propSymbol.flags & SymbolFlags.Accessor) { - // See GH#16922 - Debug.assert(!!(propSymbol.flags & SymbolFlags.Transient)); - return (propSymbol as TransientSymbol).target; - } - return propSymbol; + return typeOfPattern && checker.getPropertyOfType(typeOfPattern, bindingElement.name.text); } /** diff --git a/tests/cases/fourslash/findAllRefsDestructureGetter2.ts b/tests/cases/fourslash/findAllRefsDestructureGetter2.ts new file mode 100644 index 00000000000..d26983a1091 --- /dev/null +++ b/tests/cases/fourslash/findAllRefsDestructureGetter2.ts @@ -0,0 +1,26 @@ +/// + +// @noLib: true + +// @Filename: /a.ts +////class C { +//// get [|{| "isWriteAccess": true, "isDefinition": true |}g|](): number { return 0; } +//// +//// set [|{| "isWriteAccess": true, "isDefinition": true |}s|](value: number) {} +////} +////const { [|{| "isWriteAccess": true, "isDefinition": true |}g|], [|{| "isWriteAccess": true, "isDefinition": true |}s|] } = new C(); + +const [g0, s0, g1, s1] = test.ranges(); +verify.quickInfoAt(g0, "(property) C.g: number"); +verify.referenceGroups(g0, [{ definition: "(property) C.g: number", ranges: [g0, g1] }]); +verify.referenceGroups(g1, [ + { definition: "(property) C.g: number", ranges: [g0] }, + { definition: "const g: number", ranges: [g1] }, +]); + +verify.quickInfoAt(s0, "(property) C.s: number"); +verify.referenceGroups(s0, [{ definition: "(property) C.s: number", ranges: [s0, s1] }]); +verify.referenceGroups(s1, [ + { definition: "(property) C.s: number", ranges: [s0] }, + { definition: "const s: number", ranges: [s1] } +]); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 4603d61a286..e58e12a9ecf 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -322,7 +322,7 @@ declare namespace FourSlashInterface { /** Verify the quick info available at the current marker. */ quickInfoIs(expectedText: string, expectedDocumentation?: string): void; /** Goto a marker and call `quickInfoIs`. */ - quickInfoAt(markerName: string, expectedText?: string, expectedDocumentation?: string): void; + quickInfoAt(markerName: string | Range, expectedText?: string, expectedDocumentation?: string): void; /** * Call `quickInfoAt` for each pair in the object. * (If the value is an array, it is [expectedText, expectedDocumentation].) diff --git a/tests/cases/fourslash/moveToNewFile_bindingPatterns.ts b/tests/cases/fourslash/moveToNewFile_bindingPatterns.ts new file mode 100644 index 00000000000..f004fdbdde9 --- /dev/null +++ b/tests/cases/fourslash/moveToNewFile_bindingPatterns.ts @@ -0,0 +1,23 @@ +/// + +// @Filename: /a.ts +////[|export const [x, { p: y }] = [0, { p: 1 }];|] + +// @Filename: /b.ts +////import { x, y } from "./a"; + +verify.noErrors(); + +verify.moveToNewFile({ + newFileContents: { + "/a.ts": "", + + "/x.ts": +`export const [x, { p: y }] = [0, { p: 1 }];`, + + "/b.ts": +` +import { x, y } from "./x"; +`, + }, +}); diff --git a/tests/cases/fourslash/moveToNewFile_getter.ts b/tests/cases/fourslash/moveToNewFile_getter.ts new file mode 100644 index 00000000000..d595d92cf07 --- /dev/null +++ b/tests/cases/fourslash/moveToNewFile_getter.ts @@ -0,0 +1,33 @@ +/// + +// @Filename: /a.ts +////class C { +//// get g() { return 0; } +//// get h() { return 1; } +////} +////[|export const { g, h: i } = new C();|] + +// @Filename: /b.ts +////import { g, i } from "./a"; + +verify.noErrors(); + +verify.moveToNewFile({ + newFileContents: { + "/a.ts": +`export class C { + get g() { return 0; } + get h() { return 1; } +} +`, + + "/g.ts": +`import { C } from "./a"; +export const { g, h: i } = new C();`, + + "/b.ts": +` +import { g, i } from "./g"; +`, + }, +});