diff --git a/crates/ruff/src/checkers/noqa.rs b/crates/ruff/src/checkers/noqa.rs index e79f93007a81d..e9441552e8903 100644 --- a/crates/ruff/src/checkers/noqa.rs +++ b/crates/ruff/src/checkers/noqa.rs @@ -1,6 +1,5 @@ //! `NoQA` enforcement and validation. -use log::warn; use nohash_hasher::IntMap; use rustpython_parser::ast::Location; @@ -10,7 +9,7 @@ use ruff_python_ast::types::Range; use crate::codes::NoqaCode; use crate::noqa; -use crate::noqa::{extract_file_exemption, Directive, Exemption}; +use crate::noqa::{Directive, FileExemption}; use crate::registry::{AsRule, Rule}; use crate::rule_redirects::get_redirect_target; use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA}; @@ -26,63 +25,47 @@ pub fn check_noqa( ) -> Vec { let enforce_noqa = settings.rules.enabled(Rule::UnusedNOQA); - // Whether the file is exempted from all checks. - let mut file_exempted = false; + let lines: Vec<&str> = contents.universal_newlines().collect(); - // Codes that are globally exempted (within the current file). - let mut file_exemptions: Vec = vec![]; + // Identify any codes that are globally exempted (within the current file). + let exemption = noqa::file_exemption(&lines, commented_lines); // Map from line number to `noqa` directive on that line, along with any codes // that were matched by the directive. let mut noqa_directives: IntMap)> = IntMap::default(); - // Indices of diagnostics that were ignored by a `noqa` directive. - let mut ignored_diagnostics = vec![]; - - let lines: Vec<&str> = contents.universal_newlines().collect(); - for lineno in commented_lines { - match extract_file_exemption(lines[lineno - 1]) { - Exemption::All => { - file_exempted = true; - } - Exemption::Codes(codes) => { - file_exemptions.extend(codes.into_iter().filter_map(|code| { - if let Ok(rule) = Rule::from_code(get_redirect_target(code).unwrap_or(code)) { - Some(rule.noqa_code()) - } else { - warn!("Invalid code provided to `# ruff: noqa`: {}", code); - None - } - })); - } - Exemption::None => {} - } - - if enforce_noqa { + // Extract all `noqa` directives. + if enforce_noqa { + for lineno in commented_lines { noqa_directives .entry(lineno - 1) .or_insert_with(|| (noqa::extract_noqa_directive(lines[lineno - 1]), vec![])); } } + // Indices of diagnostics that were ignored by a `noqa` directive. + let mut ignored_diagnostics = vec![]; + // Remove any ignored diagnostics. for (index, diagnostic) in diagnostics.iter().enumerate() { if matches!(diagnostic.kind.rule(), Rule::BlanketNOQA) { continue; } - // If the file is exempted, ignore all diagnostics. - if file_exempted { - ignored_diagnostics.push(index); - continue; - } - - // If the diagnostic is ignored by a global exemption, ignore it. - if !file_exemptions.is_empty() { - if file_exemptions.contains(&diagnostic.kind.rule().noqa_code()) { + match &exemption { + FileExemption::All => { + // If the file is exempted, ignore all diagnostics. ignored_diagnostics.push(index); continue; } + FileExemption::Codes(codes) => { + // If the diagnostic is ignored by a global exemption, ignore it. + if codes.contains(&diagnostic.kind.rule().noqa_code()) { + ignored_diagnostics.push(index); + continue; + } + } + FileExemption::None => {} } // Is the violation ignored by a `noqa` directive on the parent line? diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index f224f393d025d..ee15d7d81ea93 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -28,49 +28,6 @@ static NOQA_LINE_REGEX: Lazy = Lazy::new(|| { }); static SPLIT_COMMA_REGEX: Lazy = Lazy::new(|| Regex::new(r"[,\s]").unwrap()); -#[derive(Debug)] -pub enum Exemption<'a> { - None, - All, - Codes(Vec<&'a str>), -} - -/// Return `true` if a file is exempt from checking based on the contents of the -/// given line. -pub fn extract_file_exemption(line: &str) -> Exemption { - let line = line.trim_start(); - - if line.starts_with("# flake8: noqa") - || line.starts_with("# flake8: NOQA") - || line.starts_with("# flake8: NoQA") - { - return Exemption::All; - } - - if let Some(remainder) = line - .strip_prefix("# ruff: noqa") - .or_else(|| line.strip_prefix("# ruff: NOQA")) - .or_else(|| line.strip_prefix("# ruff: NoQA")) - { - if remainder.is_empty() { - return Exemption::All; - } else if let Some(codes) = remainder.strip_prefix(':') { - let codes: Vec<&str> = SPLIT_COMMA_REGEX - .split(codes.trim()) - .map(str::trim) - .filter(|code| !code.is_empty()) - .collect(); - if codes.is_empty() { - warn!("Expected rule codes on `noqa` directive: \"{line}\""); - } - return Exemption::Codes(codes); - } - warn!("Unexpected suffix on `noqa` directive: \"{line}\""); - } - - Exemption::None -} - #[derive(Debug)] pub enum Directive<'a> { None, @@ -119,6 +76,47 @@ pub fn extract_noqa_directive(line: &str) -> Directive { } } +enum ParsedExemption<'a> { + None, + All, + Codes(Vec<&'a str>), +} + +/// Return a [`ParsedExemption`] for a given comment line. +fn parse_file_exemption(line: &str) -> ParsedExemption { + let line = line.trim_start(); + + if line.starts_with("# flake8: noqa") + || line.starts_with("# flake8: NOQA") + || line.starts_with("# flake8: NoQA") + { + return ParsedExemption::All; + } + + if let Some(remainder) = line + .strip_prefix("# ruff: noqa") + .or_else(|| line.strip_prefix("# ruff: NOQA")) + .or_else(|| line.strip_prefix("# ruff: NoQA")) + { + if remainder.is_empty() { + return ParsedExemption::All; + } else if let Some(codes) = remainder.strip_prefix(':') { + let codes: Vec<&str> = SPLIT_COMMA_REGEX + .split(codes.trim()) + .map(str::trim) + .filter(|code| !code.is_empty()) + .collect(); + if codes.is_empty() { + warn!("Expected rule codes on `noqa` directive: \"{line}\""); + } + return ParsedExemption::Codes(codes); + } + warn!("Unexpected suffix on `noqa` directive: \"{line}\""); + } + + ParsedExemption::None +} + /// Returns `true` if the string list of `codes` includes `code` (or an alias /// thereof). pub fn includes(needle: Rule, haystack: &[&str]) -> bool { @@ -147,6 +145,43 @@ pub fn rule_is_ignored( } } +pub enum FileExemption { + None, + All, + Codes(Vec), +} + +/// Extract the [`FileExemption`] for a given Python source file, enumerating any rules that are +/// globally ignored within the file. +pub fn file_exemption(lines: &[&str], commented_lines: &[usize]) -> FileExemption { + let mut exempt_codes: Vec = vec![]; + + for lineno in commented_lines { + match parse_file_exemption(lines[lineno - 1]) { + ParsedExemption::All => { + return FileExemption::All; + } + ParsedExemption::Codes(codes) => { + exempt_codes.extend(codes.into_iter().filter_map(|code| { + if let Ok(rule) = Rule::from_code(get_redirect_target(code).unwrap_or(code)) { + Some(rule.noqa_code()) + } else { + warn!("Invalid code provided to `# ruff: noqa`: {}", code); + None + } + })); + } + ParsedExemption::None => {} + } + } + + if exempt_codes.is_empty() { + FileExemption::None + } else { + FileExemption::Codes(exempt_codes) + } +} + pub fn add_noqa( path: &Path, diagnostics: &[Diagnostic], @@ -176,44 +211,26 @@ fn add_noqa_inner( // Map of line number to set of (non-ignored) diagnostic codes that are triggered on that line. let mut matches_by_line: FxHashMap = FxHashMap::default(); - // Whether the file is exempted from all checks. - let mut file_exempted = false; + let lines: Vec<&str> = contents.universal_newlines().collect(); + // Whether the file is exempted from all checks. // Codes that are globally exempted (within the current file). - let mut file_exemptions: Vec = vec![]; - - let lines: Vec<&str> = contents.universal_newlines().collect(); - for lineno in commented_lines { - match extract_file_exemption(lines[lineno - 1]) { - Exemption::All => { - file_exempted = true; - } - Exemption::Codes(codes) => { - file_exemptions.extend(codes.into_iter().filter_map(|code| { - if let Ok(rule) = Rule::from_code(get_redirect_target(code).unwrap_or(code)) { - Some(rule.noqa_code()) - } else { - warn!("Invalid code provided to `# ruff: noqa`: {}", code); - None - } - })); - } - Exemption::None => {} - } - } + let exemption = file_exemption(&lines, commented_lines); // Mark any non-ignored diagnostics. for diagnostic in diagnostics { - // If the file is exempted, don't add any noqa directives. - if file_exempted { - continue; - } - - // If the diagnostic is ignored by a global exemption, don't add a noqa directive. - if !file_exemptions.is_empty() { - if file_exemptions.contains(&diagnostic.kind.rule().noqa_code()) { + match &exemption { + FileExemption::All => { + // If the file is exempted, don't add any noqa directives. continue; } + 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()) { + continue; + } + } + FileExemption::None => {} } // Is the violation ignored by a `noqa` directive on the parent line?