Skip to content

Commit 06ec43c

Browse files
committed
fix(linter): enhance handling of disable directives for next-line and line spans (#13157)
fixes #13155
1 parent f897e30 commit 06ec43c

File tree

1 file changed

+119
-12
lines changed

1 file changed

+119
-12
lines changed

crates/oxc_linter/src/disable_directives.rs

Lines changed: 119 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,26 @@ use crate::fixer::Fix;
1010

1111
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
1212
enum DisabledRule<'a> {
13-
All { comment_span: Span },
14-
Single { rule_name: &'a str, name_span: Span, comment_span: Span },
13+
All { comment_span: Span, is_next_line: bool },
14+
Single { rule_name: &'a str, name_span: Span, comment_span: Span, is_next_line: bool },
1515
}
1616

1717
impl DisabledRule<'_> {
1818
pub fn comment_span(&self) -> &Span {
1919
match self {
20-
DisabledRule::All { comment_span } | DisabledRule::Single { comment_span, .. } => {
20+
DisabledRule::All { comment_span, .. } | DisabledRule::Single { comment_span, .. } => {
2121
comment_span
2222
}
2323
}
2424
}
25+
26+
pub fn is_next_line(&self) -> bool {
27+
match self {
28+
DisabledRule::All { is_next_line, .. } | DisabledRule::Single { is_next_line, .. } => {
29+
*is_next_line
30+
}
31+
}
32+
}
2533
}
2634

2735
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
@@ -118,16 +126,44 @@ impl<'a> DisableDirectives<'a> {
118126
}
119127

120128
pub fn contains(&self, rule_name: &'static str, span: Span) -> bool {
129+
// For `eslint-disable-next-line` and `eslint-disable-line` directives, we only check
130+
// if the diagnostic's starting position falls within the disabled interval.
131+
// This prevents suppressing diagnostics for larger constructs (like functions) that
132+
// contain disabled lines.
133+
//
134+
// For regular `eslint-disable` directives (which disable rules for the rest of the file),
135+
// we check if any part of the diagnostic span overlaps with the disabled interval.
136+
// This ensures that diagnostics starting before the disable comment (like no-empty-file)
137+
// are still suppressed.
121138
let matched_intervals = self
122139
.intervals
123140
.find(span.start, span.end)
124141
.filter(|interval| {
125-
match interval.val {
142+
// Check if this rule should be disabled
143+
let rule_matches = match interval.val {
126144
DisabledRule::All { .. } => true,
127145
// Our rule name currently does not contain the prefix.
128146
// For example, this will match `@typescript-eslint/no-var-requires` given
129147
// our rule_name is `no-var-requires`.
130148
DisabledRule::Single { rule_name: name, .. } => name.contains(rule_name),
149+
};
150+
151+
if !rule_matches {
152+
return false;
153+
}
154+
155+
// Check if the diagnostic span is covered by this interval
156+
if interval.val.is_next_line() {
157+
// For next-line directives, only check if the diagnostic starts within the interval
158+
// We intentionally only check span.start (not span.end) to avoid suppressing
159+
// diagnostics for large constructs that merely contain the disabled line
160+
#[expect(clippy::suspicious_operation_groupings)]
161+
{
162+
span.start >= interval.start && span.start < interval.stop
163+
}
164+
} else {
165+
// For regular disable directives, check if there's any overlap
166+
span.start < interval.stop && span.end > interval.start
131167
}
132168
})
133169
.map(|interval| interval.val)
@@ -299,7 +335,7 @@ impl<'a> DisableDirectivesBuilder<'a> {
299335
self.add_interval(
300336
comment_span.end,
301337
stop,
302-
DisabledRule::All { comment_span },
338+
DisabledRule::All { comment_span, is_next_line: true },
303339
);
304340
self.disable_rule_comments.push(DisableRuleComment {
305341
span: comment_span,
@@ -312,7 +348,12 @@ impl<'a> DisableDirectivesBuilder<'a> {
312348
self.add_interval(
313349
comment_span.end,
314350
stop,
315-
DisabledRule::Single { rule_name, name_span, comment_span },
351+
DisabledRule::Single {
352+
rule_name,
353+
name_span,
354+
comment_span,
355+
is_next_line: true,
356+
},
316357
);
317358
rules.push(RuleCommentRule { rule_name, name_span });
318359
});
@@ -336,7 +377,11 @@ impl<'a> DisableDirectivesBuilder<'a> {
336377

337378
// `eslint-disable-line`
338379
if text.trim().is_empty() {
339-
self.add_interval(start, stop, DisabledRule::All { comment_span });
380+
self.add_interval(
381+
start,
382+
stop,
383+
DisabledRule::All { comment_span, is_next_line: true },
384+
);
340385
self.disable_rule_comments.push(DisableRuleComment {
341386
span: comment_span,
342387
r#type: RuleCommentType::All,
@@ -348,7 +393,12 @@ impl<'a> DisableDirectivesBuilder<'a> {
348393
self.add_interval(
349394
start,
350395
stop,
351-
DisabledRule::Single { rule_name, name_span, comment_span },
396+
DisabledRule::Single {
397+
rule_name,
398+
name_span,
399+
comment_span,
400+
is_next_line: true,
401+
},
352402
);
353403
rules.push(RuleCommentRule { rule_name, name_span });
354404
});
@@ -390,7 +440,7 @@ impl<'a> DisableDirectivesBuilder<'a> {
390440
self.add_interval(
391441
start,
392442
comment_span.start,
393-
DisabledRule::All { comment_span },
443+
DisabledRule::All { comment_span, is_next_line: false },
394444
);
395445
} else {
396446
// collect as unused enable (see more at note comments in beginning of this method)
@@ -403,7 +453,12 @@ impl<'a> DisableDirectivesBuilder<'a> {
403453
self.add_interval(
404454
start,
405455
comment_span.start,
406-
DisabledRule::Single { rule_name, name_span, comment_span },
456+
DisabledRule::Single {
457+
rule_name,
458+
name_span,
459+
comment_span,
460+
is_next_line: false,
461+
},
407462
);
408463
} else {
409464
// collect as unused enable (see more at note comments in beginning of this method)
@@ -416,7 +471,11 @@ impl<'a> DisableDirectivesBuilder<'a> {
416471

417472
// Lone `eslint-disable`
418473
if let Some((start, comment_span)) = self.disable_all_start {
419-
self.add_interval(start, source_len, DisabledRule::All { comment_span });
474+
self.add_interval(
475+
start,
476+
source_len,
477+
DisabledRule::All { comment_span, is_next_line: false },
478+
);
420479
}
421480

422481
// Lone `eslint-disable rule_name`
@@ -425,7 +484,7 @@ impl<'a> DisableDirectivesBuilder<'a> {
425484
self.add_interval(
426485
start,
427486
source_len,
428-
DisabledRule::Single { rule_name, name_span, comment_span },
487+
DisabledRule::Single { rule_name, name_span, comment_span, is_next_line: false },
429488
);
430489
}
431490

@@ -1059,11 +1118,13 @@ mod tests {
10591118
rule_name: "no-console",
10601119
name_span: Span::sized(comments[0].content_span().start + 16, 10),
10611120
comment_span: comments[0].content_span(),
1121+
is_next_line: false,
10621122
});
10631123
directives.mark_disable_directive_used(DisabledRule::Single {
10641124
rule_name: "no-debugger",
10651125
name_span: Span::sized(comments[1].content_span().start + 16, 11),
10661126
comment_span: comments[1].content_span(),
1127+
is_next_line: false,
10671128
});
10681129

10691130
assert!(directives.collect_unused_disable_comments().is_empty());
@@ -1126,4 +1187,50 @@ mod tests {
11261187
RuleCommentRule { rule_name: "max-params", name_span: Span::sized(28, 10) }
11271188
.create_fix(source_text, comment_span);
11281189
}
1190+
1191+
#[test]
1192+
fn test_disable_next_line_should_not_disable_large_span_diagnostics() {
1193+
// This test demonstrates that eslint-disable-next-line should NOT suppress
1194+
// diagnostics for larger constructs (like functions) that contain the disabled line.
1195+
// It should only suppress diagnostics that START on the disabled line.
1196+
let source_text = r"
1197+
function test() {
1198+
// eslint-disable-next-line
1199+
console.log('this line is disabled');
1200+
console.warn('this line is not disabled');
1201+
}
1202+
";
1203+
let allocator = Allocator::default();
1204+
let semantic = process_source(&allocator, source_text);
1205+
let directives =
1206+
DisableDirectivesBuilder::new().build(semantic.source_text(), semantic.comments());
1207+
1208+
// The function spans from line 2 to line 6 (positions 1 to 138)
1209+
let function_span = Span::new(1, 138);
1210+
1211+
// The diagnostic for the entire function should NOT be suppressed
1212+
// even though it contains a disable-next-line directive
1213+
assert!(
1214+
!directives.contains("max-lines-per-function", function_span),
1215+
"eslint-disable-next-line should not suppress diagnostics for the entire function"
1216+
);
1217+
1218+
// A diagnostic that starts on the disabled line (line 4) SHOULD be suppressed
1219+
// The first console.log on line 4 starts at position 59
1220+
let first_console_log_span = Span::new(55, 66);
1221+
assert_eq!(first_console_log_span.source_text(source_text), "console.log");
1222+
assert!(
1223+
directives.contains("no-console", first_console_log_span),
1224+
"eslint-disable-next-line should suppress diagnostics on the next line"
1225+
);
1226+
1227+
// A diagnostic that starts on a non-disabled line (line 5) should NOT be suppressed
1228+
// The second console.log on line 5 starts at position 102
1229+
let second_console_log_span = Span::new(97, 109);
1230+
assert_eq!(second_console_log_span.source_text(source_text), "console.warn");
1231+
assert!(
1232+
!directives.contains("no-console", second_console_log_span),
1233+
"eslint-disable-next-line should NOT suppress diagnostics on lines after the next line"
1234+
);
1235+
}
11291236
}

0 commit comments

Comments
 (0)