-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
add refactor of extract type #30562
add refactor of extract type #30562
Changes from all commits
0b66cfc
8a6f1ed
3948938
379de4b
0f414b4
9e405fc
ead7b05
5749f7f
0e37661
0c1866c
65ecee5
26ff926
566da68
73e8297
5c28b60
a861ec4
788d0d2
ee09157
af994fc
2a8a7e4
1dd1c68
bff56b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,147 @@ | ||
/* @internal */ | ||
namespace ts.refactor { | ||
const refactorName = "Extract type"; | ||
const extractToTypeAlias = "Extract to type alias"; | ||
const extractToTypeDef = "Extract to typedef"; | ||
registerRefactor(refactorName, { | ||
getAvailableActions(context): ReadonlyArray<ApplicableRefactorInfo> { | ||
const info = getRangeToExtract(context); | ||
if (!info) return emptyArray; | ||
|
||
return [{ | ||
name: refactorName, | ||
description: getLocaleSpecificMessage(Diagnostics.Extract_type), | ||
actions: [info.isJS ? { | ||
name: extractToTypeDef, description: getLocaleSpecificMessage(Diagnostics.Extract_to_typedef) | ||
} : { | ||
name: extractToTypeAlias, description: getLocaleSpecificMessage(Diagnostics.Extract_to_type_alias) | ||
}] | ||
}]; | ||
}, | ||
getEditsForAction(context, actionName): RefactorEditInfo { | ||
Debug.assert(actionName === extractToTypeAlias || actionName === extractToTypeDef); | ||
const { file } = context; | ||
const info = Debug.assertDefined(getRangeToExtract(context)); | ||
Debug.assert(actionName === extractToTypeAlias && !info.isJS || actionName === extractToTypeDef && info.isJS); | ||
|
||
const name = getUniqueName("NewType", file); | ||
const edits = textChanges.ChangeTracker.with(context, changes => info.isJS ? | ||
doTypedefChange(changes, file, name, info.firstStatement, info.selection, info.typeParameters) : | ||
doTypeAliasChange(changes, file, name, info.firstStatement, info.selection, info.typeParameters)); | ||
|
||
const renameFilename = file.fileName; | ||
const renameLocation = getRenameLocation(edits, renameFilename, name, /*preferLastLocation*/ false); | ||
return { edits, renameFilename, renameLocation }; | ||
} | ||
}); | ||
|
||
interface Info { isJS: boolean; selection: TypeNode; firstStatement: Statement; typeParameters: ReadonlyArray<TypeParameterDeclaration>; } | ||
|
||
function getRangeToExtract(context: RefactorContext): Info | undefined { | ||
const { file, startPosition } = context; | ||
const isJS = isSourceFileJS(file); | ||
const current = getTokenAtPosition(file, startPosition); | ||
const range = createTextRangeFromSpan(getRefactorContextSpan(context)); | ||
|
||
const selection = findAncestor(current, (node => node.parent && rangeContainsSkipTrivia(range, node, file) && !rangeContainsSkipTrivia(range, node.parent, file))); | ||
if (!selection || !isTypeNode(selection)) return undefined; | ||
|
||
const checker = context.program.getTypeChecker(); | ||
const firstStatement = Debug.assertDefined(isJS ? findAncestor(selection, isStatementAndHasJSDoc) : findAncestor(selection, isStatement)); | ||
const typeParameters = collectTypeParameters(checker, selection, firstStatement, file); | ||
if (!typeParameters) return undefined; | ||
|
||
return { isJS, selection, firstStatement, typeParameters }; | ||
} | ||
|
||
function isStatementAndHasJSDoc(n: Node): n is (Statement & HasJSDoc) { | ||
return isStatement(n) && hasJSDocNodes(n); | ||
} | ||
|
||
function rangeContainsSkipTrivia(r1: TextRange, node: Node, file: SourceFile): boolean { | ||
return rangeContainsStartEnd(r1, skipTrivia(file.text, node.pos), node.end); | ||
} | ||
|
||
function collectTypeParameters(checker: TypeChecker, selection: TypeNode, statement: Statement, file: SourceFile): TypeParameterDeclaration[] | undefined { | ||
const result: TypeParameterDeclaration[] = []; | ||
return visitor(selection) ? undefined : result; | ||
|
||
function visitor(node: Node): true | undefined { | ||
if (isTypeReferenceNode(node)) { | ||
if (isIdentifier(node.typeName)) { | ||
const symbol = checker.resolveName(node.typeName.text, node.typeName, SymbolFlags.TypeParameter, /* excludeGlobals */ true); | ||
if (symbol) { | ||
const declaration = cast(first(symbol.declarations), isTypeParameterDeclaration); | ||
if (rangeContainsSkipTrivia(statement, declaration, file) && !rangeContainsSkipTrivia(selection, declaration, file)) { | ||
result.push(declaration); | ||
} | ||
} | ||
} | ||
} | ||
else if (isInferTypeNode(node)) { | ||
const conditionalTypeNode = findAncestor(node, n => isConditionalTypeNode(n) && rangeContainsSkipTrivia(n.extendsType, node, file)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if the selection contains only a syntactically-correct infer type, then I think this code should (1) not set Test cases 31-33 show that this works, but I don't understand why. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That caused:
in the following case: type A = /*a*/Promise/*b*/ The selection is the In cases 31 and 32 were set the type Item<T> = T extends (infer /*a*/P/*b*/)[] ? P : never The selection is the |
||
if (!conditionalTypeNode || !rangeContainsSkipTrivia(selection, conditionalTypeNode, file)) { | ||
return true; | ||
} | ||
} | ||
else if ((isTypePredicateNode(node) || isThisTypeNode(node))) { | ||
const functionLikeNode = findAncestor(node.parent, isFunctionLike); | ||
if (functionLikeNode && functionLikeNode.type && rangeContainsSkipTrivia(functionLikeNode.type, node, file) && !rangeContainsSkipTrivia(selection, functionLikeNode, file)) { | ||
return true; | ||
} | ||
} | ||
else if (isTypeQueryNode(node)) { | ||
if (isIdentifier(node.exprName)) { | ||
const symbol = checker.resolveName(node.exprName.text, node.exprName, SymbolFlags.Value, /* excludeGlobals */ false); | ||
if (symbol && rangeContainsSkipTrivia(statement, symbol.valueDeclaration, file) && !rangeContainsSkipTrivia(selection, symbol.valueDeclaration, file)) { | ||
return true; | ||
} | ||
} | ||
else { | ||
if (isThisIdentifier(node.exprName.left) && !rangeContainsSkipTrivia(selection, node.parent, file)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should skip class C {
m<T>(): /*a*/T | this | number/*b*/ {
return {} as any
}
} currently produces |
||
return true; | ||
} | ||
} | ||
} | ||
return forEachChild(node, visitor); | ||
} | ||
} | ||
|
||
function doTypeAliasChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, firstStatement: Statement, selection: TypeNode, typeParameters: ReadonlyArray<TypeParameterDeclaration>) { | ||
const newTypeNode = createTypeAliasDeclaration( | ||
/* decorators */ undefined, | ||
/* modifiers */ undefined, | ||
name, | ||
typeParameters.map(id => updateTypeParameterDeclaration(id, id.name, id.constraint, /* defaultType */ undefined)), | ||
selection | ||
); | ||
changes.insertNodeBefore(file, firstStatement, newTypeNode, /* blankLineBetween */ true); | ||
changes.replaceNode(file, selection, createTypeReferenceNode(name, typeParameters.map(id => createTypeReferenceNode(id.name, /* typeArguments */ undefined)))); | ||
} | ||
|
||
function doTypedefChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, firstStatement: Statement, selection: TypeNode, typeParameters: ReadonlyArray<TypeParameterDeclaration>) { | ||
const node = <JSDocTypedefTag>createNode(SyntaxKind.JSDocTypedefTag); | ||
node.tagName = createIdentifier("typedef"); // TODO: jsdoc factory https://github.com/Microsoft/TypeScript/pull/29539 | ||
node.fullName = createIdentifier(name); | ||
node.name = node.fullName; | ||
node.typeExpression = createJSDocTypeExpression(selection); | ||
|
||
const templates: JSDocTemplateTag[] = []; | ||
forEach(typeParameters, typeParameter => { | ||
const constraint = getEffectiveConstraintOfTypeParameter(typeParameter); | ||
|
||
const template = <JSDocTemplateTag>createNode(SyntaxKind.JSDocTemplateTag); | ||
template.tagName = createIdentifier("template"); | ||
template.constraint = constraint && cast(constraint, isJSDocTypeExpression); | ||
|
||
const parameter = <TypeParameterDeclaration>createNode(SyntaxKind.TypeParameter); | ||
parameter.name = typeParameter.name; | ||
template.typeParameters = createNodeArray([parameter]); | ||
|
||
templates.push(template); | ||
}); | ||
|
||
changes.insertNodeBefore(file, firstStatement, createJSDocComment(/* comment */ undefined, createNodeArray(concatenate<JSDocTag>(templates, [node]))), /* blankLineBetween */ true); | ||
changes.replaceNode(file, selection, createTypeReferenceNode(name, typeParameters.map(id => createTypeReferenceNode(id.name, /* typeArguments */ undefined)))); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// var x: /*a*/{ a?: number, b?: string }/*b*/ = { }; | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType = { | ||
a?: number; | ||
b?: string; | ||
}; | ||
|
||
var x: NewType = { };`, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// function foo(a: number, b?: number, ...c: number[]): /*a*/boolean/*b*/ { | ||
//// return false as boolean | ||
//// } | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType = boolean; | ||
|
||
function foo(a: number, b?: number, ...c: number[]): NewType { | ||
return false as boolean | ||
}`, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// function foo(a: number, b?: number, ...c: number[]): boolean { | ||
//// return false as /*a*/boolean/*b*/ | ||
//// } | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `function foo(a: number, b?: number, ...c: number[]): boolean { | ||
type /*RENAME*/NewType = boolean; | ||
|
||
return false as NewType | ||
}`, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// interface A<T = /*a*/string/*b*/> { | ||
//// a: boolean | ||
//// b: number | ||
//// c: T | ||
//// } | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType = string; | ||
|
||
interface A<T = NewType> { | ||
a: boolean | ||
b: number | ||
c: T | ||
}`, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// interface A<T = string> { | ||
//// a: /*a*/boolean/*b*/ | ||
//// b: number | ||
//// c: T | ||
//// } | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType = boolean; | ||
|
||
interface A<T = string> { | ||
a: NewType | ||
b: number | ||
c: T | ||
}`, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// type A<T = /*a*/boolean/*b*/> = string | number | T | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType = boolean; | ||
|
||
type A<T = NewType> = string | number | T`, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// type A<T = boolean> = /*a*/string/*b*/ | number | T | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType = string; | ||
|
||
type A<T = boolean> = NewType | number | T`, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// var x: { a?: /*a*/number/*b*/, b?: string } = { }; | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType = number; | ||
|
||
var x: { a?: NewType, b?: string } = { };`, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// type A<T = boolean> = string | number | /*a*/T/*b*/ | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType<T> = T; | ||
|
||
type A<T = boolean> = string | number | NewType<T>`, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// type A<B, C, D = B> = /*a*/Partial<C | string>/*b*/ & D | C | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType<C> = Partial<C | string>; | ||
|
||
type A<B, C, D = B> = NewType<C> & D | C`, | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// type A<B, C, D = B> = /*a*/Partial<C | string | D>/*b*/ & D | C | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType<C, D> = Partial<C | string | D>; | ||
|
||
type A<B, C, D = B> = NewType<C, D> & D | C`, | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// var x: /*a*/string/*b*/ = ''; | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType = string; | ||
|
||
var x: NewType = '';`, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/// <reference path='fourslash.ts' /> | ||
|
||
//// type A<T, U> = () => <T>(v: /*a*/T/*b*/) => (v: T) => <T>(v: T) => U | ||
|
||
goTo.select("a", "b"); | ||
edit.applyRefactor({ | ||
refactorName: "Extract type", | ||
actionName: "Extract to type alias", | ||
actionDescription: "Extract to type alias", | ||
newContent: `type /*RENAME*/NewType<T> = T; | ||
|
||
type A<T, U> = () => <T>(v: NewType<T>) => (v: T) => <T>(v: T) => U`, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it not necessary to handle the case where
node.typeName
is not an identifier but is a qualified name?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could TypeParameter be a
QualifiedName
?In
checkAndCollectionTypeArguments
we only iter the TypeParameter whitch defined in the statementThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think in the common cases where you have to collect the type, the type name is an identifier and not a qualified name. but i'm really not sure, so that's why i asked.
update: came up with an example of a qualified name for the type query check. i think there could be unusual cases where this check can fail.