Skip to content

Commit

Permalink
[pygrep-hooks] Adds Check for Blanket # noqa (#1440)
Browse files Browse the repository at this point in the history
  • Loading branch information
saadmk11 authored Dec 29, 2022
1 parent acf0b82 commit 781bbbc
Show file tree
Hide file tree
Showing 12 changed files with 152 additions and 22 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,7 @@ For more, see [pygrep-hooks](https://github.com/pre-commit/pygrep-hooks) on GitH
| PGH001 | NoEval | No builtin `eval()` allowed | |
| PGH002 | DeprecatedLogWarn | `warn` is deprecated in favor of `warning` | |
| PGH003 | BlanketTypeIgnore | Use specific error codes when ignoring type issues | |
| PGH004 | BlanketNOQA | Use specific error codes when using `noqa` | |
### Pylint (PLC, PLE, PLR, PLW)
Expand Down
11 changes: 11 additions & 0 deletions resources/test/fixtures/pygrep-hooks/PGH004_0.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
x = 1 # noqa
x = 1 # NOQA:F401,W203
# noqa
# NOQA
# noqa:F401
# noqa:F401,W203

x = 1
x = 1 # noqa: F401, W203
# noqa: F401
# noqa: F401, W203
1 change: 1 addition & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,7 @@
"PGH001",
"PGH002",
"PGH003",
"PGH004",
"PLC",
"PLC0",
"PLC04",
Expand Down
11 changes: 10 additions & 1 deletion src/checkers/lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use crate::checks::{Check, CheckCode};
use crate::pycodestyle::checks::{line_too_long, no_newline_at_end_of_file};
use crate::pygrep_hooks::plugins::blanket_type_ignore;
use crate::pygrep_hooks::plugins::{blanket_noqa, blanket_type_ignore};
use crate::pyupgrade::checks::unnecessary_coding_comment;
use crate::settings::{flags, Settings};

Expand All @@ -18,6 +18,7 @@ pub fn check_lines(
let enforce_line_too_long = settings.enabled.contains(&CheckCode::E501);
let enforce_no_newline_at_end_of_file = settings.enabled.contains(&CheckCode::W292);
let enforce_blanket_type_ignore = settings.enabled.contains(&CheckCode::PGH003);
let enforce_blanket_noqa = settings.enabled.contains(&CheckCode::PGH004);

let mut commented_lines_iter = commented_lines.iter().peekable();
for (index, line) in contents.lines().enumerate() {
Expand Down Expand Up @@ -45,6 +46,14 @@ pub fn check_lines(
}
}
}

if enforce_blanket_noqa {
if commented_lines.contains(&(index + 1)) {
if let Some(check) = blanket_noqa(index, line) {
checks.push(check);
}
}
}
}

