Skip to content

fix: resolve to module scope for top level statements #292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 8, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
@@ -58,6 +58,7 @@ type ScriptLetRestoreCallbackOption = {
registerNodeToScope: (node: any, scope: Scope) => void;
scopeManager: ScopeManager;
visitorKeys?: { [type: string]: string[] };
addPostProcess: (callback: () => void) => void;
};

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

private readonly restoreCallbacks: RestoreCallback[] = [];

private readonly programRestoreCallbacks: ScriptLetRestoreCallback[] = [];

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

private readonly unique = new UniqueIdGenerator();
@@ -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,
@@ -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;
@@ -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())) {
@@ -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);
@@ -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
);
}
}

25 changes: 25 additions & 0 deletions src/parser/converts/root.ts
Original file line number Diff line number Diff line change
@@ -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
@@ -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;
}

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"
);
});
});