Skip to content

Conversation

@lilnasy
Copy link
Contributor

@lilnasy lilnasy commented Oct 22, 2025

  • "For now" implementation of Linter plugins: Scope analysis #14827
  • Uses @typescript-eslint/scope-manager which works thanks to recent efforts to align JS side AST with ESTree.
  • @typescript-eslint/scope-manager is well-built: It is factored out from ts-eslint neatly, and is pure scope analysis with minimal dependencies. Tsconfig resolution and syntax parsing is done by its dependents.

Tasks

  • ScopeManager class instance at sourceCode.scopeManager
  • Direct usage APIs: sourceCode.isGlobalReference, sourceCode.getDeclaredVariables, sourceCode.getScope, sourceCode.markVariableAsUsed
    • Tests
  • Limit API surface. We want maneuverability for our eventual implementation, so we should limit the exposed methods to the only two methods documented as non-deprecated: ScopeManager | ESLint.
  • Limiting other interfaces will probably need monkey patching; only public types are limited instead.

ScopeManager API real world use examples

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI C-enhancement Category - New feature or request labels Oct 22, 2025
@camc314 camc314 self-assigned this Oct 22, 2025
@lilnasy lilnasy force-pushed the feat/linter/plugins/scope-manager branch 3 times, most recently from e019ac5 to 307530d Compare October 22, 2025 12:30
@lilnasy lilnasy marked this pull request as ready for review October 22, 2025 14:33
@lilnasy lilnasy requested a review from camc314 as a code owner October 22, 2025 14:33
Copilot AI review requested due to automatic review settings October 22, 2025 14:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a scope manager API for the linter plugin system using @typescript-eslint/scope-manager. It provides scope analysis capabilities through the sourceCode.scopeManager property and helper methods like isGlobalReference, getDeclaredVariables, getScope, and markVariableAsUsed.

Key Changes:

  • Added @typescript-eslint/scope-manager package dependency for scope analysis
  • Implemented ScopeManager class that wraps the TypeScript ESLint scope manager with a limited API surface
  • Added scope-related helper methods to sourceCode for direct usage in plugins

