Repository: elm-tooling/elm-language-server
Pull request #421 (2020-12-28 18:12:32)

Fix bug with record references leading to maximum call stack exception


Patch:
diff --git a/src/util/types/recordFieldReferenceTable.ts b/src/util/types/recordFieldReferenceTable.ts
index cf72f66..e5637b4 100644
--- a/src/util/types/recordFieldReferenceTable.ts
+++ b/src/util/types/recordFieldReferenceTable.ts
@@ -1,35 +1,36 @@
 import { SyntaxNode } from "web-tree-sitter";
 import { EFieldType } from "./expressionTree";
+import { SyntaxNodeSet } from "./syntaxNodeSet";
 
 export class RecordFieldReferenceTable {
-  private refsByField: Map<string, SyntaxNode[]>;
+  private refsByField: Map<string, SyntaxNodeSet>;
   private _frozen = false;
 
   public get frozen(): boolean {
     return this._frozen;
   }
 
-  constructor(refsByField?: Map<string, SyntaxNode[]>) {
-    this.refsByField = refsByField ?? new Map<string, SyntaxNode[]>();
+  constructor(refsByField?: Map<string, SyntaxNodeSet>) {
+    this.refsByField = refsByField ?? new Map<string, SyntaxNodeSet>();
   }
 
   public static fromExpressions(
     fieldExpressions: EFieldType[],
   ): RecordFieldReferenceTable {
-    const fieldRefs: Map<string, SyntaxNode[]> = new Map<
+    const fieldRefs: Map<string, SyntaxNodeSet> = new Map<
       string,
-      SyntaxNode[]
+      SyntaxNodeSet
     >();
 
     fieldExpressions.forEach((field) => {
-      fieldRefs.set(field.name, [field]);
+      fieldRefs.set(field.name, new SyntaxNodeSet(field));
     });
 
     return new RecordFieldReferenceTable(fieldRefs);
   }
 
   public get(field: string): SyntaxNode[] {
-    return this.refsByField.get(field) ?? [];
+    return this.refsByField.get(field)?.toArray() ?? [];
   }
 
   public addAll(other: RecordFieldReferenceTable): void {
@@ -41,20 +42,26 @@ export class RecordFieldReferenceTable {
       let set = this.refsByField.get(field);
 
       if (!set) {
-        this.refsByField.set(field, []);
-        set = this.refsByField.get(field) ?? [];
+        set = new SyntaxNodeSet();
+        this.refsByField.set(field, set);
       }
 
-      set.push(...refs);
+      set.addAll(refs);
     });
   }
 
   public plus(other: RecordFieldReferenceTable): RecordFieldReferenceTable {
-    const newRefs = new Map<string, SyntaxNode[]>();
+    const newRefs = new Map<string, SyntaxNodeSet>();
 
     this.refsByField.forEach((set, field) => {
-      const otherSet = other.refsByField.get(field) ?? [];
-      newRefs.set(field, [...set, ...otherSet]);
+      const newRefSet = new SyntaxNodeSet().addAll(set);
+      const otherSet = other.refsByField.get(field);
+
+      if (otherSet) {
+        newRefSet.addAll(otherSet);
+      }
+
+      newRefs.set(field, newRefSet);
     });
     other.refsByField.forEach((set, field) => {
       if (!newRefs.get(field)) {
diff --git a/src/util/types/syntaxNodeSet.ts b/src/util/types/syntaxNodeSet.ts
new file mode 100644
index 0000000..e83f65c
--- /dev/null
+++ b/src/util/types/syntaxNodeSet.ts
@@ -0,0 +1,50 @@
+import { SyntaxNode } from "web-tree-sitter";
+
+export class SyntaxNodeSet<K extends SyntaxNode = SyntaxNode> {
+  private map = new Map<number, K>();
+
+  constructor(...items: K[]) {
+    items.forEach((value) => {
+      this.add(value);
+    });
+  }
+  public get size(): number {
+    return this.map.size;
+  }
+
+  public add(value: K): this {
+    this.map.set(value.id, value);
+    return this;
+  }
+
+  public clear(): void {
+    this.map.clear();
+  }
+
+  public delete(value: K): boolean {
+    return this.map.delete(value.id);
+  }
+
+  public forEach(callbackfn: (value: K) => void): void {
+    this.map.forEach((value) => {
+      callbackfn(value);
+    });
+  }
+
+  public has(value: K): boolean {
+    return this.map.has(value.id);
+  }
+
+  public values(): IterableIterator<K> {
+    return this.map.values();
+  }
+
+  public toArray(): K[] {
+    return Array.from(this.values());
+  }
+
+  public addAll(other: SyntaxNodeSet<K>): this {
+    other.forEach((value) => this.add(value));
+    return this;
+  }
+}


Test patch:
diff --git a/test/diagnosticTests/elmDiagnostics.test.ts b/test/diagnosticTests/elmDiagnostics.test.ts
index 889fa7b..b023e56 100644
--- a/test/diagnosticTests/elmDiagnostics.test.ts
+++ b/test/diagnosticTests/elmDiagnostics.test.ts
@@ -18,11 +18,12 @@ import { baseUri, SourceTreeParser } from "../utils/sourceTreeParser";
 
 const basicsSources = `
 --@ Basics.elm
-module Basics exposing ((+), (|>), (==), Int, Float, Bool(..), Order(..))
+module Basics exposing ((+), (|>), (>>), (==), Int, Float, Bool(..), Order(..), negate)
 
 infix left  0 (|>) = apR
 infix non   4 (==) = eq
 infix left  6 (+)  = add
+infix right 9 (>>) = composeR
 
 type Int = Int
 
@@ -42,7 +43,15 @@ eq : a -> a -> Bool
 eq =
   Elm.Kernel.Utils.equal
 
+composeR : (a -> b) -> (b -> c) -> (a -> c)
+composeR f g x =
+  g (f x)
+
 type Order = LT | EQ | GT
+
+negate : number -> number
+negate n =
+  -n
 `;
 describe("test elm diagnostics", () => {
   const treeParser = new SourceTreeParser();
@@ -219,4 +228,59 @@ field =
   `;
     await testTypeInference(basicsSources + source, []);
   });
+
+  test("field references accessed in complex ways", async () => {
+    const source = `
+--@ Test.elm
+module Test exposing (..)
+
+type alias TopChart a comparable =
+    { toValue : a -> Int
+    , items : List a
+    , toValueLabel : a -> Int
+    , toLabel : a -> Int
+    , sorter : a -> comparable
+    , filter : a -> Bool
+    }
+
+
+topChart : List { name : Int, amount : Int } -> TopChart { name : Int, amount : Int } Int
+topChart items =
+    default
+        { toValue = .amount
+        , items = items
+        }
+        |> withSorter (.amount >> negate)
+        |> withFilter (.amount >> (\\x -> x > 0))
+
+
+default : { toValue : a -> Int, items : List a } -> TopChart a Int
+default { toValue, items } =
+    { toValue = toValue
+    , items = items
+    , toValueLabel = \\_ -> 1
+    , toLabel = toValue
+    , sorter = toValue
+    , filter = \\_ -> True
+    }
+
+
+withSorter : (a -> comparable2) -> TopChart a comparable -> TopChart a comparable2
+withSorter sorter chart =
+    { toValue = chart.toValue
+    , items = chart.items
+    , toValueLabel = chart.toValueLabel
+    , toLabel = chart.toLabel
+    , sorter = sorter
+    , filter = chart.filter
+    }
+
+
+withFilter : (a -> Bool) -> TopChart a comparable -> TopChart a comparable
+withFilter filter chart =
+    { chart | filter = filter }
+    `;
+
+    await testTypeInference(basicsSources + source, []);
+  });
 });