[`ruff`] Prevent PYI025 from clashing with isort-required check by TaKO8Ki · Pull Request #21115 · astral-sh/ruff

TaKO8Ki · 2025-10-28T22:11:37Z

Summary

Fixes #20891

Test Plan

Added required_import_skips_pyi025.

remove new line

github-actions · 2025-10-28T22:26:37Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

ntBre

Thank you!

I confirmed locally that this fixes the issue, but this test doesn't reproduce the error from the issue when I revert the rule change. I think you need to include a flag like --fix or --diff in the Command to reproduce the infinite loop.

AlexWaygood · 2025-10-29T19:07:51Z

crates/ruff_linter/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs

+    // Skip if this import is required by isort to prevent infinite loops with I002 and F401
+    if let Some(stmt @ Stmt::ImportFrom(StmtImportFrom { names, .. })) =
+        binding.statement(checker.semantic())
+        && names.iter().any(|alias| {
+            is_import_required_by_isort(
+                &checker.settings().isort.required_imports,
+                stmt.into(),
+                alias,
+            )
+        })
+    {
+        return;
+    }


I don't think this is the best fix. To me I think it would make more sense to reject this configuration entirely at an earlier stage. Fundamentally, it just doesn't make sense to tell Ruff that you simultaneously want from collections.abc import Set to always appear in every module, and also from collections.abc import Set to never appear in any module.

That implies that we'd want to edit this part of the code:

ruff/crates/ruff_workspace/src/configuration.rs

Lines 1614 to 1651 in aca8ba7

/// Detect conflicts between I002 (missing-required-import) and ICN001 (unconventional-import-alias)

fn conflicting_import_settings(

isort: &isort::settings::Settings,

flake8_import_conventions: &flake8_import_conventions::settings::Settings,

) -> Result<()> {

use std::fmt::Write;

let mut err_body = String::new();

for required_import in &isort.required_imports {

// Ex: `from foo import bar as baz` OR `import foo.bar as baz`

// - qualified name: `foo.bar`

// - bound name: `baz`

// - conflicts with: `{"foo.bar":"buzz"}`

// - does not conflict with either of

// - `{"bar":"buzz"}`

// - `{"foo.bar":"baz"}`

let qualified_name = required_import.qualified_name().to_string();

let bound_name = required_import.bound_name();

let Some(alias) = flake8_import_conventions.aliases.get(&qualified_name) else {

continue;

};

if alias != bound_name {

writeln!(err_body, " - `{qualified_name}` -> `{alias}`").unwrap();

}

}

if !err_body.is_empty() {

return Err(anyhow!(

"Required import specified in `lint.isort.required-imports` (I002) \

conflicts with the required import alias specified in either \

`lint.flake8-import-conventions.aliases` or \

`lint.flake8-import-conventions.extend-aliases` (ICN001):\

\n{err_body}\n\

Help: Remove the required import or alias from your configuration."

));

}

Ok(())

}

I thought about that too. This seemed like a pretty niche issue, so I didn't feel too strongly, but this also makes sense to me.

I generally have a pretty strong preference towards rejecting invalid configurations rather than silently accepting them and assuming we understand what the user actually wanted 😄

We've done a bit of both with these conflicts, I think. #16802 is one example, but maybe that's different 🙂

Ok. I will check it.

MichaReiser · 2026-01-21T10:30:23Z

Thanks for working on this. I'll close this PR due to inactivity. Feel free to resubmit the changes with the feedback addressed.

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

Prevent PYI025 from clashing with isort-required check

7c97ada

remove new line

TaKO8Ki requested a review from AlexWaygood as a code owner October 28, 2025 22:11

amyreese added fixes Related to suggested fixes for violations bug Something isn't working labels Oct 28, 2025

amyreese requested a review from ntBre October 28, 2025 23:53

ntBre reviewed Oct 29, 2025

View reviewed changes

AlexWaygood reviewed Oct 29, 2025

View reviewed changes

MichaReiser closed this Jan 21, 2026

kar-ganap mentioned this pull request Feb 16, 2026

Avoid infinite loop between I002 and PYI025 #23352

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[`ruff`] Prevent PYI025 from clashing with isort-required check#21115

[`ruff`] Prevent PYI025 from clashing with isort-required check#21115
TaKO8Ki wants to merge 1 commit intoastral-sh:mainfrom
TaKO8Ki:fix-piy025-isort-loop

TaKO8Ki commented Oct 28, 2025

Uh oh!

github-actions bot commented Oct 28, 2025

Uh oh!

ntBre left a comment

Uh oh!

AlexWaygood Oct 29, 2025

Uh oh!

ntBre Oct 29, 2025

Uh oh!

AlexWaygood Oct 29, 2025

Uh oh!

ntBre Oct 29, 2025

Uh oh!

TaKO8Ki Oct 31, 2025

Uh oh!

MichaReiser commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	/// Detect conflicts between I002 (missing-required-import) and ICN001 (unconventional-import-alias)
	fn conflicting_import_settings(
	isort: &isort::settings::Settings,
	flake8_import_conventions: &flake8_import_conventions::settings::Settings,
	) -> Result<()> {
	use std::fmt::Write;
	let mut err_body = String::new();
	for required_import in &isort.required_imports {
	// Ex: `from foo import bar as baz` OR `import foo.bar as baz`
	// - qualified name: `foo.bar`
	// - bound name: `baz`
	// - conflicts with: `{"foo.bar":"buzz"}`
	// - does not conflict with either of
	// - `{"bar":"buzz"}`
	// - `{"foo.bar":"baz"}`
	let qualified_name = required_import.qualified_name().to_string();
	let bound_name = required_import.bound_name();
	let Some(alias) = flake8_import_conventions.aliases.get(&qualified_name) else {
	continue;
	};
	if alias != bound_name {
	writeln!(err_body, " - `{qualified_name}` -> `{alias}`").unwrap();
	}
	}

	if !err_body.is_empty() {
	return Err(anyhow!(
	"Required import specified in `lint.isort.required-imports` (I002) \
	conflicts with the required import alias specified in either \
	`lint.flake8-import-conventions.aliases` or \
	`lint.flake8-import-conventions.extend-aliases` (ICN001):\
	\n{err_body}\n\
	Help: Remove the required import or alias from your configuration."
	));
	}

	Ok(())
	}

Conversation

TaKO8Ki commented Oct 28, 2025

Summary

Test Plan

Uh oh!

github-actions bot commented Oct 28, 2025

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

TaKO8Ki Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

`ruff-ecosystem` results