From 1f9eaa32e16d74f1ab0f23974a427db25248d755 Mon Sep 17 00:00:00 2001 From: Cameron Little Date: Mon, 16 Jan 2023 14:07:13 -0800 Subject: [PATCH] Setting to prevent unused import removal Using feature added to the language server in v0.7.0 Resolves #273 --- src/commands/organizeImports.test.ts | 6 +- src/commands/organizeImports.ts | 6 +- src/main.test.ts | 3 + src/skipDestructiveOrganizeImports.test.ts | 81 ++++++++++++++++++++++ src/skipDestructiveOrganizeImports.ts | 37 ++++++++++ typescript.novaextension/extension.json | 13 ++++ 6 files changed, 144 insertions(+), 2 deletions(-) create mode 100644 src/skipDestructiveOrganizeImports.test.ts create mode 100644 src/skipDestructiveOrganizeImports.ts diff --git a/src/commands/organizeImports.test.ts b/src/commands/organizeImports.test.ts index 68ef5d30..e4a68924 100644 --- a/src/commands/organizeImports.test.ts +++ b/src/commands/organizeImports.test.ts @@ -6,6 +6,10 @@ class MockRange { } (global as any).Range = MockRange; +jest.mock("../skipDestructiveOrganizeImports", () => ({ + skipDestructiveOrganizeImports: () => false, +})); + describe("organizeImports command", () => { beforeEach(() => { (global as any).nova = Object.assign(nova, { @@ -89,7 +93,7 @@ describe("organizeImports command", () => { 2, "workspace/executeCommand", { - arguments: ["/path"], + arguments: ["/path", { skipDestructiveCodeActions: false }], command: "_typescript.organizeImports", } ); diff --git a/src/commands/organizeImports.ts b/src/commands/organizeImports.ts index 3cf1b164..398b740c 100644 --- a/src/commands/organizeImports.ts +++ b/src/commands/organizeImports.ts @@ -1,5 +1,6 @@ import type * as lspTypes from "vscode-languageserver-protocol"; import { wrapCommand } from "../novaUtils"; +import { skipDestructiveOrganizeImports } from "../skipDestructiveOrganizeImports"; // NOTE: this is explicitly built for the typescript-language-server; it directly invokes the specific command it uses. // In order to decouple and become LSP generic, we'd need to first send a code action request for only @@ -51,7 +52,10 @@ export function registerOrganizeImports(client: LanguageClient) { const organizeImportsCommand: lspTypes.ExecuteCommandParams = { command: "_typescript.organizeImports", - arguments: [editor.document.path], + arguments: [ + editor.document.path, + { skipDestructiveCodeActions: skipDestructiveOrganizeImports() }, + ], }; await client.sendRequest( "workspace/executeCommand", diff --git a/src/main.test.ts b/src/main.test.ts index 2c9264cf..1b9a7b35 100644 --- a/src/main.test.ts +++ b/src/main.test.ts @@ -9,6 +9,9 @@ jest.mock("./tsUserPreferences", () => ({ jest.mock("./isEnabledForJavascript", () => ({ isEnabledForJavascript: () => true, })); +jest.mock("./skipDestructiveOrganizeImports", () => ({ + skipDestructiveOrganizeImports: () => false, +})); jest.mock("nova-extension-utils"); jest.useFakeTimers(); diff --git a/src/skipDestructiveOrganizeImports.test.ts b/src/skipDestructiveOrganizeImports.test.ts new file mode 100644 index 00000000..fceb9ea2 --- /dev/null +++ b/src/skipDestructiveOrganizeImports.test.ts @@ -0,0 +1,81 @@ +(global as any).nova = Object.assign(nova, { + commands: { + invoke: jest.fn(), + }, + config: { + onDidChange: jest.fn(), + ["get"]: jest.fn(), + }, + workspace: { + config: { onDidChange: jest.fn(), ["get"]: jest.fn() }, + }, +}); + +describe("skipDestructiveOrganizeImports", () => { + beforeEach(() => { + (nova.workspace.config.get as jest.Mock).mockReset(); + (nova.config.get as jest.Mock).mockReset(); + }); + + const { + skipDestructiveOrganizeImports, + } = require("./skipDestructiveOrganizeImports"); + + describe("reloads extension when it changes", () => { + it("globally and for the workspace", () => { + expect(nova.config.onDidChange).toBeCalledTimes(1); + expect(nova.config.onDidChange).toBeCalledWith( + "apexskier.typescript.config.skipDestructiveOrganizeImports", + expect.any(Function) + ); + expect(nova.workspace.config.onDidChange).toBeCalledTimes(1); + expect(nova.workspace.config.onDidChange).toBeCalledWith( + "apexskier.typescript.config.skipDestructiveOrganizeImports", + expect.any(Function) + ); + // same function + const onWorkspaceChange = (nova.workspace.config.onDidChange as jest.Mock) + .mock.calls[0][1]; + const onGlobalChange = (nova.config.onDidChange as jest.Mock).mock + .calls[0][1]; + expect(onWorkspaceChange).toBe(onGlobalChange); + }); + + it("by calling the reload command", () => { + const reload = (nova.config.onDidChange as jest.Mock).mock.calls[0][1]; + reload(); + expect(nova.commands.invoke).toBeCalledTimes(1); + expect(nova.commands.invoke).toBeCalledWith( + "apexskier.typescript.reload" + ); + }); + }); + + describe("is true by default", () => { + expect(skipDestructiveOrganizeImports()).toBe(true); + }); + + describe("can be disabled globally", () => { + (nova.workspace.config.get as jest.Mock).mockReturnValueOnce(null); + (nova.config.get as jest.Mock).mockReturnValueOnce(false); + expect(skipDestructiveOrganizeImports()).toBe(false); + }); + + describe("can be enabled globally", () => { + (nova.workspace.config.get as jest.Mock).mockReturnValueOnce(null); + (nova.config.get as jest.Mock).mockReturnValueOnce(true); + expect(skipDestructiveOrganizeImports()).toBe(true); + }); + + describe("can be disabled in the workspace", () => { + (nova.workspace.config.get as jest.Mock).mockReturnValueOnce("False"); + (nova.config.get as jest.Mock).mockReturnValueOnce(true); + expect(skipDestructiveOrganizeImports()).toBe(false); + }); + + describe("can be enabled in the workspace", () => { + (nova.workspace.config.get as jest.Mock).mockReturnValueOnce("True"); + (nova.config.get as jest.Mock).mockReturnValueOnce(false); + expect(skipDestructiveOrganizeImports()).toBe(true); + }); +}); diff --git a/src/skipDestructiveOrganizeImports.ts b/src/skipDestructiveOrganizeImports.ts new file mode 100644 index 00000000..af83c201 --- /dev/null +++ b/src/skipDestructiveOrganizeImports.ts @@ -0,0 +1,37 @@ +function reload() { + nova.commands.invoke("apexskier.typescript.reload"); +} +nova.config.onDidChange( + "apexskier.typescript.config.skipDestructiveOrganizeImports", + reload +); +nova.workspace.config.onDidChange( + "apexskier.typescript.config.skipDestructiveOrganizeImports", + reload +); + +function getWorkspaceSetting(): boolean | null { + const str = nova.workspace.config.get( + "apexskier.typescript.config.skipDestructiveOrganizeImports", + "string" + ); + switch (str) { + case "False": + return false; + case "True": + return true; + default: + return null; + } +} + +export function skipDestructiveOrganizeImports(): boolean { + return ( + getWorkspaceSetting() ?? + nova.config.get( + "apexskier.typescript.config.skipDestructiveOrganizeImports", + "boolean" + ) ?? + true + ); +} diff --git a/typescript.novaextension/extension.json b/typescript.novaextension/extension.json index 8ae7a6f1..e9588311 100644 --- a/typescript.novaextension/extension.json +++ b/typescript.novaextension/extension.json @@ -60,6 +60,12 @@ "type": "boolean", "default": true }, + { + "key": "apexskier.typescript.config.skipDestructiveOrganizeImports", + "title": "Skip destructive organize imports changes", + "type": "boolean", + "default": false + }, { "title": "TypeScript server User Preferences", "description": "Advanced configuration passed to the underlying typescript server. These may not apply to older versions of TypeScript.", @@ -352,6 +358,13 @@ "values": ["Inherit from Global Settings", "Disable", "Enable"], "default": "Inherit from Global Settings" }, + { + "key": "apexskier.typescript.config.skipDestructiveOrganizeImports", + "title": "Skip destructive organize imports changes", + "type": "enum", + "values": ["Inherit from Global Settings", "False", "True"], + "default": "Inherit from Global Settings" + }, { "title": "TypeScript server User Preferences", "description": "Advanced configuration passed to the underlying typescript server. These may not apply to older versions of TypeScript.",