diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP041.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP041.py new file mode 100644 index 0000000000000..2c30094f59a08 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP041.py @@ -0,0 +1,68 @@ +import asyncio, socket +# These should be fixed +try: + pass +except asyncio.TimeoutError: + pass + +try: + pass +except socket.timeout: + pass + +# Should NOT be in parentheses when replaced + +try: + pass +except (asyncio.TimeoutError,): + pass + +try: + pass +except (socket.timeout,): + pass + +try: + pass +except (asyncio.TimeoutError, socket.timeout,): + pass + +# Should be kept in parentheses (because multiple) + +try: + pass +except (asyncio.TimeoutError, socket.timeout, KeyError, TimeoutError): + pass + +# First should change, second should not + +from .mmap import error +try: + pass +except (asyncio.TimeoutError, error): + pass + +# These should not change + +from foo import error + +try: + pass +except (TimeoutError, error): + pass + +try: + pass +except: + pass + +try: + pass +except TimeoutError: + pass + + +try: + pass +except (TimeoutError, KeyError): + pass diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 62878e61aceaf..aa000bcf5d569 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -466,6 +466,11 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::OSErrorAlias) { pyupgrade::rules::os_error_alias_call(checker, func); } + if checker.enabled(Rule::TimeoutErrorAlias) { + if checker.settings.target_version >= PythonVersion::Py310 { + pyupgrade::rules::timeout_error_alias_call(checker, func); + } + } if checker.enabled(Rule::NonPEP604Isinstance) { if checker.settings.target_version >= PythonVersion::Py310 { pyupgrade::rules::use_pep604_isinstance(checker, expr, func, args); diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 0a567da1c4961..51863307ea29d 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1006,6 +1006,13 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { pyupgrade::rules::os_error_alias_raise(checker, item); } } + if checker.enabled(Rule::TimeoutErrorAlias) { + if checker.settings.target_version >= PythonVersion::Py310 { + if let Some(item) = exc { + pyupgrade::rules::timeout_error_alias_raise(checker, item); + } + } + } if checker.enabled(Rule::RaiseVanillaClass) { if let Some(expr) = exc { tryceratops::rules::raise_vanilla_class(checker, expr); @@ -1304,6 +1311,11 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::OSErrorAlias) { pyupgrade::rules::os_error_alias_handlers(checker, handlers); } + if checker.enabled(Rule::TimeoutErrorAlias) { + if checker.settings.target_version >= PythonVersion::Py310 { + pyupgrade::rules::timeout_error_alias_handlers(checker, handlers); + } + } if checker.enabled(Rule::PytestAssertInExcept) { flake8_pytest_style::rules::assert_in_exception_handler(checker, handlers); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 8df3ad330448e..9e0ceccfd7b9e 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -500,6 +500,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pyupgrade, "038") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP604Isinstance), (Pyupgrade, "039") => (RuleGroup::Stable, rules::pyupgrade::rules::UnnecessaryClassParentheses), (Pyupgrade, "040") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP695TypeAlias), + (Pyupgrade, "041") => (RuleGroup::Preview, rules::pyupgrade::rules::TimeoutErrorAlias), // pydocstyle (Pydocstyle, "100") => (RuleGroup::Stable, rules::pydocstyle::rules::UndocumentedPublicModule), diff --git a/crates/ruff_linter/src/rules/pyupgrade/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/mod.rs index d9ba5d890a2ba..987e82608e03b 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/mod.rs @@ -60,6 +60,7 @@ mod tests { #[test_case(Rule::ReplaceStdoutStderr, Path::new("UP022.py"))] #[test_case(Rule::ReplaceUniversalNewlines, Path::new("UP021.py"))] #[test_case(Rule::SuperCallWithParameters, Path::new("UP008.py"))] + #[test_case(Rule::TimeoutErrorAlias, Path::new("UP041.py"))] #[test_case(Rule::TypeOfPrimitive, Path::new("UP003.py"))] #[test_case(Rule::TypingTextStrAlias, Path::new("UP019.py"))] #[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_0.py"))] diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs index 0a924c145120d..aa6a60732fbed 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/mod.rs @@ -20,6 +20,7 @@ pub(crate) use redundant_open_modes::*; pub(crate) use replace_stdout_stderr::*; pub(crate) use replace_universal_newlines::*; pub(crate) use super_call_with_parameters::*; +pub(crate) use timeout_error_alias::*; pub(crate) use type_of_primitive::*; pub(crate) use typing_text_str_alias::*; pub(crate) use unicode_kind_prefix::*; @@ -59,6 +60,7 @@ mod redundant_open_modes; mod replace_stdout_stderr; mod replace_universal_newlines; mod super_call_with_parameters; +mod timeout_error_alias; mod type_of_primitive; mod typing_text_str_alias; mod unicode_kind_prefix; diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs new file mode 100644 index 0000000000000..1bb999f96d651 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs @@ -0,0 +1,192 @@ +use ruff_python_ast::{self as ast, ExceptHandler, Expr, ExprContext}; +use ruff_text_size::{Ranged, TextRange}; + +use crate::fix::edits::pad; +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::call_path::compose_call_path; +use ruff_python_semantic::SemanticModel; + +use crate::checkers::ast::Checker; +use crate::settings::types::PythonVersion; + +/// ## What it does +/// Checks for uses of exceptions that alias `TimeoutError`. +/// +/// ## Why is this bad? +/// `TimeoutError` is the builtin error type used for exceptions when a system +/// function timed out at the system level. +/// +/// In Python 3.10, `socket.timeout` was aliased to `TimeoutError`. In Python +/// 3.11, `asyncio.TimeoutError` was aliased to `TimeoutError`. +/// +/// These aliases remain in place for compatibility with older versions of +/// Python, but may be removed in future versions. +/// +/// Prefer using `TimeoutError` directly, as it is more idiomatic and future-proof. +/// +/// ## Example +/// ```python +/// raise asyncio.TimeoutError +/// ``` +/// +/// Use instead: +/// ```python +/// raise TimeoutError +/// ``` +/// +/// ## References +/// - [Python documentation: `TimeoutError`](https://docs.python.org/3/library/exceptions.html#TimeoutError) +#[violation] +pub struct TimeoutErrorAlias { + name: Option, +} + +impl AlwaysFixableViolation for TimeoutErrorAlias { + #[derive_message_formats] + fn message(&self) -> String { + format!("Replace aliased errors with `TimeoutError`") + } + + fn fix_title(&self) -> String { + let TimeoutErrorAlias { name } = self; + match name { + None => "Replace with builtin `TimeoutError`".to_string(), + Some(name) => format!("Replace `{name}` with builtin `TimeoutError`"), + } + } +} + +/// Return `true` if an [`Expr`] is an alias of `TimeoutError`. +fn is_alias(expr: &Expr, semantic: &SemanticModel, target_version: PythonVersion) -> bool { + semantic.resolve_call_path(expr).is_some_and(|call_path| { + if target_version >= PythonVersion::Py311 { + matches!(call_path.as_slice(), [""] | ["asyncio", "TimeoutError"]) + } else { + matches!( + call_path.as_slice(), + [""] | ["asyncio", "TimeoutError"] | ["socket", "timeout"] + ) + } + }) +} + +/// Return `true` if an [`Expr`] is `TimeoutError`. +fn is_timeout_error(expr: &Expr, semantic: &SemanticModel) -> bool { + semantic + .resolve_call_path(expr) + .is_some_and(|call_path| matches!(call_path.as_slice(), ["", "TimeoutError"])) +} + +/// Create a [`Diagnostic`] for a single target, like an [`Expr::Name`]. +fn atom_diagnostic(checker: &mut Checker, target: &Expr) { + let mut diagnostic = Diagnostic::new( + TimeoutErrorAlias { + name: compose_call_path(target), + }, + target.range(), + ); + if checker.semantic().is_builtin("TimeoutError") { + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + "TimeoutError".to_string(), + target.range(), + ))); + } + checker.diagnostics.push(diagnostic); +} + +/// Create a [`Diagnostic`] for a tuple of expressions. +fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&Expr]) { + let mut diagnostic = Diagnostic::new(TimeoutErrorAlias { name: None }, tuple.range()); + if checker.semantic().is_builtin("TimeoutError") { + // Filter out any `TimeoutErrors` aliases. + let mut remaining: Vec = tuple + .elts + .iter() + .filter_map(|elt| { + if aliases.contains(&elt) { + None + } else { + Some(elt.clone()) + } + }) + .collect(); + + // If `TimeoutError` itself isn't already in the tuple, add it. + if tuple + .elts + .iter() + .all(|elt| !is_timeout_error(elt, checker.semantic())) + { + let node = ast::ExprName { + id: "TimeoutError".into(), + ctx: ExprContext::Load, + range: TextRange::default(), + }; + remaining.insert(0, node.into()); + } + + let content = if remaining.len() == 1 { + "TimeoutError".to_string() + } else { + let node = ast::ExprTuple { + elts: remaining, + ctx: ExprContext::Load, + range: TextRange::default(), + }; + format!("({})", checker.generator().expr(&node.into())) + }; + + diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( + pad(content, tuple.range(), checker.locator()), + tuple.range(), + ))); + } + checker.diagnostics.push(diagnostic); +} + +/// UP041 +pub(crate) fn timeout_error_alias_handlers(checker: &mut Checker, handlers: &[ExceptHandler]) { + for handler in handlers { + let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { type_, .. }) = handler; + let Some(expr) = type_.as_ref() else { + continue; + }; + match expr.as_ref() { + Expr::Name(_) | Expr::Attribute(_) => { + if is_alias(expr, checker.semantic(), checker.settings.target_version) { + atom_diagnostic(checker, expr); + } + } + Expr::Tuple(tuple) => { + // List of aliases to replace with `TimeoutError`. + let mut aliases: Vec<&Expr> = vec![]; + for elt in &tuple.elts { + if is_alias(elt, checker.semantic(), checker.settings.target_version) { + aliases.push(elt); + } + } + if !aliases.is_empty() { + tuple_diagnostic(checker, tuple, &aliases); + } + } + _ => {} + } + } +} + +/// UP041 +pub(crate) fn timeout_error_alias_call(checker: &mut Checker, func: &Expr) { + if is_alias(func, checker.semantic(), checker.settings.target_version) { + atom_diagnostic(checker, func); + } +} + +/// UP041 +pub(crate) fn timeout_error_alias_raise(checker: &mut Checker, expr: &Expr) { + if matches!(expr, Expr::Name(_) | Expr::Attribute(_)) { + if is_alias(expr, checker.semantic(), checker.settings.target_version) { + atom_diagnostic(checker, expr); + } + } +} diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP041.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP041.py.snap new file mode 100644 index 0000000000000..9c288554d26d2 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP041.py.snap @@ -0,0 +1,104 @@ +--- +source: crates/ruff_linter/src/rules/pyupgrade/mod.rs +--- +UP041.py:5:8: UP041 [*] Replace aliased errors with `TimeoutError` + | +3 | try: +4 | pass +5 | except asyncio.TimeoutError: + | ^^^^^^^^^^^^^^^^^^^^ UP041 +6 | pass + | + = help: Replace `asyncio.TimeoutError` with builtin `TimeoutError` + +ℹ Fix +2 2 | # These should be fixed +3 3 | try: +4 4 | pass +5 |-except asyncio.TimeoutError: + 5 |+except TimeoutError: +6 6 | pass +7 7 | +8 8 | try: + +UP041.py:17:8: UP041 [*] Replace aliased errors with `TimeoutError` + | +15 | try: +16 | pass +17 | except (asyncio.TimeoutError,): + | ^^^^^^^^^^^^^^^^^^^^^^^ UP041 +18 | pass + | + = help: Replace with builtin `TimeoutError` + +ℹ Fix +14 14 | +15 15 | try: +16 16 | pass +17 |-except (asyncio.TimeoutError,): + 17 |+except TimeoutError: +18 18 | pass +19 19 | +20 20 | try: + +UP041.py:27:8: UP041 [*] Replace aliased errors with `TimeoutError` + | +25 | try: +26 | pass +27 | except (asyncio.TimeoutError, socket.timeout,): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP041 +28 | pass + | + = help: Replace with builtin `TimeoutError` + +ℹ Fix +24 24 | +25 25 | try: +26 26 | pass +27 |-except (asyncio.TimeoutError, socket.timeout,): + 27 |+except (TimeoutError, socket.timeout): +28 28 | pass +29 29 | +30 30 | # Should be kept in parentheses (because multiple) + +UP041.py:34:8: UP041 [*] Replace aliased errors with `TimeoutError` + | +32 | try: +33 | pass +34 | except (asyncio.TimeoutError, socket.timeout, KeyError, TimeoutError): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP041 +35 | pass + | + = help: Replace with builtin `TimeoutError` + +ℹ Fix +31 31 | +32 32 | try: +33 33 | pass +34 |-except (asyncio.TimeoutError, socket.timeout, KeyError, TimeoutError): + 34 |+except (socket.timeout, KeyError, TimeoutError): +35 35 | pass +36 36 | +37 37 | # First should change, second should not + +UP041.py:42:8: UP041 [*] Replace aliased errors with `TimeoutError` + | +40 | try: +41 | pass +42 | except (asyncio.TimeoutError, error): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ UP041 +43 | pass + | + = help: Replace with builtin `TimeoutError` + +ℹ Fix +39 39 | from .mmap import error +40 40 | try: +41 41 | pass +42 |-except (asyncio.TimeoutError, error): + 42 |+except (TimeoutError, error): +43 43 | pass +44 44 | +45 45 | # These should not change + + diff --git a/ruff.schema.json b/ruff.schema.json index b861d4bf2457f..1886b55e838d2 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3536,6 +3536,7 @@ "UP039", "UP04", "UP040", + "UP041", "W", "W1", "W19",