Repository: elm-tooling/elm-language-server
Pull request #386 (2020-12-04 12:48:15)

Properly calculate non shadowable names
Adds a test case for the proper error message.


Patch:
diff --git a/src/elmWorkspace.ts b/src/elmWorkspace.ts
index bb9dc0c..d1f016d 100644
--- a/src/elmWorkspace.ts
+++ b/src/elmWorkspace.ts
@@ -78,6 +78,7 @@ export interface IElmWorkspace {
   hasDocument(uri: URI): boolean;
   hasPath(uri: URI): boolean;
   getPath(uri: URI): string | undefined;
+  getSourceFile(uri: string): ITreeContainer | undefined;
   getForest(synchronize?: boolean): IForest;
   getRootPath(): URI;
   getTypeCache(): TypeCache;
@@ -152,6 +153,10 @@ export class ElmWorkspace implements IElmWorkspace {
     );
   }
 
+  public getSourceFile(uri: string): ITreeContainer | undefined {
+    return this.getForest().getByUri(uri);
+  }
+
   public getForest(synchronize = true): IForest {
     if (this.dirty && synchronize) {
       this.forest.synchronize();
diff --git a/src/forest.ts b/src/forest.ts
index afdaadf..a7e07f7 100644
--- a/src/forest.ts
+++ b/src/forest.ts
@@ -26,6 +26,7 @@ export interface ITreeContainer {
   // Resolved during binding
   exposing?: IExposing;
   symbolLinks?: SyntaxNodeMap<SyntaxNode, SymbolMap>;
+  nonShadowableNames?: Set<string>; // Top level function names
 }
 
 export interface IForest {
diff --git a/src/util/types/binder.ts b/src/util/types/binder.ts
index e961a97..43837f2 100644
--- a/src/util/types/binder.ts
+++ b/src/util/types/binder.ts
@@ -24,12 +24,14 @@ export function bindTreeContainer(treeContainer: ITreeContainer): void {
   }
 
   const symbolLinks = new SyntaxNodeMap<SyntaxNode, SymbolMap>();
+  const nonShadowableNames = new Set<string>();
   let container: SymbolMap;
   let parent: SyntaxNode;
   const treeCursor = treeContainer.tree.walk();
 
   bind();
   treeContainer.symbolLinks = symbolLinks;
+  treeContainer.nonShadowableNames = nonShadowableNames;
 
   // Bind exposing must happen after symbolLinks is bound
   bindExposing();
@@ -112,10 +114,16 @@ export function bindTreeContainer(treeContainer: ITreeContainer): void {
     );
 
     if (functionDeclarationLeft && functionDeclarationLeft.firstChild) {
-      container.set(functionDeclarationLeft.firstChild.text, {
+      const functionName = functionDeclarationLeft.firstChild.text;
+      container.set(functionName, {
         node: functionDeclarationLeft,
         type: "Function",
       });
+
+      // Add to nonShadowableNames if it is top level
+      if (node.parent?.type === "file") {
+        nonShadowableNames.add(functionDeclarationLeft.firstChild.text);
+      }
     } else {
       // If there is a pattern, bind it to the parent container
       const pattern = node.childForFieldName("pattern");
diff --git a/src/util/types/typeInference.ts b/src/util/types/typeInference.ts
index 640d734..51b45ed 100644
--- a/src/util/types/typeInference.ts
+++ b/src/util/types/typeInference.ts
@@ -548,18 +548,18 @@ export class InferenceScope {
   public static valueDeclarationInference(
     declaration: EValueDeclaration,
     uri: string,
-    elmWorkspace: IElmWorkspace,
+    program: IElmWorkspace,
     activeScopes: Set<EValueDeclaration>,
     cancellationToken?: ICancellationToken,
   ): InferenceResult {
-    // TODO: Need a good way to get all visible values
-    const shadowableNames = new Set<string>();
+    const nonShadowableNames =
+      program.getSourceFile(uri)?.nonShadowableNames ?? new Set<string>();
 
     const setter = (): InferenceResult =>
       new InferenceScope(
         uri,
-        elmWorkspace,
-        shadowableNames,
+        program,
+        nonShadowableNames,
         new Set(activeScopes.values()),
         /* recursionAllowed */ false,
         cancellationToken,
@@ -567,12 +567,12 @@ export class InferenceScope {
 
     const start = performance.now();
     try {
-      if (!elmWorkspace.getForest().getByUri(uri)?.writeable) {
-        return elmWorkspace
+      if (!program.getForest().getByUri(uri)?.writeable) {
+        return program
           .getTypeCache()
           .getOrSet("PACKAGE_VALUE_DECLARATION", declaration, setter);
       } else {
-        return elmWorkspace
+        return program
           .getTypeCache()
           .getOrSet("PROJECT_VALUE_DECLARATION", declaration, setter);
       }


Test patch:
diff --git a/test/diagnosticTests/elmDiagnostics.test.ts b/test/diagnosticTests/elmDiagnostics.test.ts
index 9d27846..78faef8 100644
--- a/test/diagnosticTests/elmDiagnostics.test.ts
+++ b/test/diagnosticTests/elmDiagnostics.test.ts
@@ -158,4 +158,23 @@ func = 5
       { message: Diagnostics.ImportMissing, args: ["App"] },
     ]);
   });
+
+  test("Shadowing a function with the same name", async () => {
+    const source = `
+--@ Test.elm
+module Test exposing (..)
+
+type alias Model =
+    { field : Int
+    }
+
+field : Model -> Int
+field { field } =
+        --^
+    4
+`;
+    await testTypeInference(basicsSources + source, [
+      { message: Diagnostics.Redefinition, args: ["field"] },
+    ]);
+  });
 });
diff --git a/test/utils/mockElmWorkspace.ts b/test/utils/mockElmWorkspace.ts
index 759d7f1..362f976 100644
--- a/test/utils/mockElmWorkspace.ts
+++ b/test/utils/mockElmWorkspace.ts
@@ -62,6 +62,10 @@ export class MockElmWorkspace implements IElmWorkspace {
     return baseUri;
   }
 
+  public getSourceFile(uri: string): ITreeContainer | undefined {
+    return this.getForest().getByUri(uri);
+  }
+
   getForest(): IForest {
     return this.forest;
   }