Applying or ignoring RUF022/RUF023 (__all__ & __slots__ sorting)

[Avasam]

As per #13090 (comment), here's a PR to discuss whether we want RUF022 in typeshed.

From a typing perspective, this technically changes the type of tuple-literals __all__:

__all__ = ("a", "b")  # tuple[Literal['a'], Literal['b']]
# is not the same as
__all__ = ("b", "a")  # tuple[Literal['b'], Literal['a']]

Whilst the cases where this matters are extremely niche, there is an argument for accuracy.

Essentially the same thing is mentioned in https://docs.astral.sh/ruff/rules/unsorted-dunder-all/#fix-safety

This rule's fix is marked as always being safe, in that it should very rarely alter the semantics of any Python code. However, note that (although it's rare) the value of all could be read by code elsewhere that depends on the exact iteration order of the items in all, in which case this rule's fix could theoretically cause breakage.

Although is that's something we care about, stubtest could probably test for it, which would counter the "pros" of ordering consistency mentioned below as it would be enforced by stubtest and the order defined by implementation.

Another argument is about copying __all__ from source. It might be easier to update/understand/debug if one can copy __all__ from implementation and the order stays the same.

---

Arguments for applying RUF022 is that autosorting and consistency are generally nice and after the initial commit help avoid conflicts. There's likely a few places in typeshed where we don't follow runtime ordering anyway.

---

The same question and overall arguments also apply to __slots__ sorting with RUF023

[Avasam]

In case you're searching for it, this is the only RUF023 change

[Avasam]

If we decide to ignore, these should go in the .pyi-only ignores.

[AlexWaygood]

I'm actually sort-of surprised that stubtest isn't freaking out about __all__ being ("foo", "bar") at runtime and ("bar", "foo") in the stub -- I would have expected stubtest to see those as different types (tuple[Literal["foo"], Literal["bar"]] vs tuple[Literal["bar"], Literal["foo"]]) and then complain when we changed the stub to have a different order than exists at runtime. Do we know why stubtest isn't complaining here? Did something in stubtest change, or was I just always wrong about how stubtest checked __all__ when __all__ is a tuple?

[AlexWaygood]

As long as this doesn't affect stubtest's ability to verify which items are in __all__ in the stub and how those compare to the runtime definition (and I don't see how it could affect that), I think I'm weakly in favour of this. I'm sure there are quite a few __all__ definitions in typeshed where we already don't match the order of __all__ at runtime, and we'd have to setup some custom tooling (either as part of stubtest or as a standalone tool) if that's something we wanted to verify. And as @tungol mentioned in #13090 (comment), answering the question "Does this have the same order as at runtime?" is kind of a complicated question for us because of all our __all__ modifications in sys.version_info and sys.platform branches.

[srittau]

I'm also +0 about this change.

[tungol]

Is there an option to sort list-based __all__ but not tuple-based __all__? The tuple ones are the ones that are technically not the same and also the ones were we don't dynamically add things in branches (but only because we can't, really). Getting stubtest to look for exact matches if __all__ is a tuple would probably be straightforward.

[JelleZijlstra]

I'd generally favor consistency with the runtime, but I don't really see any practical reason why the order should matter, so we might as well sort them.

[AlexWaygood]

Is there an option to sort list-based __all__ but not tuple-based __all__?

There isn't. I'd consider implementing such a feature at ruff (I'm a maintainer there), but it feels like a slightly niche thing for Ruff to add a configuration option for. (Outside of typeshed, who would use it? Would it be obvious to anybody why the option exists unless you're a stub author and typing expert?)

[JelleZijlstra]

I don't think there's a strong reason to treat list and tuple __all__ differently. Yes, in theory you could assign a tuple __all__ to a place with a precise type like tuple[Literal["a"], Literal["b"]], but that seems highly unlikely to matter in practice.

[Avasam]

Is there an option to sort list-based __all__ but not tuple-based __all__?

it feels like a slightly niche thing for Ruff to add a configuration option for. (Outside of typeshed, who would use it? [...])

The thought crossed my mind as well. But I have to agree, it would be extremely niche. And would only apply to stubs (as to re-expose the same type). The general comment about fix safety from the rule's doc still applies to tuples.

I thought about it some more over dinner, and I can't think of a single legitimate case where not only the order matters, but that you also can't deal with it programmatically (ie: getting all names that are in all-caps instead of the first 5 if you wanted the constants for example, and even that example isn't affected by tuple types). Like it would be so niche that you aren't looking at stubs/types anyway.

[tungol]

Do we know why stubtest isn't complaining here?

I don't know the whole answer, but I stuck a breakpoint in stubtest. The stub.type for asyncio.base_events.__all__ is showing as tuple[builtins.str, builtins.str] which compares favorably with the runtime's type of tuple[Literal['BaseEventLoop'], Literal['Server']]

[Avasam]

Do we know why stubtest isn't complaining here?

I don't know the whole answer, but I stuck a breakpoint in stubtest. The stub.type for asyncio.base_events.__all__ is showing as tuple[builtins.str, builtins.str]

It's a mypy vs pyright difference on inferred types.

So in other words, this doesn't even currently affect stubtest and mypy users.

[Akuli]

I would somewhat prefer using the copy/paste order as is, so that stubs are closer to implementation. As already mentioned, this way it's much easier to see how a long __all__ differs from the __all__ in the implementation. For example, consider reviewing a pull request where one more public member is added to a large module.

[github-actions]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

[AlexWaygood]

I feel like we should probably close this at this point (though we should probably get rid of the TODO comment in our pyproject.toml file). Since we don't have consensus among the core team, the status quo wins out IMO :-)

[Avasam]

I'm fine closing this.
Although I should still move the ignores to be stubs-only. I'll do that in a different PR.

Edit: #13322