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

Adopt code action ranges for refactorings #57605

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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: 27 additions & 1 deletion src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
OrganizeImportsArgs,
OutliningSpan,
PatternMatchKind,
PostPasteImportFixes,
Program,
QuickInfo,
RefactorEditInfo,
Expand Down Expand Up @@ -834,7 +835,11 @@ export class SessionClient implements LanguageService {
if (preferences) { // Restore preferences
this.configure(oldPreferences || {});
}
return response.body!; // TODO: GH#18217
// TODO: GH#18217
return response.body!.map(result => ({
...result,
actions: result.actions.map(entry => ({ ...entry, range: entry.range ? createTextSpanFromBounds(this.lineOffsetToPosition(fileName, entry.range.start), this.lineOffsetToPosition(fileName, entry.range.end)) : undefined })),
}));
}

getMoveToRefactoringFileSuggestions(fileName: string, positionOrRange: number | TextRange): { newFileName: string; files: string[]; } {
Expand Down Expand Up @@ -1009,6 +1014,27 @@ export class SessionClient implements LanguageService {
return getSupportedCodeFixes();
}

getPostPasteImportFixes(
targetFile: string,
copies: { text: string; copyRange?: { file: string; range: TextRange; }; }[],
pastes: TextRange[],
_preferences: UserPreferences,
_formatOptions: FormatCodeSettings,
): PostPasteImportFixes {
const args: protocol.GetPostPasteImportFixesRequestArgs = {
file: targetFile,
copies: copies.map(copy => ({ text: copy.text, range: copy.copyRange ? { file: copy.copyRange.file, start: this.positionToOneBasedLineOffset(copy.copyRange.file, copy.copyRange.range.pos), end: this.positionToOneBasedLineOffset(copy.copyRange.file, copy.copyRange.range.end) } : undefined })),
pastes: pastes.map(paste => ({ start: this.positionToOneBasedLineOffset(targetFile, paste.pos), end: this.positionToOneBasedLineOffset(targetFile, paste.end) })),
};
const request = this.processRequest<protocol.GetPostPasteImportFixesRequest>(protocol.CommandTypes.GetPostPasteImportFixes, args);
const response = this.processResponse<protocol.GetPostPasteImportFixesResponse>(request);
if (!response.body) {
return { edits: [] };
}
const edits: FileTextChanges[] = this.convertCodeEditsToTextChanges(response.body.edits);
return { edits };
}

getProgram(): Program {
throw new Error("Program objects are not serializable through the server protocol.");
}
Expand Down
5 changes: 5 additions & 0 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3562,6 +3562,11 @@ export class TestState {
assert.deepEqual(actualModuleSpecifiers, moduleSpecifiers);
}

public verifyPostPasteImportFixes(options: FourSlashInterface.PostPasteImportFixOptions): void {
const editInfo = this.languageService.getPostPasteImportFixes(this.activeFile.fileName, options.copies, options.pastes, options.preferences, this.formatCodeSettings);
this.verifyNewContent({ newFileContent: options.newFileContents }, editInfo.edits);
}

public verifyDocCommentTemplate(expected: ts.TextInsertion | undefined, options?: ts.DocCommentTemplateOptions) {
const name = "verifyDocCommentTemplate";
const actual = this.languageService.getDocCommentTemplateAtPosition(this.activeFile.fileName, this.currentCaretPosition, options || { generateReturnInDocTemplate: true }, this.formatCodeSettings)!;
Expand Down
18 changes: 18 additions & 0 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,10 @@ export class Verify extends VerifyNegatable {
public organizeImports(newContent: string, mode?: ts.OrganizeImportsMode, preferences?: ts.UserPreferences): void {
this.state.verifyOrganizeImports(newContent, mode, preferences);
}

public postPasteImportFixes(options: PostPasteImportFixOptions): void {
this.state.verifyPostPasteImportFixes(options);
}
}

export class Edit {
Expand Down Expand Up @@ -1883,6 +1887,13 @@ export interface VerifyCodeFixAllOptions {
preferences?: ts.UserPreferences;
}

export interface VerifyPostPasteImportFix {
targetFile: string;
pastes: { text: string; range: ts.TextRange; }[];
copySpan?: { file: string; range: ts.TextRange; };
preferences: ts.UserPreferences;
}

export interface VerifyRefactorOptions {
name: string;
actionName: string;
Expand Down Expand Up @@ -1923,6 +1934,13 @@ export interface MoveToFileOptions {
readonly preferences?: ts.UserPreferences;
}

export interface PostPasteImportFixOptions {
readonly newFileContents: { readonly [fileName: string]: string; };
readonly copies: { text: string; copyRange?: { file: string; range: ts.TextRange; }; }[];
readonly pastes: ts.TextRange[];
readonly preferences: ts.UserPreferences;
}

export type RenameLocationsOptions = readonly RenameLocationOptions[] | {
readonly findInStrings?: boolean;
readonly findInComments?: boolean;
Expand Down
13 changes: 13 additions & 0 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2215,6 +2215,19 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
return this.noDtsResolutionProject;
}

/** @internal */
runWithTemporaryFileUpdate(rootFile: string, pastedText: string, cb: (updatedProgram: Program | undefined, originalProgram: Program | undefined, updatedFile: SourceFile) => void) {
const originalProgram = this.program;
const originalText = this.program?.getSourceFile(rootFile)?.getText();
Debug.assert(this.program && this.program.getSourceFile(rootFile) && originalText);

this.getScriptInfo(rootFile)?.editContent(0, this.program.getSourceFile(rootFile)!.getText().length, pastedText);
this.updateGraph();
cb(this.program, originalProgram, (this.program?.getSourceFile(rootFile))!);
this.getScriptInfo(rootFile)?.editContent(0, this.program.getSourceFile(rootFile)!.getText().length, originalText);
this.updateGraph();
}

/** @internal */
private getCompilerOptionsForNoDtsResolutionProject() {
return {
Expand Down
29 changes: 29 additions & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export const enum CommandTypes {
GetApplicableRefactors = "getApplicableRefactors",
GetEditsForRefactor = "getEditsForRefactor",
GetMoveToRefactoringFileSuggestions = "getMoveToRefactoringFileSuggestions",
GetPostPasteImportFixes = "getPostPasteImportFixes",
/** @internal */
GetEditsForRefactorFull = "getEditsForRefactor-full",

Expand Down Expand Up @@ -630,6 +631,29 @@ export interface GetMoveToRefactoringFileSuggestions extends Response {
};
}

/**
* Request refactorings at a given position post pasting text from some other location.
*/

export interface GetPostPasteImportFixesRequest extends Request {
command: CommandTypes.GetPostPasteImportFixes;
arguments: GetPostPasteImportFixesRequestArgs;
}

export type GetPostPasteImportFixesRequestArgs = FileRequestArgs & {
copies: {
text: string;
range?: FileSpan;
}[];
pastes: TextSpan[];
};
export interface GetPostPasteImportFixesResponse extends Response {
body: PostPasteImportAction;
}
export interface PostPasteImportAction {
edits: FileCodeEdits[];
}

/**
* A set of one or more available refactoring actions, grouped under a parent refactoring.
*/
Expand Down Expand Up @@ -688,6 +712,11 @@ export interface RefactorActionInfo {
* when calling 'GetEditsForRefactor'.
*/
isInteractive?: boolean;

/**
* Range of code the refactoring will be applied to.
*/
range?: TextSpan;
}

export interface GetEditsForRefactorRequest extends Request {
Expand Down
31 changes: 30 additions & 1 deletion src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ import {
perfLogger,
PerformanceEvent,
PossibleProgramFileInfo,
PostPasteImportFixes,
Program,
QuickInfo,
RefactorEditInfo,
Expand Down Expand Up @@ -2751,7 +2752,8 @@ export class Session<TMessage = string> implements EventSender {
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, args.kind, args.includeInteractiveActions);
const result = project.getLanguageService().getApplicableRefactors(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file), args.triggerReason, args.kind, args.includeInteractiveActions);
return result.map(result => ({ ...result, actions: result.actions.map(action => ({ ...action, range: action.range ? toProtocolTextSpan(action.range, scriptInfo) : undefined })) }));
}

private getEditsForRefactor(args: protocol.GetEditsForRefactorRequestArgs, simplifiedResult: boolean): RefactorEditInfo | protocol.RefactorEditInfo {
Expand Down Expand Up @@ -2796,6 +2798,26 @@ export class Session<TMessage = string> implements EventSender {
return project.getLanguageService().getMoveToRefactoringFileSuggestions(file, this.extractPositionOrRange(args, scriptInfo), this.getPreferences(file));
}

private getPostPasteImportFixes(args: protocol.GetPostPasteImportFixesRequestArgs): protocol.PostPasteImportAction | undefined {
const { file, project } = this.getFileAndProject(args);
const copyFile = args.copies[0].range ? args.copies[0].range.file : undefined;
const result = project.getLanguageService().getPostPasteImportFixes(
file,
args.copies.map(copy => ({
text: copy.text,
copyRange: copy.range && copyFile
? { file: copy.range.file, range: this.getRange({ file: copyFile, startLine: copy.range.start.line, startOffset: copy.range.start.offset, endLine: copy.range.end.line, endOffset: copy.range.end.offset }, project.getScriptInfoForNormalizedPath(toNormalizedPath(copyFile))!) } : undefined,
})),
args.pastes.map(paste => this.getRange({ file, startLine: paste.start.line, startOffset: paste.start.offset, endLine: paste.end.line, endOffset: paste.end.offset }, project.getScriptInfoForNormalizedPath(file)!)),
this.getPreferences(file),
this.getFormatOptions(file),
);
if (result === undefined) {
return undefined;
}
return this.mapPostPasteAction(result);
}

private organizeImports(args: protocol.OrganizeImportsRequestArgs, simplifiedResult: boolean): readonly protocol.FileCodeEdits[] | readonly FileTextChanges[] {
Debug.assert(args.scope.type === "file");
const { file, project } = this.getFileAndProject(args.scope.args);
Expand Down Expand Up @@ -2928,6 +2950,10 @@ export class Session<TMessage = string> implements EventSender {
return { fixName, description, changes: this.mapTextChangesToCodeEdits(changes), commands, fixId, fixAllDescription };
}

private mapPostPasteAction({ edits }: PostPasteImportFixes): protocol.PostPasteImportAction {
return { edits: this.mapTextChangesToCodeEdits(edits) };
}

private mapTextChangesToCodeEdits(textChanges: readonly FileTextChanges[]): protocol.FileCodeEdits[] {
return textChanges.map(change => this.mapTextChangeToCodeEdit(change));
}
Expand Down Expand Up @@ -3521,6 +3547,9 @@ export class Session<TMessage = string> implements EventSender {
[protocol.CommandTypes.GetMoveToRefactoringFileSuggestions]: (request: protocol.GetMoveToRefactoringFileSuggestionsRequest) => {
return this.requiredResponse(this.getMoveToRefactoringFileSuggestions(request.arguments));
},
[protocol.CommandTypes.GetPostPasteImportFixes]: (request: protocol.GetPostPasteImportFixesRequest) => {
return this.requiredResponse(this.getPostPasteImportFixes(request.arguments));
},
[protocol.CommandTypes.GetEditsForRefactorFull]: (request: protocol.GetEditsForRefactorRequest) => {
return this.requiredResponse(this.getEditsForRefactor(request.arguments, /*simplifiedResult*/ false));
},
Expand Down
1 change: 1 addition & 0 deletions src/services/_namespaces/ts.postPasteImportFixes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "../postPasteImportFixes";
2 changes: 2 additions & 0 deletions src/services/_namespaces/ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,5 @@ import * as textChanges from "./ts.textChanges";
export { textChanges };
import * as formatting from "./ts.formatting";
export { formatting };
import * as postPasteImportFixes from "./ts.postPasteImportFixes";
export { postPasteImportFixes };
15 changes: 14 additions & 1 deletion src/services/codefixes/importFixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ export interface ImportAdder {
hasFixes(): boolean;
addImportFromDiagnostic: (diagnostic: DiagnosticWithLocation, context: CodeFixContextBase) => void;
addImportFromExportedSymbol: (exportedSymbol: Symbol, isValidTypeOnlyUseSite?: boolean) => void;
addImportForUnresolvedIdentifier: (context: CodeFixContextBase, symbolToken: Identifier, useAutoImportProvider: boolean) => void;
writeFixes: (changeTracker: textChanges.ChangeTracker, oldFileQuotePreference?: QuotePreference) => void;
}

Expand All @@ -235,7 +236,13 @@ function createImportAdderWorker(sourceFile: SourceFile, program: Program, useAu
type NewImportsKey = `${0 | 1}|${string}`;
/** Use `getNewImportEntry` for access */
const newImports = new Map<NewImportsKey, Mutable<ImportsCollection & { useRequire: boolean; }>>();
return { addImportFromDiagnostic, addImportFromExportedSymbol, writeFixes, hasFixes };
return { addImportFromDiagnostic, addImportFromExportedSymbol, writeFixes, hasFixes, addImportForUnresolvedIdentifier: addImportsForUnknownSymbols };

function addImportsForUnknownSymbols(context: CodeFixContextBase, symbolToken: Identifier, useAutoImportProvider: boolean) {
const info = getFixInfosWithoutDiagnostic(context, symbolToken, useAutoImportProvider);
if (!info || !info.length) return;
addImport(first(info));
}

function addImportFromDiagnostic(diagnostic: DiagnosticWithLocation, context: CodeFixContextBase) {
const info = getFixInfos(context, diagnostic.code, diagnostic.start, useAutoImportProvider);
Expand Down Expand Up @@ -1009,6 +1016,12 @@ function sortFixInfo(fixes: readonly (FixInfo & { fix: ImportFixWithModuleSpecif
compareModuleSpecifiers(a.fix, b.fix, sourceFile, program, packageJsonImportFilter.allowsImportingSpecifier, _toPath));
}

function getFixInfosWithoutDiagnostic(context: CodeFixContextBase, symbolToken: Identifier, useAutoImportProvider: boolean): readonly FixInfo[] | undefined {
const info = getFixesInfoForNonUMDImport(context, symbolToken, useAutoImportProvider);
const packageJsonImportFilter = createPackageJsonImportFilter(context.sourceFile, context.preferences, context.host);
return info && sortFixInfo(info, context.sourceFile, context.program, packageJsonImportFilter, context.host);
}

function getBestFix(fixes: readonly ImportFixWithModuleSpecifier[], sourceFile: SourceFile, program: Program, packageJsonImportFilter: PackageJsonImportFilter, host: LanguageServiceHost): ImportFixWithModuleSpecifier | undefined {
if (!some(fixes)) return;
// These will always be placed first if available, and are better than other kinds
Expand Down
103 changes: 103 additions & 0 deletions src/services/postPasteImportFixes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import {
addRange,
} from "../compiler/core";
import {
CancellationToken,
SourceFile,
Statement,
SymbolFlags,
TextRange,
UserPreferences,
} from "../compiler/types";
import {
getLineOfLocalPosition,
} from "../compiler/utilities";
import {
codefix,
fileShouldUseJavaScriptRequire,
forEachChild,
formatting,
getQuotePreference,
getTargetFileImportsAndAddExportInOldFile,
insertImports,
isIdentifier,
textChanges,
} from "./_namespaces/ts";
import {
getExistingLocals,
getUsageInfo,
} from "./refactors/moveToFile";
import {
CodeFixContextBase,
FileTextChanges,
LanguageServiceHost,
PostPasteImportFixes,
} from "./types";

/** @internal */
export function postPasteImportFixesProvider(
targetFile: SourceFile,
copies: { text: string; copyRange?: { file: SourceFile; range: TextRange; }; }[],
pastes: TextRange[],
host: LanguageServiceHost,
preferences: UserPreferences,
formatContext: formatting.FormatContext,
cancellationToken: CancellationToken,
): PostPasteImportFixes {
const changes: FileTextChanges[] = textChanges.ChangeTracker.with({ host, formatContext, preferences }, changeTracker => postPasteFixes(targetFile, copies, pastes, host, preferences, formatContext, cancellationToken, changeTracker));
return { edits: changes };
}

function postPasteFixes(
targetFile: SourceFile,
copies: { text: string; copyRange?: { file: SourceFile; range: TextRange; }; }[],
pastes: TextRange[],
host: LanguageServiceHost,
preferences: UserPreferences,
formatContext: formatting.FormatContext,
cancellationToken: CancellationToken,
changes: textChanges.ChangeTracker,
) {
const copy = copies[0];
const statements: Statement[] = [];

host.runWithTemporaryFileUpdate?.(targetFile.fileName, targetFile.getText().slice(0, pastes[0].pos) + copy.text + targetFile.getText().slice(pastes[0].end), (updatedProgram, originalProgram, updatedFile) => {
if (copy.copyRange) {
addRange(statements, copy.copyRange.file.statements, getLineOfLocalPosition(copy.copyRange.file, copy.copyRange.range.pos), getLineOfLocalPosition(copy.copyRange.file, copy.copyRange.range.end) + 1);
const usage = getUsageInfo(copy.copyRange.file, statements, originalProgram!.getTypeChecker(), getExistingLocals(updatedFile, statements, originalProgram!.getTypeChecker()));
const importAdder = codefix.createImportAdder(updatedFile, updatedProgram!, preferences, host);

const imports = getTargetFileImportsAndAddExportInOldFile(copy.copyRange.file, targetFile.fileName, usage.oldImportsNeededByTargetFile, usage.targetFileImportsFromOldFile, changes, originalProgram!.getTypeChecker(), updatedProgram!, host, !fileShouldUseJavaScriptRequire(targetFile.fileName, updatedProgram!, host, !!copy.copyRange.file.commonJsModuleIndicator), getQuotePreference(targetFile, preferences), importAdder);
if (imports.length > 0) {
insertImports(changes, targetFile, imports, /*blankLineBetween*/ true, preferences);
}
importAdder.writeFixes(changes, getQuotePreference(copy.copyRange.file, preferences));
}
else {
const context: CodeFixContextBase = {
sourceFile: updatedFile,
program: originalProgram!,
cancellationToken,
host,
preferences,
formatContext,
};
const importAdder = codefix.createImportAdder(updatedFile, updatedProgram!, preferences, host);
forEachChild(updatedFile, function cb(node) {
if (isIdentifier(node)) {
if (!originalProgram?.getTypeChecker().resolveName(node.text, node, SymbolFlags.All, /*excludeGlobals*/ false)) {
// generate imports
importAdder.addImportForUnresolvedIdentifier(context, node, /*useAutoImportProvider*/ true);
}
}
else {
node.forEachChild(cb);
}
});
importAdder.writeFixes(changes, getQuotePreference(targetFile, preferences));
}
});
pastes.forEach(paste => {
changes.replaceRangeWithText(targetFile, { pos: paste.pos, end: paste.end }, copy.text);
});
}
Loading
Loading