Skip to content

🤖 Pick PR #38213 (fix(38177): Extract to function can...) into release-3.9 #38281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions src/services/codefixes/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,18 +226,8 @@ namespace ts.codefix {
const scriptTarget = getEmitScriptTarget(context.program.getCompilerOptions());
const checker = context.program.getTypeChecker();
const tracker = getNoopSymbolTrackerWithResolver(context);
const types = map(args, arg => {
const type = checker.getBaseTypeOfLiteralType(checker.getTypeAtLocation(arg));
const typeNode = checker.typeToTypeNode(type, contextNode, /*flags*/ undefined, tracker);
if (typeNode?.kind === SyntaxKind.ImportType) {
const importableReference = tryGetAutoImportableReferenceFromImportTypeNode(typeNode, type, scriptTarget);
if (importableReference) {
importSymbols(importAdder, importableReference.symbols);
return importableReference.typeReference;
}
}
return typeNode;
});
const types = map(args, arg =>
typeToAutoImportableTypeNode(checker, importAdder, checker.getBaseTypeOfLiteralType(checker.getTypeAtLocation(arg)), contextNode, scriptTarget, /*flags*/ undefined, tracker));
const names = map(args, arg =>
isIdentifier(arg) ? arg.text : isPropertyAccessExpression(arg) && isIdentifier(arg.name) ? arg.name.text : undefined);
const contextualType = checker.getContextualType(call);
Expand All @@ -255,6 +245,18 @@ namespace ts.codefix {
body ? createStubbedMethodBody(context.preferences) : undefined);
}

export function typeToAutoImportableTypeNode(checker: TypeChecker, importAdder: ImportAdder, type: Type, contextNode: Node, scriptTarget: ScriptTarget, flags?: NodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined {
const typeNode = checker.typeToTypeNode(type, contextNode, flags, tracker);
if (typeNode && isImportTypeNode(typeNode)) {
const importableReference = tryGetAutoImportableReferenceFromImportTypeNode(typeNode, type, scriptTarget);
if (importableReference) {
importSymbols(importAdder, importableReference.symbols);
return importableReference.typeReference;
}
}
return typeNode;
}

function createDummyParameters(argCount: number, names: (string | undefined)[] | undefined, types: (TypeNode | undefined)[] | undefined, minArgumentCount: number | undefined, inJs: boolean): ParameterDeclaration[] {
const parameters: ParameterDeclaration[] = [];
for (let i = 0; i < argCount; i++) {
Expand Down Expand Up @@ -456,7 +458,7 @@ namespace ts.codefix {
return createQualifiedName(replaceFirstIdentifierOfEntityName(name.left, newIdentifier), name.right);
}

function importSymbols(importAdder: ImportAdder, symbols: readonly Symbol[]) {
export function importSymbols(importAdder: ImportAdder, symbols: readonly Symbol[]) {
symbols.forEach(s => importAdder.addImportFromExportedSymbol(s, /*usageIsTypeOnly*/ true));
}
}
5 changes: 4 additions & 1 deletion src/services/refactors/extractSymbol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,8 @@ namespace ts.refactor.extractSymbol {
context: RefactorContext): RefactorEditInfo {

const checker = context.program.getTypeChecker();
const scriptTarget = getEmitScriptTarget(context.program.getCompilerOptions());
const importAdder = codefix.createImportAdder(context.file, context.program, context.preferences, context.host);

// Make a unique name for the extracted function
const file = scope.getSourceFile();
Expand All @@ -737,7 +739,7 @@ namespace ts.refactor.extractSymbol {
let type = checker.getTypeOfSymbolAtLocation(usage.symbol, usage.node);
// Widen the type so we don't emit nonsense annotations like "function fn(x: 3) {"
type = checker.getBaseTypeOfLiteralType(type);
typeNode = checker.typeToTypeNode(type, scope, NodeBuilderFlags.NoTruncation);
typeNode = codefix.typeToAutoImportableTypeNode(checker, importAdder, type, scope, scriptTarget, NodeBuilderFlags.NoTruncation);
}

const paramDecl = createParameter(
Expand Down Expand Up @@ -823,6 +825,7 @@ namespace ts.refactor.extractSymbol {
else {
changeTracker.insertNodeAtEndOfScope(context.file, scope, newFunction);
}
importAdder.writeFixes(changeTracker);

const newNodes: Node[] = [];
// replace range with function call
Expand Down
35 changes: 35 additions & 0 deletions tests/cases/fourslash/extract-method32.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/// <reference path='fourslash.ts' />

// @Filename: /a.ts
////export interface A {
//// x: number;
////}
////export const a: A = { x: 1 };

// @Filename: /b.ts
////import { a } from "./a";
////
////function foo() {
//// const arg = a;
//// /*a*/console.log(arg);/*b*/
////}

goTo.file("/b.ts");
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Extract Symbol",
actionName: "function_scope_1",
actionDescription: "Extract to function in module scope",
newContent:
`import { a, A } from "./a";

function foo() {
const arg = a;
/*RENAME*/newFunction(arg);
}

function newFunction(arg: A) {
console.log(arg);
}
`
});
44 changes: 44 additions & 0 deletions tests/cases/fourslash/extract-method33.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/// <reference path='fourslash.ts' />

// @Filename: /a.ts
////export interface A {
//// x: number;
////}

// @Filename: /b.ts
////import { A } from "./a";
////export interface B<T> {
//// payload: T;
////}
////export const b: B<A> = {
//// payload: { x: 1 }
////}

// @Filename: /c.ts
////import { b } from "./b";
////
////function foo() {
//// const prop = b;
//// /*a*/console.log(prop);/*b*/
////}

goTo.file("/c.ts");
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Extract Symbol",
actionName: "function_scope_1",
actionDescription: "Extract to function in module scope",
newContent:
`import { b, B } from "./b";
import { A } from "./a";

function foo() {
const prop = b;
/*RENAME*/newFunction(prop);
}

function newFunction(prop: B<A>) {
console.log(prop);
}
`
});
54 changes: 54 additions & 0 deletions tests/cases/fourslash/extract-method34.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/// <reference path='fourslash.ts' />

// @Filename: /a.ts
////export interface A {
//// x: number;
////}

// @Filename: /b.ts
////export interface B<T> {
//// payload: T;
////}

// @Filename: /c.ts
////import { A } from "./a";
////import { B } from "./b";
////export interface C<T> {
//// payload: T;
////}
////
////export const c: C<B<A>> = {
//// payload: {
//// payload: { x: 1 }
//// }
////}

// @Filename: /d.ts
////import { c } from "./c";
////
////function foo() {
//// const prop = c;
//// /*a*/console.log(prop);/*b*/
////}

goTo.file("/c.ts");
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Extract Symbol",
actionName: "function_scope_1",
actionDescription: "Extract to function in module scope",
newContent:
`import { c, C } from "./c";
import { B } from "./b";
import { A } from "./a";

function foo() {
const prop = c;
/*RENAME*/newFunction(prop);
}

function newFunction(prop: C<B<A>>) {
console.log(prop);
}
`
});
38 changes: 38 additions & 0 deletions tests/cases/fourslash/extract-method35.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/// <reference path='fourslash.ts' />

