Repository: elm-tooling/elm-language-server
Pull request #279 (2020-05-24 12:37:54)

Add data on package imports to differentiate between exposed and not …
…exposed

Fixes https://github.com/elm-tooling/elm-language-server/issues/274


Patch:
diff --git a/src/elmWorkspace.ts b/src/elmWorkspace.ts
index 5ad2920..210b3f2 100644
--- a/src/elmWorkspace.ts
+++ b/src/elmWorkspace.ts
@@ -18,6 +18,7 @@ interface IFolder {
   path: string;
   maintainerAndPackageName?: string;
   writeable: boolean;
+  isExposed: boolean;
 }
 
 export interface IElmWorkspace {
@@ -79,7 +80,9 @@ export class ElmWorkspace implements IElmWorkspace {
     return this.rootPath;
   }
 
-  private async initWorkspace(progressCallback: (percent: number) => void) {
+  private async initWorkspace(
+    progressCallback: (percent: number) => void,
+  ): Promise<void> {
     let progress = 0;
     let elmVersion;
     try {
@@ -136,11 +139,11 @@ export class ElmWorkspace implements IElmWorkspace {
           : { ...elmJson.dependencies, ...elmJson["test-dependencies"] };
       if (type === "application") {
         for (const key in dependencies) {
-          if (dependencies.hasOwnProperty(key)) {
+          if (Object.prototype.hasOwnProperty.call(dependencies, key)) {
             const maintainer = key.substring(0, key.indexOf("/"));
             const packageName = key.substring(key.indexOf("/") + 1, key.length);
 
-            const pathToPackageWithVersion = `${packagesRoot}${maintainer}/${packageName}/${dependencies[key]}/src`;
+            const pathToPackageWithVersion = `${packagesRoot}${maintainer}/${packageName}/${dependencies[key]}`;
             this.elmFolders.push({
               maintainerAndPackageName: `${maintainer}/${packageName}`,
               uri: pathToPackageWithVersion,
@@ -150,7 +153,7 @@ export class ElmWorkspace implements IElmWorkspace {
         }
       } else {
         for (const key in dependencies) {
-          if (dependencies.hasOwnProperty(key)) {
+          if (Object.prototype.hasOwnProperty.call(dependencies, key)) {
             const maintainer = key.substring(0, key.indexOf("/"));
             const packageName = key.substring(key.indexOf("/") + 1, key.length);
 
@@ -169,10 +172,10 @@ export class ElmWorkspace implements IElmWorkspace {
               dependencies[key],
             );
             const pathToPackageWithVersion = matchedFolder
-              ? `${matchedFolder.versionPath}/src`
+              ? `${matchedFolder.versionPath}`
               : `${
                   allVersionFolders[allVersionFolders.length - 1].versionPath
-                }/src`;
+                }`;
 
             this.elmFolders.push({
               maintainerAndPackageName: `${maintainer}/${packageName}`,
@@ -183,7 +186,7 @@ export class ElmWorkspace implements IElmWorkspace {
         }
       }
 
-      const elmFilePaths = this.findElmFilesInFolders(this.elmFolders);
+      const elmFilePaths = await this.findElmFilesInFolders(this.elmFolders);
       this.connection.console.info(
         `Found ${elmFilePaths.length.toString()} files to add to the project`,
       );
@@ -225,42 +228,106 @@ export class ElmWorkspace implements IElmWorkspace {
     }
   }
 
-  private findElmFilesInFolders(
+  private async findElmFilesInFolders(
     elmFolders: {
       uri: string;
       writeable: boolean;
       maintainerAndPackageName?: string;
     }[],
-  ): IFolder[] {
-    let elmFilePaths: IFolder[] = [];
+  ): Promise<IFolder[]> {
+    let elmFilePathPromises: Promise<IFolder[]>[] = [];
     for (const element of elmFolders) {
-      elmFilePaths = elmFilePaths.concat(this.findElmFilesInFolder(element));
+      elmFilePathPromises = elmFilePathPromises.concat(
+        this.findElmFilesInFolder(element),
+      );
     }
-    return elmFilePaths;
+    return (await Promise.all(elmFilePathPromises)).reduce(
+      (a, b) => a.concat(b),
+      [],
+    );
   }
 
-  private findElmFilesInFolder(element: {
+  private async findElmFilesInFolder(element: {
     uri: string;
     writeable: boolean;
     maintainerAndPackageName?: string;
-  }): IFolder[] {
+  }): Promise<IFolder[]> {
     // Cleanup the path on windows, as globby does not like backslashes
     const globUri = element.uri.replace(/\\/g, "/");
 
-    return globby
-      .sync(`${globUri}/**/*.elm`, { suppressErrors: true })
-      .map((matchingPath) => ({
+    // As packages are not writeable, we want to handle these differently
+    if (element.writeable) {
+      return (
+        await globby(`${globUri}/**/*.elm`, {
+          suppressErrors: true,
+        })
+      ).map((matchingPath) => ({
         maintainerAndPackageName: element.maintainerAndPackageName,
         path: matchingPath,
         writeable: element.writeable,
+        isExposed: true,
       }));
+    } else {
+      const [elmFiles, elmJsonString] = await Promise.all([
+        globby(`${globUri}/src/**/*.elm`, { suppressErrors: true }),
+        readFile(`${element.uri}/elm.json`, {
+          encoding: "utf-8",
+        }),
+      ]);
+      const exposedModules = this.modulesToFilenames(
+        JSON.parse(elmJsonString),
+        element.uri,
+      );
+      return elmFiles.map((matchingPath) => ({
+        maintainerAndPackageName: element.maintainerAndPackageName,
+        path: matchingPath,
+        writeable: element.writeable,
+        isExposed: exposedModules.includes(matchingPath),
+      }));
+    }
+  }
+
+  private modulesToFilenames(
+    elmJson: unknown,
+    pathToPackage: string,
+  ): string[] {
+    if (!elmJson || !Object.hasOwnProperty.call(elmJson, "exposed-modules")) {
+      return [];
+    }
+    const x = (elmJson as {
+      "exposed-modules": Record<string, string | string[]>;
+    })["exposed-modules"];
+
+    const result: string[] = [];
+
+    for (const key in x) {
+      if (Object.hasOwnProperty.call(x, key)) {
+        const element = x[key];
+        if (typeof element === "string") {
+          result.push(
+            pathToPackage
+              .concat("/src/")
+              .concat(element.replace(".", "/").concat(".elm")),
+          );
+        } else {
+          result.push(
+            ...element.map((element) =>
+              pathToPackage
+                .concat("/src/")
+                .concat(element.replace(".", "/").concat(".elm")),
+            ),
+          );
+        }
+      }
+    }
+    return result;
   }
 
   private packageOrPackagesFolder(elmVersion: string | undefined): string {
     return elmVersion === "0.19.0" ? "package" : "packages";
   }
 
-  private findElmHome() {
+  private findElmHome(): string {
     const elmHomeVar = process.env.ELM_HOME;
 
     if (elmHomeVar) {
@@ -276,27 +343,24 @@ export class ElmWorkspace implements IElmWorkspace {
     filePath: IFolder,
     callback: () => void,
   ): Promise<void> {
-    return new Promise(async (resolve, reject) => {
-      try {
-        this.connection.console.info(`Adding ${filePath.path.toString()}`);
-        const fileContent: string = await readFile(filePath.path.toString(), {
-          encoding: "utf-8",
-        });
+    try {
+      this.connection.console.info(`Adding ${filePath.path.toString()}`);
+      const fileContent: string = await readFile(filePath.path.toString(), {
+        encoding: "utf-8",
+      });
 
-        const tree: Tree | undefined = this.parser.parse(fileContent);
-        this.forest.setTree(
-          URI.file(filePath.path).toString(),
-          filePath.writeable,
-          true,
-          tree,
-          filePath.maintainerAndPackageName,
-        );
-        callback();
-        resolve();
-      } catch (error) {
-        this.connection.console.error(error.stack);
-        reject(error);
-      }
-    });
+      const tree: Tree | undefined = this.parser.parse(fileContent);
+      this.forest.setTree(
+        URI.file(filePath.path).toString(),
+        filePath.writeable,
+        true,
+        tree,
+        filePath.isExposed,
+        filePath.maintainerAndPackageName,
+      );
+      callback();
+    } catch (error) {
+      this.connection.console.error(error.stack);
+    }
   }
 }
diff --git a/src/forest.ts b/src/forest.ts
index 9a6c605..d20ed2c 100644
--- a/src/forest.ts
+++ b/src/forest.ts
@@ -7,8 +7,9 @@ export interface ITreeContainer {
   referenced: boolean;
   moduleName?: string;
   maintainerAndPackageName?: string;
-  exposing?: IExposing[];
+  exposing?: IExposing[]; // This file exposes
   tree: Tree;
+  isExposed: boolean; // Is this file exposed by the elm.json
 }
 
 export interface IForest {
@@ -23,6 +24,7 @@ export interface IForest {
     writeable: boolean,
     referenced: boolean,
     tree: Tree,
+    isExposed: boolean,
     packageName?: string,
   ): void;
   removeTree(uri: string): void;
@@ -39,7 +41,7 @@ export class Forest implements IForest {
 
   public getExposingByModuleName(moduleName: string): IExposing[] | undefined {
     const result = this.treeIndex.find(
-      (tree) => tree.moduleName === moduleName,
+      (tree) => tree.moduleName === moduleName && tree.isExposed,
     );
     return result && result.exposing;
   }
@@ -53,7 +55,9 @@ export class Forest implements IForest {
   }
 
   public getByModuleName(moduleName: string): ITreeContainer | undefined {
-    return this.treeIndex.find((tree) => tree.moduleName === moduleName);
+    return this.treeIndex.find(
+      (tree) => tree.moduleName === moduleName && tree.isExposed,
+    );
   }
 
   public getByUri(uri: string): ITreeContainer | undefined {
@@ -65,6 +69,7 @@ export class Forest implements IForest {
     writeable: boolean,
     referenced: boolean,
     tree: Tree,
+    isExposed: boolean,
     maintainerAndPackageName?: string,
   ): void {
     const moduleResult = TreeUtils.getModuleNameAndExposing(tree);
@@ -84,6 +89,7 @@ export class Forest implements IForest {
       tree,
       uri,
       writeable,
+      isExposed,
     };
 
     if (existingTree === -1) {
diff --git a/src/imports.ts b/src/imports.ts
index cb242e0..843ae0b 100644
--- a/src/imports.ts
+++ b/src/imports.ts
@@ -285,20 +285,16 @@ import Platform.Sub as Sub exposing ( Sub )
     uri: string,
     maintainerAndPackageName?: string,
   ): IImport[] {
-    const result: IImport[] = [];
-
-    exposed.forEach((element) => {
-      result.push({
+    return exposed.map((element: IExposing) => {
+      return {
         alias: element.name,
         fromModuleName: moduleName,
         fromUri: uri,
         maintainerAndPackageName,
         node: element.syntaxNode,
         type: element.type,
-      });
+      };
     });
-
-    return result;
   }
 
   private exposedNodesToImports(
diff --git a/src/providers/astProvider.ts b/src/providers/astProvider.ts
index 6befab1..b0034f2 100644
--- a/src/providers/astProvider.ts
+++ b/src/providers/astProvider.ts
@@ -52,7 +52,7 @@ export class ASTProvider {
       tree = this.parser.parse(changeEvent.text);
     }
     if (tree) {
-      forest.setTree(document.uri, true, true, tree);
+      forest.setTree(document.uri, true, true, tree, true);
 
       // Figure out if we have files importing our changed file - update them
       const urisToRefresh = [];


Test patch:
diff --git a/src/test/utils/mockElmWorkspace.ts b/src/test/utils/mockElmWorkspace.ts
index 0fdfbc0..e656b66 100644
--- a/src/test/utils/mockElmWorkspace.ts
+++ b/src/test/utils/mockElmWorkspace.ts
@@ -14,7 +14,7 @@ export class MockElmWorkspace implements IElmWorkspace {
   constructor(source: string, parser: Parser) {
     const tree = parser.parse(source);
 
-    this.forest.setTree(mockUri, true, true, tree);
+    this.forest.setTree(mockUri, true, true, tree, true);
     this.imports = new Imports(parser);
     this.imports.updateImports(mockUri, tree, this.forest);
   }