Skip to content

Commit

Permalink
feat: source.removeUnusedImports.ts and source.sortImports.ts act…
Browse files Browse the repository at this point in the history 
…ions (#681)
  • Loading branch information
@rchl
rchl committed Feb 12, 2023
1 parent c3f3529 commit a43b2df
Show file tree
Hide file tree
Showing 7 changed files with 134 additions and 126 deletions.
4 changes: 3 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,11 @@ implicitProjectConfiguration.target: string;

Server announces support for the following code action kinds:

- `source.addMissingImports.ts` - adds imports for used but not imported symbols
- `source.fixAll.ts` - despite the name, fixes a couple of specific issues: unreachable code, await in non-async functions, incorrectly implemented interface
- `source.removeUnused.ts` - removes declared but unused variables
- `source.addMissingImports.ts` - adds imports for used but not imported symbols
- `source.removeUnusedImports.ts` - removes unused imports
- `source.sortImports.ts` - sorts imports
- `source.organizeImports.ts` - organizes and removes unused imports

This allows editors that support running code actions on save to automatically run fixes associated with those kinds.
Expand Down
106 changes: 50 additions & 56 deletions src/lsp-server.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1483,52 +1483,65 @@ describe('code actions', () => {
expect(result).toEqual([]);
});

it('provides "add missing imports" when explicitly requested in only', async () => {
it('provides organize imports when there are no errors', async () => {
const doc = {
uri: uri('bar.ts'),
languageId: 'typescript',
version: 1,
text: 'existsSync(\'t\');',
text: `import { existsSync } from 'fs';
import { accessSync } from 'fs';
existsSync('t');
accessSync('t');`,
};
server.didOpenTextDocument({
textDocument: doc,
});
await server.requestDiagnostics();
await new Promise(resolve => setTimeout(resolve, 200));
const result = (await server.codeAction({
textDocument: doc,
range: {
start: { line: 1, character: 29 },
end: { line: 1, character: 53 },
start: { line: 0, character: 0 },
end: { line: 3, character: 0 },
},
context: {
diagnostics: [],
only: [CodeActionKind.SourceAddMissingImportsTs.value],
only: [CodeActionKind.SourceOrganizeImportsTs.value],
},
}))!;

expect(result).toMatchObject([
{
kind: CodeActionKind.SourceAddMissingImportsTs.value,
title: 'Add all missing imports',
kind: CodeActionKind.SourceOrganizeImportsTs.value,
title: 'Organize Imports',
edit: {
documentChanges: [
{
edits: [
{
// Prefers import that is declared in package.json.
newText: 'import { existsSync } from "fs-extra";\n\n',
newText: "import { accessSync, existsSync } from 'fs';\n",
range: {
end: {
character: 0,
line: 0,
line: 1,
},
start: {
character: 0,
line: 0,
},
},
},
{
newText: '',
range: {
end: {
character: 0,
line: 2,
},
start: {
character: 0,
line: 1,
},
},
},
],
textDocument: {
uri: uri('bar.ts'),
Expand All @@ -1541,15 +1554,12 @@ describe('code actions', () => {
]);
});

it('provides "fix all" when explicitly requested in only', async () => {
it('provides "add missing imports" when explicitly requested in only', async () => {
const doc = {
uri: uri('bar.ts'),
languageId: 'typescript',
version: 1,
text: `function foo() {
return
setTimeout(() => {})
}`,
text: 'existsSync(\'t\');',
};
server.didOpenTextDocument({
textDocument: doc,
Expand All @@ -1559,33 +1569,34 @@ describe('code actions', () => {
const result = (await server.codeAction({
textDocument: doc,
range: {
start: { line: 0, character: 0 },
end: { line: 4, character: 0 },
start: { line: 1, character: 29 },
end: { line: 1, character: 53 },
},
context: {
diagnostics: [],
only: [CodeActionKind.SourceFixAllTs.value],
only: [CodeActionKind.SourceAddMissingImportsTs.value],
},
}))!;

expect(result).toMatchObject([
{
kind: CodeActionKind.SourceFixAllTs.value,
title: 'Fix all',
kind: CodeActionKind.SourceAddMissingImportsTs.value,
title: 'Add all missing imports',
edit: {
documentChanges: [
{
edits: [
{
newText: '',
// Prefers import that is declared in package.json.
newText: 'import { existsSync } from "fs-extra";\n\n',
range: {
end: {
character: 0,
line: 3,
line: 0,
},
start: {
character: 0,
line: 2,
line: 0,
},
},
},
Expand All @@ -1601,68 +1612,51 @@ describe('code actions', () => {
]);
});

it('provides organize imports when explicitly requested in only', async () => {
it('provides "fix all" when explicitly requested in only', async () => {
const doc = {
uri: uri('bar.ts'),
languageId: 'typescript',
version: 1,
text: `import { existsSync } from 'fs';
import { accessSync } from 'fs';
existsSync('t');`,
text: `function foo() {
return
setTimeout(() => {})
}`,
};
server.didOpenTextDocument({
textDocument: doc,
});
await server.requestDiagnostics();
await new Promise(resolve => setTimeout(resolve, 200));
const result = (await server.codeAction({
textDocument: doc,
range: {
start: { line: 0, character: 0 },
end: { line: 3, character: 0 },
end: { line: 4, character: 0 },
},
context: {
diagnostics: [{
range: {
start: { line: 1, character: 25 },
end: { line: 1, character: 49 },
},
code: 6133,
message: 'unused arg',
}],
only: [CodeActionKind.SourceOrganizeImportsTs.value],
diagnostics: [],
only: [CodeActionKind.SourceFixAllTs.value],
},
}))!;

expect(result).toMatchObject([
{
kind: CodeActionKind.SourceOrganizeImportsTs.value,
title: 'Organize imports',
kind: CodeActionKind.SourceFixAllTs.value,
title: 'Fix all',
edit: {
documentChanges: [
{
edits: [
{
newText: "import { accessSync, existsSync } from 'fs';\n",
range: {
end: {
character: 0,
line: 1,
},
start: {
character: 0,
line: 0,
},
},
},
{
newText: '',
range: {
end: {
character: 0,
line: 2,
line: 3,
},
start: {
character: 0,
line: 1,
line: 2,
},
},
},
Expand Down
52 changes: 28 additions & 24 deletions src/lsp-server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import { asSignatureHelp, toTsTriggerReason } from './hover.js';
import { Commands, TypescriptVersionNotification } from './commands.js';
import { provideQuickFix } from './quickfix.js';
import { provideRefactors } from './refactor.js';
import { provideOrganizeImports } from './organize-imports.js';
import { CommandTypes, EventTypes, TypeScriptInitializeParams, TypeScriptInitializationOptions, SupportedFeatures } from './ts-protocol.js';
import { organizeImportsCommands, provideOrganizeImports } from './organize-imports.js';
import { CommandTypes, EventTypes, OrganizeImportsMode, TypeScriptInitializeParams, TypeScriptInitializationOptions, SupportedFeatures } from './ts-protocol.js';
import type { ts } from './ts-protocol.js';
import { collectDocumentSymbols, collectSymbolInformation } from './document-symbol.js';
import { TsServerLogLevel, TypeScriptServiceConfiguration } from './utils/configuration.js';
Expand Down Expand Up @@ -197,6 +197,8 @@ export class LspServer {
? { codeActionKinds: [
...TypeScriptAutoFixProvider.kinds.map(kind => kind.value),
CodeActionKind.SourceOrganizeImportsTs.value,
CodeActionKind.SourceRemoveUnusedImportsTs.value,
CodeActionKind.SourceSortImportsTs.value,
CodeActionKind.QuickFix.value,
CodeActionKind.Refactor.value,
] } : true,
Expand Down Expand Up @@ -911,6 +913,7 @@ export class LspServer {
if (!file) {
return [];
}
await this.configurationManager.configureGloballyFromDocument(file);
const fileRangeArgs = Range.toFileRangeRequestArgs(file, params.range);
const actions: lsp.CodeAction[] = [];
const kinds = params.context.only?.map(kind => new CodeActionKind(kind));
Expand All @@ -921,19 +924,28 @@ export class LspServer {
actions.push(...provideRefactors(await this.getRefactors(fileRangeArgs, params.context, token), fileRangeArgs, this.features));
}

// organize import is provided by tsserver for any line, so we only get it if explicitly requested
if (kinds?.some(kind => kind.contains(CodeActionKind.SourceOrganizeImportsTs))) {
// see this issue for more context about how this argument is used
// https://github.com/microsoft/TypeScript/issues/43051
const skipDestructiveCodeActions = params.context.diagnostics.some(
// assume no severity is an error
d => (d.severity ?? 0) <= 2,
);
const response = await this.getOrganizeImports({
scope: { type: 'file', args: fileRangeArgs },
skipDestructiveCodeActions,
}, token);
actions.push(...provideOrganizeImports(response, this.documents));
for (const kind of kinds || []) {
for (const command of organizeImportsCommands) {
console.error({ kinds, command });
if (!kind.contains(command.kind) || command.minVersion && this.tspClient.apiVersion.lt(command.minVersion)) {
continue;
}
// see this issue for more context: https://github.com/microsoft/TypeScript/issues/43051
if (command.kind.equals(CodeActionKind.SourceOrganizeImportsTs)
&& params.context.diagnostics.some(d => (d.severity ?? 0) <= 2)) { // assume no severity is an error
continue;
}
const response = await this.tspClient.request(
CommandTypes.OrganizeImports,
{
scope: { type: 'file', args: fileRangeArgs },
// Deprecated in 4.9; `mode` takes priority.
skipDestructiveCodeActions: command.mode === OrganizeImportsMode.SortAndCombine,
mode: command.mode,
},
token);
actions.push(...provideOrganizeImports(command, response, this.documents));
}
}

// TODO: Since we rely on diagnostics pointing at errors in the correct places, we can't proceed if we are not
Expand Down Expand Up @@ -974,14 +986,6 @@ export class LspServer {
return undefined;
}
}
protected async getOrganizeImports(args: ts.server.protocol.OrganizeImportsRequestArgs, token?: lsp.CancellationToken): Promise<ts.server.protocol.OrganizeImportsResponse | undefined> {
try {
await this.configurationManager.configureGloballyFromDocument(args.scope.args.file);
return await this.tspClient.request(CommandTypes.OrganizeImports, args, token);
} catch (err) {
return undefined;
}
}

async executeCommand(arg: lsp.ExecuteCommandParams, token?: lsp.CancellationToken, workDoneProgress?: lsp.WorkDoneProgressReporter): Promise<any> {
this.logger.log('executeCommand', arg);
Expand Down Expand Up @@ -1022,7 +1026,7 @@ export class LspServer {
} else if (arg.command === Commands.CONFIGURE_PLUGIN && arg.arguments) {
const [pluginName, configuration] = arg.arguments as [string, unknown];

if (this.tspClient?.apiVersion.gte(API.v314)) {
if (this.tspClient.apiVersion.gte(API.v314)) {
this.tspClient.executeWithoutWaitingForResponse(
CommandTypes.ConfigurePlugin,
{
Expand Down
39 changes: 0 additions & 39 deletions src/organize-imports.spec.ts
View file

This file was deleted.

0 comments on commit a43b2df

Please sign in to comment.