diff --git a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH001_0.py b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH001_0.py deleted file mode 100644 index eed83b81f987c..0000000000000 --- a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH001_0.py +++ /dev/null @@ -1,9 +0,0 @@ -from ast import literal_eval - -eval("3 + 4") - -literal_eval({1: 2}) - - -def fn() -> None: - eval("3 + 4") diff --git a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH001_1.py b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH001_1.py deleted file mode 100644 index ecb3e91a3a5d5..0000000000000 --- a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH001_1.py +++ /dev/null @@ -1,11 +0,0 @@ -def eval(content: str) -> None: - pass - - -eval("3 + 4") - -literal_eval({1: 2}) - - -def fn() -> None: - eval("3 + 4") diff --git a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH002_0.py b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH002_0.py deleted file mode 100644 index d1bc313fc821d..0000000000000 --- a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH002_0.py +++ /dev/null @@ -1,10 +0,0 @@ -import logging -import warnings -from warnings import warn - -warnings.warn("this is ok") -warn("by itself is also ok") -logging.warning("this is fine") - -logger = logging.getLogger(__name__) -logger.warning("this is fine") diff --git a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH002_1.py b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH002_1.py deleted file mode 100644 index 8ff160407c6fa..0000000000000 --- a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH002_1.py +++ /dev/null @@ -1,8 +0,0 @@ -import logging -from logging import warn - -logging.warn("this is not ok") -warn("not ok") - -logger = logging.getLogger(__name__) -logger.warn("this is not ok") diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index ba1bea34f1a8c..f02fe6670a40a 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -16,8 +16,7 @@ use crate::rules::{ flake8_future_annotations, flake8_gettext, flake8_implicit_str_concat, flake8_logging, flake8_logging_format, flake8_pie, flake8_print, flake8_pyi, flake8_pytest_style, flake8_self, flake8_simplify, flake8_tidy_imports, flake8_trio, flake8_type_checking, flake8_use_pathlib, - flynt, numpy, pandas_vet, pep8_naming, pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, - refurb, ruff, + flynt, numpy, pandas_vet, pep8_naming, pycodestyle, pyflakes, pylint, pyupgrade, refurb, ruff, }; use crate::settings::types::PythonVersion; @@ -773,12 +772,6 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::CallDateFromtimestamp) { flake8_datetimez::rules::call_date_fromtimestamp(checker, func, expr.range()); } - if checker.enabled(Rule::Eval) { - pygrep_hooks::rules::no_eval(checker, func); - } - if checker.enabled(Rule::DeprecatedLogWarn) { - pygrep_hooks::rules::deprecated_log_warn(checker, call); - } if checker.enabled(Rule::UnnecessaryDirectLambdaCall) { pylint::rules::unnecessary_direct_lambda_call(checker, expr, func); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 19da7d8aef40d..92b91e11e9a0b 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -705,8 +705,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Datetimez, "012") => (RuleGroup::Stable, rules::flake8_datetimez::rules::CallDateFromtimestamp), // pygrep-hooks - (PygrepHooks, "001") => (RuleGroup::Stable, rules::pygrep_hooks::rules::Eval), - (PygrepHooks, "002") => (RuleGroup::Stable, rules::pygrep_hooks::rules::DeprecatedLogWarn), + (PygrepHooks, "001") => (RuleGroup::Removed, rules::pygrep_hooks::rules::Eval), + (PygrepHooks, "002") => (RuleGroup::Removed, rules::pygrep_hooks::rules::DeprecatedLogWarn), (PygrepHooks, "003") => (RuleGroup::Stable, rules::pygrep_hooks::rules::BlanketTypeIgnore), (PygrepHooks, "004") => (RuleGroup::Stable, rules::pygrep_hooks::rules::BlanketNOQA), (PygrepHooks, "005") => (RuleGroup::Stable, rules::pygrep_hooks::rules::InvalidMockAccess), diff --git a/crates/ruff_linter/src/rule_redirects.rs b/crates/ruff_linter/src/rule_redirects.rs index 8cd7cb401dcf9..afd8ac39580cc 100644 --- a/crates/ruff_linter/src/rule_redirects.rs +++ b/crates/ruff_linter/src/rule_redirects.rs @@ -101,6 +101,8 @@ static REDIRECTS: Lazy> = Lazy::new(|| { ("RUF011", "B035"), ("TCH006", "TCH010"), ("TRY200", "B904"), + ("PGH001", "S307"), + ("PHG002", "G010"), // Test redirect by exact code #[cfg(feature = "test-rules")] ("RUF940", "RUF950"), diff --git a/crates/ruff_linter/src/rule_selector.rs b/crates/ruff_linter/src/rule_selector.rs index b7e0745d903bc..a6486e0b302f5 100644 --- a/crates/ruff_linter/src/rule_selector.rs +++ b/crates/ruff_linter/src/rule_selector.rs @@ -48,6 +48,7 @@ impl FromStr for RuleSelector { type Err = ParseError; fn from_str(s: &str) -> Result { + // **Changes should be reflected in `parse_no_redirect` as well** match s { "ALL" => Ok(Self::All), #[allow(deprecated)] @@ -67,7 +68,6 @@ impl FromStr for RuleSelector { return Ok(Self::Linter(linter)); } - // Does the selector select a single rule? let prefix = RuleCodePrefix::parse(&linter, code) .map_err(|_| ParseError::Unknown(s.to_string()))?; @@ -254,8 +254,6 @@ pub struct PreviewOptions { #[cfg(feature = "schemars")] mod schema { - use std::str::FromStr; - use itertools::Itertools; use schemars::JsonSchema; use schemars::_serde_json::Value; @@ -302,7 +300,7 @@ mod schema { .filter(|p| { // Exclude any prefixes where all of the rules are removed if let Ok(Self::Rule { prefix, .. } | Self::Prefix { prefix, .. }) = - RuleSelector::from_str(p) + RuleSelector::parse_no_redirect(p) { !prefix.rules().all(|rule| rule.is_removed()) } else { @@ -341,6 +339,41 @@ impl RuleSelector { } } } + + /// Parse [`RuleSelector`] from a string; but do not follow redirects. + pub fn parse_no_redirect(s: &str) -> Result { + // **Changes should be reflected in `from_str` as well** + match s { + "ALL" => Ok(Self::All), + #[allow(deprecated)] + "NURSERY" => Ok(Self::Nursery), + "C" => Ok(Self::C), + "T" => Ok(Self::T), + _ => { + let (linter, code) = + Linter::parse_code(s).ok_or_else(|| ParseError::Unknown(s.to_string()))?; + + if code.is_empty() { + return Ok(Self::Linter(linter)); + } + + let prefix = RuleCodePrefix::parse(&linter, code) + .map_err(|_| ParseError::Unknown(s.to_string()))?; + + if is_single_rule_selector(&prefix) { + Ok(Self::Rule { + prefix, + redirected_from: None, + }) + } else { + Ok(Self::Prefix { + prefix, + redirected_from: None, + }) + } + } + } + } } #[derive(EnumIter, PartialEq, Eq, PartialOrd, Ord, Copy, Clone, Debug)] diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs b/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs index 086f9fddd7f83..8cce061b151c3 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs +++ b/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs @@ -13,10 +13,6 @@ mod tests { use crate::test::test_path; use crate::{assert_messages, settings}; - #[test_case(Rule::Eval, Path::new("PGH001_0.py"))] - #[test_case(Rule::Eval, Path::new("PGH001_1.py"))] - #[test_case(Rule::DeprecatedLogWarn, Path::new("PGH002_0.py"))] - #[test_case(Rule::DeprecatedLogWarn, Path::new("PGH002_1.py"))] #[test_case(Rule::BlanketTypeIgnore, Path::new("PGH003_0.py"))] #[test_case(Rule::BlanketTypeIgnore, Path::new("PGH003_1.py"))] #[test_case(Rule::BlanketNOQA, Path::new("PGH004_0.py"))] diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/rules/deprecated_log_warn.rs b/crates/ruff_linter/src/rules/pygrep_hooks/rules/deprecated_log_warn.rs index 3408a37012cca..ec9754414456b 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/rules/deprecated_log_warn.rs +++ b/crates/ruff_linter/src/rules/pygrep_hooks/rules/deprecated_log_warn.rs @@ -1,13 +1,9 @@ -use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_diagnostics::{FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::{self as ast, Expr}; -use ruff_python_semantic::analyze::logging; -use ruff_python_stdlib::logging::LoggingLevel; -use ruff_text_size::Ranged; - -use crate::checkers::ast::Checker; -use crate::importer::ImportRequest; +/// ## Removed +/// This rule is identical to [G010] which should be used instead. +/// /// ## What it does /// Check for usages of the deprecated `warn` method from the `logging` module. /// @@ -34,9 +30,12 @@ use crate::importer::ImportRequest; /// /// ## References /// - [Python documentation: `logger.Logger.warning`](https://docs.python.org/3/library/logging.html#logging.Logger.warning) +/// +/// [G010]: https://docs.astral.sh/ruff/rules/logging-warn/ #[violation] pub struct DeprecatedLogWarn; +/// PGH002 impl Violation for DeprecatedLogWarn { const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; @@ -49,59 +48,3 @@ impl Violation for DeprecatedLogWarn { Some(format!("Replace with `warning`")) } } - -/// PGH002 -pub(crate) fn deprecated_log_warn(checker: &mut Checker, call: &ast::ExprCall) { - match call.func.as_ref() { - Expr::Attribute(ast::ExprAttribute { attr, .. }) => { - if !logging::is_logger_candidate( - &call.func, - checker.semantic(), - &checker.settings.logger_objects, - ) { - return; - } - if !matches!( - LoggingLevel::from_attribute(attr.as_str()), - Some(LoggingLevel::Warn) - ) { - return; - } - } - Expr::Name(_) => { - if !checker - .semantic() - .resolve_call_path(call.func.as_ref()) - .is_some_and(|call_path| matches!(call_path.as_slice(), ["logging", "warn"])) - { - return; - } - } - _ => return, - } - - let mut diagnostic = Diagnostic::new(DeprecatedLogWarn, call.func.range()); - - match call.func.as_ref() { - Expr::Attribute(ast::ExprAttribute { attr, .. }) => { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - "warning".to_string(), - attr.range(), - ))); - } - Expr::Name(_) => { - diagnostic.try_set_fix(|| { - let (import_edit, binding) = checker.importer().get_or_import_symbol( - &ImportRequest::import("logging", "warning"), - call.start(), - checker.semantic(), - )?; - let name_edit = Edit::range_replacement(binding, call.func.range()); - Ok(Fix::safe_edits(import_edit, [name_edit])) - }); - } - _ => {} - } - - checker.diagnostics.push(diagnostic); -} diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/rules/no_eval.rs b/crates/ruff_linter/src/rules/pygrep_hooks/rules/no_eval.rs index a9d49f7689fcc..4acfd491214b0 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/rules/no_eval.rs +++ b/crates/ruff_linter/src/rules/pygrep_hooks/rules/no_eval.rs @@ -1,11 +1,9 @@ -use ruff_python_ast::{self as ast, Expr}; - -use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_diagnostics::Violation; use ruff_macros::{derive_message_formats, violation}; -use ruff_text_size::Ranged; - -use crate::checkers::ast::Checker; +/// ## Removed +/// This rule is identical to [S307] which should be used instead. +/// /// ## What it does /// Checks for uses of the builtin `eval()` function. /// @@ -29,28 +27,15 @@ use crate::checkers::ast::Checker; /// ## References /// - [Python documentation: `eval`](https://docs.python.org/3/library/functions.html#eval) /// - [_Eval really is dangerous_ by Ned Batchelder](https://nedbatchelder.com/blog/201206/eval_really_is_dangerous.html) +/// +/// [S307]: https://docs.astral.sh/ruff/rules/suspicious-eval-usage/ #[violation] pub struct Eval; +/// PGH001 impl Violation for Eval { #[derive_message_formats] fn message(&self) -> String { format!("No builtin `eval()` allowed") } } - -/// PGH001 -pub(crate) fn no_eval(checker: &mut Checker, func: &Expr) { - let Expr::Name(ast::ExprName { id, .. }) = func else { - return; - }; - if id != "eval" { - return; - } - if !checker.semantic().is_builtin("eval") { - return; - } - checker - .diagnostics - .push(Diagnostic::new(Eval, func.range())); -} diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH001_PGH001_0.py.snap b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH001_PGH001_0.py.snap deleted file mode 100644 index 8dad4d98ac34e..0000000000000 --- a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH001_PGH001_0.py.snap +++ /dev/null @@ -1,21 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/pygrep_hooks/mod.rs ---- -PGH001_0.py:3:1: PGH001 No builtin `eval()` allowed - | -1 | from ast import literal_eval -2 | -3 | eval("3 + 4") - | ^^^^ PGH001 -4 | -5 | literal_eval({1: 2}) - | - -PGH001_0.py:9:5: PGH001 No builtin `eval()` allowed - | -8 | def fn() -> None: -9 | eval("3 + 4") - | ^^^^ PGH001 - | - - diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH001_PGH001_1.py.snap b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH001_PGH001_1.py.snap deleted file mode 100644 index d5e81ab92031e..0000000000000 --- a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH001_PGH001_1.py.snap +++ /dev/null @@ -1,4 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/pygrep_hooks/mod.rs ---- - diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH002_PGH002_0.py.snap b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH002_PGH002_0.py.snap deleted file mode 100644 index d5e81ab92031e..0000000000000 --- a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH002_PGH002_0.py.snap +++ /dev/null @@ -1,4 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/pygrep_hooks/mod.rs ---- - diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH002_PGH002_1.py.snap b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH002_PGH002_1.py.snap deleted file mode 100644 index 6c1c5f1f712be..0000000000000 --- a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH002_PGH002_1.py.snap +++ /dev/null @@ -1,59 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/pygrep_hooks/mod.rs ---- -PGH002_1.py:4:1: PGH002 [*] `warn` is deprecated in favor of `warning` - | -2 | from logging import warn -3 | -4 | logging.warn("this is not ok") - | ^^^^^^^^^^^^ PGH002 -5 | warn("not ok") - | - = help: Replace with `warning` - -ℹ Safe fix -1 1 | import logging -2 2 | from logging import warn -3 3 | -4 |-logging.warn("this is not ok") - 4 |+logging.warning("this is not ok") -5 5 | warn("not ok") -6 6 | -7 7 | logger = logging.getLogger(__name__) - -PGH002_1.py:5:1: PGH002 [*] `warn` is deprecated in favor of `warning` - | -4 | logging.warn("this is not ok") -5 | warn("not ok") - | ^^^^ PGH002 -6 | -7 | logger = logging.getLogger(__name__) - | - = help: Replace with `warning` - -ℹ Safe fix -2 2 | from logging import warn -3 3 | -4 4 | logging.warn("this is not ok") -5 |-warn("not ok") - 5 |+logging.warning("not ok") -6 6 | -7 7 | logger = logging.getLogger(__name__) -8 8 | logger.warn("this is not ok") - -PGH002_1.py:8:1: PGH002 [*] `warn` is deprecated in favor of `warning` - | -7 | logger = logging.getLogger(__name__) -8 | logger.warn("this is not ok") - | ^^^^^^^^^^^ PGH002 - | - = help: Replace with `warning` - -ℹ Safe fix -5 5 | warn("not ok") -6 6 | -7 7 | logger = logging.getLogger(__name__) -8 |-logger.warn("this is not ok") - 8 |+logger.warning("this is not ok") - - diff --git a/ruff.schema.json b/ruff.schema.json index 2d73566f2db20..7e1d8a62d017c 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3120,8 +3120,6 @@ "PGH", "PGH0", "PGH00", - "PGH001", - "PGH002", "PGH003", "PGH004", "PGH005", @@ -3722,7 +3720,6 @@ "TRY004", "TRY2", "TRY20", - "TRY200", "TRY201", "TRY3", "TRY30",