Skip to content

Commit

Permalink
fix: resolve to module scope for top level statements (#292)
Browse files Browse the repository at this point in the history
* fix: resolve to module scope for top level statements

* Create .changeset/modern-spiders-retire.md
  • Loading branch information
ota-meshi authored Mar 8, 2023
1 parent 97a1a68 commit ec061f5
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 35 deletions.
7 changes: 7 additions & 0 deletions .changeset/modern-spiders-retire.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"svelte-eslint-parser": minor
---

BREAKING: fix resolve to module scope for top level statements

This change corrects the result of `context.getScope()`, but it is a breaking change.
114 changes: 79 additions & 35 deletions src/context/script-let.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ type ScriptLetRestoreCallbackOption = {
registerNodeToScope: (node: any, scope: Scope) => void;
scopeManager: ScopeManager;
visitorKeys?: { [type: string]: string[] };
addPostProcess: (callback: () => void) => void;
};

/**
Expand Down Expand Up @@ -130,6 +131,8 @@ export class ScriptLetContext {

private readonly restoreCallbacks: RestoreCallback[] = [];

private readonly programRestoreCallbacks: ScriptLetRestoreCallback[] = [];

private readonly closeScopeCallbacks: (() => void)[] = [];

private readonly unique = new UniqueIdGenerator();
Expand Down Expand Up @@ -574,6 +577,10 @@ export class ScriptLetContext {
this.closeScopeCallbacks.pop()!();
}

public addProgramRestore(callback: ScriptLetRestoreCallback): void {
this.programRestoreCallbacks.push(callback);
}

private appendScript(
text: string,
offset: number,
Expand Down Expand Up @@ -631,6 +638,57 @@ export class ScriptLetContext {
* Restore AST nodes
*/
public restore(result: ESLintExtendedProgram): void {
const nodeToScope = getNodeToScope(result.scopeManager!);
const postprocessList: (() => void)[] = [];

const callbackOption: ScriptLetRestoreCallbackOption = {
getScope,
getInnermostScope,
registerNodeToScope,
scopeManager: result.scopeManager!,
visitorKeys: result.visitorKeys,
addPostProcess: (cb) => postprocessList.push(cb),
};

this.restoreNodes(result, callbackOption);
this.restoreProgram(result, callbackOption);
postprocessList.forEach((p) => p());

// Helpers
/** Get scope */
function getScope(node: ESTree.Node) {
return getScopeFromNode(result.scopeManager!, node);
}

/** Get innermost scope */
function getInnermostScope(node: ESTree.Node) {
return getInnermostScopeFromNode(result.scopeManager!, node);
}

/** Register node to scope */
function registerNodeToScope(node: any, scope: Scope): void {
// If we replace the `scope.block` at this time,
// the scope restore calculation will not work, so we will replace the `scope.block` later.
postprocessList.push(() => {
scope.block = node;
});

const scopes = nodeToScope.get(node);
if (scopes) {
scopes.push(scope);
} else {
nodeToScope.set(node, [scope]);
}
}
}

/**
* Restore AST nodes
*/
private restoreNodes(
result: ESLintExtendedProgram,
callbackOption: ScriptLetRestoreCallbackOption
): void {
let orderedRestoreCallback = this.restoreCallbacks.shift();
if (!orderedRestoreCallback) {
return;
Expand All @@ -640,8 +698,6 @@ export class ScriptLetContext {
const processedTokens = [];
const comments = result.ast.comments;
const processedComments = [];
const nodeToScope = getNodeToScope(result.scopeManager!);
const postprocessList: (() => void)[] = [];

let tok;
while ((tok = tokens.shift())) {
Expand Down Expand Up @@ -731,13 +787,12 @@ export class ScriptLetContext {
startIndex.comment,
endIndex.comment - startIndex.comment
);
restoreCallback.callback(node, targetTokens, targetComments, {
getScope,
getInnermostScope,
registerNodeToScope,
scopeManager: result.scopeManager!,
visitorKeys: result.visitorKeys,
});
restoreCallback.callback(
node,
targetTokens,
targetComments,
callbackOption
);

processedTokens.push(...targetTokens);
processedComments.push(...targetComments);
Expand All @@ -750,33 +805,22 @@ export class ScriptLetContext {

result.ast.tokens = processedTokens;
result.ast.comments = processedComments;
postprocessList.forEach((p) => p());

// Helpers
/** Get scope */
function getScope(node: ESTree.Node) {
return getScopeFromNode(result.scopeManager!, node);
}

/** Get innermost scope */
function getInnermostScope(node: ESTree.Node) {
return getInnermostScopeFromNode(result.scopeManager!, node);
}

/** Register node to scope */
function registerNodeToScope(node: any, scope: Scope): void {
// If we replace the `scope.block` at this time,
// the scope restore calculation will not work, so we will replace the `scope.block` later.
postprocessList.push(() => {
scope.block = node;
});
}

const scopes = nodeToScope.get(node);
if (scopes) {
scopes.push(scope);
} else {
nodeToScope.set(node, [scope]);
}
/**
* Restore program node
*/
private restoreProgram(
result: ESLintExtendedProgram,
callbackOption: ScriptLetRestoreCallbackOption
): void {
for (const callback of this.programRestoreCallbacks) {
callback(
result.ast,
result.ast.tokens,
result.ast.comments,
callbackOption
);
}
}

Expand Down
25 changes: 25 additions & 0 deletions src/parser/converts/root.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {} from "./common";
import type { Context } from "../../context";
import { convertChildren, extractElementTags } from "./element";
import { convertAttributeTokens } from "./attr";
import type { Scope } from "eslint-scope";

/**
* Convert root
Expand Down Expand Up @@ -127,6 +128,30 @@ export function convertSvelteRoot(
body.push(style);
}

// Set the scope of the Program node.
ctx.scriptLet.addProgramRestore(
(
node,
_tokens,
_comments,
{ scopeManager, registerNodeToScope, addPostProcess }
) => {
const scopes: Scope[] = [];
for (const scope of scopeManager.scopes) {
if (scope.block === node) {
registerNodeToScope(ast, scope);
scopes.push(scope);
}
}
addPostProcess(() => {
// Reverts the node indicated by `block` to the original Program node.
// This state is incorrect, but `eslint-utils`'s `referenceTracker.iterateEsmReferences()` tracks import statements
// from Program nodes set to `block` in global scope. This can only be handled by the original Program node.
scopeManager.globalScope.block = node;
});
}
);

return ast;
}

Expand Down
77 changes: 77 additions & 0 deletions tests/src/scope/scope.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { Linter } from "eslint";
import assert from "assert";
import * as parser from "../../../src/index";
import type { Scope } from "eslint-scope";

function generateScopeTestCase(code: string, selector: string, type: string) {
const linter = new Linter();
let scope: Scope;
linter.defineParser("svelte-eslint-parser", parser as any);
linter.defineRule("test", {
create(context) {
return {
[selector]() {
scope = context.getScope();
},
};
},
});
linter.verify(code, {
parser: "svelte-eslint-parser",
parserOptions: { ecmaVersion: 2020, sourceType: "module" },
rules: {
test: "error",
},
});
assert.strictEqual(scope!.type, type);
}

describe("context.getScope", () => {
it("returns the global scope for the root node", () => {
generateScopeTestCase("", "Program", "global");
});

it("returns the global scope for the script element", () => {
generateScopeTestCase("<script></script>", "SvelteScriptElement", "module");
});

it("returns the module scope for nodes for top level nodes of script", () => {
generateScopeTestCase(
'<script>import mod from "mod";</script>',
"ImportDeclaration",
"module"
);
});

it("returns the module scope for nested nodes without their own scope", () => {
generateScopeTestCase(
"<script>a || b</script>",
"LogicalExpression",
"module"
);
});

it("returns the the child scope of top level nodes with their own scope", () => {
generateScopeTestCase(
"<script>function fn() {}</script>",
"FunctionDeclaration",
"function"
);
});

it("returns the own scope for nested nodes", () => {
generateScopeTestCase(
"<script>a || (() => {})</script>",
"ArrowFunctionExpression",
"function"
);
});

it("returns the the nearest child scope for statements inside non-global scopes", () => {
generateScopeTestCase(
"<script>function fn() { nested; }</script>",
"ExpressionStatement",
"function"
);
});
});

0 comments on commit ec061f5

Please sign in to comment.