Make no-unused-disable fixable #13
Conversation
|
Ironic, I will fix those lint errors :) |
Codecov Report
@@ Coverage Diff @@
## master #13 +/- ##
==========================================
+ Coverage 96.87% 96.94% +0.06%
==========================================
Files 25 25
Lines 320 327 +7
==========================================
+ Hits 310 317 +7
Misses 10 10
Continue to review full report at Codecov.
|
|
Thank you very much for this PR. However, unfortunately, I rewrote the If you are still interesting to implement this, you can investigate in lib/patch.js file. This file does the following actions:
|
|
I'll check that out, thanks! |
|
@ianobermiller Still interested in finishing this? :) |
|
Perhaps at some point, but it is no longer a priority. |
ianobermiller
force-pushed
the
unused-disable-fixer
branch
from
5df9cc1 to
4ca9160
Compare
|
That should do it :) |
ianobermiller
force-pushed
the
unused-disable-fixer
branch
from
4ca9160 to
79a8f2e
Compare
lib/utils/patch.js
Outdated
| } else { | ||
| clone.endLine = comment.loc.end.line | ||
| clone.endColumn = comment.loc.end.column + 1 | ||
| clone.fix = function fix(fixer) { | ||
| return fixer.remove(comment) |
There was a problem hiding this comment.
Please keep line breaks which were in the comment. This can make invalid syntax in semicolon-less style. For example:
foo /*eslint-disable
*/ bar
↓↓↓
foo barThere was a problem hiding this comment.
@ianobermiller Thank you for the update! LGTM except this matter. Would you make the replaced text comment.value.includes("\n") ? "\n" : "" or something like (and, add a test case)?
There was a problem hiding this comment.
Missed that one from the original review, thanks!
ianobermiller
force-pushed
the
unused-disable-fixer
branch
2 times, most recently
from
e2c4e67 to
11dcac7
Compare
|
@mysticatea the fixer will run only when there is a single rule (or no particular rule) being disabled. |
Summary: Add a fixer for no-unused-disable that just removes the entire comment node. This behavior is undesirable if you frequently disable multiple rules at a time, as it removes the entire comment even if only one was unused. Since disabling multiple rules is the exception, I still think this fixer has tremendous value. Test plan: - `npm run test` - run the rule on a handful of files in a local project, note that it finds many issues and successfully fixes them
ianobermiller
force-pushed
the
unused-disable-fixer
branch
from
11dcac7 to
1da89ae
Compare
|
I apologize for my delay. I had been diving into busy days. LGTM, thank you so much! |
This reverts commit f625cd6. # Conflicts: # docs/rules/no-unused-disable.md
Summary:
Add a fixer for no-unused-disable that just removes the entire comment node. If the comment is disabling multiple rules at once, do not offer to fix.
Test plan:
npm run testfinds many issues and successfully fixes them