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 <milos.djermanovic@gmail.com>

* Update docs/src/extend/custom-rules.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* 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 <milos.djermanovic@gmail.com>

* Fix lint errors

---------

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

docs/src/extend/custom-rules.md

@@ -131,6 +131,7 @@ The `context` object has the following properties:
 * `parserOptions`: The parser options configured for this run (more details [here](../use/configure/language-options#specifying-parser-options)).
 
 Additionally, the `context` object has the following methods:
+
 * `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.
 * `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.
 * `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:
 * `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.
 * `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.
 * `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)).
-* `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`.
+* `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`.
 * `report(descriptor)`. Reports a problem in the code (see the [dedicated section](#reporting-problems)).
 
 **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
 * [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)
 * [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)
 
+### Marking Variables as Used
+
+**Deprecated:** The `context.markVariableAsUsed()` method is deprecated in favor of `sourceCode.markVariableAsUsed()`.
+
+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".
+
+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:
+
+```js
+module.exports = {
+    create: function(context) {
+        var sourceCode = context.getSourceCode();
+
+        return {
+            ReturnStatement(node) {
+
+                // look in the scope of the function for myCustomVar and mark as used
+                sourceCode.markVariableAsUsed("myCustomVar", node);
+
+                // or: look in the global scope for myCustomVar and mark as used
+                sourceCode.markVariableAsUsed("myCustomVar");
+            }
+        }
+        // ...
+    }
+};
+```
+
+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.)
+
 ### Accessing Code Paths
 
 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

@@ -857,38 +857,6 @@ function parse(text, languageOptions, filePath) {
     }
 }
 
-/**
- * Marks a variable as used in the current scope
- * @param {SourceCode} sourceCode The source code for the currently linted file.
- * @param {ASTNode} currentNode The node currently being traversed
- * @param {LanguageOptions} languageOptions The options used to parse this text
- * @param {string} name The name of the variable that should be marked as used.
- * @returns {boolean} True if the variable was found and marked as used, false if not.
- */
-function markVariableAsUsed(sourceCode, currentNode, languageOptions, name) {
-    const parserOptions = languageOptions.parserOptions;
-    const sourceType = languageOptions.sourceType;
-    const hasGlobalReturn =
-        (parserOptions.ecmaFeatures && parserOptions.ecmaFeatures.globalReturn) ||
-        sourceType === "commonjs";
-    const specialScope = hasGlobalReturn || sourceType === "module";
-    const currentScope = sourceCode.getScope(currentNode);
-
-    // Special Node.js scope means we need to start one level deeper
-    const initialScope = currentScope.type === "global" && specialScope ? currentScope.childScopes[0] : currentScope;
-
-    for (let scope = initialScope; scope; scope = scope.upper) {
-        const variable = scope.variables.find(scopeVar => scopeVar.name === name);
-
-        if (variable) {
-            variable.eslintUsed = true;
-            return true;
-        }
-    }
-
-    return false;
-}
-
 /**
  * Runs a rule, and gets its listeners
  * @param {Rule} rule A normalized rule with a `create` method
@@ -987,7 +955,7 @@ function runRules(sourceCode, configuredRules, ruleMapper, parserName, languageO
                 getPhysicalFilename: () => physicalFilename || filename,
                 getScope: () => sourceCode.getScope(currentNode),
                 getSourceCode: () => sourceCode,
-                markVariableAsUsed: name => markVariableAsUsed(sourceCode, currentNode, languageOptions, name),
+                markVariableAsUsed: name => sourceCode.markVariableAsUsed(name, currentNode),
                 parserOptions: {
                     ...languageOptions.parserOptions
                 },

lib/source-code/source-code.js

@@ -681,6 +681,49 @@ class SourceCode extends TokenStore {
     }
     /* eslint-enable class-methods-use-this -- node is owned by SourceCode */
 
