diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index 8a2857a19664f0..15159230c571a7 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -455,6 +455,7 @@ pub struct CheckCommand { num_args = 0..=1, require_equals = true, // conflicts_with = "add_noqa", + conflicts_with = "add_ignore", conflicts_with = "show_files", conflicts_with = "show_settings", // Unsupported default-command arguments. @@ -466,6 +467,28 @@ pub struct CheckCommand { conflicts_with = "diff", )] pub add_noqa: Option, + /// Enable automatic additions of `ruff:ignore` comments to failing lines. + /// Optionally provide a reason to append after the rule names. + /// Requires preview mode. + #[arg( + long, + value_name = "REASON", + default_missing_value = "", + num_args = 0..=1, + require_equals = true, + // conflicts_with = "add_ignore", + conflicts_with = "add_noqa", + conflicts_with = "show_files", + conflicts_with = "show_settings", + // Unsupported default-command arguments. + conflicts_with = "ignore_noqa", + conflicts_with = "statistics", + conflicts_with = "stdin_filename", + conflicts_with = "watch", + conflicts_with = "fix", + conflicts_with = "diff", + )] + pub add_ignore: Option, /// See the files Ruff will be run against with the current settings. #[arg( long, @@ -774,6 +797,7 @@ impl CheckCommand { ) -> anyhow::Result<(CheckArguments, ConfigArguments)> { let check_arguments = CheckArguments { add_noqa: self.add_noqa, + add_ignore: self.add_ignore, diff: self.diff, exit_non_zero_on_fix: self.exit_non_zero_on_fix, exit_zero: self.exit_zero, @@ -1112,6 +1136,7 @@ Possible choices: #[expect(clippy::struct_excessive_bools)] pub struct CheckArguments { pub add_noqa: Option, + pub add_ignore: Option, pub diff: bool, pub exit_non_zero_on_fix: bool, pub exit_zero: bool, diff --git a/crates/ruff/src/commands/add_noqa.rs b/crates/ruff/src/commands/add_noqa.rs index f46deeecf1e555..159a7fa0bbad63 100644 --- a/crates/ruff/src/commands/add_noqa.rs +++ b/crates/ruff/src/commands/add_noqa.rs @@ -1,12 +1,14 @@ use std::path::PathBuf; use std::time::Instant; -use anyhow::Result; +use anyhow::{Result, bail}; use log::{debug, error}; #[cfg(not(target_family = "wasm"))] use rayon::prelude::*; -use ruff_linter::linter::add_noqa_to_path; +use ruff_linter::SuppressionKind; +use ruff_linter::linter::add_suppressions_to_path; +use ruff_linter::preview::is_human_readable_names_enabled; use ruff_linter::source_kind::SourceKind; use ruff_linter::warn_user_once; use ruff_python_ast::{PySourceType, SourceType}; @@ -16,12 +18,13 @@ use ruff_workspace::resolver::{ use crate::args::ConfigArguments; -/// Add `noqa` directives to a collection of files. +/// Add suppression directives to a collection of files. pub(crate) fn add_noqa( files: &[PathBuf], pyproject_config: &PyprojectConfig, config_arguments: &ConfigArguments, reason: Option<&str>, + suppression_kind: SuppressionKind, ) -> Result { // Collect all the files to check. let start = Instant::now(); @@ -59,7 +62,7 @@ pub(crate) fn add_noqa( let modifications: usize = paths .par_iter() .flatten() - .filter_map(|resolved_file| { + .map(|resolved_file| -> Result { let source_type = SourceType::from(resolved_file.path()); let path = resolved_file.path(); let package = resolved_file @@ -75,35 +78,44 @@ pub(crate) fn add_noqa( &settings.linter.exclude, ) { - return None; + return Ok(0); + } + if matches!(suppression_kind, SuppressionKind::Ignore) + && !is_human_readable_names_enabled(settings.linter.preview) + { + bail!( + "`--add-ignore` requires preview mode, but preview is disabled for `{}`", + path.display() + ); } let source_kind = match SourceKind::from_path(path, source_type) { Ok(Some(source_kind)) => source_kind, - Ok(None) => return None, + Ok(None) => return Ok(0), Err(e) => { error!("Failed to extract source from {}: {e}", path.display()); - return None; + return Ok(0); } }; - match add_noqa_to_path( + match add_suppressions_to_path( path, package, &source_kind, source_type.expect_python(), &settings.linter, reason, + suppression_kind, ) { - Ok(count) => Some(count), + Ok(count) => Ok(count), Err(e) => { - error!("Failed to add noqa to {}: {e}", path.display()); - None + error!("Failed to add suppression to {}: {e}", path.display()); + Ok(0) } } }) - .sum(); + .sum::>()?; let duration = start.elapsed(); - debug!("Added noqa to files in: {duration:?}"); + debug!("Added suppressions to files in: {duration:?}"); Ok(modifications) } diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs index 9d25a0967494a4..34bccc3b8a1de2 100644 --- a/crates/ruff/src/lib.rs +++ b/crates/ruff/src/lib.rs @@ -17,7 +17,7 @@ use args::{GlobalConfigArgs, ServerCommand}; use ruff_db::diagnostic::{Diagnostic, Severity}; use ruff_linter::logging::{LogLevel, set_up_logging}; use ruff_linter::settings::flags::FixMode; -use ruff_linter::{fs, warn_user, warn_user_once}; +use ruff_linter::{SuppressionKind, fs, warn_user, warn_user_once}; use ruff_workspace::Settings; use crate::args::{ @@ -326,25 +326,44 @@ pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result cannot contain newline characters" + "{flag} cannot contain newline characters" )); } let reason_opt = (!reason.is_empty()).then_some(reason.as_str()); - let modifications = - commands::add_noqa::add_noqa(&files, &pyproject_config, &config_arguments, reason_opt)?; + let modifications = commands::add_noqa::add_noqa( + &files, + &pyproject_config, + &config_arguments, + reason_opt, + suppression_kind, + )?; if modifications > 0 && config_arguments.log_level >= LogLevel::Default { let s = if modifications == 1 { "" } else { "s" }; + let suppression = match suppression_kind { + SuppressionKind::Noqa => "noqa directive", + SuppressionKind::Ignore => "ignore comment", + }; #[expect(clippy::print_stderr)] { - eprintln!("Added {modifications} noqa directive{s}."); + eprintln!("Added {modifications} {suppression}{s}."); } } return Ok(ExitStatus::Success); diff --git a/crates/ruff/tests/cli/lint.rs b/crates/ruff/tests/cli/lint.rs index 3a619ced507d3e..65361b4f3568a3 100644 --- a/crates/ruff/tests/cli/lint.rs +++ b/crates/ruff/tests/cli/lint.rs @@ -2596,6 +2596,115 @@ fn add_noqa_with_newline_in_reason() -> Result<()> { Ok(()) } +#[test] +fn add_ignore() -> Result<()> { + let fixture = CliTest::new()?; + fixture.write_file( + "noqa.py", + r#" + def first_square(): + return [x * x for x in range(20)][0] + "#, + )?; + + assert_cmd_snapshot!( + fixture + .check_command() + .arg("--select=RUF015") + .arg("--preview") + .arg("--add-ignore"), + @" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Added 1 ignore comment. + ", + ); + + let test_code = fixture.read_file("noqa.py")?; + + insta::assert_snapshot!( + test_code, + @" + + def first_square(): + return [x * x for x in range(20)][0] # ruff:ignore[unnecessary-iterable-allocation-for-first-element] + ", + ); + + Ok(()) +} + +#[test] +fn add_ignore_requires_preview() -> Result<()> { + let fixture = CliTest::new()?; + fixture.write_file("noqa.py", "import os\n")?; + + assert_cmd_snapshot!( + fixture + .check_command() + .arg("--select=F401") + .arg("--add-ignore"), + @" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + ruff failed + Cause: `--add-ignore` requires preview mode, but preview is disabled for `[TMP]/noqa.py` + ", + ); + + let test_code = fixture.read_file("noqa.py")?; + insta::assert_snapshot!(test_code, @"import os"); + + Ok(()) +} + +#[test] +fn add_noqa_existing_ignore() -> Result<()> { + let fixture = CliTest::new()?; + fixture.write_file( + "noqa.py", + r#" + def unused(x): # ruff:ignore[ANN001, ARG001, D103] + pass + "#, + )?; + + assert_cmd_snapshot!( + fixture + .check_command() + .arg("--select=ANN001,ANN201,ARG001,D103") + .arg("--add-noqa"), + @" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: #ruff:ignore comment found but not active, enable preview mode + Added 1 noqa directive. + ", + ); + + let test_code = fixture.read_file("noqa.py")?; + + insta::assert_snapshot!( + test_code, + @" + + def unused(x): # ruff:ignore[ANN001, ARG001, D103] # noqa: ANN001, ANN201, D103 + pass + ", + ); + + Ok(()) +} + /// Infer `3.11` from `requires-python` in `pyproject.toml`. #[test] fn requires_python() -> Result<()> { diff --git a/crates/ruff/tests/cli/main.rs b/crates/ruff/tests/cli/main.rs index bd2536bd8a1b24..a09d39ff5fd153 100644 --- a/crates/ruff/tests/cli/main.rs +++ b/crates/ruff/tests/cli/main.rs @@ -158,6 +158,13 @@ impl CliTest { Ok(()) } + /// Reads a file from the test directory. + pub(crate) fn read_file(&self, path: impl AsRef) -> Result { + let file_path = self.project_dir.join(path); + fs::read_to_string(&file_path) + .with_context(|| format!("Failed to read file `{}`", file_path.display())) + } + pub(crate) fn write_files<'a>( &self, files: impl IntoIterator, diff --git a/crates/ruff_linter/src/lib.rs b/crates/ruff_linter/src/lib.rs index bd6cd082254ae7..4a59b41a926051 100644 --- a/crates/ruff_linter/src/lib.rs +++ b/crates/ruff_linter/src/lib.rs @@ -6,7 +6,7 @@ //! [Ruff]: https://github.com/astral-sh/ruff pub use locator::Locator; -pub use noqa::generate_noqa_edits; +pub use noqa::{SuppressionKind, generate_suppression_edits}; #[cfg(feature = "clap")] pub use registry::clap_completion::RuleParser; #[cfg(feature = "clap")] diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 4157bd51135278..b8039417a96483 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -23,7 +23,7 @@ use crate::checkers::tokens::check_tokens; use crate::directives::Directives; use crate::doc_lines::{doc_lines_from_ast, doc_lines_from_tokens}; use crate::fix::{FixResult, fix_file}; -use crate::noqa::add_noqa; +use crate::noqa::add_suppression; use crate::package::PackageRoot; use crate::preview::is_py315_support_enabled; use crate::registry::Rule; @@ -33,7 +33,7 @@ use crate::settings::types::UnsafeFixes; use crate::settings::{LinterSettings, TargetVersion, flags}; use crate::source_kind::SourceKind; use crate::suppression::Suppressions; -use crate::{Locator, directives, fs, warn_user_once}; +use crate::{Locator, SuppressionKind, directives, fs, warn_user_once}; pub(crate) mod float; @@ -371,14 +371,15 @@ pub fn check_path( const MAX_ITERATIONS: usize = 100; -/// Add any missing `# noqa` pragmas to the source code at the given `Path`. -pub fn add_noqa_to_path( +/// Add any missing suppression comments to the source code at the given `Path`. +pub fn add_suppressions_to_path( path: &Path, package: Option>, source_kind: &SourceKind, source_type: PySourceType, settings: &LinterSettings, reason: Option<&str>, + suppression_kind: SuppressionKind, ) -> Result { // Parse once. let target_version = settings.resolve_target_version(path); @@ -422,9 +423,9 @@ pub fn add_noqa_to_path( &suppressions, ); - // Add any missing `# noqa` pragmas. + // Add any missing suppression comments. // TODO(dhruvmanila): Add support for Jupyter Notebooks - add_noqa( + add_suppression( path, &diagnostics, &locator, @@ -434,6 +435,7 @@ pub fn add_noqa_to_path( stylist.line_ending(), reason, &suppressions, + suppression_kind, ) } diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index f4f434d530b60f..eb645456d833e1 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -21,15 +21,15 @@ use crate::Locator; use crate::fs::relativize_path; use crate::registry::Rule; use crate::rule_redirects::get_redirect_target; -use crate::suppression::Suppressions; +use crate::suppression::{self, Suppressions}; -/// Generates an array of edits that matches the length of `messages`. +/// Generates an array of edits that matches the length of `diagnostics`. /// Each potential edit in the array is paired, in order, with the associated diagnostic. -/// Each edit will add a `noqa` comment to the appropriate line in the source to hide +/// Each edit will add a suppression comment to the appropriate line in the source to hide /// the diagnostic. These edits may conflict with each other and should not be applied /// simultaneously. #[expect(clippy::too_many_arguments)] -pub fn generate_noqa_edits( +pub fn generate_suppression_edits( path: &Path, diagnostics: &[Diagnostic], locator: &Locator, @@ -38,19 +38,21 @@ pub fn generate_noqa_edits( noqa_line_for: &NoqaMapping, line_ending: LineEnding, suppressions: &Suppressions, + suppression_kind: SuppressionKind, ) -> Vec> { let file_directives = FileNoqaDirectives::extract(locator, comment_ranges, external, path); let exemption = FileExemption::from(&file_directives); let directives = NoqaDirectives::from_commented_ranges(comment_ranges, path, locator); - let comments = find_noqa_comments( + let comments = find_suppression_comments( diagnostics, locator, &exemption, &directives, noqa_line_for, suppressions, + suppression_kind, ); - build_noqa_edits_by_diagnostic(comments, locator, line_ending, None) + build_suppression_edits_by_diagnostic(comments, locator, line_ending, None, suppression_kind) } /// A directive to ignore a set of rules either for a given line of Python source code or an entire file (e.g., @@ -743,9 +745,18 @@ impl Display for LexicalError { impl Error for LexicalError {} -/// Adds noqa comments to suppress all messages of a file. +/// The kind of suppression comment to be added to suppress a diagnostic. +#[derive(Copy, Clone, Debug)] +pub enum SuppressionKind { + /// A `noqa` comment + Noqa, + /// A `ruff:ignore` comment + Ignore, +} + +/// Adds suppression comments to suppress all messages of a file. #[expect(clippy::too_many_arguments)] -pub(crate) fn add_noqa( +pub(crate) fn add_suppression( path: &Path, diagnostics: &[Diagnostic], locator: &Locator, @@ -755,8 +766,9 @@ pub(crate) fn add_noqa( line_ending: LineEnding, reason: Option<&str>, suppressions: &Suppressions, + suppression_kind: SuppressionKind, ) -> Result { - let (count, output) = add_noqa_inner( + let (count, output) = add_suppression_inner( path, diagnostics, locator, @@ -766,6 +778,7 @@ pub(crate) fn add_noqa( line_ending, reason, suppressions, + suppression_kind, ); fs::write(path, output)?; @@ -773,7 +786,7 @@ pub(crate) fn add_noqa( } #[expect(clippy::too_many_arguments)] -fn add_noqa_inner( +fn add_suppression_inner( path: &Path, diagnostics: &[Diagnostic], locator: &Locator, @@ -783,6 +796,7 @@ fn add_noqa_inner( line_ending: LineEnding, reason: Option<&str>, suppressions: &Suppressions, + suppression_kind: SuppressionKind, ) -> (usize, String) { let mut count = 0; @@ -792,16 +806,18 @@ fn add_noqa_inner( let directives = NoqaDirectives::from_commented_ranges(comment_ranges, path, locator); - let comments = find_noqa_comments( + let comments = find_suppression_comments( diagnostics, locator, &exemption, &directives, noqa_line_for, suppressions, + suppression_kind, ); - let edits = build_noqa_edits_by_line(comments, locator, line_ending, reason); + let edits = + build_suppression_edits_by_line(comments, locator, line_ending, reason, suppression_kind); let contents = locator.contents(); @@ -823,26 +839,27 @@ fn add_noqa_inner( (count, output) } -fn build_noqa_edits_by_diagnostic( - comments: Vec>, +fn build_suppression_edits_by_diagnostic( + comments: Vec>, locator: &Locator, line_ending: LineEnding, reason: Option<&str>, + suppression_kind: SuppressionKind, ) -> Vec> { let mut edits = Vec::default(); for comment in comments { match comment { Some(comment) => { - if let Some(noqa_edit) = generate_noqa_edit( + let suppression_edit = generate_suppression_edit( comment.directive, comment.line, - FxHashSet::from_iter([comment.code]), + FxHashSet::from_iter([comment.identifier]), locator, line_ending, reason, - ) { - edits.push(Some(noqa_edit.into_edit())); - } + suppression_kind, + ); + edits.push(Some(suppression_edit.into_edit())); } None => edits.push(None), } @@ -850,12 +867,13 @@ fn build_noqa_edits_by_diagnostic( edits } -fn build_noqa_edits_by_line<'a>( - comments: Vec>>, - locator: &Locator, +fn build_suppression_edits_by_line<'a>( + comments: Vec>>, + locator: &Locator<'a>, line_ending: LineEnding, reason: Option<&'a str>, -) -> BTreeMap> { + suppression_kind: SuppressionKind, +) -> BTreeMap> { let mut comments_by_line = BTreeMap::default(); for comment in comments.into_iter().flatten() { comments_by_line @@ -869,39 +887,55 @@ fn build_noqa_edits_by_line<'a>( continue; }; let directive = first_match.directive; - if let Some(edit) = generate_noqa_edit( + let suppression_edit = generate_suppression_edit( directive, offset, matches .into_iter() - .map(|NoqaComment { code, .. }| code) + .map(|comment| comment.identifier) .collect(), locator, line_ending, reason, - ) { - edits.insert(offset, edit); - } + suppression_kind, + ); + edits.insert(offset, suppression_edit); } edits } -struct NoqaComment<'a> { +struct SuppressionComment<'a> { line: TextSize, - code: &'a SecondaryCode, - directive: Option<&'a Directive<'a>>, + identifier: &'a str, + directive: Option>, +} + +#[derive(Copy, Clone)] +enum ExistingDirective<'a> { + Noqa(&'a Codes<'a>), + Ignore(&'a suppression::SuppressionComment), +} + +impl Ranged for ExistingDirective<'_> { + fn range(&self) -> TextRange { + match self { + ExistingDirective::Noqa(codes) => codes.range(), + ExistingDirective::Ignore(comment) => comment.range(), + } + } } -fn find_noqa_comments<'a>( +fn find_suppression_comments<'a>( diagnostics: &'a [Diagnostic], locator: &'a Locator, exemption: &'a FileExemption, directives: &'a NoqaDirectives, noqa_line_for: &NoqaMapping, suppressions: &'a Suppressions, -) -> Vec>> { - // List of noqa comments, ordered to match up with `messages` - let mut comments_by_line: Vec>> = vec![]; + suppression_kind: SuppressionKind, +) -> Vec>> { + // List of suppression comments, ordered to match up with `messages` + let mut comments_by_line: Vec>> = vec![]; // Mark any non-ignored diagnostics. for message in diagnostics { @@ -946,47 +980,60 @@ fn find_noqa_comments<'a>( .map(|range| noqa_line_for.resolve(range.start())) .unwrap_or_default(); - // Or ignored by the directive itself? - if let Some(directive_line) = directives.find_line_with_directive(noqa_offset) { - match &directive_line.directive { - Directive::All(_) => { - comments_by_line.push(None); - continue; - } - directive @ Directive::Codes(codes) => { - if !codes.includes(code) { - comments_by_line.push(Some(NoqaComment { - line: directive_line.start(), - code, - directive: Some(directive), - })); + // Or ignored by a `noqa` directive on the diagnostic's line itself? + let existing_noqa = + if let Some(directive_line) = directives.find_line_with_directive(noqa_offset) { + match &directive_line.directive { + Directive::All(_) => { + comments_by_line.push(None); + continue; + } + Directive::Codes(codes) => { + if codes.includes(code) { + comments_by_line.push(None); + continue; + } + Some(ExistingDirective::Noqa(codes)) } - continue; } - } - } + } else { + None + }; + + // Reuse an existing directive that matches the requested suppression style. + let directive = match suppression_kind { + SuppressionKind::Noqa => existing_noqa, + SuppressionKind::Ignore => suppressions + .find_applicable_ignore(message) + .map(ExistingDirective::Ignore), + }; - // There's no existing noqa directive that suppresses the diagnostic. - comments_by_line.push(Some(NoqaComment { - line: locator.line_start(noqa_offset), - code, - directive: None, + let identifier = match suppression_kind { + SuppressionKind::Noqa => code.as_str(), + SuppressionKind::Ignore => message.name(), + }; + + comments_by_line.push(Some(SuppressionComment { + line: locator.line_start(directive.map_or(noqa_offset, |directive| directive.start())), + identifier, + directive, })); } comments_by_line } -struct NoqaEdit<'a> { +struct SuppressionEdit<'a> { edit_range: TextRange, - noqa_codes: FxHashSet<&'a SecondaryCode>, - codes: Option<&'a Codes<'a>>, + noqa_codes: FxHashSet<&'a str>, + existing_codes: Vec<&'a str>, line_ending: LineEnding, reason: Option<&'a str>, blank_line: bool, + suppression_kind: SuppressionKind, } -impl NoqaEdit<'_> { +impl SuppressionEdit<'_> { fn into_edit(self) -> Edit { let mut edit_content = String::new(); self.write(&mut edit_content); @@ -998,21 +1045,20 @@ impl NoqaEdit<'_> { if !self.blank_line { write!(writer, " ").unwrap(); } - write!(writer, "# noqa: ").unwrap(); - match self.codes { - Some(codes) => { - push_codes( - writer, - self.noqa_codes - .iter() - .map(|code| code.as_str()) - .chain(codes.iter().map(Code::as_str)) - .sorted_unstable(), - ); - } - None => { - push_codes(writer, self.noqa_codes.iter().sorted_unstable()); - } + match self.suppression_kind { + SuppressionKind::Noqa => write!(writer, "# noqa: ").unwrap(), + SuppressionKind::Ignore => write!(writer, "# ruff:ignore[").unwrap(), + } + push_codes( + writer, + self.noqa_codes + .iter() + .chain(self.existing_codes.iter()) + .sorted_unstable(), + ); + match self.suppression_kind { + SuppressionKind::Noqa => {} + SuppressionKind::Ignore => write!(writer, "]").unwrap(), } if let Some(reason) = self.reason { write!(writer, " {reason}").unwrap(); @@ -1021,54 +1067,72 @@ impl NoqaEdit<'_> { } } -impl Ranged for NoqaEdit<'_> { +impl Ranged for SuppressionEdit<'_> { fn range(&self) -> TextRange { self.edit_range } } -fn generate_noqa_edit<'a>( - directive: Option<&'a Directive>, +fn generate_suppression_edit<'a>( + directive: Option>, offset: TextSize, - noqa_codes: FxHashSet<&'a SecondaryCode>, - locator: &Locator, + noqa_codes: FxHashSet<&'a str>, + locator: &Locator<'a>, line_ending: LineEnding, reason: Option<&'a str>, -) -> Option> { + suppression_kind: SuppressionKind, +) -> SuppressionEdit<'a> { let line_range = locator.full_line_range(offset); let edit_range; - let codes; + let mut existing_codes = Vec::new(); let blank_line; - // Add codes. - match directive { - None => { + match (directive, suppression_kind) { + // Add additional rule codes to an existing `noqa` comment. + (Some(ExistingDirective::Noqa(codes)), SuppressionKind::Noqa) => { + (edit_range, blank_line) = suppression_edit_range(locator, line_range, codes.start()); + existing_codes.extend(codes.iter().map(Code::as_str)); + } + // Add additional rule names to an existing `ruff:ignore` comment. + (Some(ExistingDirective::Ignore(comment)), SuppressionKind::Ignore) => { + (edit_range, blank_line) = suppression_edit_range(locator, line_range, comment.start()); + existing_codes.extend(comment.codes_as_str(locator.contents())); + } + // Add a new comment when one doesn't exist, or the "wrong" kind is present. In either case, + // append a new comment. + (None | Some(_), _) => { let trimmed_line = locator.slice(line_range).trim_end(); blank_line = trimmed_line.trim_whitespace_start().is_empty(); edit_range = TextRange::new(TextSize::of(trimmed_line), line_range.len()) + offset; - codes = None; - } - Some(Directive::Codes(existing_codes)) => { - // find trimmed line without the noqa - let trimmed_line = locator - .slice(TextRange::new(line_range.start(), existing_codes.start())) - .trim_end(); - blank_line = false; - edit_range = TextRange::new(TextSize::of(trimmed_line), line_range.len()) + offset; - codes = Some(existing_codes); } - Some(Directive::All(_)) => return None, } - Some(NoqaEdit { + SuppressionEdit { edit_range, noqa_codes, - codes, + existing_codes, line_ending, reason, blank_line, - }) + suppression_kind, + } +} + +/// Returns the range to replace when updating an existing suppression directive, along with whether +/// the directive is on an otherwise blank line. +fn suppression_edit_range( + locator: &Locator, + line_range: TextRange, + directive_start: TextSize, +) -> (TextRange, bool) { + let prefix = locator.slice(TextRange::new(line_range.start(), directive_start)); + if prefix.trim_whitespace().is_empty() { + (TextRange::new(directive_start, line_range.end()), true) + } else { + let edit_start = line_range.start() + prefix.trim_end().text_len(); + (TextRange::new(edit_start, line_range.end()), false) + } } fn push_codes(writer: &mut dyn std::fmt::Write, codes: impl Iterator) { @@ -1268,24 +1332,141 @@ impl FromIterator for NoqaMapping { #[cfg(test)] mod tests { + use std::fmt::Write; use std::path::Path; - use insta::assert_debug_snapshot; + use anyhow::{Result, anyhow}; + use insta::{assert_debug_snapshot, assert_snapshot}; - use ruff_python_trivia::CommentRanges; + use ruff_diagnostics::SourceMap; + use ruff_python_ast::{PySourceType, SourceType}; + use ruff_python_codegen::Stylist; + use ruff_python_index::Indexer; + use ruff_python_trivia::{CommentRanges, textwrap::dedent}; use ruff_source_file::{LineEnding, SourceFileBuilder}; use ruff_text_size::{TextLen, TextRange, TextSize}; + use crate::codes::Rule; + use crate::linter::{check_path, parse_unchecked_source}; use crate::noqa::{ - Directive, LexicalError, NoqaLexerOutput, NoqaMapping, add_noqa_inner, lex_codes, - lex_file_exemption, lex_inline_noqa, + Directive, LexicalError, NoqaLexerOutput, NoqaMapping, SuppressionKind, + add_suppression_inner, lex_codes, lex_file_exemption, lex_inline_noqa, }; use crate::rules::pycodestyle::rules::{AmbiguousVariableName, UselessSemicolon}; use crate::rules::pyflakes::rules::UnusedVariable; use crate::rules::pyupgrade::rules::PrintfStringFormatting; + use crate::settings::{LinterSettings, flags, types::PreviewMode}; + use crate::source_kind::SourceKind; use crate::suppression::Suppressions; - use crate::{Edit, Violation}; - use crate::{Locator, generate_noqa_edits}; + use crate::test::{print_messages, test_contents}; + use crate::{Edit, Violation, directives}; + use crate::{Locator, generate_suppression_edits}; + + fn add_suppressions( + path: &Path, + source_kind: &SourceKind, + settings: &LinterSettings, + suppression_kind: SuppressionKind, + ) -> (usize, String) { + let source_type = source_kind.py_source_type(); + let target_version = settings.resolve_target_version(path); + let parsed = + parse_unchecked_source(source_kind, source_type, target_version.parser_version()); + let locator = Locator::new(source_kind.source_code()); + let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents()); + let indexer = Indexer::from_tokens(parsed.tokens(), locator.contents()); + let directives = directives::extract_directives( + parsed.tokens(), + directives::Flags::from_settings(settings), + &locator, + &indexer, + ); + let suppressions = + Suppressions::from_tokens(locator.contents(), parsed.tokens(), &indexer, settings); + let diagnostics = check_path( + path, + None, + &locator, + &stylist, + &indexer, + &directives, + settings, + flags::Noqa::Disabled, + source_kind, + source_type, + &parsed, + target_version, + &suppressions, + ); + + add_suppression_inner( + path, + &diagnostics, + &locator, + indexer.comment_ranges(), + &settings.external, + &directives.noqa_line_for, + stylist.line_ending(), + None, + &suppressions, + suppression_kind, + ) + } + + #[track_caller] + fn add_suppressions_in( + source: &str, + suppression_kind: SuppressionKind, + preview: PreviewMode, + ) -> Result { + let settings = LinterSettings { + preview, + ..LinterSettings::for_rules([ + Rule::MissingTypeFunctionArgument, + Rule::MissingReturnTypeUndocumentedPublicFunction, + Rule::UnsortedImports, + Rule::UnusedFunctionArgument, + Rule::UndocumentedPublicFunction, + ]) + }; + let path = Path::new(""); + let source_map = SourceMap::default(); + let source_kind = SourceKind::from_source_code( + dedent(source).trim().to_string(), + SourceType::Python(PySourceType::Python), + )? + .ok_or_else(|| anyhow!("test file should be Python"))?; + + let (count, fixed) = add_suppressions(path, &source_kind, &settings, suppression_kind); + let plural = if count == 1 { "" } else { "s" }; + let mut output = String::new(); + writeln!( + output, + "Added {count} suppression{plural}\n\n## Fixed source\n\n```py\n{fixed}\n```\n" + )?; + + let source_kind = source_kind.updated(fixed, &source_map); + let (second_count, fixed) = + add_suppressions(path, &source_kind, &settings, suppression_kind); + if second_count > 0 { + writeln!( + output, + "## Additional suppressions added on a second pass: {second_count}\n\n```py\n{fixed}\n```\n" + )?; + } + + let source_kind = source_kind.updated(fixed, &source_map); + let (diagnostics, _) = test_contents(&source_kind, path, &settings); + if !diagnostics.is_empty() { + writeln!( + output, + "## New diagnostics after re-checking file\n\n{diagnostics}\n", + diagnostics = print_messages(&diagnostics) + )?; + } + + Ok(output) + } fn assert_lexed_ranges_match_slices( directive: Result, LexicalError>, @@ -2864,13 +3045,194 @@ mod tests { assert_lexed_ranges_match_slices(exemption, source); } + #[test] + fn add_noqa_to_existing_ignore() -> Result<()> { + assert_snapshot!( + add_suppressions_in( + r#" + def unused(x): # ruff:ignore[ANN001, ARG001, D103] + pass + "#, + SuppressionKind::Noqa, + PreviewMode::Disabled, + )?, + @" + Added 1 suppression + + ## Fixed source + + ```py + def unused(x): # ruff:ignore[ANN001, ARG001, D103] # noqa: ANN001, ANN201, D103 + pass + ``` + " + ); + Ok(()) + } + + #[test] + fn add_noqa_to_existing_ignore_preview() -> Result<()> { + assert_snapshot!( + add_suppressions_in( + r#" + def unused(x): # ruff:ignore[ANN001, ARG001, D103] + pass + "#, + SuppressionKind::Noqa, + PreviewMode::Enabled, + )?, + @" + Added 1 suppression + + ## Fixed source + + ```py + def unused(x): # ruff:ignore[ANN001, ARG001, D103] # noqa: ANN201 + pass + ``` + " + ); + Ok(()) + } + + #[test] + fn add_ignore_to_existing_noqa() -> Result<()> { + assert_snapshot!( + add_suppressions_in( + r#" + def unused(x): # noqa: ANN001, ARG001, D103 + pass + "#, + SuppressionKind::Ignore, + PreviewMode::Enabled, + )?, + @" + Added 1 suppression + + ## Fixed source + + ```py + def unused(x): # noqa: ANN001, ARG001, D103 # ruff:ignore[missing-return-type-undocumented-public-function] + pass + ``` + " + ); + Ok(()) + } + + #[test] + fn add_ignore_to_existing_ignore() -> Result<()> { + assert_snapshot!( + add_suppressions_in( + r#" + def unused(x): # ruff:ignore[ANN001, ARG001, D103] + pass + "#, + SuppressionKind::Ignore, + PreviewMode::Enabled, + )?, + @" + Added 1 suppression + + ## Fixed source + + ```py + def unused(x): # ruff:ignore[ANN001, ARG001, D103, missing-return-type-undocumented-public-function] + pass + ``` + " + ); + Ok(()) + } + + #[test] + fn add_ignore_multiple_codes() -> Result<()> { + assert_snapshot!( + add_suppressions_in( + r#" + def unused(x): + pass + "#, + SuppressionKind::Ignore, + PreviewMode::Enabled, + )?, + @" + Added 1 suppression + + ## Fixed source + + ```py + def unused(x): # ruff:ignore[missing-return-type-undocumented-public-function, missing-type-function-argument, undocumented-public-function] + pass + ``` + " + ); + Ok(()) + } + + #[test] + fn add_ignore_multiline_diagnostic() -> Result<()> { + assert_snapshot!( + add_suppressions_in( + r#" + import z + import c + import a + "#, + SuppressionKind::Ignore, + PreviewMode::Enabled, + )?, + @" + Added 1 suppression + + ## Fixed source + + ```py + import z # ruff:ignore[unsorted-imports] + import c + import a + ``` + " + ); + Ok(()) + } + + #[test] + fn add_ignore_existing_own_line_ignore() -> Result<()> { + assert_snapshot!( + add_suppressions_in( + r#" + # ruff:ignore[ANN001] + def public(x): + """Return x.""" + return x + "#, + SuppressionKind::Ignore, + PreviewMode::Enabled, + )?, + @r#" + Added 1 suppression + + ## Fixed source + + ```py + # ruff:ignore[ANN001, missing-return-type-undocumented-public-function] + def public(x): + """Return x.""" + return x + ``` + "# + ); + Ok(()) + } + #[test] fn modification() { let path = Path::new("/tmp/foo.txt"); let contents = "x = 1"; let noqa_line_for = NoqaMapping::default(); - let (count, output) = add_noqa_inner( + let (count, output) = add_suppression_inner( path, &[], &Locator::new(contents), @@ -2880,6 +3242,7 @@ mod tests { LineEnding::Lf, None, &Suppressions::default(), + SuppressionKind::Noqa, ); assert_eq!(count, 0); assert_eq!(output, format!("{contents}")); @@ -2895,7 +3258,7 @@ mod tests { let contents = "x = 1"; let noqa_line_for = NoqaMapping::default(); - let (count, output) = add_noqa_inner( + let (count, output) = add_suppression_inner( path, &messages, &Locator::new(contents), @@ -2905,6 +3268,7 @@ mod tests { LineEnding::Lf, None, &Suppressions::default(), + SuppressionKind::Noqa, ); assert_eq!(count, 1); assert_eq!(output, "x = 1 # noqa: F841\n"); @@ -2927,7 +3291,7 @@ mod tests { let noqa_line_for = NoqaMapping::default(); let comment_ranges = CommentRanges::new(vec![TextRange::new(TextSize::from(7), TextSize::from(19))]); - let (count, output) = add_noqa_inner( + let (count, output) = add_suppression_inner( path, &messages, &Locator::new(contents), @@ -2937,6 +3301,7 @@ mod tests { LineEnding::Lf, None, &Suppressions::default(), + SuppressionKind::Noqa, ); assert_eq!(count, 1); assert_eq!(output, "x = 1 # noqa: E741, F841\n"); @@ -2959,7 +3324,7 @@ mod tests { let noqa_line_for = NoqaMapping::default(); let comment_ranges = CommentRanges::new(vec![TextRange::new(TextSize::from(7), TextSize::from(13))]); - let (count, output) = add_noqa_inner( + let (count, output) = add_suppression_inner( path, &messages, &Locator::new(contents), @@ -2969,6 +3334,7 @@ mod tests { LineEnding::Lf, None, &Suppressions::default(), + SuppressionKind::Noqa, ); assert_eq!(count, 0); assert_eq!(output, "x = 1 # noqa"); @@ -2992,7 +3358,7 @@ print( .into_diagnostic(TextRange::new(12.into(), 79.into()), &source_file)]; let comment_ranges = CommentRanges::default(); let suppressions = Suppressions::default(); - let edits = generate_noqa_edits( + let edits = generate_suppression_edits( path, &messages, &Locator::new(source), @@ -3001,6 +3367,7 @@ print( &noqa_line_for, LineEnding::Lf, &suppressions, + SuppressionKind::Noqa, ); assert_eq!( edits, @@ -3025,7 +3392,7 @@ bar = let noqa_line_for = NoqaMapping::default(); let comment_ranges = CommentRanges::default(); let suppressions = Suppressions::default(); - let edits = generate_noqa_edits( + let edits = generate_suppression_edits( path, &messages, &Locator::new(source), @@ -3034,6 +3401,7 @@ bar = &noqa_line_for, LineEnding::Lf, &suppressions, + SuppressionKind::Noqa, ); assert_eq!( edits, diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 4ebf99d380f872..d6f2d9059c9edf 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -75,7 +75,7 @@ pub(crate) struct SuppressionComment { impl SuppressionComment { /// Return the suppressed codes as strings - fn codes_as_str<'src>(&self, source: &'src str) -> impl Iterator { + pub(crate) fn codes_as_str<'src>(&self, source: &'src str) -> impl Iterator { self.codes.iter().map(|range| source.slice(range)) } @@ -85,6 +85,12 @@ impl SuppressionComment { } } +impl Ranged for SuppressionComment { + fn range(&self) -> TextRange { + self.range + } +} + #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct PendingSuppressionComment<'a> { /// How indented an own-line comment is, or None for trailing comments @@ -140,6 +146,20 @@ impl Suppression { ) } + /// Returns whether the suppression's range applies to a diagnostic. + /// + /// `ruff:ignore` comments only need to contain the start of the diagnostic range (or its + /// parent), while range suppression comments must contain the entire diagnostic range. + fn applies_to_diagnostic(&self, range: TextRange, parent: Option) -> bool { + if self.is_ignore() { + self.range.contains(range.start()) + || range.is_empty() && self.range.end() == range.start() + || parent.is_some_and(|parent| self.range.contains(parent)) + } else { + self.range.contains_range(range) + } + } + /// Return the [`Rule`] associated with this suppression. fn rule(&self, preview: PreviewMode) -> Option { if let Ok(rule) = Rule::from_code(get_redirect_target(&self.code).unwrap_or(&self.code)) { @@ -266,6 +286,21 @@ impl Suppressions { self.valid.is_empty() && self.invalid.is_empty() && self.errors.is_empty() } + pub(crate) fn find_applicable_ignore( + &self, + diagnostic: &Diagnostic, + ) -> Option<&SuppressionComment> { + let range = diagnostic.primary_span()?.range()?; + + self.valid + .iter() + .find(|suppression| { + suppression.is_ignore() + && suppression.applies_to_diagnostic(range, diagnostic.parent()) + }) + .map(|suppression| suppression.comments.first()) + } + /// Check if a diagnostic is suppressed by any known range suppressions. /// /// A suppression applies for the given diagnostic if it fully contains the diagnostic's range. @@ -325,20 +360,7 @@ impl Suppressions { continue; } - // For `ruff:ignore` comments, only require that the start of the diagnostic range (or - // its parent) is covered by the suppression. Range suppression comments must fully - // contain the diagnostic range. - let suppressed = if suppression.is_ignore() { - suppression.range.contains(range.start()) - || range.is_empty() && suppression.range.end() == range.start() - || diagnostic - .parent() - .is_some_and(|parent| suppression.range.contains(parent)) - } else { - suppression.range.contains_range(range) - }; - - if suppressed { + if suppression.applies_to_diagnostic(range, diagnostic.parent()) { suppression.used.set(true); return true; } diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index f2af9a44b93828..5b2208a17fa32b 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -16,9 +16,9 @@ use crate::{ use ruff_db::diagnostic::{Annotation, Diagnostic, Span, SubDiagnostic}; use ruff_diagnostics::{Applicability, Edit, Fix}; use ruff_linter::{ - Locator, + Locator, SuppressionKind, directives::{Flags, extract_directives}, - generate_noqa_edits, + generate_suppression_edits, linter::{check_path, parse_unchecked_source}, package::PackageRoot, packaging::detect_package_root, @@ -43,7 +43,7 @@ pub(crate) struct AssociatedDiagnosticData { pub(crate) edits: Vec, /// The identifier displayed for the diagnostic. pub(crate) code: String, - /// Possible edit to add a `noqa` comment which will disable this diagnostic. + /// Possible edit to add a suppression comment which will disable this diagnostic. pub(crate) noqa_edit: Option, } @@ -60,7 +60,7 @@ pub(crate) struct DiagnosticFix { /// Edits to fix the diagnostic. If this is empty, a fix /// does not exist. pub(crate) edits: Vec, - /// Possible edit to add a `noqa` comment which will disable this diagnostic. + /// Possible edit to add a suppression comment which will disable this diagnostic. pub(crate) noqa_edit: Option, } @@ -148,7 +148,7 @@ pub(crate) fn check( &suppressions, ); - let noqa_edits = generate_noqa_edits( + let suppression_edits = generate_suppression_edits( &document_path, &diagnostics, &locator, @@ -157,6 +157,11 @@ pub(crate) fn check( &directives.noqa_line_for, stylist.line_ending(), &suppressions, + if is_human_readable_names_enabled(settings.linter.preview) { + SuppressionKind::Ignore + } else { + SuppressionKind::Noqa + }, ); let context = LspDiagnosticContext { source_kind: &source_kind, @@ -186,7 +191,7 @@ pub(crate) fn check( let lsp_diagnostics = diagnostics .into_iter() - .zip(noqa_edits) + .zip(suppression_edits) .filter_map(|(message, noqa_edit)| { if message.is_invalid_syntax() && !show_syntax_errors { None diff --git a/crates/ruff_server/tests/e2e/diagnostics.rs b/crates/ruff_server/tests/e2e/diagnostics.rs index 55669814572051..5e86a01a7cb808 100644 --- a/crates/ruff_server/tests/e2e/diagnostics.rs +++ b/crates/ruff_server/tests/e2e/diagnostics.rs @@ -58,7 +58,7 @@ fn uses_human_readable_names_in_preview() -> Result<()> { } ], "noqa_edit": { - "newText": " # noqa: F401\n", + "newText": " # ruff:ignore[unused-import]\n", "range": { "end": { "character": 0, diff --git a/docs/configuration.md b/docs/configuration.md index 2d86e5018e048d..3b424f66aac7a6 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -629,6 +629,10 @@ Options: --add-noqa[=] Enable automatic additions of `noqa` directives to failing lines. Optionally provide a reason to append after the codes + --add-ignore[=] + Enable automatic additions of `ruff:ignore` comments to failing + lines. Optionally provide a reason to append after the rule names. + Requires preview mode --show-files See the files Ruff will be run against with the current settings --show-settings diff --git a/docs/linter.md b/docs/linter.md index 8f6fac6bb3999a..22ce06c4b67b06 100644 --- a/docs/linter.md +++ b/docs/linter.md @@ -533,14 +533,17 @@ $ ruff check /path/to/file.py --extend-select RUF100 --fix ### Inserting necessary suppression comments -Ruff can _automatically add_ `noqa` directives to all lines that contain violations, which is -useful when migrating a new codebase to Ruff. To automatically add `noqa` directives to all -relevant lines (with the appropriate rule codes), run Ruff with `--add-noqa`, like so: +Ruff can _automatically add_ suppression comments to all lines that contain violations, which is +useful when migrating a new codebase to Ruff. To add the appropriate comments to all relevant +lines, run Ruff with `--add-noqa`: ```shell-session $ ruff check /path/to/file.py --add-noqa ``` +The `--add-noqa` flag adds `noqa` directives with rule codes. To add `ruff:ignore` comments with +human-readable rule names instead, use `--add-ignore` with preview mode enabled. + ### isort action comments Ruff respects isort's [action comments](https://pycqa.github.io/isort/docs/configuration/action_comments.html) diff --git a/docs/tutorial.md b/docs/tutorial.md index a428ca9b9ee408..dd96fc9a87242b 100644 --- a/docs/tutorial.md +++ b/docs/tutorial.md @@ -361,6 +361,9 @@ index 71fca60c8d..e92d839f1b 100644 +from typing import Iterable # noqa: UP035 ``` +To add `# ruff:ignore[...]` comments with human-readable rule names instead, use the +`--add-ignore` flag with preview mode enabled. + ## Integrations This tutorial has focused on Ruff's command-line interface, but Ruff can also be used as a