diff --git a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_2.py b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_2.py new file mode 100644 index 0000000000000..0a2124da2624f --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_2.py @@ -0,0 +1,7 @@ +# noqa +# ruff : noqa +# ruff: noqa: F401 + + +# flake8: noqa +import math as m diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index d68b76fcad977..25caddc08266b 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -11,7 +11,9 @@ use ruff_source_file::Locator; use ruff_text_size::Ranged; use crate::fix::edits::delete_comment; -use crate::noqa::{Code, Directive, FileExemption, NoqaDirectives, NoqaMapping}; +use crate::noqa::{ + Code, Directive, FileExemption, FileNoqaDirectives, NoqaDirectives, NoqaMapping, +}; use crate::registry::{AsRule, Rule, RuleSet}; use crate::rule_redirects::get_redirect_target; use crate::rules::pygrep_hooks; @@ -31,13 +33,14 @@ pub(crate) fn check_noqa( settings: &LinterSettings, ) -> Vec { // Identify any codes that are globally exempted (within the current file). - let exemption = FileExemption::try_extract( + let file_noqa_directives = FileNoqaDirectives::extract( locator.contents(), comment_ranges, &settings.external, path, locator, ); + let exemption = FileExemption::from(&file_noqa_directives); // Extract all `noqa` directives. let mut noqa_directives = NoqaDirectives::from_commented_ranges(comment_ranges, path, locator); @@ -52,19 +55,18 @@ pub(crate) fn check_noqa( } match &exemption { - Some(FileExemption::All) => { + FileExemption::All => { // If the file is exempted, ignore all diagnostics. ignored_diagnostics.push(index); continue; } - Some(FileExemption::Codes(codes)) => { + FileExemption::Codes(codes) => { // If the diagnostic is ignored by a global exemption, ignore it. - if codes.contains(&diagnostic.kind.rule().noqa_code()) { + if codes.contains(&&diagnostic.kind.rule().noqa_code()) { ignored_diagnostics.push(index); continue; } } - None => {} } let noqa_offsets = diagnostic @@ -109,10 +111,7 @@ pub(crate) fn check_noqa( // suppressed. if settings.rules.enabled(Rule::UnusedNOQA) && analyze_directives - && !exemption.is_some_and(|exemption| match exemption { - FileExemption::All => true, - FileExemption::Codes(codes) => codes.contains(&Rule::UnusedNOQA.noqa_code()), - }) + && !exemption.includes(Rule::UnusedNOQA) && !per_file_ignores.contains(Rule::UnusedNOQA) { for line in noqa_directives.lines() { @@ -215,7 +214,13 @@ pub(crate) fn check_noqa( } if settings.rules.enabled(Rule::BlanketNOQA) { - pygrep_hooks::rules::blanket_noqa(diagnostics, &noqa_directives, locator); + pygrep_hooks::rules::blanket_noqa( + diagnostics, + &noqa_directives, + locator, + &file_noqa_directives, + settings.preview, + ); } if settings.rules.enabled(Rule::RedirectedNOQA) { diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index a62ff92d59cdd..fa0fe1da6dc61 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -33,8 +33,9 @@ pub fn generate_noqa_edits( noqa_line_for: &NoqaMapping, line_ending: LineEnding, ) -> Vec> { - let exemption = - FileExemption::try_extract(locator.contents(), comment_ranges, external, path, locator); + let file_directives = + FileNoqaDirectives::extract(locator.contents(), comment_ranges, external, path, locator); + let exemption = FileExemption::from(&file_directives); let directives = NoqaDirectives::from_commented_ranges(comment_ranges, path, locator); let comments = find_noqa_comments(diagnostics, locator, &exemption, &directives, noqa_line_for); build_noqa_edits_by_diagnostic(comments, locator, line_ending) @@ -275,26 +276,79 @@ pub(crate) fn rule_is_ignored( } } -/// The file-level exemptions extracted from a given Python file. +/// A summary of the file-level exemption as extracted from [`FileNoqaDirectives`]. #[derive(Debug)] -pub(crate) enum FileExemption { +pub(crate) enum FileExemption<'a> { /// The file is exempt from all rules. All, /// The file is exempt from the given rules. - Codes(Vec), + Codes(Vec<&'a NoqaCode>), } -impl FileExemption { - /// Extract the [`FileExemption`] for a given Python source file, enumerating any rules that are - /// globally ignored within the file. - pub(crate) fn try_extract( - contents: &str, +impl<'a> FileExemption<'a> { + /// Returns `true` if the file is exempt from the given rule. + pub(crate) fn includes(&self, needle: Rule) -> bool { + let needle = needle.noqa_code(); + match self { + FileExemption::All => true, + FileExemption::Codes(codes) => codes.iter().any(|code| needle == **code), + } + } +} + +impl<'a> From<&'a FileNoqaDirectives<'a>> for FileExemption<'a> { + fn from(directives: &'a FileNoqaDirectives) -> Self { + if directives + .lines() + .iter() + .any(|line| ParsedFileExemption::All == line.parsed_file_exemption) + { + FileExemption::All + } else { + FileExemption::Codes( + directives + .lines() + .iter() + .flat_map(|line| &line.matches) + .collect(), + ) + } + } +} + +/// The directive for a file-level exemption (e.g., `# ruff: noqa`) from an individual line. +#[derive(Debug)] +pub(crate) struct FileNoqaDirectiveLine<'a> { + /// The range of the text line for which the noqa directive applies. + pub(crate) range: TextRange, + /// The blanket noqa directive. + pub(crate) parsed_file_exemption: ParsedFileExemption<'a>, + /// The codes that are ignored by the parsed exemptions. + pub(crate) matches: Vec, +} + +impl Ranged for FileNoqaDirectiveLine<'_> { + /// The range of the `noqa` directive. + fn range(&self) -> TextRange { + self.range + } +} + +/// All file-level exemptions (e.g., `# ruff: noqa`) from a given Python file. +#[derive(Debug)] +pub(crate) struct FileNoqaDirectives<'a>(Vec>); + +impl<'a> FileNoqaDirectives<'a> { + /// Extract the [`FileNoqaDirectives`] for a given Python source file, enumerating any rules + /// that are globally ignored within the file. + pub(crate) fn extract( + contents: &'a str, comment_ranges: &CommentRanges, external: &[String], path: &Path, locator: &Locator, - ) -> Option { - let mut exempt_codes: Vec = vec![]; + ) -> Self { + let mut lines = vec![]; for range in comment_ranges { match ParsedFileExemption::try_extract(&contents[*range]) { @@ -313,12 +367,12 @@ impl FileExemption { continue; } - match exemption { + let matches = match &exemption { ParsedFileExemption::All => { - return Some(Self::All); + vec![] } ParsedFileExemption::Codes(codes) => { - exempt_codes.extend(codes.into_iter().filter_map(|code| { + codes.iter().filter_map(|code| { // Ignore externally-defined rules. if external.iter().any(|external| code.starts_with(external)) { return None; @@ -334,27 +388,33 @@ impl FileExemption { warn!("Invalid rule code provided to `# ruff: noqa` at {path_display}:{line}: {code}"); None } - })); + }).collect() } - } + }; + + lines.push(FileNoqaDirectiveLine { + range: *range, + parsed_file_exemption: exemption, + matches, + }); } Ok(None) => {} } } - if exempt_codes.is_empty() { - None - } else { - Some(Self::Codes(exempt_codes)) - } + Self(lines) + } + + pub(crate) fn lines(&self) -> &[FileNoqaDirectiveLine] { + &self.0 } } /// An individual file-level exemption (e.g., `# ruff: noqa` or `# ruff: noqa: F401, F841`). Like -/// [`FileExemption`], but only for a single line, as opposed to an aggregated set of exemptions +/// [`FileNoqaDirectives`], but only for a single line, as opposed to an aggregated set of exemptions /// across a source file. -#[derive(Debug)] -enum ParsedFileExemption<'a> { +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum ParsedFileExemption<'a> { /// The file-level exemption ignores all rules (e.g., `# ruff: noqa`). All, /// The file-level exemption ignores specific rules (e.g., `# ruff: noqa: F401, F841`). @@ -549,9 +609,10 @@ fn add_noqa_inner( let mut count = 0; // Whether the file is exempted from all checks. - // Codes that are globally exempted (within the current file). - let exemption = - FileExemption::try_extract(locator.contents(), comment_ranges, external, path, locator); + let directives = + FileNoqaDirectives::extract(locator.contents(), comment_ranges, external, path, locator); + let exemption = FileExemption::from(&directives); + let directives = NoqaDirectives::from_commented_ranges(comment_ranges, path, locator); let comments = find_noqa_comments(diagnostics, locator, &exemption, &directives, noqa_line_for); @@ -646,7 +707,7 @@ struct NoqaComment<'a> { fn find_noqa_comments<'a>( diagnostics: &'a [Diagnostic], locator: &'a Locator, - exemption: &Option, + exemption: &'a FileExemption, directives: &'a NoqaDirectives, noqa_line_for: &NoqaMapping, ) -> Vec>> { @@ -656,19 +717,18 @@ fn find_noqa_comments<'a>( // Mark any non-ignored diagnostics. for diagnostic in diagnostics { match &exemption { - Some(FileExemption::All) => { + FileExemption::All => { // If the file is exempted, don't add any noqa directives. comments_by_line.push(None); continue; } - Some(FileExemption::Codes(codes)) => { + FileExemption::Codes(codes) => { // If the diagnostic is ignored by a global exemption, don't add a noqa directive. - if codes.contains(&diagnostic.kind.rule().noqa_code()) { + if codes.contains(&&diagnostic.kind.rule().noqa_code()) { comments_by_line.push(None); continue; } } - None => {} } // Is the violation ignored by a `noqa` directive on the parent line? diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs b/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs index f06e8eda13780..96b9cb66e6859 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs +++ b/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs @@ -10,6 +10,7 @@ mod tests { use crate::registry::Rule; + use crate::settings::types::PreviewMode; use crate::test::test_path; use crate::{assert_messages, settings}; @@ -17,6 +18,7 @@ mod tests { #[test_case(Rule::BlanketTypeIgnore, Path::new("PGH003_1.py"))] #[test_case(Rule::BlanketNOQA, Path::new("PGH004_0.py"))] #[test_case(Rule::BlanketNOQA, Path::new("PGH004_1.py"))] + #[test_case(Rule::BlanketNOQA, Path::new("PGH004_2.py"))] #[test_case(Rule::InvalidMockAccess, Path::new("PGH005_0.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); @@ -27,4 +29,22 @@ mod tests { assert_messages!(snapshot, diagnostics); Ok(()) } + + #[test_case(Rule::BlanketNOQA, Path::new("PGH004_2.py"))] + fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!( + "preview__{}_{}", + rule_code.noqa_code(), + path.to_string_lossy() + ); + let diagnostics = test_path( + Path::new("pygrep_hooks").join(path).as_path(), + &settings::LinterSettings { + preview: PreviewMode::Enabled, + ..settings::LinterSettings::for_rule(rule_code) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs b/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs index 62e40938dae56..0e1833778590e 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs +++ b/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs @@ -4,7 +4,8 @@ use ruff_python_trivia::Cursor; use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextRange}; -use crate::noqa::{Directive, NoqaDirectives}; +use crate::noqa::{Directive, FileNoqaDirectives, NoqaDirectives, ParsedFileExemption}; +use crate::settings::types::PreviewMode; /// ## What it does /// Check for `noqa` annotations that suppress all diagnostics, as opposed to @@ -17,6 +18,9 @@ use crate::noqa::{Directive, NoqaDirectives}; /// maintain, as the annotation does not clarify which diagnostics are intended /// to be suppressed. /// +/// In [preview], this rule also checks for blanket file-level annotations (e.g., +/// `# ruff: noqa`, as opposed to `# ruff: noqa: F401`). +/// /// ## Example /// ```python /// from .base import * # noqa @@ -37,10 +41,13 @@ use crate::noqa::{Directive, NoqaDirectives}; /// /// ## References /// - [Ruff documentation](https://docs.astral.sh/ruff/configuration/#error-suppression) +/// +/// [preview]: https://docs.astral.sh/ruff/preview/ #[violation] pub struct BlanketNOQA { missing_colon: bool, space_before_colon: bool, + file_exemption: bool, } impl Violation for BlanketNOQA { @@ -51,12 +58,15 @@ impl Violation for BlanketNOQA { let BlanketNOQA { missing_colon, space_before_colon, + file_exemption, } = self; // This awkward branching is necessary to ensure that the generic message is picked up by // `derive_message_formats`. - if !missing_colon && !space_before_colon { + if !missing_colon && !space_before_colon && !file_exemption { format!("Use specific rule codes when using `noqa`") + } else if *file_exemption { + format!("Use specific rule codes when using `ruff: noqa`") } else if *missing_colon { format!("Use a colon when specifying `noqa` rule codes") } else { @@ -68,6 +78,7 @@ impl Violation for BlanketNOQA { let BlanketNOQA { missing_colon, space_before_colon, + .. } = self; if *missing_colon { @@ -85,7 +96,24 @@ pub(crate) fn blanket_noqa( diagnostics: &mut Vec, noqa_directives: &NoqaDirectives, locator: &Locator, + file_noqa_directives: &FileNoqaDirectives, + preview: PreviewMode, ) { + if preview.is_enabled() { + for line in file_noqa_directives.lines() { + if let ParsedFileExemption::All = line.parsed_file_exemption { + diagnostics.push(Diagnostic::new( + BlanketNOQA { + missing_colon: false, + space_before_colon: false, + file_exemption: true, + }, + line.range(), + )); + } + } + } + for directive_line in noqa_directives.lines() { if let Directive::All(all) = &directive_line.directive { let line = locator.slice(directive_line); @@ -104,6 +132,7 @@ pub(crate) fn blanket_noqa( BlanketNOQA { missing_colon: false, space_before_colon: true, + file_exemption: false, }, TextRange::new(all.start(), end), ); @@ -118,6 +147,7 @@ pub(crate) fn blanket_noqa( BlanketNOQA { missing_colon: true, space_before_colon: false, + file_exemption: false, }, TextRange::new(all.start(), end), ); @@ -129,6 +159,7 @@ pub(crate) fn blanket_noqa( BlanketNOQA { missing_colon: false, space_before_colon: false, + file_exemption: false, }, all.range(), )); diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_2.py.snap b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_2.py.snap new file mode 100644 index 0000000000000..e2b9bdf292282 --- /dev/null +++ b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_2.py.snap @@ -0,0 +1,10 @@ +--- +source: crates/ruff_linter/src/rules/pygrep_hooks/mod.rs +--- +PGH004_2.py:1:1: PGH004 Use specific rule codes when using `noqa` + | +1 | # noqa + | ^^^^^^ PGH004 +2 | # ruff : noqa +3 | # ruff: noqa: F401 + | diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__preview__PGH004_PGH004_2.py.snap b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__preview__PGH004_PGH004_2.py.snap new file mode 100644 index 0000000000000..6c93b8a1083f8 --- /dev/null +++ b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__preview__PGH004_PGH004_2.py.snap @@ -0,0 +1,25 @@ +--- +source: crates/ruff_linter/src/rules/pygrep_hooks/mod.rs +--- +PGH004_2.py:1:1: PGH004 Use specific rule codes when using `noqa` + | +1 | # noqa + | ^^^^^^ PGH004 +2 | # ruff : noqa +3 | # ruff: noqa: F401 + | + +PGH004_2.py:2:1: PGH004 Use specific rule codes when using `ruff: noqa` + | +1 | # noqa +2 | # ruff : noqa + | ^^^^^^^^^^^^^ PGH004 +3 | # ruff: noqa: F401 + | + +PGH004_2.py:6:1: PGH004 Use specific rule codes when using `ruff: noqa` + | +6 | # flake8: noqa + | ^^^^^^^^^^^^^^ PGH004 +7 | import math as m + |