Skip to content

Commit

Permalink
Refactor out common exemption-parsing logic (#3670)
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh authored Mar 22, 2023
1 parent fe568c0 commit 07808a5
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 112 deletions.
59 changes: 21 additions & 38 deletions crates/ruff/src/checkers/noqa.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! `NoQA` enforcement and validation.

use log::warn;
use nohash_hasher::IntMap;
use rustpython_parser::ast::Location;

Expand All @@ -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};
Expand All @@ -26,63 +25,47 @@ pub fn check_noqa(
) -> Vec<usize> {
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<NoqaCode> = 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<usize, (Directive, Vec<NoqaCode>)> = 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?
Expand Down
165 changes: 91 additions & 74 deletions crates/ruff/src/noqa.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,49 +28,6 @@ static NOQA_LINE_REGEX: Lazy<Regex> = Lazy::new(|| {
});
static SPLIT_COMMA_REGEX: Lazy<Regex> = 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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -147,6 +145,43 @@ pub fn rule_is_ignored(
}
}

pub enum FileExemption {
None,
All,
Codes(Vec<NoqaCode>),
}

/// 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<NoqaCode> = 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],
Expand Down Expand Up @@ -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<usize, RuleSet> = 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<NoqaCode> = 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?
Expand Down

0 comments on commit 07808a5

Please sign in to comment.