-
-
Notifications
You must be signed in to change notification settings - Fork 788
feat(linter): PoC to enable type-aware linting via the .oxlintrc.json file #18047
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?
Conversation
And add support in the LSP server for this.
I think this isn't working because I determine whether type-aware is enabled or not in [config_store](https://github.com/oxc-project/oxc/blob/8928b58ed6a7df8f6d77ce3f78b246db2cb64133/crates/oxc_linter/src/config/config_store.rs) via the base config. And so this line in `lint.rs` always returns false if the root config is unset/false, no matter what the nested configs do: ```rs let effective_type_aware = if self.options.type_aware { true } else { config_store.type_aware_enabled() };` ``` Which means tsgolint never gets activated, and there's no way to activate it _after_ the base config has been evaluated. I guess I could try to enable it if _any_ of the configs have it set as true, but I'm not sure that's really the desired behavior?
Merging this PR will not alter performance
Comparing Footnotes
|
| @@ -0,0 +1,13 @@ | |||
| { | |||
| // nested config with typeAware true | |||
| "plugins": ["typescript"], | |||
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.
This same file is copied a few times with different names for different test cases because I can't (or couldn't figure out how to) pass anything into the snapshot tool to add further context, so the only thing that helps distinguish the snapshots and what they're intended to show is the name of the file being linted.
Can probably improve this somehow.
Part of #15972.
Built from the guts of #16583 (basically the same PR as that one, but the refactors over the last few weeks made rebasing too difficult so I copied most of the changes over manually).
Examples:
{ "linterOptions": { "typeAware": false }, "rules": { /// Even though it's set to `error,` this is NOT run because it is a type-aware rule and we have `typeAware: false`. "typescript/no-floating-promises": "error" } }{ "linterOptions": { "typeAware": true }, "rules": { /// This is a type-aware rule and will be run because `typeAware` is true in linterOptions. "typescript/no-floating-promises": "error" } }How this works currently:
typeAware: truein theoxlintrc.jsonfile to get type-aware linting without needing the--type-awareflag.false.typeAware: falseor have it unset, it will not be enabled.--type-awareCLI flag overridestypeAware: falsein the config, and type-aware linting is enabled.typeAware: falsein the oxlint config, and type-aware linting is enabled.So, the major problem that is still remaining is full nested config support. We either need to:
The latter is outside my current ability level with Rust, frankly, so I'll need someone else to tackle that part assuming we want to do it, which we maybe should? I'm still a bit hesitant.
(Alternatively, I guess another option is to ship the current behavior, where any config - nested or base - enabling type-aware linting means all configs enable type-aware linting, but I don't love that behavior).
And here's an example of current behavior working correctly :)
(note that the output of this rule is busted, see oxc-project/tsgolint#595)