From 478b2988c0715ae90018dd0930de06a507ecedf7 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Tue, 17 Aug 2021 19:22:57 +0200 Subject: [PATCH 01/12] Add Destructure Tuple --- .../src/handlers/destructure_tuple_binding.rs | 894 ++++++++++++++++++ crates/ide_assists/src/lib.rs | 2 + 2 files changed, 896 insertions(+) create mode 100644 crates/ide_assists/src/handlers/destructure_tuple_binding.rs diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs new file mode 100644 index 000000000000..4cc0ece3f40c --- /dev/null +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -0,0 +1,894 @@ +use ide_db::{ + assists::{AssistId, AssistKind}, + defs::Definition, + search::{FileReference, SearchScope, UsageSearchResult}, +}; +use itertools::Itertools; +use syntax::{ + ast::{self, AstNode, FieldExpr, IdentPat, NameOwner}, + SyntaxNode, TextRange, +}; + +use crate::assist_context::{AssistBuilder, AssistContext, Assists}; + +// Assist: destructure_tuple_binding +// +// Destructures a tuple binding into its items. +// +// ``` +// fn main() { +// let $0t = (1,2); +// let v = t.0; +// } +// ``` +// -> +// ``` +// fn main() { +// let (_0, _1) = (1,2); +// let v = _0; +// } +// ``` + +pub(crate) fn destructure_tuple_binding(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + // find variable pattern under cursor + let ident_pat = ctx.find_node_at_offset::()?; + let data = collect_data(ident_pat, ctx)?; + + acc.add( + AssistId("destructure_tuple", AssistKind::RefactorRewrite), + "Destructure tuple", + data.range, + |builder| { + edit_tuple_assignment(&data, builder, ctx); + edit_tuple_usages(&data, builder, ctx); + }, + ); + + Some(()) +} + +fn collect_data(ident_pat: IdentPat, ctx: &AssistContext) -> Option { + if ident_pat.at_token().is_some() { + // cannot destructure pattern with sub-pattern: + // Only IdentPat can have sub-pattern, + // but not TuplePat (`(a,b)`) + cov_mark::hit!(destructure_tuple_subpattern); + return None; + } + + let ty = ctx.sema.type_of_pat(&ident_pat.clone().into())?; + // might be reference + let ty = ty.strip_references(); + // must be tuple + let field_types = ty.tuple_fields(ctx.db()); + if field_types.is_empty() { + cov_mark::hit!(destructure_tuple_no_tuple); + return None; + } + + let name = ident_pat.name()?.to_string(); + let range = ident_pat.syntax().text_range(); + + let usages = ctx.sema.to_def(&ident_pat).map(|def| { + Definition::Local(def) + .usages(&ctx.sema) + .in_scope(SearchScope::single_file(ctx.frange.file_id)) + .all() + }); + + let field_names = (0..field_types.len()) + .map(|i| generate_name(i, &name, &ident_pat, &usages, ctx)) + .collect_vec(); + + Some(TupleData { range, field_names, usages }) +} + +fn generate_name( + index: usize, + _tuple_name: &str, + _ident_pat: &IdentPat, + _usages: &Option, + _ctx: &AssistContext, +) -> String { + //TODO: detect if name already used + format!("_{}", index) +} + +struct TupleData { + // ident_pat: IdentPat, + // name: String, + range: TextRange, + field_names: Vec, + // field_types: Vec, + usages: Option, +} +fn edit_tuple_assignment(data: &TupleData, builder: &mut AssistBuilder, ctx: &AssistContext) { + let new_tuple = { + let fields = data + .field_names + .iter() + .map(|name| ast::Pat::from(ast::make::ident_pat(false, false, ast::make::name(name)))); + ast::make::tuple_pat(fields) + }; + + match ctx.config.snippet_cap { + Some(cap) => { + // No support for placeholders in Code Actions, except for special rust analyzer handling which supports only first `$0` + // -> place cursor on first variable + let mut snip = new_tuple.to_string(); + snip.insert_str(1, "$0"); + builder.replace_snippet(cap, data.range, snip); + } + None => builder.replace(data.range, new_tuple.to_string()), + }; +} + +fn edit_tuple_usages( + data: &TupleData, + builder: &mut AssistBuilder, + ctx: &AssistContext, +) { + if let Some(usages) = data.usages.as_ref() { + for (file_id, refs) in usages.iter() { + builder.edit_file(*file_id); + + for r in refs { + edit_tuple_usage(r, data, builder, ctx); + } + } + } +} +fn edit_tuple_usage( + usage: &FileReference, + data: &TupleData, + builder: &mut AssistBuilder, + _ctx: &AssistContext, +) { + match detect_tuple_index(usage.name.syntax(), data) { + Some((expr, idx)) => { + // index field access + let text = &data.field_names[idx]; + let range = expr.syntax().text_range(); + builder.replace(range, text); + } + None => { + // no index access -> make invalid -> requires handling by user + // -> put usage in block comment + builder.insert(usage.range.start(), "/*"); + builder.insert(usage.range.end(), "*/"); + } + } +} + +fn detect_tuple_index(usage: &SyntaxNode, data: &TupleData) -> Option<(FieldExpr, usize)> { + // usage is IDENT + // IDENT + // NAME_REF + // PATH_SEGMENT + // PATH + // PATH_EXPR + // PAREN_EXRP* + // FIELD_EXPR + let node = usage + .ancestors() + .skip_while(|s| !ast::PathExpr::can_cast(s.kind())) + .skip(1) // PATH_EXPR + .find(|s| !ast::ParenExpr::can_cast(s.kind()))?; // skip parentheses + + if let Some(field_expr) = ast::FieldExpr::cast(node) { + let idx = field_expr.name_ref()?.as_tuple_field()?; + if idx < data.field_names.len() { + Some((field_expr, idx)) + } else { + // tuple index out of range + None + } + } else { + None + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use crate::tests::{check_assist, check_assist_not_applicable}; + + #[test] + fn dont_trigger_on_unit() { + cov_mark::check!(destructure_tuple_no_tuple); + check_assist_not_applicable( + destructure_tuple_binding, + r#" +fn main() { +let $0v = (); +} + "#, + ) + } + #[test] + fn dont_trigger_on_number() { + cov_mark::check!(destructure_tuple_no_tuple); + check_assist_not_applicable( + destructure_tuple_binding, + r#" +fn main() { +let $0v = 32; +} + "#, + ) + } + + #[test] + fn destructure_3_tuple() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let $0tup = (1,2,3); +} + "#, + r#" +fn main() { + let ($0_0, _1, _2) = (1,2,3); +} + "#, + ) + } + #[test] + fn destructure_2_tuple() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let $0tup = (1,2); +} + "#, + r#" +fn main() { + let ($0_0, _1) = (1,2); +} + "#, + ) + } + #[test] + fn replace_indices() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let $0tup = (1,2,3); + let v1 = tup.0; + let v2 = tup.1; + let v3 = tup.2; +} + "#, + r#" +fn main() { + let ($0_0, _1, _2) = (1,2,3); + let v1 = _0; + let v2 = _1; + let v3 = _2; +} + "#, + ) + } + + #[test] + fn replace_usage_in_parentheses() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let $0tup = (1,2,3); + let a = (tup).1; + let b = ((tup)).1; +} + "#, + r#" +fn main() { + let ($0_0, _1, _2) = (1,2,3); + let a = _1; + let b = _1; +} + "#, + ) + } + + #[test] + fn handle_function_call() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let $0tup = (1,2); + let v = tup.into(); +} + "#, + r#" +fn main() { + let ($0_0, _1) = (1,2); + let v = /*tup*/.into(); +} + "#, + ) + } + + #[test] + fn handle_invalid_index() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let $0tup = (1,2); + let v = tup.3; +} + "#, + r#" +fn main() { + let ($0_0, _1) = (1,2); + let v = /*tup*/.3; +} + "#, + ) + } + + #[test] + fn dont_replace_variable_with_same_name_as_tuple() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let tup = (1,2); + let v = tup.1; + let $0tup = (1,2,3); + let v = tup.1; + let tup = (1,2,3); + let v = tup.1; +} + "#, + r#" +fn main() { + let tup = (1,2); + let v = tup.1; + let ($0_0, _1, _2) = (1,2,3); + let v = _1; + let tup = (1,2,3); + let v = tup.1; +} + "#, + ) + } + + #[test] + fn keep_function_call_in_tuple_item() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let $0t = ("3.14", 0); + let pi: f32 = t.0.parse().unwrap(); +} + "#, + r#" +fn main() { + let ($0_0, _1) = ("3.14", 0); + let pi: f32 = _0.parse().unwrap(); +} + "#, + ) + } + + #[test] + fn keep_type() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let $0t: (usize, i32) = (1,2); +} + "#, + r#" +fn main() { + let ($0_0, _1): (usize, i32) = (1,2); +} + "#, + ) + } + + #[test] + fn destructure_reference() { + //Note: `v` has different types: + // * in 1st: `i32` + // * in 2nd: `&i32` + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let t = (1,2); + let $0t = &t; + let v = t.0; +} + "#, + r#" +fn main() { + let t = (1,2); + let ($0_0, _1) = &t; + let v = _0; +} + "#, + ) + } + + #[test] + fn destructure_multiple_reference() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let t = (1,2); + let $0t = &&t; + let v = t.0; +} + "#, + r#" +fn main() { + let t = (1,2); + let ($0_0, _1) = &&t; + let v = _0; +} + "#, + ) + } + + #[test] + fn keep_reference() { + check_assist( + destructure_tuple_binding, + r#" +fn foo(t: &(usize, usize)) -> usize { + match t { + &$0t => t.0 + } +} + "#, + r#" +fn foo(t: &(usize, usize)) -> usize { + match t { + &($0_0, _1) => _0 + } +} + "#, + ) + } + + #[test] + fn dont_trigger_for_non_tuple_reference() { + check_assist_not_applicable( + destructure_tuple_binding, + r#" +fn main() { + let v = 42; + let $0v = &42; +} + "#, + ) + } + + #[test] + fn dont_trigger_on_static_tuple() { + check_assist_not_applicable( + destructure_tuple_binding, + r#" +static $0TUP: (usize, usize) = (1,2); + "#, + ) + } + + #[test] + fn dont_trigger_on_wildcard() { + check_assist_not_applicable( + destructure_tuple_binding, + r#" +fn main() { + let $0_ = (1,2); +} + "#, + ) + } + + #[test] + fn dont_trigger_in_struct() { + check_assist_not_applicable( + destructure_tuple_binding, + r#" +struct S { + $0tup: (usize, usize), +} + "#, + ) + } + + #[test] + fn dont_trigger_in_struct_creation() { + check_assist_not_applicable( + destructure_tuple_binding, + r#" +struct S { + tup: (usize, usize), +} +fn main() { + let s = S { + $0tup: (1,2), + }; +} + "#, + ) + } + + #[test] + fn dont_trigger_on_tuple_struct() { + check_assist_not_applicable( + destructure_tuple_binding, + r#" +struct S(usize, usize); +fn main() { + let $0s = S(1,2); +} + "#, + ) + } + + #[test] + fn dont_trigger_when_subpattern_exists() { + // sub-pattern is only allowed with IdentPat (name), not other patterns (like TuplePat) + cov_mark::check!(destructure_tuple_subpattern); + check_assist_not_applicable( + destructure_tuple_binding, + r#" +fn sum(t: (usize, usize)) -> usize { + match t { + $0t @ (1..=3,1..=3) => t.0 + t.1, + _ => 0, + } +} + "#, + ) + } + + #[test] + fn in_subpattern() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let t1 @ (_, $0t2) = (1, (2,3)); + let v = t1.0 + t2.0 + t2.1; +} + "#, + r#" +fn main() { + let t1 @ (_, ($0_0, _1)) = (1, (2,3)); + let v = t1.0 + _0 + _1; +} + "#, + ) + } + + #[test] + fn in_nested_tuple() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let ($0tup, v) = ((1,2),3); +} + "#, + r#" +fn main() { + let (($0_0, _1), v) = ((1,2),3); +} + "#, + ) + } + + #[test] + fn in_closure() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let $0tup = (1,2,3); + let f = |v| v + tup.1; +} + "#, + r#" +fn main() { + let ($0_0, _1, _2) = (1,2,3); + let f = |v| v + _1; +} + "#, + ) + } + + #[test] + fn in_closure_args() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let f = |$0t| t.0 + t.1; + let v = f((1,2)); +} + "#, + r#" +fn main() { + let f = |($0_0, _1)| _0 + _1; + let v = f((1,2)); +} + "#, + ) + } + + #[test] + fn in_function_args() { + check_assist( + destructure_tuple_binding, + r#" +fn f($0t: (usize, usize)) { + let v = t.0; +} + "#, + r#" +fn f(($0_0, _1): (usize, usize)) { + let v = _0; +} + "#, + ) + } + + #[test] + fn in_if_let() { + check_assist( + destructure_tuple_binding, + r#" +fn f(t: (usize, usize)) { + if let $0t = t { + let v = t.0; + } +} + "#, + r#" +fn f(t: (usize, usize)) { + if let ($0_0, _1) = t { + let v = _0; + } +} + "#, + ) + } + #[test] + fn in_if_let_option() { + check_assist( + destructure_tuple_binding, + r#" +//- minicore: option +fn f(o: Option<(usize, usize)>) { + if let Some($0t) = o { + let v = t.0; + } +} + "#, + r#" +fn f(o: Option<(usize, usize)>) { + if let Some(($0_0, _1)) = o { + let v = _0; + } +} + "#, + ) + } + + + #[test] + fn in_match() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + match (1,2) { + $0t => t.1, + }; +} + "#, + r#" +fn main() { + match (1,2) { + ($0_0, _1) => _1, + }; +} + "#, + ) + } + #[test] + fn in_match_option() { + check_assist( + destructure_tuple_binding, + r#" +//- minicore: option +fn main() { + match Some((1,2)) { + Some($0t) => t.1, + _ => 0, + }; +} + "#, + r#" +fn main() { + match Some((1,2)) { + Some(($0_0, _1)) => _1, + _ => 0, + }; +} + "#, + ) + } + + #[test] + fn in_for() { + check_assist( + destructure_tuple_binding, + r#" +//- minicore: iterators +fn main() { + for $0t in core::iter::repeat((1,2)) { + let v = t.1; + } +} + "#, + r#" +fn main() { + for ($0_0, _1) in core::iter::repeat((1,2)) { + let v = _1; + } +} + "#, + ) + } + #[test] + fn in_for_nested() { + check_assist( + destructure_tuple_binding, + r#" +//- minicore: iterators +fn main() { + for (a, $0b) in core::iter::repeat((1,(2,3))) { + let v = b.1; + } +} + "#, + r#" +fn main() { + for (a, ($0_0, _1)) in core::iter::repeat((1,(2,3))) { + let v = _1; + } +} + "#, + ) + } + + #[test] + fn not_applicable_on_tuple_usage() { + //Improvement: might be reasonable to allow & implement + check_assist_not_applicable( + destructure_tuple_binding, + r#" +fn main() { + let t = (1,2); + let v = $0t.0; +} + "#, + ) + } + + #[test] + fn replace_all() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let $0t = (1,2); + let v = t.1; + let s = (t.0 + t.1) / 2; + let f = |v| v + t.0; + let r = f(t.1); + let e = t == (9,0); + let m = + match t { + (_,2) if t.0 > 2 => 1, + _ => 0, + }; +} + "#, + r#" +fn main() { + let ($0_0, _1) = (1,2); + let v = _1; + let s = (_0 + _1) / 2; + let f = |v| v + _0; + let r = f(_1); + let e = /*t*/ == (9,0); + let m = + match /*t*/ { + (_,2) if _0 > 2 => 1, + _ => 0, + }; +} + "#, + ) + } + + #[test] + fn non_trivial_tuple_assignment() { + check_assist( + destructure_tuple_binding, + r#" +fn main { + let $0t = + if 1 > 2 { + (1,2) + } else { + (5,6) + }; + let v1 = t.0; + let v2 = + if t.0 > t.1 { + t.0 - t.1 + } else { + t.1 - t.0 + }; +} + "#, + r#" +fn main { + let ($0_0, _1) = + if 1 > 2 { + (1,2) + } else { + (5,6) + }; + let v1 = _0; + let v2 = + if _0 > _1 { + _0 - _1 + } else { + _1 - _0 + }; +} + "#, + ) + } + + // ----------------------- + #[test] + #[ignore] + fn replace_in_macro_call() { + // doesn't work: cannot find usage in `println!` + // Note: Just `t` works, but not `t.0` + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let $0t = (1,2); + println!("{}", t.0); +} + "#, + r#" +fn main() { + let ($0_0, _1) = (1,2); + println!("{}", _0); +} + "#, + ) + } +} diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 98a9085a51ed..7c074a4f6321 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -62,6 +62,7 @@ mod handlers { mod convert_iter_for_each_to_for; mod convert_tuple_struct_to_named_struct; mod convert_to_guarded_return; + mod destructure_tuple_binding; mod expand_glob_import; mod extract_function; mod extract_struct_from_enum_variant; @@ -134,6 +135,7 @@ mod handlers { convert_iter_for_each_to_for::convert_iter_for_each_to_for, convert_to_guarded_return::convert_to_guarded_return, convert_tuple_struct_to_named_struct::convert_tuple_struct_to_named_struct, + destructure_tuple_binding::destructure_tuple_binding, expand_glob_import::expand_glob_import, extract_struct_from_enum_variant::extract_struct_from_enum_variant, extract_type_alias::extract_type_alias, From 3e4051938d5d14fd72498a2d3e515b11437d73c9 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Wed, 4 Aug 2021 21:16:53 +0200 Subject: [PATCH 02/12] Handle `mut` & `ref` --- .../src/handlers/destructure_tuple_binding.rs | 121 ++++++++++++++++-- 1 file changed, 111 insertions(+), 10 deletions(-) diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs index 4cc0ece3f40c..c2e7535537e7 100644 --- a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -80,7 +80,7 @@ fn collect_data(ident_pat: IdentPat, ctx: &AssistContext) -> Option { .map(|i| generate_name(i, &name, &ident_pat, &usages, ctx)) .collect_vec(); - Some(TupleData { range, field_names, usages }) + Some(TupleData { ident_pat, range, field_names, usages }) } fn generate_name( @@ -95,7 +95,7 @@ fn generate_name( } struct TupleData { - // ident_pat: IdentPat, + ident_pat: IdentPat, // name: String, range: TextRange, field_names: Vec, @@ -103,23 +103,31 @@ struct TupleData { usages: Option, } fn edit_tuple_assignment(data: &TupleData, builder: &mut AssistBuilder, ctx: &AssistContext) { - let new_tuple = { - let fields = data + let tuple_pat = { + let original = &data.ident_pat; + let is_ref = original.ref_token().is_some(); + let is_mut = original.mut_token().is_some(); + let fields = + data .field_names .iter() - .map(|name| ast::Pat::from(ast::make::ident_pat(false, false, ast::make::name(name)))); + .map(|name| ast::Pat::from(ast::make::ident_pat(is_ref, is_mut, ast::make::name(name)))); ast::make::tuple_pat(fields) }; + let add_cursor = |text: &str| { + // place cursor on first tuple item + let first_tuple = &data.field_names[0]; + text.replacen(first_tuple, &format!("$0{}", first_tuple), 1) + }; + + let text = tuple_pat.to_string(); match ctx.config.snippet_cap { Some(cap) => { - // No support for placeholders in Code Actions, except for special rust analyzer handling which supports only first `$0` - // -> place cursor on first variable - let mut snip = new_tuple.to_string(); - snip.insert_str(1, "$0"); + let snip = add_cursor(&text); builder.replace_snippet(cap, data.range, snip); } - None => builder.replace(data.range, new_tuple.to_string()), + None => builder.replace(data.range, text), }; } @@ -462,6 +470,74 @@ fn foo(t: &(usize, usize)) -> usize { ) } + #[test] + fn with_ref() { + //Note: `v` has different types: + // * in 1st: `i32` + // * in 2nd: `&i32` + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let ref $0t = (1,2); + let v = t.0; +} + "#, + r#" +fn main() { + let (ref $0_0, ref _1) = (1,2); + let v = _0; +} + "#, + ) + } + + #[test] + fn with_mut() { + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let mut $0t = (1,2); + t.0 = 42; + let v = t.0; +} + "#, + r#" +fn main() { + let (mut $0_0, mut _1) = (1,2); + _0 = 42; + let v = _0; +} + "#, + ) + } + + #[test] + fn with_ref_mut() { + //Note: `v` has different types: + // * in 1st: `i32` + // * in 2nd: `&mut i32` + // Note: 2nd `_0 = 42` isn't valid; requires dereferencing (`*_0`), but isn't handled here! + check_assist( + destructure_tuple_binding, + r#" +fn main() { + let ref mut $0t = (1,2); + t.0 = 42; + let v = t.0; +} + "#, + r#" +fn main() { + let (ref mut $0_0, ref mut _1) = (1,2); + _0 = 42; + let v = _0; +} + "#, + ) + } + #[test] fn dont_trigger_for_non_tuple_reference() { check_assist_not_applicable( @@ -733,6 +809,31 @@ fn main() { "#, ) } + #[test] + fn in_match_reference_option() { + check_assist( + destructure_tuple_binding, + r#" +//- minicore: option +fn main() { + let t = (1,2); + match Some(&t) { + Some($0t) => t.1, + _ => 0, + }; +} + "#, + r#" +fn main() { + let t = (1,2); + match Some(&t) { + Some(($0_0, _1)) => _1, + _ => 0, + }; +} + "#, + ) + } #[test] fn in_for() { From 450a9c5b122f3807c9dbabcf29e2bd7d40410dc7 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 6 Aug 2021 10:55:44 +0200 Subject: [PATCH 03/12] Add destructure in sub-pattern (after `@`) --- .../src/handlers/destructure_tuple_binding.rs | 379 +++++++++++++++--- 1 file changed, 323 insertions(+), 56 deletions(-) diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs index c2e7535537e7..4f34c4ea0df8 100644 --- a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -13,7 +13,7 @@ use crate::assist_context::{AssistBuilder, AssistContext, Assists}; // Assist: destructure_tuple_binding // -// Destructures a tuple binding into its items. +// Destructures a tuple binding in place. // // ``` // fn main() { @@ -28,22 +28,56 @@ use crate::assist_context::{AssistBuilder, AssistContext, Assists}; // let v = _0; // } // ``` - +// +// +// And (currently disabled): +// Assist: destructure_tuple_binding_in_sub_pattern +// +// Destructures tuple items in sub-pattern (after `@`). +// +// ``` +// fn main() { +// let $0t = (1,2); +// let v = t.0; +// } +// ``` +// -> +// ``` +// fn main() { +// let t @ (_0, _1) = (1,2); +// let v = _0; +// } +// ``` pub(crate) fn destructure_tuple_binding(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - // find variable pattern under cursor + destructure_tuple_binding_impl(acc, ctx, false) +} + +pub(crate) fn destructure_tuple_binding_impl(acc: &mut Assists, ctx: &AssistContext, with_sub_pattern: bool) -> Option<()> { let ident_pat = ctx.find_node_at_offset::()?; let data = collect_data(ident_pat, ctx)?; acc.add( AssistId("destructure_tuple", AssistKind::RefactorRewrite), - "Destructure tuple", + if with_sub_pattern { "Destructure tuple in place" } else { "Destructure tuple" }, data.range, |builder| { - edit_tuple_assignment(&data, builder, ctx); - edit_tuple_usages(&data, builder, ctx); + edit_tuple_assignment(&data, builder, ctx, false); + edit_tuple_usages(&data, builder, ctx, false); }, ); + if with_sub_pattern { + acc.add( + AssistId("destructure_tuple_in_sub_pattern", AssistKind::RefactorRewrite), + "Destructure tuple in sub-pattern", + data.range, + |builder| { + edit_tuple_assignment(&data, builder, ctx, true); + edit_tuple_usages(&data, builder, ctx, true); + }, + ); + } + Some(()) } @@ -102,7 +136,7 @@ struct TupleData { // field_types: Vec, usages: Option, } -fn edit_tuple_assignment(data: &TupleData, builder: &mut AssistBuilder, ctx: &AssistContext) { +fn edit_tuple_assignment(data: &TupleData, builder: &mut AssistBuilder, ctx: &AssistContext, in_sub_pattern: bool) { let tuple_pat = { let original = &data.ident_pat; let is_ref = original.ref_token().is_some(); @@ -121,27 +155,40 @@ fn edit_tuple_assignment(data: &TupleData, builder: &mut AssistBuilder, ctx: &As text.replacen(first_tuple, &format!("$0{}", first_tuple), 1) }; - let text = tuple_pat.to_string(); - match ctx.config.snippet_cap { - Some(cap) => { - let snip = add_cursor(&text); - builder.replace_snippet(cap, data.range, snip); - } - None => builder.replace(data.range, text), - }; + // with sub_pattern: keep original tuple and add subpattern: `tup @ (_0, _1)` + if in_sub_pattern { + let text = format!(" @ {}", tuple_pat.to_string()); + match ctx.config.snippet_cap { + Some(cap) => { + let snip = add_cursor(&text); + builder.insert_snippet(cap, data.range.end(), snip); + } + None => builder.insert(data.range.end(), text), + }; + } else { + let text = tuple_pat.to_string(); + match ctx.config.snippet_cap { + Some(cap) => { + let snip = add_cursor(&text); + builder.replace_snippet(cap, data.range, snip); + } + None => builder.replace(data.range, text), + }; + } } fn edit_tuple_usages( data: &TupleData, builder: &mut AssistBuilder, ctx: &AssistContext, + in_sub_pattern: bool, ) { if let Some(usages) = data.usages.as_ref() { for (file_id, refs) in usages.iter() { builder.edit_file(*file_id); for r in refs { - edit_tuple_usage(r, data, builder, ctx); + edit_tuple_usage(r, data, builder, ctx, in_sub_pattern); } } } @@ -151,6 +198,7 @@ fn edit_tuple_usage( data: &TupleData, builder: &mut AssistBuilder, _ctx: &AssistContext, + in_sub_pattern: bool, ) { match detect_tuple_index(usage.name.syntax(), data) { Some((expr, idx)) => { @@ -160,6 +208,11 @@ fn edit_tuple_usage( builder.replace(range, text); } None => { + if in_sub_pattern { + cov_mark::hit!(destructure_tuple_call_with_subpattern); + return; + } + // no index access -> make invalid -> requires handling by user // -> put usage in block comment builder.insert(usage.range.start(), "/*"); @@ -202,11 +255,18 @@ mod tests { use crate::tests::{check_assist, check_assist_not_applicable}; + // Tests for direct tuple destructure: + // `let $0t = (1,2);` -> `let (_0, _1) = (1,2);` + + fn assist(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + destructure_tuple_binding_impl(acc, ctx, false) + } + #[test] fn dont_trigger_on_unit() { cov_mark::check!(destructure_tuple_no_tuple); check_assist_not_applicable( - destructure_tuple_binding, + assist, r#" fn main() { let $0v = (); @@ -218,7 +278,7 @@ let $0v = (); fn dont_trigger_on_number() { cov_mark::check!(destructure_tuple_no_tuple); check_assist_not_applicable( - destructure_tuple_binding, + assist, r#" fn main() { let $0v = 32; @@ -230,7 +290,7 @@ let $0v = 32; #[test] fn destructure_3_tuple() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let $0tup = (1,2,3); @@ -246,7 +306,7 @@ fn main() { #[test] fn destructure_2_tuple() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let $0tup = (1,2); @@ -262,7 +322,7 @@ fn main() { #[test] fn replace_indices() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let $0tup = (1,2,3); @@ -285,7 +345,7 @@ fn main() { #[test] fn replace_usage_in_parentheses() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let $0tup = (1,2,3); @@ -306,7 +366,7 @@ fn main() { #[test] fn handle_function_call() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let $0tup = (1,2); @@ -325,7 +385,7 @@ fn main() { #[test] fn handle_invalid_index() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let $0tup = (1,2); @@ -344,7 +404,7 @@ fn main() { #[test] fn dont_replace_variable_with_same_name_as_tuple() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let tup = (1,2); @@ -371,7 +431,7 @@ fn main() { #[test] fn keep_function_call_in_tuple_item() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let $0t = ("3.14", 0); @@ -390,7 +450,7 @@ fn main() { #[test] fn keep_type() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let $0t: (usize, i32) = (1,2); @@ -410,7 +470,7 @@ fn main() { // * in 1st: `i32` // * in 2nd: `&i32` check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let t = (1,2); @@ -431,7 +491,7 @@ fn main() { #[test] fn destructure_multiple_reference() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let t = (1,2); @@ -452,7 +512,7 @@ fn main() { #[test] fn keep_reference() { check_assist( - destructure_tuple_binding, + assist, r#" fn foo(t: &(usize, usize)) -> usize { match t { @@ -476,7 +536,7 @@ fn foo(t: &(usize, usize)) -> usize { // * in 1st: `i32` // * in 2nd: `&i32` check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let ref $0t = (1,2); @@ -495,7 +555,7 @@ fn main() { #[test] fn with_mut() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let mut $0t = (1,2); @@ -520,7 +580,7 @@ fn main() { // * in 2nd: `&mut i32` // Note: 2nd `_0 = 42` isn't valid; requires dereferencing (`*_0`), but isn't handled here! check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let ref mut $0t = (1,2); @@ -541,7 +601,7 @@ fn main() { #[test] fn dont_trigger_for_non_tuple_reference() { check_assist_not_applicable( - destructure_tuple_binding, + assist, r#" fn main() { let v = 42; @@ -554,7 +614,7 @@ fn main() { #[test] fn dont_trigger_on_static_tuple() { check_assist_not_applicable( - destructure_tuple_binding, + assist, r#" static $0TUP: (usize, usize) = (1,2); "#, @@ -564,7 +624,7 @@ static $0TUP: (usize, usize) = (1,2); #[test] fn dont_trigger_on_wildcard() { check_assist_not_applicable( - destructure_tuple_binding, + assist, r#" fn main() { let $0_ = (1,2); @@ -576,7 +636,7 @@ fn main() { #[test] fn dont_trigger_in_struct() { check_assist_not_applicable( - destructure_tuple_binding, + assist, r#" struct S { $0tup: (usize, usize), @@ -588,7 +648,7 @@ struct S { #[test] fn dont_trigger_in_struct_creation() { check_assist_not_applicable( - destructure_tuple_binding, + assist, r#" struct S { tup: (usize, usize), @@ -605,7 +665,7 @@ fn main() { #[test] fn dont_trigger_on_tuple_struct() { check_assist_not_applicable( - destructure_tuple_binding, + assist, r#" struct S(usize, usize); fn main() { @@ -620,7 +680,7 @@ fn main() { // sub-pattern is only allowed with IdentPat (name), not other patterns (like TuplePat) cov_mark::check!(destructure_tuple_subpattern); check_assist_not_applicable( - destructure_tuple_binding, + assist, r#" fn sum(t: (usize, usize)) -> usize { match t { @@ -635,7 +695,7 @@ fn sum(t: (usize, usize)) -> usize { #[test] fn in_subpattern() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let t1 @ (_, $0t2) = (1, (2,3)); @@ -654,7 +714,7 @@ fn main() { #[test] fn in_nested_tuple() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let ($0tup, v) = ((1,2),3); @@ -671,7 +731,7 @@ fn main() { #[test] fn in_closure() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let $0tup = (1,2,3); @@ -690,7 +750,7 @@ fn main() { #[test] fn in_closure_args() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let f = |$0t| t.0 + t.1; @@ -709,7 +769,7 @@ fn main() { #[test] fn in_function_args() { check_assist( - destructure_tuple_binding, + assist, r#" fn f($0t: (usize, usize)) { let v = t.0; @@ -726,7 +786,7 @@ fn f(($0_0, _1): (usize, usize)) { #[test] fn in_if_let() { check_assist( - destructure_tuple_binding, + assist, r#" fn f(t: (usize, usize)) { if let $0t = t { @@ -746,7 +806,7 @@ fn f(t: (usize, usize)) { #[test] fn in_if_let_option() { check_assist( - destructure_tuple_binding, + assist, r#" //- minicore: option fn f(o: Option<(usize, usize)>) { @@ -769,7 +829,7 @@ fn f(o: Option<(usize, usize)>) { #[test] fn in_match() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { match (1,2) { @@ -789,7 +849,7 @@ fn main() { #[test] fn in_match_option() { check_assist( - destructure_tuple_binding, + assist, r#" //- minicore: option fn main() { @@ -812,7 +872,7 @@ fn main() { #[test] fn in_match_reference_option() { check_assist( - destructure_tuple_binding, + assist, r#" //- minicore: option fn main() { @@ -838,7 +898,7 @@ fn main() { #[test] fn in_for() { check_assist( - destructure_tuple_binding, + assist, r#" //- minicore: iterators fn main() { @@ -859,7 +919,7 @@ fn main() { #[test] fn in_for_nested() { check_assist( - destructure_tuple_binding, + assist, r#" //- minicore: iterators fn main() { @@ -882,7 +942,7 @@ fn main() { fn not_applicable_on_tuple_usage() { //Improvement: might be reasonable to allow & implement check_assist_not_applicable( - destructure_tuple_binding, + assist, r#" fn main() { let t = (1,2); @@ -895,7 +955,7 @@ fn main() { #[test] fn replace_all() { check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let $0t = (1,2); @@ -932,7 +992,7 @@ fn main() { #[test] fn non_trivial_tuple_assignment() { check_assist( - destructure_tuple_binding, + assist, r#" fn main { let $0t = @@ -977,7 +1037,7 @@ fn main { // doesn't work: cannot find usage in `println!` // Note: Just `t` works, but not `t.0` check_assist( - destructure_tuple_binding, + assist, r#" fn main() { let $0t = (1,2); @@ -992,4 +1052,211 @@ fn main() { "#, ) } + + /// Tests for destructure of tuple in sub-pattern: + /// `let $0t = (1,2);` -> `let t @ (_0, _1) = (1,2);` + mod sub_pattern { + use super::*; + use crate::tests::check_assist_by_label; + + fn assist(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + destructure_tuple_binding_impl(acc, ctx, true) + } + + fn check_sub_pattern_assist( + ra_fixture_before: &str, + ra_fixture_after: &str, + ) { + // with sub-pattern there are two assists: One with and one without sub-pattern + check_assist_by_label( + assist, + ra_fixture_before, + ra_fixture_after, + "Destructure tuple in sub-pattern") + } + + #[test] + fn destructure_in_sub_pattern() { + check_sub_pattern_assist( + r#" +#![feature(bindings_after_at)] + +fn main() { + let $0t = (1,2); +} + "#, + r#" +#![feature(bindings_after_at)] + +fn main() { + let t @ ($0_0, _1) = (1,2); +} + "#, + ) + } + + #[test] + fn trigger_both_destructure_tuple_assists() { + let text = r#" +fn main() { + let $0t = (1,2); +} + "#; + check_assist_by_label( + assist, + text, + r#" +fn main() { + let ($0_0, _1) = (1,2); +} + "#, + "Destructure tuple in place", + ); + check_assist_by_label( + assist, + text, + r#" +fn main() { + let t @ ($0_0, _1) = (1,2); +} + "#, + "Destructure tuple in sub-pattern", + ); + } + + #[test] + fn replace_indices() { + check_sub_pattern_assist( + r#" +fn main() { + let $0t = (1,2); + let v1 = t.0; + let v2 = t.1; +} + "#, + r#" +fn main() { + let t @ ($0_0, _1) = (1,2); + let v1 = _0; + let v2 = _1; +} + "#, + ) + } + + #[test] + fn keep_function_call() { + cov_mark::check!(destructure_tuple_call_with_subpattern); + check_sub_pattern_assist( + r#" +fn main() { + let $0t = (1,2); + let v = t.into(); +} + "#, + r#" +fn main() { + let t @ ($0_0, _1) = (1,2); + let v = t.into(); +} + "#, + ) + } + + #[test] + fn keep_type() { + check_sub_pattern_assist( + r#" +fn main() { + let $0t: (usize, i32) = (1,2); + let v = t.1; + let f = t.into(); +} + "#, + r#" +fn main() { + let t @ ($0_0, _1): (usize, i32) = (1,2); + let v = _1; + let f = t.into(); +} + "#, + ) + } + + #[test] + fn in_function_args() { + check_sub_pattern_assist( + r#" +fn f($0t: (usize, usize)) { + let v = t.0; + let f = t.into(); +} + "#, + r#" +fn f(t @ ($0_0, _1): (usize, usize)) { + let v = _0; + let f = t.into(); +} + "#, + ) + } + + #[test] + fn with_ref() { + check_sub_pattern_assist( + r#" +fn main() { + let ref $0t = (1,2); + let v = t.1; + let f = t.into(); +} + "#, + r#" +fn main() { + let ref t @ (ref $0_0, ref _1) = (1,2); + let v = _1; + let f = t.into(); +} + "#, + ) + } + #[test] + fn with_mut() { + check_sub_pattern_assist( + r#" +fn main() { + let mut $0t = (1,2); + let v = t.1; + let f = t.into(); +} + "#, + r#" +fn main() { + let mut t @ (mut $0_0, mut _1) = (1,2); + let v = _1; + let f = t.into(); +} + "#, + ) + } + #[test] + fn with_ref_mut() { + check_sub_pattern_assist( + r#" +fn main() { + let ref mut $0t = (1,2); + let v = t.1; + let f = t.into(); +} + "#, + r#" +fn main() { + let ref mut t @ (ref mut $0_0, ref mut _1) = (1,2); + let v = _1; + let f = t.into(); +} + "#, + ) + } + } } From 0d2490f785031708867a013887337661106ff7bd Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Mon, 9 Aug 2021 19:56:11 +0200 Subject: [PATCH 04/12] Handle tuple in macro call Only tuple name is handled (uncommented), not tuple index --- .../src/handlers/destructure_tuple_binding.rs | 369 ++++++++++++++++-- 1 file changed, 326 insertions(+), 43 deletions(-) diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs index 4f34c4ea0df8..5694aa4c9a7e 100644 --- a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -4,10 +4,7 @@ use ide_db::{ search::{FileReference, SearchScope, UsageSearchResult}, }; use itertools::Itertools; -use syntax::{ - ast::{self, AstNode, FieldExpr, IdentPat, NameOwner}, - SyntaxNode, TextRange, -}; +use syntax::{TextRange, ast::{self, AstNode, IdentPat, NameOwner}}; use crate::assist_context::{AssistBuilder, AssistContext, Assists}; @@ -186,7 +183,7 @@ fn edit_tuple_usages( if let Some(usages) = data.usages.as_ref() { for (file_id, refs) in usages.iter() { builder.edit_file(*file_id); - + for r in refs { edit_tuple_usage(r, data, builder, ctx, in_sub_pattern); } @@ -200,12 +197,10 @@ fn edit_tuple_usage( _ctx: &AssistContext, in_sub_pattern: bool, ) { - match detect_tuple_index(usage.name.syntax(), data) { - Some((expr, idx)) => { - // index field access - let text = &data.field_names[idx]; - let range = expr.syntax().text_range(); - builder.replace(range, text); + match detect_tuple_index(usage, data) { + Some(index) => { + let text = &data.field_names[index.index]; + builder.replace(index.range, text); } None => { if in_sub_pattern { @@ -221,7 +216,12 @@ fn edit_tuple_usage( } } -fn detect_tuple_index(usage: &SyntaxNode, data: &TupleData) -> Option<(FieldExpr, usize)> { +struct TupleIndex { + index: usize, + range: TextRange, +} + +fn detect_tuple_index(usage: &FileReference, data: &TupleData) -> Option { // usage is IDENT // IDENT // NAME_REF @@ -230,7 +230,9 @@ fn detect_tuple_index(usage: &SyntaxNode, data: &TupleData) -> Option<(FieldExpr // PATH_EXPR // PAREN_EXRP* // FIELD_EXPR - let node = usage + + let node = + usage.name.syntax() .ancestors() .skip_while(|s| !ast::PathExpr::can_cast(s.kind())) .skip(1) // PATH_EXPR @@ -239,7 +241,29 @@ fn detect_tuple_index(usage: &SyntaxNode, data: &TupleData) -> Option<(FieldExpr if let Some(field_expr) = ast::FieldExpr::cast(node) { let idx = field_expr.name_ref()?.as_tuple_field()?; if idx < data.field_names.len() { - Some((field_expr, idx)) + + // special case: in macro call -> range of `field_expr` in applied macro, NOT range in actual file! + if field_expr.syntax().ancestors().any(|a| ast::MacroStmts::can_cast(a.kind())) { + cov_mark::hit!(destructure_tuple_macro_call); + + // issue: cannot differentiate between tuple index passed into macro or tuple index as result of macro: + // ```rust + // macro_rules! m { + // ($t1:expr, $t2:expr) => { $t1; $t2.0 } + // } + // let t = (1,2); + // m!(t.0, t) + // ``` + // -> 2 tuple index usages detected! + // + // -> only handle `t` + return None; + } + + Some(TupleIndex { + index: idx, + range: field_expr.syntax().text_range(), + }) } else { // tuple index out of range None @@ -1030,32 +1054,7 @@ fn main { ) } - // ----------------------- - #[test] - #[ignore] - fn replace_in_macro_call() { - // doesn't work: cannot find usage in `println!` - // Note: Just `t` works, but not `t.0` - check_assist( - assist, - r#" -fn main() { - let $0t = (1,2); - println!("{}", t.0); -} - "#, - r#" -fn main() { - let ($0_0, _1) = (1,2); - println!("{}", _0); -} - "#, - ) - } - - /// Tests for destructure of tuple in sub-pattern: - /// `let $0t = (1,2);` -> `let t @ (_0, _1) = (1,2);` - mod sub_pattern { + mod assist { use super::*; use crate::tests::check_assist_by_label; @@ -1063,18 +1062,48 @@ fn main() { destructure_tuple_binding_impl(acc, ctx, true) } - fn check_sub_pattern_assist( + pub(crate) fn check_in_place_assist( + + ra_fixture_before: &str, + ra_fixture_after: &str, + ) { + check_assist_by_label( + assist, + ra_fixture_before, + ra_fixture_after, + "Destructure tuple in place" + ); + } + + pub(crate) fn check_sub_pattern_assist( ra_fixture_before: &str, ra_fixture_after: &str, ) { - // with sub-pattern there are two assists: One with and one without sub-pattern check_assist_by_label( assist, ra_fixture_before, ra_fixture_after, - "Destructure tuple in sub-pattern") + "Destructure tuple in sub-pattern" + ); } + pub(crate) fn check_both_assists( + ra_fixture_before: &str, + ra_fixture_after_in_place: &str, + ra_fixture_after_in_sub_pattern: &str, + ) { + check_in_place_assist(ra_fixture_before, ra_fixture_after_in_place); + check_sub_pattern_assist(ra_fixture_before, ra_fixture_after_in_sub_pattern); + } + } + + /// Tests for destructure of tuple in sub-pattern: + /// `let $0t = (1,2);` -> `let t @ (_0, _1) = (1,2);` + mod sub_pattern { + use super::*; + use super::assist::*; + use crate::tests::check_assist_by_label; + #[test] fn destructure_in_sub_pattern() { check_sub_pattern_assist( @@ -1097,6 +1126,9 @@ fn main() { #[test] fn trigger_both_destructure_tuple_assists() { + fn assist(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + destructure_tuple_binding_impl(acc, ctx, true) + } let text = r#" fn main() { let $0t = (1,2); @@ -1259,4 +1291,255 @@ fn main() { ) } } + + /// Tests for tuple usage in macro call: + /// `dbg!(t.0)` + mod in_macro_call { + use super::assist::*; + + + #[test] + fn detect_macro_call() { + cov_mark::check!(destructure_tuple_macro_call); + check_in_place_assist( + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let $0t = (1,2); + m!(t.0); +} + "#, + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/.0); +} + "#, + ) + } + + #[test] + fn tuple_usage() { + check_both_assists( + // leading `"foo"` to ensure `$e` doesn't start at position `0` + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let $0t = (1,2); + m!(t); +} + "#, + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/); +} + "#, + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let t @ ($0_0, _1) = (1,2); + m!(t); +} + "#, + ) + } + + #[test] + fn tuple_function_usage() { + check_both_assists( + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let $0t = (1,2); + m!(t.into()); +} + "#, + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/.into()); +} + "#, + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let t @ ($0_0, _1) = (1,2); + m!(t.into()); +} + "#, + ) + } + + #[test] + fn tuple_index_usage() { + check_both_assists( + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let $0t = (1,2); + m!(t.0); +} + "#, + // FIXME: replace `t.0` with `_0` (cannot detect range of tuple index in macro call) + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/.0); +} + "#, + // FIXME: replace `t.0` with `_0` + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let t @ ($0_0, _1) = (1,2); + m!(t.0); +} + "#, + ) + } + + #[test] + fn tuple_in_parentheses_index_usage() { + check_both_assists( + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let $0t = (1,2); + m!((t).0); +} + "#, + // FIXME: replace `(t).0` with `_0` + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!((/*t*/).0); +} + "#, + // FIXME: replace `(t).0` with `_0` + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let t @ ($0_0, _1) = (1,2); + m!((t).0); +} + "#, + ) + } + + #[test] + fn empty_macro() { + check_in_place_assist( + r#" +macro_rules! m { + () => { "foo" }; + ($e:expr) => { $e; "foo" }; +} + +fn main() { + let $0t = (1,2); + m!(t); +} + "#, + // FIXME: macro allows no arg -> is valid. But assist should result in invalid code + r#" +macro_rules! m { + () => { "foo" }; + ($e:expr) => { $e; "foo" }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/); +} + "#, + ) + } + + #[test] + fn tuple_index_in_macro() { + check_both_assists( + r#" +macro_rules! m { + ($t:expr, $i:expr) => { $t.0 + $i }; +} + +fn main() { + let $0t = (1,2); + m!(t, t.0); +} + "#, + // FIXME: replace `t.0` in macro call (not IN macro) with `_0` + r#" +macro_rules! m { + ($t:expr, $i:expr) => { $t.0 + $i }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/, /*t*/.0); +} + "#, + // FIXME: replace `t.0` in macro call with `_0` + r#" +macro_rules! m { + ($t:expr, $i:expr) => { $t.0 + $i }; +} + +fn main() { + let t @ ($0_0, _1) = (1,2); + m!(t, t.0); +} + "#, + ) + + } + } } From 45ef57bd23dd3b97184b167385c81c0f0b34d8cf Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Mon, 9 Aug 2021 20:58:33 +0200 Subject: [PATCH 05/12] Fix: different assist ids in doc and code --- .../ide_assists/src/handlers/destructure_tuple_binding.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs index 5694aa4c9a7e..c4e6d48058d9 100644 --- a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -21,7 +21,7 @@ use crate::assist_context::{AssistBuilder, AssistContext, Assists}; // -> // ``` // fn main() { -// let (_0, _1) = (1,2); +// let ($0_0, _1) = (1,2); // let v = _0; // } // ``` @@ -41,7 +41,7 @@ use crate::assist_context::{AssistBuilder, AssistContext, Assists}; // -> // ``` // fn main() { -// let t @ (_0, _1) = (1,2); +// let t @ ($0_0, _1) = (1,2); // let v = _0; // } // ``` @@ -54,7 +54,7 @@ pub(crate) fn destructure_tuple_binding_impl(acc: &mut Assists, ctx: &AssistCont let data = collect_data(ident_pat, ctx)?; acc.add( - AssistId("destructure_tuple", AssistKind::RefactorRewrite), + AssistId("destructure_tuple_binding", AssistKind::RefactorRewrite), if with_sub_pattern { "Destructure tuple in place" } else { "Destructure tuple" }, data.range, |builder| { @@ -65,7 +65,7 @@ pub(crate) fn destructure_tuple_binding_impl(acc: &mut Assists, ctx: &AssistCont if with_sub_pattern { acc.add( - AssistId("destructure_tuple_in_sub_pattern", AssistKind::RefactorRewrite), + AssistId("destructure_tuple_binding_in_sub_pattern", AssistKind::RefactorRewrite), "Destructure tuple in sub-pattern", data.range, |builder| { From b441aa20466e4bc4cf086cc8c7a7cab5942ae771 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Tue, 10 Aug 2021 13:28:32 +0200 Subject: [PATCH 06/12] Cleanup according to style-requirements in tests --- .../src/handlers/destructure_tuple_binding.rs | 107 +++++++++--------- 1 file changed, 52 insertions(+), 55 deletions(-) diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs index c4e6d48058d9..01a7d462d71b 100644 --- a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -4,7 +4,10 @@ use ide_db::{ search::{FileReference, SearchScope, UsageSearchResult}, }; use itertools::Itertools; -use syntax::{TextRange, ast::{self, AstNode, IdentPat, NameOwner}}; +use syntax::{ + ast::{self, AstNode, IdentPat, NameOwner}, + TextRange, +}; use crate::assist_context::{AssistBuilder, AssistContext, Assists}; @@ -49,7 +52,11 @@ pub(crate) fn destructure_tuple_binding(acc: &mut Assists, ctx: &AssistContext) destructure_tuple_binding_impl(acc, ctx, false) } -pub(crate) fn destructure_tuple_binding_impl(acc: &mut Assists, ctx: &AssistContext, with_sub_pattern: bool) -> Option<()> { +pub(crate) fn destructure_tuple_binding_impl( + acc: &mut Assists, + ctx: &AssistContext, + with_sub_pattern: bool, +) -> Option<()> { let ident_pat = ctx.find_node_at_offset::()?; let data = collect_data(ident_pat, ctx)?; @@ -121,7 +128,7 @@ fn generate_name( _usages: &Option, _ctx: &AssistContext, ) -> String { - //TODO: detect if name already used + // FIXME: detect if name already used format!("_{}", index) } @@ -133,16 +140,19 @@ struct TupleData { // field_types: Vec, usages: Option, } -fn edit_tuple_assignment(data: &TupleData, builder: &mut AssistBuilder, ctx: &AssistContext, in_sub_pattern: bool) { +fn edit_tuple_assignment( + data: &TupleData, + builder: &mut AssistBuilder, + ctx: &AssistContext, + in_sub_pattern: bool, +) { let tuple_pat = { let original = &data.ident_pat; let is_ref = original.ref_token().is_some(); let is_mut = original.mut_token().is_some(); - let fields = - data - .field_names - .iter() - .map(|name| ast::Pat::from(ast::make::ident_pat(is_ref, is_mut, ast::make::name(name)))); + let fields = data.field_names.iter().map(|name| { + ast::Pat::from(ast::make::ident_pat(is_ref, is_mut, ast::make::name(name))) + }); ast::make::tuple_pat(fields) }; @@ -231,17 +241,17 @@ fn detect_tuple_index(usage: &FileReference, data: &TupleData) -> Option range of `field_expr` in applied macro, NOT range in actual file! if field_expr.syntax().ancestors().any(|a| ast::MacroStmts::can_cast(a.kind())) { cov_mark::hit!(destructure_tuple_macro_call); @@ -260,10 +270,7 @@ fn detect_tuple_index(usage: &FileReference, data: &TupleData) -> Option `let (_0, _1) = (1,2);` fn assist(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { @@ -459,13 +466,13 @@ fn main() { r#" fn main() { let $0t = ("3.14", 0); - let pi: f32 = t.0.parse().unwrap(); + let pi: f32 = t.0.parse().unwrap_or(0.0); } "#, r#" fn main() { let ($0_0, _1) = ("3.14", 0); - let pi: f32 = _0.parse().unwrap(); + let pi: f32 = _0.parse().unwrap_or(0.0); } "#, ) @@ -849,7 +856,6 @@ fn f(o: Option<(usize, usize)>) { ) } - #[test] fn in_match() { check_assist( @@ -1019,7 +1025,7 @@ fn main() { assist, r#" fn main { - let $0t = + let $0t = if 1 > 2 { (1,2) } else { @@ -1036,7 +1042,7 @@ fn main { "#, r#" fn main { - let ($0_0, _1) = + let ($0_0, _1) = if 1 > 2 { (1,2) } else { @@ -1062,28 +1068,21 @@ fn main { destructure_tuple_binding_impl(acc, ctx, true) } - pub(crate) fn check_in_place_assist( - - ra_fixture_before: &str, - ra_fixture_after: &str, - ) { + pub(crate) fn check_in_place_assist(ra_fixture_before: &str, ra_fixture_after: &str) { check_assist_by_label( - assist, - ra_fixture_before, - ra_fixture_after, - "Destructure tuple in place" + assist, + ra_fixture_before, + ra_fixture_after, + "Destructure tuple in place", ); } - pub(crate) fn check_sub_pattern_assist( - ra_fixture_before: &str, - ra_fixture_after: &str, - ) { + pub(crate) fn check_sub_pattern_assist(ra_fixture_before: &str, ra_fixture_after: &str) { check_assist_by_label( - assist, - ra_fixture_before, - ra_fixture_after, - "Destructure tuple in sub-pattern" + assist, + ra_fixture_before, + ra_fixture_after, + "Destructure tuple in sub-pattern", ); } @@ -1097,11 +1096,11 @@ fn main { } } - /// Tests for destructure of tuple in sub-pattern: + /// Tests for destructure of tuple in sub-pattern: /// `let $0t = (1,2);` -> `let t @ (_0, _1) = (1,2);` mod sub_pattern { - use super::*; use super::assist::*; + use super::*; use crate::tests::check_assist_by_label; #[test] @@ -1123,7 +1122,7 @@ fn main() { "#, ) } - + #[test] fn trigger_both_destructure_tuple_assists() { fn assist(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { @@ -1135,23 +1134,23 @@ fn main() { } "#; check_assist_by_label( - assist, - text, + assist, + text, r#" fn main() { let ($0_0, _1) = (1,2); } - "#, + "#, "Destructure tuple in place", ); check_assist_by_label( - assist, - text, + assist, + text, r#" fn main() { let t @ ($0_0, _1) = (1,2); } - "#, + "#, "Destructure tuple in sub-pattern", ); } @@ -1175,7 +1174,7 @@ fn main() { "#, ) } - + #[test] fn keep_function_call() { cov_mark::check!(destructure_tuple_call_with_subpattern); @@ -1194,7 +1193,7 @@ fn main() { "#, ) } - + #[test] fn keep_type() { check_sub_pattern_assist( @@ -1293,11 +1292,10 @@ fn main() { } /// Tests for tuple usage in macro call: - /// `dbg!(t.0)` - mod in_macro_call { + /// `println!("{}", t.0)` + mod in_macro_call { use super::assist::*; - #[test] fn detect_macro_call() { cov_mark::check!(destructure_tuple_macro_call); @@ -1539,7 +1537,6 @@ fn main() { } "#, ) - } } } From 5b9f8e7e8e309eb77ec8129ffeb555ff1435cc0f Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Tue, 10 Aug 2021 14:15:12 +0200 Subject: [PATCH 07/12] Fix errors from `rebase master` Note: 2nd Assist description is moved down: generated doc tests extracts now all tests (previously only the first one). But it uses the first `Assist` name -- which is the wrong one for the 2nd test. And 2nd assist is currently disabled -> would fail anyway. --- .../src/handlers/destructure_tuple_binding.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs index 01a7d462d71b..a1aa0a04572b 100644 --- a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -28,9 +28,11 @@ use crate::assist_context::{AssistBuilder, AssistContext, Assists}; // let v = _0; // } // ``` -// -// -// And (currently disabled): +pub(crate) fn destructure_tuple_binding(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + destructure_tuple_binding_impl(acc, ctx, false) +} + +// And when `with_sub_pattern` enabled (currently disabled): // Assist: destructure_tuple_binding_in_sub_pattern // // Destructures tuple items in sub-pattern (after `@`). @@ -48,10 +50,6 @@ use crate::assist_context::{AssistBuilder, AssistContext, Assists}; // let v = _0; // } // ``` -pub(crate) fn destructure_tuple_binding(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - destructure_tuple_binding_impl(acc, ctx, false) -} - pub(crate) fn destructure_tuple_binding_impl( acc: &mut Assists, ctx: &AssistContext, @@ -96,7 +94,7 @@ fn collect_data(ident_pat: IdentPat, ctx: &AssistContext) -> Option { let ty = ctx.sema.type_of_pat(&ident_pat.clone().into())?; // might be reference - let ty = ty.strip_references(); + let ty = ty.adjusted().strip_references(); // must be tuple let field_types = ty.tuple_fields(ctx.db()); if field_types.is_empty() { From d0cf28322a2ae5a7df2bf739469b64754a3b9bd0 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Tue, 10 Aug 2021 14:20:14 +0200 Subject: [PATCH 08/12] Add generated doctest --- crates/ide_assists/src/tests/generated.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 853c41f78f43..54791f646644 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -367,6 +367,25 @@ impl Point { ) } +#[test] +fn doctest_destructure_tuple_binding() { + check_doc_test( + "destructure_tuple_binding", + r#####" +fn main() { + let $0t = (1,2); + let v = t.0; +} +"#####, + r#####" +fn main() { + let ($0_0, _1) = (1,2); + let v = _0; +} +"#####, + ) +} + #[test] fn doctest_expand_glob_import() { check_doc_test( From 384fae7fcd8e8063b30b3d318bcc8e545fca7d6b Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Fri, 13 Aug 2021 19:16:36 +0200 Subject: [PATCH 09/12] Switch order of assists Destructure in sub-pattern before Destructure in place to favor the first one --- .../src/handlers/destructure_tuple_binding.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs index a1aa0a04572b..1fe58442bf7e 100644 --- a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -58,16 +58,6 @@ pub(crate) fn destructure_tuple_binding_impl( let ident_pat = ctx.find_node_at_offset::()?; let data = collect_data(ident_pat, ctx)?; - acc.add( - AssistId("destructure_tuple_binding", AssistKind::RefactorRewrite), - if with_sub_pattern { "Destructure tuple in place" } else { "Destructure tuple" }, - data.range, - |builder| { - edit_tuple_assignment(&data, builder, ctx, false); - edit_tuple_usages(&data, builder, ctx, false); - }, - ); - if with_sub_pattern { acc.add( AssistId("destructure_tuple_binding_in_sub_pattern", AssistKind::RefactorRewrite), @@ -80,6 +70,16 @@ pub(crate) fn destructure_tuple_binding_impl( ); } + acc.add( + AssistId("destructure_tuple_binding", AssistKind::RefactorRewrite), + if with_sub_pattern { "Destructure tuple in place" } else { "Destructure tuple" }, + data.range, + |builder| { + edit_tuple_assignment(&data, builder, ctx, false); + edit_tuple_usages(&data, builder, ctx, false); + }, + ); + Some(()) } From b1ebb82f32a200480a5a35dbf961b07cf50df451 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Tue, 17 Aug 2021 18:18:02 +0200 Subject: [PATCH 10/12] Add deref (`*`) handling for usages of ref tuples --- .../src/handlers/destructure_tuple_binding.rs | 685 ++++++++++++++++-- 1 file changed, 643 insertions(+), 42 deletions(-) diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs index 1fe58442bf7e..0b7c54b20b44 100644 --- a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -5,8 +5,8 @@ use ide_db::{ }; use itertools::Itertools; use syntax::{ - ast::{self, AstNode, IdentPat, NameOwner}, - TextRange, + ast::{self, AstNode, FieldExpr, IdentPat, MethodCallExpr, NameOwner}, + match_ast, TextRange, }; use crate::assist_context::{AssistBuilder, AssistContext, Assists}; @@ -64,7 +64,7 @@ pub(crate) fn destructure_tuple_binding_impl( "Destructure tuple in sub-pattern", data.range, |builder| { - edit_tuple_assignment(&data, builder, ctx, true); + edit_tuple_assignment(ctx, builder, &data, true); edit_tuple_usages(&data, builder, ctx, true); }, ); @@ -75,7 +75,7 @@ pub(crate) fn destructure_tuple_binding_impl( if with_sub_pattern { "Destructure tuple in place" } else { "Destructure tuple" }, data.range, |builder| { - edit_tuple_assignment(&data, builder, ctx, false); + edit_tuple_assignment(ctx, builder, &data, false); edit_tuple_usages(&data, builder, ctx, false); }, ); @@ -85,16 +85,23 @@ pub(crate) fn destructure_tuple_binding_impl( fn collect_data(ident_pat: IdentPat, ctx: &AssistContext) -> Option { if ident_pat.at_token().is_some() { - // cannot destructure pattern with sub-pattern: + // Cannot destructure pattern with sub-pattern: // Only IdentPat can have sub-pattern, - // but not TuplePat (`(a,b)`) + // but not TuplePat (`(a,b)`). cov_mark::hit!(destructure_tuple_subpattern); return None; } - let ty = ctx.sema.type_of_pat(&ident_pat.clone().into())?; + let ty = ctx.sema.type_of_pat(&ident_pat.clone().into())?.adjusted(); + let ref_type = if ty.is_mutable_reference() { + Some(RefType::Mutable) + } else if ty.is_reference() { + Some(RefType::ReadOnly) + } else { + None + }; // might be reference - let ty = ty.adjusted().strip_references(); + let ty = ty.strip_references(); // must be tuple let field_types = ty.tuple_fields(ctx.db()); if field_types.is_empty() { @@ -113,35 +120,40 @@ fn collect_data(ident_pat: IdentPat, ctx: &AssistContext) -> Option { }); let field_names = (0..field_types.len()) - .map(|i| generate_name(i, &name, &ident_pat, &usages, ctx)) + .map(|i| generate_name(ctx, i, &name, &ident_pat, &usages)) .collect_vec(); - Some(TupleData { ident_pat, range, field_names, usages }) + Some(TupleData { ident_pat, range, ref_type, field_names, usages }) } fn generate_name( + _ctx: &AssistContext, index: usize, _tuple_name: &str, _ident_pat: &IdentPat, _usages: &Option, - _ctx: &AssistContext, ) -> String { // FIXME: detect if name already used format!("_{}", index) } +enum RefType { + ReadOnly, + Mutable, +} struct TupleData { ident_pat: IdentPat, // name: String, range: TextRange, + ref_type: Option, field_names: Vec, // field_types: Vec, usages: Option, } fn edit_tuple_assignment( - data: &TupleData, - builder: &mut AssistBuilder, ctx: &AssistContext, + builder: &mut AssistBuilder, + data: &TupleData, in_sub_pattern: bool, ) { let tuple_pat = { @@ -193,23 +205,20 @@ fn edit_tuple_usages( builder.edit_file(*file_id); for r in refs { - edit_tuple_usage(r, data, builder, ctx, in_sub_pattern); + edit_tuple_usage(ctx, builder, r, data, in_sub_pattern); } } } } fn edit_tuple_usage( + ctx: &AssistContext, + builder: &mut AssistBuilder, usage: &FileReference, data: &TupleData, - builder: &mut AssistBuilder, - _ctx: &AssistContext, in_sub_pattern: bool, ) { match detect_tuple_index(usage, data) { - Some(index) => { - let text = &data.field_names[index.index]; - builder.replace(index.range, text); - } + Some(index) => edit_tuple_field_usage(ctx, builder, data, index), None => { if in_sub_pattern { cov_mark::hit!(destructure_tuple_call_with_subpattern); @@ -224,11 +233,26 @@ fn edit_tuple_usage( } } +fn edit_tuple_field_usage( + ctx: &AssistContext, + builder: &mut AssistBuilder, + data: &TupleData, + index: TupleIndex, +) { + let field_name = &data.field_names[index.index]; + + if data.ref_type.is_some() { + let ref_data = handle_ref_field_usage(ctx, &index.field_expr); + builder.replace(ref_data.range, ref_data.format(field_name)); + } else { + builder.replace(index.range, field_name); + } +} struct TupleIndex { index: usize, range: TextRange, + field_expr: FieldExpr, } - fn detect_tuple_index(usage: &FileReference, data: &TupleData) -> Option { // usage is IDENT // IDENT @@ -268,7 +292,7 @@ fn detect_tuple_index(usage: &FileReference, data: &TupleData) -> Option Option String { + match (self.needs_deref, self.needs_parentheses) { + (true, true) => format!("(*{})", field_name), + (true, false) => format!("*{}", field_name), + (false, true) => format!("({})", field_name), + (false, false) => field_name.to_string(), + } + } +} +fn handle_ref_field_usage(ctx: &AssistContext, field_expr: &FieldExpr) -> RefData { + let s = field_expr.syntax(); + let mut ref_data = + RefData { range: s.text_range(), needs_deref: true, needs_parentheses: true }; + + let parent = match s.parent() { + Some(parent) => parent, + None => return ref_data, + }; + + match_ast! { + match parent { + ast::ParenExpr(it) => { + // already parens in place -> don't replace + ref_data.needs_parentheses = false; + // there might be a ref outside: `&(t.0)` -> can be removed + if let Some(it) = it.syntax().parent().and_then(ast::RefExpr::cast) { + ref_data.needs_deref = false; + ref_data.range = it.syntax().text_range(); + } + }, + ast::RefExpr(it) => { + // `&*` -> cancel each other out + ref_data.needs_deref = false; + ref_data.needs_parentheses = false; + // might be surrounded by parens -> can be removed too + match it.syntax().parent().and_then(ast::ParenExpr::cast) { + Some(parent) => ref_data.range = parent.syntax().text_range(), + None => ref_data.range = it.syntax().text_range(), + }; + }, + // higher precedence than deref `*` + // https://doc.rust-lang.org/reference/expressions.html#expression-precedence + // -> requires parentheses + ast::PathExpr(_it) => {}, + ast::MethodCallExpr(it) => { + // `field_expr` is `self_param` (otherwise it would be in `ArgList`) + + // test if there's already auto-ref in place (`value` -> `&value`) + // -> no method accepting `self`, but `&self` -> no need for deref + // + // other combinations (`&value` -> `value`, `&&value` -> `&value`, `&value` -> `&&value`) might or might not be able to auto-ref/deref, + // but there might be trait implementations an added `&` might resolve to + // -> ONLY handle auto-ref from `value` to `&value` + fn is_auto_ref(ctx: &AssistContext, call_expr: &MethodCallExpr) -> bool { + fn impl_(ctx: &AssistContext, call_expr: &MethodCallExpr) -> Option { + let rec = call_expr.receiver()?; + let rec_ty = ctx.sema.type_of_expr(&rec)?.adjusted(); + // input must be actual value + if rec_ty.is_reference() { + return Some(false); + } + + // doesn't resolve trait impl + let f = ctx.sema.resolve_method_call(call_expr)?; + let self_param = f.self_param(ctx.db())?; + // self must be ref + match self_param.access(ctx.db()) { + hir::Access::Shared | hir::Access::Exclusive => Some(true), + hir::Access::Owned => Some(false), + } + } + impl_(ctx, call_expr).unwrap_or(false) + } + + if is_auto_ref(ctx, &it) { + ref_data.needs_deref = false; + ref_data.needs_parentheses = false; + } + }, + ast::FieldExpr(_it) => { + // `t.0.my_field` + ref_data.needs_deref = false; + ref_data.needs_parentheses = false; + }, + ast::IndexExpr(_it) => { + // `t.0[1]` + ref_data.needs_deref = false; + ref_data.needs_parentheses = false; + }, + ast::TryExpr(_it) => { + // `t.0?` + // requires deref and parens: `(*_0)` + }, + // lower precedence than deref `*` -> no parens + _ => { + ref_data.needs_parentheses = false; + }, + } + }; + + ref_data +} + #[cfg(test)] mod tests { use super::*; @@ -495,9 +628,6 @@ fn main() { #[test] fn destructure_reference() { - //Note: `v` has different types: - // * in 1st: `i32` - // * in 2nd: `&i32` check_assist( assist, r#" @@ -511,7 +641,7 @@ fn main() { fn main() { let t = (1,2); let ($0_0, _1) = &t; - let v = _0; + let v = *_0; } "#, ) @@ -532,7 +662,7 @@ fn main() { fn main() { let t = (1,2); let ($0_0, _1) = &&t; - let v = _0; + let v = *_0; } "#, ) @@ -561,9 +691,6 @@ fn foo(t: &(usize, usize)) -> usize { #[test] fn with_ref() { - //Note: `v` has different types: - // * in 1st: `i32` - // * in 2nd: `&i32` check_assist( assist, r#" @@ -575,7 +702,7 @@ fn main() { r#" fn main() { let (ref $0_0, ref _1) = (1,2); - let v = _0; + let v = *_0; } "#, ) @@ -604,10 +731,6 @@ fn main() { #[test] fn with_ref_mut() { - //Note: `v` has different types: - // * in 1st: `i32` - // * in 2nd: `&mut i32` - // Note: 2nd `_0 = 42` isn't valid; requires dereferencing (`*_0`), but isn't handled here! check_assist( assist, r#" @@ -620,8 +743,8 @@ fn main() { r#" fn main() { let (ref mut $0_0, ref mut _1) = (1,2); - _0 = 42; - let v = _0; + *_0 = 42; + let v = *_0; } "#, ) @@ -915,7 +1038,7 @@ fn main() { fn main() { let t = (1,2); match Some(&t) { - Some(($0_0, _1)) => _1, + Some(($0_0, _1)) => *_1, _ => 0, }; } @@ -1065,13 +1188,17 @@ fn main { fn assist(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { destructure_tuple_binding_impl(acc, ctx, true) } + fn in_place_assist(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + destructure_tuple_binding_impl(acc, ctx, false) + } pub(crate) fn check_in_place_assist(ra_fixture_before: &str, ra_fixture_after: &str) { check_assist_by_label( - assist, + in_place_assist, ra_fixture_before, ra_fixture_after, - "Destructure tuple in place", + // "Destructure tuple in place", + "Destructure tuple", ); } @@ -1243,7 +1370,7 @@ fn main() { r#" fn main() { let ref t @ (ref $0_0, ref _1) = (1,2); - let v = _1; + let v = *_1; let f = t.into(); } "#, @@ -1281,7 +1408,7 @@ fn main() { r#" fn main() { let ref mut t @ (ref mut $0_0, ref mut _1) = (1,2); - let v = _1; + let v = *_1; let f = t.into(); } "#, @@ -1532,6 +1659,480 @@ macro_rules! m { fn main() { let t @ ($0_0, _1) = (1,2); m!(t, t.0); +} + "#, + ) + } + } + + mod refs { + use super::assist::*; + + #[test] + fn no_ref() { + check_in_place_assist( + r#" +fn main() { + let $0t = &(1,2); + let v: i32 = t.0; +} + "#, + r#" +fn main() { + let ($0_0, _1) = &(1,2); + let v: i32 = *_0; +} + "#, + ) + } + #[test] + fn no_ref_with_parens() { + check_in_place_assist( + r#" +fn main() { + let $0t = &(1,2); + let v: i32 = (t.0); +} + "#, + r#" +fn main() { + let ($0_0, _1) = &(1,2); + let v: i32 = (*_0); +} + "#, + ) + } + #[test] + fn with_ref() { + check_in_place_assist( + r#" +fn main() { + let $0t = &(1,2); + let v: &i32 = &t.0; +} + "#, + r#" +fn main() { + let ($0_0, _1) = &(1,2); + let v: &i32 = _0; +} + "#, + ) + } + #[test] + fn with_ref_in_parens_ref() { + check_in_place_assist( + r#" +fn main() { + let $0t = &(1,2); + let v: &i32 = &(t.0); +} + "#, + r#" +fn main() { + let ($0_0, _1) = &(1,2); + let v: &i32 = _0; +} + "#, + ) + } + #[test] + fn with_ref_in_ref_parens() { + check_in_place_assist( + r#" +fn main() { + let $0t = &(1,2); + let v: &i32 = (&t.0); +} + "#, + r#" +fn main() { + let ($0_0, _1) = &(1,2); + let v: &i32 = _0; +} + "#, + ) + } + + #[test] + fn deref_and_parentheses() { + // Operator/Expressions with higher precedence than deref (`*`): + // https://doc.rust-lang.org/reference/expressions.html#expression-precedence + // * Path + // * Method call + // * Field expression + // * Function calls, array indexing + // * `?` + check_in_place_assist( + r#" +//- minicore: option +fn f1(v: i32) {} +fn f2(v: &i32) {} +trait T { + fn do_stuff(self) {} +} +impl T for i32 { + fn do_stuff(self) {} +} +impl T for &i32 { + fn do_stuff(self) {} +} +struct S4 { + value: i32, +} + +fn foo() -> Option<()> { + let $0t = &(0, (1,"1"), Some(2), [3;3], S4 { value: 4 }, &5); + let v: i32 = t.0; // deref, no parens + let v: &i32 = &t.0; // no deref, no parens, remove `&` + f1(t.0); // deref, no parens + f2(&t.0); // `&*` -> cancel out -> no deref, no parens + // https://github.com/rust-analyzer/rust-analyzer/issues/1109#issuecomment-658868639 + // let v: i32 = t.1.0; // no deref, no parens + let v: i32 = t.4.value; // no deref, no parens + t.0.do_stuff(); // deref, parens + let v: i32 = t.2?; // deref, parens + let v: i32 = t.3[0]; // no deref, no parens + (t.0).do_stuff(); // deref, no additional parens + let v: i32 = *t.5; // deref (-> 2), no parens + + None +} + "#, + r#" +fn f1(v: i32) {} +fn f2(v: &i32) {} +trait T { + fn do_stuff(self) {} +} +impl T for i32 { + fn do_stuff(self) {} +} +impl T for &i32 { + fn do_stuff(self) {} +} +struct S4 { + value: i32, +} + +fn foo() -> Option<()> { + let ($0_0, _1, _2, _3, _4, _5) = &(0, (1,"1"), Some(2), [3;3], S4 { value: 4 }, &5); + let v: i32 = *_0; // deref, no parens + let v: &i32 = _0; // no deref, no parens, remove `&` + f1(*_0); // deref, no parens + f2(_0); // `&*` -> cancel out -> no deref, no parens + // https://github.com/rust-analyzer/rust-analyzer/issues/1109#issuecomment-658868639 + // let v: i32 = t.1.0; // no deref, no parens + let v: i32 = _4.value; // no deref, no parens + (*_0).do_stuff(); // deref, parens + let v: i32 = (*_2)?; // deref, parens + let v: i32 = _3[0]; // no deref, no parens + (*_0).do_stuff(); // deref, no additional parens + let v: i32 = **_5; // deref (-> 2), no parens + + None +} + "#, + ) + } + + // --------- + // auto-ref/deref + + #[test] + fn self_auto_ref_doesnt_need_deref() { + check_in_place_assist( + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn f(&self) {} +} + +fn main() { + let $0t = &(S,2); + let s = t.0.f(); +} + "#, + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn f(&self) {} +} + +fn main() { + let ($0_0, _1) = &(S,2); + let s = _0.f(); +} + "#, + ) + } + + #[test] + fn self_owned_requires_deref() { + check_in_place_assist( + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn f(self) {} +} + +fn main() { + let $0t = &(S,2); + let s = t.0.f(); +} + "#, + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn f(self) {} +} + +fn main() { + let ($0_0, _1) = &(S,2); + let s = (*_0).f(); +} + "#, + ) + } + + #[test] + fn self_auto_ref_in_trait_call_doesnt_require_deref() { + check_in_place_assist( + r#" +trait T { + fn f(self); +} +#[derive(Clone, Copy)] +struct S; +impl T for &S { + fn f(self) {} +} + +fn main() { + let $0t = &(S,2); + let s = t.0.f(); +} + "#, + // FIXME: doesn't need deref * parens. But `ctx.sema.resolve_method_call` doesn't resolve trait implementations + r#" +trait T { + fn f(self); +} +#[derive(Clone, Copy)] +struct S; +impl T for &S { + fn f(self) {} +} + +fn main() { + let ($0_0, _1) = &(S,2); + let s = (*_0).f(); +} + "#, + ) + } + #[test] + fn no_auto_deref_because_of_owned_and_ref_trait_impl() { + check_in_place_assist( + r#" +trait T { + fn f(self); +} +#[derive(Clone, Copy)] +struct S; +impl T for S { + fn f(self) {} +} +impl T for &S { + fn f(self) {} +} + +fn main() { + let $0t = &(S,2); + let s = t.0.f(); +} + "#, + r#" +trait T { + fn f(self); +} +#[derive(Clone, Copy)] +struct S; +impl T for S { + fn f(self) {} +} +impl T for &S { + fn f(self) {} +} + +fn main() { + let ($0_0, _1) = &(S,2); + let s = (*_0).f(); +} + "#, + ) + } + + #[test] + fn no_outer_parens_when_ref_deref() { + check_in_place_assist( + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn do_stuff(&self) -> i32 { 42 } +} +fn main() { + let $0t = &(S,&S); + let v = (&t.0).do_stuff(); +} + "#, + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn do_stuff(&self) -> i32 { 42 } +} +fn main() { + let ($0_0, _1) = &(S,&S); + let v = _0.do_stuff(); +} + "#, + ) + } + + #[test] + fn auto_ref_deref() { + check_in_place_assist( + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn do_stuff(&self) -> i32 { 42 } +} +fn main() { + let $0t = &(S,&S); + let v = (&t.0).do_stuff(); // no deref, remove parens + // `t.0` gets auto-refed -> no deref needed -> no parens + let v = t.0.do_stuff(); // no deref, no parens + let v = &t.0.do_stuff(); // `&` is for result -> no deref, no parens + // deref: `_1` is `&&S`, but method called is on `&S` -> there might be a method accepting `&&S` + let v = t.1.do_stuff(); // deref, parens +} + "#, + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn do_stuff(&self) -> i32 { 42 } +} +fn main() { + let ($0_0, _1) = &(S,&S); + let v = _0.do_stuff(); // no deref, remove parens + // `t.0` gets auto-refed -> no deref needed -> no parens + let v = _0.do_stuff(); // no deref, no parens + let v = &_0.do_stuff(); // `&` is for result -> no deref, no parens + // deref: `_1` is `&&S`, but method called is on `&S` -> there might be a method accepting `&&S` + let v = (*_1).do_stuff(); // deref, parens +} + "#, + ) + } + + #[test] + fn mutable() { + check_in_place_assist( + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} +fn f_mut(v: &mut i32) { *v = 42; } + +fn main() { + let $0t = &mut (1,2); + let v = t.0; + t.0 = 42; + f_owned(t.0); + f(&t.0); + f_mut(&mut t.0); +} + "#, + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} +fn f_mut(v: &mut i32) { *v = 42; } + +fn main() { + let ($0_0, _1) = &mut (1,2); + let v = *_0; + *_0 = 42; + f_owned(*_0); + f(_0); + f_mut(_0); +} + "#, + ) + } + + #[test] + fn with_ref_keyword() { + check_in_place_assist( + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} + +fn main() { + let ref $0t = (1,2); + let v = t.0; + f_owned(t.0); + f(&t.0); +} + "#, + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} + +fn main() { + let (ref $0_0, ref _1) = (1,2); + let v = *_0; + f_owned(*_0); + f(_0); +} + "#, + ) + } + #[test] + fn with_ref_mut_keywords() { + check_in_place_assist( + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} +fn f_mut(v: &mut i32) { *v = 42; } + +fn main() { + let ref mut $0t = (1,2); + let v = t.0; + t.0 = 42; + f_owned(t.0); + f(&t.0); + f_mut(&mut t.0); +} + "#, + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} +fn f_mut(v: &mut i32) { *v = 42; } + +fn main() { + let (ref mut $0_0, ref mut _1) = (1,2); + let v = *_0; + *_0 = 42; + f_owned(*_0); + f(_0); + f_mut(_0); } "#, ) From 2c27adc0a3fbc239a9f3ef0557309a87325df3d1 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Thu, 19 Aug 2021 14:09:52 +0200 Subject: [PATCH 11/12] Remove `match_ast!` macro call Add node about uncommenting tuple in macro call --- .../src/handlers/destructure_tuple_binding.rs | 153 +++++++++--------- 1 file changed, 80 insertions(+), 73 deletions(-) diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs index 0b7c54b20b44..eebeb2c4a515 100644 --- a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -227,6 +227,11 @@ fn edit_tuple_usage( // no index access -> make invalid -> requires handling by user // -> put usage in block comment + // + // Note: For macro invocations this might result in still valid code: + // When a macro accepts the tuple as argument, as well as no arguments at all, + // uncommenting the tuple still leaves the macro call working (see `tests::in_macro_call::empty_macro`). + // But this is an unlikely case. Usually the resulting macro call will become erroneous. builder.insert(usage.range.start(), "/*"); builder.insert(usage.range.end(), "*/"); } @@ -322,89 +327,91 @@ fn handle_ref_field_usage(ctx: &AssistContext, field_expr: &FieldExpr) -> RefDat let mut ref_data = RefData { range: s.text_range(), needs_deref: true, needs_parentheses: true }; - let parent = match s.parent() { - Some(parent) => parent, + let parent = match s.parent().map(ast::Expr::cast) { + Some(Some(parent)) => parent, + Some(None) => { + ref_data.needs_parentheses = false; + return ref_data; + } None => return ref_data, }; - match_ast! { - match parent { - ast::ParenExpr(it) => { - // already parens in place -> don't replace - ref_data.needs_parentheses = false; - // there might be a ref outside: `&(t.0)` -> can be removed - if let Some(it) = it.syntax().parent().and_then(ast::RefExpr::cast) { - ref_data.needs_deref = false; - ref_data.range = it.syntax().text_range(); - } - }, - ast::RefExpr(it) => { - // `&*` -> cancel each other out + match parent { + ast::Expr::ParenExpr(it) => { + // already parens in place -> don't replace + ref_data.needs_parentheses = false; + // there might be a ref outside: `&(t.0)` -> can be removed + if let Some(it) = it.syntax().parent().and_then(ast::RefExpr::cast) { ref_data.needs_deref = false; - ref_data.needs_parentheses = false; - // might be surrounded by parens -> can be removed too - match it.syntax().parent().and_then(ast::ParenExpr::cast) { - Some(parent) => ref_data.range = parent.syntax().text_range(), - None => ref_data.range = it.syntax().text_range(), - }; - }, - // higher precedence than deref `*` - // https://doc.rust-lang.org/reference/expressions.html#expression-precedence - // -> requires parentheses - ast::PathExpr(_it) => {}, - ast::MethodCallExpr(it) => { - // `field_expr` is `self_param` (otherwise it would be in `ArgList`) + ref_data.range = it.syntax().text_range(); + } + } + ast::Expr::RefExpr(it) => { + // `&*` -> cancel each other out + ref_data.needs_deref = false; + ref_data.needs_parentheses = false; + // might be surrounded by parens -> can be removed too + match it.syntax().parent().and_then(ast::ParenExpr::cast) { + Some(parent) => ref_data.range = parent.syntax().text_range(), + None => ref_data.range = it.syntax().text_range(), + }; + } + // higher precedence than deref `*` + // https://doc.rust-lang.org/reference/expressions.html#expression-precedence + // -> requires parentheses + ast::Expr::PathExpr(_it) => {} + ast::Expr::MethodCallExpr(it) => { + // `field_expr` is `self_param` (otherwise it would be in `ArgList`) + + // test if there's already auto-ref in place (`value` -> `&value`) + // -> no method accepting `self`, but `&self` -> no need for deref + // + // other combinations (`&value` -> `value`, `&&value` -> `&value`, `&value` -> `&&value`) might or might not be able to auto-ref/deref, + // but there might be trait implementations an added `&` might resolve to + // -> ONLY handle auto-ref from `value` to `&value` + fn is_auto_ref(ctx: &AssistContext, call_expr: &MethodCallExpr) -> bool { + fn impl_(ctx: &AssistContext, call_expr: &MethodCallExpr) -> Option { + let rec = call_expr.receiver()?; + let rec_ty = ctx.sema.type_of_expr(&rec)?.adjusted(); + // input must be actual value + if rec_ty.is_reference() { + return Some(false); + } - // test if there's already auto-ref in place (`value` -> `&value`) - // -> no method accepting `self`, but `&self` -> no need for deref - // - // other combinations (`&value` -> `value`, `&&value` -> `&value`, `&value` -> `&&value`) might or might not be able to auto-ref/deref, - // but there might be trait implementations an added `&` might resolve to - // -> ONLY handle auto-ref from `value` to `&value` - fn is_auto_ref(ctx: &AssistContext, call_expr: &MethodCallExpr) -> bool { - fn impl_(ctx: &AssistContext, call_expr: &MethodCallExpr) -> Option { - let rec = call_expr.receiver()?; - let rec_ty = ctx.sema.type_of_expr(&rec)?.adjusted(); - // input must be actual value - if rec_ty.is_reference() { - return Some(false); - } - - // doesn't resolve trait impl - let f = ctx.sema.resolve_method_call(call_expr)?; - let self_param = f.self_param(ctx.db())?; - // self must be ref - match self_param.access(ctx.db()) { - hir::Access::Shared | hir::Access::Exclusive => Some(true), - hir::Access::Owned => Some(false), - } + // doesn't resolve trait impl + let f = ctx.sema.resolve_method_call(call_expr)?; + let self_param = f.self_param(ctx.db())?; + // self must be ref + match self_param.access(ctx.db()) { + hir::Access::Shared | hir::Access::Exclusive => Some(true), + hir::Access::Owned => Some(false), } - impl_(ctx, call_expr).unwrap_or(false) } + impl_(ctx, call_expr).unwrap_or(false) + } - if is_auto_ref(ctx, &it) { - ref_data.needs_deref = false; - ref_data.needs_parentheses = false; - } - }, - ast::FieldExpr(_it) => { - // `t.0.my_field` - ref_data.needs_deref = false; - ref_data.needs_parentheses = false; - }, - ast::IndexExpr(_it) => { - // `t.0[1]` + if is_auto_ref(ctx, &it) { ref_data.needs_deref = false; ref_data.needs_parentheses = false; - }, - ast::TryExpr(_it) => { - // `t.0?` - // requires deref and parens: `(*_0)` - }, - // lower precedence than deref `*` -> no parens - _ => { - ref_data.needs_parentheses = false; - }, + } + } + ast::Expr::FieldExpr(_it) => { + // `t.0.my_field` + ref_data.needs_deref = false; + ref_data.needs_parentheses = false; + } + ast::Expr::IndexExpr(_it) => { + // `t.0[1]` + ref_data.needs_deref = false; + ref_data.needs_parentheses = false; + } + ast::Expr::TryExpr(_it) => { + // `t.0?` + // requires deref and parens: `(*_0)` + } + // lower precedence than deref `*` -> no parens + _ => { + ref_data.needs_parentheses = false; } }; From 8a9feeddd34d7a765e401613d8a4293b16660e29 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Thu, 19 Aug 2021 14:17:23 +0200 Subject: [PATCH 12/12] Remove `match_ast` usage --- crates/ide_assists/src/handlers/destructure_tuple_binding.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs index eebeb2c4a515..a421f5775467 100644 --- a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -6,7 +6,7 @@ use ide_db::{ use itertools::Itertools; use syntax::{ ast::{self, AstNode, FieldExpr, IdentPat, MethodCallExpr, NameOwner}, - match_ast, TextRange, + TextRange, }; use crate::assist_context::{AssistBuilder, AssistContext, Assists};