if enforce_line_too_long {
Expand Down
3 changes: 3 additions & 0 deletions src/checkers/noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ pub fn check_noqa(
while let Some((index, check)) =
checks_iter.next_if(|(_index, check)| check.location.row() <= *lineno)
{
if check.kind == CheckKind::BlanketNOQA {
continue;
}
// Grab the noqa (logical) line number for the current (physical) line.
// If there are newlines at the end of the file, they won't be represented in
// `noqa_line_for`, so fallback to the current line.
Expand Down
22 changes: 15 additions & 7 deletions src/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ pub enum CheckCode {
PGH001,
PGH002,
PGH003,
PGH004,
// pandas-vet
PD002,
PD003,
Expand Down Expand Up @@ -931,6 +932,7 @@ pub enum CheckKind {
NoEval,
DeprecatedLogWarn,
BlanketTypeIgnore,
BlanketNOQA,
// flake8-unused-arguments
UnusedFunctionArgument(String),
UnusedMethodArgument(String),
Expand Down Expand Up @@ -980,9 +982,11 @@ impl CheckCode {
pub fn lint_source(&self) -> &'static LintSource {
match self {
CheckCode::RUF100 => &LintSource::NoQA,
CheckCode::E501 | CheckCode::W292 | CheckCode::UP009 | CheckCode::PGH003 => {
&LintSource::Lines
}
CheckCode::E501
| CheckCode::W292
| CheckCode::UP009
| CheckCode::PGH003
| CheckCode::PGH004 => &LintSource::Lines,
CheckCode::ERA001
| CheckCode::Q000
| CheckCode::Q001
Expand Down Expand Up @@ -1327,6 +1331,7 @@ impl CheckCode {
CheckCode::PGH001 => CheckKind::NoEval,
CheckCode::PGH002 => CheckKind::DeprecatedLogWarn,
CheckCode::PGH003 => CheckKind::BlanketTypeIgnore,
CheckCode::PGH004 => CheckKind::BlanketNOQA,
// flake8-unused-arguments
CheckCode::ARG001 => CheckKind::UnusedFunctionArgument("...".to_string()),
CheckCode::ARG002 => CheckKind::UnusedMethodArgument("...".to_string()),
Expand Down Expand Up @@ -1594,6 +1599,7 @@ impl CheckCode {
CheckCode::PGH001 => CheckCategory::PygrepHooks,
CheckCode::PGH002 => CheckCategory::PygrepHooks,
CheckCode::PGH003 => CheckCategory::PygrepHooks,
CheckCode::PGH004 => CheckCategory::PygrepHooks,
CheckCode::PLC0414 => CheckCategory::Pylint,
CheckCode::PLC2201 => CheckCategory::Pylint,
CheckCode::PLC3002 => CheckCategory::Pylint,
Expand Down Expand Up @@ -1955,6 +1961,7 @@ impl CheckKind {
CheckKind::NoEval => &CheckCode::PGH001,
CheckKind::DeprecatedLogWarn => &CheckCode::PGH002,
CheckKind::BlanketTypeIgnore => &CheckCode::PGH003,
CheckKind::BlanketNOQA => &CheckCode::PGH004,
// flake8-unused-arguments
CheckKind::UnusedFunctionArgument(..) => &CheckCode::ARG001,
CheckKind::UnusedMethodArgument(..) => &CheckCode::ARG002,
Expand Down Expand Up @@ -2816,13 +2823,14 @@ impl CheckKind {
"Boolean positional value in function call".to_string()
}
// pygrep-hooks
CheckKind::NoEval => "No builtin `eval()` allowed".to_string(),
CheckKind::DeprecatedLogWarn => {
"`warn` is deprecated in favor of `warning`".to_string()
}
CheckKind::BlanketNOQA => "Use specific error codes when using `noqa`".to_string(),
CheckKind::BlanketTypeIgnore => {
"Use specific error codes when ignoring type issues".to_string()
}
CheckKind::DeprecatedLogWarn => {
"`warn` is deprecated in favor of `warning`".to_string()
}
CheckKind::NoEval => "No builtin `eval()` allowed".to_string(),
// flake8-unused-arguments
CheckKind::UnusedFunctionArgument(name) => {
format!("Unused function argument: `{name}`")
Expand Down
25 changes: 22 additions & 3 deletions src/checks_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ pub enum CheckCodePrefix {
PGH001,
PGH002,
PGH003,
PGH004,
PLC,
PLC0,
PLC04,
Expand Down Expand Up @@ -857,6 +858,7 @@ impl CheckCodePrefix {
CheckCode::PGH001,
CheckCode::PGH002,
CheckCode::PGH003,
CheckCode::PGH004,
CheckCode::PD002,
CheckCode::PD003,
CheckCode::PD004,
Expand Down Expand Up @@ -2073,12 +2075,28 @@ impl CheckCodePrefix {
);
vec![CheckCode::PD901]
}
CheckCodePrefix::PGH => vec![CheckCode::PGH001, CheckCode::PGH002, CheckCode::PGH003],
CheckCodePrefix::PGH0 => vec![CheckCode::PGH001, CheckCode::PGH002, CheckCode::PGH003],
CheckCodePrefix::PGH00 => vec![CheckCode::PGH001, CheckCode::PGH002, CheckCode::PGH003],
CheckCodePrefix::PGH => vec![
CheckCode::PGH001,
CheckCode::PGH002,
CheckCode::PGH003,
CheckCode::PGH004,
],
CheckCodePrefix::PGH0 => vec![
CheckCode::PGH001,
CheckCode::PGH002,
CheckCode::PGH003,
CheckCode::PGH004,
],
CheckCodePrefix::PGH00 => vec![
CheckCode::PGH001,
CheckCode::PGH002,
CheckCode::PGH003,
CheckCode::PGH004,
],
CheckCodePrefix::PGH001 => vec![CheckCode::PGH001],
CheckCodePrefix::PGH002 => vec![CheckCode::PGH002],
CheckCodePrefix::PGH003 => vec![CheckCode::PGH003],
CheckCodePrefix::PGH004 => vec![CheckCode::PGH004],
CheckCodePrefix::PLC => {
vec![CheckCode::PLC0414, CheckCode::PLC2201, CheckCode::PLC3002]
}
Expand Down Expand Up @@ -3152,6 +3170,7 @@ impl CheckCodePrefix {
CheckCodePrefix::PGH001 => SuffixLength::Three,
CheckCodePrefix::PGH002 => SuffixLength::Three,
CheckCodePrefix::PGH003 => SuffixLength::Three,
CheckCodePrefix::PGH004 => SuffixLength::Three,
CheckCodePrefix::PLC => SuffixLength::Zero,
CheckCodePrefix::PLC0 => SuffixLength::One,
CheckCodePrefix::PLC04 => SuffixLength::Two,
Expand Down
22 changes: 11 additions & 11 deletions src/noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustc_hash::{FxHashMap, FxHashSet};

use crate::checks::{Check, CheckCode, CODE_REDIRECTS};

static NO_QA_LINE_REGEX: Lazy<Regex> = Lazy::new(|| {
static NOQA_LINE_REGEX: Lazy<Regex> = Lazy::new(|| {
Regex::new(
r"(?P<spaces>\s*)(?P<noqa>(?i:# noqa)(?::\s?(?P<codes>([A-Z]+[0-9]+(?:[,\s]+)?)+))?)",
)
Expand Down Expand Up @@ -39,7 +39,7 @@ pub enum Directive<'a> {

/// Extract the noqa `Directive` from a line of Python source code.
pub fn extract_noqa_directive(line: &str) -> Directive {
match NO_QA_LINE_REGEX.captures(line) {
match NOQA_LINE_REGEX.captures(line) {
Some(caps) => match caps.name("spaces") {
Some(spaces) => match caps.name("noqa") {
Some(noqa) => match caps.name("codes") {
Expand Down Expand Up @@ -206,20 +206,20 @@ mod tests {

use crate::ast::types::Range;
use crate::checks::{Check, CheckKind};
use crate::noqa::{add_noqa_inner, NO_QA_LINE_REGEX};
use crate::noqa::{add_noqa_inner, NOQA_LINE_REGEX};

#[test]
fn regex() {
assert!(NO_QA_LINE_REGEX.is_match("# noqa"));
assert!(NO_QA_LINE_REGEX.is_match("# NoQA"));
assert!(NOQA_LINE_REGEX.is_match("# noqa"));
assert!(NOQA_LINE_REGEX.is_match("# NoQA"));

assert!(NO_QA_LINE_REGEX.is_match("# noqa: F401"));
assert!(NO_QA_LINE_REGEX.is_match("# NoQA: F401"));
assert!(NO_QA_LINE_REGEX.is_match("# noqa: F401, E501"));
assert!(NOQA_LINE_REGEX.is_match("# noqa: F401"));
assert!(NOQA_LINE_REGEX.is_match("# NoQA: F401"));
assert!(NOQA_LINE_REGEX.is_match("# noqa: F401, E501"));

assert!(NO_QA_LINE_REGEX.is_match("# noqa:F401"));
assert!(NO_QA_LINE_REGEX.is_match("# NoQA:F401"));
assert!(NO_QA_LINE_REGEX.is_match("# noqa:F401, E501"));
assert!(NOQA_LINE_REGEX.is_match("# noqa:F401"));
assert!(NOQA_LINE_REGEX.is_match("# NoQA:F401"));
assert!(NOQA_LINE_REGEX.is_match("# noqa:F401, E501"));
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions src/pygrep_hooks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ mod tests {
#[test_case(CheckCode::PGH002, Path::new("PGH002_0.py"); "PGH002_0")]
#[test_case(CheckCode::PGH002, Path::new("PGH002_1.py"); "PGH002_1")]
#[test_case(CheckCode::PGH003, Path::new("PGH003_0.py"); "PGH003_0")]
#[test_case(CheckCode::PGH004, Path::new("PGH004_0.py"); "PGH004_0")]
fn checks(check_code: CheckCode, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy());
let mut checks = test_path(
Expand Down
22 changes: 22 additions & 0 deletions src/pygrep_hooks/plugins/blanket_noqa.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use once_cell::sync::Lazy;
use regex::Regex;
use rustpython_ast::Location;

use crate::ast::types::Range;
use crate::checks::{Check, CheckKind};

static BLANKET_NOQA_REGEX: Lazy<Regex> =
Lazy::new(|| Regex::new(r"(?i)# noqa($|\s|:[^ ])").unwrap());

/// PGH004 - use of blanket noqa comments
pub fn blanket_noqa(lineno: usize, line: &str) -> Option<Check> {
BLANKET_NOQA_REGEX.find(line).map(|m| {
Check::new(
CheckKind::BlanketNOQA,
Range {
location: Location::new(lineno + 1, m.start()),
end_location: Location::new(lineno + 1, m.end()),
},
)
})
}
2 changes: 2 additions & 0 deletions src/pygrep_hooks/plugins/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
pub use blanket_noqa::blanket_noqa;
pub use blanket_type_ignore::blanket_type_ignore;
pub use deprecated_log_warn::deprecated_log_warn;
pub use no_eval::no_eval;

mod blanket_noqa;
mod blanket_type_ignore;
mod deprecated_log_warn;
mod no_eval;
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
---
source: src/pygrep_hooks/mod.rs
expression: checks
---
- kind: BlanketNoqa
location:
row: 1
column: 7
end_location:
row: 1
column: 13
fix: ~
- kind: BlanketNoqa
location:
row: 2
column: 7
end_location:
row: 2
column: 15
fix: ~
- kind: BlanketNoqa
location:
row: 3
column: 0
end_location:
row: 3
column: 6
fix: ~
- kind: BlanketNoqa
location:
row: 4
column: 0
end_location:
row: 4
column: 6
fix: ~
- kind: BlanketNoqa
location:
row: 5
column: 0
end_location:
row: 5
column: 8
fix: ~
- kind: BlanketNoqa
location:
row: 6
column: 0
end_location:
row: 6
column: 8
fix: ~

0 comments on commit 781bbbc

Please sign in to comment.