Skip to content

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

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
10 changes: 7 additions & 3 deletions src/scope/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type * as ESTree from "estree";
import type { TSESTree } from "@typescript-eslint/types";
import { traverseNodes } from "../traverse";
import { addElementsToSortedArray, addElementToSortedArray } from "../utils";
import type { SvelteHTMLNode } from "../ast";

/** Remove all scope, variable, and reference */
export function removeAllScopeAndVariableAndReference(
Expand Down Expand Up @@ -58,9 +59,9 @@ export function removeAllScopeAndVariableAndReference(
*/
export function getScopeFromNode(
scopeManager: ScopeManager,
currentNode: ESTree.Node
currentNode: ESTree.Node | SvelteHTMLNode
): Scope {
let node: ESTree.Node | null = currentNode;
let node: ESTree.Node | SvelteHTMLNode | null = currentNode;
for (; node; node = (node as any).parent || null) {
const scope = scopeManager.acquire(node, false);
if (scope) {
Expand All @@ -78,7 +79,10 @@ export function getScopeFromNode(
}
}
const global = scopeManager.globalScope;
return global;

return currentNode.type === "Program" || currentNode.type === "SvelteScriptElement" ?
global :
getProgramScope(scopeManager);
}
/**
* Gets the scope for the Program node
Expand Down
61 changes: 61 additions & 0 deletions tests/src/scope/scope.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import assert from "assert";
import { parseForESLint } from "../../../src";
import { getScopeFromNode } from "../../../src/scope";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR.

Can you replace the test with ESLint's context.getScope instead of getScopeFromNode? getScopeFromNode is not used in the actual rule implementation.


describe('getScopeFromNode', () => {
it('returns the global scope for the root node', () => {
const { ast, scopeManager } = parseForESLint('');

assert.strictEqual(getScopeFromNode(scopeManager, ast), scopeManager.globalScope);
});

it('returns the global scope for the script element', () => {
const { ast, scopeManager } = parseForESLint('<script></script>');
const script = ast.body[0];

assert.strictEqual(getScopeFromNode(scopeManager, script), scopeManager.globalScope);
});

it('returns the module scope for nodes for top level nodes of script', () => {
const { ast, scopeManager } = parseForESLint('<script>import mod from "mod";</script>');
const importStatement = ast.body[0].body[0];

assert.strictEqual(getScopeFromNode(scopeManager, importStatement), scopeManager.globalScope.childScopes[0]);
});

it('returns the module scope for nested nodes without their own scope', () => {
const { ast, scopeManager } = parseForESLint('<script>a || b</script>');
const importStatement = ast.body[0].body[0].expression.right;

assert.strictEqual(getScopeFromNode(scopeManager, importStatement), scopeManager.globalScope.childScopes[0]);
});

it('returns the module scope for nested nodes for non-modules', () => {
const { ast, scopeManager } = parseForESLint('<script>a || b</script>', { sourceType: 'script' });
const importStatement = ast.body[0].body[0].expression.right;

assert.strictEqual(getScopeFromNode(scopeManager, importStatement), scopeManager.globalScope.childScopes[0]);
});

it('returns the the child scope of top level nodes with their own scope', () => {
const { ast, scopeManager } = parseForESLint('<script>function fn() {}</script>');
const fnNode = ast.body[0].body[0];

assert.strictEqual(getScopeFromNode(scopeManager, fnNode), scopeManager.globalScope.childScopes[0].childScopes[0]);
});

it('returns the own scope for nested nodes', () => {
const { ast, scopeManager } = parseForESLint('<script>a || (() => {})</script>');
const importStatement = ast.body[0].body[0].expression.right;

assert.strictEqual(getScopeFromNode(scopeManager, importStatement), scopeManager.globalScope.childScopes[0].childScopes[0]);
});

it('returns the the nearest child scope for statements inside non-global scopes', () => {
const { ast, scopeManager } = parseForESLint('<script>function fn() { nested; }</script>');
const fnNode = ast.body[0].body[0];
const nestedStatement = fnNode.body.body[0];

assert.strictEqual(getScopeFromNode(scopeManager, nestedStatement), scopeManager.globalScope.childScopes[0].childScopes[0]);
});
});