Skip to content
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

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

Merged
merged 2 commits into from
Mar 8, 2023
Merged
Show file tree
Hide file tree
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
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"
);
});
});