Repository: elm-tooling/elm-language-server
Pull request #897 (2023-02-26 15:22:22)

Sort imported value completion above corresponding module prefix completion
For example:
```elm
import Foo exposing (bar)
```

`bar` should be sorted above `Foo.bar`. This was not always the case, depending on the client and filter text. Now we are explicitly sorting them after.


Patch:
diff --git a/src/providers/completionProvider.ts b/src/providers/completionProvider.ts
index 44da1e8..a6be3cb 100644
--- a/src/providers/completionProvider.ts
+++ b/src/providers/completionProvider.ts
@@ -551,20 +551,10 @@ export class CompletionProvider {
   ): CompletionItem[] {
     const completions: CompletionItem[] = [];
 
-    checker.getAllImports(sourceFile).forEach((element): void => {
-      const markdownDocumentation = HintHelper.createHint(element.node);
-      let sortPrefix = "d";
-      if (element.fromModule.maintainerAndPackageName) {
-        const matchedRanking: string = (
-          RANKING_LIST as {
-            [index: string]: string;
-          }
-        )[element.fromModule.maintainerAndPackageName];
+    const imports = checker.getAllImports(sourceFile);
 
-        if (matchedRanking) {
-          sortPrefix = `e${matchedRanking}`;
-        }
-      }
+    imports.forEach((element): void => {
+      const markdownDocumentation = HintHelper.createHint(element.node);
 
       const label = element.name;
       const labelDescription = element.fromModule.name;
@@ -593,6 +583,29 @@ export class CompletionProvider {
         filterText = valuePart;
       }
 
+      let sortPrefix = "d";
+
+      // If the value is imported directly already, then the qualified import should be sorted after
+      if (dotIndex >= 0) {
+        const valueImport = imports.getVar(valuePart, element.fromModule.name);
+
+        if (valueImport.length > 0) {
+          sortPrefix = "e";
+        }
+      }
+
+      if (element.fromModule.maintainerAndPackageName) {
+        const matchedRanking: string = (
+          RANKING_LIST as {
+            [index: string]: string;
+          }
+        )[element.fromModule.maintainerAndPackageName];
+
+        if (matchedRanking) {
+          sortPrefix = `e${matchedRanking}`;
+        }
+      }
+
       switch (element.type) {
         case "Function":
           completions.push(


Test patch:
diff --git a/test/completionProvider.test.ts b/test/completionProvider.test.ts
index 3281f14..0f4a02a 100644
--- a/test/completionProvider.test.ts
+++ b/test/completionProvider.test.ts
@@ -76,9 +76,23 @@ describe("CompletionProvider", () => {
           sourceFile,
         }) ?? [];
 
-      const completionsList = Array.isArray(completions)
-        ? completions
-        : completions.items;
+      const completionsList = (
+        Array.isArray(completions) ? completions : completions.items
+      ).sort((a, b) => {
+        if (a.sortText && b.sortText) {
+          return a.sortText.localeCompare(b.sortText);
+        }
+
+        if (a.sortText) {
+          return -1;
+        }
+
+        if (b.sortText) {
+          return 1;
+        }
+
+        return a.label.localeCompare(b.label);
+      });
 
       if (debug && completionsList.length === 0) {
         console.log(
@@ -106,8 +120,9 @@ describe("CompletionProvider", () => {
         );
       }
 
+      let lastFoundIndex = -1;
       expectedCompletions.forEach((completion) => {
-        let result = !!completionsList.find((c) => {
+        const foundIndex = completionsList.findIndex((c) => {
           if (typeof completion === "string") {
             return c.label === completion;
           } else if ("shouldNotExist" in completion) {
@@ -127,6 +142,8 @@ describe("CompletionProvider", () => {
           }
         });
 
+        const result = foundIndex >= 0;
+
         // Flip result if it should not exist
         if (
           typeof completion === "object" &&
@@ -146,6 +163,21 @@ describe("CompletionProvider", () => {
         }
 
         expect(result).toBe(true);
+
+        if (foundIndex <= lastFoundIndex && debug) {
+          console.log(
+            `Completion ${
+              typeof completion === "object" && "label" in completion
+                ? completion.label
+                : completion
+            } was in the wrong order. Completion order is [ ${completionsList
+              .map((c) => c.label)
+              .join(", ")} ]`,
+          );
+        }
+
+        expect(foundIndex).toBeGreaterThan(lastFoundIndex);
+        lastFoundIndex = foundIndex;
       });
     }
 
@@ -505,7 +537,7 @@ import OtherModule exposing ({-caret-})
 
     await testCompletions(
       otherSource + source,
-      ["testFunction", "Msg", "TestType"],
+      ["Msg", "testFunction", "TestType"],
       "exactMatch",
     );
 
@@ -518,7 +550,7 @@ import OtherModule exposing (testFunction, {-caret-})
 
     await testCompletions(
       otherSource + source2,
-      ["testFunction", "Msg", "TestType"],
+      ["Msg", "testFunction", "TestType"],
       "exactMatch",
     );
 
@@ -531,7 +563,7 @@ import OtherModule exposing (testFunction, T{-caret-})
 
     await testCompletions(
       otherSource + source3,
-      ["testFunction", "Msg", "TestType"],
+      ["Msg", "testFunction", "TestType"],
       "exactMatch",
     );
   });
@@ -553,7 +585,7 @@ type alias TestType =
 
     await testCompletions(
       source,
-      ["testFunc", "Msg", "Msg(..)", "Msg1", "Msg2", "TestType"],
+      ["Msg", "Msg(..)", "Msg1", "Msg2", "testFunc", "TestType"],
       "exactMatch",
     );
 
@@ -573,7 +605,7 @@ type alias TestType =
 
     await testCompletions(
       source2,
-      ["testFunc", "Msg", "Msg(..)", "Msg1", "Msg2", "TestType"],
+      ["Msg", "Msg(..)", "Msg1", "Msg2", "testFunc", "TestType"],
       "exactMatch",
     );
   });
@@ -821,8 +853,8 @@ testFunction = ""
 
     await testCompletions(
       source,
-      ["Home", "Away"],
-      "partialMatch",
+      ["Away", "Home", "Page"],
+      "exactMatch",
       "triggeredByDot",
     );
 
@@ -843,8 +875,8 @@ defaultPage =
 
     await testCompletions(
       source2,
-      ["Home", "Away"],
-      "partialMatch",
+      ["Away", "Home", "Page"],
+      "exactMatch",
       "triggeredByDot",
     );
   });
@@ -947,7 +979,7 @@ test = Module.Submodule.{-caret-}
 
     await testCompletions(
       source2,
-      ["AnotherSubmodule", "testFunction"],
+      ["testFunction", "AnotherSubmodule"],
       "exactMatch",
       "triggeredByDot",
     );
@@ -1087,7 +1119,7 @@ test = div [] [ Module.{-caret-} ]
 
     await testCompletions(
       source,
-      ["testFunc", "Msg", "Msg1", "Msg2", "Model"],
+      ["Model", "Msg", "Msg1", "Msg2", "testFunc"],
       "exactMatch",
       "triggeredByDot",
     );
@@ -1112,7 +1144,7 @@ test = Module.fu{-caret-}
 
     await testCompletions(
       source2,
-      ["testFunc", "Msg", "Msg1", "Msg2", "Model"],
+      ["Model", "Msg", "Msg1", "Msg2", "testFunc"],
       "exactMatch",
     );
 
@@ -1136,7 +1168,7 @@ test = Module.{-caret-}
 
     await testCompletions(
       source3,
-      ["testFunc", "Msg", "Msg1", "Msg2", "Model"],
+      ["Model", "Msg", "Msg1", "Msg2", "testFunc"],
       "exactMatch",
       "triggeredByDot",
     );
@@ -1161,12 +1193,13 @@ test = div [] [ Module.{-caret-} ]
       source,
       [
         {
-          label: "testFunc",
+          label: "Model",
           detail: "Auto import module 'Module'",
           additionalTextEdits: [
             TextEdit.insert(Position.create(1, 0), "import Module\n"),
           ],
         },
+
         {
           label: "Msg",
           detail: "Auto import module 'Module'",
@@ -1189,7 +1222,7 @@ test = div [] [ Module.{-caret-} ]
           ],
         },
         {
-          label: "Model",
+          label: "testFunc",
           detail: "Auto import module 'Module'",
           additionalTextEdits: [
             TextEdit.insert(Position.create(1, 0), "import Module\n"),
@@ -1225,12 +1258,13 @@ test = div [] [ Module.{-caret-} ]
       source,
       [
         {
-          label: "testFunc",
+          label: "Model",
           detail: "Auto import module 'Module'",
           additionalTextEdits: [
             TextEdit.insert(Position.create(3, 0), "import Module\n"),
           ],
         },
+
         {
           label: "Msg",
           detail: "Auto import module 'Module'",
@@ -1253,7 +1287,7 @@ test = div [] [ Module.{-caret-} ]
           ],
         },
         {
-          label: "Model",
+          label: "testFunc",
           detail: "Auto import module 'Module'",
           additionalTextEdits: [
             TextEdit.insert(Position.create(3, 0), "import Module\n"),
@@ -1297,7 +1331,7 @@ view model =
 
     await testCompletions(
       source,
-      ["testFunc", "Data"],
+      ["Data", "testFunc"],
       "exactMatch",
       "triggeredByDot",
     );
@@ -1480,7 +1514,7 @@ testFunction model =
 
   it("Functions from other modules should be importable", async () => {
     const source = `
-    --@ Test.elm
+--@ Test.elm
 module Test exposing (..)
 
 test =
@@ -1613,7 +1647,7 @@ testFunction =
 
     await testCompletions(
       source,
-      ["foo", "Maybe", "Just", "Nothing"],
+      ["foo", "Just", "Maybe", "Nothing"],
       "exactMatch",
     );
   });
@@ -1667,7 +1701,7 @@ testFunction =
 
     await testCompletions(
       source,
-      ["foo", "Maybe", "Just", "Nothing", "bar", "Result", "Ok", "Err"],
+      ["bar", "Err", "foo", "Just", "Maybe", "Nothing", "Ok", "Result"],
       "exactMatch",
     );
   });
@@ -1709,7 +1743,7 @@ testFunction =
     Cmd.{-caret-}
 `;
 
-    await testCompletions(source, ["Cmd", "batch"], "exactMatch");
+    await testCompletions(source, ["batch", "Cmd"], "exactMatch");
   });
 
   it("Completions from lambda function", async () => {
@@ -1769,7 +1803,7 @@ port foo : String -> Cmd msg
 port fbar : (String -> msg) -> Sub msg
 `;
 
-    await testCompletions(source, ["foo", "fbar"], "partialMatch");
+    await testCompletions(source, ["fbar", "foo"], "partialMatch");
   });
 
   it("Completions for record fields", async () => {