bugfix(formatter): Sort merged items via compare_names. - #10123
Conversation
PR SummaryLow Risk Overview Adds Reviewed by Cursor Bugbot for commit 294280c. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83e0aeffdd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
83e0aef to
294280c
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi resolved 1 discussion.
Reviewable status: 0 of 4 files reviewed, all discussions resolved (waiting on eytan-starkware and TomerStarkware).
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on eytan-starkware).

Summary
Fixes incorrect leaf ordering when merging
useitems withallow_duplicate_usesdisabled. Previously, leaves were converted to strings before deduplication and sorting, which meant the sort operated on formatted strings (e.g.,"x as y") rather than on the structuredname/aliasfields. This causedselfto not be placed first in merged braces groups. Now, sorting and deduplication happen directly onLeafstructs, sorting byname(usingcompare_names) and then byalias, before formatting. ADisplayimpl andPartialEq/Eqderives were added toLeafto support this.A new test case (
use_merge_no_sort.cairo) covers the scenario where module-level sorting is disabled but merged braces must still honor theself-first convention.Type of change
Please check one:
Why is this change needed?
When merging
usepaths with duplicate-use checking enabled, leaves likeself,b, andzunder the same prefix were not being ordered correctly. The old code sorted string representations of leaves (e.g.,"self","b","z") after formatting them, butcompare_names(which enforcesself-first ordering) was never applied to leaves — only to the string-level sort. This meant the output could be{b, self, z}instead of the correct{self, b, z}.What was the behavior or documentation before?
Given:
The formatter could produce
use a::{b, self, z};instead ofuse a::{self, b, z};when merging was enabled but module-level sorting was disabled.What is the behavior or documentation after?
The formatter now correctly produces:
Leaf nodes are sorted using
compare_nameson theirnamefield (withaliasas a tiebreaker) before being formatted and merged into braces groups.Related issue or discussion (if any)
None.
Additional context
The
Leafstruct now derivesPartialEqandEq(required fordedup) and implementsDisplayto avoid duplicating thename as aliasformatting logic.