fix: wrong variable scope in let directive (#271)

* fix: wrong variable scope in let directive

* Create angry-pumpkins-ring.md

* fix

Author: ota-meshi

.changeset/angry-pumpkins-ring.md

@@ -0,0 +1,5 @@
+---
+"svelte-eslint-parser": minor
+---
+
+fix: wrong variable scope in let directive

src/context/let-directive-collection.ts

@@ -1,45 +1,13 @@
-import type { SvelteLetDirective, SvelteName, SvelteNode } from "../ast";
+import type { SvelteLetDirective, SvelteName } from "../ast";
 import type * as ESTree from "estree";
-import type { ScriptLetCallback, ScriptLetCallbackOption } from "./script-let";
+import type { ScriptLetBlockParam, ScriptLetCallback } from "./script-let";
 
 /** A class that collects pattern nodes for Let directives. */
 export class LetDirectiveCollection {
-  private readonly list: {
-    pattern: ESTree.Pattern | SvelteName;
-    directive: SvelteLetDirective;
-    typing: string;
-    callbacks: ScriptLetCallback<ESTree.Pattern>[];
-  }[] = [];
+  private readonly list: ScriptLetBlockParam[] = [];
 
-  public isEmpty(): boolean {
-    return this.list.length === 0;
-  }
-
-  public getLetParams(): (ESTree.Pattern | SvelteName)[] {
-    return this.list.map((d) => d.pattern);
-  }
-
-  public getParents(): SvelteNode[] {
-    return this.list.map((d) => d.directive);
-  }
-
-  public getCallback(): (
-    nodes: ESTree.Pattern[],
-    options: ScriptLetCallbackOption
-  ) => void {
-    return (nodes, options) => {
-      for (let index = 0; index < nodes.length; index++) {
-        const node = nodes[index];
-        const { callbacks } = this.list[index];
-        for (const callback of callbacks) {
-          callback(node, options);
-        }
-      }
-    };
-  }
-
-  public getTypes(): string[] {
-    return this.list.map((d) => d.typing);
+  public getLetParams(): ScriptLetBlockParam[] {
+    return this.list;
   }
 
   public addPattern(
@@ -49,10 +17,14 @@ export class LetDirectiveCollection {
     ...callbacks: ScriptLetCallback<ESTree.Pattern>[]
   ): ScriptLetCallback<ESTree.Pattern>[] {
     this.list.push({
-      pattern,
-      directive,
+      node: pattern,
+      parent: directive,
       typing,
-      callbacks,
+      callback(node, options) {
+        for (const callback of callbacks) {
+          callback(node, options);
+        }
+      },
     });
     return callbacks;
   }

src/context/script-let.ts

@@ -113,6 +113,12 @@ type ObjectShorthandProperty = ESTree.Property & {
   value: ESTree.Identifier;
 };
 
+export type ScriptLetBlockParam = {
+  node: ESTree.Pattern | SvelteName;
+  parent: SvelteNode;
+  typing: string;
+  callback: (node: ESTree.Pattern, options: ScriptLetCallbackOption) => void;
+};
 /**
  * A class that handles script fragments.
  * The script fragment AST node remaps and connects to the original directive AST node.
@@ -408,62 +414,15 @@ export class ScriptLetContext {
     this.pushScope(restore, "});");
   }
 
-  public nestBlock(block: SvelteNode): void;
-
-  public nestBlock(
-    block: SvelteNode,
-    params: (ESTree.Pattern | SvelteName)[],
-    parents: SvelteNode[],
-    callback: (
-      nodes: ESTree.Pattern[],
-      options: ScriptLetCallbackOption
-    ) => void,
-    typings:
-      | string[]
-      | ((helper: TypeGenHelper) => {
-          typings: string[];
-          preparationScript?: string[];
-        })
-  ): void;
-
   public nestBlock(
     block: SvelteNode,
-    params?: (ESTree.Pattern | SvelteName)[],
-    parents?: SvelteNode[],
-    callback?: (
-      nodes: ESTree.Pattern[],
-      options: ScriptLetCallbackOption
-    ) => void,
-    typings?:
-      | string[]
-      | ((helper: TypeGenHelper) => {
-          typings: string[];
+    params?:
+      | ScriptLetBlockParam[]
+      | ((helper: TypeGenHelper | null) => {
+          param: ScriptLetBlockParam;
           preparationScript?: string[];
         })
   ): void {
-    let arrayTypings: string[] = [];
-    if (typings && this.ctx.isTypeScript()) {
-      if (Array.isArray(typings)) {
-        arrayTypings = typings;
-      } else {
-        const generatedTypes = typings({
-          generateUniqueId: (base) => this.generateUniqueId(base),
-        });
-        arrayTypings = generatedTypes.typings;
-        if (generatedTypes.preparationScript) {
-          for (const preparationScript of generatedTypes.preparationScript) {
-            this.appendScriptWithoutOffset(
-              preparationScript,
-              (node, tokens, comments, result) => {
-                tokens.length = 0;
-                comments.length = 0;
-                removeAllScopeAndVariableAndReference(node, result);
-              }
-            );
-          }
-        }
-      }
-    }
     if (!params) {
       const restore = this.appendScript(
         `{`,
@@ -483,7 +442,44 @@ export class ScriptLetContext {
       );
       this.pushScope(restore, "}");
     } else {
-      const ranges = params.map(getNodeRange).sort(([a], [b]) => a - b);
+      let resolvedParams;
+      if (typeof params === "function") {
+        if (this.ctx.isTypeScript()) {
+          const generatedTypes = params({
+            generateUniqueId: (base) => this.generateUniqueId(base),
+          });
+          resolvedParams = [generatedTypes.param];
+          if (generatedTypes.preparationScript) {
+            for (const preparationScript of generatedTypes.preparationScript) {
+              this.appendScriptWithoutOffset(
+                preparationScript,
+                (node, tokens, comments, result) => {
+                  tokens.length = 0;
+                  comments.length = 0;
+                  removeAllScopeAndVariableAndReference(node, result);
+                }
+              );
+            }
+          }
+        } else {
+          const generatedTypes = params(null);
+          resolvedParams = [generatedTypes.param];
+        }
+      } else {
+        resolvedParams = params;
+      }
+      const sortedParams = [...resolvedParams]
+        .map((d) => {
+          return {
+            ...d,
+            range: getNodeRange(d.node),
+          };
+        })
+        .sort((a, b) => {
+          const [pA] = a.range;
+          const [pB] = b.range;
+          return pA - pB;
+        });
 
       const maps: {
         index: number;
@@ -492,8 +488,9 @@ export class ScriptLetContext {
       }[] = [];
 
       let source = "";
-      for (let index = 0; index < ranges.length; index++) {
-        const range = ranges[index];
+      for (let index = 0; index < sortedParams.length; index++) {
+        const param = sortedParams[index];
+        const range = param.range;
         if (source) {
           source += ",";
         }
@@ -505,7 +502,7 @@ export class ScriptLetContext {
           range,
         });
         if (this.ctx.isTypeScript()) {
-          source += `: (${arrayTypings[index]})`;
+          source += `: (${param.typing})`;
         }
       }
       const restore = this.appendScript(
@@ -517,9 +514,9 @@ export class ScriptLetContext {
           const scope = result.getScope(fn.body);
 
           // Process for nodes
-          callback!(fn.params, result);
           for (let index = 0; index < fn.params.length; index++) {
             const p = fn.params[index];
+            sortedParams[index].callback(p, result);
             if (this.ctx.isTypeScript()) {
               const typeAnnotation = (p as any).typeAnnotation;
               delete (p as any).typeAnnotation;
@@ -532,7 +529,7 @@ export class ScriptLetContext {
 
               removeAllScopeAndVariableAndReference(typeAnnotation, result);
             }
-            (p as any).parent = parents![index];
+            (p as any).parent = sortedParams[index].parent;
           }
 
           // Process for scope

src/parser/converts/block.ts

@@ -18,6 +18,7 @@ import type {
 import type { Context } from "../../context";
 import { convertChildren } from "./element";
 import { getWithLoc, indexOf, lastIndexOf } from "./common";
+import type * as ESTree from "estree";
 
 /** Get start index of block */
 function startBlockIndex(code: string, endIndex: number): number {
@@ -281,37 +282,52 @@ export function convertAwaitBlock(
       }),
     };
     if (node.value) {
-      ctx.scriptLet.nestBlock(
-        thenBlock,
-        [node.value],
-        [thenBlock],
-        ([value]) => {
+      const baseParam = {
+        node: node.value,
+        parent: thenBlock,
+        callback(value: ESTree.Pattern) {
           thenBlock.value = value;
         },
-        ({ generateUniqueId }) => {
-          const expression = ctx.getText(node.expression);
-          if (node.expression.type === "Literal") {
-            return {
-              typings: [expression],
-            };
-          }
-          const idAwaitThenValue = generateUniqueId("AwaitThenValue");
-          if (node.expression.type === "Identifier") {
-            return {
-              preparationScript: [generateAwaitThenValueType(idAwaitThenValue)],
-              typings: [`${idAwaitThenValue}<(typeof ${expression})>`],
-            };
-          }
-          const id = generateUniqueId(expression);
+        typing: "any",
+      };
+
+      ctx.scriptLet.nestBlock(thenBlock, (typeCtx) => {
+        if (!typeCtx) {
           return {
-            preparationScript: [
-              `const ${id} = ${expression};`,
-              generateAwaitThenValueType(idAwaitThenValue),
-            ],
-            typings: [`${idAwaitThenValue}<(typeof ${id})>`],
+            param: baseParam,
           };
         }
-      );
+        const expression = ctx.getText(node.expression);
+        if (node.expression.type === "Literal") {
+          return {
+            param: {
+              ...baseParam,
+              typing: expression,
+            },
+          };
+        }
+        const idAwaitThenValue = typeCtx.generateUniqueId("AwaitThenValue");
+        if (node.expression.type === "Identifier") {
+          return {
+            preparationScript: [generateAwaitThenValueType(idAwaitThenValue)],
+            param: {
+              ...baseParam,
+              typing: `${idAwaitThenValue}<(typeof ${expression})>`,
+            },
+          };
+        }
+        const id = typeCtx.generateUniqueId(expression);
+        return {
+          preparationScript: [
+            `const ${id} = ${expression};`,
+            generateAwaitThenValueType(idAwaitThenValue),
+          ],
+          param: {
+            ...baseParam,
+            typing: `${idAwaitThenValue}<(typeof ${id})>`,
+          },
+        };
+      });
     } else {
       ctx.scriptLet.nestBlock(thenBlock);
     }
@@ -351,15 +367,16 @@ export function convertAwaitBlock(
     } as SvelteAwaitCatchBlock;
 
     if (node.error) {
-      ctx.scriptLet.nestBlock(
-        catchBlock,
-        [node.error],
-        [catchBlock],
-        ([error]) => {
-          catchBlock.error = error;
+      ctx.scriptLet.nestBlock(catchBlock, [
+        {
+          node: node.error,
+          parent: catchBlock,
+          typing: "Error",
+          callback: (error) => {
+            catchBlock.error = error;
+          },
         },
-        ["Error"]
-      );
+      ]);
     } else {
       ctx.scriptLet.nestBlock(catchBlock);
     }