Skip to content

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented May 27, 2025

Summary

Unlike a series of integers or strings, there's no self-evident way in which a series of KnownInstanceTypes should always be ordered if you're sorting them. This has always been the motivation for not deriving PartialOrd and Ord on this enum, and instead tediously writing this huge match statement out by hand.

Following #18212, however, we must explicitly derive PartialOrd and Ord for any Salsa-tracked structs that we wish to sort into an order. This is a good change -- it sharply brings into focus the fact that the orderings Salsa was previously implicitly deriving for us were also arbitrary orderings that had no grounding in the underlying type being represented by the Salsa ID. We already knew this, but now it's explicit.

The ordering of KnownInstanceTypes isn't inherently any more arbitrary than the orderings we're deriving elsewhere; so let's just derive PartialOrd, Ord here too. It makes the code a fair bit simpler :-)

Test Plan

cargo test -p ty_python_semantic

@AlexWaygood AlexWaygood added internal An internal refactor or improvement ty Multi-file analysis & type inference labels May 27, 2025
@github-actions
Copy link
Contributor

github-actions bot commented May 27, 2025

mypy_primer results

No ecosystem changes detected ✅

@AlexWaygood AlexWaygood force-pushed the alex/knowninstance-ord branch from 17f26b4 to ed5a8cb Compare May 27, 2025 18:21
@AlexWaygood AlexWaygood merged commit 3e811fc into main May 27, 2025
35 checks passed
@AlexWaygood AlexWaygood deleted the alex/knowninstance-ord branch May 27, 2025 18:37
carljm added a commit to MatthewMckee4/ruff that referenced this pull request May 28, 2025
* main: (246 commits)
  [ty] Simplify signature types, use them in `CallableType` (astral-sh#18344)
  [ty] Support ephemeral uv virtual environments (astral-sh#18335)
  Add a `ViolationMetadata::rule` method (astral-sh#18234)
  Return `DiagnosticGuard` from `Checker::report_diagnostic` (astral-sh#18232)
  [flake8_use_pathlib]: Replace os.symlink with Path.symlink_to (PTH211) (astral-sh#18337)
  [ty] Support cancellation and retry in the server (astral-sh#18273)
  [ty] Synthetic function-like callables (astral-sh#18242)
  [ty] Support publishing diagnostics in the server (astral-sh#18309)
  Add Autofix for ISC003 (astral-sh#18256)
  [`pyupgrade`]: new rule UP050 (`useless-class-metaclass-type`) (astral-sh#18334)
  [pycodestyle] Make `E712` suggestion not assume a context (astral-sh#18328)
  put similar dunder-call tests next to each other (astral-sh#18343)
  [ty] Derive `PartialOrd, Ord` for `KnownInstanceType` (astral-sh#18340)
  [ty] Simplify `Type::try_bool()` (astral-sh#18342)
  [ty] Simplify `Type::normalized` slightly (astral-sh#18339)
  [ty] Move arviz off the list of selected primer projects (astral-sh#18336)
  [ty] Add --config-file CLI arg (astral-sh#18083)
  [ty] Tell the user why we inferred a certain Python version when reporting version-specific syntax errors (astral-sh#18295)
  [ty] Implement implicit inheritance from `Generic[]` for PEP-695 generic classes (astral-sh#18283)
  [ty] Add hint if async context manager is used in non-async with statement (astral-sh#18299)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal An internal refactor or improvement ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants