Skip to content

fix54035, extractType now allows parts of union and intersection types to be extracted #56131

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
merged 8 commits into from
Oct 28, 2023
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
136 changes: 100 additions & 36 deletions src/services/refactors/extractType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
getTokenAtPosition,
getUniqueName,
ignoreSourceNewlines,
isArray,
isConditionalTypeNode,
isFunctionLike,
isIdentifier,
Expand All @@ -44,6 +45,7 @@ import {
isTypePredicateNode,
isTypeQueryNode,
isTypeReferenceNode,
isUnionTypeNode,
JSDocTag,
JSDocTemplateTag,
Node,
Expand All @@ -59,6 +61,7 @@ import {
SymbolFlags,
textChanges,
TextRange,
toArray,
TypeChecker,
TypeElement,
TypeNode,
Expand Down Expand Up @@ -151,15 +154,15 @@ registerRefactor(refactorName, {

interface TypeAliasInfo {
isJS: boolean;
selection: TypeNode;
selection: TypeNode | TypeNode[];
enclosingNode: Node;
typeParameters: readonly TypeParameterDeclaration[];
typeElements?: readonly TypeElement[];
}

interface InterfaceInfo {
isJS: boolean;
selection: TypeNode;
selection: TypeNode | TypeNode[];
enclosingNode: Node;
typeParameters: readonly TypeParameterDeclaration[];
typeElements: readonly TypeElement[];
Expand All @@ -173,29 +176,54 @@ function getRangeToExtract(context: RefactorContext, considerEmptySpans = true):
const current = getTokenAtPosition(file, startPosition);
const range = createTextRangeFromSpan(getRefactorContextSpan(context));
const cursorRequest = range.pos === range.end && considerEmptySpans;
const overlappingRange = nodeOverlapsWithStartEnd(current, file, range.pos, range.end);

const selection = findAncestor(current, node =>
const firstType = findAncestor(current, node =>
node.parent && isTypeNode(node) && !rangeContainsSkipTrivia(range, node.parent, file) &&
(cursorRequest || nodeOverlapsWithStartEnd(current, file, range.pos, range.end)));
if (!selection || !isTypeNode(selection)) return { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) };
(cursorRequest || overlappingRange));
if (!firstType || !isTypeNode(firstType)) return { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) };

const checker = context.program.getTypeChecker();
const enclosingNode = getEnclosingNode(selection, isJS);
const enclosingNode = getEnclosingNode(firstType, isJS);
if (enclosingNode === undefined) return { error: getLocaleSpecificMessage(Diagnostics.No_type_could_be_extracted_from_this_type_node) };

const expandedFirstType = getExpandedSelectionNode(firstType, enclosingNode);
if (!isTypeNode(expandedFirstType)) return { error: getLocaleSpecificMessage(Diagnostics.Selection_is_not_a_valid_type_node) };

const typeList: TypeNode[] = [];
if ((isUnionTypeNode(expandedFirstType.parent) || isIntersectionTypeNode(expandedFirstType.parent)) && range.end > firstType.end) {
// the only extraction cases in which multiple nodes may need to be selected to capture the entire type are union and intersection types
addRange(
typeList,
expandedFirstType.parent.types.filter(type => {
return nodeOverlapsWithStartEnd(type, file, range.pos, range.end);
}),
);
}
const selection = typeList.length > 1 ? typeList : expandedFirstType;

const typeParameters = collectTypeParameters(checker, selection, enclosingNode, file);
if (!typeParameters) return { error: getLocaleSpecificMessage(Diagnostics.No_type_could_be_extracted_from_this_type_node) };

const typeElements = flattenTypeLiteralNodeReference(checker, selection);
return { isJS, selection, enclosingNode, typeParameters, typeElements };
}

function flattenTypeLiteralNodeReference(checker: TypeChecker, node: TypeNode | undefined): readonly TypeElement[] | undefined {
if (!node) return undefined;
if (isIntersectionTypeNode(node)) {
function flattenTypeLiteralNodeReference(checker: TypeChecker, selection: TypeNode | TypeNode[] | undefined): readonly TypeElement[] | undefined {
if (!selection) return undefined;
if (isArray(selection)) {
const result: TypeElement[] = [];
for (const type of selection) {
const flattenedTypeMembers = flattenTypeLiteralNodeReference(checker, type);
if (!flattenedTypeMembers) return undefined;
addRange(result, flattenedTypeMembers);
}
return result;
}
if (isIntersectionTypeNode(selection)) {
const result: TypeElement[] = [];
const seen = new Map<string, true>();
for (const type of node.types) {
for (const type of selection.types) {
const flattenedTypeMembers = flattenTypeLiteralNodeReference(checker, type);
if (!flattenedTypeMembers || !flattenedTypeMembers.every(type => type.name && addToSeen(seen, getNameFromPropertyName(type.name) as string))) {
return undefined;
Expand All @@ -205,22 +233,27 @@ function flattenTypeLiteralNodeReference(checker: TypeChecker, node: TypeNode |
}
return result;
}
else if (isParenthesizedTypeNode(node)) {
return flattenTypeLiteralNodeReference(checker, node.type);
else if (isParenthesizedTypeNode(selection)) {
return flattenTypeLiteralNodeReference(checker, selection.type);
}
else if (isTypeLiteralNode(node)) {
return node.members;
else if (isTypeLiteralNode(selection)) {
return selection.members;
}
return undefined;
}

function rangeContainsSkipTrivia(r1: TextRange, node: Node, file: SourceFile): boolean {
function rangeContainsSkipTrivia(r1: TextRange, node: TextRange, file: SourceFile): boolean {
return rangeContainsStartEnd(r1, skipTrivia(file.text, node.pos), node.end);
}

function collectTypeParameters(checker: TypeChecker, selection: TypeNode, enclosingNode: Node, file: SourceFile): TypeParameterDeclaration[] | undefined {
function collectTypeParameters(checker: TypeChecker, selection: TypeNode | TypeNode[], enclosingNode: Node, file: SourceFile): TypeParameterDeclaration[] | undefined {
const result: TypeParameterDeclaration[] = [];
return visitor(selection) ? undefined : result;
const selectionArray = toArray(selection);
const selectionRange = { pos: selectionArray[0].pos, end: selectionArray[selectionArray.length - 1].end };
for (const t of selectionArray) {
if (visitor(t)) return undefined;
}
return result;

function visitor(node: Node): true | undefined {
if (isTypeReferenceNode(node)) {
Expand All @@ -231,11 +264,11 @@ function collectTypeParameters(checker: TypeChecker, selection: TypeNode, enclos
if (isTypeParameterDeclaration(decl) && decl.getSourceFile() === file) {
// skip extraction if the type node is in the range of the type parameter declaration.
// function foo<T extends { a?: /**/T }>(): void;
if (decl.name.escapedText === typeName.escapedText && rangeContainsSkipTrivia(decl, selection, file)) {
if (decl.name.escapedText === typeName.escapedText && rangeContainsSkipTrivia(decl, selectionRange, file)) {
return true;
}

if (rangeContainsSkipTrivia(enclosingNode, decl, file) && !rangeContainsSkipTrivia(selection, decl, file)) {
if (rangeContainsSkipTrivia(enclosingNode, decl, file) && !rangeContainsSkipTrivia(selectionRange, decl, file)) {
pushIfUnique(result, decl);
break;
}
Expand All @@ -245,25 +278,25 @@ function collectTypeParameters(checker: TypeChecker, selection: TypeNode, enclos
}
else if (isInferTypeNode(node)) {
const conditionalTypeNode = findAncestor(node, n => isConditionalTypeNode(n) && rangeContainsSkipTrivia(n.extendsType, node, file));
if (!conditionalTypeNode || !rangeContainsSkipTrivia(selection, conditionalTypeNode, file)) {
if (!conditionalTypeNode || !rangeContainsSkipTrivia(selectionRange, 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)) {
if (functionLikeNode && functionLikeNode.type && rangeContainsSkipTrivia(functionLikeNode.type, node, file) && !rangeContainsSkipTrivia(selectionRange, 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?.valueDeclaration && rangeContainsSkipTrivia(enclosingNode, symbol.valueDeclaration, file) && !rangeContainsSkipTrivia(selection, symbol.valueDeclaration, file)) {
if (symbol?.valueDeclaration && rangeContainsSkipTrivia(enclosingNode, symbol.valueDeclaration, file) && !rangeContainsSkipTrivia(selectionRange, symbol.valueDeclaration, file)) {
return true;
}
}
else {
if (isThisIdentifier(node.exprName.left) && !rangeContainsSkipTrivia(selection, node.parent, file)) {
if (isThisIdentifier(node.exprName.left) && !rangeContainsSkipTrivia(selectionRange, node.parent, file)) {
return true;
}
}
Expand All @@ -278,20 +311,20 @@ function collectTypeParameters(checker: TypeChecker, selection: TypeNode, enclos
}

function doTypeAliasChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, info: TypeAliasInfo) {
const { enclosingNode, selection, typeParameters } = info;

const newTypeNode = factory.createTypeAliasDeclaration(
const { enclosingNode, typeParameters } = info;
const { firstTypeNode, lastTypeNode, newTypeNode } = getNodesToEdit(info);
const newTypeDeclaration = factory.createTypeAliasDeclaration(
/*modifiers*/ undefined,
name,
typeParameters.map(id => factory.updateTypeParameterDeclaration(id, id.modifiers, id.name, id.constraint, /*defaultType*/ undefined)),
selection,
newTypeNode,
);
changes.insertNodeBefore(file, enclosingNode, ignoreSourceNewlines(newTypeNode), /*blankLineBetween*/ true);
changes.replaceNode(file, selection, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /*typeArguments*/ undefined))), { leadingTriviaOption: textChanges.LeadingTriviaOption.Exclude, trailingTriviaOption: textChanges.TrailingTriviaOption.ExcludeWhitespace });
changes.insertNodeBefore(file, enclosingNode, ignoreSourceNewlines(newTypeDeclaration), /*blankLineBetween*/ true);
changes.replaceNodeRange(file, firstTypeNode, lastTypeNode, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /*typeArguments*/ undefined))), { leadingTriviaOption: textChanges.LeadingTriviaOption.Exclude, trailingTriviaOption: textChanges.TrailingTriviaOption.ExcludeWhitespace });
}

function doInterfaceChange(changes: textChanges.ChangeTracker, file: SourceFile, name: string, info: InterfaceInfo) {
const { enclosingNode, selection, typeParameters, typeElements } = info;
const { enclosingNode, typeParameters, typeElements } = info;

const newTypeNode = factory.createInterfaceDeclaration(
/*modifiers*/ undefined,
Expand All @@ -302,17 +335,21 @@ function doInterfaceChange(changes: textChanges.ChangeTracker, file: SourceFile,
);
setTextRange(newTypeNode, typeElements[0]?.parent);
changes.insertNodeBefore(file, enclosingNode, ignoreSourceNewlines(newTypeNode), /*blankLineBetween*/ true);
changes.replaceNode(file, selection, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /*typeArguments*/ undefined))), { leadingTriviaOption: textChanges.LeadingTriviaOption.Exclude, trailingTriviaOption: textChanges.TrailingTriviaOption.ExcludeWhitespace });

const { firstTypeNode, lastTypeNode } = getNodesToEdit(info);
changes.replaceNodeRange(file, firstTypeNode, lastTypeNode, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /*typeArguments*/ undefined))), { leadingTriviaOption: textChanges.LeadingTriviaOption.Exclude, trailingTriviaOption: textChanges.TrailingTriviaOption.ExcludeWhitespace });
}

function doTypedefChange(changes: textChanges.ChangeTracker, context: RefactorContext, file: SourceFile, name: string, info: ExtractInfo) {
const { enclosingNode, selection, typeParameters } = info;

setEmitFlags(selection, EmitFlags.NoComments | EmitFlags.NoNestedComments);
toArray(info.selection).forEach(typeNode => {
setEmitFlags(typeNode, EmitFlags.NoComments | EmitFlags.NoNestedComments);
});
const { enclosingNode, typeParameters } = info;
const { firstTypeNode, lastTypeNode, newTypeNode } = getNodesToEdit(info);

const node = factory.createJSDocTypedefTag(
factory.createIdentifier("typedef"),
factory.createJSDocTypeExpression(selection),
factory.createJSDocTypeExpression(newTypeNode),
factory.createIdentifier(name),
);

Expand All @@ -339,9 +376,36 @@ function doTypedefChange(changes: textChanges.ChangeTracker, context: RefactorCo
else {
changes.insertNodeBefore(file, enclosingNode, jsDoc, /*blankLineBetween*/ true);
}
changes.replaceNode(file, selection, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /*typeArguments*/ undefined))));
changes.replaceNodeRange(file, firstTypeNode, lastTypeNode, factory.createTypeReferenceNode(name, typeParameters.map(id => factory.createTypeReferenceNode(id.name, /*typeArguments*/ undefined))));
}

function getNodesToEdit(info: ExtractInfo) {
if (isArray(info.selection)) {
return {
firstTypeNode: info.selection[0],
lastTypeNode: info.selection[info.selection.length - 1],
newTypeNode: isUnionTypeNode(info.selection[0].parent) ? factory.createUnionTypeNode(info.selection) : factory.createIntersectionTypeNode(info.selection),
};
}
return {
firstTypeNode: info.selection,
lastTypeNode: info.selection,
newTypeNode: info.selection,
};
}

function getEnclosingNode(node: Node, isJS: boolean) {
return findAncestor(node, isStatement) || (isJS ? findAncestor(node, isJSDoc) : undefined);
}

function getExpandedSelectionNode(firstType: Node, enclosingNode: Node) {
// intended to capture the entire type in cases where the user selection is not exactly the entire type
// currently only implemented for union and intersection types
return findAncestor(firstType, node => {
if (node === enclosingNode) return "quit";
if (isUnionTypeNode(node.parent) || isIntersectionTypeNode(node.parent)) {
return true;
}
return false;
}) ?? firstType;
}
18 changes: 18 additions & 0 deletions tests/cases/fourslash/refactorExtractType78.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts' />

//// type A = { a: string } | /*1*/{ b: string } | { c: string }/*2*/ | { d: string };

goTo.select("1", "2");
edit.applyRefactor({
refactorName: "Extract type",
actionName: "Extract to type alias",
actionDescription: "Extract to type alias",
newContent:
`type /*RENAME*/NewType = {
b: string;
} | {
c: string;
};

type A = { a: string } | NewType | { d: string };`,
});
18 changes: 18 additions & 0 deletions tests/cases/fourslash/refactorExtractType79.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts' />

//// type B = string;
//// type C = number;
//// type A = { a: string } | /*1*/B | C/*2*/;

goTo.select("1", "2");
edit.applyRefactor({
refactorName: "Extract type",
actionName: "Extract to type alias",
actionDescription: "Extract to type alias",
newContent:
`type B = string;
type C = number;
type /*RENAME*/NewType = B | C;

type A = { a: string } | NewType;`,
});
24 changes: 24 additions & 0 deletions tests/cases/fourslash/refactorExtractType80.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/// <reference path='fourslash.ts' />

//// type B = string;
//// type C = number;
////
//// export function foo<T extends boolean | /*1*/B | C/*2*/>(x: T): T {
//// return x;
//// }

goTo.select("1", "2");
edit.applyRefactor({
refactorName: "Extract type",
actionName: "Extract to type alias",
actionDescription: "Extract to type alias",
newContent:
`type B = string;
type C = number;

type /*RENAME*/NewType = B | C;

export function foo<T extends boolean | NewType>(x: T): T {
return x;
}`,
});
17 changes: 17 additions & 0 deletions tests/cases/fourslash/refactorExtractType81.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/// <reference path='fourslash.ts' />

//// type A = { a: string } & /*1*/{ b: string } & { c: string }/*2*/;

goTo.select("1", "2");
edit.applyRefactor({
refactorName: "Extract type",
actionName: "Extract to interface",
actionDescription: "Extract to interface",
newContent:
`interface /*RENAME*/NewType {
b: string;
c: string;
}

type A = { a: string } & NewType;`,
});
20 changes: 20 additions & 0 deletions tests/cases/fourslash/refactorExtractType82.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />

//// type A<T,S> = /*1*/{ a: string } | { b: T } | { c: string }/*2*/ | { d: string } | S;

goTo.select("1", "2");
edit.applyRefactor({
refactorName: "Extract type",
actionName: "Extract to type alias",
actionDescription: "Extract to type alias",
newContent:
`type /*RENAME*/NewType<T> = {
a: string;
} | {
b: T;
} | {
c: string;
};

type A<T,S> = NewType<T> | { d: string } | S;`,
});
20 changes: 20 additions & 0 deletions tests/cases/fourslash/refactorExtractType83.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />

//// type A = { a: str/*1*/ing } | { b: string } | { c: string }/*2*/;

goTo.select("1", "2");
edit.applyRefactor({
refactorName: "Extract type",
actionName: "Extract to type alias",
actionDescription: "Extract to type alias",
newContent:
`type /*RENAME*/NewType = {
a: string;
} | {
b: string;
} | {
c: string;
};

type A = NewType;`,
});
Loading