You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fixes incorrect sorting of nested use path groups containing Multi variants.
The compare_use_paths function previously fell back to an empty string when comparing Multi use path variants, causing incorrect sort ordering for groups like {self, m} and {a, m}. The comparison now recurses via compare_use_paths itself to find the minimum child path, producing a correct and stable ordering.
Type of change
Please check one:
Bug fix (fixes incorrect behavior)
New feature
Performance improvement
Documentation change with concrete technical impact
Style, wording, formatting, or typo-only change
Why is this change needed?
When sorting nested use import groups, the Multi variant was assigned an empty string as its sort key instead of being compared recursively. This caused use x::{{self, m}, {a, m}}; and similar patterns to be ordered incorrectly.
What was the behavior or documentation before?
Nested use path groups with Multi variants were compared using an empty string fallback, so their relative ordering was undefined and incorrect. For example, {self, m} and {a, m} were not sorted by their actual minimum child paths.
What is the behavior or documentation after?
Nested use path groups are sorted by recursively comparing their minimum child paths using compare_use_paths, producing a correct and stable ordering. The expected sort order for several existing test cases is also corrected as a result.
Related issue or discussion (if any)
None.
Additional context
A new test case use x::{{self, m}, {a, m}}; is added to sort_inner_use.cairo to cover the previously broken multi-group comparison. The get_min_child closure is simplified by replacing the manual key extraction with a direct call to compare_use_paths, removing the need for the empty_string fallback variable.
The reason will be displayed to describe this comment to others. Learn more.
@TomerStarkware reviewed 3 files and all commit messages, and made 1 comment. Reviewable status: complete! all files reviewed, all discussions resolved (waiting on eytan-starkware).
Low Risk
Formatter-only sort/merge behavior for use statements; no compilation, runtime, or security impact.
Overview
Fixes incorrect ordering when the formatter sorts nested use path groups that contain Multi branches (e.g. use x::{{self, m}, {a, m}}).
In compare_use_paths, comparing two UsePath::Multi nodes no longer picks a minimum child via min_by_key on leaf/single idents with an empty-string fallback for nested multis. get_min_child now uses min_by(|a, b| compare_use_paths(a, b, db)), so sibling groups are ordered by their true minimum path under the same rules as the rest of the sorter.
Formatter tests add use x::{{self, m}, {a, m}} and refresh expected output for a few existing nested-a cases so golden files match the corrected sort.
Reviewed by Cursor Bugbot for commit 7a52914. Bugbot is set up for automated code reviews on this repo. Configure here.
orizi
deleted the
orizi/06-22-bugfix_formatter_made_formatter_merge_in_use_more_consistent
branch
June 23, 2026 08:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TomerStarkware
left a comment
complete! all files reviewed, all discussions resolved (waiting on eytan-starkware).
Summary
Fixes incorrect sorting of nested
usepath groups containingMultivariants.The
compare_use_pathsfunction previously fell back to an empty string when comparingMultiuse path variants, causing incorrect sort ordering for groups like{self, m}and{a, m}. The comparison now recurses viacompare_use_pathsitself to find the minimum child path, producing a correct and stable ordering.Type of change
Please check one:
Why is this change needed?
When sorting nested
useimport groups, theMultivariant was assigned an empty string as its sort key instead of being compared recursively. This causeduse x::{{self, m}, {a, m}};and similar patterns to be ordered incorrectly.What was the behavior or documentation before?
Nested
usepath groups withMultivariants were compared using an empty string fallback, so their relative ordering was undefined and incorrect. For example,{self, m}and{a, m}were not sorted by their actual minimum child paths.What is the behavior or documentation after?
Nested
usepath groups are sorted by recursively comparing their minimum child paths usingcompare_use_paths, producing a correct and stable ordering. The expected sort order for several existing test cases is also corrected as a result.Related issue or discussion (if any)
None.
Additional context
A new test case
use x::{{self, m}, {a, m}};is added tosort_inner_use.cairoto cover the previously broken multi-group comparison. Theget_min_childclosure is simplified by replacing the manual key extraction with a direct call tocompare_use_paths, removing the need for theempty_stringfallback variable.