Skip to content

🤖 Pick PR #54300 (fix(54283): Provide better UX when ...) into release-5.1 #54312

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
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
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 3 additions & 2 deletions src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -832,7 +832,8 @@ export class SessionClient implements LanguageService {
return {
edits,
renameFilename,
renameLocation
renameLocation,
notApplicableReason: response.body.notApplicableReason,
};
}

Expand Down
1 change: 1 addition & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,7 @@ export interface RefactorEditInfo {
*/
renameLocation?: Location;
renameFilename?: string;
notApplicableReason?: string;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2707,7 +2707,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;
Expand Down
10 changes: 8 additions & 2 deletions src/services/refactors/moveToFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ import {
getRelativePathFromFile,
getSynthesizedDeepClone,
getUniqueName,
hasJSFileExtension,
hasSyntacticModifier,
hasTSFileExtension,
hostGetCanonicalFileName,
Identifier,
ImportDeclaration,
Expand Down Expand Up @@ -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) };
}
});

Expand Down
1 change: 1 addition & 0 deletions src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,7 @@ export interface RefactorEditInfo {
renameFilename?: string;
renameLocation?: number;
commands?: CodeActionCommand[];
notApplicableReason?: string;
}

export type RefactorTriggerReason = "implicit" | "invoked";
Expand Down
75 changes: 54 additions & 21 deletions src/testRunner/unittests/tsserver/refactors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 2 additions & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,7 @@ declare namespace ts {
*/
renameLocation?: Location;
renameFilename?: string;
notApplicableReason?: string;
}
/**
* Organize imports by:
Expand Down Expand Up @@ -10480,6 +10481,7 @@ declare namespace ts {
renameFilename?: string;
renameLocation?: number;
commands?: CodeActionCommand[];
notApplicableReason?: string;
}
type RefactorTriggerReason = "implicit" | "invoked";
interface TextInsertion {
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6517,6 +6517,7 @@ declare namespace ts {
renameFilename?: string;
renameLocation?: number;
commands?: CodeActionCommand[];
notApplicableReason?: string;
}
type RefactorTriggerReason = "implicit" | "invoked";
interface TextInsertion {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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