fix(language_server): append rule to existing disable comment instead of creating duplicate

When using the 'Disable rule for this line' quick fix, if there's already a
disable-next-line comment on the line above, append the new rule to the existing
comment using comma separation instead of creating a new duplicate comment.

Fixes #16060

Author: camc314

crates/oxc_language_server/src/linter/error_with_position.rs

@@ -7,7 +7,7 @@ use tower_lsp_server::lsp_types::{
 
 use oxc_data_structures::rope::{Rope, get_line_column};
 use oxc_diagnostics::{OxcCode, Severity};
-use oxc_linter::{Fix, Message, PossibleFixes};
+use oxc_linter::{DisableDirectives, Fix, Message, PossibleFixes, RuleCommentType};
 
 #[derive(Debug, Clone, Default)]
 pub struct DiagnosticReport {
@@ -38,6 +38,7 @@ pub fn message_to_lsp_diagnostic(
     uri: &Uri,
     source_text: &str,
     rope: &Rope,
+    directives: Option<&DisableDirectives>,
 ) -> DiagnosticReport {
     let severity = match message.error.severity {
         Severity::Error => Some(lsp_types::DiagnosticSeverity::ERROR),
@@ -130,6 +131,7 @@ pub fn message_to_lsp_diagnostic(
         section_offset,
         rope,
         source_text,
+        directives,
     );
 
     DiagnosticReport { diagnostic, fixed_content }
@@ -203,6 +205,7 @@ fn add_ignore_fixes(
     section_offset: u32,
     rope: &Rope,
     source_text: &str,
+    directives: Option<&DisableDirectives>,
 ) -> PossibleFixContent {
     // do not append ignore code actions when the error is the ignore action
     if matches!(fixes, PossibleFixContent::Single(ref fix) if fix.message.as_ref().is_some_and(|message| message.starts_with("remove unused disable directive")))
@@ -218,13 +221,13 @@ fn add_ignore_fixes(
     }
 
     if let Some(rule_name) = code.number.as_ref() {
-        // TODO: doesn't support disabling multiple rules by name for a given line.
         new_fixes.push(disable_for_this_line(
             rule_name,
             error_offset,
             section_offset,
             rope,
             source_text,
+            directives,
         ));
         new_fixes.push(disable_for_this_section(rule_name, section_offset, rope, source_text));
     }
@@ -244,6 +247,76 @@ fn disable_for_this_line(
     section_offset: u32,
     rope: &Rope,
     source_text: &str,
+    directives: Option<&DisableDirectives>,
+) -> FixedContent {
+    if let Some(directives) = directives
+        && let Some(existing_comment) =
+            directives.find_disable_next_line_comment_for_position(error_offset)
+    {
+        return append_rule_to_existing_comment(rule_name, existing_comment, rope, source_text);
+    }
+
+    create_new_disable_comment(rule_name, error_offset, section_offset, rope, source_text)
+}
+
+/// Append a rule to an existing disable-next-line comment, or return a no-op if the rule already exists.
+fn append_rule_to_existing_comment(
+    rule_name: &str,
+    existing_comment: &oxc_linter::DisableRuleComment,
+    rope: &Rope,
+    source_text: &str,
+) -> FixedContent {
+    let comment_span = existing_comment.span;
+    let comment_text = &source_text[comment_span.start as usize..comment_span.end as usize];
+
+    // Get the existing rules from the comment
+    let existing_rules: Vec<&str> = match &existing_comment.r#type {
+        RuleCommentType::All => {
+            // If it's an "all" directive, just return a no-op (can't add more rules)
+            let start_position = offset_to_position(rope, comment_span.start, source_text);
+            let end_position = offset_to_position(rope, comment_span.end, source_text);
+            return FixedContent {
+                message: Some(format!("Disable {rule_name} for this line")),
+                code: comment_text.to_string(),
+                range: Range::new(start_position, end_position),
+            };
+        }
+        RuleCommentType::Single(rules) => rules.iter().map(|r| r.rule_name.as_str()).collect(),
+    };
+
+    // Check if the rule is already in the comment
+    if existing_rules.contains(&rule_name) {
+        // Rule already exists, return a no-op fix (same content)
+        let start_position = offset_to_position(rope, comment_span.start, source_text);
+        let end_position = offset_to_position(rope, comment_span.end, source_text);
+        return FixedContent {
+            message: Some(format!("Disable {rule_name} for this line")),
+            code: comment_text.to_string(),
+            range: Range::new(start_position, end_position),
+        };
+    }
+
+    // Append the new rule to the comment using comma separation (ESLint standard format)
+    // The comment_text is just the content inside the comment (without // prefix for line comments)
+    let new_comment = format!("{comment_text}, {rule_name}");
+
+    let start_position = offset_to_position(rope, comment_span.start, source_text);
+    let end_position = offset_to_position(rope, comment_span.end, source_text);
+
+    FixedContent {
+        message: Some(format!("Disable {rule_name} for this line")),
+        code: new_comment,
+        range: Range::new(start_position, end_position),
+    }
+}
+
+/// Create a new disable-next-line comment when no existing comment is found.
+fn create_new_disable_comment(
+    rule_name: &str,
+    error_offset: u32,
+    section_offset: u32,
+    rope: &Rope,
+    source_text: &str,
 ) -> FixedContent {
     let bytes = source_text.as_bytes();
     // Find the line break before the error
@@ -436,7 +509,7 @@ mod test {
     fn disable_for_this_line_single_line() {
         let source = "console.log('hello');";
         let rope = Rope::from_str(source);
-        let fix = super::disable_for_this_line("no-console", 0, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 0, 0, &rope, source, None);
 
         assert_eq!(fix.code, "// oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 0);
@@ -447,7 +520,7 @@ mod test {
     fn disable_for_this_line_with_spaces() {
         let source = "  console.log('hello');";
         let rope = Rope::from_str(source);
-        let fix = super::disable_for_this_line("no-console", 10, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 10, 0, &rope, source, None);
 
         assert_eq!(fix.code, "  // oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 0);
@@ -458,7 +531,7 @@ mod test {
     fn disable_for_this_line_with_tabs() {
         let source = "\t\tconsole.log('hello');";
         let rope = Rope::from_str(source);
-        let fix = super::disable_for_this_line("no-console", 10, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 10, 0, &rope, source, None);
 
         assert_eq!(fix.code, "\t\t// oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 0);
@@ -469,7 +542,7 @@ mod test {
     fn disable_for_this_line_mixed_tabs_spaces() {
         let source = "\t  \tconsole.log('hello');";
         let rope = Rope::from_str(source);
-        let fix = super::disable_for_this_line("no-console", 12, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 12, 0, &rope, source, None);
 
         assert_eq!(fix.code, "\t  \t// oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 0);
@@ -480,7 +553,7 @@ mod test {
     fn disable_for_this_line_multiline_with_tabs() {
         let source = "function test() {\n\tconsole.log('hello');\n}";
         let rope = Rope::from_str(source);
-        let fix = super::disable_for_this_line("no-console", 27, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 27, 0, &rope, source, None);
 
         assert_eq!(fix.code, "\t// oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 1);
@@ -491,7 +564,7 @@ mod test {
     fn disable_for_this_line_multiline_with_spaces() {
         let source = "function test() {\n    console.log('hello');\n}";
         let rope = Rope::from_str(source);
-        let fix = super::disable_for_this_line("no-console", 30, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 30, 0, &rope, source, None);
 
         assert_eq!(fix.code, "    // oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 1);
@@ -502,7 +575,7 @@ mod test {
     fn disable_for_this_line_complex_indentation() {
         let source = "function test() {\n\t  \t  console.log('hello');\n}";
         let rope = Rope::from_str(source);
-        let fix = super::disable_for_this_line("no-console", 33, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 33, 0, &rope, source, None);
 
         assert_eq!(fix.code, "\t  \t  // oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 1);
@@ -513,7 +586,7 @@ mod test {
     fn disable_for_this_line_no_indentation() {
         let source = "function test() {\nconsole.log('hello');\n}";
         let rope = Rope::from_str(source);
-        let fix = super::disable_for_this_line("no-console", 26, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 26, 0, &rope, source, None);
 
         assert_eq!(fix.code, "// oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 1);
@@ -524,7 +597,7 @@ mod test {
     fn disable_for_this_line_crlf_with_tabs() {
         let source = "function test() {\r\n\tconsole.log('hello');\r\n}";
         let rope = Rope::from_str(source);
-        let fix = super::disable_for_this_line("no-console", 28, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 28, 0, &rope, source, None);
 
         assert_eq!(fix.code, "\t// oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 1);
@@ -535,7 +608,7 @@ mod test {
     fn disable_for_this_line_deeply_nested() {
         let source = "if (true) {\n\t\tif (nested) {\n\t\t\tconsole.log('deep');\n\t\t}\n}";
         let rope = Rope::from_str(source);
-        let fix = super::disable_for_this_line("no-console", 40, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 40, 0, &rope, source, None);
 
         assert_eq!(fix.code, "\t\t\t// oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 2);
@@ -546,7 +619,7 @@ mod test {
     fn disable_for_this_line_at_start_of_file() {
         let source = "console.log('hello');";
         let rope = Rope::from_str(source);
-        let fix = super::disable_for_this_line("no-console", 0, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 0, 0, &rope, source, None);
 
         assert_eq!(fix.code, "// oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 0);
@@ -559,7 +632,7 @@ mod test {
         let source = "function test() {\n  \tcode  \there\n}";
         let rope = Rope::from_str(source);
         // Error at position of 'code' (after "  \t")
-        let fix = super::disable_for_this_line("no-console", 21, 0, &rope, source);
+        let fix = super::disable_for_this_line("no-console", 21, 0, &rope, source, None);
 
         // Should only capture "  \t" at the beginning, not the spaces around "here"
         assert_eq!(fix.code, "  \t// oxlint-disable-next-line no-console\n");
@@ -574,8 +647,14 @@ mod test {
         let rope = Rope::from_str(source);
         let section_offset = 8; // At the \n after "<script>"
         let error_offset = 17; // At 'console'
-        let fix =
-            super::disable_for_this_line("no-console", error_offset, section_offset, &rope, source);
+        let fix = super::disable_for_this_line(
+            "no-console",
+            error_offset,
+            section_offset,
+            &rope,
+            source,
+            None,
+        );
 
         assert_eq!(fix.code, "// oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 1);
@@ -589,8 +668,14 @@ mod test {
         let rope = Rope::from_str(source);
         let section_offset = 8; // After "<script>"
         let error_offset = 16; // At 'console'
-        let fix =
-            super::disable_for_this_line("no-console", error_offset, section_offset, &rope, source);
+        let fix = super::disable_for_this_line(
+            "no-console",
+            error_offset,
+            section_offset,
+            &rope,
+            source,
+            None,
+        );
 
         assert_eq!(fix.code, "\n// oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 0);
@@ -604,8 +689,14 @@ mod test {
         let rope = Rope::from_str(source);
         let section_offset = 31; // At \n after "<script>"
         let error_offset = 36; // At 'console' (after "  ")
-        let fix =
-            super::disable_for_this_line("no-console", error_offset, section_offset, &rope, source);
+        let fix = super::disable_for_this_line(
+            "no-console",
+            error_offset,
+            section_offset,
+            &rope,
+            source,
+            None,
+        );
 
         assert_eq!(fix.code, "  // oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 3);
@@ -619,8 +710,14 @@ mod test {
         let rope = Rope::from_str(source);
         let section_offset = 8; // At the \n after "<script>"
         let error_offset = 8; // Error exactly at section offset
-        let fix =
-            super::disable_for_this_line("no-console", error_offset, section_offset, &rope, source);
+        let fix = super::disable_for_this_line(
+            "no-console",
+            error_offset,
+            section_offset,
+            &rope,
+            source,
+            None,
+        );
 
         assert_eq!(fix.code, "// oxlint-disable-next-line no-console\n");
         assert_eq!(fix.range.start.line, 1);

crates/oxc_language_server/src/linter/isolated_lint_handler.rs

@@ -123,21 +123,25 @@ impl IsolatedLintHandler {
 
         let fs = IsolatedLintHandlerFileSystem::new(path.to_path_buf(), Arc::from(source_text));
 
-        let mut messages: Vec<DiagnosticReport> = self
-            .runner
-            .run_source(&Arc::from(path.as_os_str()), source_text.to_string(), &fs)
+        let lint_messages =
+            self.runner.run_source(&Arc::from(path.as_os_str()), source_text.to_string(), &fs);
+
+        let directives = self.runner.directives_coordinator().get(path);
+
+        let mut messages: Vec<DiagnosticReport> = lint_messages
             .iter()
-            .map(|message| message_to_lsp_diagnostic(message, uri, source_text, rope))
+            .map(|message| {
+                message_to_lsp_diagnostic(message, uri, source_text, rope, directives.as_ref())
+            })
             .collect();
 
-        // Add unused directives if configured
         if let Some(severity) = self.unused_directives_severity
-            && let Some(directives) = self.runner.directives_coordinator().get(path)
+            && let Some(directives) = &directives
         {
             messages.extend(
-                create_unused_directives_messages(&directives, severity, source_text)
-                    .iter()
-                    .map(|message| message_to_lsp_diagnostic(message, uri, source_text, rope)),
+                create_unused_directives_messages(directives, severity, source_text).iter().map(
+                    |message| message_to_lsp_diagnostic(message, uri, source_text, rope, None),
+                ),
             );
         }
 

crates/oxc_language_server/src/linter/snapshots/fixtures_linter_unused_disabled_directives@test.js.snap

@@ -66,15 +66,15 @@ Title: Disable no-console for this line
 TextEdit: TextEdit {
     range: Range {
         start: Position {
-            line: 9,
-            character: 0,
+            line: 8,
+            character: 2,
         },
         end: Position {
-            line: 9,
-            character: 0,
+            line: 8,
+            character: 52,
         },
     },
-    new_text: "// oxlint-disable-next-line no-console\n",
+    new_text: " oxlint-disable-next-line no-debugger, no-for-loop, no-console",
 }