Skip to content

Commit df313c2

Browse files
authored
Avoid infinite loop between I002 and PYI025 (#23352)
## Summary Fixes #20891. When `lint.isort.required-imports` includes `from collections.abc import Set` (unaliased) and PYI025 is enabled, the two rules conflict: I002 inserts the unaliased import, while PYI025 demands it be aliased as `AbstractSet`. This causes an infinite autofix loop ("Failed to converge after 100 iterations"). Rather than patching the import-insertion logic at the rule level, this PR rejects the contradictory configuration at startup in `Configuration::into_settings()`, following the existing pattern established by `conflicting_import_settings()`. The error message explains the conflict and suggests two resolutions: alias the required import as `AbstractSet`, or disable PYI025. ### Prior art PR #21115 took a different approach (modifying `add_required_imports.rs` and `imports.rs` to relax alias matching). As noted in review, the better fix is to "reject this configuration entirely at an earlier stage" since it doesn't make sense to simultaneously require an unaliased import and forbid it. This PR implements that suggestion directly. ## Test plan - 3 unit tests: conflicting config → error, aliased as `AbstractSet` → ok, PYI025 not enabled → ok - Reproduction case: `ruff check --config ruff.toml --unsafe-fixes --fix` now emits a clear configuration error instead of entering the infinite loop - `cargo test -p ruff_workspace` — all 26 tests pass
1 parent b2d856f commit df313c2

5 files changed

Lines changed: 150 additions & 0 deletions

crates/ruff/tests/cli/lint.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2871,6 +2871,48 @@ fn flake8_import_convention_unused_aliased_import_no_conflict() {
28712871
);
28722872
}
28732873

