-
-
Notifications
You must be signed in to change notification settings - Fork 477
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
linter: rule fixer improvements #4179
Comments
Part of #4179. Adds a fixer for some common typos.
Part of #4179. Fixes cases like: ```js if (foo) debugger // got fixed into if (foo) // but now gets fixed to if (foo) {} ```
no-const-assign cannot have a fixer because we cannot infer the intend of the user . Or we can add a dangerous fix that will replace the const with a let. |
…-trim-start-end Part of oxc-project#4179
Related to fixes, can oxlint's diagnostic also print that a fix is available. Ruff for example shows:
oxlint prints no information at the moment:
|
@jelly check out my PR with suggestions; it does this. |
Adding tests for the fixer uncovered some bugs where it would produce the wrong results or invalid JavaScript. Related: oxc-project#4179
I think another task should be added, rules with fixers without tests. Last night I discovered
While:
I simply copied some of the failing test cases into a JS file, made a backup and ran eslint fix on it as unicorn sadly has no tests for the fixer it seems. Collected the list of fixers without
|
Add a `FIX` constant to `RuleMeta` that describes what kinds of auto fixes a rule can perform (if any). This PR also updates `declare_oxc_lint` to accept a fix capabilities field. Follow-up PRs in this stack will update existing rules and update other parts of the codebase to use this new field. The end goal of this stack is to 1. automate creation of #4179 in a similar way we auto-update rule progress, 2. use it in rule documentation pages when we eventually add lint rules to the website
For |
I'm not sure if I'm getting it right or not, So feel free to correct me. If you are talking about adding new fixers where there isn't one in eslint(or other linting solutions), It is completely welcomed here, We want backward compatibility with eslint as a drop-in replacement for most parts; But as long as we are adding(and not subtracting) features, It is allowed with no restrictions whatsoever. As long as it makes sense and is useful. Although for non-exiting fixers/linters, you'd have to write through tests - since there isn't any source to borrow the tests from - and it might need a more rigorous review process to make sure it is doing the intended behavior we have in mind and assess the risks for the fixer(in case it have to be a dangerous fix). With that said if you ever think the change you have in mind might be controversial it'd be best to create an issue for it beforehand or ask for reviews early in the process to make sure there aren't any strong oppositions against it. The last thing we want is to allow contributors' time to go to waste. Let me know if I didn't answer your question. |
Yes, thank you. That's what I wanted to ask. |
@heygsc it depends. I'd recommend starting there if you're just getting your feet wet. Porting existing fixers is definitely easier than writing new ones. In some cases, rules intentionally do not have auto fixes for DX reasons. For example, imagine an unused import getting deleted when your editor saves a file. That would get really annoying quite quickly. For those cases, we've added infrastructure for suggestions and "dangerous" fixes, which do not get applied when --fix is used, but instead get their own CLI flags. |
What you said makes sense. Thank you. |
part of #4179 --------- Co-authored-by: wenzhe <mysteryven@gmail.com>
Close as stale given no progress has been made, but feel free to implement fixers when needed. |
I've been adding and repairing fixers to a lot of rules, but my ad-hoc approach
is not sustainable. This issue is a call for assistance, and tracks fixer
progress for each rule.
Prerequisites
--fix-dangerously
flag #4211--fix
oxlint-ecosystem-ci#19Rules
For rules with an existing fixer:
diagnostic_with_fix
), ensure that there areno cases where the fixer could create a syntax error or semantically different
code.
differently different code in others, use
diagnostic_with_fix_dangerous
.update this table. If no changes are needed, leave a comment here and we'll
also update the table.
For rules without a fixer:
a fixer implementation that links to this issue.
a maintainer will update the table.
Legend
Correctness (159):
Code that is outright wrong or useless.
Perf (6):
Code that can be written to run faster.
Restriction (49):
Lints which prevent the use of language and library features. Must not be enabled as a whole, should be considered on a case-by-case basis before enabling.
Suspicious (14):
code that is most likely wrong or useless.
Pedantic (66):
Lints which are rather strict or have occasional false positives.
Style (80):
Code that should be written in a more idiomatic way.
Nursery (9):
New lints that are still under development.
The text was updated successfully, but these errors were encountered: