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

Issue:
Run `elm make` on server initialization to download dependencies
I think without passing an entry file path, it will just download the dependencies and quit, so it shouldn't affect startup time very much. 


Pull request:
Run elm make on init and clean up dependency solver
Better alternative to #408. Closes #344.

Patch:
diff --git a/src/elmPackageCache.ts b/src/elmPackageCache.ts
new file mode 100644
index 0000000..a24ed66
--- /dev/null
+++ b/src/elmPackageCache.ts
@@ -0,0 +1,113 @@
+import { readdir } from "fs";
+import * as utils from "./util/elmUtils";
+import { ElmJson } from "./elmWorkspace";
+import { promisify } from "util";
+import { IConstraint, IVersion } from "./util/elmUtils";
+
+const readDir = promisify(readdir);
+
+export interface IPackage {
+  dependencies: Map<string, IConstraint>;
+  version: IVersion;
+}
+
+export interface IElmPackageCache {
+  getVersions(packageName: string): Promise<IVersion[]>;
+  getDependencies(
+    packageName: string,
+    version: IVersion,
+  ): Promise<Map<string, IConstraint>>;
+}
+
+export class ElmPackageCache implements IElmPackageCache {
+  private versionsCache = new Map<string, IVersion[]>();
+  private dependenciesCache = new Map<string, Map<string, IConstraint>>();
+
+  constructor(
+    private packagesRoot: string,
+    private loadElmJson: (elmJsonPath: string) => Promise<ElmJson>,
+  ) {}
+
+  public async getVersions(packageName: string): Promise<IVersion[]> {
+    const cached = this.versionsCache.get(packageName);
+
+    if (cached) {
+      return cached;
+    }
+
+    const versions = await this.getVersionsFromFileSystem(packageName);
+
+    this.versionsCache.set(packageName, versions);
+
+    return versions;
+  }
+
+  public async getDependencies(
+    packageName: string,
+    version: IVersion,
+  ): Promise<Map<string, IConstraint>> {
+    const cacheKey = `${packageName}@${version.string}`;
+    const cached = this.dependenciesCache.get(cacheKey);
+
+    if (cached) {
+      return cached;
+    }
+
+    const dependencies = await this.getDependenciesFromFileSystem(
+      packageName,
+      version,
+    );
+
+    this.dependenciesCache.set(cacheKey, dependencies);
+
+    return dependencies;
+  }
+
+  private async getVersionsFromFileSystem(
+    packageName: string,
+  ): Promise<IVersion[]> {
+    const maintainer = packageName.substring(0, packageName.indexOf("/"));
+    const name = packageName.substring(
+      packageName.indexOf("/") + 1,
+      packageName.length,
+    );
+
+    const pathToPackage = `${this.packagesRoot}${maintainer}/${name}/`;
+    const folders = await readDir(pathToPackage, "utf8");
+
+    const allVersions: IVersion[] = [];
+
+    for (const folderName of folders) {
+      const version = utils.parseVersion(folderName);
+
+      if (
+        Number.isInteger(version.major) &&
+        Number.isInteger(version.minor) &&
+        Number.isInteger(version.patch)
+      ) {
+        allVersions.push(version);
+      }
+    }
+
+    return allVersions;
+  }
+
+  private async getDependenciesFromFileSystem(
+    packageName: string,
+    version: IVersion,
+  ): Promise<Map<string, IConstraint>> {
+    const elmJsonPath = `${this.packagesRoot}${packageName}/${version.string}/elm.json`;
+    const elmJson = await this.loadElmJson(elmJsonPath);
+
+    return this.parseDependencies(elmJson);
+  }
+
+  private parseDependencies(elmJson: ElmJson): Map<string, IConstraint> {
+    return new Map<string, IConstraint>(
+      Object.entries(elmJson.dependencies).map(([name, constraint]) => [
+        name,
+        utils.parseConstraint(constraint),
+      ]),
+    );
+  }
+}
diff --git a/src/elmWorkspace.ts b/src/elmWorkspace.ts
index 281be88..16692e9 100644
--- a/src/elmWorkspace.ts
+++ b/src/elmWorkspace.ts
@@ -1,4 +1,4 @@
-import fs, { readdirSync } from "fs";
+import fs from "fs";
 import globby from "globby";
 import os from "os";
 import { container } from "tsyringe";
@@ -7,8 +7,10 @@ import { Connection } from "vscode-languageserver";
 import { URI } from "vscode-uri";
 import Parser, { Tree } from "web-tree-sitter";
 import { ICancellationToken } from "./cancellation";
+import { ElmPackageCache, IElmPackageCache } from "./elmPackageCache";
 import { Forest, IForest, ITreeContainer } from "./forest";
 import * as utils from "./util/elmUtils";
+import { IVersion } from "./util/elmUtils";
 import * as path from "./util/path";
 import { normalizeUri } from "./util/path";
 import {
@@ -33,7 +35,7 @@ interface IElmFile {
   project: ElmProject;
 }
 
-type ElmJson = IElmApplicationJson | IElmPackageJson;
+export type ElmJson = IElmApplicationJson | IElmPackageJson;
 
 interface IElmApplicationJson {
   type: "application";
@@ -124,87 +126,8 @@ interface IElmPackage extends IElmProject {
   exposedModules: Set<string>;
 }
 
-export interface IVersion {
-  major: number;
-  minor: number;
-  patch: number;
-  string: string;
-}
-
-export interface IConstraint {
-  upper: IVersion;
-  lower: IVersion;
-  upperOperator: "<" | "<=";
-  lowerOperator: "<" | "<=";
-}
-
-export interface IPackage {
-  dependencies: Map<string, IConstraint>;
-  version: IVersion;
-}
-
-export interface IElmPackageCache {
-  get(packageName: string): IPackage[];
-}
-
-export class ElmPackageCache implements IElmPackageCache {
-  private cache = new Map<string, IPackage[]>();
-
-  constructor(
-    private packagesRoot: string,
-    private loadElmJson: (elmJsonPath: string) => ElmJson,
-  ) {}
-
-  public get(packageName: string): IPackage[] {
-    const cached = this.cache.get(packageName);
-
-    if (cached) {
-      return cached;
-    }
-
-    const maintainer = packageName.substring(0, packageName.indexOf("/"));
-    const name = packageName.substring(
-      packageName.indexOf("/") + 1,
-      packageName.length,
-    );
-
-    const pathToPackage = `${this.packagesRoot}${maintainer}/${name}/`;
-    const readDir = readdirSync(pathToPackage, "utf8");
-
-    const allVersions: IPackage[] = [];
-
-    for (const folderName of readDir) {
-      const version = utils.parseVersion(folderName);
-
-      if (
-        Number.isInteger(version.major) &&
-        Number.isInteger(version.minor) &&
-        Number.isInteger(version.patch)
-      ) {
-        const elmJsonPath = path.join(pathToPackage, folderName, "elm.json");
-        const elmJson = this.loadElmJson(elmJsonPath);
-
-        allVersions.push({
-          version,
-          dependencies: new Map(
-            Object.entries(elmJson.dependencies).map(([name, constraint]) => [
-              name,
-              utils.parseContraint(constraint),
-            ]),
-          ),
-        });
-      }
-    }
-
-    this.cache.set(packageName, allVersions);
-
-    return allVersions;
-  }
-}
-
 export interface IProgramHost {
   readFile(uri: string): Promise<string>;
-  readFileSync(uri: string): string;
   readDirectory(uri: string): Promise<string[]>;
   watchFile(uri: string, callback: () => void): void;
 }
@@ -413,6 +336,19 @@ export class ElmWorkspace implements IElmWorkspace {
         elmVersion,
       )}/`;
 
+      try {
+        // Run `elm make` to download dependencies
+        await utils.execCmd(
+          clientSettings.elmPath,
+          "elm",
+          { cmdArguments: ["make"] },
+          this.rootPath.fsPath,
+          this.connection,
+        );
+      } catch {
+        // Compile failed
+      }
+
       this.elmPackageCache = new ElmPackageCache(
         this.packagesRoot,
         this.loadElmJson.bind(this),
@@ -457,7 +393,7 @@ export class ElmWorkspace implements IElmWorkspace {
   }
 
   private async loadRootProject(elmJsonPath: string): Promise<ElmProject> {
-    const elmJson = this.loadElmJson(elmJsonPath);
+    const elmJson = await this.loadElmJson(elmJsonPath);
 
     if (elmJson.type === "application") {
       const allDependencies = new Map(
@@ -492,15 +428,18 @@ export class ElmWorkspace implements IElmWorkspace {
       const deps = new Map(
         Object.entries(
           Object.assign(elmJson.dependencies, elmJson["test-dependencies"]),
-        ).map(([dep, version]) => [dep, utils.parseContraint(version)]),
+        ).map(([dep, version]) => [dep, utils.parseConstraint(version)]),
       );
 
-      const solvedVersions = utils.solveDependencies(
+      const solvedVersions = await utils.solveDependencies(
         this.elmPackageCache,
         deps,
       );
 
       if (!solvedVersions) {
+        this.connection.window.showErrorMessage(
+          "There is a problem with elm.json. Could not solve dependencies with the given constraints. Try running `elm make` to install missing dependencies.",
+        );
         throw new Error("Unsolvable package constraints");
       }
 
@@ -551,7 +490,7 @@ export class ElmWorkspace implements IElmWorkspace {
     const pathToPackageWithVersion = `${this.packagesRoot}${maintainer}/${name}/${version.string}`;
 
     const elmJsonPath = path.join(pathToPackageWithVersion, "elm.json");
-    const elmJson = this.loadElmJson(elmJsonPath);
+    const elmJson = await this.loadElmJson(elmJsonPath);
 
     if (elmJson.type === "package") {
       const resolvedPackage = {
@@ -583,7 +522,7 @@ export class ElmWorkspace implements IElmWorkspace {
     },
     packageVersions: ReadonlyMap<string, IVersion>,
   ): Promise<Map<string, IElmPackage>> {
-    const dependencyMap = new Map();
+    const dependencyMap = new Map<string, IElmPackage>();
     for (const dep in deps) {
       dependencyMap.set(dep, await this.loadPackage(dep, packageVersions));
     }
@@ -738,8 +677,8 @@ export class ElmWorkspace implements IElmWorkspace {
     }
   }
 
-  private loadElmJson(elmJsonPath: string): ElmJson {
-    return JSON.parse(this.host.readFileSync(elmJsonPath)) as ElmJson;
+  private async loadElmJson(elmJsonPath: string): Promise<ElmJson> {
+    return JSON.parse(await this.host.readFile(elmJsonPath)) as ElmJson;
   }
 
   private createProgramHost(): IProgramHost {
@@ -748,10 +687,6 @@ export class ElmWorkspace implements IElmWorkspace {
         readFile(uri, {
           encoding: "utf-8",
         }),
-      readFileSync: (uri): string =>
-        fs.readFileSync(uri, {
-          encoding: "utf-8",
-        }),
       readDirectory: (uri: string): Promise<string[]> =>
         // Cleanup the path on windows, as globby does not like backslashes
         globby(`${uri.replace(/\\/g, "/")}/**/*.elm`, {
diff --git a/src/util/elmUtils.ts b/src/util/elmUtils.ts
index 88b1b4b..dc8067d 100644
--- a/src/util/elmUtils.ts
+++ b/src/util/elmUtils.ts
@@ -2,7 +2,7 @@ import execa, { ExecaReturnValue } from "execa";
 import * as path from "path";
 import { Connection, CompletionItemKind } from "vscode-languageserver";
 import { URI } from "vscode-uri";
-import { IElmPackageCache, IConstraint, IVersion } from "../elmWorkspace";
+import { IElmPackageCache } from "../elmPackageCache";
 import { IClientSettings } from "./settings";
 
 export const isWindows = process.platform === "win32";
@@ -117,11 +117,11 @@ type SolverResult =
     }
   | undefined;
 
-export function solveDependencies(
+export async function solveDependencies(
   packageCache: IElmPackageCache,
   deps: ReadonlyMap<string, IConstraint>,
-): ReadonlyMap<string, IVersion> | undefined {
-  const result = solveDependenciesWorker(
+): Promise<ReadonlyMap<string, IVersion> | undefined> {
+  const result = await solveDependenciesWorker(
     packageCache,
     deps,
     new Map<string, IVersion>(),
@@ -132,11 +132,11 @@ export function solveDependencies(
   }
 }
 
-function solveDependenciesWorker(
+async function solveDependenciesWorker(
   packageCache: IElmPackageCache,
   deps: ReadonlyMap<string, IConstraint>,
   solutions: ReadonlyMap<string, IVersion>,
-): SolverResult {
+): Promise<SolverResult> {
   function pickDep(): [
     { name: string; constraint: IConstraint } | undefined,
     Map<string, IConstraint>,
@@ -190,33 +190,32 @@ function solveDependenciesWorker(
   }
 
   // Find versions that satisfy the constraint
-  let candidates = packageCache
-    .get(dep.name)
-    .filter(({ version }) => versionSatifiesConstraint(version, dep.constraint))
-    .sort((a, b) =>
-      a.version < b.version ? -1 : a.version > b.version ? 1 : 0,
-    )
+  let candidates = (await packageCache.getVersions(dep.name))
+    .filter((version) => versionSatifiesConstraint(version, dep.constraint))
+    .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))
     .reverse();
 
   const solvedVersion = solutions.get(dep.name);
 
   if (solvedVersion) {
-    candidates = candidates.filter(
-      (a) => a.version.string === solvedVersion.string,
-    );
+    candidates = candidates.filter((a) => a.string === solvedVersion.string);
   }
 
   for (const candidate of candidates) {
-    const tentativeDeps = combineDeps(restDeps, candidate.dependencies);
+    const candidateDeps = await packageCache.getDependencies(
+      dep.name,
+      candidate,
+    );
+    const tentativeDeps = combineDeps(restDeps, candidateDeps);
 
     if (!tentativeDeps) {
       continue;
     }
 
     const tentativeSolutions = new Map(solutions);
-    tentativeSolutions.set(dep.name, candidate.version);
+    tentativeSolutions.set(dep.name, candidate);
 
-    const result = solveDependenciesWorker(
+    const result = await solveDependenciesWorker(
       packageCache,
       tentativeDeps,
       tentativeSolutions,
@@ -232,6 +231,20 @@ function solveDependenciesWorker(
   }
 }
 
+export interface IVersion {
+  major: number;
+  minor: number;
+  patch: number;
+  string: string;
+}
+
+export interface IConstraint {
+  upper: IVersion;
+  lower: IVersion;
+  upperOperator: "<" | "<=";
+  lowerOperator: "<" | "<=";
+}
+
 export function parseVersion(version: string): IVersion {
   const [major, minor, patch] = version.split(".");
 
@@ -243,7 +256,7 @@ export function parseVersion(version: string): IVersion {
   };
 }
 
-export function parseContraint(contraint: string): IConstraint {
+export function parseConstraint(contraint: string): IConstraint {
   const regex = /^(\d+\.\d+\.\d+) (<|<=) v (<|<=) (\d+\.\d+\.\d+)$/gm;
 
   const m = regex.exec(contraint);


Test patch:
diff --git a/test/constraint.test.ts b/test/constraint.test.ts
index fb71831..f34d3d3 100644
--- a/test/constraint.test.ts
+++ b/test/constraint.test.ts
@@ -1,11 +1,10 @@
-import { assert } from "console";
-import { IConstraint, IVersion } from "../src/elmWorkspace";
 import {
   constraintIntersect,
-  parseContraint,
+  IConstraint,
+  IVersion,
+  parseConstraint,
   versionSatifiesConstraint,
 } from "../src/util/elmUtils";
-import { Utils } from "../src/util/utils";
 
 describe("constraint test", () => {
   function v(
@@ -24,7 +23,7 @@ describe("constraint test", () => {
     };
   }
 
-  const c = parseContraint;
+  const c = parseConstraint;
 
   it("can determine whether a version satifies a half-open constraint", () => {
     const c: IConstraint = {
diff --git a/test/diagnostics.ts b/test/diagnostics.ts
index 6b57c78..cdb288b 100644
--- a/test/diagnostics.ts
+++ b/test/diagnostics.ts
@@ -1,16 +1,15 @@
 import "reflect-metadata";
 import { container } from "tsyringe";
 import { URI } from "vscode-uri";
-import { ElmWorkspace, IProgramHost } from "../src/elmWorkspace";
+import { ElmWorkspace } from "../src/elmWorkspace";
 import * as path from "path";
 import { Settings } from "../src/util/settings";
 import Parser from "web-tree-sitter";
 import { spawnSync } from "child_process";
-import { readFile, readFileSync } from "fs";
+import { readFileSync } from "fs";
 import { Diagnostic } from "../src/util/types/diagnostics";
 import { performance } from "perf_hooks";
-import globby from "globby";
-import { promisify } from "util";
+import { createProgramHost } from "./utils/sourceTreeParser";
 
 container.register("Connection", {
   useValue: {
@@ -47,27 +46,6 @@ async function initParser(): Promise<void> {
   container.resolve<Parser>("Parser").setLanguage(language);
 }
 
-export function createProgramHost(): IProgramHost {
-  return {
-    readFile: (uri): Promise<string> =>
-      promisify(readFile)(uri, {
-        encoding: "utf-8",
-      }),
-    readFileSync: (uri): string =>
-      readFileSync(uri, {
-        encoding: "utf-8",
-      }),
-    readDirectory: (uri: string): Promise<string[]> =>
-      // Cleanup the path on windows, as globby does not like backslashes
-      globby(`${uri.replace(/\\/g, "/")}/**/*.elm`, {
-        suppressErrors: true,
-      }),
-    watchFile: (): void => {
-      return;
-    },
-  };
-}
-
 const failed: string[] = [];
 const diagnosticTimes = new Map<string, number>();
 const parsingErrors = new Set();
diff --git a/test/performance.ts b/test/performance.ts
index f1ed347..38587e5 100644
--- a/test/performance.ts
+++ b/test/performance.ts
@@ -25,7 +25,7 @@ import {
   ThrottledCancellationToken,
 } from "../src/cancellation";
 import { randomBytes } from "crypto";
-import { createProgramHost } from "./diagnostics";
+import { createProgramHost } from "./utils/sourceTreeParser";
 
 container.register("Connection", {
   useValue: {
diff --git a/test/solver.test.ts b/test/solver.test.ts
index b5e6a86..8c588fe 100644
--- a/test/solver.test.ts
+++ b/test/solver.test.ts
@@ -1,10 +1,10 @@
+import { IElmPackageCache, IPackage } from "../src/elmPackageCache";
 import {
   IConstraint,
-  IElmPackageCache,
-  IPackage,
   IVersion,
-} from "../src/elmWorkspace";
-import { parseContraint, solveDependencies } from "../src/util/elmUtils";
+  parseConstraint,
+  solveDependencies,
+} from "../src/util/elmUtils";
 
 describe("module resolution solver", () => {
   function v(
@@ -23,9 +23,9 @@ describe("module resolution solver", () => {
     };
   }
 
-  const c = parseContraint;
+  const c = parseConstraint;
 
-  const packageCache: IElmPackageCache = {
+  const packageCache = {
     get: (packageName: string): IPackage[] => {
       switch (packageName) {
         case "B":
@@ -72,14 +72,27 @@ describe("module resolution solver", () => {
     },
   };
 
-  function test(
+  async function test(
     deps: [string, IConstraint][],
     expected: [string, IVersion][] | undefined,
     cache = packageCache,
   ) {
-    expect(solveDependencies(cache, new Map(deps))).toEqual(
-      expected ? new Map(expected) : undefined,
-    );
+    expect(
+      await solveDependencies(
+        {
+          getVersions: (pacakgeName) =>
+            Promise.resolve(cache.get(pacakgeName).map((p) => p.version)),
+          getDependencies: (packageName, version) =>
+            Promise.resolve(
+              cache
+                .get(packageName)
+                .find((p) => p.version.string === version.string)
+                ?.dependencies ?? new Map(),
+            ),
+        },
+        new Map(deps),
+      ),
+    ).toEqual(expected ? new Map(expected) : undefined);
   }
 
   it("handles empty input", () => {
diff --git a/test/utils/sourceTreeParser.ts b/test/utils/sourceTreeParser.ts
index 2d93704..50eca5d 100644
--- a/test/utils/sourceTreeParser.ts
+++ b/test/utils/sourceTreeParser.ts
@@ -1,7 +1,14 @@
+import { readFile } from "fs";
+import globby from "globby";
 import { container } from "tsyringe";
+import { promisify } from "util";
 import { URI } from "vscode-uri";
 import Parser from "web-tree-sitter";
-import { ElmWorkspace, IElmWorkspace } from "../../src/elmWorkspace";
+import {
+  ElmWorkspace,
+  IElmWorkspace,
+  IProgramHost,
+} from "../../src/elmWorkspace";
 import * as path from "../../src/util/path";
 
 export const baseUri = path.join(__dirname, "../sources/src/");
@@ -53,7 +60,6 @@ export class SourceTreeParser {
     const program = new ElmWorkspace(URI.file(baseUri), {
       readFile: (uri: string): Promise<string> =>
         Promise.resolve(readFile(uri)),
-      readFileSync: readFile,
       readDirectory: (uri: string): Promise<string[]> => {
         return Promise.resolve(
           path.normalizeUri(uri) ===
@@ -74,3 +80,20 @@ export class SourceTreeParser {
     return program;
   }
 }
+
+export function createProgramHost(): IProgramHost {
+  return {
+    readFile: (uri): Promise<string> =>
+      promisify(readFile)(uri, {
+        encoding: "utf-8",
+      }),
+    readDirectory: (uri: string): Promise<string[]> =>
+      // Cleanup the path on windows, as globby does not like backslashes
+      globby(`${uri.replace(/\\/g, "/")}/**/*.elm`, {
+        suppressErrors: true,
+      }),
+    watchFile: (): void => {
+      return;
+    },
+  };
+}