From 5c0a2284e0c339ced9283becfac92f5d276b5e32 Mon Sep 17 00:00:00 2001 From: Will Buckner <1458615+willbuckner@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:24:54 -0600 Subject: [PATCH] fix: make imports_granularity One preserve aliases Fix `imports_granularity = "One"` to preserve aliases. In the following example by sivizius: ```rust pub use foo::x; pub use foo::x as x2; pub use foo::y; use bar::a; use bar::b; use bar::b::f; use bar::b::f as f2; use bar::b::g; use bar::c; use bar::d::e; use bar::d::e as e2; use qux::h; use qux::i; use qux::i as j; ``` `bar::b::f as f2`; and `qux::i as j`; were silently dropped, returning this merged result: ```rust pub use foo::{x, x as x2, y}; use { bar::{ a, b::{self, f, g}, c, d::{e, e as e2}, }, qux::{h, i}, }; ``` Two import paths that only differ by the alias of their last segment were being treated as equal when merging, keeping only one of the two names. They now get merged into a list containing both, e.g., `qux::{h, i, i as j}`. This also fixes two related problems: - Merging `use qux::h;` followed by `use qux as Q;` produced the invalid `use qux as Q::{self as Q, h};` because the merged root kept the alias of the shorter path. - The result of merging depends on the order in which the `use` trees get visited, and once aliases are preserved a single pass over inputs like `use a; use a as b; use a::c;` did not produce a stable result across runs. Aliased root imports now get rewritten to the `self` import they are short for (`a::{self as b}`) before merging whenever something deeper from the same root is imported too, so every order of the input merges to the same `use a::{self, self as b, c};` in a single pass. Fixes: #6027 --- src/imports.rs | 149 ++++++++++++++++++++++++++++++++++++- tests/source/5131_one.rs | 1 + tests/source/issue-6027.rs | 10 +++ tests/target/5131_one.rs | 4 +- tests/target/issue-6027.rs | 10 +++ 5 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 tests/source/issue-6027.rs create mode 100644 tests/target/issue-6027.rs diff --git a/src/imports.rs b/src/imports.rs index c5a2a5de2f1..2d5dbc4c8eb 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -40,6 +40,29 @@ fn module_prefix(path: &[UseSegment]) -> &[UseSegment] { &path[..(path.len() - 1).max(1)] } +/// Rewrites a path ending in an aliased `self` (e.g., flattened from +/// `use a::{self as b};`) to the equivalent `a as b`. Used for One-level +/// imports_granularity. +fn collapse_trailing_self_alias(path: &mut Vec) { + if path.len() < 2 { + return; + } + + let alias = match &path.last().unwrap().kind { + UseSegmentKind::Slf(Some(alias)) => alias.clone(), + _ => return, + }; + let prev = path.len() - 2; + match &mut path[prev].kind { + UseSegmentKind::Ident(_, a @ None) + | UseSegmentKind::Super(a @ None) + | UseSegmentKind::Crate(a @ None) => *a = Some(alias), + _ => return, + } + + path.pop(); +} + impl<'a> FmtVisitor<'a> { pub(crate) fn format_import(&mut self, item: &ast::Item, tree: &ast::UseTree) { let span = item.span(); @@ -231,6 +254,12 @@ pub(crate) fn normalize_use_trees_with_granularity( ImportGranularity::One => SharedPrefix::One, }; + let use_trees = if merge_by == SharedPrefix::One { + nest_aliased_root_imports(use_trees) + } else { + use_trees + }; + let mut result = Vec::with_capacity(use_trees.len()); for use_tree in use_trees { if use_tree.contains_comment() || use_tree.attrs.is_some() { @@ -256,6 +285,61 @@ pub(crate) fn normalize_use_trees_with_granularity( result } +/// Flattens the given use trees and rewrites an aliased import of a module +/// root as the `self` import it is short for (`use a as b;` becomes +/// `a::{self as b}`) whenever the same batch imports something deeper from +/// that root. +/// +/// A root import changes representation when it is merged into a deeper path, +/// so merging the original form may not be stable (as in, a future run could +/// reformat the same use statement again). Doing this canonicalization ahead +/// of time gives a consistent result across runs. Used for One-level +/// imports_granularity. +fn nest_aliased_root_imports(use_trees: Vec) -> Vec { + let mut flattened = Vec::with_capacity(use_trees.len()); + for use_tree in use_trees { + if use_tree.contains_comment() || use_tree.attrs.is_some() { + flattened.push(use_tree); + } else { + flattened.append(&mut use_tree.flatten(ImportGranularity::One)); + } + } + + for i in 0..flattened.len() { + let tree = &flattened[i]; + if tree.path.len() != 1 || tree.attrs.is_some() || tree.contains_comment() { + continue; + } + if !matches!(tree.path[0].kind, UseSegmentKind::Ident(_, Some(_))) { + continue; + } + let has_deeper_import = flattened.iter().enumerate().any(|(j, other)| { + j != i + && other.path.len() > 1 + && other.attrs.is_none() + && !other.contains_comment() + && other.path[0].equal_except_alias(&flattened[i].path[0]) + && flattened[i].same_visibility(other) + }); + if !has_deeper_import { + continue; + } + + // Move the alias from the root over to a trailing `self` segment. + let root = &mut flattened[i].path[0]; + let style_edition = root.style_edition; + let alias = match &mut root.kind { + UseSegmentKind::Ident(_, alias) => alias.take(), + _ => None, + }; + flattened[i].path.push(UseSegment { + kind: UseSegmentKind::Slf(alias), + style_edition, + }); + } + flattened +} + fn flatten_use_trees( use_trees: Vec, import_granularity: ImportGranularity, @@ -713,6 +797,12 @@ impl UseTree { for flattened in &mut nested_use_tree.clone().flatten(import_granularity) { let mut new_path = prefix.to_vec(); new_path.append(&mut flattened.path); + // `use a::{self as b};` is equivalent to `use a as b;`. + // Normalize to the latter so that merging treats both + // forms the same way. + if import_granularity == ImportGranularity::One { + collapse_trailing_self_alias(&mut new_path); + } result.push(UseTree { path: new_path, span: self.span, @@ -775,7 +865,14 @@ fn merge_rest( merge_by: SharedPrefix, ) -> Option> { if a.len() == len && b.len() == len { - return None; + if a[len - 1] == b[len - 1] { + return None; + } + + // The paths only differ by the alias of the last segment, e.g., + // `foo::bar` and `foo::bar as baz`. These are distinct imports, so + // keep both in a list: `foo::{bar, bar as baz}`. + len -= 1; } if a.len() != len && b.len() != len { let style_edition = a[len].style_edition; @@ -819,7 +916,9 @@ fn merge_rest( _ => list.push(UseTree::from_path(rest.to_vec(), DUMMY_SP)), } return Some(vec![ - b[0].clone(), + // The alias of the common segment (if any) has moved to the + // `self` item in the list; drop it from the root. + common.remove_alias(), UseSegment { kind: UseSegmentKind::List(list), style_edition, @@ -857,11 +956,24 @@ fn merge_use_trees_inner(trees: &mut Vec, use_tree: UseTree, merge_by: // tree `use_tree` should be merge. // In other cases `similarity` won't be used, so set it to `0` as a dummy value. let similarity = if merge_by == SharedPrefix::One { - tree.path + let similarity = tree + .path .iter() .zip(&use_tree.path) .take_while(|(a, b)| a.equal_except_alias(b)) - .count() + .count(); + // Single-segment trees that only differ by alias, e.g., `foo` + // and `foo as bar`, import distinct names that must not be + // merged into each other. + if similarity == 1 + && tree.path.len() == 1 + && use_tree.path.len() == 1 + && tree.path != use_tree.path + { + 0 + } else { + similarity + } } else { 0 }; @@ -1431,6 +1543,35 @@ mod test { ["b", "a::ac::{aca, acb}", "a::{aa::*, ab}"], ["{a::{aa::*, ab, ac::{aca, acb}}, b}"] ); + + // aliases should not be dropped when merging (#6027) + test_merge!(One, ["a::b", "a::b as c"], ["a::{b, b as c}"]); + + test_merge!(One, ["a::b as c", "a::b"], ["a::{b as c, b}"]); + + test_merge!(One, ["a::z", "a::b", "a::b as c"], ["a::{b, b as c, z}"]); + + test_merge!(One, ["a", "a as b"], ["{a, a as b}"]); + + test_merge!(One, ["a as b", "a as c"], ["{a as b, a as c}"]); + + test_merge!(One, ["a::b as c", "a::b as d"], ["a::{b as c, b as d}"]); + + // the alias of a shared root is rewritten as `self as alias`, + // regardless of which side of the merge it appears on + test_merge!(One, ["a as x", "a::b"], ["a::{self as x, b}"]); + + test_merge!(One, ["a::b", "a as x"], ["a::{self as x, b}"]); + + // an aliased root and a deeper path under the same root should merge + // to the same result regardless of their order + test_merge!(One, ["a", "a as b", "a::c"], ["a::{self, self as b, c}"]); + + test_merge!(One, ["a as b", "a", "a::c"], ["a::{self, self as b, c}"]); + + test_merge!(One, ["a::c", "a", "a as b"], ["a::{self, self as b, c}"]); + + test_merge!(One, ["a::c", "a as b", "a"], ["a::{self, self as b, c}"]); } #[test] diff --git a/tests/source/5131_one.rs b/tests/source/5131_one.rs index 61ddf13410d..91d5fb0de95 100644 --- a/tests/source/5131_one.rs +++ b/tests/source/5131_one.rs @@ -13,3 +13,4 @@ use bar::d::e; use bar::d::e as e2; use qux::h; use qux::i; +use qux::i as j; diff --git a/tests/source/issue-6027.rs b/tests/source/issue-6027.rs new file mode 100644 index 00000000000..a985a2e6256 --- /dev/null +++ b/tests/source/issue-6027.rs @@ -0,0 +1,10 @@ +// rustfmt-imports_granularity: One + +use qux::h; +use qux::i; +use qux::i as j; +use bar::a; +use bar::b::f; +use bar::b as B; +use baz::c as x; +use baz::c as y; diff --git a/tests/target/5131_one.rs b/tests/target/5131_one.rs index a086dae5a42..61ab9f3b8b3 100644 --- a/tests/target/5131_one.rs +++ b/tests/target/5131_one.rs @@ -4,9 +4,9 @@ pub use foo::{x, x as x2, y}; use { bar::{ a, - b::{self, f, g}, + b::{self, f, f as f2, g}, c, d::{e, e as e2}, }, - qux::{h, i}, + qux::{h, i, i as j}, }; diff --git a/tests/target/issue-6027.rs b/tests/target/issue-6027.rs new file mode 100644 index 00000000000..de5b44de32e --- /dev/null +++ b/tests/target/issue-6027.rs @@ -0,0 +1,10 @@ +// rustfmt-imports_granularity: One + +use { + bar::{ + a, + b::{self as B, f}, + }, + baz::{c as x, c as y}, + qux::{h, i, i as j}, +};