Add an organize import command (#129)

* Add an organize import command

Resolves #104 (partially)

* lint

* Add eslint prettier config

* Allow organizing imports on save

* changelog

* bump nova-extension-utils

* Optimize handler

* lint

Author: apexskier

.eslintrc.js

@@ -7,6 +7,7 @@ module.exports = {
   extends: [
     "eslint:recommended",
     "plugin:@typescript-eslint/eslint-recommended",
+    "prettier",
   ],
   parser: "@typescript-eslint/parser",
   parserOptions: {

.nova/Configuration.json

@@ -1,4 +1,5 @@
 {
+  "apexskier.typescript.config.organizeImportsOnSave": "true",
   "editor.default_syntax": "typescript",
   "workspace.name": "TypeScript Extension"
 }

package.json

@@ -35,9 +35,10 @@
     "@typescript-eslint/parser": "^3.10.1",
     "concurrently": "^5.3.0",
     "eslint": "^7.13.0",
+    "eslint-config-prettier": "^6.15.0",
     "eslint-plugin-nova": "^1.2.0",
     "jest": "^26.6.3",
-    "nova-extension-utils": "^1.2.2",
+    "nova-extension-utils": "^1.3.0",
     "onchange": "^7.1.0",
     "prettier": "^2.1.2",
     "rollup": "^2.33.1",

rollup.config.main.js

@@ -1,6 +1,6 @@
-import typescript from "rollup-plugin-typescript2";
 import commonjs from "@rollup/plugin-commonjs";
 import resolve from "@rollup/plugin-node-resolve";
+import typescript from "rollup-plugin-typescript2";
 
 export default {
   input: "src/main.ts",

src/commands/organizeImports.test.ts

@@ -0,0 +1,117 @@
+// eslint-disable-next-line no-unused-vars
+import type * as lspTypes from "vscode-languageserver-protocol";
+import { registerOrganizeImports } from "./organizeImports";
+
+class MockRange {
+  // eslint-disable-next-line no-unused-vars
+  constructor(readonly start: number, readonly end: number) {}
+}
+(global as any).Range = MockRange;
+
+describe("organizeImports command", () => {
+  beforeEach(() => {
+    (global as any).nova = Object.assign(nova, {
+      commands: {
+        register: jest.fn(),
+      },
+      workspace: {
+        showErrorMessage(err: Error) {
+          throw err;
+        },
+        showWarningMessage: jest.fn(),
+      },
+    });
+
+    mockEditor.edit.mockClear();
+    mockEditor.scrollToCursorPosition.mockClear();
+    mockEditor.selectWordsContainingCursors.mockClear();
+  });
+
+  const mockEditor = {
+    selectedRanges: [new Range(2, 3)],
+    selectedText: "selectedText",
+    document: {
+      length: 10,
+      uri: "currentDocURI",
+      path: "/path",
+      getTextInRange() {
+        return "";
+      },
+      eol: "\n",
+    },
+    edit: jest.fn(),
+    selectWordsContainingCursors: jest.fn(),
+    scrollToCursorPosition: jest.fn(),
+  };
+
+  function getCommand(
+    languageClient: LanguageClient,
+    register: (client: LanguageClient) => Disposable
+  ): (...args: Array<any>) => Promise<void> {
+    register(languageClient);
+    expect(nova.commands.register).toHaveBeenCalledWith(
+      "apexskier.typescript.commands.organizeImports",
+      expect.any(Function)
+    );
+    const command = (nova.commands.register as jest.Mock).mock.calls[0][1];
+    (nova.commands.register as jest.Mock).mockClear();
+    return command;
+  }
+
+  it("asks the server to organize imports, then resets your selection", async () => {
+    const mockLanguageClient = {
+      sendRequest: jest.fn().mockImplementationOnce(() => {
+        mockEditor.document.length = 14;
+      }),
+    };
+    const command = getCommand(
+      (mockLanguageClient as any) as LanguageClient,
+      registerOrganizeImports
+    );
+    expect(mockEditor.selectedRanges).toEqual([new Range(2, 3)]);
+    await command(mockEditor);
+
+    expect(mockLanguageClient.sendRequest).toBeCalledTimes(1);
+    expect(mockLanguageClient.sendRequest).toHaveBeenCalledWith(
+      "workspace/executeCommand",
+      {
+        arguments: ["/path"],
+        command: "_typescript.organizeImports",
+      }
+    );
+    expect(mockEditor.selectedRanges).toEqual([new Range(6, 7)]);
+    expect(mockEditor.scrollToCursorPosition).toBeCalledTimes(1);
+  });
+
+  it("doesn't reset scroll to a negative value", async () => {
+    const mockLanguageClient = {
+      sendRequest: jest.fn().mockImplementationOnce(() => {
+        mockEditor.document.length = 0;
+      }),
+    };
+    const command = getCommand(
+      (mockLanguageClient as any) as LanguageClient,
+      registerOrganizeImports
+    );
+    await command(mockEditor);
+    expect(mockEditor.selectedRanges).toEqual([new Range(0, 0)]);
+    expect(mockEditor.scrollToCursorPosition).toBeCalledTimes(1);
+  });
+
+  it("warns if the document isn't saved", async () => {
+    const mockLanguageClient = {
+      sendRequest: jest.fn().mockReturnValueOnce(Promise.resolve(null)),
+    };
+    mockEditor.document.path = "";
+    const command = getCommand(
+      (mockLanguageClient as any) as LanguageClient,
+      registerOrganizeImports
+    );
+    await command(mockEditor);
+
+    expect(nova.workspace.showWarningMessage).toBeCalledTimes(1);
+    expect(nova.workspace.showWarningMessage).toHaveBeenCalledWith(
+      "Please save this document before organizing imports."
+    );
+  });
+});

src/commands/organizeImports.ts

@@ -0,0 +1,63 @@
+// eslint-disable-next-line no-unused-vars
+import type * as lspTypes from "vscode-languageserver-protocol";
+import { wrapCommand } from "../novaUtils";
+
+// 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
+// lspTypes.CodeActionKind.SourceOrganizeImports, then process the response's code action or commands.
+// That would mean reimplementing that processing in the extension, which I don't like.
+// Related conversation at https://devforum.nova.app/t/ability-to-send-lsp-messages-to-nova/466
+
+export function registerOrganizeImports(client: LanguageClient) {
+  return nova.commands.register(
+    "apexskier.typescript.commands.organizeImports",
+    wrapCommand(organizeImports)
+  );
+
+  async function organizeImports(editor: TextEditor): Promise<void>;
+  async function organizeImports(
+    workspace: Workspace,
+    editor: TextEditor
+  ): Promise<void>;
+  async function organizeImports(
+    editorOrWorkspace: TextEditor | Workspace,
+    maybeEditor?: TextEditor
+  ) {
+    const editor: TextEditor = maybeEditor ?? (editorOrWorkspace as TextEditor);
+
+    const originalSelections = editor.selectedRanges;
+    const originalLength = editor.document.length;
+
+    if (!editor.document.path) {
+      nova.workspace.showWarningMessage(
+        "Please save this document before organizing imports."
+      );
+      return;
+    }
+
+    const organizeImportsCommand: lspTypes.ExecuteCommandParams = {
+      command: "_typescript.organizeImports",
+      arguments: [editor.document.path],
+    };
+    await client.sendRequest(
+      "workspace/executeCommand",
+      organizeImportsCommand
+    );
+
+    // Move selection/cursor back to where it was
+    // NOTE: this isn't fully perfect, since it doesn't know where the changes were made.
+    // If your cursor is above the imports it won't be returned properly.
+    // I'm okay with this for now. To fully fix it we'd need to do the import organization manually
+    // based on an explicit code action, which isn't worth it.
+    const newLength = editor.document.length;
+    const lengthChange = originalLength - newLength;
+    editor.selectedRanges = originalSelections.map(
+      (r) =>
+        new Range(
+          Math.max(0, r.start - lengthChange),
+          Math.max(0, r.end - lengthChange)
+        )
+    );
+    editor.scrollToCursorPosition();
+  }
+}

src/main.test.ts

@@ -18,6 +18,7 @@ jest.useFakeTimers();
   },
   workspace: {
     path: "/workspace",
+    onDidAddTextEditor: jest.fn(),
   },
   extension: {
     path: "/extension",
@@ -69,6 +70,7 @@ describe("test suite", () => {
       .mockImplementation(() => Promise.resolve());
     nova.fs.access = jest.fn().mockReturnValue(true);
     (nova.commands.register as jest.Mock).mockReset();
+    (nova.commands.invoke as jest.Mock).mockReset();
     LanguageClientMock.mockReset().mockImplementation(() => ({
       onRequest: jest.fn(),
       onNotification: jest.fn(),
@@ -86,6 +88,7 @@ describe("test suite", () => {
       start: jest.fn(),
     }));
     (informationViewModule.InformationView as jest.Mock).mockReset();
+    (nova.workspace.onDidAddTextEditor as jest.Mock).mockReset();
   }
 
   const reload = (nova.commands.register as jest.Mock).mock.calls.find(
@@ -115,7 +118,7 @@ describe("test suite", () => {
   });
 
   function assertActivationBehavior() {
-    expect(nova.commands.register).toBeCalledTimes(3);
+    expect(nova.commands.register).toBeCalledTimes(4);
     expect(nova.commands.register).toBeCalledWith(
       "apexskier.typescript.rename",
       expect.any(Function)
@@ -128,6 +131,10 @@ describe("test suite", () => {
       "apexskier.typescript.findReferences",
       expect.any(Function)
     );
+    expect(nova.commands.register).toBeCalledWith(
+      "apexskier.typescript.commands.organizeImports",
+      expect.any(Function)
+    );
 
     // installs dependencies
 
@@ -287,5 +294,42 @@ describe("test suite", () => {
 
       assertActivationBehavior();
     });
+
+    test("watches files for import organization", async () => {
+      resetMocks();
+
+      await activate();
+
+      (nova as any).config = { onDidChange: jest.fn() };
+      (nova as any).workspace.config = { onDidChange: jest.fn() };
+
+      expect(nova.workspace.onDidAddTextEditor).toBeCalledTimes(1);
+      const setupWatcher = (nova.workspace.onDidAddTextEditor as jest.Mock).mock
+        .calls[0][0];
+      const mockEditor = {
+        onWillSave: jest.fn(),
+        onDidDestroy: jest.fn(),
+        document: {
+          syntax: "typescript",
+          onDidChangeSyntax: jest.fn(),
+        },
+      };
+      setupWatcher(mockEditor);
+      expect(mockEditor.onWillSave).toBeCalledTimes(0);
+
+      require("nova-extension-utils").preferences.getOverridableBoolean.mockReturnValue(
+        true
+      );
+      setupWatcher(mockEditor);
+      expect(mockEditor.onWillSave).toBeCalledTimes(1);
+
+      const saveHandler = (mockEditor.onWillSave as jest.Mock).mock.calls[0][0];
+      await saveHandler(mockEditor);
+      expect(nova.commands.invoke).toHaveBeenNthCalledWith(
+        1,
+        "apexskier.typescript.commands.organizeImports",
+        mockEditor
+      );
+    });
   });
 });

src/main.ts

@@ -1,12 +1,16 @@
-import { dependencyManagement } from "nova-extension-utils";
+import { dependencyManagement, preferences } from "nova-extension-utils";
 import { registerFindReferences } from "./commands/findReferences";
 import { registerFindSymbol } from "./commands/findSymbol";
+import { registerOrganizeImports } from "./commands/organizeImports";
 import { registerRename } from "./commands/rename";
 import { registerSignatureHelp } from "./commands/signatureHelp";
-import { wrapCommand } from "./novaUtils";
 import { InformationView } from "./informationView";
-import { getTsLibPath } from "./tsLibPath";
 import { isEnabledForJavascript } from "./isEnabledForJavascript";
+import { wrapCommand } from "./novaUtils";
+import { getTsLibPath } from "./tsLibPath";
+
+const organizeImportsOnSaveKey =
+  "apexskier.typescript.config.organizeImportsOnSave";
 
 nova.commands.register(
   "apexskier.typescript.openWorkspaceConfig",
@@ -154,6 +158,7 @@ async function asyncActivate() {
   compositeDisposable.add(registerFindReferences(client));
   compositeDisposable.add(registerFindSymbol(client));
   compositeDisposable.add(registerRename(client));
+  compositeDisposable.add(registerOrganizeImports(client));
   if (nova.inDevMode()) {
     compositeDisposable.add(registerSignatureHelp(client));
   }
@@ -186,6 +191,56 @@ async function asyncActivate() {
 
   client.start();
 
+  // auto-organize imports on save
+  compositeDisposable.add(
+    nova.workspace.onDidAddTextEditor((editor) => {
+      const editorDisposable = new CompositeDisposable();
+      compositeDisposable.add(editorDisposable);
+      compositeDisposable.add(
+        editor.onDidDestroy(() => editorDisposable.dispose())
+      );
+
+      // watch things that might change if this needs to happen or not
+      editorDisposable.add(editor.document.onDidChangeSyntax(refreshListener));
+      editorDisposable.add(
+        nova.config.onDidChange(organizeImportsOnSaveKey, refreshListener)
+      );
+      editorDisposable.add(
+        nova.workspace.config.onDidChange(
+          organizeImportsOnSaveKey,
+          refreshListener
+        )
+      );
+
+      let willSaveListener = setupListener();
+      compositeDisposable.add({
+        dispose() {
+          willSaveListener?.dispose();
+        },
+      });
+
+      function refreshListener() {
+        willSaveListener?.dispose();
+        willSaveListener = setupListener();
+      }
+
+      function setupListener() {
+        if (
+          (syntaxes as Array<string | null>).includes(editor.document.syntax) &&
+          preferences.getOverridableBoolean(organizeImportsOnSaveKey)
+        ) {
+          return editor.onWillSave(async (editor) =>
+            nova.commands.invoke(
+              "apexskier.typescript.commands.organizeImports",
+              editor
+            )
+          );
+        }
+        return null;
+      }
+    })
+  );
+
   getTsVersion(tslibPath).then((version) => {
     informationView.tsVersion = version;
   });