+    /**
+     * Marks a variable as used in the current scope
+     * @param {string} name The name of the variable to mark as used.
+     * @param {ASTNode} [refNode] The closest node to the variable reference.
+     * @returns {boolean} True if the variable was found and marked as used, false if not.
+     */
+    markVariableAsUsed(name, refNode = this.ast) {
+
+        const currentScope = this.getScope(refNode);
+        let initialScope = currentScope;
+
+        /*
+         * When we are in an ESM or CommonJS module, we need to start searching
+         * from the top-level scope, not the global scope. For ESM the top-level
+         * scope is the module scope; for CommonJS the top-level scope is the
+         * outer function scope.
+         *
+         * Without this check, we might miss a variable declared with `var` at
+         * the top-level because it won't exist in the global scope.
+         */
+        if (
+            currentScope.type === "global" &&
+            currentScope.childScopes.length > 0 &&
+
+            // top-level scopes refer to a `Program` node
+            currentScope.childScopes[0].block === this.ast
+        ) {
+            initialScope = currentScope.childScopes[0];
+        }
+
+        for (let scope = initialScope; scope; scope = scope.upper) {
+            const variable = scope.variables.find(scopeVar => scopeVar.name === name);
+
+            if (variable) {
+                variable.eslintUsed = true;
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+
 }
 
 module.exports = SourceCode;

package.json

@@ -73,7 +73,7 @@
     "debug": "^4.3.2",
     "doctrine": "^3.0.0",
     "escape-string-regexp": "^4.0.0",
-    "eslint-scope": "^7.1.1",
+    "eslint-scope": "^7.2.0",
     "eslint-visitor-keys": "^3.4.0",
     "espree": "^9.5.1",
     "esquery": "^1.4.2",

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

@@ -21,13 +21,16 @@ const ruleTester = new RuleTester();
 ruleTester.defineRule("use-every-a", {
     create(context) {
 
+        const sourceCode = context.getSourceCode();
+
         /**
          * Mark a variable as used
+         * @param {ASTNode} node The node representing the scope to search
          * @returns {void}
          * @private
          */
-        function useA() {
-            context.markVariableAsUsed("a");
+        function useA(node) {
+            sourceCode.markVariableAsUsed("a", node);
         }
         return {
             VariableDeclaration: useA,

tests/lib/rules/prefer-const.js

@@ -20,11 +20,15 @@ const rule = require("../../../lib/rules/prefer-const"),
 const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
 
 ruleTester.defineRule("use-x", {
-    create: context => ({
-        VariableDeclaration() {
-            context.markVariableAsUsed("x");
-        }
-    })
+    create(context) {
+        const sourceCode = context.getSourceCode();
+
+        return {
+            VariableDeclaration(node) {
+                sourceCode.markVariableAsUsed("x", node);
+            }
+        };
+    }
 });
 
 ruleTester.run("prefer-const", rule, {

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

@@ -33,6 +33,19 @@ const flatLinter = new Linter({ configType: "flat" });
 const AST = espree.parse("let foo = bar;", DEFAULT_CONFIG),
     TEST_CODE = "var answer = 6 * 7;",
     SHEBANG_TEST_CODE = `#!/usr/bin/env node\n${TEST_CODE}`;
+const filename = "foo.js";
+
+/**
+ * Get variables in the current scope
+ * @param {Object} scope current scope
+ * @param {string} name name of the variable to look for
+ * @returns {ASTNode|null} The variable object
+ * @private
+ */
+function getVariable(scope, name) {
+    return scope.variables.find(v => v.name === name) || null;
+}
+
 
 //------------------------------------------------------------------------------
 // Tests
@@ -3287,7 +3300,6 @@ describe("SourceCode", () => {
 
     describe("getAncestors()", () => {
         const code = TEST_CODE;
-        const filename = "foo.js";
 
         it("should retrieve all ancestors when used", () => {
 
@@ -3623,4 +3635,156 @@ describe("SourceCode", () => {
         });
     });
 
+    describe("markVariableAsUsed()", () => {
+
+        it("should mark variables in current scope as used", () => {
+            const code = "var a = 1, b = 2;";
+            let spy;
+
+            linter.defineRule("checker", {
+                create(context) {
+                    const sourceCode = context.getSourceCode();
+
+                    spy = sinon.spy(() => {
+                        assert.isTrue(sourceCode.markVariableAsUsed("a"));
+
+                        const scope = context.getScope();
+
+                        assert.isTrue(getVariable(scope, "a").eslintUsed);
+                        assert.notOk(getVariable(scope, "b").eslintUsed);
+                    });
+
+                    return { "Program:exit": spy };
+                }
+            });
+
+            linter.verify(code, { rules: { checker: "error" } });
+            assert(spy && spy.calledOnce);
+        });
+
+        it("should mark variables in function args as used", () => {
+            const code = "function abc(a, b) { return 1; }";
+            let spy;
+
+            linter.defineRule("checker", {
+                create(context) {
+                    const sourceCode = context.getSourceCode();
+
+                    spy = sinon.spy(node => {
+                        assert.isTrue(sourceCode.markVariableAsUsed("a", node));
+
+                        const scope = context.getScope();
+
+                        assert.isTrue(getVariable(scope, "a").eslintUsed);
+                        assert.notOk(getVariable(scope, "b").eslintUsed);
+                    });
+
+                    return { ReturnStatement: spy };
+                }
+            });
+
+            linter.verify(code, { rules: { checker: "error" } });
+            assert(spy && spy.calledOnce);
+        });
+
+        it("should mark variables in higher scopes as used", () => {
+            const code = "var a, b; function abc() { return 1; }";
+            let returnSpy, exitSpy;
+
+            linter.defineRule("checker", {
+                create(context) {
+                    const sourceCode = context.getSourceCode();
+
+                    returnSpy = sinon.spy(node => {
+                        assert.isTrue(sourceCode.markVariableAsUsed("a", node));
+                    });
+                    exitSpy = sinon.spy(() => {
+                        const scope = context.getScope();
+
+                        assert.isTrue(getVariable(scope, "a").eslintUsed);
+                        assert.notOk(getVariable(scope, "b").eslintUsed);
+                    });
+
+                    return { ReturnStatement: returnSpy, "Program:exit": exitSpy };
+                }
+            });
+
+            linter.verify(code, { rules: { checker: "error" } });
+            assert(returnSpy && returnSpy.calledOnce);
+            assert(exitSpy && exitSpy.calledOnce);
+        });
+
+        it("should mark variables in Node.js environment as used", () => {
+            const code = "var a = 1, b = 2;";
+            let spy;
+
+            linter.defineRule("checker", {
+                create(context) {
+                    const sourceCode = context.getSourceCode();
+
+                    spy = sinon.spy(() => {
+                        const globalScope = context.getScope(),
+                            childScope = globalScope.childScopes[0];
+
+                        assert.isTrue(sourceCode.markVariableAsUsed("a"));
+
+                        assert.isTrue(getVariable(childScope, "a").eslintUsed);
+                        assert.isUndefined(getVariable(childScope, "b").eslintUsed);
+                    });
+
+                    return { "Program:exit": spy };
+                }
+            });
+
+            linter.verify(code, { rules: { checker: "error" }, env: { node: true } });
+            assert(spy && spy.calledOnce);
+        });
+
+        it("should mark variables in modules as used", () => {
+            const code = "var a = 1, b = 2;";
+            let spy;
+
+            linter.defineRule("checker", {
+                create(context) {
+                    const sourceCode = context.getSourceCode();
+
+                    spy = sinon.spy(() => {
+                        const globalScope = context.getScope(),
+                            childScope = globalScope.childScopes[0];
+
+                        assert.isTrue(sourceCode.markVariableAsUsed("a"));
+
+                        assert.isTrue(getVariable(childScope, "a").eslintUsed);
+                        assert.isUndefined(getVariable(childScope, "b").eslintUsed);
+                    });
+
+                    return { "Program:exit": spy };
+                }
+            });
+
+            linter.verify(code, { rules: { checker: "error" }, parserOptions: { ecmaVersion: 6, sourceType: "module" } }, filename, true);
+            assert(spy && spy.calledOnce);
+        });
+
+        it("should return false if the given variable is not found", () => {
+            const code = "var a = 1, b = 2;";
+            let spy;
+
+            linter.defineRule("checker", {
+                create(context) {
+                    const sourceCode = context.getSourceCode();
+
+                    spy = sinon.spy(() => {
+                        assert.isFalse(sourceCode.markVariableAsUsed("c"));
+                    });
+
+                    return { "Program:exit": spy };
+                }
+            });
+
+            linter.verify(code, { rules: { checker: "error" } });
+            assert(spy && spy.calledOnce);
+        });
+
+    });
 });