Repository: elm-tooling/elm-language-server
Pull request #569 (2021-03-31 12:11:41)

Fix major performance issue with getting possible imports
When `exp.node` was a `type_alias_declaration`, we were calling `findFirstNamedChildOfType` on the `file` many times, causing a big performance hit.


Patch:
diff --git a/src/providers/codeActionLs/removeUnusedCodeAction.ts b/src/providers/codeActionLs/removeUnusedCodeAction.ts
index e4f27e6..d8e001d 100644
--- a/src/providers/codeActionLs/removeUnusedCodeAction.ts
+++ b/src/providers/codeActionLs/removeUnusedCodeAction.ts
@@ -39,7 +39,7 @@ CodeActionProvider.registerCodeAction({
   getFixAllCodeAction: (params) => {
     const importsMap = new Map<string, Set<string>>();
     return CodeActionProvider.getFixAllCodeAction(
-      "Remove all reported unused code",
+      "Remove all unused code",
       params,
       errorCodes,
       fixId,
diff --git a/src/providers/diagnostics/elmLsDiagnostics.ts b/src/providers/diagnostics/elmLsDiagnostics.ts
index 271fd26..f7df28b 100644
--- a/src/providers/diagnostics/elmLsDiagnostics.ts
+++ b/src/providers/diagnostics/elmLsDiagnostics.ts
@@ -970,19 +970,6 @@ export class ElmLsDiagnostics {
   private getUnusedTypeAliasDiagnostics(tree: Tree): IDiagnostic[] {
     const diagnostics: IDiagnostic[] = [];
 
-    // Currently if the file exports the variant, either through an explicit
-    // reference to the type, or through a '(..)' to expose everything, then
-    // we won't mark the value constructor as unused. Ideally this should take
-    // multiple files into account, then these conditions can be removed.
-    const exposingAll = !!tree.rootNode
-      .childForFieldName("moduleDeclaration")
-      ?.childForFieldName("exposing")
-      ?.childForFieldName("doubleDot");
-
-    if (exposingAll) {
-      return diagnostics;
-    }
-
     const typeAliases = this.typeAliasesQuery
       .matches(tree.rootNode)
       .map((match) => match.captures[0].node)
diff --git a/src/util/importUtils.ts b/src/util/importUtils.ts
index 1f75847..7b78d26 100644
--- a/src/util/importUtils.ts
+++ b/src/util/importUtils.ts
@@ -77,11 +77,8 @@ export class ImportUtils {
         });
 
         exposed.constructors?.forEach((exp) => {
-          if (exp.node.parent) {
-            const value = TreeUtils.findFirstNamedChildOfType(
-              "upper_case_identifier",
-              exp.node.parent,
-            )?.text;
+          if (exp.node.parent && exp.type === "UnionConstructor") {
+            const value = exp.node.parent.childForFieldName("name")?.text;
 
             if (value) {
               exposedValues.push({


Test patch:
diff --git a/test/codeActionTests/removeUnusedCodeAction.test.ts b/test/codeActionTests/removeUnusedCodeAction.test.ts
index 3b9d64d..cd49cc9 100644
--- a/test/codeActionTests/removeUnusedCodeAction.test.ts
+++ b/test/codeActionTests/removeUnusedCodeAction.test.ts
@@ -104,7 +104,7 @@ func = ""
 
     await testCodeAction(
       source,
-      [{ title: "Remove all reported unused code" }],
+      [{ title: "Remove all unused code" }],
       expectedSource,
       true,
     );
@@ -132,7 +132,7 @@ func = foo
 
     await testCodeAction(
       source,
-      [{ title: "Remove all reported unused code" }],
+      [{ title: "Remove all unused code" }],
       expectedSource,
       true,
     );
@@ -160,7 +160,7 @@ func = foo
 
     await testCodeAction(
       source,
-      [{ title: "Remove all reported unused code" }],
+      [{ title: "Remove all unused code" }],
       expectedSource,
       true,
     );
@@ -188,7 +188,7 @@ func = foo
 
     await testCodeAction(
       source,
-      [{ title: "Remove all reported unused code" }],
+      [{ title: "Remove all unused code" }],
       expectedSource,
       true,
     );
@@ -216,7 +216,7 @@ func = foo + bar
 
     await testCodeAction(
       source,
-      [{ title: "Remove all reported unused code" }],
+      [{ title: "Remove all unused code" }],
       expectedSource,
       true,
     );
@@ -244,7 +244,7 @@ func = foo + bar
 
     await testCodeAction(
       source,
-      [{ title: "Remove all reported unused code" }],
+      [{ title: "Remove all unused code" }],
       expectedSource,
       true,
     );
@@ -272,7 +272,7 @@ func = foo + bar
 
     await testCodeAction(
       source,
-      [{ title: "Remove all reported unused code" }],
+      [{ title: "Remove all unused code" }],
       expectedSource,
       true,
     );
@@ -300,7 +300,7 @@ func = foo + bar
 
     await testCodeAction(
       source,
-      [{ title: "Remove all reported unused code" }],
+      [{ title: "Remove all unused code" }],
       expectedSource,
       true,
     );
diff --git a/test/diagnosticTests/elmLsDiagnostics.test.ts b/test/diagnosticTests/elmLsDiagnostics.test.ts
index efece76..0d07d4e 100644
--- a/test/diagnosticTests/elmLsDiagnostics.test.ts
+++ b/test/diagnosticTests/elmLsDiagnostics.test.ts
@@ -1511,49 +1511,6 @@ type Patch
       await testDiagnostics(source, "unused_type_alias", []);
     });
 
-    it("used type alias in different file", async () => {
-      const source = `
---@ Model.elm
-module Model exposing (..)
-
-
-type alias IAmUsed =
-    { used : Bool
-    , optional : Bool
-    }
-
---@ Main.elm
-module Main exposing (main)
-
-import Html
-import Model
-
-
-main : Html.Html msg
-main =
-    view example
-
-
-example : Model.IAmUsed
-example =
-    { used = True
-    , optional = False
-    }
-
-
-view : Model.IAmUsed -> Html.Html msg
-view blah =
-    Html.text <|
-        if blah.used then
-            "used"
-
-        else
-            "not used"
-			`;
-
-      await testDiagnostics(source, "unused_type_alias", []);
-    });
-
     it("unused type alias", async () => {
       const source = `
 module Foo exposing (foo)