microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2026-02-05 16:38:05 -06:00
Code Issues Packages Projects Releases 212 Wiki Activity

Handle imports and exports in 'convert parameters to destructured object' (#30475)

Browse Source 
* add test for imported function

* start to implement import references check

* fix imported function test

* skip alias when looking for symbol target

* recognize ES6 imports

* recognize some export syntax

* add tests for imports/exports

* add test for imported function

* start to implement import references check

* fix imported function test

* skip alias when looking for symbol target

* recognize ES6 imports

* recognize some export syntax

* add tests for imports/exports

* add test for imported function

* start to implement import references check

* fix imported function test

* recognize ES6 imports

* recognize some export syntax

* add mode import/export syntax cases

* fix entryToFunctionCall to deal with new calls through property/element access expressions

* add more tests for imports/exports

* allow function and class declarations that have no name but have a default modifier

* rename tests

* fix conflict

* fix tests

* add test for nameless class

* rename function

* minor refactor

* remove old tests

* delete old test

* refactor as suggested

* use getContainingFunctionDeclaration
This commit is contained in:
Gabriela Araujo Britto 2019-03-28 13:34:29 -07:00 committed by GitHub
parent c03b7f525c
commit bb5eb025a8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 270 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/compiler/utilities.ts
View File

@ -1330,6 +1330,10 @@ namespace ts {
return findAncestor(node.parent, isFunctionLike);
}
export function getContainingFunctionDeclaration(node: Node): FunctionLikeDeclaration | undefined {
return findAncestor(node.parent, isFunctionLikeDeclaration);
}
export function getContainingClass(node: Node): ClassLikeDeclaration | undefined {
return findAncestor(node.parent, isClassLike);
}

120
src/services/refactors/convertParamsToDestructuredObject.ts
View File

@ -90,8 +90,8 @@ namespace ts.refactor.convertParamsToDestructuredObject {
function groupReferences(referenceEntries: ReadonlyArray<FindAllReferences.Entry>): GroupedReferences {
const classReferences: ClassReferences = { accessExpressions: [], typeUsages: [] };
const groupedReferences: GroupedReferences = { functionCalls: [], declarations: [], classReferences, valid: true };
const functionSymbols = map(functionNames, checker.getSymbolAtLocation);
const classSymbols = map(classNames, checker.getSymbolAtLocation);
const functionSymbols = map(functionNames, getSymbolTargetAtLocation);
const classSymbols = map(classNames, getSymbolTargetAtLocation);
const isConstructor = isConstructorDeclaration(functionDeclaration);
for (const entry of referenceEntries) {
@ -111,7 +111,11 @@ namespace ts.refactor.convertParamsToDestructuredObject {
So we need to add a special case for this because when calling a constructor of a class through one of its subclasses,
the symbols are going to be different.
*/
if (contains(functionSymbols, checker.getSymbolAtLocation(entry.node), symbolComparer) || isNewExpressionTarget(entry.node)) {
if (contains(functionSymbols, getSymbolTargetAtLocation(entry.node)) || isNewExpressionTarget(entry.node)) {
const importOrExportReference = entryToImportOrExport(entry);
if (importOrExportReference) {
continue;
}
const decl = entryToDeclaration(entry);
if (decl) {
groupedReferences.declarations.push(decl);
@ -125,7 +129,12 @@ namespace ts.refactor.convertParamsToDestructuredObject {
}
}
// if the refactored function is a constructor, we must also check if the references to its class are valid
if (isConstructor && contains(classSymbols, checker.getSymbolAtLocation(entry.node), symbolComparer)) {
if (isConstructor && contains(classSymbols, getSymbolTargetAtLocation(entry.node))) {
const importOrExportReference = entryToImportOrExport(entry);
if (importOrExportReference) {
continue;
}
const decl = entryToDeclaration(entry);
if (decl) {
groupedReferences.declarations.push(decl);
@ -153,10 +162,27 @@ namespace ts.refactor.convertParamsToDestructuredObject {
return groupedReferences;
}
function getSymbolTargetAtLocation(node: Node) {
const symbol = checker.getSymbolAtLocation(node);
return symbol && getSymbolTarget(symbol, checker);
}
}
function symbolComparer(a: Symbol, b: Symbol): boolean {
return getSymbolTarget(a) === getSymbolTarget(b);
function entryToImportOrExport(entry: FindAllReferences.NodeEntry): Node | undefined {
const node = entry.node;
if (isImportSpecifier(node.parent)
|| isImportClause(node.parent)
|| isImportEqualsDeclaration(node.parent)
|| isNamespaceImport(node.parent)) {
return node;
}
if (isExportSpecifier(node.parent) || isExportAssignment(node.parent)) {
return node;
}
return undefined;
}
function entryToDeclaration(entry: FindAllReferences.NodeEntry): Node | undefined {
@ -171,37 +197,31 @@ namespace ts.refactor.convertParamsToDestructuredObject {
const functionReference = entry.node;
const parent = functionReference.parent;
switch (parent.kind) {
// Function call (foo(...) or super(...))
// foo(...) or super(...) or new Foo(...)
case SyntaxKind.CallExpression:
const callExpression = tryCast(parent, isCallExpression);
if (callExpression && callExpression.expression === functionReference) {
return callExpression;
}
break;
// Constructor call (new Foo(...))
case SyntaxKind.NewExpression:
const newExpression = tryCast(parent, isNewExpression);
if (newExpression && newExpression.expression === functionReference) {
return newExpression;
const callOrNewExpression = tryCast(parent, isCallOrNewExpression);
if (callOrNewExpression && callOrNewExpression.expression === functionReference) {
return callOrNewExpression;
}
break;
// Method call (x.foo(...))
// x.foo(...)
case SyntaxKind.PropertyAccessExpression:
const propertyAccessExpression = tryCast(parent, isPropertyAccessExpression);
if (propertyAccessExpression && propertyAccessExpression.parent && propertyAccessExpression.name === functionReference) {
const callExpression = tryCast(propertyAccessExpression.parent, isCallExpression);
if (callExpression && callExpression.expression === propertyAccessExpression) {
return callExpression;
const callOrNewExpression = tryCast(propertyAccessExpression.parent, isCallOrNewExpression);
if (callOrNewExpression && callOrNewExpression.expression === propertyAccessExpression) {
return callOrNewExpression;
}
}
break;
// Method call (x["foo"](...))
// x["foo"](...)
case SyntaxKind.ElementAccessExpression:
const elementAccessExpression = tryCast(parent, isElementAccessExpression);
if (elementAccessExpression && elementAccessExpression.parent && elementAccessExpression.argumentExpression === functionReference) {
const callExpression = tryCast(elementAccessExpression.parent, isCallExpression);
if (callExpression && callExpression.expression === elementAccessExpression) {
return callExpression;
const callOrNewExpression = tryCast(elementAccessExpression.parent, isCallOrNewExpression);
if (callOrNewExpression && callOrNewExpression.expression === elementAccessExpression) {
return callOrNewExpression;
}
}
break;
@ -244,7 +264,7 @@ namespace ts.refactor.convertParamsToDestructuredObject {
function getFunctionDeclarationAtPosition(file: SourceFile, startPosition: number, checker: TypeChecker): ValidFunctionDeclaration | undefined {
const node = getTouchingToken(file, startPosition);
const functionDeclaration = getContainingFunction(node);
const functionDeclaration = getContainingFunctionDeclaration(node);
// don't offer refactor on top-level JSDoc
if (isTopLevelJSDoc(node)) return undefined;
@ -267,25 +287,21 @@ namespace ts.refactor.convertParamsToDestructuredObject {
}
function isValidFunctionDeclaration(
functionDeclaration: SignatureDeclaration,
functionDeclaration: FunctionLikeDeclaration,
checker: TypeChecker): functionDeclaration is ValidFunctionDeclaration {
if (!isValidParameterNodeArray(functionDeclaration.parameters, checker)) return false;
switch (functionDeclaration.kind) {
case SyntaxKind.FunctionDeclaration:
return hasNameOrDefault(functionDeclaration) && isSingleImplementation(functionDeclaration, checker);
case SyntaxKind.MethodDeclaration:
return !!functionDeclaration.name
&& !!functionDeclaration.body
&& !checker.isImplementationOfOverload(functionDeclaration);
return isSingleImplementation(functionDeclaration, checker);
case SyntaxKind.Constructor:
if (isClassDeclaration(functionDeclaration.parent)) {
return !!functionDeclaration.body
&& !!functionDeclaration.parent.name
&& !checker.isImplementationOfOverload(functionDeclaration);
return hasNameOrDefault(functionDeclaration.parent) && isSingleImplementation(functionDeclaration, checker);
}
else {
return isValidVariableDeclaration(functionDeclaration.parent.parent)
&& !!functionDeclaration.body
&& !checker.isImplementationOfOverload(functionDeclaration);
&& isSingleImplementation(functionDeclaration, checker);
}
case SyntaxKind.FunctionExpression:
case SyntaxKind.ArrowFunction:
@ -294,6 +310,18 @@ namespace ts.refactor.convertParamsToDestructuredObject {
return false;
}
function isSingleImplementation(functionDeclaration: FunctionLikeDeclaration, checker: TypeChecker): boolean {
return !!functionDeclaration.body && !checker.isImplementationOfOverload(functionDeclaration);
}
function hasNameOrDefault(functionOrClassDeclaration: FunctionDeclaration | ClassDeclaration): boolean {
if (!functionOrClassDeclaration.name) {
const defaultKeyword = findModifier(functionOrClassDeclaration, SyntaxKind.DefaultKeyword);
return !!defaultKeyword;
}
return true;
}
function isValidParameterNodeArray(
parameters: NodeArray<ParameterDeclaration>,
checker: TypeChecker): parameters is ValidParameterNodeArray {
@ -488,11 +516,17 @@ namespace ts.refactor.convertParamsToDestructuredObject {
return getTextOfIdentifierOrLiteral(paramDeclaration.name);
}
function getClassNames(constructorDeclaration: ValidConstructor): Identifier[] {
function getClassNames(constructorDeclaration: ValidConstructor): (Identifier | Modifier)[] {
switch (constructorDeclaration.parent.kind) {
case SyntaxKind.ClassDeclaration:
const classDeclaration = constructorDeclaration.parent;
return [classDeclaration.name];
if (classDeclaration.name) return [classDeclaration.name];
// If the class declaration doesn't have a name, it should have a default modifier.
// We validated this in `isValidFunctionDeclaration` through `hasNameOrDefault`
const defaultModifier = Debug.assertDefined(
findModifier(classDeclaration, SyntaxKind.DefaultKeyword),
"Nameless class declaration should be a default export");
return [defaultModifier];
case SyntaxKind.ClassExpression:
const classExpression = constructorDeclaration.parent;
const variableDeclaration = constructorDeclaration.parent.parent;
@ -505,10 +539,19 @@ namespace ts.refactor.convertParamsToDestructuredObject {
function getFunctionNames(functionDeclaration: ValidFunctionDeclaration): Node[] {
switch (functionDeclaration.kind) {
case SyntaxKind.FunctionDeclaration:
if (functionDeclaration.name) return [functionDeclaration.name];
// If the function declaration doesn't have a name, it should have a default modifier.
// We validated this in `isValidFunctionDeclaration` through `hasNameOrDefault`
const defaultModifier = Debug.assertDefined(
findModifier(functionDeclaration, SyntaxKind.DefaultKeyword),
"Nameless function declaration should be a default export");
return [defaultModifier];
case SyntaxKind.MethodDeclaration:
return [functionDeclaration.name];
case SyntaxKind.Constructor:
const ctrKeyword = findChildOfKind(functionDeclaration, SyntaxKind.ConstructorKeyword, functionDeclaration.getSourceFile())!;
const ctrKeyword = Debug.assertDefined(
findChildOfKind(functionDeclaration, SyntaxKind.ConstructorKeyword, functionDeclaration.getSourceFile()),
"Constructor declaration should have constructor keyword");
if (functionDeclaration.parent.kind === SyntaxKind.ClassExpression) {
const variableDeclaration = functionDeclaration.parent.parent;
return [variableDeclaration.name, ctrKeyword];
@ -532,13 +575,12 @@ namespace ts.refactor.convertParamsToDestructuredObject {
}
interface ValidConstructor extends ConstructorDeclaration {
parent: (ClassDeclaration & { name: Identifier }) | (ClassExpression & { parent: ValidVariableDeclaration });
parent: ClassDeclaration | (ClassExpression & { parent: ValidVariableDeclaration });
parameters: NodeArray<ValidParameterDeclaration>;
body: FunctionBody;
}
interface ValidFunction extends FunctionDeclaration {
name: Identifier;
parameters: NodeArray<ValidParameterDeclaration>;
body: FunctionBody;
}

15
src/services/utilities.ts
View File

@ -1664,10 +1664,15 @@ namespace ts {
return ensureScriptKind(fileName, host && host.getScriptKind && host.getScriptKind(fileName));
}
export function getSymbolTarget(symbol: Symbol): Symbol {
export function getSymbolTarget(symbol: Symbol, checker: TypeChecker): Symbol {
let next: Symbol = symbol;
while (isTransientSymbol(next) && next.target) {
next = next.target;
while (isAliasSymbol(next) || (isTransientSymbol(next) && next.target)) {
if (isTransientSymbol(next) && next.target) {
next = next.target;
}
else {
next = skipAlias(next, checker);
}
}
return next;
}
@ -1676,6 +1681,10 @@ namespace ts {
return (symbol.flags & SymbolFlags.Transient) !== 0;
}
function isAliasSymbol(symbol: Symbol): boolean {
return (symbol.flags & SymbolFlags.Alias) !== 0;
}
export function getUniqueSymbolId(symbol: Symbol, checker: TypeChecker) {
return getSymbolId(skipAlias(symbol, checker));
}

4
tests/cases/fourslash/refactorConvertParamsToDestructuredObject_badRestParam.ts
View File

@ -14,7 +14,7 @@
////}
goTo.select("a", "b");
verify.not.refactorAvailable("Convert to named parameters");
verify.not.refactorAvailable("Convert parameters to destructured object");
goTo.select("c", "d");
verify.not.refactorAvailable("Convert to named parameters");
verify.not.refactorAvailable("Convert parameters to destructured object");

8
tests/cases/fourslash/refactorConvertParamsToDestructuredObject_defaultClass.ts
View File

@ -1,8 +0,0 @@
/// <reference path='fourslash.ts' />
/////export default class {
//// constructor(/*a*/a: number, b = { x: 1 }/*b*/) {}
////}
goTo.select("a", "b");
verify.not.refactorAvailable("Convert parameters to destructured object");

24
tests/cases/fourslash/refactorConvertParamsToDestructuredObject_importedFunction.ts Normal file
View File

@ -0,0 +1,24 @@
/// <reference path='fourslash.ts' />
// @Filename: f.ts
////export function f(/*a*/a: number, b: string/*b*/): string {
//// return b;
////}
// @Filename: a.ts
////import { f as g } from "./f";
////g(4, "b");
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Convert parameters to destructured object",
actionName: "Convert parameters to destructured object",
actionDescription: "Convert parameters to destructured object",
newContent: `export function f({ a, b }: { a: number; b: string; }): string {
return b;
}`
});
goTo.file("a.ts");
verify.currentFileContentIs(`import { f as g } from "./f";
g({ a: 4, b: "b" });`)

24
tests/cases/fourslash/refactorConvertParamsToDestructuredObject_importedFunction2.ts Normal file
View File

@ -0,0 +1,24 @@
/// <reference path='fourslash.ts' />
// @Filename: f.ts
////export default function f(/*a*/a: number, b: string/*b*/): string {
//// return b;
////}
// @Filename: a.ts
////import g from "./f";
////g(4, "b");
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Convert parameters to destructured object",
actionName: "Convert parameters to destructured object",
actionDescription: "Convert parameters to destructured object",
newContent: `export default function f({ a, b }: { a: number; b: string; }): string {
return b;
}`
});
goTo.file("a.ts");
verify.currentFileContentIs(`import g from "./f";
g({ a: 4, b: "b" });`)

23
tests/cases/fourslash/refactorConvertParamsToDestructuredObject_importedFunction3.ts Normal file
View File

@ -0,0 +1,23 @@
/// <reference path='fourslash.ts' />
// @Filename: f.ts
////function foo(/*a*/a: string, b: string/*b*/) { }
////export = foo;
// @Filename: a.ts
////import bar = require("./f");
////bar("a", "b");
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Convert parameters to destructured object",
actionName: "Convert parameters to destructured object",
actionDescription: "Convert parameters to destructured object",
newContent: `function foo({ a, b }: { a: string; b: string; }) { }
export = foo;`
});
goTo.file("a.ts");
verify.currentFileContentIs(`import bar = require("./f");
bar({ a: "a", b: "b" });`)

27
tests/cases/fourslash/refactorConvertParamsToDestructuredObject_importedFunction4.ts Normal file
View File

@ -0,0 +1,27 @@
/// <reference path='fourslash.ts' />
// @Filename: f.ts
////export { foo as default };
////function /*a*/foo/*b*/(a: number, b: number) {
//// return a + b;
////}
// @Filename: a.ts
////import bar from "./f";
////bar(1, 2);
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Convert parameters to destructured object",
actionName: "Convert parameters to destructured object",
actionDescription: "Convert parameters to destructured object",
newContent: `export { foo as default };
function foo({ a, b }: { a: number; b: number; }) {
return a + b;
}`
});
goTo.file("a.ts");
verify.currentFileContentIs(`import bar from "./f";
bar({ a: 1, b: 2 });`)

27
tests/cases/fourslash/refactorConvertParamsToDestructuredObject_importedFunction5.ts Normal file
View File

@ -0,0 +1,27 @@
/// <reference path='fourslash.ts' />
// @Filename: f.ts
////export class C {
//// /*a*/constructor/*b*/(a: number, b: number) { }
////}
// @Filename: a.ts
////import f = require("./f");
////const c = new f.C(1, 2);
////const c1 = new f["C"](1, 2);
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Convert parameters to destructured object",
actionName: "Convert parameters to destructured object",
actionDescription: "Convert parameters to destructured object",
newContent: `export class C {
constructor({ a, b }: { a: number; b: number; }) { }
}`
});
goTo.file("a.ts");
verify.currentFileContentIs(`import f = require("./f");
const c = new f.C({ a: 1, b: 2 });
const c1 = new f["C"]({ a: 1, b: 2 });`)

21
tests/cases/fourslash/refactorConvertParamsToDestructuredObject_importedFunction6.ts Normal file
View File

@ -0,0 +1,21 @@
/// <reference path='fourslash.ts' />
// @Filename: f.ts
////export function /*a*/foo/*b*/(a: string, b: string) { }
// @Filename: a.ts
////import * as f from "./f";
////f.foo("a", "b");
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Convert parameters to destructured object",
actionName: "Convert parameters to destructured object",
actionDescription: "Convert parameters to destructured object",
newContent: `export function foo({ a, b }: { a: string; b: string; }) { }`
});
goTo.file("a.ts");
verify.currentFileContentIs(`import * as f from "./f";
f.foo({ a: "a", b: "b" });`)

25
tests/cases/fourslash/refactorConvertParamsToDestructuredObject_namelessClass.ts Normal file
View File

@ -0,0 +1,25 @@
/// <reference path='fourslash.ts' />
// @Filename: f.ts
////export default class {
//// /*a*/constructor/*b*/(a: string, b: string) { }
////}
// @Filename: a.ts
////import C from "./f";
////const c = new C("a", "b");
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Convert parameters to destructured object",
actionName: "Convert parameters to destructured object",
actionDescription: "Convert parameters to destructured object",
newContent: `export default class {
constructor({ a, b }: { a: string; b: string; }) { }
}`
});
goTo.file("a.ts");
verify.currentFileContentIs(`import C from "./f";
const c = new C({ a: "a", b: "b" });`)