Skip to content

Commit f625cd6

Browse files
ianobermillermysticatea
ianobermiller
authored and
mysticatea
committed
Update: make no-unused-disable fixable (#13)
1 parent c427c1c commit f625cd6

File tree

4 files changed

+132
-19
lines changed

4 files changed

+132
-19
lines changed

‎docs/rules/no-unused-disable.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
# disallow unused `eslint-disable` comments (eslint-comments/no-unused-disable)
1+
# disallows unused `eslint-disable` comments (eslint-comments/no-unused-disable)
2+
3+
- ✒️ This rule is fixable. The fixer removes the comment. Note that it removes the entire comment, even if multiple rules were disabled, and only one was unused, which is likely undesirable.
24

35
Since refactoring or a bug fix of upstream, an `eslint-disable` directive-comment may become unnecessary.
46
In that case, you should remove the garbage as soon as possible since the garbage may cause to overlook ESLint warnings in future.

‎lib/rules/no-unused-disable.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ module.exports = {
1616
url:
1717
"https://mysticatea.github.io/eslint-plugin-eslint-comments/rules/no-unused-disable.html",
1818
},
19-
fixable: null,
19+
fixable: "code",
2020
schema: [],
2121
},
2222

‎lib/utils/patch.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,17 @@ function createNoUnusedDisableError(ruleId, severity, message, comment) {
9898
clone.endLine = comment.loc.end.line
9999
clone.endColumn = comment.loc.end.column + 1
100100
}
101+
102+
// Remove the whole node if it is the only rule, otherwise
103+
// don't try to fix because it is quite complicated.
104+
if (!comment.value.includes(",")) {
105+
// We can't use the typical `fixer` helper because we are injecting
106+
// this message after the fixes are resolved.
107+
clone.fix = {
108+
range: comment.range,
109+
text: comment.value.includes("\n") ? "\n" : "",
110+
}
111+
}
101112
}
102113

103114
return clone

‎tests/lib/rules/no-unused-disable.js

Lines changed: 117 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,13 @@ function baz() {
143143
})
144144

