Skip to content

Commit

Permalink
refine pyupgrade's TimeoutErrorAlias lint (UP041) to remove false pos…
Browse files Browse the repository at this point in the history 
…itives (#8587)

Previously, this lint had its alias detection logic a little
backwards. That is, for Python 3.11+, it would *only* detect
asyncio.TimeoutError as an alias, but it should have also detected
socket.timeout as an alias. And in Python <3.11, it would falsely
detect asyncio.TimeoutError as an alias where it should have only
detected socket.timeout as an alias.

We fix it so that both asyncio.TimeoutError and socket.timeout are
detected as aliases in Python 3.11+, and only socket.timeout is
detected as an alias in Python 3.10.

Fixes #8565

## Test Plan

I tested this by updating the existing snapshot test which had
erroneously
asserted that socket.timeout should not be replaced with TimeoutError in
Python
3.11+. I also added a new regression test that targets Python 3.10 and
ensures
that the suggestion to replace asyncio.TimeoutError with TimeoutError
does not
occur.
  • Loading branch information
@BurntSushi
BurntSushi authored Nov 10, 2023
1 parent 036b6bc commit a7dbe9d
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 5 deletions.
13 changes: 13 additions & 0 deletions crates/ruff_linter/src/rules/pyupgrade/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,19 @@ mod tests {
Ok(())
}

#[test]
fn async_timeout_error_alias_not_applied_py310() -> Result<()> {
let diagnostics = test_path(
Path::new("pyupgrade/UP041.py"),
&settings::LinterSettings {
target_version: PythonVersion::Py310,
..settings::LinterSettings::for_rule(Rule::TimeoutErrorAlias)
},
)?;
assert_messages!(diagnostics);
Ok(())
}

#[test]
fn non_pep695_type_alias_not_applied_py311() -> Result<()> {
let diagnostics = test_path(
Expand Down
14 changes: 11 additions & 3 deletions crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,20 @@ impl AlwaysFixableViolation for TimeoutErrorAlias {
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"]
["socket", "timeout"] | ["asyncio", "TimeoutError"]
)
} else {
// N.B. This lint is only invoked for Python 3.10+. We assume
// as much here since otherwise socket.timeout would be an unsafe
// fix in Python <3.10. We add an assert to make this assumption
// explicit.
assert!(
target_version >= PythonVersion::Py310,
"lint should only be used for Python 3.10+",
);
matches!(call_path.as_slice(), ["socket", "timeout"])
}
})
}
Expand Down
44 changes: 42 additions & 2 deletions ..._linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP041.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,26 @@ UP041.py:5:8: UP041 [*] Replace aliased errors with `TimeoutError`
7 7 |
8 8 | try:

UP041.py:10:8: UP041 [*] Replace aliased errors with `TimeoutError`
|
8 | try:
9 | pass
10 | except socket.timeout:
| ^^^^^^^^^^^^^^ UP041
11 | pass
|
= help: Replace `socket.timeout` with builtin `TimeoutError`

ℹ Safe fix
7 7 |
8 8 | try:
9 9 | pass
10 |-except socket.timeout:
10 |+except TimeoutError:
11 11 | pass
12 12 |
13 13 | # Should NOT be in parentheses when replaced

UP041.py:17:8: UP041 [*] Replace aliased errors with `TimeoutError`
|
15 | try:
Expand All @@ -41,6 +61,26 @@ UP041.py:17:8: UP041 [*] Replace aliased errors with `TimeoutError`
19 19 |
20 20 | try:

UP041.py:22:8: UP041 [*] Replace aliased errors with `TimeoutError`
|
20 | try:
21 | pass
22 | except (socket.timeout,):
| ^^^^^^^^^^^^^^^^^ UP041
23 | pass
|
= help: Replace with builtin `TimeoutError`

ℹ Safe fix
19 19 |
20 20 | try:
21 21 | pass
22 |-except (socket.timeout,):
22 |+except TimeoutError:
23 23 | pass
24 24 |
25 25 | try:

UP041.py:27:8: UP041 [*] Replace aliased errors with `TimeoutError`
|
25 | try:
Expand All @@ -56,7 +96,7 @@ UP041.py:27:8: UP041 [*] Replace aliased errors with `TimeoutError`
25 25 | try:
26 26 | pass
27 |-except (asyncio.TimeoutError, socket.timeout,):
27 |+except (TimeoutError, socket.timeout):
27 |+except TimeoutError:
28 28 | pass
29 29 |
30 30 | # Should be kept in parentheses (because multiple)
Expand All @@ -76,7 +116,7 @@ UP041.py:34:8: UP041 [*] Replace aliased errors with `TimeoutError`
32 32 | try:
33 33 | pass
34 |-except (asyncio.TimeoutError, socket.timeout, KeyError, TimeoutError):
34 |+except (socket.timeout, KeyError, TimeoutError):
34 |+except (KeyError, TimeoutError):
35 35 | pass
36 36 |
37 37 | # First should change, second should not
Expand Down
84 changes: 84 additions & 0 deletions ...ts/ruff_linter__rules__pyupgrade__tests__async_timeout_error_alias_not_applied_py310.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
---
source: crates/ruff_linter/src/rules/pyupgrade/mod.rs
---
UP041.py:10:8: UP041 [*] Replace aliased errors with `TimeoutError`
|
8 | try:
9 | pass
10 | except socket.timeout:
| ^^^^^^^^^^^^^^ UP041
11 | pass
|
= help: Replace `socket.timeout` with builtin `TimeoutError`

ℹ Safe fix
7 7 |
8 8 | try:
9 9 | pass
10 |-except socket.timeout:
10 |+except TimeoutError:
11 11 | pass
12 12 |
13 13 | # Should NOT be in parentheses when replaced

UP041.py:22:8: UP041 [*] Replace aliased errors with `TimeoutError`
|
20 | try:
21 | pass
22 | except (socket.timeout,):
| ^^^^^^^^^^^^^^^^^ UP041
23 | pass
|
= help: Replace with builtin `TimeoutError`

ℹ Safe fix
19 19 |
20 20 | try:
21 21 | pass
22 |-except (socket.timeout,):
22 |+except TimeoutError:
23 23 | pass
24 24 |
25 25 | 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`

ℹ Safe fix
24 24 |
25 25 | try:
26 26 | pass
27 |-except (asyncio.TimeoutError, socket.timeout,):
27 |+except (TimeoutError, asyncio.TimeoutError):
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`

ℹ Safe fix
31 31 |
32 32 | try:
33 33 | pass
34 |-except (asyncio.TimeoutError, socket.timeout, KeyError, TimeoutError):
34 |+except (asyncio.TimeoutError, KeyError, TimeoutError):
35 35 | pass
36 36 |
37 37 | # First should change, second should not


0 comments on commit a7dbe9d

Please sign in to comment.