-
-
Notifications
You must be signed in to change notification settings - Fork 713
feat(oxlint): add maxWarnings config option #15118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(oxlint): add maxWarnings config option #15118
Conversation
Add support for maxWarnings configuration option in .oxlintrc.json to control exit code behavior based on warning count. - Add max_warnings field to Oxlintrc struct - Wire max_warnings through config system to DiagnosticService - CLI --max-warnings flag takes precedence over config value - Add validation to prevent maxWarnings in nested configs - Add unit tests for maxWarnings parsing and merging Closes oxc-project#15020
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. |
There was a problem hiding this 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 adds support for configuring a maxWarnings threshold in the .oxlintrc.json configuration file. This allows projects to enforce a maximum number of warnings before the linter exits with an error status, complementing the existing CLI flag --max-warnings.
Key changes:
- Added
max_warningsfield to theOxlintrcstruct with JSON serialization support - Updated the merge logic to handle
max_warningsconfiguration inheritance - Modified the diagnostic service initialization to prioritize CLI flag over config file value
- Added validation to prevent
maxWarningsfrom being used in nested configuration files
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/oxc_linter/src/config/oxlintrc.rs | Adds max_warnings field to the Oxlintrc struct, updates merge logic to handle the new field, and includes comprehensive tests for serialization and merging behavior |
| apps/oxlint/src/lint.rs | Integrates config-based max_warnings with CLI flag, adds validation to reject maxWarnings in nested configs, and updates diagnostic service initialization to use the merged value |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this!
Can we add some integration tests for this.
The test cases (real files) should go in apps/oxlint/src/fixtures, and the test fn should go in apps/oxlint/src/lint.rs
|
Hi @camc314 Already added integration tests as requested. The tests cover:
All tests pass and snapshots have been created. |
Summary
This PR adds support for the
maxWarningsconfiguration option in.oxlintrc.jsonfiles, allowing users to specify a warning threshold that will cause the linter to exit with a non-zero code.Changes
max_warningsfield toOxlintrcstruct incrates/oxc_linter/src/config/oxlintrc.rsmax_warningsthrough the config system toDiagnosticService--max-warningsflag takes precedence over config file valuemaxWarningsin nested config files (raises error as specified)Example Usage
{ "maxWarnings": 0 }Testing
Closes #15020