145145
describe("invalid", () => {
146-
for (const { code, errors, reportUnusedDisableDirectives } of [
146+
for (const testCase of [
147147
{
148+
title: "Generic same line",
148149
code: `/*eslint no-undef:off*/
149150
var a = b //eslint-disable-line`,
151+
output: `/*eslint no-undef:off*/
152+
var a = b `,
150153
errors: [
151154
{
152155
message:
@@ -159,8 +162,11 @@ var a = b //eslint-disable-line`,
159162
],
160163
},
161164
{
165+
title: "Specific same line",
162166
code: `/*eslint no-undef:off*/
163167
var a = b //eslint-disable-line no-undef`,
168+
output: `/*eslint no-undef:off*/
169+
var a = b `,
164170
errors: [
165171
{
166172
message:
@@ -173,6 +179,7 @@ var a = b //eslint-disable-line no-undef`,
173179
],
174180
},
175181
{
182+
title: "Multiple in a same line",
176183
code: `/*eslint no-undef:off, no-unused-vars:off*/
177184
var a = b //eslint-disable-line no-undef,no-unused-vars`,
178185
errors: [
@@ -195,8 +202,12 @@ var a = b //eslint-disable-line no-undef,no-unused-vars`,
195202
],
196203
},
197204
{
205+
title: "Generic next line",
198206
code: `/*eslint no-undef:off*/
199207
//eslint-disable-next-line
208+
var a = b`,
209+
output: `/*eslint no-undef:off*/
210+
200211
var a = b`,
201212
errors: [
202213
{
@@ -210,8 +221,12 @@ var a = b`,
210221
],
211222
},
212223
{
224+
title: "Specific next line",
213225
code: `/*eslint no-undef:off*/
214226
//eslint-disable-next-line no-undef
227+
var a = b`,
228+
output: `/*eslint no-undef:off*/
229+
215230
var a = b`,
216231
errors: [
217232
{
@@ -225,6 +240,7 @@ var a = b`,
225240
],
226241
},
227242
{
243+
title: "Multiple next line",
228244
code: `/*eslint no-undef:off, no-unused-vars:off*/
229245
//eslint-disable-next-line no-undef,no-unused-vars
230246
var a = b`,
@@ -248,8 +264,12 @@ var a = b`,
248264
],
249265
},
250266
{
267+
title: "Generic block",
251268
code: `/*eslint no-undef:off*/
252269
/*eslint-disable*/
270+
var a = b`,
271+
output: `/*eslint no-undef:off*/
272+
253273
var a = b`,
254274
errors: [
255275
{
@@ -263,8 +283,29 @@ var a = b`,
263283
],
264284
},
265285
{
286+
title: "Replaces multi-line block comments with a newline",
287+
code: `foo/* eslint-disable
288+
*/ bar`,
289+
output: `foo
290+
bar`,
291+
errors: [
292+
{
293+
message:
294+
"ESLint rules are disabled but never reported.",
295+
line: 1,
296+
column: 4,
297+
endLine: 2,
298+
endColumn: 3,
299+
},
300+
],
301+
},
302+
{
303+
title: "Specific block",
266304
code: `/*eslint no-undef:off*/
267305
/*eslint-disable no-undef*/
306+
var a = b`,
307+
output: `/*eslint no-undef:off*/
308+
268309
var a = b`,
269310
errors: [
270311
{
@@ -278,6 +319,7 @@ var a = b`,
278319
],
279320
},
280321
{
322+
title: "Multiple block",
281323
code: `/*eslint no-undef:off, no-unused-vars:off*/
282324
/*eslint-disable no-undef,no-unused-vars*/
283325
var a = b`,
@@ -301,8 +343,13 @@ var a = b`,
301343
],
302344
},
303345
{
346+
title: "Generic block with enable after",
304347
code: `/*eslint no-undef:off*/
305348
/*eslint-disable*/
349+
var a = b
350+
/*eslint-enable*/`,
351+
output: `/*eslint no-undef:off*/
352+
306353
var a = b
307354
/*eslint-enable*/`,
308355
errors: [
@@ -317,8 +364,13 @@ var a = b
317364
],
318365
},
319366
{
367+
title: "Specific block with enable after",
320368
code: `/*eslint no-undef:off*/
321369
/*eslint-disable no-undef*/
370+
var a = b
371+
/*eslint-enable*/`,
372+
output: `/*eslint no-undef:off*/
373+
322374
var a = b
323375
/*eslint-enable*/`,
324376
errors: [
@@ -333,6 +385,7 @@ var a = b
333385
],
334386
},
335387
{
388+
title: "Multiple block with enable after",
336389
code: `/*eslint no-undef:off, no-unused-vars:off*/
337390
/*eslint-disable no-undef,no-unused-vars*/
338391
var a = b
@@ -357,8 +410,13 @@ var a = b
357410
],
358411
},
359412
{
413+
title: "Generic block disable with no error inside",
360414
code: `/*eslint no-undef:error*/
361415
/*eslint-disable*/
416+
/*eslint-enable*/
417+
var a = b//eslint-disable-line no-undef`,
418+
output: `/*eslint no-undef:error*/
419+
362420
/*eslint-enable*/
363421
var a = b//eslint-disable-line no-undef`,
364422
errors: [
@@ -373,8 +431,13 @@ var a = b//eslint-disable-line no-undef`,
373431
],
374432
},
375433
{
434+
title: "Specific block disable with no error inside",
376435
code: `/*eslint no-undef:error*/
377436
/*eslint-disable no-undef*/
437+
/*eslint-enable no-undef*/
438+
var a = b//eslint-disable-line no-undef`,
439+
output: `/*eslint no-undef:error*/
440+
378441
/*eslint-enable no-undef*/
379442
var a = b//eslint-disable-line no-undef`,
380443
errors: [
@@ -389,6 +452,7 @@ var a = b//eslint-disable-line no-undef`,
389452
],
390453
},
391454
{
455+
title: "Multiple specific block disable with no error inside",
392456
code: `/*eslint no-undef:error, no-unused-vars:error*/
393457
/*eslint-disable no-undef,no-unused-vars*/
394458
/*eslint-enable no-undef*/
@@ -405,6 +469,8 @@ var a = b//eslint-disable-line no-undef`,
405469
],
406470
},
407471
{
472+
title:
473+
"Multiple specific block disable with only one error inside",
408474
code: `/*eslint no-undef:error, no-unused-vars:error*/
409475
/*eslint-disable
410476
no-undef,
@@ -424,8 +490,10 @@ var a = b
424490
],
425491
},
426492
{
493+
title: "Specific block disable at end of input",
427494
code:
428495
"/* eslint new-parens:error*/ /*eslint-disable new-parens*/",
496+
output: "/* eslint new-parens:error*/ ",
429497
errors: [
430498
{
431499
message:
@@ -438,8 +506,11 @@ var a = b
438506
],
439507
},
440508
{
509+
title: "Generic same line with rule off",
441510
code: `/*eslint no-undef:off*/
442511
var a = b //eslint-disable-line`,
512+
output: `/*eslint no-undef:off*/
513+
var a = b `,
443514
errors: [
444515
{
445516
message:
@@ -457,8 +528,11 @@ var a = b //eslint-disable-line`,
457528
reportUnusedDisableDirectives: true,
458529
},
459530
{
531+
title: "Specific same line with rule off",
460532
code: `/*eslint no-undef:off*/
461533
var a = b //eslint-disable-line no-undef`,
534+
output: `/*eslint no-undef:off*/
535+
var a = b `,
462536
errors: [
463537
{
464538
message:
@@ -476,8 +550,8 @@ var a = b //eslint-disable-line no-undef`,
476550
reportUnusedDisableDirectives: true,
477551
},
478552

479-
// Don't crash even if the source code has a parse error.
480553
{
554+
title: "Don't crash even if the source code has a parse error",
481555
code:
482556
"/*eslint no-undef:error*/\nvar a = b c //eslint-disable-line no-undef",
483557
errors: [
@@ -487,24 +561,50 @@ var a = b //eslint-disable-line no-undef`,
487561
],
488562
},
489563
]) {
490-
it(code, () =>
491-
runESLint(code, reportUnusedDisableDirectives).then(
492-
actualMessages => {
493-
assert.strictEqual(actualMessages.length, errors.length)
494-
for (let i = 0; i < errors.length; ++i) {
495-
const actual = actualMessages[i]
496-
const expected = errors[i]
564+
it(testCase.title || testCase.code, () =>
565+
runESLint(
566+
testCase.code,
567+
testCase.reportUnusedDisableDirectives
568+
).then(actualMessages => {
569+
assert.strictEqual(
570+
actualMessages.length,
571+
testCase.errors.length
572+
)
573+
574+
let actualOutput = testCase.code
497575

498-
for (const key of Object.keys(expected)) {
499-
assert.strictEqual(
500-
actual[key],
501-
expected[key],
502-
`'${key}' is not expected.`
503-
)
504-
}
576+
for (let i = 0; i < testCase.errors.length; ++i) {
577+
const actual = actualMessages[i]
578+
const expected = testCase.errors[i]
579+
580+
// We need to duplicate the simple logic in ESLint's
581+
// source-code-fixer.js to apply the fix. If we run
582+
// ESLint with --fix-dry-run, it won't report any
583+
// errors since it would have fixed them.
584+
if (actual.fix) {
585+
actualOutput =
586+
actualOutput.slice(0, actual.fix.range[0]) +
587+
actual.fix.text +
588+
actualOutput.slice(actual.fix.range[1])
505589
}
590+
591+
for (const key of Object.keys(expected)) {
592+
assert.strictEqual(
593+
actual[key],
594+
expected[key],
595+
`'${key}' is not expected.`
596+
)
597+
}
598+
}
599+
600+
if (testCase.output) {
601+
assert.strictEqual(
602+
actualOutput,
603+
testCase.output,
604+
"output is not expected"
605+
)
506606
}
507-
)
607+
})
508608
)
509609
}
510610
})

0 commit comments

Comments
 (0)