Skip to content

Comments

refactor(linter/plugins): remove oxlint2 Cargo feature#13648

Merged
graphite-app[bot] merged 1 commit intomainfrom
09-10-refactor_linter_plugins_remove_oxlint2_cargo_feature
Sep 11, 2025
Merged

refactor(linter/plugins): remove oxlint2 Cargo feature#13648
graphite-app[bot] merged 1 commit intomainfrom
09-10-refactor_linter_plugins_remove_oxlint2_cargo_feature

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Sep 10, 2025

Remove oxlint2 Cargo feature from oxlint and oxc_linter crates.

Now that enabling fixed_size Cargo feature of oxc_allocator doesn't change behavior, we don't need the feature gate. The incompatibility of tokio with WASM has been resolved by #13647.

Copy link
Member Author

overlookmotel commented Sep 10, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Sep 10, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 10, 2025

CodSpeed Instrumentation Performance Report

Merging #13648 will not alter performance

Comparing 09-10-refactor_linter_plugins_remove_oxlint2_cargo_feature (7775c21) with main (08cbd39)1

Summary

✅ 37 untouched benchmarks

Footnotes

  1. No successful run was found on main (7775c21) during the generation of this report, so 08cbd39 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@overlookmotel
Copy link
Member Author

@camc314 Is this a good idea?

Language server: At present, removing the oxlint2 feature causes some code which never runs to get compiled in language server. But presumably pretty soon we'll want to add JS plugin support to language server too, at which point it won't be dead code any more.

Playground: The playground probably isn't going to support linter JS plugins any time soon (how would you provide the plugin?), so removing the feature will cause some wasted bytes in the playground's WASM file.

On the positive side, this does remove quite a bit of mess and complexity.

@overlookmotel overlookmotel self-assigned this Sep 10, 2025
@camc314
Copy link
Contributor

camc314 commented Sep 11, 2025

Yes, i think this is a good idea. It simplifies the code which is good. I don't think the playground will ever (i guess technically we could but probably more effort than it's worth) support custom JS plugins. Language server will definitely support this (at some point)

@graphite-app graphite-app bot changed the base branch from 09-10-refactor_linter_plugins_move_tokio_usage_from_oxc_linter_to_napi_oxlint2_ to graphite-base/13648 September 11, 2025 05:23
@graphite-app graphite-app bot force-pushed the graphite-base/13648 branch from 550cba0 to 2d53203 Compare September 11, 2025 05:29
@graphite-app graphite-app bot force-pushed the 09-10-refactor_linter_plugins_remove_oxlint2_cargo_feature branch from 3a03e1c to 3f2ba5b Compare September 11, 2025 05:29
@graphite-app graphite-app bot changed the base branch from graphite-base/13648 to main September 11, 2025 05:29
@graphite-app graphite-app bot force-pushed the 09-10-refactor_linter_plugins_remove_oxlint2_cargo_feature branch from 3f2ba5b to 5c057e0 Compare September 11, 2025 05:30
@overlookmotel overlookmotel marked this pull request as ready for review September 11, 2025 06:17
Copilot AI review requested due to automatic review settings September 11, 2025 06:17
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Sep 11, 2025
Copy link
Member Author

overlookmotel commented Sep 11, 2025

Merge activity

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 removes the oxlint2 Cargo feature from the oxlint and oxc_linter crates. The feature gate is no longer necessary since enabling the fixed_size Cargo feature of oxc_allocator doesn't change behavior, and WASM compatibility issues with tokio have been resolved.

Key changes:

  • Remove conditional compilation blocks gated by oxlint2 feature
  • Enable fixed_size allocator features unconditionally
  • Clean up unused feature dependencies and configurations

Reviewed Changes

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

Show a summary per file
File Description
napi/oxlint2/Cargo.toml Remove oxlint2 feature from oxlint dependency
crates/oxc_linter/src/service/runtime.rs Remove feature-gated allocator selection logic
crates/oxc_linter/src/lib.rs Remove conditional compilation for external rule functionality
crates/oxc_linter/src/external_linter.rs Remove dead code attribute
crates/oxc_linter/src/config/config_store.rs Remove dead code attribute
crates/oxc_linter/src/config/config_builder.rs Remove unused feature-gated function variant
crates/oxc_linter/Cargo.toml Update feature configuration and dependencies
apps/oxlint/src/lint.rs Remove conditional compilation for external linter
apps/oxlint/src/lib.rs Remove conditional module inclusion
apps/oxlint/Cargo.toml Update dependencies and remove feature

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

Remove `oxlint2` Cargo feature from `oxlint` and `oxc_linter` crates.

Now that enabling `fixed_size` Cargo feature of `oxc_allocator` doesn't change behavior, we don't need the feature gate. The incompatibility of `tokio` with WASM has been resolved by #13647.
@graphite-app graphite-app bot force-pushed the 09-10-refactor_linter_plugins_remove_oxlint2_cargo_feature branch from 5c057e0 to 7775c21 Compare September 11, 2025 06:18
@graphite-app graphite-app bot merged commit 7775c21 into main Sep 11, 2025
25 checks passed
@graphite-app graphite-app bot deleted the 09-10-refactor_linter_plugins_remove_oxlint2_cargo_feature branch September 11, 2025 06:23
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Sep 11, 2025
camc314 added a commit that referenced this pull request Sep 11, 2025
## [1.15.0] - 2025-09-11

### 💥 BREAKING CHANGES