2874+
// https://github.com/astral-sh/ruff/issues/20891
2875+
#[test]
2876+
fn required_import_set_conflicts_with_pyi025() {
2877+
assert_cmd_snapshot!(
2878+
Command::new(get_cargo_bin(BIN_NAME))
2879+
.args(STDIN_BASE_OPTIONS)
2880+
.arg("--config")
2881+
.arg(r#"lint.isort.required-imports = ["from collections.abc import Set"]"#)
2882+
.args(["--select", "I002,PYI025"])
2883+
.arg("-")
2884+
.pass_stdin("1")
2885+
);
2886+
}
2887+
2888+
// https://github.com/astral-sh/ruff/issues/20891
2889+
#[test]
2890+
fn required_import_set_aliased_as_abstract_set_no_conflict() {
2891+
assert_cmd_snapshot!(
2892+
Command::new(get_cargo_bin(BIN_NAME))
2893+
.args(STDIN_BASE_OPTIONS)
2894+
.arg("--config")
2895+
.arg(r#"lint.isort.required-imports = ["from collections.abc import Set as AbstractSet"]"#)
2896+
.args(["--select", "I002,PYI025"])
2897+
.arg("-")
2898+
.pass_stdin("1")
2899+
);
2900+
}
2901+
2902+
// https://github.com/astral-sh/ruff/issues/20891
2903+
#[test]
2904+
fn required_import_set_without_pyi025_no_conflict() {
2905+
assert_cmd_snapshot!(
2906+
Command::new(get_cargo_bin(BIN_NAME))
2907+
.args(STDIN_BASE_OPTIONS)
2908+
.arg("--config")
2909+
.arg(r#"lint.isort.required-imports = ["from collections.abc import Set"]"#)
2910+
.args(["--select", "I002"])
2911+
.arg("-")
2912+
.pass_stdin("1")
2913+
);
2914+
}
2915+
28742916
// https://github.com/astral-sh/ruff/issues/19842
28752917
#[test]
28762918
fn pyupgrade_up026_respects_isort_required_import_fix() {
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
source: crates/ruff/tests/cli/lint.rs
3+
info:
4+
program: ruff
5+
args:
6+
- check
7+
- "--no-cache"
8+
- "--output-format"
9+
- concise
10+
- "--config"
11+
- "lint.isort.required-imports = [\"from collections.abc import Set as AbstractSet\"]"
12+
- "--select"
13+
- "I002,PYI025"
14+
- "-"
15+
stdin: "1"
16+
---
17+
success: false
18+
exit_code: 1
19+
----- stdout -----
20+
-:1:1: I002 [*] Missing required import: `from collections.abc import Set as AbstractSet`
21+
Found 1 error.
22+
[*] 1 fixable with the `--fix` option.
23+
24+
----- stderr -----
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
source: crates/ruff/tests/cli/lint.rs
3+
info:
4+
program: ruff
5+
args:
6+
- check
7+
- "--no-cache"
8+
- "--output-format"
9+
- concise
10+
- "--config"
11+
- "lint.isort.required-imports = [\"from collections.abc import Set\"]"
12+
- "--select"
13+
- "I002,PYI025"
14+
- "-"
15+
stdin: "1"
16+
---
17+
success: false
18+
exit_code: 2
19+
----- stdout -----
20+
21+
----- stderr -----
22+
ruff failed
23+
Cause: Required import `from collections.abc import Set` specified in `lint.isort.required-imports` (I002) conflicts with `unaliased-collections-abc-set-import` (PYI025), which requires this import to be aliased as `AbstractSet`.
24+
25+
Help: Either alias the required import (`from collections.abc import Set as AbstractSet`), or disable PYI025.
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
source: crates/ruff/tests/cli/lint.rs
3+
info:
4+
program: ruff
5+
args:
6+
- check
7+
- "--no-cache"
8+
- "--output-format"
9+
- concise
10+
- "--config"
11+
- "lint.isort.required-imports = [\"from collections.abc import Set\"]"
12+
- "--select"
13+
- I002
14+
- "-"
15+
stdin: "1"
16+
---
17+
success: false
18+
exit_code: 1
19+
----- stdout -----
20+
-:1:1: I002 [*] Missing required import: `from collections.abc import Set`
21+
Found 1 error.
22+
[*] 1 fixable with the `--fix` option.
23+
24+
----- stderr -----

crates/ruff_workspace/src/configuration.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ impl Configuration {
254254
.unwrap_or_default();
255255

256256
conflicting_import_settings(&isort, &flake8_import_conventions)?;
257+
conflicting_required_import_pyi025(&isort, &rules)?;
257258

258259
let future_annotations = lint.future_annotations.unwrap_or_default();
259260

@@ -1693,6 +1694,40 @@ fn conflicting_import_settings(
16931694
Ok(())
16941695
}
16951696

1697+
/// Detect conflicts between I002 (missing-required-import) and PYI025
1698+
/// (unaliased-collections-abc-set-import).
1699+
///
1700+
/// If `required-imports` includes `from collections.abc import Set` (without
1701+
/// aliasing it as `AbstractSet`) and PYI025 is enabled, the configuration is
1702+
/// contradictory: I002 requires the unaliased import, while PYI025 forbids it.
1703+
fn conflicting_required_import_pyi025(
1704+
isort: &isort::settings::Settings,
1705+
rules: &RuleTable,
1706+
) -> Result<()> {
1707+
if !rules.enabled(Rule::UnaliasedCollectionsAbcSetImport) {
1708+
return Ok(());
1709+
}
1710+
1711+
for required_import in &isort.required_imports {
1712+
let qualified_name = required_import.qualified_name();
1713+
if qualified_name.segments() == ["collections", "abc", "Set"]
1714+
&& required_import.bound_name() != "AbstractSet"
1715+
{
1716+
return Err(anyhow!(
1717+
"Required import `from collections.abc import Set` specified in \
1718+
`lint.isort.required-imports` (I002) conflicts with \
1719+
`unaliased-collections-abc-set-import` (PYI025), which requires \
1720+
this import to be aliased as `AbstractSet`.\n\n\
1721+
Help: Either alias the required import \
1722+
(`from collections.abc import Set as AbstractSet`), \
1723+
or disable PYI025."
1724+
));
1725+
}
1726+
}
1727+
1728+
Ok(())
1729+
}
1730+
16961731
#[cfg(test)]
16971732
mod tests {
16981733
use std::str::FromStr;

0 commit comments

Comments
 (0)