Skip to content

Fix Ruff import sorting when fix batches omit package markers#23298

Open
tdyas wants to merge 7 commits intomainfrom
tdyas/ruff-init_py-fixes
Open

Fix Ruff import sorting when fix batches omit package markers#23298
tdyas wants to merge 7 commits intomainfrom
tdyas/ruff-init_py-fixes

Conversation

@tdyas
Copy link
Copy Markdown
Contributor

@tdyas tdyas commented Apr 29, 2026

Problem

As observed with CI test failures in #23284, Ruff’s isort rules can classify imports differently depending on whether or not ancestor __init__.py files are present in the sandbox. The fix goal may split files into batches, which can leave a Ruff batch without the package marker files that were present during lint. In that case, pants lint can report a fixable I001, while pants fix reports "All checks passed!."

Solution

This change adds a custom partitioner to the Ruff fix rules to record the necessary __init__.py files as per-partition metadata. When the fix goal executes on a partition, it includes the __init__.py files set in the metadata back into the input digest so Ruff sees the correct package behavior.

Also adds a regression test covering a local package whose name shadows a third-party package.

AI Disclousre

Codex (GPT-5.5) was used to diagnose the problem observed in #23284, write a reproduction test, and write the fix with design guidance from me.

@tdyas tdyas added category:bugfix Bug fixes for released features release-notes:not-required [CI] PR doesn't require mention in release notes labels Apr 29, 2026
@tdyas tdyas force-pushed the tdyas/ruff-init_py-fixes branch from 7d39a1a to 66a73ab Compare April 29, 2026 08:00
@tdyas tdyas marked this pull request as ready for review April 29, 2026 08:08
@tdyas tdyas requested review from benjyw and sureshjoshi April 29, 2026 08:08
@tdyas tdyas removed the release-notes:not-required [CI] PR doesn't require mention in release notes label Apr 29, 2026
Comment thread src/python/pants/backend/python/lint/ruff/check/rules.py Outdated
@dataclass(frozen=True)
class RunRuffRequest:
snapshot: Snapshot
files: tuple[str, ...]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bear with me, as I haven't had my coffee - but why do we have this files? It's an alias for snapshot.files in several of these cases.

So, it's files for the files we want fixed, while snapshot is files + init.py?

Copy link
Copy Markdown
Contributor Author

@tdyas tdyas Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. RunRuffRequest.files is the Python files to fix while snapshot contains those files plus any __init__.py which needed to be added back.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of naming, files is consistent with the AbstractFmtRequest.files property.

@sureshjoshi
Copy link
Copy Markdown
Member

I have a vague recollection that there was an idea to not have ruff partitions, as it would be faster to just run it over the largest set of code, than to go through the overhead to run it otherwise.

However, I don't see how that would have worked in multi-py-version repos, as each has it's own python level.

@tdyas
Copy link
Copy Markdown
Contributor Author

tdyas commented Apr 29, 2026

I have a vague recollection that there was an idea to not have ruff partitions, as it would be faster to just run it over the largest set of code, than to go through the overhead to run it otherwise.

However, I don't see how that would have worked in multi-py-version repos, as each has it's own python level.

I don't have any context on the Ruff design choices. The custom partitioner here is putting everything into a single partition. The fix goal, however, is capable of splitting partitions; for example, by size. The __init__.py files don't get special treatment in that case and could be separated from other Python files. This PR makes sure they get added back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:bugfix Bug fixes for released features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants