Repository: elm-tooling/elm-language-server (ts)

Issue:
Add code action for creating missing function in other module
Similar to "Create local function", offer to create a function in another module.

Pull request:
Add code action to create function in another module
Closes #576.

Patch:
diff --git a/src/compiler/references.ts b/src/compiler/references.ts
index 49f91da..f593be7 100644
--- a/src/compiler/references.ts
+++ b/src/compiler/references.ts
@@ -453,10 +453,10 @@ export class References {
                   );
 
                 if (imported) {
-                  const importNameNode = checker.findImportModuleNameNode(
+                  const importNameNode = checker.findImportModuleNameNodes(
                     moduleNameToLookFor,
                     sourceFileToCheck,
-                  );
+                  )[0];
 
                   if (importNameNode) {
                     references.push({ node: importNameNode, uri });
diff --git a/src/compiler/typeChecker.ts b/src/compiler/typeChecker.ts
index 9254d7c..fb26229 100644
--- a/src/compiler/typeChecker.ts
+++ b/src/compiler/typeChecker.ts
@@ -91,10 +91,10 @@ export interface TypeChecker {
     sourceFile: ISourceFile,
     cancellationToken?: ICancellationToken,
   ) => Diagnostic[];
-  findImportModuleNameNode: (
+  findImportModuleNameNodes: (
     moduleNameOrAlias: string,
     sourceFile: ISourceFile,
-  ) => SyntaxNode | undefined;
+  ) => SyntaxNode[];
 }
 
 export function createTypeChecker(program: IProgram): TypeChecker {
@@ -123,7 +123,7 @@ export function createTypeChecker(program: IProgram): TypeChecker {
     getDiagnostics,
     getDiagnosticsAsync,
     getSuggestionDiagnostics,
-    findImportModuleNameNode,
+    findImportModuleNameNodes,
   };
 
   return typeChecker;
@@ -549,7 +549,7 @@ export function createTypeChecker(program: IProgram): TypeChecker {
 
         const moduleNameOrAlias = nodeParent.text.substring(0, endPos);
         const moduleName =
-          findImportModuleNameNode(moduleNameOrAlias, sourceFile)?.text ??
+          findImportModuleNameNodes(moduleNameOrAlias, sourceFile)[0]?.text ??
           moduleNameOrAlias;
 
         const moduleImport = findImport(sourceFile, moduleName, "Module")[0];
@@ -564,7 +564,7 @@ export function createTypeChecker(program: IProgram): TypeChecker {
 
       const moduleImport = findImport(
         sourceFile,
-        findImportModuleNameNode(upperCaseQidText, sourceFile)?.text ??
+        findImportModuleNameNodes(upperCaseQidText, sourceFile)[0]?.text ??
           upperCaseQidText,
         "Module",
       )[0];
@@ -642,7 +642,7 @@ export function createTypeChecker(program: IProgram): TypeChecker {
               ?.map((node) => node.text)
               .join(".") ?? "";
           const moduleName =
-            findImportModuleNameNode(moduleNameOrAlias, sourceFile)?.text ??
+            findImportModuleNameNodes(moduleNameOrAlias, sourceFile)[0]?.text ??
             moduleNameOrAlias;
 
           const moduleImport = findImport(sourceFile, moduleName, "Module")[0];
@@ -762,7 +762,7 @@ export function createTypeChecker(program: IProgram): TypeChecker {
 
       const moduleImport = findImport(
         sourceFile,
-        findImportModuleNameNode(fullModuleName, sourceFile)?.text ??
+        findImportModuleNameNodes(fullModuleName, sourceFile)[0]?.text ??
           fullModuleName,
         "Module",
       )[0];
@@ -850,7 +850,8 @@ export function createTypeChecker(program: IProgram): TypeChecker {
       return "";
     }
 
-    const moduleImport = findImportModuleNameNode(module, sourceFile)?.parent;
+    const moduleImport = findImportModuleNameNodes(module, sourceFile)[0]
+      ?.parent;
 
     if (!moduleImport) {
       return;
@@ -875,15 +876,17 @@ export function createTypeChecker(program: IProgram): TypeChecker {
   /**
    * Get the module name node if the name is an alias
    */
-  function findImportModuleNameNode(
+  function findImportModuleNameNodes(
     moduleNameOrAlias: string,
     sourceFile: ISourceFile,
-  ): SyntaxNode | undefined {
+  ): SyntaxNode[] {
     return (
       sourceFile.symbolLinks
         ?.get(sourceFile.tree.rootNode)
-        ?.get(moduleNameOrAlias, (s) => s.type === "Import")
-        ?.node.childForFieldName("moduleName") ?? undefined
+        ?.getAll(moduleNameOrAlias)
+        ?.filter((s) => s.type === "Import")
+        .map((s) => s.node.childForFieldName("moduleName"))
+        .filter(Utils.notUndefinedOrNull.bind(createTypeChecker)) ?? []
     );
   }
 
diff --git a/src/providers/codeAction/addMissingRecordFieldCodeAction.ts b/src/providers/codeAction/addMissingRecordFieldCodeAction.ts
index c73375d..560dc86 100644
--- a/src/providers/codeAction/addMissingRecordFieldCodeAction.ts
+++ b/src/providers/codeAction/addMissingRecordFieldCodeAction.ts
@@ -8,6 +8,7 @@ import { Diagnostics } from "../../compiler/diagnostics";
 import { TypeChecker } from "../../compiler/typeChecker";
 import { Type } from "../../compiler/typeInference";
 import { ICodeActionParams } from "../paramsExtensions";
+import { Utils } from "../../util/utils";
 
 const errorCodes = [Diagnostics.RecordField.code];
 const fixId = "add_missing_record_field";
@@ -42,7 +43,7 @@ CodeActionProvider.registerCodeAction({
         // Not used
       },
       (changes, diagnostic) => {
-        mergeChanges(changes, getEdits(params, diagnostic.range));
+        Utils.mergeChanges(changes, getEdits(params, diagnostic.range));
       },
     );
   },
@@ -135,16 +136,3 @@ function createFields(
 
   return {};
 }
-
-function mergeChanges(
-  a: { [uri: string]: TextEdit[] },
-  b: { [uri: string]: TextEdit[] },
-): void {
-  Object.entries(b).forEach(([uri, edits]) => {
-    if (a[uri]) {
-      a[uri].push(...edits);
-    } else {
-      a[uri] = edits;
-    }
-  });
-}
diff --git a/src/providers/codeAction/index.ts b/src/providers/codeAction/index.ts
index 1a06af5..b839e45 100644
--- a/src/providers/codeAction/index.ts
+++ b/src/providers/codeAction/index.ts
@@ -1,5 +1,6 @@
 import "./importCodeAction";
 import "./makeDeclarationFromUsageCodeAction";
+import "./makeExternalDeclarationFromUsageCodeAction";
 import "./addTypeAnnotationCodeAction";
 import "./extractFunctionCodeAction";
 import "./exposeUnexposeCodeAction";
diff --git a/src/providers/codeAction/makeExternalDeclarationFromUsageCodeAction.ts b/src/providers/codeAction/makeExternalDeclarationFromUsageCodeAction.ts
new file mode 100644
index 0000000..e1e6279
--- /dev/null
+++ b/src/providers/codeAction/makeExternalDeclarationFromUsageCodeAction.ts
@@ -0,0 +1,114 @@
+import { Range } from "vscode-languageserver";
+import { TextEdit } from "vscode-languageserver-textdocument";
+import { RefactorEditUtils } from "../../util/refactorEditUtils";
+import { TreeUtils } from "../../util/treeUtils";
+import { Diagnostics } from "../../compiler/diagnostics";
+import { CodeActionProvider, ICodeAction } from "../codeActionProvider";
+import { ICodeActionParams } from "../paramsExtensions";
+import { Utils } from "../../util/utils";
+
+const errorCodes = [Diagnostics.MissingValue.code];
+const fixId = "make_external_declaration_from_usage";
+
+CodeActionProvider.registerCodeAction({
+  errorCodes,
+  fixId,
+  getCodeActions: (params: ICodeActionParams) => {
+    const allEdits = getEdits(params, params.range);
+
+    return allEdits
+      .map(([edits, moduleName]) => {
+        if (Object.keys(edits).length > 0) {
+          return CodeActionProvider.getCodeAction(
+            params,
+            `Create function in module '${moduleName}'`,
+            edits,
+          );
+        }
+      })
+      .filter(Utils.notUndefinedOrNull.bind(CodeActionProvider));
+  },
+  getFixAllCodeAction: (params: ICodeActionParams): ICodeAction | undefined => {
+    return CodeActionProvider.getFixAllCodeAction(
+      "Create all missing external functions",
+      params,
+      errorCodes,
+      fixId,
+      () => {
+        // Not used
+      },
+      (edits, diagnostic) => {
+        const firstEdits = getEdits(params, diagnostic.range)[0];
+        if (firstEdits) {
+          Utils.mergeChanges(edits, firstEdits[0]);
+        }
+      },
+    );
+  },
+});
+
+function getEdits(
+  params: ICodeActionParams,
+  range: Range,
+): [{ [uri: string]: TextEdit[] }, string][] {
+  const nodeAtPosition = TreeUtils.getNamedDescendantForRange(
+    params.sourceFile,
+    range,
+  );
+
+  if (
+    nodeAtPosition.type === "value_qid" &&
+    nodeAtPosition.parent?.type === "value_expr"
+  ) {
+    const funcName = nodeAtPosition.lastNamedChild?.text ?? nodeAtPosition.text;
+    const modulePrefix = nodeAtPosition.namedChildren
+      .slice(0, -2)
+      .map((n) => n.text)
+      .join("");
+
+    const checker = params.program.getTypeChecker();
+
+    const moduleNames = checker
+      .findImportModuleNameNodes(modulePrefix, params.sourceFile)
+      .map((n) => n.text);
+
+    return moduleNames
+      .map((moduleName) => {
+        const destinationSource = params.program.getSourceFileOfImportableModule(
+          params.sourceFile,
+          moduleName,
+        );
+
+        if (destinationSource && destinationSource.writeable) {
+          const type = checker.findType(nodeAtPosition);
+          const typeString: string = checker.typeToString(
+            type,
+            params.sourceFile,
+          );
+
+          const edit = RefactorEditUtils.createTopLevelFunction(
+            destinationSource.tree.rootNode.endPosition.row,
+            funcName,
+            typeString,
+            type.nodeType === "Function" ? type.params.length : 0,
+            `Debug.todo "TODO"`,
+          );
+
+          const exposeEdit = RefactorEditUtils.exposeValueInModule(
+            destinationSource.tree,
+            funcName,
+          );
+
+          if (edit && exposeEdit) {
+            return [
+              { [destinationSource.uri]: [edit, exposeEdit] },
+              moduleName,
+            ] as [{ [uri: string]: TextEdit[] }, string];
+          }
+        }
+      })
+      .filter(Utils.notUndefinedOrNull.bind(getEdits));
+  }
+
+  return [];
+}
diff --git a/src/providers/completionProvider.ts b/src/providers/completionProvider.ts
index 71c49e3..1c7b45e 100644
--- a/src/providers/completionProvider.ts
+++ b/src/providers/completionProvider.ts
@@ -558,10 +558,10 @@ export class CompletionProvider {
       const dotIndex = label.lastIndexOf(".");
       const valuePart = label.slice(dotIndex + 1);
 
-      const importNode = checker.findImportModuleNameNode(
+      const importNode = checker.findImportModuleNameNodes(
         element.fromModule.name,
         sourceFile,
-      )?.parent;
+      )[0]?.parent;
 
       // Check if a value is already imported for this module using the exposing list
       // In this case, we want to prefex the unqualified value since they are using the import exposing list
@@ -1156,14 +1156,14 @@ export class CompletionProvider {
             } else if (!aMatches && bMatches) {
               return 1;
             } else {
-              const aModuleImported = !!checker.findImportModuleNameNode(
+              const aModuleImported = !!checker.findImportModuleNameNodes(
                 a.module,
                 sourceFile,
-              );
-              const bModuleImported = !!checker.findImportModuleNameNode(
+              )[0];
+              const bModuleImported = !!checker.findImportModuleNameNodes(
                 b.module,
                 sourceFile,
-              );
+              )[0];
 
               if (aModuleImported && !bModuleImported) {
                 return -1;
diff --git a/src/util/utils.ts b/src/util/utils.ts
index 68bb2d5..62d2ef5 100644
--- a/src/util/utils.ts
+++ b/src/util/utils.ts
@@ -1,4 +1,4 @@
-import { Range } from "vscode-languageserver";
+import { Range, TextEdit } from "vscode-languageserver";
 
 export type NonEmptyArray<T> = [T, ...T[]];
 
@@ -84,4 +84,17 @@ export class Utils {
     }
     return newArray;
   }
+
+  public static mergeChanges(
+    a: { [uri: string]: TextEdit[] },
+    b: { [uri: string]: TextEdit[] },
+  ): void {
+    Object.entries(b).forEach(([uri, edits]) => {
+      if (a[uri]) {
+        a[uri].push(...edits);
+      } else {
+        a[uri] = edits;
+      }
+    });
+  }
 }


Test patch:
diff --git a/test/codeActionTests/codeActionTestBase.ts b/test/codeActionTests/codeActionTestBase.ts
index d18cd4a..784d638 100644
--- a/test/codeActionTests/codeActionTestBase.ts
+++ b/test/codeActionTests/codeActionTestBase.ts
@@ -141,15 +141,16 @@ export async function testCodeAction(
       trimTrailingWhitespace(expectedResultAfterEdits),
     );
 
+    const changesToApply = codeActions[testFixAll ? codeActions.length - 1 : 0]
+      .edit!.changes!;
+
     Object.entries(expectedSources).forEach(([uri, source]) => {
-      expect(
-        applyEditsToSource(
-          stripCommentLines(result.sources[uri]),
-          codeActions[testFixAll ? codeActions.length - 1 : 0].edit!.changes![
-            URI.file(baseUri + uri).toString()
-          ],
-        ),
-      ).toEqual(source);
+      const edits = changesToApply[URI.file(baseUri + uri).toString()];
+      if (edits) {
+        expect(
+          applyEditsToSource(stripCommentLines(result.sources[uri]), edits),
+        ).toEqual(source);
+      }
     });
   }
 }
diff --git a/test/codeActionTests/makeExternalDeclarationFromUsageCodeActions.test.ts b/test/codeActionTests/makeExternalDeclarationFromUsageCodeActions.test.ts
new file mode 100644
index 0000000..14eeb9e
--- /dev/null
+++ b/test/codeActionTests/makeExternalDeclarationFromUsageCodeActions.test.ts
@@ -0,0 +1,172 @@
+import { testCodeAction } from "./codeActionTestBase";
+
+describe("make external declaration from usage", () => {
+  it("make external function", async () => {
+    const source = `
+--@ Test.elm
+module Test exposing (..)
+
+import App
+
+func = App.bar
+          --^
+
+--@ App.elm
+module App exposing (foo)
+
+foo = ""
+`;
+
+    const expectedSource = `
+--@ Test.elm
+module Test exposing (..)
+
+import App
+
+func = App.bar
+          --^
+
+--@ App.elm
+module App exposing (foo, bar)
+
+foo = ""
+
+
+
+bar : a
+bar =
+    Debug.todo "TODO"`;
+
+    await testCodeAction(
+      source,
+      [{ title: `Create function in module 'App'` }],
+      expectedSource,
+    );
+  });
+
+  it("make external function in nested module", async () => {
+    const source = `
+--@ Test.elm
+module Test exposing (..)
+
+import Modules.App
+
+func = Modules.App.bar
+                  --^
+
+--@ Modules/App.elm
+module Modules.App exposing (foo)
+
+foo = ""
+`;
+
+    const expectedSource = `
+--@ Test.elm
+module Test exposing (..)
+
+import Modules.App
+
+func = Modules.App.bar
+
+--@ Modules/App.elm
+module Modules.App exposing (foo, bar)
+
+foo = ""
+
+
+
+bar : a
+bar =
+    Debug.todo "TODO"`;
+
+    await testCodeAction(
+      source,
+      [{ title: `Create function in module 'Modules.App'` }],
+      expectedSource,
+    );
+  });
+
+  it("make external function in aliased module", async () => {
+    const source = `
+--@ Test.elm
+module Test exposing (..)
+
+import Modules.App as App
+
+func = App.bar
+          --^
+
+--@ Modules/App.elm
+module Modules.App exposing (foo)
+
+foo = ""
+`;
+
+    const expectedSource = `
+--@ Test.elm
+module Test exposing (..)
+
+import Modules.App as App
+
+func = App.bar
+
+--@ Modules/App.elm
+module Modules.App exposing (foo, bar)
+
+foo = ""
+
+
+
+bar : a
+bar =
+    Debug.todo "TODO"`;
+
+    await testCodeAction(
+      source,
+      [{ title: `Create function in module 'Modules.App'` }],
+      expectedSource,
+    );
+  });
+
+  it("make external function in multiple modules", async () => {
+    const source = `
+--@ Test.elm
+module Test exposing (..)
+
+import Modules.App as App
+import App
+
+func = App.bar
+          --^
+
+--@ Modules/App.elm
+module Modules.App exposing (foo)
+
+foo = ""
+
+--@ App.elm
+module Modules.App exposing (foo)
+
+foo = ""
+`;
+
+    await testCodeAction(source, [
+      { title: `Create function in module 'Modules.App'` },
+      { title: `Create function in module 'App'` },
+    ]);
+  });
+
+  it("make external function in non existent module", async () => {
+    const source = `
+--@ Test.elm
+module Test exposing (..)
+
+import Modules.App as App
+
+func = App.bar
+          --^
+`;
+
+    await testCodeAction(source, []);
+  });
+});