mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-28 12:55:05 +00:00
ruff server
: Support noqa
comment code action (#11276)
## Summary
Fixes https://github.com/astral-sh/ruff/issues/10594.
Code actions to disable a diagnostic via `noqa` comment are now
available.
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
This commit is contained in:
parent
4b330b11c6
commit
d7f093ef9e
3 changed files with 126 additions and 38 deletions
|
@ -1,8 +1,9 @@
|
||||||
//! Access to the Ruff linting API for the LSP
|
//! 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::{
|
use ruff_linter::{
|
||||||
directives::{extract_directives, Flags},
|
directives::{extract_directives, Flags},
|
||||||
|
generate_noqa_edits,
|
||||||
linter::{check_path, LinterResult, TokenSource},
|
linter::{check_path, LinterResult, TokenSource},
|
||||||
packaging::detect_package_root,
|
packaging::detect_package_root,
|
||||||
registry::AsRule,
|
registry::AsRule,
|
||||||
|
@ -24,17 +25,29 @@ use crate::{edit::ToRangeExt, PositionEncoding, DIAGNOSTIC_NAME};
|
||||||
#[derive(Serialize, Deserialize, Debug, Clone)]
|
#[derive(Serialize, Deserialize, Debug, Clone)]
|
||||||
pub(crate) struct AssociatedDiagnosticData {
|
pub(crate) struct AssociatedDiagnosticData {
|
||||||
pub(crate) kind: DiagnosticKind,
|
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,
|
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)]
|
#[derive(Clone, Debug)]
|
||||||
pub(crate) struct DiagnosticFix {
|
pub(crate) struct DiagnosticFix {
|
||||||
|
/// The original diagnostic to be fixed
|
||||||
pub(crate) fixed_diagnostic: lsp_types::Diagnostic,
|
pub(crate) fixed_diagnostic: lsp_types::Diagnostic,
|
||||||
|
/// The message describing what the fix does.
|
||||||
pub(crate) title: String,
|
pub(crate) title: String,
|
||||||
|
/// The NOQA code for the diagnostic.
|
||||||
pub(crate) code: String,
|
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>,
|
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(
|
pub(crate) fn check(
|
||||||
|
@ -75,7 +88,7 @@ pub(crate) fn check(
|
||||||
let indexer = Indexer::from_tokens(&tokens, &locator);
|
let indexer = Indexer::from_tokens(&tokens, &locator);
|
||||||
|
|
||||||
// Extract the `# noqa` and `# isort: skip` directives from the source.
|
// 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.
|
// Generate checks.
|
||||||
let LinterResult {
|
let LinterResult {
|
||||||
|
@ -94,9 +107,20 @@ pub(crate) fn check(
|
||||||
TokenSource::Tokens(tokens),
|
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
|
diagnostics
|
||||||
.into_iter()
|
.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()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -118,14 +142,34 @@ pub(crate) fn fixes_for_diagnostics(
|
||||||
})?;
|
})?;
|
||||||
let edits = associated_data
|
let edits = associated_data
|
||||||
.fix
|
.fix
|
||||||
.edits()
|
.map(|fix| {
|
||||||
.iter()
|
fix.edits()
|
||||||
.map(|edit| lsp_types::TextEdit {
|
.iter()
|
||||||
range: edit
|
.map(|edit| lsp_types::TextEdit {
|
||||||
.range()
|
range: edit.range().to_range(
|
||||||
.to_range(document.contents(), document.index(), encoding),
|
document.contents(),
|
||||||
new_text: edit.content().unwrap_or_default().to_string(),
|
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 {
|
Ok(Some(DiagnosticFix {
|
||||||
fixed_diagnostic,
|
fixed_diagnostic,
|
||||||
code: associated_data.code,
|
code: associated_data.code,
|
||||||
|
@ -133,7 +177,8 @@ pub(crate) fn fixes_for_diagnostics(
|
||||||
.kind
|
.kind
|
||||||
.suggestion
|
.suggestion
|
||||||
.unwrap_or(associated_data.kind.name),
|
.unwrap_or(associated_data.kind.name),
|
||||||
edits: edits.collect(),
|
edits,
|
||||||
|
noqa_edit,
|
||||||
}))
|
}))
|
||||||
})
|
})
|
||||||
.filter_map(crate::Result::transpose)
|
.filter_map(crate::Result::transpose)
|
||||||
|
@ -142,6 +187,7 @@ pub(crate) fn fixes_for_diagnostics(
|
||||||
|
|
||||||
fn to_lsp_diagnostic(
|
fn to_lsp_diagnostic(
|
||||||
diagnostic: Diagnostic,
|
diagnostic: Diagnostic,
|
||||||
|
noqa_edit: Option<Edit>,
|
||||||
document: &crate::edit::Document,
|
document: &crate::edit::Document,
|
||||||
encoding: PositionEncoding,
|
encoding: PositionEncoding,
|
||||||
) -> lsp_types::Diagnostic {
|
) -> lsp_types::Diagnostic {
|
||||||
|
@ -151,18 +197,19 @@ fn to_lsp_diagnostic(
|
||||||
|
|
||||||
let rule = kind.rule();
|
let rule = kind.rule();
|
||||||
|
|
||||||
let data = fix.and_then(|fix| {
|
let fix = fix.and_then(|fix| fix.applies(Applicability::Unsafe).then_some(fix));
|
||||||
fix.applies(Applicability::Unsafe)
|
|
||||||
.then(|| {
|
let data = (fix.is_some() || noqa_edit.is_some())
|
||||||
serde_json::to_value(&AssociatedDiagnosticData {
|
.then(|| {
|
||||||
kind: kind.clone(),
|
serde_json::to_value(&AssociatedDiagnosticData {
|
||||||
fix,
|
kind: kind.clone(),
|
||||||
code: rule.noqa_code().to_string(),
|
fix,
|
||||||
})
|
code: rule.noqa_code().to_string(),
|
||||||
.ok()
|
noqa_edit,
|
||||||
})
|
})
|
||||||
.flatten()
|
.ok()
|
||||||
});
|
})
|
||||||
|
.flatten();
|
||||||
|
|
||||||
let code = rule.noqa_code().to_string();
|
let code = rule.noqa_code().to_string();
|
||||||
|
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
use crate::edit::WorkspaceEditTracker;
|
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::api::LSPResult;
|
||||||
use crate::server::SupportedCodeAction;
|
use crate::server::SupportedCodeAction;
|
||||||
use crate::server::{client::Notifier, Result};
|
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 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()
|
if snapshot.client_settings().fix_violation()
|
||||||
&& supported_code_actions.contains(&SupportedCodeAction::QuickFix)
|
&& supported_code_actions.contains(&SupportedCodeAction::QuickFix)
|
||||||
{
|
{
|
||||||
response.extend(
|
response
|
||||||
quick_fix(&snapshot, params.context.diagnostics.clone())
|
.extend(quick_fix(&snapshot, &fixes).with_failure_code(ErrorCode::InternalError)?);
|
||||||
.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()
|
if snapshot.client_settings().fix_all()
|
||||||
|
@ -56,21 +67,19 @@ impl super::BackgroundDocumentRequestHandler for CodeActions {
|
||||||
|
|
||||||
fn quick_fix(
|
fn quick_fix(
|
||||||
snapshot: &DocumentSnapshot,
|
snapshot: &DocumentSnapshot,
|
||||||
diagnostics: Vec<types::Diagnostic>,
|
fixes: &[DiagnosticFix],
|
||||||
) -> crate::Result<Vec<CodeActionOrCommand>> {
|
) -> crate::Result<Vec<CodeActionOrCommand>> {
|
||||||
let document = snapshot.document();
|
let document = snapshot.document();
|
||||||
|
|
||||||
let fixes = fixes_for_diagnostics(document, snapshot.encoding(), diagnostics)?;
|
|
||||||
|
|
||||||
fixes
|
fixes
|
||||||
.into_iter()
|
.iter()
|
||||||
|
.filter(|fix| !fix.edits.is_empty())
|
||||||
.map(|fix| {
|
.map(|fix| {
|
||||||
let mut tracker = WorkspaceEditTracker::new(snapshot.resolved_client_capabilities());
|
let mut tracker = WorkspaceEditTracker::new(snapshot.resolved_client_capabilities());
|
||||||
|
|
||||||
tracker.set_edits_for_document(
|
tracker.set_edits_for_document(
|
||||||
snapshot.url().clone(),
|
snapshot.url().clone(),
|
||||||
document.version(),
|
document.version(),
|
||||||
fix.edits,
|
fix.edits.clone(),
|
||||||
)?;
|
)?;
|
||||||
|
|
||||||
Ok(types::CodeActionOrCommand::CodeAction(types::CodeAction {
|
Ok(types::CodeActionOrCommand::CodeAction(types::CodeAction {
|
||||||
|
@ -87,6 +96,36 @@ fn quick_fix(
|
||||||
.collect()
|
.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> {
|
fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCommand> {
|
||||||
let document = snapshot.document();
|
let document = snapshot.document();
|
||||||
|
|
||||||
|
|
|
@ -19,8 +19,6 @@ pub(crate) struct ResolvedClientSettings {
|
||||||
fix_all: bool,
|
fix_all: bool,
|
||||||
organize_imports: bool,
|
organize_imports: bool,
|
||||||
lint_enable: bool,
|
lint_enable: bool,
|
||||||
// TODO(jane): Remove once noqa auto-fix is implemented
|
|
||||||
#[allow(dead_code)]
|
|
||||||
disable_rule_comment_enable: bool,
|
disable_rule_comment_enable: bool,
|
||||||
fix_violation_enable: bool,
|
fix_violation_enable: bool,
|
||||||
editor_settings: ResolvedEditorSettings,
|
editor_settings: ResolvedEditorSettings,
|
||||||
|
@ -323,6 +321,10 @@ impl ResolvedClientSettings {
|
||||||
self.lint_enable
|
self.lint_enable
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(crate) fn noqa_comments(&self) -> bool {
|
||||||
|
self.disable_rule_comment_enable
|
||||||
|
}
|
||||||
|
|
||||||
pub(crate) fn fix_violation(&self) -> bool {
|
pub(crate) fn fix_violation(&self) -> bool {
|
||||||
self.fix_violation_enable
|
self.fix_violation_enable
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue