Merge #8176

8176: Cleanup r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ide_assists/src/handlers/reorder_fields.rs

@@ -1,9 +1,6 @@
-use itertools::Itertools;
 use rustc_hash::FxHashMap;
 
-use hir::{Adt, ModuleDef, PathResolution, Semantics, Struct};
-use ide_db::RootDatabase;
-use syntax::{algo, ast, match_ast, AstNode, SyntaxKind, SyntaxKind::*, SyntaxNode};
+use syntax::{algo, ast, match_ast, AstNode, SyntaxKind::*, SyntaxNode};
 
 use crate::{AssistContext, AssistId, AssistKind, Assists};
 
@@ -23,26 +20,39 @@ use crate::{AssistContext, AssistId, AssistKind, Assists};
 // ```
 //
 pub(crate) fn reorder_fields(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
-    reorder::<ast::RecordExpr>(acc, ctx).or_else(|| reorder::<ast::RecordPat>(acc, ctx))
-}
+    let record = ctx
+        .find_node_at_offset::<ast::RecordExpr>()
+        .map(|it| it.syntax().clone())
+        .or_else(|| ctx.find_node_at_offset::<ast::RecordPat>().map(|it| it.syntax().clone()))?;
 
-fn reorder<R: AstNode>(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
-    let record = ctx.find_node_at_offset::<R>()?;
-    let path = record.syntax().children().find_map(ast::Path::cast)?;
+    let path = record.children().find_map(ast::Path::cast)?;
 
     let ranks = compute_fields_ranks(&path, &ctx)?;
 
-    let fields = get_fields(&record.syntax());
-    let sorted_fields = sorted_by_rank(&fields, |node| {
-        *ranks.get(&get_field_name(node)).unwrap_or(&usize::max_value())
-    });
+    let fields: Vec<SyntaxNode> = {
+        let field_kind = match record.kind() {
+            RECORD_EXPR => RECORD_EXPR_FIELD,
+            RECORD_PAT => RECORD_PAT_FIELD,
+            _ => {
+                stdx::never!();
+                return None;
+            }
+        };
+        record.children().flat_map(|n| n.children()).filter(|n| n.kind() == field_kind).collect()
+    };
+
+    let sorted_fields = {
+        let mut fields = fields.clone();
+        fields.sort_by_key(|node| *ranks.get(&get_field_name(node)).unwrap_or(&usize::max_value()));
+        fields
+    };
 
     if sorted_fields == fields {
         cov_mark::hit!(reorder_sorted_fields);
         return None;
     }
 
-    let target = record.syntax().text_range();
+    let target = record.text_range();
     acc.add(
         AssistId("reorder_fields", AssistKind::RefactorRewrite),
         "Reorder record fields",
@@ -57,14 +67,6 @@ fn reorder<R: AstNode>(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     )
 }
 
-fn get_fields_kind(node: &SyntaxNode) -> Vec<SyntaxKind> {
-    match node.kind() {
-        RECORD_EXPR => vec![RECORD_EXPR_FIELD],
-        RECORD_PAT => vec![RECORD_PAT_FIELD, IDENT_PAT],
-        _ => vec![],
-    }
-}
-
 fn get_field_name(node: &SyntaxNode) -> String {
     let res = match_ast! {
         match node {
@@ -76,34 +78,20 @@ fn get_field_name(node: &SyntaxNode) -> String {
     res.unwrap_or_default()
 }
 
-fn get_fields(record: &SyntaxNode) -> Vec<SyntaxNode> {
-    let kinds = get_fields_kind(record);
-    record.children().flat_map(|n| n.children()).filter(|n| kinds.contains(&n.kind())).collect()
-}
-
-fn sorted_by_rank(
-    fields: &[SyntaxNode],
-    get_rank: impl Fn(&SyntaxNode) -> usize,
-) -> Vec<SyntaxNode> {
-    fields.iter().cloned().sorted_by_key(get_rank).collect()
-}
-
-fn struct_definition(path: &ast::Path, sema: &Semantics<RootDatabase>) -> Option<Struct> {
-    match sema.resolve_path(path) {
-        Some(PathResolution::Def(ModuleDef::Adt(Adt::Struct(s)))) => Some(s),
-        _ => None,
-    }
-}
-
 fn compute_fields_ranks(path: &ast::Path, ctx: &AssistContext) -> Option<FxHashMap<String, usize>> {
-    Some(
-        struct_definition(path, &ctx.sema)?
-            .fields(ctx.db())
-            .iter()
-            .enumerate()
-            .map(|(idx, field)| (field.name(ctx.db()).to_string(), idx))
-            .collect(),
-    )
+    let strukt = match ctx.sema.resolve_path(path) {
+        Some(hir::PathResolution::Def(hir::ModuleDef::Adt(hir::Adt::Struct(it)))) => it,
+        _ => return None,
+    };
+
+    let res = strukt
+        .fields(ctx.db())
+        .iter()
+        .enumerate()
+        .map(|(idx, field)| (field.name(ctx.db()).to_string(), idx))
+        .collect();
+
+    Some(res)
 }
 
 #[cfg(test)]
@@ -118,11 +106,7 @@ mod tests {
         check_assist_not_applicable(
             reorder_fields,
             r#"
-struct Foo {
-    foo: i32,
-    bar: i32,
-}
-
+struct Foo { foo: i32, bar: i32 }
 const test: Foo = $0Foo { foo: 0, bar: 0 };
 "#,
         )
@@ -133,8 +117,8 @@ const test: Foo = $0Foo { foo: 0, bar: 0 };
         check_assist_not_applicable(
             reorder_fields,
             r#"
-struct Foo {};
-const test: Foo = $0Foo {}
+struct Foo {}
+const test: Foo = $0Foo {};
 "#,
         )
     }
@@ -144,12 +128,12 @@ const test: Foo = $0Foo {}
         check_assist(
             reorder_fields,
             r#"
-struct Foo {foo: i32, bar: i32};
-const test: Foo = $0Foo {bar: 0, foo: 1}
+struct Foo { foo: i32, bar: i32 }
+const test: Foo = $0Foo { bar: 0, foo: 1 };
 "#,
             r#"
-struct Foo {foo: i32, bar: i32};
-const test: Foo = Foo {foo: 1, bar: 0}
+struct Foo { foo: i32, bar: i32 }
+const test: Foo = Foo { foo: 1, bar: 0 };
 "#,
         )
     }
@@ -186,10 +170,7 @@ fn f(f: Foo) -> {
         check_assist(
             reorder_fields,
             r#"
-struct Foo {
-    foo: String,
-    bar: String,
-}
+struct Foo { foo: String, bar: String }
 
 impl Foo {
     fn new() -> Foo {
@@ -203,10 +184,7 @@ impl Foo {
 }
 "#,
             r#"
-struct Foo {
-    foo: String,
-    bar: String,
-}
+struct Foo { foo: String, bar: String }
 
 impl Foo {
     fn new() -> Foo {