Skip to content

Commit 3f7af9f

Browse files
nzakasmdjermanovic
nzakas
and
mdjermanovic
authored
feat: Implement SourceCode#markVariableAsUsed() (#17086)
* feat: Implement `SourceCode#markVariableAsUsed()` Implements `SourceCode#markVariableAsUsed()` while leaving `context.markVariableAsUsed()` alone. Refs #16999 * Refactor markVariableAsUsed * Finish refactor * Update docs/src/extend/custom-rules.md Co-authored-by: Milos Djermanovic <[email protected]> * Update docs/src/extend/custom-rules.md Co-authored-by: Milos Djermanovic <[email protected]> * Update docs/src/extend/custom-rules.md * Update logic to eliminate scopeManager dependency * Update docs/src/extend/custom-rules.md Co-authored-by: Milos Djermanovic <[email protected]> * Fix lint errors --------- Co-authored-by: Milos Djermanovic <[email protected]>
1 parent d8e9887 commit 3f7af9f

7 files changed

Lines changed: 256 additions & 43 deletions

File tree

‎docs/src/extend/custom-rules.md‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ The `context` object has the following properties:
131131
* `parserOptions`: The parser options configured for this run (more details [here](../use/configure/language-options#specifying-parser-options)).
132132

133133
Additionally, the `context` object has the following methods:
134+
134135
* `getAncestors()` - (**Deprecated:** Use `SourceCode#getAncestors(node)` instead.) Returns an array of the ancestors of the currently-traversed node, starting at the root of the AST and continuing through the direct parent of the current node. This array does not include the currently-traversed node itself.
135136
* `getCwd()` - Returns the `cwd` option passed to the [Linter](../integrate/nodejs-api#linter). It is a path to a directory that should be considered the current working directory.
136137
* `getDeclaredVariables(node)` - (**Deprecated:** Use `SourceCode#getDeclaredVariables(node)` instead.) Returns a list of [variables](./scope-manager-interface#variable-interface) declared by the given node. This information can be used to track references to variables.
@@ -147,7 +148,7 @@ Additionally, the `context` object has the following methods:
147148
* `getPhysicalFilename()`: When linting a file, it returns the full path of the file on disk without any code block information. When linting text, it returns the value passed to `—stdin-filename` or `<text>` if not specified.
148149
* `getScope()`: (**Deprecated:** Use `SourceCode#getScope(node)` instead.) Returns the [scope](./scope-manager-interface#scope-interface) of the currently-traversed node. This information can be used to track references to variables.
149150
* `getSourceCode()`: Returns a `SourceCode` object that you can use to work with the source that was passed to ESLint (see [Accessing the Source Code](#accessing-the-source-code)).
150-
* `markVariableAsUsed(name)`: Marks a variable with the given name in the current scope as used. This affects the [no-unused-vars](../rules/no-unused-vars) rule. Returns `true` if a variable with the given name was found and marked as used, otherwise `false`.
151+
* `markVariableAsUsed(name)` - (**Deprecated:** Use `SourceCode#markVariableAsUsed(name, node)` instead.) Marks a variable with the given name in the current scope as used. This affects the [no-unused-vars](../rules/no-unused-vars) rule. Returns `true` if a variable with the given name was found and marked as used, otherwise `false`.
151152
* `report(descriptor)`. Reports a problem in the code (see the [dedicated section](#reporting-problems)).
152153

153154
**Note:** Earlier versions of ESLint supported additional methods on the `context` object. Those methods were removed in the new format and should not be relied upon.
@@ -695,6 +696,36 @@ For examples of using `SourceCode#getScope()` to track variables, refer to the s
695696
* [no-shadow](https://github.com/eslint/eslint/blob/main/lib/rules/no-shadow.js): Calls `sourceCode.getScope()` at the `Program` node and inspects all child scopes to make sure a variable name is not reused at a lower scope. ([no-shadow](../rules/no-shadow) documentation)
696697
* [no-redeclare](https://github.com/eslint/eslint/blob/main/lib/rules/no-redeclare.js): Calls `sourceCode.getScope()` at each scope to make sure that a variable is not declared twice in the same scope. ([no-redeclare](../rules/no-redeclare) documentation)
697698

699+
### Marking Variables as Used
700+
701+
**Deprecated:** The `context.markVariableAsUsed()` method is deprecated in favor of `sourceCode.markVariableAsUsed()`.
702+
703+
Certain ESLint rules, such as [`no-unused-vars`](../rules/no-unused-vars), check to see if a variable has been used. ESLint itself only knows about the standard rules of variable access and so custom ways of accessing variables may not register as "used".
704+
705+
To help with this, you can use the `sourceCode.markVariableAsUsed()` method. This method takes two arguments: the name of the variable to mark as used and an option reference node indicating the scope in which you are working. Here's an example:
706+
707+
```js
708+
module.exports = {
709+
create: function(context) {
710+
var sourceCode = context.getSourceCode();
711+
712+
return {
713+
ReturnStatement(node) {
714+
715+
// look in the scope of the function for myCustomVar and mark as used
716+
sourceCode.markVariableAsUsed("myCustomVar", node);
717+
718+
// or: look in the global scope for myCustomVar and mark as used
719+
sourceCode.markVariableAsUsed("myCustomVar");
720+
}
721+
}
722+
// ...
723+
}
724+
};
725+
```
726+
727+
Here, the `myCustomVar` variable is marked as used relative to a `ReturnStatement` node, which means ESLint will start searching from the scope closest to that node. If you omit the second argument, then the top-level scope is used. (For ESM files, the top-level scope is the module scope; for CommonJS files, the top-level scope is the first function scope.)
728+
698729
### Accessing Code Paths
699730

700731
ESLint analyzes code paths while traversing AST. You can access code path objects with five events related to code paths. For more information, refer to [Code Path Analysis](code-path-analysis).

‎lib/linter/linter.js‎

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -857,38 +857,6 @@ function parse(text, languageOptions, filePath) {
857857
}
858858
}
859859

860-
/**
861-
* Marks a variable as used in the current scope
862-
* @param {SourceCode} sourceCode The source code for the currently linted file.
863-
* @param {ASTNode} currentNode The node currently being traversed
864-
* @param {LanguageOptions} languageOptions The options used to parse this text
865-
* @param {string} name The name of the variable that should be marked as used.
866-
* @returns {boolean} True if the variable was found and marked as used, false if not.
867-
*/
868-
function markVariableAsUsed(sourceCode, currentNode, languageOptions, name) {
869-
const parserOptions = languageOptions.parserOptions;
870-
const sourceType = languageOptions.sourceType;
871-
const hasGlobalReturn =
872-
(parserOptions.ecmaFeatures && parserOptions.ecmaFeatures.globalReturn) ||
873-
sourceType === "commonjs";
874-
const specialScope = hasGlobalReturn || sourceType === "module";
875-
const currentScope = sourceCode.getScope(currentNode);
876-
877-
// Special Node.js scope means we need to start one level deeper
878-
const initialScope = currentScope.type === "global" && specialScope ? currentScope.childScopes[0] : currentScope;
879-
880-
for (let scope = initialScope; scope; scope = scope.upper) {
881-
const variable = scope.variables.find(scopeVar => scopeVar.name === name);
882-
883-
if (variable) {
884-
variable.eslintUsed = true;
885-
return true;
886-
}
887-
}
888-
889-
return false;
890-
}
891-
892860
/**
893861
* Runs a rule, and gets its listeners
894862
* @param {Rule} rule A normalized rule with a `create` method
@@ -987,7 +955,7 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserName, languageO
987955
getPhysicalFilename: () => physicalFilename || filename,
988956
getScope: () => sourceCode.getScope(currentNode),
989957
getSourceCode: () => sourceCode,
990-
markVariableAsUsed: name => markVariableAsUsed(sourceCode, currentNode, languageOptions, name),
958+
markVariableAsUsed: name => sourceCode.markVariableAsUsed(name, currentNode),
991959
parserOptions: {
992960
...languageOptions.parserOptions
993961
},

‎lib/source-code/source-code.js‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,49 @@ class SourceCode extends TokenStore {
681681
}
682682
/* eslint-enable class-methods-use-this -- node is owned by SourceCode */
683683

684+
/**
685+
* Marks a variable as used in the current scope
686+
* @param {string} name The name of the variable to mark as used.
687+
* @param {ASTNode} [refNode] The closest node to the variable reference.
688+
* @returns {boolean} True if the variable was found and marked as used, false if not.
689+
*/
690+
markVariableAsUsed(name, refNode = this.ast) {
691+
692+
const currentScope = this.getScope(refNode);
693+
let initialScope = currentScope;
694+
695+
/*
696+
* When we are in an ESM or CommonJS module, we need to start searching
697+
* from the top-level scope, not the global scope. For ESM the top-level
698+
* scope is the module scope; for CommonJS the top-level scope is the
699+
* outer function scope.
700+
*
701+
* Without this check, we might miss a variable declared with `var` at
702+
* the top-level because it won't exist in the global scope.
703+
*/
704+
if (
705+
currentScope.type === "global" &&
706+
currentScope.childScopes.length > 0 &&
707+
708+
// top-level scopes refer to a `Program` node
709+
currentScope.childScopes[0].block === this.ast
710+
) {
711+
initialScope = currentScope.childScopes[0];
712+
}
713+
714+
for (let scope = initialScope; scope; scope = scope.upper) {
715+
const variable = scope.variables.find(scopeVar => scopeVar.name === name);
716+
717+
if (variable) {
718+
variable.eslintUsed = true;
719+
return true;
720+
}
721+
}
722+
723+
return false;
724+
}
725+
726+
684727
}
685728

686729
module.exports = SourceCode;

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@
7373
"debug": "^4.3.2",
7474
"doctrine": "^3.0.0",
7575
"escape-string-regexp": "^4.0.0",
76-
"eslint-scope": "^7.1.1",
76+
"eslint-scope": "^7.2.0",
7777
"eslint-visitor-keys": "^3.4.0",
7878
"espree": "^9.5.1",
7979
"esquery": "^1.4.2",

‎tests/lib/rules/no-unused-vars.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,16 @@ const ruleTester = new RuleTester();
2121
ruleTester.defineRule("use-every-a", {
2222
create(context) {
2323

24+
const sourceCode = context.getSourceCode();
25+
2426
/**
2527
* Mark a variable as used
28+
* @param {ASTNode} node The node representing the scope to search
2629
* @returns {void}
2730
* @private
2831
*/
29-
function useA() {
30-
context.markVariableAsUsed("a");
32+
function useA(node) {
33+
sourceCode.markVariableAsUsed("a", node);
3134
}
3235
return {
3336
VariableDeclaration: useA,

‎tests/lib/rules/prefer-const.js‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,15 @@ const rule = require("../../../lib/rules/prefer-const"),
2020
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
2121

2222
ruleTester.defineRule("use-x", {
23-
create: context => ({
24-
VariableDeclaration() {
25-
context.markVariableAsUsed("x");
26-
}
27-
})
23+
create(context) {
24+
const sourceCode = context.getSourceCode();
25+
26+
return {
27+
VariableDeclaration(node) {
28+
sourceCode.markVariableAsUsed("x", node);
29+
}
30+
};
31+
}
2832
});
2933

3034
ruleTester.run("prefer-const", rule, {

‎tests/lib/source-code/source-code.js‎

Lines changed: 165 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,19 @@ const flatLinter = new Linter({ configType: "flat" });
3333
const AST = espree.parse("let foo = bar;", DEFAULT_CONFIG),
3434
TEST_CODE = "var answer = 6 * 7;",
3535
SHEBANG_TEST_CODE = `#!/usr/bin/env node\n${TEST_CODE}`;
36+
const filename = "foo.js";
37+
38+
/**
39+
* Get variables in the current scope
40+
* @param {Object} scope current scope
41+
* @param {string} name name of the variable to look for
42+
* @returns {ASTNode|null} The variable object
43+
* @private
44+
*/
45+
function getVariable(scope, name) {
46+
return scope.variables.find(v => v.name === name) || null;
47+
}
48+
3649

3750
//------------------------------------------------------------------------------
3851
// Tests
@@ -3287,7 +3300,6 @@ describe("SourceCode", () => {
32873300

32883301
describe("getAncestors()", () => {
32893302
const code = TEST_CODE;
3290-
const filename = "foo.js";
32913303

32923304
it("should retrieve all ancestors when used", () => {
32933305

@@ -3623,4 +3635,156 @@ describe("SourceCode", () => {
36233635
});
36243636
});
36253637

3638+
describe("markVariableAsUsed()", () => {
3639+
3640+
it("should mark variables in current scope as used", () => {
3641+
const code = "var a = 1, b = 2;";
3642+
let spy;
3643+
3644+
linter.defineRule("checker", {
3645+
create(context) {
3646+
const sourceCode = context.getSourceCode();
3647+
3648+
spy = sinon.spy(() => {
3649+
assert.isTrue(sourceCode.markVariableAsUsed("a"));
3650+
3651+
const scope = context.getScope();
3652+
3653+
assert.isTrue(getVariable(scope, "a").eslintUsed);
3654+
assert.notOk(getVariable(scope, "b").eslintUsed);
3655+
});
3656+
3657+
return { "Program:exit": spy };
3658+
}
3659+
});
3660+
3661+
linter.verify(code, { rules: { checker: "error" } });
3662+
assert(spy && spy.calledOnce);
3663+
});
3664+
3665+
it("should mark variables in function args as used", () => {
3666+
const code = "function abc(a, b) { return 1; }";
3667+
let spy;
3668+
3669+
linter.defineRule("checker", {
3670+
create(context) {
3671+
const sourceCode = context.getSourceCode();
3672+
3673+
spy = sinon.spy(node => {
3674+
assert.isTrue(sourceCode.markVariableAsUsed("a", node));
3675+
3676+
const scope = context.getScope();
3677+
3678+
assert.isTrue(getVariable(scope, "a").eslintUsed);
3679+
assert.notOk(getVariable(scope, "b").eslintUsed);
3680+
});
3681+
3682+
return { ReturnStatement: spy };
3683+
}
3684+
});
3685+
3686+
linter.verify(code, { rules: { checker: "error" } });
3687+
assert(spy && spy.calledOnce);
3688+
});
3689+
3690+
it("should mark variables in higher scopes as used", () => {
3691+
const code = "var a, b; function abc() { return 1; }";
3692+
let returnSpy, exitSpy;
3693+
3694+
linter.defineRule("checker", {
3695+
create(context) {
3696+
const sourceCode = context.getSourceCode();
3697+
3698+
returnSpy = sinon.spy(node => {
3699+
assert.isTrue(sourceCode.markVariableAsUsed("a", node));
3700+
});
3701+
exitSpy = sinon.spy(() => {
3702+
const scope = context.getScope();
3703+
3704+
assert.isTrue(getVariable(scope, "a").eslintUsed);
3705+
assert.notOk(getVariable(scope, "b").eslintUsed);
3706+
});
3707+
3708+
return { ReturnStatement: returnSpy, "Program:exit": exitSpy };
3709+
}
3710+
});
3711+
3712+
linter.verify(code, { rules: { checker: "error" } });
3713+
assert(returnSpy && returnSpy.calledOnce);
3714+
assert(exitSpy && exitSpy.calledOnce);
3715+
});
3716+
3717+
it("should mark variables in Node.js environment as used", () => {
3718+
const code = "var a = 1, b = 2;";
3719+
let spy;
3720+
3721+
linter.defineRule("checker", {
3722+
create(context) {
3723+
const sourceCode = context.getSourceCode();
3724+
3725+
spy = sinon.spy(() => {
3726+
const globalScope = context.getScope(),
3727+
childScope = globalScope.childScopes[0];
3728+
3729+
assert.isTrue(sourceCode.markVariableAsUsed("a"));
3730+
3731+
assert.isTrue(getVariable(childScope, "a").eslintUsed);
3732+
assert.isUndefined(getVariable(childScope, "b").eslintUsed);
3733+
});
3734+
3735+
return { "Program:exit": spy };
3736+
}
3737+
});
3738+
3739+
linter.verify(code, { rules: { checker: "error" }, env: { node: true } });
3740+
assert(spy && spy.calledOnce);
3741+
});
3742+
3743+
it("should mark variables in modules as used", () => {
3744+
const code = "var a = 1, b = 2;";
3745+
let spy;
3746+
3747+
linter.defineRule("checker", {
3748+
create(context) {
3749+
const sourceCode = context.getSourceCode();
3750+
3751+
spy = sinon.spy(() => {
3752+
const globalScope = context.getScope(),
3753+
childScope = globalScope.childScopes[0];
3754+
3755+
assert.isTrue(sourceCode.markVariableAsUsed("a"));
3756+
3757+
assert.isTrue(getVariable(childScope, "a").eslintUsed);
3758+
assert.isUndefined(getVariable(childScope, "b").eslintUsed);
3759+
});
3760+
3761+
return { "Program:exit": spy };
3762+
}
3763+
});
3764+
3765+
linter.verify(code, { rules: { checker: "error" }, parserOptions: { ecmaVersion: 6, sourceType: "module" } }, filename, true);
3766+
assert(spy && spy.calledOnce);
3767+
});
3768+
3769+
it("should return false if the given variable is not found", () => {
3770+
const code = "var a = 1, b = 2;";
3771+
let spy;
3772+
3773+
linter.defineRule("checker", {
3774+
create(context) {
3775+
const sourceCode = context.getSourceCode();
3776+
3777+
spy = sinon.spy(() => {
3778+
assert.isFalse(sourceCode.markVariableAsUsed("c"));
3779+
});
3780+
3781+
return { "Program:exit": spy };
3782+
}
3783+
});
3784+
3785+
linter.verify(code, { rules: { checker: "error" } });
3786+
assert(spy && spy.calledOnce);
3787+
});
3788+
3789+
});
36263790
});

0 commit comments

Comments
 (0)