feat(linter): support .oxlintrc.jsonc config files
#14267
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. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for .oxlintrc.jsonc configuration files to the oxlint linter, expanding the existing .json format support to include JSON with comments (JSONC).
- Updates auto-detection logic to prioritize
.oxlintrc.jsoncover.oxlintrc.json - Updates documentation and help text to reflect support for both formats
- Adds comprehensive test coverage for JSONC config functionality
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/oxc_linter/src/config/oxlintrc.rs | Updates documentation to mention both .json and .jsonc format support |
| apps/oxlint/src/command/lint.rs | Updates CLI help text to reflect support for both config file formats |
| apps/oxlint/src/lint.rs | Implements JSONC config auto-detection logic with priority over JSON format |
| apps/oxlint/fixtures/* | Adds test fixtures for JSONC configuration scenarios |
| apps/oxlint/src/snapshots/* | Adds test snapshots validating JSONC config functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Try .oxlintrc.jsonc first, then .oxlintrc.json | ||
| let jsonc_path = cwd.join(Self::DEFAULT_OXLINTRC_JSONC); | ||
| if jsonc_path.exists() { | ||
| return Oxlintrc::from_file(&jsonc_path); | ||
| } | ||
|
|
||
| let json_path = cwd.join(Self::DEFAULT_OXLINTRC); | ||
| if json_path.exists() { | ||
| return Oxlintrc::from_file(&json_path); | ||
| } |
There was a problem hiding this comment.
[nitpick] Consider extracting the repeated config file detection logic into a helper function to reduce duplication between find_oxlint_config and find_oxlint_config_in_directory. Both functions implement the same priority order (.jsonc then .json) with similar structure.
Copilot uses AI. Check for mistakes.
| // Try .oxlintrc.jsonc first, then .oxlintrc.json | ||
| let jsonc_path = cwd.join(Self::DEFAULT_OXLINTRC_JSONC); | ||
| if jsonc_path.exists() { | ||
| return Oxlintrc::from_file(&jsonc_path); | ||
| } | ||
|
|
||
| let json_path = cwd.join(Self::DEFAULT_OXLINTRC); | ||
| if json_path.exists() { | ||
| return Oxlintrc::from_file(&json_path); | ||
| } | ||
|
|
||
| Ok(Oxlintrc::default()) | ||
| } | ||
|
|
||
| /// Looks in a directory for an oxlint config file, returns the oxlint config if it exists | ||
| /// and returns `Err` if none exists or the file is invalid. Does not apply the default | ||
| /// config file. | ||
| fn find_oxlint_config_in_directory(dir: &Path) -> Result<Option<Oxlintrc>, OxcDiagnostic> { | ||
| let possible_config_path = dir.join(Self::DEFAULT_OXLINTRC); | ||
| if possible_config_path.is_file() { | ||
| Oxlintrc::from_file(&possible_config_path).map(Some) | ||
| } else { | ||
| Ok(None) | ||
| // Try .oxlintrc.jsonc first, then .oxlintrc.json | ||
| let jsonc_path = dir.join(Self::DEFAULT_OXLINTRC_JSONC); | ||
| if jsonc_path.is_file() { | ||
| return Oxlintrc::from_file(&jsonc_path).map(Some); | ||
| } | ||
|
|
||
| let json_path = dir.join(Self::DEFAULT_OXLINTRC); | ||
| if json_path.is_file() { | ||
| return Oxlintrc::from_file(&json_path).map(Some); | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] This code duplicates the same logic as in find_oxlint_config. Consider extracting this into a helper function that takes a directory path and returns the first existing config file path, or None if neither exists.
Copilot uses AI. Check for mistakes.
CodSpeed Instrumentation Performance ReportMerging #14267 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
I do not like this idea of adding a second config file to search for.
More info: #7348 (comment) and #8688
Review:
Can we add a test for .oxlintrc.json and .oxlintrc.jsonc file in the same directory? Looks like .jsonc should win.
Please the same test for the server too ❤️
No description provided.