Repository: elm-tooling/elm-language-server
Pull request #320 (2020-09-26 07:50:14)

Parse files incrementally and use more type caching
Work done to switch the tree-sitter parsing to incremental, so we can cache types between changes. I actual did most of this before I wrote inference. This part will need the most testing, but it seems to be working pretty good for me. 

I also added type inference record completions when all other records completions fail. This should be safe since it is only used as a backup for now (still a little slower than current methods), and won't break anything if it fails. This leads to some pretty cools results and it worked pretty well in my brief testing.

Things like:
``` elm
test: List Data -> Data
test data =
    data |> List.filter (\d -> d.{-caret-})
```
now work great!
I was kinda surprised at how much just worked.

I expect that this PR will probably need some cleanup and good testing before we want to merge. 

Razzeee: As I want to build a version and post it to slack for extended testing, this probably won't hurt too much?
jmbockhorst: I do need to clean up the cache invalidating a little tonight. But then it should be good to include with the build to test

Patch:
diff --git a/src/capabilityCalculator.ts b/src/capabilityCalculator.ts
index 10caf5c..71e4289 100644
--- a/src/capabilityCalculator.ts
+++ b/src/capabilityCalculator.ts
@@ -41,7 +41,7 @@ export class CapabilityCalculator {
         prepareProvider: true,
       },
       selectionRangeProvider: true,
-      textDocumentSync: TextDocumentSyncKind.Full,
+      textDocumentSync: TextDocumentSyncKind.Incremental,
       workspaceSymbolProvider: true,
     };
   }
diff --git a/src/providers/astProvider.ts b/src/providers/astProvider.ts
index 5d57e7c..ec22405 100644
--- a/src/providers/astProvider.ts
+++ b/src/providers/astProvider.ts
@@ -9,15 +9,18 @@ import {
   Emitter,
 } from "vscode-languageserver";
 import { URI } from "vscode-uri";
-import Parser, { Tree, SyntaxNode } from "web-tree-sitter";
+import Parser, { Tree, Edit, Point } from "web-tree-sitter";
 import { IElmWorkspace } from "../elmWorkspace";
-import { IDocumentEvents } from "../util/documentEvents";
 import { ElmWorkspaceMatcher } from "../util/elmWorkspaceMatcher";
+import { Position, Range } from "vscode-languageserver-textdocument";
 import { FileEventsHandler } from "./handlers/fileEventsHandler";
