-
-
Notifications
You must be signed in to change notification settings - Fork 738
feat(linter): Add linterOptions.typeAware
#16583
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
This will ensure that teams can standardize whether they want type-aware linting enabled or not via their config file, rather than relying solely on the CLI flag and individual developers enabling type-aware rules in their editors. Mostly built with GitHub Copilot + Raptor mini. This still needs work to ensure that the flag works with the VS Code extension, and also to potentially nest this setting inside a `linterOptions` object. The documentation phrasing should also be improved. And I'm assuming the performance here may need to be improved. Nested configs should also error if they have this value set. And a few more tests should be added.
…unset, true, and false.
Ensure the intent and behavior are clear.
CodSpeed Performance ReportMerging #16583 will not alter performanceComparing Summary
Footnotes
|
| true | ||
| } else { | ||
| // ConfigStore::type_aware_enabled returns whether the base config requested it | ||
| config_store.type_aware_enabled() |
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.
Note that I gave type_aware the type Option because I intended to make it not effect nested config files, we can undo that if we want it to work for nested config files.
Nested Configs are the default. For me, it would be strange to only support it in the root config.
Example: Team X owns packages/package-X and they want to test type aware.
Team Y does not want type aware.
The extends should cover project wide configurations.
The Language server part should be here:
| type_aware: options.type_aware, |
Built with help from GitHub Copilot + Raptor mini (GitHub's preview model).
Part of #15972.
This allows setting the
typeAwareoption in the config file. It is nested under the new top-levellinterOptionsfield. The name was chosen as it's the one used by ESLint and will also hold things like thereportUnusedDisableDirectivessetting in the future.The goal of this to allow teams to set this in the config file, and that'll help ensure that everyone on a project is using the same linter rules. Otherwise, when I adopt this at work (hopefully in the near-ish future) I will need to have every dev on the team do unnecessary manual configuration.
Without this, everyone would need to configure their editor settings to enable type-aware lint rules and also ensure that they're running oxlint with the right command if they use it via the CLI.
My hope is that more linter options will be added under
linterOptionsin the near-ish future to enable more consistency like this, but this seemed like one of the more important options to add directly to the config file.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" } }TODO:
--type-awareflag docs.LintConfighave alinter_optionsfield at all, is that what determines the available options for a nested config file? Or should it be excluded like categories are?--disable-type-awareflag to disable the behavior, for the case where the config hastypeAware: true?Note that I gave
type_awarethe typeOption<bool>because I intended to make it not effect nested config files, we can undo that if we want it to work for nested config files.