Skip to content

Commit

Permalink
fix: wrong variable scope in let directive (#271)
Browse files Browse the repository at this point in the history
* fix: wrong variable scope in let directive

* Create angry-pumpkins-ring.md

* fix
  • Loading branch information
ota-meshi authored Feb 3, 2023
1 parent 2274c23 commit e355d5c
Show file tree
Hide file tree
Showing 14 changed files with 4,676 additions and 217 deletions.
5 changes: 5 additions & 0 deletions .changeset/angry-pumpkins-ring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte-eslint-parser": minor
---

fix: wrong variable scope in let directive
52 changes: 12 additions & 40 deletions src/context/let-directive-collection.ts
Original file line number Diff line number Diff line change
@@ -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(
Expand All @@ -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;
}
Expand Down
111 changes: 54 additions & 57 deletions src/context/script-let.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(
`{`,
Expand All @@ -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;
Expand All @@ -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 += ",";
}
Expand All @@ -505,7 +502,7 @@ export class ScriptLetContext {
range,
});
if (this.ctx.isTypeScript()) {
source += `: (${arrayTypings[index]})`;
source += `: (${param.typing})`;
}
}
const restore = this.appendScript(
Expand All @@ -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;
Expand All @@ -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
Expand Down
85 changes: 51 additions & 34 deletions src/parser/converts/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
Loading

0 comments on commit e355d5c

Please sign in to comment.