Add pyupgrade UP041 to replace TimeoutError aliases (#8476)

## Summary

Add UP041 to replace `TimeoutError` aliases:

* Python 3.10+: `socket.timeout`
* Python 3.11+: `asyncio.TimeoutError`

Re:

* https://github.com/asottile/pyupgrade#timeouterror-aliases
*
https://docs.python.org/3/library/asyncio-exceptions.html#asyncio.TimeoutError
* https://docs.python.org/3/library/socket.html#socket.timeout

Based on `os_error_alias.rs`.

## Test Plan

<!-- How was it tested? -->

By running:

```
cargo clippy --workspace --all-targets --all-features -- -D warnings  # Rust linting
RUFF_UPDATE_SCHEMA=1 cargo test  # Rust testing and updating ruff.schema.json
pre-commit run --all-files --show-diff-on-failure  # Rust and Python formatting, Markdown and Python linting, etc.
cargo insta review
```

And also running with different `--target-version` values:

```sh
cargo run -p ruff_cli -- check crates/ruff_linter/resources/test/fixtures/pyupgrade/UP041.py --no-cache --select UP041 --target-version py37 --diff
cargo run -p ruff_cli -- check crates/ruff_linter/resources/test/fixtures/pyupgrade/UP041.py --no-cache --select UP041 --target-version py310 --diff
cargo run -p ruff_cli -- check crates/ruff_linter/resources/test/fixtures/pyupgrade/UP041.py --no-cache --select UP041 --target-version py311 --diff
```
This commit is contained in:
Hugo van Kemenade 2023-11-03 19:24:47 +02:00 committed by GitHub
parent 4982694b54
commit 65effc6666
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 386 additions and 0 deletions

View file

@ -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

View file

@ -466,6 +466,11 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::OSErrorAlias) { if checker.enabled(Rule::OSErrorAlias) {
pyupgrade::rules::os_error_alias_call(checker, func); 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.enabled(Rule::NonPEP604Isinstance) {
if checker.settings.target_version >= PythonVersion::Py310 { if checker.settings.target_version >= PythonVersion::Py310 {
pyupgrade::rules::use_pep604_isinstance(checker, expr, func, args); pyupgrade::rules::use_pep604_isinstance(checker, expr, func, args);

View file

@ -1006,6 +1006,13 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
pyupgrade::rules::os_error_alias_raise(checker, item); 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 checker.enabled(Rule::RaiseVanillaClass) {
if let Some(expr) = exc { if let Some(expr) = exc {
tryceratops::rules::raise_vanilla_class(checker, expr); 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) { if checker.enabled(Rule::OSErrorAlias) {
pyupgrade::rules::os_error_alias_handlers(checker, handlers); 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) { if checker.enabled(Rule::PytestAssertInExcept) {
flake8_pytest_style::rules::assert_in_exception_handler(checker, handlers); flake8_pytest_style::rules::assert_in_exception_handler(checker, handlers);
} }

View file

@ -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, "038") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP604Isinstance),
(Pyupgrade, "039") => (RuleGroup::Stable, rules::pyupgrade::rules::UnnecessaryClassParentheses), (Pyupgrade, "039") => (RuleGroup::Stable, rules::pyupgrade::rules::UnnecessaryClassParentheses),
(Pyupgrade, "040") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP695TypeAlias), (Pyupgrade, "040") => (RuleGroup::Stable, rules::pyupgrade::rules::NonPEP695TypeAlias),
(Pyupgrade, "041") => (RuleGroup::Preview, rules::pyupgrade::rules::TimeoutErrorAlias),
// pydocstyle // pydocstyle
(Pydocstyle, "100") => (RuleGroup::Stable, rules::pydocstyle::rules::UndocumentedPublicModule), (Pydocstyle, "100") => (RuleGroup::Stable, rules::pydocstyle::rules::UndocumentedPublicModule),

View file

@ -60,6 +60,7 @@ mod tests {
#[test_case(Rule::ReplaceStdoutStderr, Path::new("UP022.py"))] #[test_case(Rule::ReplaceStdoutStderr, Path::new("UP022.py"))]
#[test_case(Rule::ReplaceUniversalNewlines, Path::new("UP021.py"))] #[test_case(Rule::ReplaceUniversalNewlines, Path::new("UP021.py"))]
#[test_case(Rule::SuperCallWithParameters, Path::new("UP008.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::TypeOfPrimitive, Path::new("UP003.py"))]
#[test_case(Rule::TypingTextStrAlias, Path::new("UP019.py"))] #[test_case(Rule::TypingTextStrAlias, Path::new("UP019.py"))]
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_0.py"))] #[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_0.py"))]

View file

@ -20,6 +20,7 @@ pub(crate) use redundant_open_modes::*;
pub(crate) use replace_stdout_stderr::*; pub(crate) use replace_stdout_stderr::*;
pub(crate) use replace_universal_newlines::*; pub(crate) use replace_universal_newlines::*;
pub(crate) use super_call_with_parameters::*; pub(crate) use super_call_with_parameters::*;
pub(crate) use timeout_error_alias::*;
pub(crate) use type_of_primitive::*; pub(crate) use type_of_primitive::*;
pub(crate) use typing_text_str_alias::*; pub(crate) use typing_text_str_alias::*;
pub(crate) use unicode_kind_prefix::*; pub(crate) use unicode_kind_prefix::*;
@ -59,6 +60,7 @@ mod redundant_open_modes;
mod replace_stdout_stderr; mod replace_stdout_stderr;
mod replace_universal_newlines; mod replace_universal_newlines;
mod super_call_with_parameters; mod super_call_with_parameters;
mod timeout_error_alias;
mod type_of_primitive; mod type_of_primitive;
mod typing_text_str_alias; mod typing_text_str_alias;
mod unicode_kind_prefix; mod unicode_kind_prefix;

View file

@ -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<String>,
}
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<Expr> = 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);
}
}
}

View file

@ -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

1
ruff.schema.json generated
View file

@ -3536,6 +3536,7 @@
"UP039", "UP039",
"UP04", "UP04",
"UP040", "UP040",
"UP041",
"W", "W",
"W1", "W1",
"W19", "W19",