Repository: elm-tooling/elm-language-server
Pull request #327 (2020-10-02 22:15:46)

Correctly calculate imported union constructors


Patch:
diff --git a/src/imports.ts b/src/imports.ts
index 1300e61..e8eb217 100644
--- a/src/imports.ts
+++ b/src/imports.ts
@@ -227,22 +227,6 @@ export class Imports implements IImports {
                 };
               }),
             );
-
-            result.push(
-              ...element.exposedUnionConstructors
-                .filter((a) => a.accessibleWithoutPrefix)
-                .map((a) => {
-                  return {
-                    alias: `${a.name}`,
-                    fromModuleName: moduleNameNode.text,
-                    fromUri: uri,
-                    maintainerAndPackageName,
-                    node: a.syntaxNode,
-                    type: "UnionConstructor" as NodeType,
-                    explicitlyExposed: false,
-                  };
-                }),
-            );
           }
           break;
         // Do not handle operators, they are not valid if prefixed
@@ -296,17 +280,43 @@ import Platform.Sub as Sub exposing ( Sub )
     uri: string,
     maintainerAndPackageName?: string,
   ): IImport[] {
-    return exposed.map((element: IExposing) => {
-      return {
-        alias: element.name,
-        fromModuleName: moduleName,
-        fromUri: uri,
-        maintainerAndPackageName,
-        node: element.syntaxNode,
-        type: element.type,
-        explicitlyExposed: false,
-      };
-    });
+    const result: IImport[] = [];
+
+    // These need to be added additionally, as they are nested
+    exposed
+      .filter((it) => it.type === "Type")
+      .forEach((element) => {
+        if (element.exposedUnionConstructors) {
+          result.push(
+            ...element.exposedUnionConstructors.map((a) => {
+              return {
+                alias: `${a.name}`,
+                fromModuleName: moduleName,
+                fromUri: uri,
+                maintainerAndPackageName,
+                node: a.syntaxNode,
+                type: "UnionConstructor" as NodeType,
+                explicitlyExposed: false,
+              };
+            }),
+          );
+        }
+      });
+
+    return [
+      ...result,
+      ...exposed.map((element: IExposing) => {
+        return {
+          alias: element.name,
+          fromModuleName: moduleName,
+          fromUri: uri,
+          maintainerAndPackageName,
+          node: element.syntaxNode,
+          type: element.type,
+          explicitlyExposed: false,
+        };
+      }),
+    ];
   }
 
   private exposedNodesToImports(
@@ -322,16 +332,32 @@ import Platform.Sub as Sub exposing ( Sub )
     moduleNameNode: SyntaxNode,
     foundModule: ITreeContainer,
   ): IImport[] {
-    return exposedNodes.map((a) => {
-      return {
-        alias: a.name,
-        fromModuleName: moduleNameNode.text,
-        fromUri: foundModule.uri,
-        maintainerAndPackageName: foundModule.maintainerAndPackageName,
-        node: a.syntaxNode,
-        type: a.type,
-        explicitlyExposed: true,
-      };
-    });
+    return exposedNodes
+      .map((a) => {
+        return [
+          {
+            alias: a.name,
+            fromModuleName: moduleNameNode.text,
+            fromUri: foundModule.uri,
+            maintainerAndPackageName: foundModule.maintainerAndPackageName,
+            node: a.syntaxNode,
+            type: a.type,
+            explicitlyExposed: true,
+          },
+        ].concat(
+          a.exposedUnionConstructors?.map((b) => {
+            return {
+              alias: b.name,
+              fromModuleName: moduleNameNode.text,
+              fromUri: foundModule.uri,
+              maintainerAndPackageName: foundModule.maintainerAndPackageName,
+              node: b.syntaxNode,
+              type: "UnionConstructor",
+              explicitlyExposed: false,
+            };
+          }) ?? [],
+        );
+      })
+      .reduce((a, b) => a.concat(b), []);
   }
 }
diff --git a/src/util/treeUtils.ts b/src/util/treeUtils.ts
index 92d9ffd..5baf876 100644
--- a/src/util/treeUtils.ts
+++ b/src/util/treeUtils.ts
@@ -28,7 +28,6 @@ export interface IExposing {
   exposedUnionConstructors?: {
     name: string;
     syntaxNode: SyntaxNode;
-    accessibleWithoutPrefix: boolean;
   }[];
 }
 
@@ -142,7 +141,6 @@ export class TreeUtils {
               const unionConstructors: {
                 name: string;
                 syntaxNode: SyntaxNode;
-                accessibleWithoutPrefix: boolean;
               }[] = [];
               TreeUtils.descendantsOfType(
                 typeDeclaration,
@@ -154,7 +152,6 @@ export class TreeUtils {
                 );
                 if (name && name.parent) {
                   unionConstructors.push({
-                    accessibleWithoutPrefix: false,
                     name: name.text,
                     syntaxNode: name.parent,
                   });
@@ -231,7 +228,6 @@ export class TreeUtils {
                   const unionConstructors: {
                     name: string;
                     syntaxNode: SyntaxNode;
-                    accessibleWithoutPrefix: boolean;
                   }[] = [];
                   TreeUtils.descendantsOfType(
                     typeDeclaration,
@@ -243,7 +239,6 @@ export class TreeUtils {
                     );
                     if (unionConstructorName && unionConstructorName.parent) {
                       unionConstructors.push({
-                        accessibleWithoutPrefix: true,
                         name: unionConstructorName.text,
                         syntaxNode: unionConstructorName.parent,
                       });
@@ -2155,7 +2150,6 @@ export class TreeUtils {
           .filter(
             (imp) =>
               imp.fromModuleName === module &&
-              imp.explicitlyExposed &&
               (imp.type === "Type" ||
                 imp.type === "TypeAlias" ||
                 imp.type === "UnionConstructor"),


Test patch:
diff --git a/test/definitionProviderTests/aliasedImportDefinition.test.ts b/test/definitionProviderTests/aliasedImportDefinition.test.ts
index 43c514f..4978ed0 100644
--- a/test/definitionProviderTests/aliasedImportDefinition.test.ts
+++ b/test/definitionProviderTests/aliasedImportDefinition.test.ts
@@ -31,14 +31,14 @@ type Page = Home
     await testBase.testDefinition(source);
   });
 
-  xit(`test aliased, qualified union constructor ref`, async () => {
+  it(`test aliased, qualified union constructor ref`, async () => {
     const source = `
 --@ main.elm
 import App as A
 defaultPage = A.Home
                 --^App.elm
 --@ App.elm
-module App exposing (Page(Home))
+module App exposing (Page(..))
 type Page = Home
            --X
 `;
diff --git a/test/definitionProviderTests/importDefinition.test.ts b/test/definitionProviderTests/importDefinition.test.ts
index a727188..23247ce 100644
--- a/test/definitionProviderTests/importDefinition.test.ts
+++ b/test/definitionProviderTests/importDefinition.test.ts
@@ -163,41 +163,28 @@ type Page = Home
     await testBase.testDefinition(source);
   });
 
-  xit(`test union constructor ref in import exposing list`, async () => {
+  it(`test union constructor ref in expression`, async () => {
     const source = `
 --@ main.elm
-import App exposing (Page(Home))
-                          --^App.elm
---@ App.elm
-module App exposing (Page(Home))
-type Page = Home
-           --X
-`;
-    await testBase.testDefinition(source);
-  });
-
-  xit(`test union constructor ref in expression`, async () => {
-    const source = `
---@ main.elm
-import App exposing (Page(Home))
+import App exposing (Page(..))
 defaultPage = Home
               --^App.elm
 --@ App.elm
-module App exposing (Page(Home))
+module App exposing (Page(..))
 type Page = Home
            --X
 `;
     await testBase.testDefinition(source);
   });
 
-  xit(`test union constructor ref in expression via import exposing all constructors`, async () => {
+  it(`test union constructor ref in expression via import exposing all constructors`, async () => {
     const source = `
 --@ main.elm
 import App exposing (Page(..))
 defaultPage = Home
               --^App.elm
 --@ App.elm
-module App exposing (Page(Home))
+module App exposing (Page(..))
 type Page = Home
            --X
 `;
@@ -207,7 +194,7 @@ type Page = Home
   it(`test union constructor ref in expression via module exposing all constructors`, async () => {
     const source = `
 --@ main.elm
-import App exposing (Page(Home))
+import App exposing (Page(..))
 defaultPage = Home
               --^App.elm
 --@ App.elm
@@ -218,7 +205,7 @@ type Page = Home
     await testBase.testDefinition(source);
   });
 
-  xit(`test union constructor ref in expression exposing all from both sides`, async () => {
+  it(`test union constructor ref in expression exposing all from both sides`, async () => {
     const source = `
 --@ main.elm
 import App exposing (..)
@@ -235,7 +222,7 @@ type Page = Home
   it(`test union constructor ref in expression but not exposed by module`, async () => {
     const source = `
 --@ main.elm
-import App exposing (Page(Home))
+import App exposing (Page(..))
 defaultPage = Home
               --^unresolved
 --@ App.elm
@@ -252,8 +239,8 @@ import Foo as
 import App exposing (Page(..))
 defaultPage = Home
               --^App.elm
---@ App.elmtest union constructor ref in
-module App exposing (Page(Home))
+--@ App.elm
+module App exposing (Page(..))
 type Page = Home
            --X
 --@Foo.elm
@@ -262,16 +249,16 @@ module Foo exposing(..)
     await testBase.testDefinition(source);
   });
 
-  xit(`test union constructor ref in pattern destructuring`, async () => {
+  it(`test union constructor ref in pattern destructuring`, async () => {
     const source = `
 --@ main.elm
-import App exposing (Page(Home))
+import App exposing (Page(..))
 title page =
     case page of
         Home -> "home"
         --^App.elm
 --@ App.elm
-module App exposing (Page(Home))
+module App exposing (Page(..))
 type Page = Home
            --X
 `;
@@ -471,21 +458,6 @@ power a b = 42
     await testBase.testDefinition(source);
   });
 
-  xit(`test binary operator as a function`, async () => {
-    const source = `
---@ main.elm
-import Math exposing ((**))
-f = (**) 2 3
-    --^Math.elm
---@ Math.elm
-module Math exposing ((**))
-infix left 5 (**) = power
-power a b = 42
---X
-`;
-    await testBase.testDefinition(source);
-  });
-
   it(`test layered import using second import`, async () => {
     const source = `
 --@ main.elm
diff --git a/test/definitionProviderTests/qualifiedImportDefinition.test.ts b/test/definitionProviderTests/qualifiedImportDefinition.test.ts
index 98ef755..8a99e75 100644
--- a/test/definitionProviderTests/qualifiedImportDefinition.test.ts
+++ b/test/definitionProviderTests/qualifiedImportDefinition.test.ts
@@ -45,21 +45,21 @@ type Page = Home
     await testBase.testDefinition(source);
   });
 
-  xit(`test qualified union constructor ref`, async () => {
+  it(`test qualified union constructor ref`, async () => {
     const source = `
 --@ main.elm
 import App
 defaultPage = App.Home
                   --^App.elm
 --@ App.elm
-module App exposing (Page(Home))
+module App exposing (Page(..))
 type Page = Home
            --X
 `;
     await testBase.testDefinition(source);
   });
 
-  xit(`test qualified union constructor ref in pattern destructuring`, async () => {
+  it(`test qualified union constructor ref in pattern destructuring`, async () => {
     const source = `
 --@ main.elm
 import App
@@ -68,7 +68,7 @@ title page =
         App.Home -> "home"
             --^App.elm
 --@ App.elm
-module App exposing (Page(Home))
+module App exposing (Page(..))
 type Page = Home
            --X
 `;
diff --git a/test/definitionProviderTests/typeResolveDefinition.test.ts b/test/definitionProviderTests/typeResolveDefinition.test.ts
index ea3d1b2..f1bfe98 100644
--- a/test/definitionProviderTests/typeResolveDefinition.test.ts
+++ b/test/definitionProviderTests/typeResolveDefinition.test.ts
@@ -45,16 +45,6 @@ title page =
     await testBase.testDefinition(source);
   });
 
-  xit(`test union constructor ref from module exposing list`, async () => {
-    const source = `
-module Main exposing (Page(Home))
-                           --^
-type Page = Home
-            --X
-`;
-    await testBase.testDefinition(source);
-  });
-
   it(`test type alias ref from module exposing list`, async () => {
     const source = `
 module Main exposing (Person)
@@ -155,11 +145,11 @@ func: User
 func =
 User { data = "" }
 `;
-await testBase.testDefinition(source);
-});
+    await testBase.testDefinition(source);
+  });
 
-it(`test type declaration resolves to itself`, async () => {
-  const source = `
+  it(`test type declaration resolves to itself`, async () => {
+    const source = `
   type UnitlessFloat
   --X   --^
     = UnitlessFloat
@@ -167,8 +157,8 @@ it(`test type declaration resolves to itself`, async () => {
     await testBase.testDefinition(source);
   });
 
-it(`test union contructor resolves to itself`, async () => {
-  const source = `
+  it(`test union contructor resolves to itself`, async () => {
+    const source = `
   type UnitlessFloat
   = UnitlessFloat
     --X   --^