Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor out common exemption-parsing logic #3670

Merged
merged 1 commit into from
Mar 22, 2023
Merged

Refactor out common exemption-parsing logic #3670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 21 additions & 38 deletions crates/ruff/src/checkers/noqa.rs
View file
Open in desktop
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
Open in desktop
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