+import { TextDocumentEvents } from "../util/textDocumentEvents";
+import { TreeUtils } from "../util/treeUtils";
 
 export class ASTProvider {
   private connection: IConnection;
   private parser: Parser;
+  private documentEvents: TextDocumentEvents;
 
   private treeChangeEvent = new Emitter<{ uri: string; tree: Tree }>();
   readonly onTreeChange: Event<{ uri: string; tree: Tree }> = this
@@ -26,18 +29,20 @@ export class ASTProvider {
   constructor() {
     this.parser = container.resolve("Parser");
     this.connection = container.resolve<IConnection>("Connection");
-    const documentEvents = container.resolve<IDocumentEvents>("DocumentEvents");
+    this.documentEvents = container.resolve<TextDocumentEvents>(
+      TextDocumentEvents,
+    );
 
     new FileEventsHandler();
 
-    documentEvents.on(
+    this.documentEvents.on(
       "change",
       new ElmWorkspaceMatcher((params: DidChangeTextDocumentParams) =>
         URI.parse(params.textDocument.uri),
       ).handlerForWorkspace(this.handleChangeTextDocument),
     );
 
-    documentEvents.on(
+    this.documentEvents.on(
       "open",
       new ElmWorkspaceMatcher((params: DidOpenTextDocumentParams) =>
         URI.parse(params.textDocument.uri),
@@ -57,22 +62,50 @@ export class ASTProvider {
     const document: VersionedTextDocumentIdentifier = params.textDocument;
 
     let tree: Tree | undefined = forest.getTree(document.uri);
-    if (tree === undefined) {
-      const fileContent: string = readFileSync(
-        URI.parse(document.uri).fsPath,
-        "utf8",
-      );
-      tree = this.parser.parse(fileContent);
-    }
 
     if ("contentChanges" in params) {
-      for (const changeEvent of params.contentChanges) {
-        tree = this.parser.parse(changeEvent.text);
+      for (const change of params.contentChanges) {
+        if ("range" in change) {
+          tree?.edit(this.getEditFromChange(change, tree.rootNode.text));
+        }
       }
-    } else {
-      tree = this.parser.parse(params.textDocument.text);
     }
 
+    const newText =
+      this.documentEvents.get(params.textDocument.uri)?.getText() ??
+      readFileSync(URI.parse(document.uri).fsPath, "utf8");
+
+    const newTree = this.parser.parse(newText, tree);
+
+    tree
+      ?.getChangedRanges(newTree)
+      .map((range) => [
+        tree?.rootNode.descendantForPosition(range.startPosition),
+        tree?.rootNode.descendantForPosition(range.endPosition),
+      ])
+      .map(([startNode, endNode]) => [
+        startNode
+          ? TreeUtils.findParentOfType("value_declaration", startNode)
+          : undefined,
+        endNode
+          ? TreeUtils.findParentOfType("value_declaration", endNode)
+          : undefined,
+      ])
+      .forEach(([startNode, endNode]) => {
+        if (
+          startNode &&
+          endNode &&
+          startNode.id === endNode.id &&
+          TreeUtils.getTypeAnnotation(startNode)
+        ) {
+          elmWorkspace.getTypeCache().invalidateValueDeclaration(startNode);
+        } else {
+          elmWorkspace.getTypeCache().invalidateProject();
+        }
+      });
+
+    tree = newTree;
+
     if (tree) {
       forest.setTree(document.uri, true, true, tree, true);
 
@@ -97,4 +130,62 @@ export class ASTProvider {
       this.treeChangeEvent.fire({ uri: document.uri, tree });
     }
   };
+
+  private getEditFromChange(
+    change: { text: string; range: Range },
+    text: string,
+  ): Edit {
+    const [startIndex, endIndex] = this.getIndexesFromRange(change.range, text);
+
+    return {
+      startIndex,
+      oldEndIndex: endIndex,
+      newEndIndex: startIndex + change.text.length,
+      startPosition: this.toTSPoint(change.range.start),
+      oldEndPosition: this.toTSPoint(change.range.end),
+      newEndPosition: this.toTSPoint(
+        this.addPositions(change.range.start, this.textToPosition(change.text)),
+      ),
+    };
+  }
+
+  private textToPosition(text: string): Position {
+    const lines = text.split(/\r\n|\r|\n/);
+
+    return {
+      line: lines.length - 1,
+      character: lines[lines.length - 1].length,
+    };
+  }
+
+  private getIndexesFromRange(range: Range, text: string): [number, number] {
+    let startIndex = range.start.character;
+    let endIndex = range.end.character;
+
+    const regex = new RegExp(/\r\n|\r|\n/);
+    const eolResult = regex.exec(text);
+
+    const lines = text.split(regex);
+    const eol = eolResult && eolResult.length > 0 ? eolResult[0] : "";
+
+    for (let i = 0; i < range.end.line; i++) {
+      if (i < range.start.line) {
+        startIndex += lines[i].length + eol.length;
+      }
+      endIndex += lines[i].length + eol.length;
+    }
+
+    return [startIndex, endIndex];
+  }
+
+  private addPositions(pos1: Position, pos2: Position): Position {
+    return {
+      line: pos1.line + pos2.line,
+      character: pos1.character + pos2.character,
+    };
+  }
+
+  private toTSPoint(position: Position): Point {
+    return { row: position.line, column: position.character };
+  }
 }
diff --git a/src/providers/completionProvider.ts b/src/providers/completionProvider.ts
index 2cd495e..097cbe3 100644
--- a/src/providers/completionProvider.ts
+++ b/src/providers/completionProvider.ts
@@ -1,3 +1,5 @@
+import { findType } from "../util/types/typeInference";
+import { TypeRenderer } from "../util/types/typeRenderer";
 import { container } from "tsyringe";
 import {
   CompletionItem,
@@ -326,16 +328,29 @@ export class CompletionProvider {
           targetWord,
         );
 
-        return moduleCompletions.length > 0
-          ? moduleCompletions
-          : this.getRecordCompletions(
-              targetNode,
-              tree,
-              replaceRange,
-              elmWorkspace.getImports(),
-              params.textDocument.uri,
-              forest,
-            );
+        if (moduleCompletions.length > 0) {
+          return moduleCompletions;
+        }
+
+        const recordCompletions = this.getRecordCompletions(
+          targetNode,
+          tree,
+          replaceRange,
+          elmWorkspace.getImports(),
+          params.textDocument.uri,
+          forest,
+        );
+
+        if (recordCompletions.length > 0) {
+          return recordCompletions;
+        }
+
+        return this.getRecordCompletionsUsingInference(
+          targetNode,
+          replaceRange,
+          params.textDocument.uri,
+          elmWorkspace,
+        );
       }
 
       completions.push(
@@ -786,6 +801,32 @@ export class CompletionProvider {
     return result;
   }
 
+  private getRecordCompletionsUsingInference(
+    targetNode: SyntaxNode,
+    replaceRange: Range,
+    uri: string,
+    elmWorkspace: IElmWorkspace,
+  ): CompletionItem[] {
+    const result = [];
+    const foundType = findType(targetNode, uri, elmWorkspace);
+
+    if (foundType.nodeType === "Record") {
+      for (const field in foundType.fields) {
+        const hint = HintHelper.createHintForTypeAliasReference(
+          TypeRenderer.typeToString(foundType.fields[field]),
+          field,
+          foundType.alias?.name ?? "",
+        );
+
+        result.push(
+          this.createFieldOrParameterCompletion(hint, field, replaceRange),
+        );
+      }
+    }
+
+    return result;
+  }
+
   private createFunctionCompletion(
     options: ICompletionOptions,
   ): CompletionItem {
diff --git a/src/util/textDocumentEvents.ts b/src/util/textDocumentEvents.ts
index 97ee805..f2188b1 100644
--- a/src/util/textDocumentEvents.ts
+++ b/src/util/textDocumentEvents.ts
@@ -56,7 +56,7 @@ export class TextDocumentEvents extends EventEmitter {
 
       this._documents[td.uri] = document;
 
-      this.emit("change", Object.freeze({ document }));
+      this.emit("change", Object.freeze({ document, ...params }));
     });
 
     events.on("save", (params: DidSaveTextDocumentParams) => {
diff --git a/src/util/types/syntaxNodeMap.ts b/src/util/types/syntaxNodeMap.ts
index bd73b32..681742e 100644
--- a/src/util/types/syntaxNodeMap.ts
+++ b/src/util/types/syntaxNodeMap.ts
@@ -39,4 +39,12 @@ export class SyntaxNodeMap<K extends SyntaxNode, V> {
       callback(val, ({ id: key } as unknown) as K),
     );
   }
+
+  public clear(): void {
+    this.map.clear();
+  }
+
+  public delete(key: K): void {
+    this.map.delete(key.id);
+  }
 }
diff --git a/src/util/types/typeCache.ts b/src/util/types/typeCache.ts
index 56adcd6..a4b4e74 100644
--- a/src/util/types/typeCache.ts
+++ b/src/util/types/typeCache.ts
@@ -5,12 +5,18 @@ import { InferenceResult } from "./typeInference";
 type CacheKey =
   | "PACKAGE_TYPE_ANNOTATION"
   | "PACKAGE_TYPE_AND_TYPE_ALIAS"
-  | "PACKAGE_VALUE";
+  | "PACKAGE_VALUE_DECLARATION"
+  | "PROJECT_TYPE_ANNOTATION"
+  | "PROJECT_TYPE_AND_TYPE_ALIAS"
+  | "PROJECT_VALUE_DECLARATION";
 
 export class TypeCache {
   private packageTypeAnnotation: SyntaxNodeMap<SyntaxNode, InferenceResult>;
   private packageTypeAndTypeAlias: SyntaxNodeMap<SyntaxNode, InferenceResult>;
-  private packageValue: SyntaxNodeMap<SyntaxNode, InferenceResult>;
+  private packageValueDeclaration: SyntaxNodeMap<SyntaxNode, InferenceResult>;
+  private projectTypeAnnotation: SyntaxNodeMap<SyntaxNode, InferenceResult>;
+  private projectTypeAndTypeAlias: SyntaxNodeMap<SyntaxNode, InferenceResult>;
+  private projectValueDeclaration: SyntaxNodeMap<SyntaxNode, InferenceResult>;
 
   constructor() {
     this.packageTypeAnnotation = new SyntaxNodeMap<
@@ -21,7 +27,22 @@ export class TypeCache {
       SyntaxNode,
       InferenceResult
     >();
-    this.packageValue = new SyntaxNodeMap<SyntaxNode, InferenceResult>();
+    this.packageValueDeclaration = new SyntaxNodeMap<
+      SyntaxNode,
+      InferenceResult
+    >();
+    this.projectTypeAnnotation = new SyntaxNodeMap<
+      SyntaxNode,
+      InferenceResult
+    >();
+    this.projectTypeAndTypeAlias = new SyntaxNodeMap<
+      SyntaxNode,
+      InferenceResult
+    >();
+    this.projectValueDeclaration = new SyntaxNodeMap<
+      SyntaxNode,
+      InferenceResult
+    >();
   }
 
   public getOrSet(
@@ -34,8 +55,24 @@ export class TypeCache {
         return this.packageTypeAnnotation.getOrSet(node, setter);
       case "PACKAGE_TYPE_AND_TYPE_ALIAS":
         return this.packageTypeAndTypeAlias.getOrSet(node, setter);
-      case "PACKAGE_VALUE":
-        return this.packageValue.getOrSet(node, setter);
+      case "PACKAGE_VALUE_DECLARATION":
+        return this.packageValueDeclaration.getOrSet(node, setter);
+      case "PROJECT_TYPE_ANNOTATION":
+        return this.projectTypeAnnotation.getOrSet(node, setter);
+      case "PROJECT_TYPE_AND_TYPE_ALIAS":
+        return this.projectTypeAndTypeAlias.getOrSet(node, setter);
+      case "PROJECT_VALUE_DECLARATION":
+        return this.projectValueDeclaration.getOrSet(node, setter);
     }
   }
+
+  public invalidateProject(): void {
+    this.projectTypeAnnotation.clear();
+    this.projectTypeAndTypeAlias.clear();
+    this.projectValueDeclaration.clear();
+  }
+
+  public invalidateValueDeclaration(node: SyntaxNode): void {
+    this.projectValueDeclaration.delete(node);
+  }
 }
diff --git a/src/util/types/typeExpression.ts b/src/util/types/typeExpression.ts
index 2fb2538..bdbd0f0 100644
--- a/src/util/types/typeExpression.ts
+++ b/src/util/types/typeExpression.ts
@@ -83,7 +83,9 @@ export class TypeExpression {
         .getTypeCache()
         .getOrSet("PACKAGE_TYPE_AND_TYPE_ALIAS", e, setter);
     } else {
-      return setter();
+      return workspace
+        .getTypeCache()
+        .getOrSet("PROJECT_TYPE_AND_TYPE_ALIAS", e, setter);
     }
   }
 
@@ -115,7 +117,9 @@ export class TypeExpression {
         .getTypeCache()
         .getOrSet("PACKAGE_TYPE_AND_TYPE_ALIAS", e, setter);
     } else {
-      return setter();
+      return workspace
+        .getTypeCache()
+        .getOrSet("PROJECT_TYPE_AND_TYPE_ALIAS", e, setter);
     }
   }
 
@@ -152,7 +156,9 @@ export class TypeExpression {
         .getTypeCache()
         .getOrSet("PACKAGE_TYPE_ANNOTATION", e.typeExpression, setter);
     } else {
-      return setter();
+      return workspace
+        .getTypeCache()
+        .getOrSet("PROJECT_TYPE_ANNOTATION", e.typeExpression, setter);
     }
   }
 
diff --git a/src/util/types/typeInference.ts b/src/util/types/typeInference.ts
index d6168eb..b9d0f65 100644
--- a/src/util/types/typeInference.ts
+++ b/src/util/types/typeInference.ts
@@ -195,15 +195,25 @@ function allTypeVars(type: Type): TVar[] {
     case "Var":
       return [type];
     case "Union":
-      return type.params.flatMap(allTypeVars);
+      return Array.prototype.flatMap !== undefined
+        ? type.params.flatMap(allTypeVars)
+        : flatMap(type.params, allTypeVars);
     case "Function":
-      return allTypeVars(type.return).concat(type.params.flatMap(allTypeVars));
+      return allTypeVars(type.return).concat(
+        Array.prototype.flatMap !== undefined
+          ? type.params.flatMap(allTypeVars)
+          : flatMap(type.params, allTypeVars),
+      );
     case "Tuple":
-      return type.types.flatMap(allTypeVars);
+      return Array.prototype.flatMap !== undefined
+        ? type.types.flatMap(allTypeVars)
+        : flatMap(type.types, allTypeVars);
     case "Record":
     case "MutableRecord": {
       return [
-        ...Object.values(type.fields).flatMap(allTypeVars),
+        ...(Array.prototype.flatMap !== undefined
+          ? Object.values(type.fields).flatMap(allTypeVars)
+          : flatMap(Object.values(type.fields), allTypeVars)),
         ...(type.baseType ? allTypeVars(type.baseType) : []),
       ];
     }
@@ -583,9 +593,11 @@ export class InferenceScope {
     if (!elmWorkspace.getForest().getByUri(uri)?.writeable) {
       return elmWorkspace
         .getTypeCache()
-        .getOrSet("PACKAGE_VALUE", declaration, setter);
+        .getOrSet("PACKAGE_VALUE_DECLARATION", declaration, setter);
     } else {
-      return setter();
+      return elmWorkspace
+        .getTypeCache()
+        .getOrSet("PROJECT_VALUE_DECLARATION", declaration, setter);
     }
   }
 
@@ -1344,7 +1356,7 @@ export class InferenceScope {
       expr,
     );
 
-    if (!fieldIdentifier) {
+    if (!fieldIdentifier || fieldIdentifier.text === "") {
       return TUnknown;
     }
 
@@ -1394,7 +1406,7 @@ export class InferenceScope {
       }
     }
 
-    const type = targetTy.fields[fieldIdentifier.text];
+    const type = targetTy.fields[fieldIdentifier.text] ?? TUnknown;
     this.expressionTypes.set(expr, type);
     return type;
   }
@@ -1911,6 +1923,10 @@ export class InferenceScope {
     endExpr?: Expression,
     patternBinding = false,
   ): boolean {
+    if (!type1 || !type2) {
+      throw new Error("Undefined type error");
+    }
+
     let assignable: boolean;
 
     if (expr.nodeType === "CaseOfExpr") {
@@ -2423,7 +2439,7 @@ export function findType(
         expr.startIndex === expr.parent?.startIndex &&
         expr.endIndex === expr.parent?.endIndex
       ) {
-        return findTypeOrParentType(mapSyntaxNodeToExpression(expr.parent));
+        return findTypeOrParentType(expr.parent);
       }
     };
 


Test patch:
diff --git a/test/completionProvider.test.ts b/test/completionProvider.test.ts
index a32c78e..01f9a0f 100644
--- a/test/completionProvider.test.ts
+++ b/test/completionProvider.test.ts
@@ -1247,4 +1247,93 @@ encodeScope scope =
       "normal",
     );
   });
+
+  it("Record access in list expression", async () => {
+    const source = `
+--@ Test.elm
+module Test exposing (..)
+
+type alias Model = 
+  { prop1: String
+  , prop2: Int
+  }
+
+map: (a -> b) -> List a -> List b
+map func list =
+  list
+
+func : List Model -> List a
+func model =
+    map (\\m -> m.{-caret-}) model
+`;
+
+    await testCompletions(source, ["prop1", "prop2"], "exactMatch");
+  });
+
+  it("Record access of a destructed union constructor", async () => {
+    const source = `
+--@ Test.elm
+module Test exposing (..)
+
+type State
+    = State { field1 : String, field2 : String }
+
+
+func : State -> a
+func (State state) =
+    [ state.{-caret-} ]
+`;
+
+    await testCompletions(source, ["field1", "field2"], "exactMatch");
+  });
+
+  it("Record access inside Maybe case of branch", async () => {
+    const source = `
+--@ Test.elm
+module Test exposing (..)
+
+type alias Model = 
+  { prop1: String
+  , prop2: Int
+  }
+
+type Maybe a = Just a | Nothing
+
+func : Maybe Model -> a
+func model =
+    case model of
+        Just m ->
+            m.{-caret-}
+`;
+
+    await testCompletions(source, ["prop1", "prop2"], "exactMatch");
+  });
+
+  it("Record access destructured case branch", async () => {
+    const source = `
+--@ Test.elm
+module Test exposing (..)
+
+type alias Model = 
+  { prop1: Model2
+  , prop2: Int
+  }
+
+type alias Model2 = 
+  { field1: String 
+  , field2: Int }
+
+type Maybe a = Just a | Nothing
+
+func : Maybe Model -> a
+func model =
+    case model of
+        Just { prop1, prop2 } ->
+            prop1.{-caret-}
+
+        Nothing ->
+`;
+
+    await testCompletions(source, ["field1", "field2"], "exactMatch");
+  });
 });