Setting to prevent unused import removal (#454)

* Setting to prevent unused import removal

Using feature added to the language server in v0.7.0

Resolves #273

* Changelog

Author: apexskier

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",
       }
     );

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",

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();

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);
+  });
+});

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
+  );
+}

typescript.novaextension/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## future
 
+### Added
+
+- Configuration to prevent deleting unused imports when auto-organizing imports
+
 ### Changed
 
 - When search results are ready, display a notification instead of an alert.

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.",