refactor(linter): check valid Vitest calls with pattern matching#16924
Conversation
github-actions
bot
added
A-linter
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. |
CodSpeed Performance ReportMerging #16924 will not alter performanceComparing Summary
Footnotes
|
camchenry
changed the title
There was a problem hiding this comment.
Pull request overview
This PR refactors Vitest function call validation from a compile-time PHF (Perfect Hash Function) set containing 1,305 hardcoded entries to a runtime pattern-matching approach. The new implementation validates function chains by checking the base function name and then applying modifier-specific validation rules (e.g., "each" must be last, "extend" must be first for it/test, no duplicates allowed). This significantly reduces compile time and binary size while making the codebase more maintainable, though it may have a slight runtime performance cost.
Key Changes
- Replaced 1,305-line PHF set with ~80 lines of rule-based validation logic
- Changed API signature from
is_valid_vitest_call(&[Cow<str>])to genericis_valid_vitest_call<T: AsRef<str>>(&[T]) - Added explicit validation rules for modifiers per function type (bench, describe/suite, it/test)
- Removed support for Jest-style prefix functions (fdescribe, fit, xdescribe, xit, xtest) which were deprecated aliases
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/oxc_linter/src/utils/vitest/valid_vitest_fn.rs | Complete rewrite of validation logic from PHF set lookup to pattern matching with modifier-specific rules and comprehensive test coverage |
| crates/oxc_linter/src/rules/jest/require_hook.rs | Updated function call to remove Cow::Borrowed wrapper, leveraging the new generic API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
camchenry
force-pushed
the
12-15-refactor_linter_ucheck_valid_vitest_calls_with_pattern_matching
branch
from
0b002f9 to
2c7ba8c
Compare
Merge activity
|
) This trades compile time + binary size for maybe slightly less performance when checking Vitest function calls. We currently have to manually maintain this list which is a bit difficult to do as aliases make this list grow quickly. This is not sustainable long-term if more functions and aliases get added, so I'm proposing we make the switch now. As part of this I also removed support for `fdescribe`, `fit`, `xdescribe`, `xit`, etc. as these don't actually seem to be supported in Vitest? At least I didn't find any references to them in the docs or in the source code, so I think this might have been a mistake when copying from the Jest version.
graphite-app
bot
force-pushed
the
12-15-refactor_linter_ucheck_valid_vitest_calls_with_pattern_matching
branch
from
2c7ba8c to
561abd0
Compare
graphite-app
bot
deleted the
12-15-refactor_linter_ucheck_valid_vitest_calls_with_pattern_matching
branch
…-project#16924) This trades compile time + binary size for maybe slightly less performance when checking Vitest function calls. We currently have to manually maintain this list which is a bit difficult to do as aliases make this list grow quickly. This is not sustainable long-term if more functions and aliases get added, so I'm proposing we make the switch now. As part of this I also removed support for `fdescribe`, `fit`, `xdescribe`, `xit`, etc. as these don't actually seem to be supported in Vitest? At least I didn't find any references to them in the docs or in the source code, so I think this might have been a mistake when copying from the Jest version.
1 participant
This trades compile time + binary size for maybe slightly less performance when checking Vitest function calls. We currently have to manually maintain this list which is a bit difficult to do as aliases make this list grow quickly. This is not sustainable long-term if more functions and aliases get added, so I'm proposing we make the switch now.
As part of this I also removed support for
fdescribe,fit,xdescribe,xit, etc. as these don't actually seem to be supported in Vitest? At least I didn't find any references to them in the docs or in the source code, so I think this might have been a mistake when copying from the Jest version.