From 87465daacc2d6f28e2f2e084657cf47e04713616 Mon Sep 17 00:00:00 2001 From: Reiner Gerecke Date: Mon, 19 Dec 2022 00:04:21 +0100 Subject: [PATCH] pygrep-hooks - deprecated use of logging.warn & no blanket type ignore (#1275) --- README.md | 2 + .../test/fixtures/pygrep-hooks/PGH002_0.py | 8 +++ .../test/fixtures/pygrep-hooks/PGH002_1.py | 15 ++++++ .../test/fixtures/pygrep-hooks/PGH003_0.py | 11 +++++ src/checkers/ast.rs | 5 +- src/checkers/lines.rs | 49 +++++++++++++------ src/checks.rs | 20 +++++++- src/checks_gen.rs | 12 +++-- src/linter.rs | 7 ++- src/pygrep_hooks/mod.rs | 5 +- .../plugins/blanket_type_ignore.rs | 22 +++++++++ .../plugins/deprecated_log_warn.rs | 19 +++++++ src/pygrep_hooks/plugins/mod.rs | 7 +++ .../{checks.rs => plugins/no_eval.rs} | 1 + ...grep_hooks__tests__PGH002_PGH002_0.py.snap | 6 +++ ...grep_hooks__tests__PGH002_PGH002_1.py.snap | 37 ++++++++++++++ ...grep_hooks__tests__PGH003_PGH003_0.py.snap | 29 +++++++++++ 17 files changed, 233 insertions(+), 22 deletions(-) create mode 100644 resources/test/fixtures/pygrep-hooks/PGH002_0.py create mode 100644 resources/test/fixtures/pygrep-hooks/PGH002_1.py create mode 100644 resources/test/fixtures/pygrep-hooks/PGH003_0.py create mode 100644 src/pygrep_hooks/plugins/blanket_type_ignore.rs create mode 100644 src/pygrep_hooks/plugins/deprecated_log_warn.rs create mode 100644 src/pygrep_hooks/plugins/mod.rs rename src/pygrep_hooks/{checks.rs => plugins/no_eval.rs} (95%) create mode 100644 src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH002_PGH002_0.py.snap create mode 100644 src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH002_PGH002_1.py.snap create mode 100644 src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH003_PGH003_0.py.snap diff --git a/README.md b/README.md index e111394531..9e9b7a1dab 100644 --- a/README.md +++ b/README.md @@ -900,6 +900,8 @@ For more, see [pygrep-hooks](https://github.com/pre-commit/pygrep-hooks) on GitH | Code | Name | Message | Fix | | ---- | ---- | ------- | --- | | 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 | | ### Pylint (PLC, PLE, PLR, PLW) diff --git a/resources/test/fixtures/pygrep-hooks/PGH002_0.py b/resources/test/fixtures/pygrep-hooks/PGH002_0.py new file mode 100644 index 0000000000..e6aa467b13 --- /dev/null +++ b/resources/test/fixtures/pygrep-hooks/PGH002_0.py @@ -0,0 +1,8 @@ +import logging +import warnings +from warnings import warn + +warnings.warn("this is ok") +warn("by itself is also ok") +logging.warning("this is fine") +log.warning("this is ok") diff --git a/resources/test/fixtures/pygrep-hooks/PGH002_1.py b/resources/test/fixtures/pygrep-hooks/PGH002_1.py new file mode 100644 index 0000000000..861e6ed99c --- /dev/null +++ b/resources/test/fixtures/pygrep-hooks/PGH002_1.py @@ -0,0 +1,15 @@ +import logging +from logging import warn + +logging.warn("this is not ok") +log.warn("this is also not ok") +warn("not ok") + + +def foo(): + from logging import warn + + def warn(): + pass + + warn("has been redefined, but we will still report it") diff --git a/resources/test/fixtures/pygrep-hooks/PGH003_0.py b/resources/test/fixtures/pygrep-hooks/PGH003_0.py new file mode 100644 index 0000000000..5c8402c08c --- /dev/null +++ b/resources/test/fixtures/pygrep-hooks/PGH003_0.py @@ -0,0 +1,11 @@ +x = 1 # type: ignore +x = 1 # type ignore +x = 1 # type:ignore + +x = 1 +x = 1 # type ignore # noqa +x = 1 # type: ignore[attr-defined] +x = 1 # type: ignore[attr-defined, name-defined] +x = 1 # type: ignore[type-mismatch] # noqa +x = 1 # type: Union[int, str] +x = 1 # type: ignoreme diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 50d5f22509..6f042cffff 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -1957,7 +1957,10 @@ where // pygrep-hooks if self.settings.enabled.contains(&CheckCode::PGH001) { - pygrep_hooks::checks::no_eval(self, func); + pygrep_hooks::plugins::no_eval(self, func); + } + if self.settings.enabled.contains(&CheckCode::PGH002) { + pygrep_hooks::plugins::deprecated_log_warn(self, func); } // pylint diff --git a/src/checkers/lines.rs b/src/checkers/lines.rs index 3e0c6c08e8..18c929e189 100644 --- a/src/checkers/lines.rs +++ b/src/checkers/lines.rs @@ -2,40 +2,58 @@ 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::pyupgrade::checks::unnecessary_coding_comment; use crate::settings::{flags, Settings}; -pub fn check_lines(contents: &str, settings: &Settings, autofix: flags::Autofix) -> Vec { +pub fn check_lines( + contents: &str, + commented_lines: &[usize], + settings: &Settings, + autofix: flags::Autofix, +) -> Vec { let mut checks: Vec = vec![]; let enforce_unnecessary_coding_comment = settings.enabled.contains(&CheckCode::UP009); 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); - for (lineno, line) in contents.lines().enumerate() { - // Enforce unnecessary coding comments (UP009). - if enforce_unnecessary_coding_comment { - if lineno < 2 { - if let Some(check) = unnecessary_coding_comment( - lineno, - line, - matches!(autofix, flags::Autofix::Enabled) - && settings.fixable.contains(&CheckCode::UP009), - ) { - checks.push(check); + let mut commented_lines_iter = commented_lines.iter().peekable(); + for (index, line) in contents.lines().enumerate() { + while commented_lines_iter + .next_if(|lineno| &(index + 1) == *lineno) + .is_some() + { + if enforce_unnecessary_coding_comment { + if index < 2 { + if let Some(check) = unnecessary_coding_comment( + index, + line, + matches!(autofix, flags::Autofix::Enabled) + && settings.fixable.contains(&CheckCode::UP009), + ) { + checks.push(check); + } + } + } + + if enforce_blanket_type_ignore { + if commented_lines.contains(&(index + 1)) { + if let Some(check) = blanket_type_ignore(index, line) { + checks.push(check); + } } } } - // Enforce line length violations (E501). if enforce_line_too_long { - if let Some(check) = line_too_long(lineno, line, settings.line_length) { + if let Some(check) = line_too_long(index, line, settings.line_length) { checks.push(check); } } } - // Enforce newlines at end of files (W292). if enforce_no_newline_at_end_of_file { if let Some(check) = no_newline_at_end_of_file(contents) { checks.push(check); @@ -58,6 +76,7 @@ mod tests { let check_with_max_line_length = |line_length: usize| { check_lines( line, + &[], &Settings { line_length, ..Settings::for_rule(CheckCode::E501) diff --git a/src/checks.rs b/src/checks.rs index bd7e0f209d..b523c485e1 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -316,6 +316,8 @@ pub enum CheckCode { RUF100, // pygrep-hooks PGH001, + PGH002, + PGH003, // pandas-vet PDV002, PDV003, @@ -887,6 +889,8 @@ pub enum CheckKind { BooleanPositionalValueInFunctionCall, // pygrep-hooks NoEval, + DeprecatedLogWarn, + BlanketTypeIgnore, // flake8-unused-arguments UnusedFunctionArgument(String), UnusedMethodArgument(String), @@ -925,7 +929,9 @@ impl CheckCode { pub fn lint_source(&self) -> &'static LintSource { match self { CheckCode::RUF100 => &LintSource::NoQA, - CheckCode::E501 | CheckCode::W292 | CheckCode::UP009 => &LintSource::Lines, + CheckCode::E501 | CheckCode::W292 | CheckCode::UP009 | CheckCode::PGH003 => { + &LintSource::Lines + } CheckCode::ERA001 | CheckCode::Q000 | CheckCode::Q001 @@ -1260,6 +1266,8 @@ impl CheckCode { CheckCode::FBT003 => CheckKind::BooleanPositionalValueInFunctionCall, // pygrep-hooks CheckCode::PGH001 => CheckKind::NoEval, + CheckCode::PGH002 => CheckKind::DeprecatedLogWarn, + CheckCode::PGH003 => CheckKind::BlanketTypeIgnore, // flake8-unused-arguments CheckCode::ARG001 => CheckKind::UnusedFunctionArgument("...".to_string()), CheckCode::ARG002 => CheckKind::UnusedMethodArgument("...".to_string()), @@ -1504,6 +1512,8 @@ impl CheckCode { CheckCode::PDV015 => CheckCategory::PandasVet, CheckCode::PDV901 => CheckCategory::PandasVet, CheckCode::PGH001 => CheckCategory::PygrepHooks, + CheckCode::PGH002 => CheckCategory::PygrepHooks, + CheckCode::PGH003 => CheckCategory::PygrepHooks, CheckCode::PLC0414 => CheckCategory::Pylint, CheckCode::PLC2201 => CheckCategory::Pylint, CheckCode::PLC3002 => CheckCategory::Pylint, @@ -1847,6 +1857,8 @@ impl CheckKind { CheckKind::BooleanPositionalValueInFunctionCall => &CheckCode::FBT003, // pygrep-hooks CheckKind::NoEval => &CheckCode::PGH001, + CheckKind::DeprecatedLogWarn => &CheckCode::PGH002, + CheckKind::BlanketTypeIgnore => &CheckCode::PGH003, // flake8-unused-arguments CheckKind::UnusedFunctionArgument(..) => &CheckCode::ARG001, CheckKind::UnusedMethodArgument(..) => &CheckCode::ARG002, @@ -2684,6 +2696,12 @@ impl CheckKind { } // pygrep-hooks CheckKind::NoEval => "No builtin `eval()` allowed".to_string(), + CheckKind::DeprecatedLogWarn => { + "`warn` is deprecated in favor of `warning`".to_string() + } + CheckKind::BlanketTypeIgnore => { + "Use specific error codes when ignoring type issues".to_string() + } // flake8-unused-arguments CheckKind::UnusedFunctionArgument(name) => { format!("Unused function argument: `{name}`") diff --git a/src/checks_gen.rs b/src/checks_gen.rs index 6a2a06e59f..9e5a77d871 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -326,6 +326,8 @@ pub enum CheckCodePrefix { PGH0, PGH00, PGH001, + PGH002, + PGH003, PLC, PLC0, PLC04, @@ -1425,10 +1427,12 @@ impl CheckCodePrefix { CheckCodePrefix::PDV9 => vec![CheckCode::PDV901], CheckCodePrefix::PDV90 => vec![CheckCode::PDV901], CheckCodePrefix::PDV901 => vec![CheckCode::PDV901], - CheckCodePrefix::PGH => vec![CheckCode::PGH001], - CheckCodePrefix::PGH0 => vec![CheckCode::PGH001], - CheckCodePrefix::PGH00 => vec![CheckCode::PGH001], + 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::PGH001 => vec![CheckCode::PGH001], + CheckCodePrefix::PGH002 => vec![CheckCode::PGH002], + CheckCodePrefix::PGH003 => vec![CheckCode::PGH003], CheckCodePrefix::PLC => { vec![CheckCode::PLC0414, CheckCode::PLC2201, CheckCode::PLC3002] } @@ -2259,6 +2263,8 @@ impl CheckCodePrefix { CheckCodePrefix::PGH0 => SuffixLength::One, CheckCodePrefix::PGH00 => SuffixLength::Two, CheckCodePrefix::PGH001 => SuffixLength::Three, + CheckCodePrefix::PGH002 => SuffixLength::Three, + CheckCodePrefix::PGH003 => SuffixLength::Three, CheckCodePrefix::PLC => SuffixLength::Zero, CheckCodePrefix::PLC0 => SuffixLength::One, CheckCodePrefix::PLC04 => SuffixLength::Two, diff --git a/src/linter.rs b/src/linter.rs index 940cd727ae..3b7102bf57 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -126,7 +126,12 @@ pub(crate) fn check_path( .iter() .any(|check_code| matches!(check_code.lint_source(), LintSource::Lines)) { - checks.extend(check_lines(contents, settings, autofix)); + checks.extend(check_lines( + contents, + &directives.commented_lines, + settings, + autofix, + )); } // Enforce `noqa` directives. diff --git a/src/pygrep_hooks/mod.rs b/src/pygrep_hooks/mod.rs index 0e63daea8a..50cba4fa8b 100644 --- a/src/pygrep_hooks/mod.rs +++ b/src/pygrep_hooks/mod.rs @@ -1,4 +1,4 @@ -pub mod checks; +pub mod plugins; #[cfg(test)] mod tests { @@ -14,6 +14,9 @@ mod tests { #[test_case(CheckCode::PGH001, Path::new("PGH001_0.py"); "PGH001_0")] #[test_case(CheckCode::PGH001, Path::new("PGH001_1.py"); "PGH001_1")] + #[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")] fn checks(check_code: CheckCode, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy()); let mut checks = test_path( diff --git a/src/pygrep_hooks/plugins/blanket_type_ignore.rs b/src/pygrep_hooks/plugins/blanket_type_ignore.rs new file mode 100644 index 0000000000..7a44c7c4cd --- /dev/null +++ b/src/pygrep_hooks/plugins/blanket_type_ignore.rs @@ -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_TYPE_IGNORE_REGEX: Lazy = + Lazy::new(|| Regex::new(r"# type:? *ignore($|\s)").unwrap()); + +/// PGH003 - use of blanket type ignore comments +pub fn blanket_type_ignore(lineno: usize, line: &str) -> Option { + BLANKET_TYPE_IGNORE_REGEX.find(line).map(|m| { + Check::new( + CheckKind::BlanketTypeIgnore, + Range { + location: Location::new(lineno + 1, m.start()), + end_location: Location::new(lineno + 1, m.end()), + }, + ) + }) +} diff --git a/src/pygrep_hooks/plugins/deprecated_log_warn.rs b/src/pygrep_hooks/plugins/deprecated_log_warn.rs new file mode 100644 index 0000000000..45e86894e8 --- /dev/null +++ b/src/pygrep_hooks/plugins/deprecated_log_warn.rs @@ -0,0 +1,19 @@ +use rustpython_ast::Expr; + +use crate::ast::helpers::{collect_call_paths, dealias_call_path, match_call_path}; +use crate::ast::types::Range; +use crate::checkers::ast::Checker; +use crate::checks::{Check, CheckKind}; + +/// PGH002 - deprecated use of logging.warn +pub fn deprecated_log_warn(checker: &mut Checker, func: &Expr) { + let call_path = dealias_call_path(collect_call_paths(func), &checker.import_aliases); + if call_path == ["log", "warn"] + || match_call_path(&call_path, "logging", "warn", &checker.from_imports) + { + checker.add_check(Check::new( + CheckKind::DeprecatedLogWarn, + Range::from_located(func), + )); + } +} diff --git a/src/pygrep_hooks/plugins/mod.rs b/src/pygrep_hooks/plugins/mod.rs new file mode 100644 index 0000000000..7c43f149db --- /dev/null +++ b/src/pygrep_hooks/plugins/mod.rs @@ -0,0 +1,7 @@ +pub use blanket_type_ignore::blanket_type_ignore; +pub use deprecated_log_warn::deprecated_log_warn; +pub use no_eval::no_eval; + +mod blanket_type_ignore; +mod deprecated_log_warn; +mod no_eval; diff --git a/src/pygrep_hooks/checks.rs b/src/pygrep_hooks/plugins/no_eval.rs similarity index 95% rename from src/pygrep_hooks/checks.rs rename to src/pygrep_hooks/plugins/no_eval.rs index 2e1698e650..e5177e4cd7 100644 --- a/src/pygrep_hooks/checks.rs +++ b/src/pygrep_hooks/plugins/no_eval.rs @@ -4,6 +4,7 @@ use crate::ast::types::Range; use crate::checkers::ast::Checker; use crate::checks::{Check, CheckKind}; +/// PGH001 - no eval pub fn no_eval(checker: &mut Checker, func: &Expr) { let ExprKind::Name { id, .. } = &func.node else { return; diff --git a/src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH002_PGH002_0.py.snap b/src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH002_PGH002_0.py.snap new file mode 100644 index 0000000000..68aa191216 --- /dev/null +++ b/src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH002_PGH002_0.py.snap @@ -0,0 +1,6 @@ +--- +source: src/pygrep_hooks/mod.rs +expression: checks +--- +[] + diff --git a/src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH002_PGH002_1.py.snap b/src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH002_PGH002_1.py.snap new file mode 100644 index 0000000000..33165beaaf --- /dev/null +++ b/src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH002_PGH002_1.py.snap @@ -0,0 +1,37 @@ +--- +source: src/pygrep_hooks/mod.rs +expression: checks +--- +- kind: DeprecatedLogWarn + location: + row: 4 + column: 0 + end_location: + row: 4 + column: 12 + fix: ~ +- kind: DeprecatedLogWarn + location: + row: 5 + column: 0 + end_location: + row: 5 + column: 8 + fix: ~ +- kind: DeprecatedLogWarn + location: + row: 6 + column: 0 + end_location: + row: 6 + column: 4 + fix: ~ +- kind: DeprecatedLogWarn + location: + row: 15 + column: 4 + end_location: + row: 15 + column: 8 + fix: ~ + diff --git a/src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH003_PGH003_0.py.snap b/src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH003_PGH003_0.py.snap new file mode 100644 index 0000000000..99d9d58611 --- /dev/null +++ b/src/pygrep_hooks/snapshots/ruff__pygrep_hooks__tests__PGH003_PGH003_0.py.snap @@ -0,0 +1,29 @@ +--- +source: src/pygrep_hooks/mod.rs +expression: checks +--- +- kind: BlanketTypeIgnore + location: + row: 1 + column: 7 + end_location: + row: 1 + column: 21 + fix: ~ +- kind: BlanketTypeIgnore + location: + row: 2 + column: 7 + end_location: + row: 2 + column: 20 + fix: ~ +- kind: BlanketTypeIgnore + location: + row: 3 + column: 7 + end_location: + row: 3 + column: 20 + fix: ~ +