Import-sorting (a `ruff` approximation of `isort`)

[thejcannon]

isort does have some wacky heuristics to determine first v third party, but ultimately I'd love to elide the import-sorting it does with ruff ⚡

To me, doesn't have to be 1:1, so long as the behavior is there for import sorting/grouping I'm happy 😄

If we want to keep this in the realm of flake8, it'd be flake8-import-order with a fixer 😉

[thejcannon]

(I continually dip my toe into Rust and even a little PyO3 as I work on pantsbuild) Perhaps one day I'll pick this up myself 😉

[charliermarsh]

Yes! I want to do this. I've been a little intimidated by the sheer amount of functionality that's packaged with isort, but we don't need to cover every possible setting right from the start.

I'm not certain if this will fit into the same "autofixable lint error" pattern, or if it will be its own sub-command.

[andersk]

A potential first approximation that delays dealing with the first vs. third party heuristic would be to sort only within import groups separated by existing blank lines. This would be isort-compatible in that it preserves a superset of the possible isort outputs.

[tiangolo]

I like this idea! In my case it's just Isort with the Black profile, that's pretty much what I would like. 🤓

[charliermarsh]

👍 I'm thinking that this should be a separate subcommand (ruff isort). I've been meaning to introduce subcommands anyway (such that the current ruff call becomes ruff check; and maybe ruff --fix becomes a standalone ruff fix?) since it opens the door to bundling a lot more functionality into ruff. The downside is that this will likely be a breaking change in the CLI API.

[thejcannon]

Can you explain a little why you think it'd need its own subcommand and not be folded (optionally, config-enabled) into ruff fix or ruff?

From my perspective seems like I'd need to run two commands when one would be sufficient.

[charliermarsh]

You're probably right that it could be folded into ruff or ruff fix. I will aim for that outcome. My main concern is that in order to model this as an autofixable error under the current autofix API, I'll likely need to treat the entire unordered import section as a single violation with a single fix. And if there are unused imports in the import block, there will be conflicting fixes for that chunk of code, and we'll fail to autofix one of them due to the conflicting source code ranges (that is, we'll either fail to remove the import, or fail to reorder the block).

However, these are really deficiencies in the autofix API and not essential blockers to adding import-sorting as a "fixable lint error". And moving isort out to its own subcommand is arguably just a hack to get around these problems.

(One solution to this problem, which I don't love but is arguably already needed, is to iteratively re-check and autofix errors until there are no more fixable errors. So, e.g., you'd run ruff fix once, then internally, we'd check the file, find the unused import and the unsorted import block, and remove the unused import; we'd then re-check the modified source code, find the unsorted import block, fix that, etc.)

(Another solution is to try and develop a more semantically-aware fix API, or maybe something based on CRDTs (???), that lets you apply both "Reorder these statements" and "Remove this one statement" in a single pass.)

[thejcannon]

I have a suspicion that if you start with the slow-but-correct way, your bar-chart of performance will still hold very relevant.

And then when you figure out the fast-and-correct, everyone rejoices 😄

[charliermarsh]

(I've started work on this tonight.)

[charliermarsh]

Still a bunch of edge-cases to handle + proper line-length wrapping, but starting to come together:

[lithammer]

Not that my opinion matters much. But I have to admit that I'm not a big fan of how isort sorts out-of-the-box. And prefer something like this:

[tool.isort]
profile = "black"
force_sort_within_sections = true  # Don't group `from` imports separately
order_by_type = true  # Order by CONSTANT, CamelCase, snake_case

Mostly because it reduces noise in diffs when going back and forth between import foo and from foo import bar since the import would remain on the same line.

It's also bit jarring to make such a change, save, and then the line moves 10 lines up/down when you have "format on save" enabled in your editor.

But maybe that's just me 🤷‍♂️ People seems to like separating import and from.