Repository: rust-lang/rust-analyzer
Pull request #12142 (2022-05-03 16:13:55)

feat: Sort items by trait definition assist
This PR replaces the "Sort **methods** by trait definition" assist with a "Sort **items** by trait definition" assist that sorts all items, not just methods.

![sort-items-by-trait-def-showcase](https://user-images.githubusercontent.com/38225716/166491828-0bc10dbd-91be-408f-9fe0-636ef5e99377.gif)


Patch:
diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs
index 2a11c9d9bf..7f5ad41509 100644
--- a/crates/hir-ty/src/infer.rs
+++ b/crates/hir-ty/src/infer.rs
@@ -19,7 +19,7 @@ use std::sync::Arc;
 use chalk_ir::{cast::Cast, ConstValue, DebruijnIndex, Mutability, Safety, Scalar, TypeFlags};
 use hir_def::{
     body::Body,
-    data::{ConstData, StaticData},
+    data::{ConstData, FunctionData, StaticData},
     expr::{BindingAnnotation, ExprId, PatId},
     lang_item::LangItemTarget,
     path::{path, Path},
@@ -32,13 +32,12 @@ use hir_expand::name::{name, Name};
 use itertools::Either;
 use la_arena::ArenaMap;
 use rustc_hash::FxHashMap;
-use stdx::{always, impl_from};
+use stdx::impl_from;
 
 use crate::{
-    db::HirDatabase, fold_tys, fold_tys_and_consts, infer::coerce::CoerceMany,
-    lower::ImplTraitLoweringMode, to_assoc_type_id, AliasEq, AliasTy, Const, DomainGoal,
-    GenericArg, Goal, ImplTraitId, InEnvironment, Interner, ProjectionTy, Substitution,
-    TraitEnvironment, TraitRef, Ty, TyBuilder, TyExt, TyKind,
+    db::HirDatabase, fold_tys_and_consts, infer::coerce::CoerceMany, lower::ImplTraitLoweringMode,
+    to_assoc_type_id, AliasEq, AliasTy, Const, DomainGoal, GenericArg, Goal, InEnvironment,
+    Interner, ProjectionTy, Substitution, TraitEnvironment, TraitRef, Ty, TyBuilder, TyExt, TyKind,
 };
 
 // This lint has a false positive here. See the link below for details.
@@ -65,7 +64,7 @@ pub(crate) fn infer_query(db: &dyn HirDatabase, def: DefWithBodyId) -> Arc<Infer
 
     match def {
         DefWithBodyId::ConstId(c) => ctx.collect_const(&db.const_data(c)),
-        DefWithBodyId::FunctionId(f) => ctx.collect_fn(f),
+        DefWithBodyId::FunctionId(f) => ctx.collect_fn(&db.function_data(f)),
         DefWithBodyId::StaticId(s) => ctx.collect_static(&db.static_data(s)),
     }
 
@@ -458,8 +457,7 @@ impl<'a> InferenceContext<'a> {
         self.return_ty = self.make_ty(&data.type_ref);
     }
 
-    fn collect_fn(&mut self, func: FunctionId) {
-        let data = self.db.function_data(func);
+    fn collect_fn(&mut self, data: &FunctionData) {
         let ctx = crate::lower::TyLoweringContext::new(self.db, &self.resolver)
             .with_impl_trait_mode(ImplTraitLoweringMode::Param);
         let param_tys =
@@ -476,42 +474,8 @@ impl<'a> InferenceContext<'a> {
         } else {
             &*data.ret_type
         };
-        let return_ty = self.make_ty_with_mode(return_ty, ImplTraitLoweringMode::Opaque);
+        let return_ty = self.make_ty_with_mode(return_ty, ImplTraitLoweringMode::Disallowed); // FIXME implement RPIT
         self.return_ty = return_ty;
-
-        if let Some(rpits) = self.db.return_type_impl_traits(func) {
-            // RPIT opaque types use substitution of their parent function.
-            let fn_placeholders = TyBuilder::placeholder_subst(self.db, func);
-            self.return_ty = fold_tys(
-                self.return_ty.clone(),
-                |ty, _| {
-                    let opaque_ty_id = match ty.kind(Interner) {
-                        TyKind::OpaqueType(opaque_ty_id, _) => *opaque_ty_id,
-                        _ => return ty,
-                    };
-                    let idx = match self.db.lookup_intern_impl_trait_id(opaque_ty_id.into()) {
-                        ImplTraitId::ReturnTypeImplTrait(_, idx) => idx,
-                        _ => unreachable!(),
-                    };
-                    let bounds = (*rpits).map_ref(|rpits| {
-                        rpits.impl_traits[idx as usize].bounds.map_ref(|it| it.into_iter())
-                    });
-                    let var = self.table.new_type_var();
-                    let var_subst = Substitution::from1(Interner, var.clone());
-                    for bound in bounds {
-                        let predicate =
-                            bound.map(|it| it.cloned()).substitute(Interner, &fn_placeholders);
-                        let (var_predicate, binders) = predicate
-                            .substitute(Interner, &var_subst)
-                            .into_value_and_skipped_binders();
-                        always!(binders.len(Interner) == 0); // quantified where clauses not yet handled
-                        self.push_obligation(var_predicate.cast(Interner));
-                    }
-                    var
-                },
-                DebruijnIndex::INNERMOST,
-            );
-        }
     }
 
     fn infer_body(&mut self) {
diff --git a/crates/ide-assists/src/handlers/reorder_impl.rs b/crates/ide-assists/src/handlers/reorder_impl_items.rs
similarity index 51%
rename from crates/ide-assists/src/handlers/reorder_impl.rs
rename to crates/ide-assists/src/handlers/reorder_impl_items.rs
index c0991c6080..2bda3a1817 100644
--- a/crates/ide-assists/src/handlers/reorder_impl.rs
+++ b/crates/ide-assists/src/handlers/reorder_impl_items.rs
@@ -6,46 +6,46 @@ use syntax::{
     ted, AstNode,
 };
 
-use crate::{utils::get_methods, AssistContext, AssistId, AssistKind, Assists};
+use crate::{AssistContext, AssistId, AssistKind, Assists};
 
-// Assist: reorder_impl
+// Assist: reorder_impl_items
 //
-// Reorder the methods of an `impl Trait`. The methods will be ordered
+// Reorder the items of an `impl Trait`. The items will be ordered
 // in the same order as in the trait definition.
 //
 // ```
 // trait Foo {
-//     fn a() {}
-//     fn b() {}
-//     fn c() {}
+//     type A;
+//     const B: u8;
+//     fn c();
 // }
 //
 // struct Bar;
 // $0impl Foo for Bar {
-//     fn b() {}
+//     const B: u8 = 17;
 //     fn c() {}
-//     fn a() {}
+//     type A = String;
 // }
 // ```
 // ->
 // ```
 // trait Foo {
-//     fn a() {}
-//     fn b() {}
-//     fn c() {}
+//     type A;
+//     const B: u8;
+//     fn c();
 // }
 //
 // struct Bar;
 // impl Foo for Bar {
-//     fn a() {}
-//     fn b() {}
+//     type A = String;
+//     const B: u8 = 17;
 //     fn c() {}
 // }
 // ```
-pub(crate) fn reorder_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+pub(crate) fn reorder_impl_items(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let impl_ast = ctx.find_node_at_offset::<ast::Impl>()?;
     let items = impl_ast.assoc_item_list()?;
-    let methods = get_methods(&items);
+    let assoc_items = items.assoc_items().collect::<Vec<_>>();
 
     let path = impl_ast
         .trait_()
@@ -55,29 +55,37 @@ pub(crate) fn reorder_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
         })?
         .path()?;
 
-    let ranks = compute_method_ranks(&path, ctx)?;
-    let sorted: Vec<_> = methods
+    let ranks = compute_item_ranks(&path, ctx)?;
+    let sorted: Vec<_> = assoc_items
         .iter()
         .cloned()
-        .sorted_by_key(|f| {
-            f.name().and_then(|n| ranks.get(&n.to_string()).copied()).unwrap_or(usize::max_value())
+        .sorted_by_key(|i| {
+            let name = match i {
+                ast::AssocItem::Const(c) => c.name(),
+                ast::AssocItem::Fn(f) => f.name(),
+                ast::AssocItem::TypeAlias(t) => t.name(),
+                ast::AssocItem::MacroCall(_) => None,
+            };
+
+            name.and_then(|n| ranks.get(&n.to_string()).copied()).unwrap_or(usize::max_value())
         })
         .collect();
 
     // Don't edit already sorted methods:
-    if methods == sorted {
+    if assoc_items == sorted {
         cov_mark::hit!(not_applicable_if_sorted);
         return None;
     }
 
     let target = items.syntax().text_range();
     acc.add(
-        AssistId("reorder_impl", AssistKind::RefactorRewrite),
-        "Sort methods by trait definition",
+        AssistId("reorder_impl_items", AssistKind::RefactorRewrite),
+        "Sort items by trait definition",
         target,
         |builder| {
-            let methods = methods.into_iter().map(|fn_| builder.make_mut(fn_)).collect::<Vec<_>>();
-            methods
+            let assoc_items =
+                assoc_items.into_iter().map(|item| builder.make_mut(item)).collect::<Vec<_>>();
+            assoc_items
                 .into_iter()
                 .zip(sorted)
                 .for_each(|(old, new)| ted::replace(old.syntax(), new.clone_for_update().syntax()));
@@ -85,18 +93,15 @@ pub(crate) fn reorder_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
     )
 }
 
-fn compute_method_ranks(path: &ast::Path, ctx: &AssistContext) -> Option<FxHashMap<String, usize>> {
+fn compute_item_ranks(path: &ast::Path, ctx: &AssistContext) -> Option<FxHashMap<String, usize>> {
     let td = trait_definition(path, &ctx.sema)?;
 
     Some(
         td.items(ctx.db())
             .iter()
-            .flat_map(|i| match i {
-                hir::AssocItem::Function(f) => Some(f),
-                _ => None,
-            })
+            .flat_map(|i| i.name(ctx.db()))
             .enumerate()
-            .map(|(idx, func)| (func.name(ctx.db()).to_string(), idx))
+            .map(|(idx, name)| (name.to_string(), idx))
             .collect(),
     )
 }
@@ -118,15 +123,19 @@ mod tests {
     fn not_applicable_if_sorted() {
         cov_mark::check!(not_applicable_if_sorted);
         check_assist_not_applicable(
-            reorder_impl,
+            reorder_impl_items,
             r#"
 trait Bar {
+    type T;
+    const C: ();
     fn a() {}
     fn z() {}
     fn b() {}
 }
 struct Foo;
 $0impl Bar for Foo {
+    type T = ();
+    const C: () = ();
     fn a() {}
     fn z() {}
     fn b() {}
@@ -135,10 +144,49 @@ $0impl Bar for Foo {
         )
     }
 
+    #[test]
+    fn reorder_impl_trait_functions() {
+        check_assist(
+            reorder_impl_items,
+            r#"
+trait Bar {
+    fn a() {}
+    fn c() {}
+    fn b() {}
+    fn d() {}
+}
+
+struct Foo;
+$0impl Bar for Foo {
+    fn d() {}
+    fn b() {}
+    fn c() {}
+    fn a() {}
+}
+"#,
+            r#"
+trait Bar {
+    fn a() {}
+    fn c() {}
+    fn b() {}
+    fn d() {}
+}
+
+struct Foo;
+impl Bar for Foo {
+    fn a() {}
+    fn c() {}
+    fn b() {}
+    fn d() {}
+}
+"#,
+        )
+    }
+
     #[test]
     fn not_applicable_if_empty() {
         check_assist_not_applicable(
-            reorder_impl,
+            reorder_impl_items,
             r#"
 trait Bar {};
 struct Foo;
@@ -148,69 +196,85 @@ $0impl Bar for Foo {}
     }
 
     #[test]
-    fn reorder_impl_trait_functions() {
+    fn reorder_impl_trait_items() {
         check_assist(
-            reorder_impl,
+            reorder_impl_items,
             r#"
 trait Bar {
     fn a() {}
+    type T0;
     fn c() {}
+    const C1: ();
     fn b() {}
+    type T1;
     fn d() {}
+    const C0: ();
 }
 
 struct Foo;
 $0impl Bar for Foo {
+    type T1 = ();
     fn d() {}
     fn b() {}
     fn c() {}
+    const C1: () = ();
     fn a() {}
+    type T0 = ();
+    const C0: () = ();
 }
         "#,
             r#"
 trait Bar {
     fn a() {}
+    type T0;
     fn c() {}
+    const C1: ();
     fn b() {}
+    type T1;
     fn d() {}
+    const C0: ();
 }
 
 struct Foo;
 impl Bar for Foo {
     fn a() {}
+    type T0 = ();
     fn c() {}
+    const C1: () = ();
     fn b() {}
+    type T1 = ();
     fn d() {}
+    const C0: () = ();
 }
         "#,
         )
     }
 
     #[test]
-    fn reorder_impl_trait_methods_uneven_ident_lengths() {
+    fn reorder_impl_trait_items_uneven_ident_lengths() {
         check_assist(
-            reorder_impl,
+            reorder_impl_items,
             r#"
 trait Bar {
-    fn foo(&mut self) {}
-    fn fooo(&mut self) {}
+    type Foo;
+    type Fooo;
 }
 
 struct Foo;
 impl Bar for Foo {
-    fn fooo(&mut self) {}
-    fn foo(&mut self) {$0}
+    type Fooo = ();
+    type Foo = ();$0
 }"#,
             r#"
 trait Bar {
-    fn foo(&mut self) {}
-    fn fooo(&mut self) {}
+    type Foo;
+    type Fooo;
 }
 
 struct Foo;
 impl Bar for Foo {
-    fn foo(&mut self) {}
-    fn fooo(&mut self) {}
+    type Foo = ();
+    type Fooo = ();
 }"#,
         )
     }
diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs
index ef4aa1c62b..42bbc70b53 100644
--- a/crates/ide-assists/src/lib.rs
+++ b/crates/ide-assists/src/lib.rs
@@ -37,7 +37,7 @@
 //!   should be available more or less everywhere, which isn't useful. So
 //!   instead we only show it if the user *selects* the items they want to sort.
 //! * Consider grouping related assists together (see [`Assists::add_group`]).
-//! * Make assists robust. If the assist depends on results of type-inference to
+//! * Make assists robust. If the assist depends on results of type-inference too
 //!   much, it might only fire in fully-correct code. This makes assist less
 //!   useful and (worse) less predictable. The user should have a clear
 //!   intuition when each particular assist is available.
@@ -54,7 +54,6 @@
 //!   something. If something *could* be a diagnostic, it should be a
 //!   diagnostic. Conversely, it might be valuable to turn a diagnostic with a
 //!   lot of false errors into an assist.
-//! *
 //!
 //! See also this post:
 //! <https://rust-analyzer.github.io/blog/2020/09/28/how-to-make-a-light-bulb.html>
@@ -170,7 +169,7 @@ mod handlers {
     mod remove_mut;
     mod remove_unused_param;
     mod reorder_fields;
-    mod reorder_impl;
+    mod reorder_impl_items;
     mod replace_try_expr_with_match;
     mod replace_derive_with_manual_impl;
     mod replace_if_let_with_match;
@@ -257,7 +256,7 @@ mod handlers {
             remove_mut::remove_mut,
             remove_unused_param::remove_unused_param,
             reorder_fields::reorder_fields,
-            reorder_impl::reorder_impl,
+            reorder_impl_items::reorder_impl_items,
             replace_try_expr_with_match::replace_try_expr_with_match,
             replace_derive_with_manual_impl::replace_derive_with_manual_impl,
             replace_if_let_with_match::replace_if_let_with_match,


Test patch:
diff --git a/crates/hir-ty/src/tests/traits.rs b/crates/hir-ty/src/tests/traits.rs
index 0b08aa4711..5e58d5ad83 100644
--- a/crates/hir-ty/src/tests/traits.rs
+++ b/crates/hir-ty/src/tests/traits.rs
@@ -1255,32 +1255,6 @@ fn test() {
     );
 }
 
-#[test]
-fn infer_from_return_pos_impl_trait() {
-    check_infer_with_mismatches(
-        r#"
-//- minicore: fn, sized
-trait Trait<T> {}
-struct Bar<T>(T);
-impl<T> Trait<T> for Bar<T> {}
-fn foo<const C: u8, T>() -> (impl FnOnce(&str, T), impl Trait<u8>) {
-    (|input, t| {}, Bar(C))
-}
-"#,
-        expect![[r#"
-            134..165 '{     ...(C)) }': (|&str, T| -> (), Bar<u8>)
-            140..163 '(|inpu...ar(C))': (|&str, T| -> (), Bar<u8>)
-            141..154 '|input, t| {}': |&str, T| -> ()
-            142..147 'input': &str
-            149..150 't': T
-            152..154 '{}': ()
-            156..159 'Bar': Bar<u8>(u8) -> Bar<u8>
-            156..162 'Bar(C)': Bar<u8>
-            160..161 'C': u8
-        "#]],
-    );
-}
-
 #[test]
 fn dyn_trait() {
     check_infer(
@@ -2418,7 +2392,7 @@ fn test() -> impl Trait<i32> {
             171..182 '{ loop {} }': T
             173..180 'loop {}': !
             178..180 '{}': ()
-            213..309 '{     ...t()) }': S<i32>
+            213..309 '{     ...t()) }': S<{unknown}>
             223..225 's1': S<u32>
             228..229 'S': S<u32>(u32) -> S<u32>
             228..240 'S(default())': S<u32>
@@ -2434,10 +2408,10 @@ fn test() -> impl Trait<i32> {
             276..288 'S(default())': S<i32>
             278..285 'default': fn default<i32>() -> i32
             278..287 'default()': i32
-            295..296 'S': S<i32>(i32) -> S<i32>
-            295..307 'S(default())': S<i32>
-            297..304 'default': fn default<i32>() -> i32
-            297..306 'default()': i32
+            295..296 'S': S<{unknown}>({unknown}) -> S<{unknown}>
+            295..307 'S(default())': S<{unknown}>
+            297..304 'default': fn default<{unknown}>() -> {unknown}
+            297..306 'default()': {unknown}
         "#]],
     );
 }
diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs
index 8a1e95d894..eed50a8562 100644
--- a/crates/ide-assists/src/tests/generated.rs
+++ b/crates/ide-assists/src/tests/generated.rs
@@ -1737,34 +1737,34 @@ const test: Foo = Foo {foo: 1, bar: 0}
 }
 
 #[test]
-fn doctest_reorder_impl() {
+fn doctest_reorder_impl_items() {
     check_doc_test(
-        "reorder_impl",
+        "reorder_impl_items",
         r#####"
 trait Foo {
-    fn a() {}
-    fn b() {}
-    fn c() {}
+    type A;
+    const B: u8;
+    fn c();
 }
 
 struct Bar;
 $0impl Foo for Bar {
-    fn b() {}
+    const B: u8 = 17;
     fn c() {}
-    fn a() {}
+    type A = String;
 }
 "#####,
         r#####"
 trait Foo {
-    fn a() {}
-    fn b() {}
-    fn c() {}
+    type A;
+    const B: u8;
+    fn c();
 }
 
 struct Bar;
 impl Foo for Bar {
-    fn a() {}
-    fn b() {}
+    type A = String;
+    const B: u8 = 17;
     fn c() {}
 }
 "#####,