Skip to content

Commit c95951f

Browse files
overlookmoteloverlookmotel
committed
feat(linter/plugins): implement sourceCode.markVariableAsUsed (#20357)
Fixes #19971. Implement `sourceCode.markVariableAsUsed`. It has some shortcomings at present - it does mark vars as used, so any other JS plugins can see that, but it doesn't communicate this info back to Rust side, so `no-unused-vars` doesn't act on it. But at least it doesn't cause a crash with "unimplemented" error. The remaining issue is tracked in #20350.
1 parent 6eb5b01 commit c95951f

File tree

6 files changed

+387
-7
lines changed

6 files changed

+387
-7
lines changed

‎apps/oxlint/src-js/plugins/scope.ts‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -407,14 +407,53 @@ export function getScope(node: ESTree.Node): Scope {
407407

408408
/**
409409
* Marks as used a variable with the given name in a scope indicated by the given reference node.
410-
* This affects the `no-unused-vars` rule.
410+
*
411+
* IMPORTANT: At present marking variables as used only affects other JS plugins.
412+
* It does *not* get communicated to Oxlint's rules which are implemented on Rust side e.g. `no-unused-vars`.
413+
* This is a known shortcoming, and will be addressed in a future release.
414+
* https://github.com/oxc-project/oxc/issues/20350
415+
*
411416
* @param name - Variable name
412-
* @param refNode - Reference node
417+
* @param refNode - Reference node. Defaults to `Program` node if not provided.
413418
* @returns `true` if a variable with the given name was found and marked as used, otherwise `false`
414419
*/
415-
/* oxlint-disable no-unused-vars */
416-
export function markVariableAsUsed(name: string, refNode: ESTree.Node): boolean {
417-
// TODO: Implement
418-
throw new Error("`sourceCode.markVariableAsUsed` not implemented yet");
420+
export function markVariableAsUsed(name: string, refNode?: ESTree.Node): boolean {
421+
// ref: https://github.com/eslint/eslint/blob/e7cda3bdf1bdd664e6033503a3315ad81736b200/lib/languages/js/source-code/source-code.js#L984-L1024
422+
if (refNode === undefined) {
423+
if (ast === null) initAst();
424+
debugAssertIsNonNull(ast);
425+
refNode = ast;
426+
}
427+
428+
let currentScope = getScope(refNode);
429+
430+
// `getScope` calls `initTsScopeManager` which calls `initAst`, so `ast` must have been initialized
431+
debugAssertIsNonNull(ast);
432+
433+
// When in the global scope, check if there's a module/function child scope whose `block` is the Program node.
434+
// In ESM, top-level `var` declarations live in the module scope, not the global scope.
435+
// In CommonJS, they live in the outer function scope. If we don't step down into that child scope,
436+
// we'd miss those variables.
437+
if (currentScope.type === "global") {
438+
const { childScopes } = currentScope;
439+
if (childScopes.length !== 0) {
440+
// Top-level scopes refer to a `Program` node
441+
const firstChild = childScopes[0];
442+
if (firstChild.block === ast) currentScope = firstChild;
443+
}
444+
}
445+
446+
for (let scope: Scope | null = currentScope; scope !== null; scope = scope.upper) {
447+
const { variables } = scope;
448+
for (let i = 0, len = variables.length; i < len; i++) {
449+
const variable = variables[i];
450+
if (variable.name === name) {
451+
// @ts-expect-error - `eslintUsed` is a dynamic property not in TS-ESLint's types
452+
variable.eslintUsed = true;
453+
return true;
454+
}
455+
}
456+
}
457+
458+
return false;
419459
}
420-
/* oxlint-enable no-unused-vars */

‎apps/oxlint/test/fixtures/sourceCode_markVariableAsUsed/.oxlintrc.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"jsPlugins": ["./plugin.ts"],
3+
"categories": {
4+
"correctness": "off"
5+
},
6+
"rules": {
7+
"mark-used-plugin/mark-used": "error"
8+
}
9+
}

‎apps/oxlint/test/fixtures/sourceCode_markVariableAsUsed/files/index.cjs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// `unusedTopLevel` is declared but never referenced in code,
2+
// so `eslintUsed` should start as `false`.
3+
const unusedTopLevel = "top";
4+
const unusedTopLevel2 = "top2";
5+
6+
// `shadowedName` exists at module scope AND inside `inner`.
7+
// Used to test that omitting `refNode` finds the module-scope one,
8+
// and that a name can be in scope or out of scope depending on `refNode`.
9+
const shadowedName = "module-level";
10+
11+
function outer(param) {
12+
// `nestedVar` is only in `outer`'s scope
13+
const nestedVar = param;
14+
const nestedVar2 = param + 1;
15+
16+
function inner() {
17+
// Shadows module-level `shadowedName`
18+
const shadowedName = "inner-level";
19+
return shadowedName;
20+
}
21+
22+
return nestedVar + inner();
23+
}
24+
25+
module.exports = outer(unusedTopLevel);

‎apps/oxlint/test/fixtures/sourceCode_markVariableAsUsed/files/index.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// `unusedTopLevel` is declared but never referenced in code,
2+
// so `eslintUsed` should start as `false`.
3+
const unusedTopLevel = "top";
4+
const unusedTopLevel2 = "top2";
5+
6+
// `shadowedName` exists at module scope AND inside `inner`.
7+
// Used to test that omitting `refNode` finds the module-scope one,
8+
// and that a name can be in scope or out of scope depending on `refNode`.
9+
const shadowedName = "module-level";
10+
11+
function outer(param) {
12+
// `nestedVar` is only in `outer`'s scope
13+
const nestedVar = param;
14+
const nestedVar2 = param + 1;
15+
16+
function inner() {
17+
// Shadows module-level `shadowedName`
18+
const shadowedName = "inner-level";
19+
return shadowedName;
20+
}
21+
22+
return nestedVar + inner();
23+
}
24+
25+
export default outer(unusedTopLevel);

‎apps/oxlint/test/fixtures/sourceCode_markVariableAsUsed/output.snap.md‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
# Exit code
2+
1
3+
4+
# stdout
5+
```
6+
x mark-used-plugin(mark-used): [1] mark `unusedTopLevel` from Program:
7+
| before: false
8+
| result: true
9+
| after: true
10+
,-[files/index.cjs:1:1]
11+
1 | // `unusedTopLevel` is declared but never referenced in code,
12+
: ^
13+
2 | // so `eslintUsed` should start as `false`.
14+
`----
15+
16+
x mark-used-plugin(mark-used): [2] mark `nonExistent` from Program:
17+
| result: false
18+
,-[files/index.cjs:1:1]
19+
1 | // `unusedTopLevel` is declared but never referenced in code,
20+
: ^
21+
2 | // so `eslintUsed` should start as `false`.
22+
`----
23+
24+
x mark-used-plugin(mark-used): [3] mark `shadowedName` (no refNode):
25+
| before: false
26+
| result: true
27+
| after: true
28+
,-[files/index.cjs:1:1]
29+
1 | // `unusedTopLevel` is declared but never referenced in code,
30+
: ^
31+
2 | // so `eslintUsed` should start as `false`.
32+
`----
33+
34+
x mark-used-plugin(mark-used): [4] mark `nestedVar` from outer:
35+
| before: false
36+
| result: true
37+
| after: true
38+
,-[files/index.cjs:1:1]
39+
1 | // `unusedTopLevel` is declared but never referenced in code,
40+
: ^
41+
2 | // so `eslintUsed` should start as `false`.
42+
`----
43+
44+
x mark-used-plugin(mark-used): [5] mark `unusedTopLevel2` from outer:
45+
| before: false
46+
| result: true
47+
| after: true
48+
,-[files/index.cjs:1:1]
49+
1 | // `unusedTopLevel` is declared but never referenced in code,
50+
: ^
51+
2 | // so `eslintUsed` should start as `false`.
52+
`----
53+
54+
x mark-used-plugin(mark-used): [6] mark `nestedVar2` from inner:
55+
| before: false
56+
| result: true
57+
| after: true
58+
,-[files/index.cjs:1:1]
59+
1 | // `unusedTopLevel` is declared but never referenced in code,
60+
: ^
61+
2 | // so `eslintUsed` should start as `false`.
62+
`----
63+
64+
x mark-used-plugin(mark-used): [7] mark `doesNotExist` from inner:
65+
| result: false
66+
,-[files/index.cjs:1:1]
67+
1 | // `unusedTopLevel` is declared but never referenced in code,
68+
: ^
69+
2 | // so `eslintUsed` should start as `false`.
70+
`----
71+
72+
x mark-used-plugin(mark-used): [1] mark `unusedTopLevel` from Program:
73+
| before: false
74+
| result: true
75+
| after: true
76+
,-[files/index.js:1:1]
77+
1 | // `unusedTopLevel` is declared but never referenced in code,
78+
: ^
79+
2 | // so `eslintUsed` should start as `false`.
80+
`----
81+
82+
x mark-used-plugin(mark-used): [2] mark `nonExistent` from Program:
83+
| result: false
84+
,-[files/index.js:1:1]
85+
1 | // `unusedTopLevel` is declared but never referenced in code,
86+
: ^
87+
2 | // so `eslintUsed` should start as `false`.
88+
`----
89+
90+
x mark-used-plugin(mark-used): [3] mark `shadowedName` (no refNode):
91+
| before: false
92+
| result: true
93+
| after: true
94+
,-[files/index.js:1:1]
95+
1 | // `unusedTopLevel` is declared but never referenced in code,
96+
: ^
97+
2 | // so `eslintUsed` should start as `false`.
98+
`----
99+
100+
x mark-used-plugin(mark-used): [4] mark `nestedVar` from outer:
101+
| before: false
102+
| result: true
103+
| after: true
104+
,-[files/index.js:1:1]
105+
1 | // `unusedTopLevel` is declared but never referenced in code,
106+
: ^
107+
2 | // so `eslintUsed` should start as `false`.
108+
`----
109+
110+
x mark-used-plugin(mark-used): [5] mark `unusedTopLevel2` from outer:
111+
| before: false
112+
| result: true
113+
| after: true
114+
,-[files/index.js:1:1]
115+
1 | // `unusedTopLevel` is declared but never referenced in code,
116+
: ^
117+
2 | // so `eslintUsed` should start as `false`.
118+
`----
119+
120+
x mark-used-plugin(mark-used): [6] mark `nestedVar2` from inner:
121+
| before: false
122+
| result: true
123+
| after: true
124+
,-[files/index.js:1:1]
125+
1 | // `unusedTopLevel` is declared but never referenced in code,
126+
: ^
127+
2 | // so `eslintUsed` should start as `false`.
128+
`----
129+
130+
x mark-used-plugin(mark-used): [7] mark `doesNotExist` from inner:
131+
| result: false
132+
,-[files/index.js:1:1]
133+
1 | // `unusedTopLevel` is declared but never referenced in code,
134+
: ^
135+
2 | // so `eslintUsed` should start as `false`.
136+
`----
137+
138+
Found 0 warnings and 14 errors.
139+
Finished in Xms on 2 files with 1 rules using X threads.
140+
```
141+
142+
# stderr
143+
```
144+
```

0 commit comments

Comments
 (0)