Reviewed Changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
apps/oxlint/package.json Added dependencies for TypeScript ESLint scope manager packages
apps/oxlint/src-js/plugins/scope.ts Implemented ScopeManager class and scope helper methods (isGlobalReference, getDeclaredVariables, getScope, markVariableAsUsed)
apps/oxlint/src-js/plugins/source_code.ts Integrated ScopeManager into SOURCE_CODE object with lazy initialization
apps/oxlint/test/e2e.test.ts Added integration tests for scope manager functionality
apps/oxlint/test/fixtures/scope_manager/* Test fixture demonstrating ScopeManager API usage
apps/oxlint/test/fixtures/sourceCode_scope_methods/* Test fixture demonstrating scope helper methods
apps/oxlint/tsconfig.json Updated to exclude test fixture files from TypeScript compilation
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@lilnasy lilnasy force-pushed the feat/linter/plugins/scope-manager branch from 6d2ec6c to 88e9085 Compare October 22, 2025 16:03
Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

Amazing work on this, thank you

@lilnasy lilnasy force-pushed the feat/linter/plugins/scope-manager branch 2 times, most recently from f74fee0 to edffb11 Compare October 23, 2025 13:30
@lilnasy lilnasy force-pushed the feat/linter/plugins/scope-manager branch 2 times, most recently from ae58fac to 0a0dca6 Compare October 27, 2025 12:35
@camc314 camc314 force-pushed the feat/linter/plugins/scope-manager branch from 0a0dca6 to 591616e Compare October 29, 2025 10:44
Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

Thank yoU!

@camc314 camc314 merged commit f7bef73 into oxc-project:main Oct 29, 2025
18 checks passed
camc314 added a commit that referenced this pull request Oct 30, 2025
## [1.25.0] - 2025-10-30

### 💥 BREAKING CHANGES

- 659fd37 linter: [**BREAKING**] `tsgolint`: request fixes when
necessary (#15048) (camchenry)

### 🚀 Features

- ed24d60 linter: Expose tsgolint program diagnostics (#15080) (camc314)
- 23660c9 linter: `tsgolint`: handle omitted fixes and suggestions
(#15047) (camchenry)
- f7bef73 linter/plugins: Scope manager API (#14890) (Arsh)
- 3e15cdd linter/strict-boolean-expression: Add rule (#14930) (camc314)
- bd74603 linter: Add support for vitest/valid-title rule (#12085)
(Tyler Earls)

### 🐛 Bug Fixes

- e41dee5 linter/consistent-type-definition: Skip comments when looking
for token (#14909) (camc314)
- 806f9ba linter: Search system PATH for tsgolint executable (#14861)
(magic-akari)
- ee68089 linter: Normalize JS plugin names (#15010) (Peter Wagenet)
- 597340e ast-tools: Use oxfmt to format generated code (#15064)
(camc314)
- 5eaaa8e linter: Prevent underflow in count_comment_lines for JSX files
(#15026) (ityuany)
- 88577a8 import/no-namespace: Remove dot special case (#15032) (Arsh)
- 55ee962 linter/vars-on-top: False positive with typescript declare
block (#14952) (Hamir Mahal)
- 2de9f39 linter/plugins: Fall back to package name if meta.name is
missing (#14938) (Peter Wagenet)
- 5ace84b linter/no-empty-object-type: Parse `"allowWithName"` as
regular expressions (#14943) (Arsh)
- 5a2832d editor: Stop client when delete .oxlintrc.json with
`oxc.requireConfig` (#14897) (Liang Mi)
- 3e29d23 linter: Use aliases when parsing cli rules (#14912) (Arsh)
- b94b6aa linter/explicit-module-boundary-types: False negative with
export default function (#14905) (camc314)
- 7060863 linter/no-standaline-expect: False positive with expect in
callback (#14902) (camc314)
- dc5a71b linter/no-accumulating-spread: False positive in nested
callbacks within reduce (#14898) (camc314)

### 🚜 Refactor

- 8d8d508 editor: Flatten `flags` options (#15006) (Sysix)
- b1e1531 language_server: Extract library interface from main.rs
(#15036) (Boshen)
- 5de99c2 formatter: Export unified way to get_parse_options (#15027)
(leaysgur)
- b55df7f language_server: Move sub option for `flags` to the root +
deprecate flags (#14933) (Sysix)

### 📚 Documentation

- e15c91c linter: Add configuration option docs for
eslint/init-declarations rule. (#15085) (Connor Shea)
- f4505bc linter: Add configuration option docs for eslint/id-length
rule. (#15083) (Connor Shea)
- dd4c9d2 linter: Add configuration option docs for eslint/getter-return
rule. (#15081) (Connor Shea)
- 008e67a linter: Add configuration option docs for
jest/no-large-snapshots rule. (#15079) (Connor Shea)
- 31daf79 linter: Add configuration option docs for import/no-commonjs
rule. (#15077) (Connor Shea)
- 9bf8ebe linter: Add configuration option docs for
jsdoc/check-tag-names rule. (#15076) (Connor Shea)
- 491ab5e linter: Add configuration option docs for jsdoc/no-defaults
rule. (#15074) (Connor Shea)
- 2602d7e linter: Add configuration option docs for jsdoc/empty-tags
rule. (#15072) (Connor Shea)
- c3a92e0 linter: Add configuration option docs for
oxc/no-rest-spread-properties rule. (#15070) (Connor Shea)
- c7c9213 linter: Add configuration option docs for
oxc/no-optional-chaining rule. (#15071) (Connor Shea)
- 47a616f linter: Add configuration option docs for
typescript/no-empty-object-type rule. (#14968) (Connor Shea)
- fcc1e25 linter: Add configuration option docs for
typescript/no-explicit-any rule. (#15051) (Connor Shea)
- 1b88934 linter: Add configuration option docs for
eslint/no-multi-assign rule. (#15052) (Connor Shea)
- 909d4b9 linter: Add configuration option docs for
eslint/no-unsafe-negation rule. (#15053) (Connor Shea)
- f60763b linter: Add configuration option docs for
eslint/no-useless-computed-key rule. (#15054) (Connor Shea)
- c8911d4 linter: Add configuration option docs for
eslint/no-unneeded-ternary rule. (#15056) (Connor Shea)
- 9fc6374 linter: Add configuration option docs for
eslint/no-misleading-character-class rule. (#15058) (Connor Shea)
- 7656e2b linter: Add configuration option docs for eslint/sort-imports
rule. (#15013) (Connor Shea)
- bb06039 linter: Add configuration option docs for
unicorn/no-instanceof-builtins rule. (#14999) (Connor Shea)
- a4af5ff linter: Add configuration option docs for
unicorn/no-typeof-undefined rule. (#15001) (Connor Shea)
- ac2ed39 linter: Add configuration option docs for
unicorn/numeric-separators-style rule. (#15002) (Connor Shea)
- 5ce3e33 linter: Add configuration option docs for
unicorn/prefer-object-from-entries rule. (#15003) (Connor Shea)
- 07aea72 linter: Add configuration option docs for eslint/sort-keys
rule. (#15014) (Connor Shea)
- 35b36e0 linter: Add configuration option docs for eslint/no-eval rule.
(#15015) (Connor Shea)
- d222b85 linter: Add configuration option docs for eslint/no-empty
rule. (#15017) (Connor Shea)
- 83bbc37 linter: Add configuration option docs for
eslint/no-extend-native rule. (#15018) (Connor Shea)
- aade2fe linter: Add configuration option docs for
eslint/no-extra-boolean-cast rule. (#15019) (Connor Shea)
- bae0c09 linter: Add configuration option docs for jsdoc/require-yields
rule. (#15024) (Connor Shea)
- 06c77b6 linter: Add configuration option docs for unicorn/prefer-at
rule. (#15023) (Connor Shea)
- 54b2cb4 linter: Add configuration option docs for
eslint/max-classes-per-file rule. (#15022) (Connor Shea)
- b5acda7 linter: Add configuration option docs for
typescript/explicit-module-boundary-types rule. (#15021) (Connor Shea)
- a0c6162 linter: Add configuration option docs for vue/max-props rule.
(#14966) (Connor Shea)
- 3ec59c0 linter: Add configuration option docs for
no-async-endpoint-handlers rule. (#14965) (Connor Shea)
- 7a275fd linter: Add configuration option docs for no-map-spread rule.
(#14964) (Connor Shea)
- 0002b7c linter: Add configuration option docs for no-barrel-file rule.
(#14963) (Connor Shea)
- fd4b7ab linter: Add configuration option docs for
unicorn/consistent-function-scoping rule. (#14969) (Connor Shea)
- aa339b3 linter: Add configuration option docs for eslint/no-void rule.
(#14970) (Connor Shea)
- af8bdae linter: Add configuration option docs for
import/no-anonymous-default-export rule. (#14971) (Connor Shea)
- 5a81953 linter: Add configuration option docs for jest/require-hook
rule. (#14972) (Connor Shea)
- 1be268d linter: Add configuration option docs for
jest/require-top-level-describe rule. (#14973) (Connor Shea)
- 860a58c linter: Add configuration option docs for jest/no-hooks rule.
(#14974) (Connor Shea)
- b69b586 linter: Add configuration option docs for jest/max-expects
rule. (#14975) (Connor Shea)
- 2e02fe0 linter: Add configuration option docs for
jest/no-deprecated-functions rule (#14976) (Connor Shea)
- cdc6c4f linter: Add configuration option docs for
unicorn/no-array-sort rule. (#14977) (Connor Shea)
- 40eb394 linter/no-unassigned-import-config: Add auto generated config
docs (#14918) (camc314)

### ⚡ Performance

- a9f68b2 linter: Move up rule retainment into initial collection to
reduce allocation size (#14822) (camchenry)
- b27c5b9 linter: Add codegen support for regex nodes (#14874)
(camchenry)

### 🧪 Testing

- bf898e5 linter: Increase stability of tsgolint test cases (#15063)
(camc314)
- 49da411 linter/no-standalone-expect: Fix tests (#14901) (camc314)

Co-authored-by: camc314 <18101008+camc314@users.noreply.github.com>
@lilnasy lilnasy deleted the feat/linter/plugins/scope-manager branch November 1, 2025 21:19
graphite-app bot pushed a commit that referenced this pull request Nov 5, 2025
Follow-on after #14890.

That PR implemented `ScopeManager` class. Instead of a class, which an instance of is created for each file, use a singleton object, with the underlying TS-ESLint scope manager stored in a top-level var. Methods/getters of `SCOPE_MANAGER` lazily initialize the TS-ESLint scope manager as required, and cache it.

This pattern is the same as we use for `SourceCode`, enabled by the fact that only 1 file is linted at a time. This has several advantages:

1. Reduce object creation - less GC pressure.
2. Property accesses don't need to go up prototype chain, as they would for instances of a class.
3. No need for private properties, which are somewhat expensive to access.
graphite-app bot pushed a commit that referenced this pull request Nov 5, 2025
Follow-on after #14890. Small optimizations to `ScopeManager`.

* Remove temp vars.
* Test vars with `=== null` or `=== undefined` instead of `!`.
* Avoid reading out of bounds of arrays.
* Get length of array once before loop, rather than on each turn of loop.

Also add comments about potential future optimizations.
graphite-app bot pushed a commit that referenced this pull request Nov 5, 2025
Follow on after #14890. Pure refactor. Just shorten code but compacting `if` statements to a single line. Also move type import to after "real" imports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-linter Area - Linter C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants