ruff server: Support noqa comment code action (#11276)

## Summary

Fixes #10594.

Code actions to disable a diagnostic via `noqa` comment are now
available.


https://github.com/astral-sh/ruff/assets/19577865/6d3bcf11-a9d9-499b-8c7f-a10cd39cfbba

`DiagnosticFix` has been changed so that `noqa` code actions appear even
for diagnostics with no available quick fix. It can contain quick fix
edits, `noqa` comment edits, or both.

## Test Plan

The scenarios that need to be tested are as follows:
* A code action to disable a diagnostic should be available for every
diagnostic.
* Using this code action should append to the appropriate line with the
diagnostic, or modify an existing `noqa` comment.
* Adding a `noqa` comment manually should make a diagnostic disappear
* `Fix all auto-fixable problems` should not add `noqa` comments
* Removing a code from a `noqa` comment should make the diagnostic
re-appear

crates/ruff_server/src/lint.rs

@@ -1,8 +1,9 @@
 //! Access to the Ruff linting API for the LSP
 
-use ruff_diagnostics::{Applicability, Diagnostic, DiagnosticKind, Fix};
+use ruff_diagnostics::{Applicability, Diagnostic, DiagnosticKind, Edit, Fix};
 use ruff_linter::{
     directives::{extract_directives, Flags},
+    generate_noqa_edits,
     linter::{check_path, LinterResult, TokenSource},
     packaging::detect_package_root,
     registry::AsRule,
@@ -24,17 +25,29 @@ use crate::{edit::ToRangeExt, PositionEncoding, DIAGNOSTIC_NAME};
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub(crate) struct AssociatedDiagnosticData {
     pub(crate) kind: DiagnosticKind,
-    pub(crate) fix: Fix,
+    /// A possible fix for the associated diagnostic.
+    pub(crate) fix: Option<Fix>,
+    /// The NOQA code for the diagnostic.
     pub(crate) code: String,
+    /// Possible edit to add a `noqa` comment which will disable this diagnostic.
+    pub(crate) noqa_edit: Option<ruff_diagnostics::Edit>,
 }
 
-/// Describes a fix for `fixed_diagnostic` that applies `document_edits` to the source.
+/// Describes a fix for `fixed_diagnostic` that may have quick fix
+/// edits available, `noqa` comment edits, or both.
 #[derive(Clone, Debug)]
 pub(crate) struct DiagnosticFix {
+    /// The original diagnostic to be fixed
     pub(crate) fixed_diagnostic: lsp_types::Diagnostic,
+    /// The message describing what the fix does.
     pub(crate) title: String,
+    /// The NOQA code for the diagnostic.
     pub(crate) code: String,
+    /// Edits to fix the diagnostic. If this is empty, a fix
+    /// does not exist.
     pub(crate) edits: Vec<lsp_types::TextEdit>,
+    /// Possible edit to add a `noqa` comment which will disable this diagnostic.
+    pub(crate) noqa_edit: Option<lsp_types::TextEdit>,
 }
 
 pub(crate) fn check(
@@ -75,7 +88,7 @@ pub(crate) fn check(
     let indexer = Indexer::from_tokens(&tokens, &locator);
 
     // Extract the `# noqa` and `# isort: skip` directives from the source.
-    let directives = extract_directives(&tokens, Flags::empty(), &locator, &indexer);
+    let directives = extract_directives(&tokens, Flags::all(), &locator, &indexer);
 
     // Generate checks.
     let LinterResult {
@@ -94,9 +107,20 @@ pub(crate) fn check(
         TokenSource::Tokens(tokens),
     );
 
+    let noqa_edits = generate_noqa_edits(
+        &document_path,
+        diagnostics.as_slice(),
+        &locator,
+        indexer.comment_ranges(),
+        &linter_settings.external,
+        &directives.noqa_line_for,
+        stylist.line_ending(),
+    );
+
     diagnostics
         .into_iter()
-        .map(|diagnostic| to_lsp_diagnostic(diagnostic, document, encoding))
+        .zip(noqa_edits)
+        .map(|(diagnostic, noqa_edit)| to_lsp_diagnostic(diagnostic, noqa_edit, document, encoding))
         .collect()
 }
 
@@ -118,22 +142,43 @@ pub(crate) fn fixes_for_diagnostics(
                 })?;
             let edits = associated_data
                 .fix
-                .edits()
-                .iter()
-                .map(|edit| lsp_types::TextEdit {
-                    range: edit
-                        .range()
-                        .to_range(document.contents(), document.index(), encoding),
-                    new_text: edit.content().unwrap_or_default().to_string(),
-                });
+                .map(|fix| {
+                    fix.edits()
+                        .iter()
+                        .map(|edit| lsp_types::TextEdit {
+                            range: edit.range().to_range(
+                                document.contents(),
+                                document.index(),
+                                encoding,
+                            ),
+                            new_text: edit.content().unwrap_or_default().to_string(),
+                        })
+                        .collect()
+                })
+                .unwrap_or_default();
+
+            let noqa_edit =
+                associated_data
+                    .noqa_edit
+                    .as_ref()
+                    .map(|noqa_edit| lsp_types::TextEdit {
+                        range: noqa_edit.range().to_range(
+                            document.contents(),
+                            document.index(),
+                            encoding,
+                        ),
+                        new_text: noqa_edit.content().unwrap_or_default().to_string(),
+                    });
+
             Ok(Some(DiagnosticFix {
                 fixed_diagnostic,
                 code: associated_data.code,
                 title: associated_data
                     .kind
                     .suggestion
                     .unwrap_or(associated_data.kind.name),
-                edits: edits.collect(),
+                edits,
+                noqa_edit,
             }))
         })
         .filter_map(crate::Result::transpose)
@@ -142,6 +187,7 @@ pub(crate) fn fixes_for_diagnostics(
 
 fn to_lsp_diagnostic(
     diagnostic: Diagnostic,
+    noqa_edit: Option<Edit>,
     document: &crate::edit::Document,
     encoding: PositionEncoding,
 ) -> lsp_types::Diagnostic {
@@ -151,18 +197,19 @@ fn to_lsp_diagnostic(
 
     let rule = kind.rule();
 
-    let data = fix.and_then(|fix| {
-        fix.applies(Applicability::Unsafe)
-            .then(|| {
-                serde_json::to_value(&AssociatedDiagnosticData {
-                    kind: kind.clone(),
-                    fix,
-                    code: rule.noqa_code().to_string(),
-                })
-                .ok()
+    let fix = fix.and_then(|fix| fix.applies(Applicability::Unsafe).then_some(fix));
+
+    let data = (fix.is_some() || noqa_edit.is_some())
+        .then(|| {
+            serde_json::to_value(&AssociatedDiagnosticData {
+                kind: kind.clone(),
+                fix,
+                code: rule.noqa_code().to_string(),
+                noqa_edit,
             })
-            .flatten()
-    });
+            .ok()
+        })
+        .flatten();
 
     let code = rule.noqa_code().to_string();
 

crates/ruff_server/src/server/api/requests/code_action.rs

@@ -1,5 +1,5 @@
 use crate::edit::WorkspaceEditTracker;
-use crate::lint::fixes_for_diagnostics;
+use crate::lint::{fixes_for_diagnostics, DiagnosticFix};
 use crate::server::api::LSPResult;
 use crate::server::SupportedCodeAction;
 use crate::server::{client::Notifier, Result};
@@ -29,13 +29,24 @@ impl super::BackgroundDocumentRequestHandler for CodeActions {
 
         let supported_code_actions = supported_code_actions(params.context.only.clone());
 
+        let fixes = fixes_for_diagnostics(
+            snapshot.document(),
+            snapshot.encoding(),
+            params.context.diagnostics,
+        )
+        .with_failure_code(ErrorCode::InternalError)?;
+
         if snapshot.client_settings().fix_violation()
             && supported_code_actions.contains(&SupportedCodeAction::QuickFix)
         {
-            response.extend(
-                quick_fix(&snapshot, params.context.diagnostics.clone())
-                    .with_failure_code(ErrorCode::InternalError)?,
-            );
+            response
+                .extend(quick_fix(&snapshot, &fixes).with_failure_code(ErrorCode::InternalError)?);
+        }
+
+        if snapshot.client_settings().noqa_comments()
+            && supported_code_actions.contains(&SupportedCodeAction::QuickFix)
+        {
+            response.extend(noqa_comments(&snapshot, &fixes));
         }
 
         if snapshot.client_settings().fix_all()
@@ -56,21 +67,19 @@ impl super::BackgroundDocumentRequestHandler for CodeActions {
 
 fn quick_fix(
     snapshot: &DocumentSnapshot,
-    diagnostics: Vec<types::Diagnostic>,
+    fixes: &[DiagnosticFix],
 ) -> crate::Result<Vec<CodeActionOrCommand>> {
     let document = snapshot.document();
-
-    let fixes = fixes_for_diagnostics(document, snapshot.encoding(), diagnostics)?;
-
     fixes
-        .into_iter()
+        .iter()
+        .filter(|fix| !fix.edits.is_empty())
         .map(|fix| {
             let mut tracker = WorkspaceEditTracker::new(snapshot.resolved_client_capabilities());
 
             tracker.set_edits_for_document(
                 snapshot.url().clone(),
                 document.version(),
-                fix.edits,
+                fix.edits.clone(),
             )?;
 
             Ok(types::CodeActionOrCommand::CodeAction(types::CodeAction {
@@ -87,6 +96,36 @@ fn quick_fix(
         .collect()
 }
 
+fn noqa_comments(snapshot: &DocumentSnapshot, fixes: &[DiagnosticFix]) -> Vec<CodeActionOrCommand> {
+    fixes
+        .iter()
+        .filter_map(|fix| {
+            let edit = fix.noqa_edit.clone()?;
+
+            let mut tracker = WorkspaceEditTracker::new(snapshot.resolved_client_capabilities());
+
+            tracker
+                .set_edits_for_document(
+                    snapshot.url().clone(),
+                    snapshot.document().version(),
+                    vec![edit],
+                )
+                .ok()?;
+
+            Some(types::CodeActionOrCommand::CodeAction(types::CodeAction {
+                title: format!("{DIAGNOSTIC_NAME} ({}): Disable for this line", fix.code),
+                kind: Some(types::CodeActionKind::QUICKFIX),
+                edit: Some(tracker.into_workspace_edit()),
+                diagnostics: Some(vec![fix.fixed_diagnostic.clone()]),
+                data: Some(
+                    serde_json::to_value(snapshot.url()).expect("document url to serialize"),
+                ),
+                ..Default::default()
+            }))
+        })
+        .collect()
+}
+
 fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCommand> {
     let document = snapshot.document();
 

crates/ruff_server/src/session/settings.rs

@@ -19,8 +19,6 @@ pub(crate) struct ResolvedClientSettings {
     fix_all: bool,
     organize_imports: bool,
     lint_enable: bool,
-    // TODO(jane): Remove once noqa auto-fix is implemented
-    #[allow(dead_code)]
     disable_rule_comment_enable: bool,
     fix_violation_enable: bool,
     editor_settings: ResolvedEditorSettings,
@@ -323,6 +321,10 @@ impl ResolvedClientSettings {
         self.lint_enable
     }
 
+    pub(crate) fn noqa_comments(&self) -> bool {
+        self.disable_rule_comment_enable
+    }
+
     pub(crate) fn fix_violation(&self) -> bool {
         self.fix_violation_enable
     }