test(linter/plugins): avoid using scope constructor names in test snapshot #15906
+21
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
A-linter
Member
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Member
Author
|
@lilnasy Would you mind reviewing? I'm not sure if you did it this way because you specifically wanted to check what class the scopes are? |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the scope manager test snapshot to use the type field of scope objects instead of constructor names, making the test more robust against code bundling variations.
- Replaces
s.constructor.namewiths.typefor scope identification - Updates output format to use newline-separated list with
-prefix for better readability - Changes scope naming format from angle brackets to parentheses for named scopes (e.g.,
function(topLevelFunction))
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| apps/oxlint/test/fixtures/scope_manager/plugin.ts | Refactored message generation to use s.type field and improved formatting with newline separation |
| apps/oxlint/test/fixtures/scope_manager/output.snap.md | Updated snapshot to reflect new scope output format with type-based naming |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
29 tasks
overlookmotel
added
A-linter-plugins
Member
Author
Merge activity
|
…pshot (#15906) Contents of this test snapshot depends on the constructor names of scope class instances. This is a little fragile, as they can change depending on how the code is bundled (see #15861 (comment)). At some point, we'll full minify the bundle, at which point class names will be come random gibberish like `e`, `t`, `aZ`, etc. Avoid this problem by outputting `type` field of scope objects instead.
graphite-app
bot
force-pushed
the
11-20-test_linter_plugins_avoid_using_scope_constructor_names_in_test_snapshot
branch
from
a690858 to
15d367a
Compare
graphite-app
bot
deleted the
11-20-test_linter_plugins_avoid_using_scope_constructor_names_in_test_snapshot
branch
Copilot AI
pushed a commit
that referenced
this pull request
Nov 21, 2025
…pshot (#15906) Contents of this test snapshot depends on the constructor names of scope class instances. This is a little fragile, as they can change depending on how the code is bundled (see #15861 (comment)). At some point, we'll full minify the bundle, at which point class names will be come random gibberish like `e`, `t`, `aZ`, etc. Avoid this problem by outputting `type` field of scope objects instead.
overlookmotel
added a commit
that referenced
this pull request
Nov 21, 2025
…ementation) (#15861) - "Slow but working" implementation for #14829 (comment) - Uses the parser from `@typescript-eslint/typescript-estree` for tokens. The source code is fully parsed and everything but the tokens and comments is discarded. - Tests are directly adapted from `eslint`. - Direct commits from maintainers welcome. ### Tasks - [x] `getTokens` - [x] Move token types to `src-js/plugins/tokens.ts` - [x] Bundle typescript, handling `_filename` usage in ES modules. - [x] ~[Prevent class name mangling by `tsdown`](#15861 (comment)). Could be deferred.~ #15906 (review) - [x] [Test regex-divide ambiguous syntax](#15861 (comment)). - [x] Test for conformance directly against `eslint` tests. - [ ] [Parse as JSX only if source text is JSX](#15861 (comment)). Could be deferred. - [ ] Lazy load the parser on demand. Could be deferred. ### Future work - [ ] `getFirstToken` - [ ] `getFirstTokens` - [ ] `getLastToken` - [ ] `getLastTokens` - [ ] `getTokenBefore` - [ ] `getTokenOrCommentBefore` - [ ] `getTokensBefore` - [ ] `getTokenAfter` - [ ] `getTokenOrCommentAfter` - [ ] `getTokensAfter` - [ ] `getTokensBetween` - [ ] `getFirstTokenBetween` - [ ] `getFirstTokensBetween` - [ ] `getLastTokenBetween` - [ ] `getLastTokensBetween` - [ ] `getTokenByRangeStart` - [ ] `isSpaceBetween` - [ ] `isSpaceBetweenTokens` ### Decisions - [x] `@typescript-eslint/typescript-estree` peer-depends on `typescript`. How should we package it? Currently, `typescript` is being made an optional dependency. - **overlookmotel (on discord): ideally bundled, direct runtime dependency otherwise.** - [x] Deprecated methods are being removed altogether in ESLint 10: `getTokenOrCommentBefore`, `getTokenOrCommentAfter`, and `isSpaceBetweenTokens`. These are surface level deprecations: the functionality was merged with other methods (the `includeComments: true` option) and plugins can migrate with a one line change. I'm guessing we are targeting fairly modern, actively developed projects. Should we expose them? - **[overlookmotel and camc314](#15861 (comment)): yes.** - [x] `Program` range is different between ESLint and TS-ESLint. Which one should we follow? - **[overlookmotel](#15861 (comment)): they are going to both change and align soon, we should update our range calculation to match when they do.** --------- Co-authored-by: overlookmotel <theoverlookmotel@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contents of this test snapshot depends on the constructor names of scope class instances. This is a little fragile, as they can change depending on how the code is bundled (see #15861 (comment)).
At some point, we'll full minify the bundle, at which point class names will be come random gibberish like
e,t,aZ, etc.Avoid this problem by outputting
typefield of scope objects instead.