Skip to content
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

Hierarchical refactorings #41975

Merged
merged 20 commits into from
Dec 23, 2020
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: 28 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -6139,6 +6139,34 @@
"category": "Message",
"code": 95148
},
"Return type must be inferred from a function": {
"category": "Message",
"code": 95149
},
"Could not determine function return type": {
"category": "Message",
"code": 95150
},
"Could not convert to arrow function": {
"category": "Message",
"code": 95151
},
"Could not convert to named function": {
"category": "Message",
"code": 95152
},
"Could not convert to anonymous function": {
"category": "Message",
"code": 95153
},
"Can only convert string concatenation": {
"category": "Message",
"code": 95154
},
"Selection is not a valid statement or statements": {
"category": "Message",
"code": 95155
},

"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
"category": "Error",
Expand Down
18 changes: 12 additions & 6 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3372,6 +3372,12 @@ namespace FourSlash {
}
}

public verifyRefactorKindsAvailable(kind: string, expected: string[], preferences = ts.emptyOptions) {
const refactors = this.getApplicableRefactorsAtSelection("invoked", kind, preferences);
const availableKinds = ts.flatMap(refactors, refactor => refactor.actions).map(action => action.kind);
assert.deepEqual(availableKinds.sort(), expected.sort(), `Expected kinds to be equal`);
}

public verifyRefactorsAvailable(names: readonly string[]): void {
assert.deepEqual(unique(this.getApplicableRefactorsAtSelection(), r => r.name), names);
}
Expand Down Expand Up @@ -3785,14 +3791,14 @@ namespace FourSlash {
test(renameKeys(newFileContents, key => pathUpdater(key) || key), "with file moved");
}

private getApplicableRefactorsAtSelection(triggerReason: ts.RefactorTriggerReason = "implicit") {
return this.getApplicableRefactorsWorker(this.getSelection(), this.activeFile.fileName, ts.emptyOptions, triggerReason);
private getApplicableRefactorsAtSelection(triggerReason: ts.RefactorTriggerReason = "implicit", kind?: string, preferences = ts.emptyOptions) {
return this.getApplicableRefactorsWorker(this.getSelection(), this.activeFile.fileName, preferences, triggerReason, kind);
}
private getApplicableRefactors(rangeOrMarker: Range | Marker, preferences = ts.emptyOptions, triggerReason: ts.RefactorTriggerReason = "implicit"): readonly ts.ApplicableRefactorInfo[] {
return this.getApplicableRefactorsWorker("position" in rangeOrMarker ? rangeOrMarker.position : rangeOrMarker, rangeOrMarker.fileName, preferences, triggerReason); // eslint-disable-line no-in-operator
private getApplicableRefactors(rangeOrMarker: Range | Marker, preferences = ts.emptyOptions, triggerReason: ts.RefactorTriggerReason = "implicit", kind?: string): readonly ts.ApplicableRefactorInfo[] {
return this.getApplicableRefactorsWorker("position" in rangeOrMarker ? rangeOrMarker.position : rangeOrMarker, rangeOrMarker.fileName, preferences, triggerReason, kind); // eslint-disable-line no-in-operator
}
private getApplicableRefactorsWorker(positionOrRange: number | ts.TextRange, fileName: string, preferences = ts.emptyOptions, triggerReason: ts.RefactorTriggerReason): readonly ts.ApplicableRefactorInfo[] {
return this.languageService.getApplicableRefactors(fileName, positionOrRange, preferences, triggerReason) || ts.emptyArray;
private getApplicableRefactorsWorker(positionOrRange: number | ts.TextRange, fileName: string, preferences = ts.emptyOptions, triggerReason: ts.RefactorTriggerReason, kind?: string): readonly ts.ApplicableRefactorInfo[] {
return this.languageService.getApplicableRefactors(fileName, positionOrRange, preferences, triggerReason, kind) || ts.emptyArray;
}

public configurePlugin(pluginName: string, configuration: any): void {
Expand Down
4 changes: 4 additions & 0 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ namespace FourSlashInterface {
this.state.verifyRefactorAvailable(this.negative, triggerReason, name, actionName);
}

public refactorKindAvailable(kind: string, expected: string[], preferences = ts.emptyOptions) {
this.state.verifyRefactorKindsAvailable(kind, expected, preferences);
}

public toggleLineComment(newFileContent: string) {
this.state.toggleLineComment(newFileContent);
}
Expand Down
8 changes: 7 additions & 1 deletion src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,8 @@ namespace ts.server.protocol {
arguments: GetApplicableRefactorsRequestArgs;
}
export type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs & {
triggerReason?: RefactorTriggerReason
triggerReason?: RefactorTriggerReason;
kind?: string;
};

export type RefactorTriggerReason = "implicit" | "invoked";
Expand Down Expand Up @@ -623,6 +624,11 @@ namespace ts.server.protocol {
* the current context.
*/
notApplicableReason?: string;

/**
* The hierarchical dotted name of the refactor action.
*/
kind?: string;
}

export interface GetEditsForRefactorRequest extends Request {
Expand Down
2 changes: 1 addition & 1 deletion src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2100,7 +2100,7 @@ namespace ts.server {
private getApplicableRefactors(args: protocol.GetApplicableRefactorsRequestArgs): protocol.ApplicableRefactorInfo[] {
const { file, project } = this.getFileAndProject(args);
const scriptInfo = project.getScriptInfoForNormalizedPath(file)!;
return project.getLanguageService().getApplicableRefactors(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file), args.triggerReason);
return project.getLanguageService().getApplicableRefactors(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file), args.triggerReason, args.kind);
}

private getEditsForRefactor(args: protocol.GetEditsForRefactorRequestArgs, simplifiedResult: boolean): RefactorEditInfo | protocol.RefactorEditInfo {
Expand Down
37 changes: 14 additions & 23 deletions src/services/codefixes/generateAccessors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ namespace ts.codefix {
type AcceptedNameType = Identifier | StringLiteral;
type ContainerDeclaration = ClassLikeDeclaration | ObjectLiteralExpression;

interface Info {
type Info = AccessorInfo | refactor.RefactorErrorInfo;
interface AccessorInfo {
readonly container: ContainerDeclaration;
readonly isStatic: boolean;
readonly isReadonly: boolean;
Expand All @@ -16,20 +17,12 @@ namespace ts.codefix {
readonly renameAccessor: boolean;
}

type InfoOrError = {
info: Info,
error?: never
} | {
info?: never,
error: string
};

export function generateAccessorFromProperty(file: SourceFile, program: Program, start: number, end: number, context: textChanges.TextChangesContext, _actionName: string): FileTextChanges[] | undefined {
const fieldInfo = getAccessorConvertiblePropertyAtPosition(file, program, start, end);
if (!fieldInfo || !fieldInfo.info) return undefined;
if (!fieldInfo || refactor.isRefactorErrorInfo(fieldInfo)) return undefined;

const changeTracker = textChanges.ChangeTracker.fromContext(context);
const { isStatic, isReadonly, fieldName, accessorName, originalName, type, container, declaration } = fieldInfo.info;
const { isStatic, isReadonly, fieldName, accessorName, originalName, type, container, declaration } = fieldInfo;

suppressLeadingAndTrailingTrivia(fieldName);
suppressLeadingAndTrailingTrivia(accessorName);
Expand Down Expand Up @@ -112,7 +105,7 @@ namespace ts.codefix {
return modifierFlags;
}

export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, program: Program, start: number, end: number, considerEmptySpans = true): InfoOrError | undefined {
export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, program: Program, start: number, end: number, considerEmptySpans = true): Info | undefined {
const node = getTokenAtPosition(file, start);
const cursorRequest = start === end && considerEmptySpans;
const declaration = findAncestor(node.parent, isAcceptedDeclaration);
Expand Down Expand Up @@ -142,17 +135,15 @@ namespace ts.codefix {
const fieldName = createPropertyName(startWithUnderscore ? name : getUniqueName(`_${name}`, file), declaration.name);
const accessorName = createPropertyName(startWithUnderscore ? getUniqueName(name.substring(1), file) : name, declaration.name);
return {
info: {
isStatic: hasStaticModifier(declaration),
isReadonly: hasEffectiveReadonlyModifier(declaration),
type: getDeclarationType(declaration, program),
container: declaration.kind === SyntaxKind.Parameter ? declaration.parent.parent : declaration.parent,
originalName: (<AcceptedNameType>declaration.name).text,
declaration,
fieldName,
accessorName,
renameAccessor: startWithUnderscore
}
isStatic: hasStaticModifier(declaration),
isReadonly: hasEffectiveReadonlyModifier(declaration),
type: getDeclarationType(declaration, program),
container: declaration.kind === SyntaxKind.Parameter ? declaration.parent.parent : declaration.parent,
originalName: (<AcceptedNameType>declaration.name).text,
declaration,
fieldName,
accessorName,
renameAccessor: startWithUnderscore
};
}

Expand Down
4 changes: 3 additions & 1 deletion src/services/refactorProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ namespace ts.refactor {

export function getApplicableRefactors(context: RefactorContext): ApplicableRefactorInfo[] {
return arrayFrom(flatMapIterator(refactors.values(), refactor =>
context.cancellationToken && context.cancellationToken.isCancellationRequested() ? undefined : refactor.getAvailableActions(context)));
context.cancellationToken && context.cancellationToken.isCancellationRequested() ||
!refactor.kinds?.some(kind => refactorKindBeginsWith(kind, context.kind)) ? undefined :
refactor.getAvailableActions(context)));
}

export function getEditsForRefactor(context: RefactorContext, refactorName: string, actionName: string): RefactorEditInfo | undefined {
Expand Down
85 changes: 31 additions & 54 deletions src/services/refactors/addOrRemoveBracesToArrowFunction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,45 +2,40 @@
namespace ts.refactor.addOrRemoveBracesToArrowFunction {
const refactorName = "Add or remove braces in an arrow function";
const refactorDescription = Diagnostics.Add_or_remove_braces_in_an_arrow_function.message;
const addBracesActionName = "Add braces to arrow function";
const removeBracesActionName = "Remove braces from arrow function";
const addBracesActionDescription = Diagnostics.Add_braces_to_arrow_function.message;
const removeBracesActionDescription = Diagnostics.Remove_braces_from_arrow_function.message;
registerRefactor(refactorName, { getEditsForAction, getAvailableActions });

interface Info {
const addBracesAction = {
name: "Add braces to arrow function",
description: Diagnostics.Add_braces_to_arrow_function.message,
kind: "refactor.rewrite.arrow.braces.add",
};
const removeBracesAction = {
name: "Remove braces from arrow function",
description: Diagnostics.Remove_braces_from_arrow_function.message,
kind: "refactor.rewrite.arrow.braces.remove"
};
registerRefactor(refactorName, {
kinds: [removeBracesAction.kind],
getEditsForAction,
getAvailableActions });

interface FunctionBracesInfo {
func: ArrowFunction;
expression: Expression | undefined;
returnStatement?: ReturnStatement;
addBraces: boolean;
}

type InfoOrError = {
info: Info,
error?: never
} | {
info?: never,
error: string
};

function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] {
const { file, startPosition, triggerReason } = context;
const info = getConvertibleArrowFunctionAtPosition(file, startPosition, triggerReason === "invoked");
if (!info) return emptyArray;

if (info.error === undefined) {
if (!isRefactorErrorInfo(info)) {
return [{
name: refactorName,
description: refactorDescription,
actions: [
info.info.addBraces ?
{
name: addBracesActionName,
description: addBracesActionDescription
} : {
name: removeBracesActionName,
description: removeBracesActionDescription
}
info.addBraces ? addBracesAction : removeBracesAction
]
}];
}
Expand All @@ -49,15 +44,10 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction {
return [{
name: refactorName,
description: refactorDescription,
actions: [{
name: addBracesActionName,
description: addBracesActionDescription,
notApplicableReason: info.error
}, {
name: removeBracesActionName,
description: removeBracesActionDescription,
notApplicableReason: info.error
}]
actions: [
{ ...addBracesAction, notApplicableReason: info.error },
{ ...removeBracesAction, notApplicableReason: info.error },
]
}];
}

Expand All @@ -67,19 +57,19 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction {
function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined {
const { file, startPosition } = context;
const info = getConvertibleArrowFunctionAtPosition(file, startPosition);
if (!info || !info.info) return undefined;
Debug.assert(info && !isRefactorErrorInfo(info), "Expected applicable refactor info");

const { expression, returnStatement, func } = info.info;
const { expression, returnStatement, func } = info;

let body: ConciseBody;

if (actionName === addBracesActionName) {
if (actionName === addBracesAction.name) {
const returnStatement = factory.createReturnStatement(expression);
body = factory.createBlock([returnStatement], /* multiLine */ true);
suppressLeadingAndTrailingTrivia(body);
copyLeadingComments(expression!, returnStatement, file, SyntaxKind.MultiLineCommentTrivia, /* hasTrailingNewLine */ true);
}
else if (actionName === removeBracesActionName && returnStatement) {
else if (actionName === removeBracesAction.name && returnStatement) {
const actualExpression = expression || factory.createVoidZero();
body = needsParentheses(actualExpression) ? factory.createParenthesizedExpression(actualExpression) : actualExpression;
suppressLeadingAndTrailingTrivia(body);
Expand All @@ -98,7 +88,7 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction {
return { renameFilename: undefined, renameLocation: undefined, edits };
}

function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, considerFunctionBodies = true): InfoOrError | undefined {
function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number, considerFunctionBodies = true, kind?: string): FunctionBracesInfo | RefactorErrorInfo | undefined {
const node = getTokenAtPosition(file, startPosition);
const func = getContainingFunction(node);

Expand All @@ -118,26 +108,13 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction {
return undefined;
}

if (isExpression(func.body)) {
return {
info: {
func,
addBraces: true,
expression: func.body
}
};
if (refactorKindBeginsWith(addBracesAction.kind, kind) && isExpression(func.body)) {
return { func, addBraces: true, expression: func.body };
}
else if (func.body.statements.length === 1) {
else if (refactorKindBeginsWith(removeBracesAction.kind, kind) && isBlock(func.body) && func.body.statements.length === 1) {
const firstStatement = first(func.body.statements);
if (isReturnStatement(firstStatement)) {
return {
info: {
func,
addBraces: false,
expression: firstStatement.expression,
returnStatement: firstStatement
}
};
return { func, addBraces: false, expression: firstStatement.expression, returnStatement: firstStatement };
}
}
return undefined;
Expand Down
Loading