Skip to content

Commit 11dae2c

Browse files
IDrokin117Igor Drokin
andauthored
[pyupgrade] Prevent infinite loop with I002 and UP026 (#20634)
## Summary Closes #20601 Do not treat imports as unused for the rule [unnecessary-builtin-import (UP029)](https://docs.astral.sh/ruff/rules/unnecessary-builtin-import/) if they are required by `isort`([missing-required-import](https://docs.astral.sh/ruff/rules/missing-required-import/)) ## Test Plan - Added test case `i002_up029_conflict` to ensure there is no conflict Co-authored-by: Igor Drokin <drokinii1017@gmail.com>
1 parent 7fee877 commit 11dae2c

6 files changed

Lines changed: 53 additions & 5 deletions

File tree

crates/ruff_linter/resources/test/fixtures/pyupgrade/UP029.py renamed to crates/ruff_linter/resources/test/fixtures/pyupgrade/UP029_0.py

File renamed without changes.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
from builtins import str, int

crates/ruff_linter/src/rules/pyupgrade/mod.rs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ mod tests {
9797
)]
9898
#[test_case(Rule::UTF8EncodingDeclaration, Path::new("UP009_many_empty_lines.py"))]
9999
#[test_case(Rule::UnicodeKindPrefix, Path::new("UP025.py"))]
100-
#[test_case(Rule::UnnecessaryBuiltinImport, Path::new("UP029.py"))]
100+
#[test_case(Rule::UnnecessaryBuiltinImport, Path::new("UP029_0.py"))]
101101
#[test_case(Rule::UnnecessaryClassParentheses, Path::new("UP039.py"))]
102102
#[test_case(Rule::UnnecessaryDefaultTypeArgs, Path::new("UP043.py"))]
103103
#[test_case(Rule::UnnecessaryEncodeUTF8, Path::new("UP012.py"))]
@@ -357,6 +357,32 @@ mod tests {
357357
");
358358
}
359359

360+
#[test_case(Path::new("UP029_1.py"))]
361+
fn i002_up029_conflict(path: &Path) -> Result<()> {
362+
let snapshot = format!("{}_skip_required_imports", path.to_string_lossy());
363+
let diagnostics = test_path(
364+
Path::new("pyupgrade").join(path).as_path(),
365+
&settings::LinterSettings {
366+
isort: isort::settings::Settings {
367+
required_imports: BTreeSet::from_iter([
368+
// https://github.com/astral-sh/ruff/issues/20601
369+
NameImport::ImportFrom(MemberNameImport::member(
370+
"builtins".to_string(),
371+
"str".to_string(),
372+
)),
373+
]),
374+
..Default::default()
375+
},
376+
..settings::LinterSettings::for_rules([
377+
Rule::MissingRequiredImport,
378+
Rule::UnnecessaryBuiltinImport,
379+
])
380+
},
381+
)?;
382+
assert_diagnostics!(snapshot, diagnostics);
383+
Ok(())
384+
}
385+
360386
#[test]
361387
fn unnecessary_default_type_args_stubs_py312_preview() -> Result<()> {
362388
let snapshot = format!("{}__preview", "UP043.pyi");

crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use ruff_text_size::Ranged;
66

77
use crate::checkers::ast::Checker;
88
use crate::fix;
9+
use crate::rules::pyupgrade::rules::is_import_required_by_isort;
910
use crate::{AlwaysFixableViolation, Fix};
1011

1112
/// ## What it does
@@ -85,6 +86,13 @@ pub(crate) fn unnecessary_builtin_import(
8586
// Identify unaliased, builtin imports.
8687
let unused_imports: Vec<&Alias> = names
8788
.iter()
89+
.filter(|alias| {
90+
!is_import_required_by_isort(
91+
&checker.settings().isort.required_imports,
92+
stmt.into(),
93+
alias,
94+
)
95+
})
8896
.filter(|alias| alias.asname.is_none())
8997
.filter(|alias| {
9098
matches!(

crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP029.py.snap renamed to crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP029_0.py.snap

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
source: crates/ruff_linter/src/rules/pyupgrade/mod.rs
33
---
44
UP029 [*] Unnecessary builtin import: `*`
5-
--> UP029.py:1:1
5+
--> UP029_0.py:1:1
66
|
77
1 | from builtins import *
88
| ^^^^^^^^^^^^^^^^^^^^^^
@@ -17,7 +17,7 @@ help: Remove unnecessary builtin import
1717
note: This is an unsafe fix and may change runtime behavior
1818

1919
UP029 [*] Unnecessary builtin imports: `ascii`, `bytes`
20-
--> UP029.py:2:1
20+
--> UP029_0.py:2:1
2121
|
2222
1 | from builtins import *
2323
2 | from builtins import ascii, bytes, compile
@@ -35,7 +35,7 @@ help: Remove unnecessary builtin import
3535
note: This is an unsafe fix and may change runtime behavior
3636

3737
UP029 [*] Unnecessary builtin imports: `filter`, `zip`
38-
--> UP029.py:4:1
38+
--> UP029_0.py:4:1
3939
|
4040
2 | from builtins import ascii, bytes, compile
4141
3 | from builtins import str as _str
@@ -56,7 +56,7 @@ help: Remove unnecessary builtin import
5656
note: This is an unsafe fix and may change runtime behavior
5757

5858
UP029 [*] Unnecessary builtin import: `open`
59-
--> UP029.py:5:1
59+
--> UP029_0.py:5:1
6060
|
6161
3 | from builtins import str as _str
6262
4 | from six.moves import filter, zip, zip_longest
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
source: crates/ruff_linter/src/rules/pyupgrade/mod.rs
3+
---
4+
UP029 [*] Unnecessary builtin import: `int`
5+
--> UP029_1.py:1:1
6+
|
7+
1 | from builtins import str, int
8+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
9+
|
10+
help: Remove unnecessary builtin import
11+
- from builtins import str, int
12+
1 + from builtins import str
13+
note: This is an unsafe fix and may change runtime behavior

0 commit comments

Comments
 (0)