fix(linter): Fix vitest/no-restricted-vi-methods and add tests for it.#16971
fix(linter): Fix vitest/no-restricted-vi-methods and add tests for it.#16971graphite-app[bot] merged 1 commit intomainfrom
vitest/no-restricted-vi-methods and add tests for it.#16971Conversation
CodSpeed Performance ReportMerging #16971 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the vitest/no-restricted-vi-methods rule was not being correctly remapped to its underlying implementation jest/no-restricted-jest-methods. The config builder previously only handled simple plugin name swaps (e.g., jest/foo → vitest/foo) but did not support rules that need to map to an entirely different name.
Key Changes:
- Added special-case mapping logic to handle
vitest/no-restricted-vi-methods→jest/no-restricted-jest-methodsin both the config builder and disable directives system - Enhanced documentation to clarify the rule works with both Jest and Vitest, and requires configuration to be effective
- Added comprehensive test coverage to verify the mapping works correctly with disable directives
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
crates/oxc_linter/src/config/rules.rs |
Adds special-case mapping in transform_rule_and_plugin_name to handle vitest/no-restricted-vi-methods before generic plugin transformation |
crates/oxc_linter/src/disable_directives.rs |
Adds special-case logic to match disable comments for vitest/no-restricted-vi-methods against the internal rule name no-restricted-jest-methods; clarifies comment about string lengths |
crates/oxc_linter/src/rules/jest/no_restricted_jest_methods.rs |
Improves documentation to mention Vitest support, clarify that null values produce generic messages, and provide better configuration example |
apps/oxlint/src/lint.rs |
Adds new test function to verify the vitest rule name mapping works correctly with disable directives |
apps/oxlint/fixtures/disable_vitest_rules/test.js |
Test fixture demonstrating proper handling of disable directives for vitest/no-restricted-vi-methods |
apps/oxlint/fixtures/disable_vitest_rules/.oxlintrc-vitest.json |
Configuration file for the test fixture that restricts specific vi methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vitest/no-restricted-vi-methods, as it does not currently work.vitest/no-restricted-vi-methods and add tests for it.
Merge activity
|
…it. (#16971) This fixes a few things: - Fixes the config builder so `vitest/no-restricted-vi-methods` can actually be enabled via the config file. - Fixes disabling the `vitest/no-restricted-vi-methods` rule via a disable directive comment. `vitest/no-restricted-vi-methods` wasn't being remapped correctly by the config builder, as the config builder only remapped `jest/any-name` to `vitest/any-name`, and there was no special handling for rules which need to map to an entirely different name. This has added that logic. `vitest/no-restricted-jest-methods` _does_ work, but that's not what it should be called or how it should work. Also a minor docs improvements. I'm unsure if this is sufficiently performant, honestly, but it fixes the problem. There is very likely a better way to do this. As an aside, the disable comment logic seems... very prone to failure if we're just doing a `name.contains(rule_name)` in most cases. That'd allow `// oxlint-disable-next-line a` to work on any rule with an `a` in the name, I think?
4306140 to
28e73d8
Compare
…it. (#16971) This fixes a few things: - Fixes the config builder so `vitest/no-restricted-vi-methods` can actually be enabled via the config file. - Fixes disabling the `vitest/no-restricted-vi-methods` rule via a disable directive comment. `vitest/no-restricted-vi-methods` wasn't being remapped correctly by the config builder, as the config builder only remapped `jest/any-name` to `vitest/any-name`, and there was no special handling for rules which need to map to an entirely different name. This has added that logic. `vitest/no-restricted-jest-methods` _does_ work, but that's not what it should be called or how it should work. Also a minor docs improvements. I'm unsure if this is sufficiently performant, honestly, but it fixes the problem. There is very likely a better way to do this. As an aside, the disable comment logic seems... very prone to failure if we're just doing a `name.contains(rule_name)` in most cases. That'd allow `// oxlint-disable-next-line a` to work on any rule with an `a` in the name, I think?
28e73d8 to
477bb57
Compare
# Oxlint ### 🚀 Features - 6cc3fdf linter/no-inferrable-types: Implement fixer (#17090) (camc314) - 2067997 linter/no-negation-in-equality-check: Implement suggestion (#17084) (camc314) - 552f9ef vscode: Auto-generate VSCode README configuration from package.json (#16970) (Copilot) - 9190c4b linter/no-unnecessary-array-flat-depth: Implement fixer (#17057) (camc314) - ed789de linter/misrefactored-assign-op: Implement fixer (#17056) (camc314) - a0f74a0 linter/config: Allow aliasing plugin names to allow names the same as builtin plugins (#15569) (Cameron) - a43d251 linter/plugins: `RuleTester` support `languageOptions.globals` (#17009) (overlookmotel) - 35070d9 linter/bad-bitwise-operator: Implement fixer (#17006) (camc314) - 322d995 linter/prefer-enum-initializers: Implement fixer (#17004) (camc314) - ae1e5bc vscode: Add support for tsgolint binary configuration (#16921) (ColemanDunn) - 3bfe31e linter/eslint-plugin-vitest: Add prefer-called-with as vitest compatible jest rule (#16993) (Said Atrahouch) - 0cd075f linter/eslint-plugin-jest: Add fix capabilities to prefer-called-with rule (#16987) (Said Atrahouch) - 357564b linter: Add options for `typescript/require-array-sort-compare` rule. (#16980) (connorshea) - 2b0ffba linter: Add options for `typescript/no-meaningless-void-operator` rule. (#16981) (connorshea) - 8bc4287 linter/plugins: Validate options against schema (#16974) (overlookmotel) - fdc7d08 linter: Implement eslint/capitalized-comments (#16896) (Tu Shaokun) - f8b6561 linter: Add support for `test.for` in vitest (#16925) (camchenry) - 7ee0379 linter/eslint-plugin-vitest: Implement prefer-spy-on (#16426) (Said Atrahouch) - fc96ee0 linter: Implement jest/prefer-to-have-been-called-times (#16938) (秦宇航) - 291b57b ast_tools: Generate TS declaration files for deserializer and walk files (#16912) (camc314) - e31da2a linter: Implement jest/perfer-to-have-been-called (#16899) (秦宇航) - 1a31306 linter/eslint-plugin-vitest: Add require-hook as vitest compatible jest rule (#16880) (Said Atrahouch) - cd3db21 linter: Add ignoredTypeNames option to no-base-to-string rule (#16898) (camc314) - 763b25a linter: Implement eslint/no-inline-comments (#16885) (Tu Shaokun) ### 🐛 Bug Fixes - fb9e193 linter: OOM problems with custom plugins (#17082) (overlookmotel) - 005ec25 linter: Permit `$schema` `.oxlintrc.json` struct (#17060) (Copilot) - fd03131 linter/plugins: Handle plugin names containing slashes (#17073) (overlookmotel) - b2a4fac linter/plugins: Error if plugin name alias is not normalized (#17071) (overlookmotel) - e046c4e linter/no-misused-spread: Add rule options support (#17054) (camc314) - 5c1a9e0 linter/no-deprecated: Add rule options support (#17053) (camc314) - 8c9cafe linter: `import/consistent-type-specifier-style`: add support for declaration files (#16979) (camchenry) - dab4780 linter/no-empty-pattern: Misleading help message for arrays (#17039) (Copilot) - 67f8c5d linter/plugins: Get correct plugin name in all cases (#17033) (overlookmotel) - 674dab9 linter/plugins: Fix indentation in error message (#17018) (overlookmotel) - 6524f72 linter/plugins: Add `@types/node` dev dependency to `oxlint` package (#17016) (overlookmotel) - 7a35513 linter/plugins: Better error for `null` in `globals` in `RuleTester` (#17011) (overlookmotel) - 42603ba linter/plugins: Always define `languageOptions.globals` (#17008) (overlookmotel) - 4cdc2f8 linter: Fix VITEST override rule list and add test for alphabetizing the two lists (#16975) (Connor Shea) - e466562 linter/consistent-type-definitions: Handle parenthesized types in rule (#16998) (camc314) - fce267c linter: Correct vitest plugin source to be `@vitest/eslint-plugin` (#16976) (connorshea) - 477bb57 linter: Fix `vitest/no-restricted-vi-methods` and add tests for it. (#16971) (connorshea) - 7d6974d linter: Ignore oxlint directive comments in capitalized-comments (#16989) (Tu Shaokun) - 23ac6b1 linter/plugins: Apply defaults from `meta.schema` to options (#16930) (overlookmotel) - 2f946cf linter/plugins: Error if `defaultOptions` is not JSON-serializable (#16959) (overlookmotel) - d8b8a57 linter/plugins: Freeze whole of merged options (#16958) (overlookmotel) - d446c43 linter: Prevent extra fields from being present on oxlint config file (#16874) (connorshea) - b845871 linter/plugins: Correctly handle object with `__proto__` keys in options merging (#16928) (overlookmotel) - c897794 linter: Fix eslint/sort-imports allowSeparatedGroups not working with single empty line (#16012) (Duc Nghiem Xuan) - 0c347a1 linter/array-type: Handle satisfies expression (#16903) (camc314) ### ⚡ Performance - 70d853c linter: Avoid cloning source text when cloning AST into fixed-size allocator (#17088) (overlookmotel) - 4d389f7 linter: Less bounds checks in `normalize_plugin_name` (#17030) (overlookmotel) - fd8e9c6 linter/plugins: Speed up cloning JSON objects (#16997) (overlookmotel) - d77e22d linter/plugins: Use `DEFAULT_OPTIONS` for rules with empty array as default options (#16913) (overlookmotel) ### 📚 Documentation - 6d053b4 linter: Fix typo in doc comment (#17091) (overlookmotel) - b5f3c91 linter: Document intentional exclusion of ignoreCase option in jsx-no-duplicate-props (#17046) (Copilot) - a0bf5d8 linter: Fix the config option docs for no-inline-comments rule. (#16983) (connorshea) - ca26a11 linter/plugins: Fix typo in doc comment (#16966) (overlookmotel) - 3183bf8 linter/plugins: Fix typo in JSDoc comment (#16900) (overlookmotel) # Oxfmt ### 🚀 Features - 15dfb55 oxfmt: Respect single nearest `.editorconfig` (#17043) (leaysgur) - 8c33ff4 oxfmt: Expose Node.js API: `format(fileName, sourceText, options?)` (#16939) (leaysgur) ### 🐛 Bug Fixes - d340c87 oxfmt: Update api `FormatOptions` type with `& Record<string, unknown>` (#17036) (leaysgur) - 827a256 oxfmt: Place ignorePatterns at bottom of JSON in --migrate prettier (#16926) (Boshen) Co-authored-by: overlookmotel <[email protected]>
…it. (oxc-project#16971) This fixes a few things: - Fixes the config builder so `vitest/no-restricted-vi-methods` can actually be enabled via the config file. - Fixes disabling the `vitest/no-restricted-vi-methods` rule via a disable directive comment. `vitest/no-restricted-vi-methods` wasn't being remapped correctly by the config builder, as the config builder only remapped `jest/any-name` to `vitest/any-name`, and there was no special handling for rules which need to map to an entirely different name. This has added that logic. `vitest/no-restricted-jest-methods` _does_ work, but that's not what it should be called or how it should work. Also a minor docs improvements. I'm unsure if this is sufficiently performant, honestly, but it fixes the problem. There is very likely a better way to do this. As an aside, the disable comment logic seems... very prone to failure if we're just doing a `name.contains(rule_name)` in most cases. That'd allow `// oxlint-disable-next-line a` to work on any rule with an `a` in the name, I think?
This fixes a few things:
vitest/no-restricted-vi-methodscan actually be enabled via the config file.vitest/no-restricted-vi-methodsrule via a disable directive comment.vitest/no-restricted-vi-methodswasn't being remapped correctly by the config builder, as the config builder only remappedjest/any-nametovitest/any-name, and there was no special handling for rules which need to map to an entirely different name. This has added that logic.vitest/no-restricted-jest-methodsdoes work, but that's not what it should be called or how it should work.Also a minor docs improvements.
I'm unsure if this is sufficiently performant, honestly, but it fixes the problem. There is very likely a better way to do this.
As an aside, the disable comment logic seems... very prone to failure if we're just doing a
name.contains(rule_name)in most cases. That'd allow// oxlint-disable-next-line ato work on any rule with anain the name, I think?