fix(54283): Provide better UX when an invalid file is selected for 'move to file' refactoring (#54300)

This commit is contained in:
Oleksandr T
2023-05-18 10:36:34 -07:00
committed by GitHub
parent 174599c554
commit ce1c97f4f0
11 changed files with 176 additions and 30 deletions
+4
View File
@@ -7612,6 +7612,10 @@
"category": "Message",
"code": 95178
},
"Cannot move to file, selected file is invalid": {
"category": "Message",
"code": 95179
},
"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
"category": "Error",
+3 -2
View File
@@ -818,7 +818,7 @@ export class SessionClient implements LanguageService {
const response = this.processResponse<protocol.GetEditsForRefactorResponse>(request);
if (!response.body) {
return { edits: [], renameFilename: undefined, renameLocation: undefined };
return { edits: [], renameFilename: undefined, renameLocation: undefined, notApplicableReason: undefined };
}
const edits: FileTextChanges[] = this.convertCodeEditsToTextChanges(response.body.edits);
@@ -832,7 +832,8 @@ export class SessionClient implements LanguageService {
return {
edits,
renameFilename,
renameLocation
renameLocation,
notApplicableReason: response.body.notApplicableReason,
};
}
+1
View File
@@ -721,6 +721,7 @@ export interface RefactorEditInfo {
*/
renameLocation?: Location;
renameFilename?: string;
notApplicableReason?: string;
}
/**
+2 -1
View File
@@ -2709,7 +2709,8 @@ export class Session<TMessage = string> implements EventSender {
return {
renameLocation: mappedRenameLocation,
renameFilename,
edits: this.mapTextChangesToCodeEdits(edits)
edits: this.mapTextChangesToCodeEdits(edits),
notApplicableReason: result.notApplicableReason,
};
}
return result;
+8 -2
View File
@@ -53,7 +53,9 @@ import {
getRelativePathFromFile,
getSynthesizedDeepClone,
getUniqueName,
hasJSFileExtension,
hasSyntacticModifier,
hasTSFileExtension,
hostGetCanonicalFileName,
Identifier,
ImportDeclaration,
@@ -162,8 +164,12 @@ registerRefactor(refactorNameForMoveToFile, {
Debug.assert(actionName === refactorNameForMoveToFile, "Wrong refactor invoked");
const statements = Debug.checkDefined(getStatementsToMove(context));
Debug.assert(interactiveRefactorArguments, "No interactive refactor arguments available");
const edits = textChanges.ChangeTracker.with(context, t => doChange(context, context.file, interactiveRefactorArguments.targetFile, context.program, statements, t, context.host, context.preferences));
return { edits, renameFilename: undefined, renameLocation: undefined };
const targetFile = interactiveRefactorArguments.targetFile;
if (hasJSFileExtension(targetFile) || hasTSFileExtension(targetFile)) {
const edits = textChanges.ChangeTracker.with(context, t => doChange(context, context.file, interactiveRefactorArguments.targetFile, context.program, statements, t, context.host, context.preferences));
return { edits, renameFilename: undefined, renameLocation: undefined };
}
return { edits: [], renameFilename: undefined, renameLocation: undefined, notApplicableReason: getLocaleSpecificMessage(Diagnostics.Cannot_move_to_file_selected_file_is_invalid) };
}
});
+1
View File
@@ -1002,6 +1002,7 @@ export interface RefactorEditInfo {
renameFilename?: string;
renameLocation?: number;
commands?: CodeActionCommand[];
notApplicableReason?: string;
}
export type RefactorTriggerReason = "implicit" | "invoked";
+54 -21
View File
@@ -96,28 +96,61 @@ describe("unittests:: tsserver:: refactors", () => {
});
it("handles moving statement to an existing file", () => {
const aTs: File = { path: "/Foo/a.ts", content: "const x = 0;" };
const bTs: File = {
path: "/Foo/b.ts", content: `import {} from "./bar";
const a = 1;`};
const tsconfig: File = { path: "/Foo/tsconfig.json", content: `{ "files": ["./a.ts", "./b.ts"] }` };
const host = createServerHost([aTs, bTs, tsconfig]);
const session = createSession(host, { logger: createLoggerWithInMemoryLogs(host) });
openFilesForSession([aTs], session);
const aTs: File = { path: "/Foo/a.ts", content: "const x = 0;" };
const bTs: File = {
path: "/Foo/b.ts", content: `import {} from "./bar";
const a = 1;`};
const tsconfig: File = { path: "/Foo/tsconfig.json", content: `{ "files": ["./a.ts", "./b.ts"] }` };
const host = createServerHost([aTs, bTs, tsconfig]);
const session = createSession(host, { logger: createLoggerWithInMemoryLogs(host) });
openFilesForSession([aTs], session);
session.executeCommandSeq<ts.server.protocol.GetEditsForRefactorRequest>({
command: ts.server.protocol.CommandTypes.GetEditsForRefactor,
arguments: {
file: aTs.path,
startLine: 1,
startOffset: 1,
endLine: 2,
endOffset: aTs.content.length,
refactor: "Move to file",
action: "Move to file",
interactiveRefactorArguments: { targetFile: "/Foo/b.ts" },
}
session.executeCommandSeq<ts.server.protocol.GetEditsForRefactorRequest>({
command: ts.server.protocol.CommandTypes.GetEditsForRefactor,
arguments: {
file: aTs.path,
startLine: 1,
startOffset: 1,
endLine: 2,
endOffset: aTs.content.length,
refactor: "Move to file",
action: "Move to file",
interactiveRefactorArguments: { targetFile: "/Foo/b.ts" },
}
});
baselineTsserverLogs("refactors", "handles moving statement to an existing file", session);
});
baselineTsserverLogs("refactors", "handles moving statement to an existing file", session);
it("handles moving statements to a non-TS file", () => {
const aTs: File = {
path: "/Foo/a.ts",
content: "const x = 0;"
};
const bTxt: File = {
path: "/Foo/b.txt",
content: ""
};
const tsconfig: File = {
path: "/Foo/tsconfig.json",
content: `{ "files": ["./a.ts"] }`
};
const host = createServerHost([aTs, bTxt, tsconfig]);
const session = createSession(host, { logger: createLoggerWithInMemoryLogs(host) });
openFilesForSession([aTs], session);
session.executeCommandSeq<ts.server.protocol.GetEditsForRefactorRequest>({
command: ts.server.protocol.CommandTypes.GetEditsForRefactor,
arguments: {
file: aTs.path,
startLine: 1,
startOffset: 1,
endLine: 2,
endOffset: aTs.content.length,
refactor: "Move to file",
action: "Move to file",
interactiveRefactorArguments: { targetFile: "/Foo/b.txt" },
}
});
baselineTsserverLogs("refactors", "handles moving statements to a non-TS file", session);
});
});
+2
View File
@@ -619,6 +619,7 @@ declare namespace ts {
*/
renameLocation?: Location;
renameFilename?: string;
notApplicableReason?: string;
}
/**
* Organize imports by:
@@ -10485,6 +10486,7 @@ declare namespace ts {
renameFilename?: string;
renameLocation?: number;
commands?: CodeActionCommand[];
notApplicableReason?: string;
}
type RefactorTriggerReason = "implicit" | "invoked";
interface TextInsertion {
+1
View File
@@ -6517,6 +6517,7 @@ declare namespace ts {
renameFilename?: string;
renameLocation?: number;
commands?: CodeActionCommand[];
notApplicableReason?: string;
}
type RefactorTriggerReason = "implicit" | "invoked";
interface TextInsertion {
@@ -6,7 +6,7 @@ const x = 0;
//// [/Foo/b.ts]
import {} from "./bar";
const a = 1;
const a = 1;
//// [/Foo/tsconfig.json]
{ "files": ["./a.ts", "./b.ts"] }
@@ -41,7 +41,7 @@ Info seq [hh:mm:ss:mss] Finishing updateGraphWorker: Project: /Foo/tsconfig.jso
Info seq [hh:mm:ss:mss] Project '/Foo/tsconfig.json' (Configured)
Info seq [hh:mm:ss:mss] Files (2)
/Foo/a.ts SVC-1-0 "const x = 0;"
/Foo/b.ts Text-1 "import {} from \"./bar\";\nconst a = 1;"
/Foo/b.ts Text-1 "import {} from \"./bar\";\n const a = 1;"
a.ts
@@ -119,11 +119,11 @@ Info seq [hh:mm:ss:mss] response:
{
"start": {
"line": 2,
"offset": 13
"offset": 17
},
"end": {
"line": 2,
"offset": 13
"offset": 17
},
"newText": "\nconst x = 0;\n"
}
@@ -0,0 +1,96 @@
currentDirectory:: / useCaseSensitiveFileNames: false
Info seq [hh:mm:ss:mss] Provided types map file "/a/lib/typesMap.json" doesn't exist
Before request
//// [/Foo/a.ts]
const x = 0;
//// [/Foo/b.txt]
//// [/Foo/tsconfig.json]
{ "files": ["./a.ts"] }
Info seq [hh:mm:ss:mss] request:
{
"command": "open",
"arguments": {
"file": "/Foo/a.ts"
},
"seq": 1,
"type": "request"
}
Info seq [hh:mm:ss:mss] Search path: /Foo
Info seq [hh:mm:ss:mss] For info: /Foo/a.ts :: Config file name: /Foo/tsconfig.json
Info seq [hh:mm:ss:mss] Creating configuration project /Foo/tsconfig.json
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /Foo/tsconfig.json 2000 undefined Project: /Foo/tsconfig.json WatchType: Config file
Info seq [hh:mm:ss:mss] Config: /Foo/tsconfig.json : {
"rootNames": [
"/Foo/a.ts"
],
"options": {
"configFilePath": "/Foo/tsconfig.json"
}
}
Info seq [hh:mm:ss:mss] Starting updateGraphWorker: Project: /Foo/tsconfig.json
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /a/lib/lib.d.ts 500 undefined Project: /Foo/tsconfig.json WatchType: Missing file
Info seq [hh:mm:ss:mss] Finishing updateGraphWorker: Project: /Foo/tsconfig.json Version: 1 structureChanged: true structureIsReused:: Not Elapsed:: *ms
Info seq [hh:mm:ss:mss] Project '/Foo/tsconfig.json' (Configured)
Info seq [hh:mm:ss:mss] Files (1)
/Foo/a.ts SVC-1-0 "const x = 0;"
a.ts
Part of 'files' list in tsconfig.json
Info seq [hh:mm:ss:mss] -----------------------------------------------
Info seq [hh:mm:ss:mss] Project '/Foo/tsconfig.json' (Configured)
Info seq [hh:mm:ss:mss] Files (1)
Info seq [hh:mm:ss:mss] -----------------------------------------------
Info seq [hh:mm:ss:mss] Open files:
Info seq [hh:mm:ss:mss] FileName: /Foo/a.ts ProjectRootPath: undefined
Info seq [hh:mm:ss:mss] Projects: /Foo/tsconfig.json
Info seq [hh:mm:ss:mss] response:
{
"responseRequired": false
}
After request
PolledWatches::
/a/lib/lib.d.ts: *new*
{"pollingInterval":500}
FsWatches::
/foo/tsconfig.json: *new*
{}
Before request
Info seq [hh:mm:ss:mss] request:
{
"command": "getEditsForRefactor",
"arguments": {
"file": "/Foo/a.ts",
"startLine": 1,
"startOffset": 1,
"endLine": 2,
"endOffset": 12,
"refactor": "Move to file",
"action": "Move to file",
"interactiveRefactorArguments": {
"targetFile": "/Foo/b.txt"
}
},
"seq": 2,
"type": "request"
}
Info seq [hh:mm:ss:mss] response:
{
"response": {
"edits": [],
"notApplicableReason": "Cannot move to file, selected file is invalid"
},
"responseRequired": true
}
After request