From 730a927c5e6b382690e88f482a03701636242a2c Mon Sep 17 00:00:00 2001 From: Geoffrey Copin Date: Fri, 10 Apr 2020 00:35:43 +0200 Subject: Implement assist "Reorder field names" --- crates/ra_assists/src/handlers/reorder_fields.rs | 207 +++++++++++++++++++++++ crates/ra_assists/src/lib.rs | 2 + crates/ra_hir_def/src/body/lower.rs | 3 +- crates/ra_syntax/src/ast/generated/nodes.rs | 8 +- xtask/src/ast_src.rs | 1 + 5 files changed, 219 insertions(+), 2 deletions(-) create mode 100644 crates/ra_assists/src/handlers/reorder_fields.rs diff --git a/crates/ra_assists/src/handlers/reorder_fields.rs b/crates/ra_assists/src/handlers/reorder_fields.rs new file mode 100644 index 000000000..c17c1288c --- /dev/null +++ b/crates/ra_assists/src/handlers/reorder_fields.rs @@ -0,0 +1,207 @@ +use std::collections::HashMap; + +use itertools::Itertools; + +use hir::{Adt, ModuleDef, PathResolution, Semantics, Struct}; +use ra_ide_db::RootDatabase; +use ra_syntax::ast::{Name, Pat}; +use ra_syntax::{ + ast, + ast::{Path, RecordField, RecordLit, RecordPat}, + AstNode, +}; + +use crate::{ + assist_ctx::{Assist, AssistCtx}, + AssistId, +}; + +pub(crate) fn reorder_fields(ctx: AssistCtx) -> Option { + reorder_struct(ctx.clone()).or_else(|| reorder_struct_pat(ctx)) +} + +fn reorder_struct(ctx: AssistCtx) -> Option { + let record: RecordLit = ctx.find_node_at_offset()?; + reorder(ctx, &record, &record.path()?, field_name) +} + +fn field_name(r: &RecordField) -> String { + r.name_ref() + .map(|name| name.syntax().text()) + .unwrap_or_else(|| r.expr().unwrap().syntax().text()) + .to_string() +} + +fn reorder_struct_pat(ctx: AssistCtx) -> Option { + let record: RecordPat = ctx.find_node_at_offset()?; + reorder(ctx, &record, &record.path()?, field_pat_name) +} + +fn field_pat_name(field: &Pat) -> String { + field.syntax().children().find_map(Name::cast).map(|n| n.to_string()).unwrap_or_default() +} + +fn reorder( + ctx: AssistCtx, + record: &R, + path: &Path, + field_name: fn(&F) -> String, +) -> Option { + let ranks = compute_fields_ranks(path, &ctx)?; + let fields: Vec = get_fields(record); + let sorted_fields: Vec = + sort_by_rank(&fields, |f| *ranks.get(&field_name(f)).unwrap_or(&usize::max_value())); + + if sorted_fields == fields { + return None; + } + + ctx.add_assist(AssistId("reorder_fields"), "Reorder record fields", |edit| { + for (old, new) in fields.into_iter().zip(sorted_fields) { + edit.replace_ast(old, new); + } + edit.target(record.syntax().text_range()) + }) +} + +fn get_fields(record: &R) -> Vec { + record.syntax().children().flat_map(|n1| n1.children()).filter_map(|n3| F::cast(n3)).collect() +} + +fn sort_by_rank(fields: &[F], get_rank: impl FnMut(&F) -> usize) -> Vec { + fields.iter().cloned().sorted_by_key(get_rank).collect() +} + +fn struct_definition(path: &ast::Path, sema: &Semantics) -> Option { + match sema.resolve_path(path) { + Some(PathResolution::Def(ModuleDef::Adt(Adt::Struct(s)))) => Some(s), + _ => None, + } +} + +fn compute_fields_ranks(path: &Path, ctx: &AssistCtx) -> Option> { + Some( + struct_definition(path, ctx.sema)? + .fields(ctx.db) + .iter() + .enumerate() + .map(|(idx, field)| (field.name(ctx.db).to_string(), idx)) + .collect(), + ) +} + +#[cfg(test)] +mod tests { + use crate::helpers::{check_assist, check_assist_not_applicable}; + + use super::*; + + #[test] + fn not_applicable_if_sorted() { + check_assist_not_applicable( + reorder_fields, + r#" + struct Foo { + foo: i32, + bar: i32, + } + + const test: Foo = <|>Foo { foo: 0, bar: 0 }; + "#, + ) + } + + #[test] + fn trivial_empty_fields() { + check_assist_not_applicable( + reorder_fields, + r#" + struct Foo {}; + const test: Foo = <|>Foo {} + "#, + ) + } + + #[test] + fn reorder_struct_fields() { + check_assist( + reorder_fields, + r#" + struct Foo {foo: i32, bar: i32}; + const test: Foo = <|>Foo {bar: 0, foo: 1} + "#, + r#" + struct Foo {foo: i32, bar: i32}; + const test: Foo = <|>Foo {foo: 1, bar: 0} + "#, + ) + } + + #[test] + fn reorder_struct_pattern() { + check_assist( + reorder_fields, + r#" + struct Foo { foo: i64, bar: i64, baz: i64 } + + fn f(f: Foo) -> { + match f { + <|>Foo { baz: 0, ref mut bar, .. } => (), + _ => () + } + } + "#, + r#" + struct Foo { foo: i64, bar: i64, baz: i64 } + + fn f(f: Foo) -> { + match f { + <|>Foo { ref mut bar, baz: 0, .. } => (), + _ => () + } + } + "#, + ) + } + + #[test] + fn reorder_with_extra_field() { + check_assist( + reorder_fields, + r#" + struct Foo { + foo: String, + bar: String, + } + + impl Foo { + fn new() -> Foo { + let foo = String::new(); + <|>Foo { + bar: foo.clone(), + extra: "Extra field", + foo, + } + } + } + "#, + r#" + struct Foo { + foo: String, + bar: String, + } + + impl Foo { + fn new() -> Foo { + let foo = String::new(); + <|>Foo { + foo, + bar: foo.clone(), + extra: "Extra field", + } + } + } + "#, + ) + } +} diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 5ba5254fd..a00136da1 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -129,6 +129,7 @@ mod handlers { mod replace_unwrap_with_match; mod split_import; mod add_from_impl_for_enum; + mod reorder_fields; pub(crate) fn all() -> &'static [AssistHandler] { &[ @@ -170,6 +171,7 @@ mod handlers { // These are manually sorted for better priorities add_missing_impl_members::add_missing_impl_members, add_missing_impl_members::add_missing_default_members, + reorder_fields::reorder_fields, ] } } diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index b0d71eb3d..80492b733 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -689,9 +689,10 @@ impl ExprCollector<'_> { Pat::Missing } } - // FIXME: implement ast::Pat::BoxPat(_) | ast::Pat::RangePat(_) | ast::Pat::MacroPat(_) => Pat::Missing, + // FIXME: implement + ast::Pat::RecordFieldPat(_) => Pat::Missing, }; let ptr = AstPtr::new(&pat); self.alloc_pat(pattern, Either::Left(ptr)) diff --git a/crates/ra_syntax/src/ast/generated/nodes.rs b/crates/ra_syntax/src/ast/generated/nodes.rs index 20f663046..79b225622 100644 --- a/crates/ra_syntax/src/ast/generated/nodes.rs +++ b/crates/ra_syntax/src/ast/generated/nodes.rs @@ -3256,6 +3256,7 @@ pub enum Pat { RangePat(RangePat), LiteralPat(LiteralPat), MacroPat(MacroPat), + RecordFieldPat(RecordFieldPat), } impl From for Pat { fn from(node: OrPat) -> Pat { Pat::OrPat(node) } @@ -3302,12 +3303,15 @@ impl From for Pat { impl From for Pat { fn from(node: MacroPat) -> Pat { Pat::MacroPat(node) } } +impl From for Pat { + fn from(node: RecordFieldPat) -> Pat { Pat::RecordFieldPat(node) } +} impl AstNode for Pat { fn can_cast(kind: SyntaxKind) -> bool { match kind { OR_PAT | PAREN_PAT | REF_PAT | BOX_PAT | BIND_PAT | PLACEHOLDER_PAT | DOT_DOT_PAT | PATH_PAT | RECORD_PAT | TUPLE_STRUCT_PAT | TUPLE_PAT | SLICE_PAT | RANGE_PAT - | LITERAL_PAT | MACRO_PAT => true, + | LITERAL_PAT | MACRO_PAT | RECORD_FIELD_PAT => true, _ => false, } } @@ -3328,6 +3332,7 @@ impl AstNode for Pat { RANGE_PAT => Pat::RangePat(RangePat { syntax }), LITERAL_PAT => Pat::LiteralPat(LiteralPat { syntax }), MACRO_PAT => Pat::MacroPat(MacroPat { syntax }), + RECORD_FIELD_PAT => Pat::RecordFieldPat(RecordFieldPat { syntax }), _ => return None, }; Some(res) @@ -3349,6 +3354,7 @@ impl AstNode for Pat { Pat::RangePat(it) => &it.syntax, Pat::LiteralPat(it) => &it.syntax, Pat::MacroPat(it) => &it.syntax, + Pat::RecordFieldPat(it) => &it.syntax, } } } diff --git a/xtask/src/ast_src.rs b/xtask/src/ast_src.rs index bb97b13fe..eb5c3abf7 100644 --- a/xtask/src/ast_src.rs +++ b/xtask/src/ast_src.rs @@ -741,6 +741,7 @@ pub(crate) const AST_SRC: AstSrc = AstSrc { RangePat, LiteralPat, MacroPat, + RecordFieldPat, } enum RecordInnerPat { -- cgit v1.2.3 From 21443f1b4839872841d8f32e87209f2106d43ad8 Mon Sep 17 00:00:00 2001 From: Geoffrey Copin Date: Sat, 11 Apr 2020 12:58:20 +0200 Subject: Remove Option unwraping --- crates/ra_assists/src/handlers/reorder_fields.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ra_assists/src/handlers/reorder_fields.rs b/crates/ra_assists/src/handlers/reorder_fields.rs index c17c1288c..1da85fcec 100644 --- a/crates/ra_assists/src/handlers/reorder_fields.rs +++ b/crates/ra_assists/src/handlers/reorder_fields.rs @@ -27,9 +27,9 @@ fn reorder_struct(ctx: AssistCtx) -> Option { fn field_name(r: &RecordField) -> String { r.name_ref() - .map(|name| name.syntax().text()) - .unwrap_or_else(|| r.expr().unwrap().syntax().text()) - .to_string() + .map(|name| name.syntax().text().to_string()) + .or_else(|| r.expr().map(|e| e.syntax().text().to_string())) + .unwrap_or_default() } fn reorder_struct_pat(ctx: AssistCtx) -> Option { -- cgit v1.2.3 From 270bcfdfc226bcdfffd7b8aa903073929fcef5f2 Mon Sep 17 00:00:00 2001 From: Geoffrey Copin Date: Sat, 11 Apr 2020 17:04:25 +0200 Subject: Avoid adding a RecordFieldPat variant to the Pat enum --- crates/ra_assists/src/handlers/reorder_fields.rs | 86 +++++++++++++----------- crates/ra_hir_def/src/body/lower.rs | 2 - crates/ra_syntax/src/ast/generated/nodes.rs | 8 +-- xtask/src/ast_src.rs | 1 - 4 files changed, 48 insertions(+), 49 deletions(-) diff --git a/crates/ra_assists/src/handlers/reorder_fields.rs b/crates/ra_assists/src/handlers/reorder_fields.rs index 1da85fcec..100e1feb1 100644 --- a/crates/ra_assists/src/handlers/reorder_fields.rs +++ b/crates/ra_assists/src/handlers/reorder_fields.rs @@ -4,71 +4,79 @@ use itertools::Itertools; use hir::{Adt, ModuleDef, PathResolution, Semantics, Struct}; use ra_ide_db::RootDatabase; -use ra_syntax::ast::{Name, Pat}; use ra_syntax::{ - ast, - ast::{Path, RecordField, RecordLit, RecordPat}, - AstNode, + algo, ast, + ast::{Name, Path, RecordLit, RecordPat}, + AstNode, SyntaxKind, SyntaxNode, }; use crate::{ assist_ctx::{Assist, AssistCtx}, AssistId, }; +use ra_syntax::ast::{Expr, NameRef}; pub(crate) fn reorder_fields(ctx: AssistCtx) -> Option { - reorder_struct(ctx.clone()).or_else(|| reorder_struct_pat(ctx)) + reorder::(ctx.clone()).or_else(|| reorder::(ctx)) } -fn reorder_struct(ctx: AssistCtx) -> Option { - let record: RecordLit = ctx.find_node_at_offset()?; - reorder(ctx, &record, &record.path()?, field_name) -} - -fn field_name(r: &RecordField) -> String { - r.name_ref() - .map(|name| name.syntax().text().to_string()) - .or_else(|| r.expr().map(|e| e.syntax().text().to_string())) - .unwrap_or_default() -} - -fn reorder_struct_pat(ctx: AssistCtx) -> Option { - let record: RecordPat = ctx.find_node_at_offset()?; - reorder(ctx, &record, &record.path()?, field_pat_name) -} +fn reorder(ctx: AssistCtx) -> Option { + let record = ctx.find_node_at_offset::()?; + let path = record.syntax().children().find_map(Path::cast)?; -fn field_pat_name(field: &Pat) -> String { - field.syntax().children().find_map(Name::cast).map(|n| n.to_string()).unwrap_or_default() -} + let ranks = compute_fields_ranks(&path, &ctx)?; -fn reorder( - ctx: AssistCtx, - record: &R, - path: &Path, - field_name: fn(&F) -> String, -) -> Option { - let ranks = compute_fields_ranks(path, &ctx)?; - let fields: Vec = get_fields(record); - let sorted_fields: Vec = - sort_by_rank(&fields, |f| *ranks.get(&field_name(f)).unwrap_or(&usize::max_value())); + 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()) + }); if sorted_fields == fields { return None; } ctx.add_assist(AssistId("reorder_fields"), "Reorder record fields", |edit| { - for (old, new) in fields.into_iter().zip(sorted_fields) { - edit.replace_ast(old, new); + for (old, new) in fields.iter().zip(&sorted_fields) { + algo::diff(old, new).into_text_edit(edit.text_edit_builder()); } edit.target(record.syntax().text_range()) }) } -fn get_fields(record: &R) -> Vec { - record.syntax().children().flat_map(|n1| n1.children()).filter_map(|n3| F::cast(n3)).collect() +fn get_fields_kind(node: &SyntaxNode) -> Vec { + use SyntaxKind::*; + match node.kind() { + RECORD_LIT => vec![RECORD_FIELD], + RECORD_PAT => vec![RECORD_FIELD_PAT, BIND_PAT], + _ => vec![], + } +} + +fn get_field_name(node: &SyntaxNode) -> String { + use SyntaxKind::*; + match node.kind() { + RECORD_FIELD => { + if let Some(name) = node.children().find_map(NameRef::cast) { + return name.to_string(); + } + node.children().find_map(Expr::cast).map(|expr| expr.to_string()).unwrap_or_default() + } + BIND_PAT | RECORD_FIELD_PAT => { + node.children().find_map(Name::cast).map(|n| n.to_string()).unwrap_or_default() + } + _ => String::new(), + } +} + +fn get_fields(record: &SyntaxNode) -> Vec { + let kinds = get_fields_kind(record); + record.children().flat_map(|n| n.children()).filter(|n| kinds.contains(&n.kind())).collect() } -fn sort_by_rank(fields: &[F], get_rank: impl FnMut(&F) -> usize) -> Vec { +fn sorted_by_rank( + fields: &[SyntaxNode], + get_rank: impl Fn(&SyntaxNode) -> usize, +) -> Vec { fields.iter().cloned().sorted_by_key(get_rank).collect() } diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index 80492b733..e1b08d48f 100644 --- a/crates/ra_hir_def/src/body/lower.rs +++ b/crates/ra_hir_def/src/body/lower.rs @@ -691,8 +691,6 @@ impl ExprCollector<'_> { } // FIXME: implement ast::Pat::BoxPat(_) | ast::Pat::RangePat(_) | ast::Pat::MacroPat(_) => Pat::Missing, - // FIXME: implement - ast::Pat::RecordFieldPat(_) => Pat::Missing, }; let ptr = AstPtr::new(&pat); self.alloc_pat(pattern, Either::Left(ptr)) diff --git a/crates/ra_syntax/src/ast/generated/nodes.rs b/crates/ra_syntax/src/ast/generated/nodes.rs index 79b225622..20f663046 100644 --- a/crates/ra_syntax/src/ast/generated/nodes.rs +++ b/crates/ra_syntax/src/ast/generated/nodes.rs @@ -3256,7 +3256,6 @@ pub enum Pat { RangePat(RangePat), LiteralPat(LiteralPat), MacroPat(MacroPat), - RecordFieldPat(RecordFieldPat), } impl From for Pat { fn from(node: OrPat) -> Pat { Pat::OrPat(node) } @@ -3303,15 +3302,12 @@ impl From for Pat { impl From for Pat { fn from(node: MacroPat) -> Pat { Pat::MacroPat(node) } } -impl From for Pat { - fn from(node: RecordFieldPat) -> Pat { Pat::RecordFieldPat(node) } -} impl AstNode for Pat { fn can_cast(kind: SyntaxKind) -> bool { match kind { OR_PAT | PAREN_PAT | REF_PAT | BOX_PAT | BIND_PAT | PLACEHOLDER_PAT | DOT_DOT_PAT | PATH_PAT | RECORD_PAT | TUPLE_STRUCT_PAT | TUPLE_PAT | SLICE_PAT | RANGE_PAT - | LITERAL_PAT | MACRO_PAT | RECORD_FIELD_PAT => true, + | LITERAL_PAT | MACRO_PAT => true, _ => false, } } @@ -3332,7 +3328,6 @@ impl AstNode for Pat { RANGE_PAT => Pat::RangePat(RangePat { syntax }), LITERAL_PAT => Pat::LiteralPat(LiteralPat { syntax }), MACRO_PAT => Pat::MacroPat(MacroPat { syntax }), - RECORD_FIELD_PAT => Pat::RecordFieldPat(RecordFieldPat { syntax }), _ => return None, }; Some(res) @@ -3354,7 +3349,6 @@ impl AstNode for Pat { Pat::RangePat(it) => &it.syntax, Pat::LiteralPat(it) => &it.syntax, Pat::MacroPat(it) => &it.syntax, - Pat::RecordFieldPat(it) => &it.syntax, } } } diff --git a/xtask/src/ast_src.rs b/xtask/src/ast_src.rs index eb5c3abf7..bb97b13fe 100644 --- a/xtask/src/ast_src.rs +++ b/xtask/src/ast_src.rs @@ -741,7 +741,6 @@ pub(crate) const AST_SRC: AstSrc = AstSrc { RangePat, LiteralPat, MacroPat, - RecordFieldPat, } enum RecordInnerPat { -- cgit v1.2.3 From 25a0ce2e9e59ca09665055c053d0008b7bbb9cb7 Mon Sep 17 00:00:00 2001 From: Geoffrey Copin Date: Sat, 11 Apr 2020 19:39:10 +0200 Subject: Add documentation comment --- crates/ra_assists/src/handlers/reorder_fields.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/ra_assists/src/handlers/reorder_fields.rs b/crates/ra_assists/src/handlers/reorder_fields.rs index 100e1feb1..661c1bf54 100644 --- a/crates/ra_assists/src/handlers/reorder_fields.rs +++ b/crates/ra_assists/src/handlers/reorder_fields.rs @@ -16,6 +16,22 @@ use crate::{ }; use ra_syntax::ast::{Expr, NameRef}; + +// Assist: reorder_fields +// +// Reorder the fields of record literals and record patterns in the same order as in +// the definition. +// +// ``` +// struct Foo {foo: i32, bar: i32}; +// const test: Foo = <|>Foo {bar: 0, foo: 1} +// ``` +// -> +// ``` +// struct Foo {foo: i32, bar: i32}; +// const test: Foo = <|>Foo {foo: 1, bar: 0} +// ``` +// pub(crate) fn reorder_fields(ctx: AssistCtx) -> Option { reorder::(ctx.clone()).or_else(|| reorder::(ctx)) } -- cgit v1.2.3 From d362fcfc1c52f40a64a329e90b3f213afea0e6f5 Mon Sep 17 00:00:00 2001 From: Geoffrey Copin Date: Sat, 11 Apr 2020 20:32:48 +0200 Subject: Fix doc tests --- crates/ra_assists/src/handlers/reorder_fields.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/ra_assists/src/handlers/reorder_fields.rs b/crates/ra_assists/src/handlers/reorder_fields.rs index 661c1bf54..692dd1315 100644 --- a/crates/ra_assists/src/handlers/reorder_fields.rs +++ b/crates/ra_assists/src/handlers/reorder_fields.rs @@ -16,7 +16,6 @@ use crate::{ }; use ra_syntax::ast::{Expr, NameRef}; - // Assist: reorder_fields // // Reorder the fields of record literals and record patterns in the same order as in @@ -29,7 +28,7 @@ use ra_syntax::ast::{Expr, NameRef}; // -> // ``` // struct Foo {foo: i32, bar: i32}; -// const test: Foo = <|>Foo {foo: 1, bar: 0} +// const test: Foo = Foo {foo: 1, bar: 0} // ``` // pub(crate) fn reorder_fields(ctx: AssistCtx) -> Option { -- cgit v1.2.3 From d9089245fefdc4179c1d61839e78131c5e5a5a45 Mon Sep 17 00:00:00 2001 From: Geoffrey Copin Date: Sat, 11 Apr 2020 20:32:58 +0200 Subject: Generate doc --- crates/ra_assists/src/doc_tests/generated.rs | 15 +++++++++++++++ docs/user/assists.md | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs index 64444ee3a..6dc049715 100644 --- a/crates/ra_assists/src/doc_tests/generated.rs +++ b/crates/ra_assists/src/doc_tests/generated.rs @@ -606,6 +606,21 @@ impl Walrus { ) } +#[test] +fn doctest_reorder_fields() { + check( + "reorder_fields", + r#####" +struct Foo {foo: i32, bar: i32}; +const test: Foo = <|>Foo {bar: 0, foo: 1} +"#####, + r#####" +struct Foo {foo: i32, bar: i32}; +const test: Foo = Foo {foo: 1, bar: 0} +"#####, + ) +} + #[test] fn doctest_replace_if_let_with_match() { check( diff --git a/docs/user/assists.md b/docs/user/assists.md index 754131f6f..75404e289 100644 --- a/docs/user/assists.md +++ b/docs/user/assists.md @@ -582,6 +582,21 @@ impl Walrus { } ``` +## `reorder_fields` + +Reorder the fields of record literals and record patterns in the same order as in +the definition. + +```rust +// BEFORE +struct Foo {foo: i32, bar: i32}; +const test: Foo = ┃Foo {bar: 0, foo: 1} + +// AFTER +struct Foo {foo: i32, bar: i32}; +const test: Foo = Foo {foo: 1, bar: 0} +``` + ## `replace_if_let_with_match` Replaces `if let` with an else branch with a `match` expression. -- cgit v1.2.3