Skip to content

Commit

Permalink
Introduce noqa comment code action
Browse files Browse the repository at this point in the history
  • Loading branch information
snowsignal committed May 10, 2024
1 parent 452e27b commit d48d094
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 40 deletions.
92 changes: 65 additions & 27 deletions crates/ruff_server/src/lint.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -16,6 +17,7 @@ use ruff_python_parser::lexer::LexResult;
use ruff_python_parser::AsMode;
use ruff_source_file::Locator;
use ruff_text_size::Ranged;
use rustc_hash::FxHashMap;
use serde::{Deserialize, Serialize};

use crate::{edit::ToRangeExt, PositionEncoding, DIAGNOSTIC_NAME};
Expand All @@ -24,17 +26,20 @@ use crate::{edit::ToRangeExt, PositionEncoding, DIAGNOSTIC_NAME};
#[derive(Serialize, Deserialize, Debug, Clone)]
pub(crate) struct AssociatedDiagnosticData {
pub(crate) kind: DiagnosticKind,
pub(crate) fix: Fix,
pub(crate) fix: Option<Fix>,
pub(crate) code: String,
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 {
pub(crate) fixed_diagnostic: lsp_types::Diagnostic,
pub(crate) title: String,
pub(crate) code: String,
pub(crate) edits: Vec<lsp_types::TextEdit>,
pub(crate) edits: Option<Vec<lsp_types::TextEdit>>,
pub(crate) noqa_edit: Option<lsp_types::TextEdit>,
}

pub(crate) fn check(
Expand Down Expand Up @@ -94,9 +99,22 @@ pub(crate) fn check(
TokenSource::Tokens(tokens),
);

let mut noqa_edits: FxHashMap<Diagnostic, Edit> = 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))
.map(|diagnostic| {
let noqa_edit = noqa_edits.remove(&diagnostic);
to_lsp_diagnostic(diagnostic, noqa_edit, document, encoding)
})
.collect()
}

Expand All @@ -116,24 +134,42 @@ pub(crate) fn fixes_for_diagnostics(
serde_json::from_value(data).map_err(|err| {
anyhow::anyhow!("failed to deserialize diagnostic data: {err}")
})?;
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(),
});
let edits = associated_data.fix.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()
});

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)
Expand All @@ -142,6 +178,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 {
Expand All @@ -151,18 +188,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();

Expand Down
64 changes: 53 additions & 11 deletions crates/ruff_server/src/server/api/requests/code_action.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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()
Expand All @@ -56,21 +67,22 @@ 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_some())
.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
.as_ref()
.expect("should only be iterating over fixes with available edits")
.clone(),
)?;

Ok(types::CodeActionOrCommand::CodeAction(types::CodeAction {
Expand All @@ -87,6 +99,36 @@ fn quick_fix(
.collect()
}

fn noqa_comments(snapshot: &DocumentSnapshot, fixes: &[DiagnosticFix]) -> Vec<CodeActionOrCommand> {
fixes
.iter()
.filter_map(|fix| {
let edit = fix.noqa_edit.as_ref()?.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();

Expand Down
6 changes: 4 additions & 2 deletions crates/ruff_server/src/session/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit d48d094

Please sign in to comment.