// @Filename: /a.ts
////export interface A {
//// x: number;
////}
////export const a: A = { x: 1 };

// @Filename: /b.ts
////import { a } from "./a";
////
////class Foo {
//// foo() {
//// const arg = a;
//// /*a*/console.log(arg);/*b*/
//// }
////}

goTo.file("/b.ts");
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Extract Symbol",
actionName: "function_scope_1",
actionDescription: "Extract to method in class 'Foo'",
newContent:
`import { a, A } from "./a";

class Foo {
foo() {
const arg = a;
this./*RENAME*/newMethod(arg);
}

private newMethod(arg: A) {
console.log(arg);
}
}`
});
47 changes: 47 additions & 0 deletions tests/cases/fourslash/extract-method36.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/// <reference path='fourslash.ts' />

// @Filename: /a.ts
////export interface A {
//// x: number;
////}

// @Filename: /b.ts
////import { A } from "./a";
////export interface B<T> {
//// payload: T;
////}
////export const b: B<A> = {
//// payload: { x: 1 }
////}

// @Filename: /c.ts
////import { b } from "./b";
////
////class Foo {
//// foo() {
//// const arg = b;
//// /*a*/console.log(arg);/*b*/
//// }
////}

goTo.file("/c.ts");
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Extract Symbol",
actionName: "function_scope_1",
actionDescription: "Extract to method in class 'Foo'",
newContent:
`import { b, B } from "./b";
import { A } from "./a";

class Foo {
foo() {
const arg = b;
this./*RENAME*/newMethod(arg);
}

private newMethod(arg: B<A>) {
console.log(arg);
}
}`
});
57 changes: 57 additions & 0 deletions tests/cases/fourslash/extract-method37.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/// <reference path='fourslash.ts' />

// @Filename: /a.ts
////export interface A {
//// x: number;
////}

// @Filename: /b.ts
////export interface B<T> {
//// payload: T;
////}

// @Filename: /c.ts
////import { A } from "./a";
////import { B } from "./b";
////export interface C<T> {
//// payload: T;
////}
////
////export const c: C<B<A>> = {
//// payload: {
//// payload: { x: 1 }
//// }
////}

// @Filename: /d.ts
////import { c } from "./c";
////
////class Foo {
//// foo() {
//// const arg = c;
//// /*a*/console.log(arg);/*b*/
//// }
////}

goTo.file("/d.ts");
goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Extract Symbol",
actionName: "function_scope_1",
actionDescription: "Extract to method in class 'Foo'",
newContent:
`import { c, C } from "./c";
import { B } from "./b";
import { A } from "./a";

class Foo {
foo() {
const arg = c;
this./*RENAME*/newMethod(arg);
}

private newMethod(arg: C<B<A>>) {
console.log(arg);
}
}`
});