- edc70ea allocator/pool: [**BREAKING**] Remove `disable_fixed_size`
Cargo feature (#13625) (overlookmotel)

### 🚀 Features

- b20b56d linter: Add `vue/no-multiple-slot-args` rule (#13579) (Sysix)
- aafe08c linter: Add `vue/define-emits-declaration` rule (#13567)
(Sysix)
- 2ed5059 linter: Add `vue/define-props-declaration` rule (#13566)
(Sysix)
- a718c23 linter: Add `vue/valid-define-props` rule (#13565) (Sysix)
- 75a673e editor: Support relative path for `oxc.path.server` (#13542)
(Sysix)
- 4af886b linter: Add `unicorn/no-array-reverse` rule (#13530) (yefan)
- 2db32eb data_structures: Add `boxed_slice!` and `boxed_array!` macros
(#13596) (overlookmotel)

### 🐛 Bug Fixes

- fb9d0f4 language_server: Don't resend diagnostic on save, when
`typeAware` is disabled and run is onType (#13604) (YongSeok Jang (장용석))
- 2f36350 editor: Add notice for a possible restart when fixing
`filename-case` (#13557) (Sysix)
- e17fccc linter: Update `RuleRunner` impl after merge (#13642)
(camc314)
- 3d27c5b linter/no-unused-private-class-members: False positive with
spread expr (#13634) (yefan)
- 8314ed5 linter/tsgolint: Correct comment (#13589) (camc314)
- 198243b semantic: Dont parse `@` as jsdoc tags inside quotes (#13571)
(Gwenn Le Bihan)
- 89084d7 linter/custom-plugins: Enforce exact matching for disable
directives (#13538) (Copilot)
- 277c5e1 linter: Output `eslint-plugin-vue` for vue diagnostics
(#13564) (Sysix)
- 34d3cde rust: Fix clippy issues (#13540) (Boshen)
- 5fccafc linter: `unicorn/prefer-array-flat-map` ignore
`React.Children` (#13534) (Sysix)
- 7e78e39 linter: Don't panic when parsing regex with multiple
parentheses (#13524) (Sysix)
- 0d867b1 linter: Skip running tsgolint when no files need type aware
linting (#13502) (Copilot)
- b677376 language_server: Include the diagnostic of the other linter
(#13490) (Sysix)
- e87d7bd linter: Parse regex inside `new RegExp()` with parentheses
(#13448) (Sysix)
- 5990f17 linter: Change `typescript/no-confusing-void-expression` to
pedantic (#13473) (Boshen)

### 🚜 Refactor

- 7775c21 linter/plugins: Remove `oxlint2` Cargo feature (#13648)
(overlookmotel)
- 8f37e88 linter: Update tsgolint payload (#13547) (camchenry)
- 2d53203 linter/plugins: Move `tokio` usage from `oxc_linter` to
`napi/oxlint2` (#13647) (overlookmotel)
- 6cd6be2 linter: Add `--experimental-js-plugins` CLI arg (#13658)
(overlookmotel)
- 476729b linter: Simplify `RuleRunner` trait definition (#13637)
(camchenry)
- 2f02ac6 linter/plugins: Remove `disable_oxlint2` Cargo feature
(#13626) (overlookmotel)
- ff9e4fb linter/plugins: Use fixed-size allocators when
`ExternalLinter` exists (#13623) (overlookmotel)
- f9bff64 linter_codegen: Improve code style for collecting nodes
(#13636) (camchenry)
- babbaca all: Remove `pub` from modules with no exports (#13618)
(overlookmotel)
- 91759c6 linter/plugins: Only use `RawTransferFileSystem` if JS plugins
registered (#13599) (overlookmotel)
- 118020c linter/plugins: Discard `ExternalLinter` if no JS plugins
registered (#13598) (overlookmotel)
- 8d30bce linter/tsgolint: Report an error if the tsgolint exe could not
be found (#13590) (camc314)
- bccc276 eslint/for-direction: Clean up implementation and improve
documentation (#13532) (Antoine Zanardi)
- 1425da2 eslint/default-case-last: Simplify default case last check in
switch statement (#13529) (Antoine Zanardi)
- d245376 oxlint: Remove unused `runner` module (#13561) (camc314)
- 53f2fc1 eslint/default-case: Simplify implementation and enhance
readability (#13430) (Antoine Zanardi)
- 6f15060 eslint/block-scoped-var: Clean up implementation and improve
documentation (#13417) (Antoine Zanardi)
- 671e0fd language_server: Only store one instance of a diagnostic
(#13514) (Sysix)
- 1b425d6 eslint/default-case-last: Simplify implementation and enhance
readability (#13515) (Antoine Zanardi)
- e4bbbce eslint/default-param-last: Simplify implementation and enhance
readability (#13516) (Antoine Zanardi)
- e0396fd linter: Remove `static` lifetime from disable directives
function argument (#13492) (camc314)

### 📚 Documentation

- eb1f167 linter: Note which rules require type info to run on rule page
(#13675) (camc314)
- e66f93b linter: Fix backtick formatting in no-return-wrap (#13633)
(camc314)

### ⚡ Performance

- e6a25e7 linter: Remove unnecessary `should_run` check (#13639)
(camchenry)
- f6a9687 linter: Store rules by AST type in a boxed array (#13578)
(overlookmotel)
- b81f081 linter: Reduce indirection (#13574) (overlookmotel)
- a744aff linter: Skip rules that do not have any relevant node types
(#13138) (camchenry)

### 🎨 Styling

- e110476 linter: Reformat code (#13573) (overlookmotel)

### 🧪 Testing

- 58e6c94 oxlint: Add test for ignorePatterns whitelist (#13372) (Sysix)

Co-authored-by: camc314 <18101008+camc314@users.noreply.github.com>
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-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants