feat(linter): search for .oxlintrc.json when no --config is provided #4924
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
DonIsaac
added
C-enhancement
CodSpeed Performance ReportMerging #4924 will not alter performanceComparing Summary
|
|
Both of our editor extensions are providing a default oxc/editors/vscode/package.json Lines 97 to 102 in 4081293 We should follow this PR with 2 more to address this change. The I know you are OK with changing the VSCode one but in case you don't want to bother with the Vim(or even VSCode if you have other tasks in mind) Just let me know so I can take over this small change. |
|
I'll add a PR to this stack updating the VSCode defaults, but we'll need a separate PR for the CoC plugin. If you're happy to update that repo, I'd appreciate the help :) |
DonIsaac
force-pushed
the
don/08-15-feat_linter_search_for_.oxlintrc.json_when_no_--config_is_provided
branch
from
b27f6fe to
7b5eb51
Compare
| /// directory (in order): | ||
| /// | ||
| /// - `.oxlintrc.json` | ||
| /// - `oxlintrc.json` |
There was a problem hiding this comment.
I can't understand how we can guarantee the order here, What am I missing?
If we don't search with the order in mind we can drop the (in order) part.
| @@ -92,7 +92,7 @@ impl Runner for LintRunner { | |||
| let cwd = std::env::current_dir().unwrap().into_boxed_path(); | |||
| let lint_options = LintOptions::default() | |||
| .with_filter(filter) | |||
| .with_config_path(basic_options.config) | |||
| .with_config_path(basic_options.config().map(Into::into)) | |||
There was a problem hiding this comment.
I've looked into the LSP, I'm working on a PR updating our LSP to take advantage of this feature. Is it possible to provide a way to resolve the path in the with_config_path instead of doing it in the basic_options?
It allows me to pass in a None value here and let this logic resolve it.
oxc/crates/oxc_language_server/src/main.rs
Line 350 in 9c64b12
There was a problem hiding this comment.
Maybe adding a dedicated setter would be better, Or even providing a function that is both public and is used in the basic_options implementation so we can share that code. Something like resolve_config_path or find_oxlint_config? What are your thoughts on it?
| } | ||
|
|
||
| let pwd = std::env::current_dir().ok()?; | ||
| for entry in std::fs::read_dir(&pwd).ok()? { |
There was a problem hiding this comment.
I'm not sure about the performance overhead of this :-/
There was a problem hiding this comment.
We can change this to concat our paths and only check the fs stat of those few paths.
I'm more interested in this comment of yours:
This behavior differs from eslint, will this give a surprise? Is implicit better than explicit here?
I feel it's useful for LSP, How does it differ from eslint? Doesn't eslint use the cwd to find the config if none is provided? In your opinion doing this for the CLI is a bad diversion?
Can't we make this work with something like this: https://eslint.org/docs/latest/use/command-line-interface#--no-eslintrc
I can think of situations where people would want to omit the config flag to run the linter with the default configuration, But most of the time that's not the case. I'm not sure about the implications of this change so please share your thoughts on it.
|
ESLint will default to one of the various .eslintrc file names. This is equivalent behavior, but for our config file. We could also look for an .eslintrc (or other file name) if no oxlint config is found. |
|
I still think we should conform to what eslint does, because all our users come from eslint. We can merge this if we can reference this feature back to eslint. |
|
Let's revisit this after we improve our config logic. |
Boshen
deleted the
don/08-15-feat_linter_search_for_.oxlintrc.json_when_no_--config_is_provided
branch
Closes #4600