Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refine pyupgrade's TimeoutErrorAlias lint (UP041) to remove false positives #8587

Merged
merged 3 commits into from
Nov 10, 2023
Merged

refine pyupgrade's TimeoutErrorAlias lint (UP041) to remove false positives #8587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0c4ded6
ruff_linter: tweak detection logic for TimeoutErrorAlias
BurntSushi Nov 9, 2023
4ccbb4f
ruff_linter: assert that TimeoutErrorAlias only runs on 3.10+
BurntSushi Nov 9, 2023
1e45679
ruff_linter: add asyncio.TimeoutError regression test for Python 3.10+
BurntSushi Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions crates/ruff_linter/src/rules/pyupgrade/mod.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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


Loading