From 9f1d341ee9bf399fa8fa2a5d2fb5f91e1b319702 Mon Sep 17 00:00:00 2001 From: Luciano Bestia Date: Mon, 18 Jan 2021 19:45:42 +0100 Subject: added region folding --- crates/ide/Cargo.toml | 4 ++ crates/ide/src/folding_ranges.rs | 76 ++++++++++++++++++++++++++++++++---- crates/rust-analyzer/src/to_proto.rs | 2 +- 3 files changed, 73 insertions(+), 9 deletions(-) (limited to 'crates') diff --git a/crates/ide/Cargo.toml b/crates/ide/Cargo.toml index bb28cca4d..6ec106426 100644 --- a/crates/ide/Cargo.toml +++ b/crates/ide/Cargo.toml @@ -31,6 +31,10 @@ assists = { path = "../assists", version = "0.0.0" } ssr = { path = "../ssr", version = "0.0.0" } completion = { path = "../completion", version = "0.0.0" } +lazy_static = "1.4.0" +regex = "1.4.3" +env_logger = { version = "0.8.1", default-features = false } + # ide should depend only on the top-level `hir` package. if you need # something from some `hir_xxx` subpackage, reexport the API via `hir`. hir = { path = "../hir", version = "0.0.0" } diff --git a/crates/ide/src/folding_ranges.rs b/crates/ide/src/folding_ranges.rs index 45170dd29..99f0c3c99 100644 --- a/crates/ide/src/folding_ranges.rs +++ b/crates/ide/src/folding_ranges.rs @@ -6,9 +6,11 @@ use syntax::{ ast::{self, AstNode, AstToken, VisibilityOwner}, Direction, NodeOrToken, SourceFile, SyntaxKind::{self, *}, - SyntaxNode, TextRange, + SyntaxNode, TextRange, TextSize, }; +use lazy_static::lazy_static; + #[derive(Debug, PartialEq, Eq)] pub enum FoldKind { Comment, @@ -16,6 +18,7 @@ pub enum FoldKind { Mods, Block, ArgList, + Region, } #[derive(Debug)] @@ -29,6 +32,8 @@ pub(crate) fn folding_ranges(file: &SourceFile) -> Vec { let mut visited_comments = FxHashSet::default(); let mut visited_imports = FxHashSet::default(); let mut visited_mods = FxHashSet::default(); + // regions can be nested, here is a LIFO buffer + let mut regions_starts: Vec = vec![]; for element in file.syntax().descendants_with_tokens() { // Fold items that span multiple lines @@ -48,10 +53,32 @@ pub(crate) fn folding_ranges(file: &SourceFile) -> Vec { // Fold groups of comments if let Some(comment) = ast::Comment::cast(token) { if !visited_comments.contains(&comment) { - if let Some(range) = - contiguous_range_for_comment(comment, &mut visited_comments) - { - res.push(Fold { range, kind: FoldKind::Comment }) + // regions are not really comments + use regex::Regex; + lazy_static! { + static ref RE_START: Regex = + Regex::new(r"^\s*//\s*#?region\b").unwrap(); + static ref RE_END: Regex = + Regex::new(r"^\s*//\s*#?endregion\b").unwrap(); + } + if RE_START.is_match(comment.text()) { + regions_starts.push(comment.syntax().text_range().start()); + } else if RE_END.is_match(comment.text()) { + if !regions_starts.is_empty() { + res.push(Fold { + range: TextRange::new( + regions_starts.pop().unwrap(), + comment.syntax().text_range().end(), + ), + kind: FoldKind::Region, + }) + } + } else { + if let Some(range) = + contiguous_range_for_comment(comment, &mut visited_comments) + { + res.push(Fold { range, kind: FoldKind::Comment }) + } } } } @@ -175,9 +202,21 @@ fn contiguous_range_for_comment( } if let Some(c) = ast::Comment::cast(token) { if c.kind() == group_kind { - visited.insert(c.clone()); - last = c; - continue; + // regions are not really comments + use regex::Regex; + lazy_static! { + static ref RE_START: Regex = + Regex::new(r"^\s*//\s*#?region\b").unwrap(); + static ref RE_END: Regex = + Regex::new(r"^\s*//\s*#?endregion\b").unwrap(); + } + if RE_START.is_match(c.text()) || RE_END.is_match(c.text()) { + break; + } else { + visited.insert(c.clone()); + last = c; + continue; + } } } // The comment group ends because either: @@ -224,6 +263,7 @@ mod tests { FoldKind::Mods => "mods", FoldKind::Block => "block", FoldKind::ArgList => "arglist", + FoldKind::Region => "region", }; assert_eq!(kind, &attr.unwrap()); } @@ -418,4 +458,24 @@ fn foo( "#, ) } + + #[test] + fn fold_region() { + log_init_for_test_debug(); + // only error level log is printed on the terminal + log::error!("test fold_region"); + check( + r#" +// 1. some normal comment +// region: test +// 2. some normal comment +calling_function(x,y); +// endregion: test +"#, + ) + } + + fn log_init_for_test_debug() { + let _ = env_logger::builder().is_test(true).try_init(); + } } diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 1ff2d3fea..c903ab523 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -465,7 +465,7 @@ pub(crate) fn folding_range( let kind = match fold.kind { FoldKind::Comment => Some(lsp_types::FoldingRangeKind::Comment), FoldKind::Imports => Some(lsp_types::FoldingRangeKind::Imports), - FoldKind::Mods | FoldKind::Block | FoldKind::ArgList => None, + FoldKind::Mods | FoldKind::Block | FoldKind::ArgList | FoldKind::Region => None, }; let range = range(line_index, fold.range); -- cgit v1.2.3 From 084b21bc36cd624e8db708eb1e12dd6db99a0602 Mon Sep 17 00:00:00 2001 From: Luciano Bestia Date: Fri, 5 Feb 2021 14:32:03 +0100 Subject: simple comparison instead of regex --- crates/ide/Cargo.toml | 5 +---- crates/ide/src/folding_ranges.rs | 28 +++++++--------------------- 2 files changed, 8 insertions(+), 25 deletions(-) (limited to 'crates') diff --git a/crates/ide/Cargo.toml b/crates/ide/Cargo.toml index 6ec106426..15a48c0f3 100644 --- a/crates/ide/Cargo.toml +++ b/crates/ide/Cargo.toml @@ -31,13 +31,10 @@ assists = { path = "../assists", version = "0.0.0" } ssr = { path = "../ssr", version = "0.0.0" } completion = { path = "../completion", version = "0.0.0" } -lazy_static = "1.4.0" -regex = "1.4.3" -env_logger = { version = "0.8.1", default-features = false } - # ide should depend only on the top-level `hir` package. if you need # something from some `hir_xxx` subpackage, reexport the API via `hir`. hir = { path = "../hir", version = "0.0.0" } [dev-dependencies] expect-test = "1.1" +env_logger = { version = "0.8.1", default-features = false } \ No newline at end of file diff --git a/crates/ide/src/folding_ranges.rs b/crates/ide/src/folding_ranges.rs index 99f0c3c99..7ba775a77 100644 --- a/crates/ide/src/folding_ranges.rs +++ b/crates/ide/src/folding_ranges.rs @@ -9,8 +9,6 @@ use syntax::{ SyntaxNode, TextRange, TextSize, }; -use lazy_static::lazy_static; - #[derive(Debug, PartialEq, Eq)] pub enum FoldKind { Comment, @@ -53,17 +51,10 @@ pub(crate) fn folding_ranges(file: &SourceFile) -> Vec { // Fold groups of comments if let Some(comment) = ast::Comment::cast(token) { if !visited_comments.contains(&comment) { - // regions are not really comments - use regex::Regex; - lazy_static! { - static ref RE_START: Regex = - Regex::new(r"^\s*//\s*#?region\b").unwrap(); - static ref RE_END: Regex = - Regex::new(r"^\s*//\s*#?endregion\b").unwrap(); - } - if RE_START.is_match(comment.text()) { + // regions are not real comments + if comment.text().trim().starts_with("// region:") { regions_starts.push(comment.syntax().text_range().start()); - } else if RE_END.is_match(comment.text()) { + } else if comment.text().trim().starts_with("// endregion") { if !regions_starts.is_empty() { res.push(Fold { range: TextRange::new( @@ -202,15 +193,10 @@ fn contiguous_range_for_comment( } if let Some(c) = ast::Comment::cast(token) { if c.kind() == group_kind { - // regions are not really comments - use regex::Regex; - lazy_static! { - static ref RE_START: Regex = - Regex::new(r"^\s*//\s*#?region\b").unwrap(); - static ref RE_END: Regex = - Regex::new(r"^\s*//\s*#?endregion\b").unwrap(); - } - if RE_START.is_match(c.text()) || RE_END.is_match(c.text()) { + // regions are not real comments + if c.text().trim().starts_with("// region:") + || c.text().trim().starts_with("// endregion") + { break; } else { visited.insert(c.clone()); -- cgit v1.2.3 From 75015b6eaaad6cb6ce511cff2f3f1e5e34450106 Mon Sep 17 00:00:00 2001 From: Luciano <31509965+LucianoBestia@users.noreply.github.com> Date: Sat, 13 Feb 2021 17:46:26 +0100 Subject: Update crates/ide/src/folding_ranges.rs Co-authored-by: Lukas Wirth --- crates/ide/src/folding_ranges.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/folding_ranges.rs b/crates/ide/src/folding_ranges.rs index 7ba775a77..531f3c951 100644 --- a/crates/ide/src/folding_ranges.rs +++ b/crates/ide/src/folding_ranges.rs @@ -55,10 +55,10 @@ pub(crate) fn folding_ranges(file: &SourceFile) -> Vec { if comment.text().trim().starts_with("// region:") { regions_starts.push(comment.syntax().text_range().start()); } else if comment.text().trim().starts_with("// endregion") { - if !regions_starts.is_empty() { + if let Some(region) = regions_starts.pop() { res.push(Fold { range: TextRange::new( - regions_starts.pop().unwrap(), + region, comment.syntax().text_range().end(), ), kind: FoldKind::Region, -- cgit v1.2.3 From a28f9545e18918b68b5c07e0231f2af219a15bb7 Mon Sep 17 00:00:00 2001 From: Luciano Bestia Date: Sat, 13 Feb 2021 17:50:52 +0100 Subject: removed logging stuff --- crates/ide/Cargo.toml | 3 +-- crates/ide/src/folding_ranges.rs | 7 ------- 2 files changed, 1 insertion(+), 9 deletions(-) (limited to 'crates') diff --git a/crates/ide/Cargo.toml b/crates/ide/Cargo.toml index 15a48c0f3..8c49800d5 100644 --- a/crates/ide/Cargo.toml +++ b/crates/ide/Cargo.toml @@ -36,5 +36,4 @@ completion = { path = "../completion", version = "0.0.0" } hir = { path = "../hir", version = "0.0.0" } [dev-dependencies] -expect-test = "1.1" -env_logger = { version = "0.8.1", default-features = false } \ No newline at end of file +expect-test = "1.1" \ No newline at end of file diff --git a/crates/ide/src/folding_ranges.rs b/crates/ide/src/folding_ranges.rs index 531f3c951..4b1b24562 100644 --- a/crates/ide/src/folding_ranges.rs +++ b/crates/ide/src/folding_ranges.rs @@ -447,9 +447,6 @@ fn foo( #[test] fn fold_region() { - log_init_for_test_debug(); - // only error level log is printed on the terminal - log::error!("test fold_region"); check( r#" // 1. some normal comment @@ -460,8 +457,4 @@ calling_function(x,y); "#, ) } - - fn log_init_for_test_debug() { - let _ = env_logger::builder().is_test(true).try_init(); - } } -- cgit v1.2.3 From 790bda1f851eed1837415919897055f1635c066d Mon Sep 17 00:00:00 2001 From: Luciano Bestia Date: Sat, 13 Feb 2021 17:57:05 +0100 Subject: corrected no newline at end of file --- crates/ide/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crates') diff --git a/crates/ide/Cargo.toml b/crates/ide/Cargo.toml index 8c49800d5..bb28cca4d 100644 --- a/crates/ide/Cargo.toml +++ b/crates/ide/Cargo.toml @@ -36,4 +36,4 @@ completion = { path = "../completion", version = "0.0.0" } hir = { path = "../hir", version = "0.0.0" } [dev-dependencies] -expect-test = "1.1" \ No newline at end of file +expect-test = "1.1" -- cgit v1.2.3 From 864fb063a000a38ce28c8c1d0153dc080faf1cdb Mon Sep 17 00:00:00 2001 From: Luciano Bestia Date: Thu, 18 Feb 2021 16:51:21 +0100 Subject: rustfmt 1.4.30-stable --- crates/parser/src/grammar/params.rs | 11 ++++++----- crates/rust-analyzer/src/to_proto.rs | 3 ++- 2 files changed, 8 insertions(+), 6 deletions(-) (limited to 'crates') diff --git a/crates/parser/src/grammar/params.rs b/crates/parser/src/grammar/params.rs index 6a98d7368..e313f6fb7 100644 --- a/crates/parser/src/grammar/params.rs +++ b/crates/parser/src/grammar/params.rs @@ -169,11 +169,12 @@ fn opt_self_param(p: &mut Parser, m: Marker) { let la1 = p.nth(1); let la2 = p.nth(2); let la3 = p.nth(3); - if !matches!((p.current(), la1, la2, la3), - (T![&], T![self], _, _) - | (T![&], T![mut], T![self], _) - | (T![&], LIFETIME_IDENT, T![self], _) - | (T![&], LIFETIME_IDENT, T![mut], T![self]) + if !matches!( + (p.current(), la1, la2, la3), + (T![&], T![self], _, _) + | (T![&], T![mut], T![self], _) + | (T![&], LIFETIME_IDENT, T![self], _) + | (T![&], LIFETIME_IDENT, T![mut], T![self]) ) { return m.abandon(p); } diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index c903ab523..2526f4591 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -516,7 +516,8 @@ pub(crate) fn url_from_abs_path(path: &Path) -> lsp_types::Url { assert!(path.is_absolute()); let url = lsp_types::Url::from_file_path(path).unwrap(); match path.components().next() { - Some(path::Component::Prefix(prefix)) if matches!(prefix.kind(), path::Prefix::Disk(_) | path::Prefix::VerbatimDisk(_)) => + Some(path::Component::Prefix(prefix)) + if matches!(prefix.kind(), path::Prefix::Disk(_) | path::Prefix::VerbatimDisk(_)) => { // Need to lowercase driver letter } -- cgit v1.2.3 From 3b75dda7454ca26afc70aae55d48706441eceff9 Mon Sep 17 00:00:00 2001 From: Vladyslav Katasonov Date: Tue, 16 Feb 2021 04:43:32 +0300 Subject: try to suggest name when extracting variable --- .../ide_assists/src/handlers/extract_variable.rs | 294 ++++++++++++++++++++- 1 file changed, 287 insertions(+), 7 deletions(-) (limited to 'crates') diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index 98f3dc6ca..32e54fd57 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -1,6 +1,7 @@ -use stdx::format_to; +use itertools::Itertools; +use stdx::{format_to, to_lower_snake_case}; use syntax::{ - ast::{self, AstNode}, + ast::{self, AstNode, NameOwner}, SyntaxKind::{ BLOCK_EXPR, BREAK_EXPR, CLOSURE_EXPR, COMMENT, LOOP_EXPR, MATCH_ARM, PATH_EXPR, RETURN_EXPR, }, @@ -54,7 +55,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option let var_name = match &field_shorthand { Some(it) => it.to_string(), - None => "var_name".to_string(), + None => suggest_variable_name(ctx, &to_extract), }; let expr_range = match &field_shorthand { Some(it) => it.syntax().text_range().cover(to_extract.syntax().text_range()), @@ -173,6 +174,89 @@ impl Anchor { } } +fn suggest_variable_name(ctx: &AssistContext, expr: &ast::Expr) -> String { + // FIXME: account for existing names in the scope + suggest_name_from_func(expr) + .or_else(|| suggest_name_from_method(expr)) + .or_else(|| suggest_name_from_param(ctx, expr)) + .or_else(|| suggest_name_by_type(ctx, expr)) + .unwrap_or_else(|| "var_name".to_string()) +} + +fn normalize_name(name: &str) -> Option { + let name = to_lower_snake_case(name); + + let useless_names = ["new", "default", "some", "none", "ok", "err"]; + if useless_names.contains(&name.as_str()) { + return None; + } + + Some(name) +} + +fn suggest_name_from_func(expr: &ast::Expr) -> Option { + let call = match expr { + ast::Expr::CallExpr(call) => call, + _ => return None, + }; + let func = match call.expr()? { + ast::Expr::PathExpr(path) => path, + _ => return None, + }; + let ident = func.path()?.segment()?.name_ref()?.ident_token()?; + normalize_name(ident.text()) +} + +fn suggest_name_from_method(expr: &ast::Expr) -> Option { + let method = match expr { + ast::Expr::MethodCallExpr(call) => call, + _ => return None, + }; + let ident = method.name_ref()?.ident_token()?; + normalize_name(ident.text()) +} + +fn suggest_name_from_param(ctx: &AssistContext, expr: &ast::Expr) -> Option { + let arg_list = expr.syntax().parent().and_then(ast::ArgList::cast)?; + let args_parent = arg_list.syntax().parent()?; + let func = if let Some(call) = ast::CallExpr::cast(args_parent.clone()) { + let func = call.expr()?; + let func_ty = ctx.sema.type_of_expr(&func)?; + func_ty.as_callable(ctx.db())? + } else if let Some(method) = ast::MethodCallExpr::cast(args_parent) { + ctx.sema.resolve_method_call_as_callable(&method)? + } else { + return None; + }; + + let (idx, _) = arg_list.args().find_position(|it| it == expr).unwrap(); + let (pat, _) = func.params(ctx.db()).into_iter().nth(idx)?; + let param = match pat? { + either::Either::Right(ast::Pat::IdentPat(param)) => param, + _ => return None, + }; + let name = param.name()?; + normalize_name(&name.to_string()) +} + +fn suggest_name_by_type(ctx: &AssistContext, expr: &ast::Expr) -> Option { + let ty = ctx.sema.type_of_expr(expr)?; + let ty = ty.remove_ref().unwrap_or(ty); + + name_from_type(ty, ctx) +} + +fn name_from_type(ty: hir::Type, ctx: &AssistContext) -> Option { + let name = if let Some(adt) = ty.as_adt() { + adt.name(ctx.db()).to_string() + } else if let Some(trait_) = ty.as_dyn_trait() { + trait_.name(ctx.db()).to_string() + } else { + return None; + }; + normalize_name(&name) +} + #[cfg(test)] mod tests { use test_utils::mark; @@ -274,8 +358,8 @@ fn foo() { "#, r#" fn foo() { - let $0var_name = bar(1 + 1); - var_name + let $0bar = bar(1 + 1); + bar } "#, ) @@ -401,8 +485,8 @@ fn main() { ", " fn main() { - let $0var_name = bar.foo(); - let v = var_name; + let $0foo = bar.foo(); + let v = foo; } ", ); @@ -556,6 +640,202 @@ fn main() { ) } + #[test] + fn extract_var_name_from_type() { + check_assist( + extract_variable, + r#" +struct Test(i32); + +fn foo() -> Test { + $0{ Test(10) }$0 +} +"#, + r#" +struct Test(i32); + +fn foo() -> Test { + let $0test = { Test(10) }; + test +} +"#, + ) + } + + #[test] + fn extract_var_name_from_parameter() { + check_assist( + extract_variable, + r#" +fn bar(test: u32, size: u32) + +fn foo() { + bar(1, $01+1$0); +} +"#, + r#" +fn bar(test: u32, size: u32) + +fn foo() { + let $0size = 1+1; + bar(1, size); +} +"#, + ) + } + + #[test] + fn extract_var_parameter_name_has_precedence_over_type() { + check_assist( + extract_variable, + r#" +struct TextSize(u32); +fn bar(test: u32, size: TextSize) + +fn foo() { + bar(1, $0{ TextSize(1+1) }$0); +} +"#, + r#" +struct TextSize(u32); +fn bar(test: u32, size: TextSize) + +fn foo() { + let $0size = { TextSize(1+1) }; + bar(1, size); +} +"#, + ) + } + + #[test] + fn extract_var_name_from_function() { + check_assist( + extract_variable, + r#" +fn is_required(test: u32, size: u32) -> bool + +fn foo() -> bool { + $0is_required(1, 2)$0 +} +"#, + r#" +fn is_required(test: u32, size: u32) -> bool + +fn foo() -> bool { + let $0is_required = is_required(1, 2); + is_required +} +"#, + ) + } + + #[test] + fn extract_var_name_from_method() { + check_assist( + extract_variable, + r#" +struct S; +impl S { + fn bar(&self, n: u32) -> u32 { n } +} + +fn foo() -> u32 { + $0S.bar(1)$0 +} +"#, + r#" +struct S; +impl S { + fn bar(&self, n: u32) -> u32 { n } +} + +fn foo() -> u32 { + let $0bar = S.bar(1); + bar +} +"#, + ) + } + + #[test] + fn extract_var_name_from_method_param() { + check_assist( + extract_variable, + r#" +struct S; +impl S { + fn bar(&self, n: u32, size: u32) { n } +} + +fn foo() { + S.bar($01 + 1$0, 2) +} +"#, + r#" +struct S; +impl S { + fn bar(&self, n: u32, size: u32) { n } +} + +fn foo() { + let $0n = 1 + 1; + S.bar(n, 2) +} +"#, + ) + } + + #[test] + fn extract_var_name_from_ufcs_method_param() { + check_assist( + extract_variable, + r#" +struct S; +impl S { + fn bar(&self, n: u32, size: u32) { n } +} + +fn foo() { + S::bar(&S, $01 + 1$0, 2) +} +"#, + r#" +struct S; +impl S { + fn bar(&self, n: u32, size: u32) { n } +} + +fn foo() { + let $0n = 1 + 1; + S::bar(&S, n, 2) +} +"#, + ) + } + + #[test] + fn extract_var_function_name_has_precedence() { + check_assist( + extract_variable, + r#" +fn bar(test: u32, size: u32) + +fn foo() { + bar(1, $0symbol_size(1, 2)$0); +} +"#, + r#" +fn bar(test: u32, size: u32) + +fn foo() { + let $0symbol_size = symbol_size(1, 2); + bar(1, symbol_size); +} +"#, + ) + } + #[test] fn test_extract_var_for_return_not_applicable() { check_assist_not_applicable(extract_variable, "fn foo() { $0return$0; } "); -- cgit v1.2.3 From f915ab79fa1b11982b8e82e2db5b2486a893bed4 Mon Sep 17 00:00:00 2001 From: Vladyslav Katasonov Date: Tue, 16 Feb 2021 05:55:47 +0300 Subject: suggest parameter name before function name --- crates/ide_assists/src/handlers/extract_variable.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'crates') diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index 32e54fd57..2c47be987 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -176,9 +176,9 @@ impl Anchor { fn suggest_variable_name(ctx: &AssistContext, expr: &ast::Expr) -> String { // FIXME: account for existing names in the scope - suggest_name_from_func(expr) + suggest_name_from_param(ctx, expr) + .or_else(|| suggest_name_from_func(expr)) .or_else(|| suggest_name_from_method(expr)) - .or_else(|| suggest_name_from_param(ctx, expr)) .or_else(|| suggest_name_by_type(ctx, expr)) .unwrap_or_else(|| "var_name".to_string()) } @@ -815,7 +815,7 @@ fn foo() { } #[test] - fn extract_var_function_name_has_precedence() { + fn extract_var_parameter_name_has_precedence_over_function() { check_assist( extract_variable, r#" @@ -829,8 +829,8 @@ fn foo() { fn bar(test: u32, size: u32) fn foo() { - let $0symbol_size = symbol_size(1, 2); - bar(1, symbol_size); + let $0size = symbol_size(1, 2); + bar(1, size); } "#, ) -- cgit v1.2.3 From afc68277f69572944fd81d61b126732ab29b5d17 Mon Sep 17 00:00:00 2001 From: Vladyslav Katasonov Date: Tue, 16 Feb 2021 23:48:15 +0300 Subject: pull out suggest_name::* to utils; enchance heuristics --- .../ide_assists/src/handlers/extract_variable.rs | 92 +-- crates/ide_assists/src/tests.rs | 46 +- crates/ide_assists/src/utils.rs | 2 + crates/ide_assists/src/utils/suggest_name.rs | 770 +++++++++++++++++++++ 4 files changed, 821 insertions(+), 89 deletions(-) create mode 100644 crates/ide_assists/src/utils/suggest_name.rs (limited to 'crates') diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs index 2c47be987..312ac7ac4 100644 --- a/crates/ide_assists/src/handlers/extract_variable.rs +++ b/crates/ide_assists/src/handlers/extract_variable.rs @@ -1,7 +1,6 @@ -use itertools::Itertools; -use stdx::{format_to, to_lower_snake_case}; +use stdx::format_to; use syntax::{ - ast::{self, AstNode, NameOwner}, + ast::{self, AstNode}, SyntaxKind::{ BLOCK_EXPR, BREAK_EXPR, CLOSURE_EXPR, COMMENT, LOOP_EXPR, MATCH_ARM, PATH_EXPR, RETURN_EXPR, }, @@ -9,7 +8,7 @@ use syntax::{ }; use test_utils::mark; -use crate::{AssistContext, AssistId, AssistKind, Assists}; +use crate::{utils::suggest_name, AssistContext, AssistId, AssistKind, Assists}; // Assist: extract_variable // @@ -55,7 +54,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option let var_name = match &field_shorthand { Some(it) => it.to_string(), - None => suggest_variable_name(ctx, &to_extract), + None => suggest_name::variable(&to_extract, &ctx.sema), }; let expr_range = match &field_shorthand { Some(it) => it.syntax().text_range().cover(to_extract.syntax().text_range()), @@ -174,89 +173,6 @@ impl Anchor { } } -fn suggest_variable_name(ctx: &AssistContext, expr: &ast::Expr) -> String { - // FIXME: account for existing names in the scope - suggest_name_from_param(ctx, expr) - .or_else(|| suggest_name_from_func(expr)) - .or_else(|| suggest_name_from_method(expr)) - .or_else(|| suggest_name_by_type(ctx, expr)) - .unwrap_or_else(|| "var_name".to_string()) -} - -fn normalize_name(name: &str) -> Option { - let name = to_lower_snake_case(name); - - let useless_names = ["new", "default", "some", "none", "ok", "err"]; - if useless_names.contains(&name.as_str()) { - return None; - } - - Some(name) -} - -fn suggest_name_from_func(expr: &ast::Expr) -> Option { - let call = match expr { - ast::Expr::CallExpr(call) => call, - _ => return None, - }; - let func = match call.expr()? { - ast::Expr::PathExpr(path) => path, - _ => return None, - }; - let ident = func.path()?.segment()?.name_ref()?.ident_token()?; - normalize_name(ident.text()) -} - -fn suggest_name_from_method(expr: &ast::Expr) -> Option { - let method = match expr { - ast::Expr::MethodCallExpr(call) => call, - _ => return None, - }; - let ident = method.name_ref()?.ident_token()?; - normalize_name(ident.text()) -} - -fn suggest_name_from_param(ctx: &AssistContext, expr: &ast::Expr) -> Option { - let arg_list = expr.syntax().parent().and_then(ast::ArgList::cast)?; - let args_parent = arg_list.syntax().parent()?; - let func = if let Some(call) = ast::CallExpr::cast(args_parent.clone()) { - let func = call.expr()?; - let func_ty = ctx.sema.type_of_expr(&func)?; - func_ty.as_callable(ctx.db())? - } else if let Some(method) = ast::MethodCallExpr::cast(args_parent) { - ctx.sema.resolve_method_call_as_callable(&method)? - } else { - return None; - }; - - let (idx, _) = arg_list.args().find_position(|it| it == expr).unwrap(); - let (pat, _) = func.params(ctx.db()).into_iter().nth(idx)?; - let param = match pat? { - either::Either::Right(ast::Pat::IdentPat(param)) => param, - _ => return None, - }; - let name = param.name()?; - normalize_name(&name.to_string()) -} - -fn suggest_name_by_type(ctx: &AssistContext, expr: &ast::Expr) -> Option { - let ty = ctx.sema.type_of_expr(expr)?; - let ty = ty.remove_ref().unwrap_or(ty); - - name_from_type(ty, ctx) -} - -fn name_from_type(ty: hir::Type, ctx: &AssistContext) -> Option { - let name = if let Some(adt) = ty.as_adt() { - adt.name(ctx.db()).to_string() - } else if let Some(trait_) = ty.as_dyn_trait() { - trait_.name(ctx.db()).to_string() - } else { - return None; - }; - normalize_name(&name) -} - #[cfg(test)] mod tests { use test_utils::mark; diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs index b7f616760..66820058b 100644 --- a/crates/ide_assists/src/tests.rs +++ b/crates/ide_assists/src/tests.rs @@ -12,7 +12,7 @@ use ide_db::{ RootDatabase, }; use stdx::{format_to, trim_indent}; -use syntax::TextRange; +use syntax::{ast, AstNode, TextRange}; use test_utils::{assert_eq_text, extract_offset}; use crate::{handlers::Handler, Assist, AssistConfig, AssistContext, AssistKind, Assists}; @@ -180,6 +180,50 @@ fn labels(assists: &[Assist]) -> String { labels.into_iter().collect::() } +pub(crate) type NameSuggestion = fn(&ast::Expr, &Semantics<'_, RootDatabase>) -> Option; + +#[track_caller] +pub(crate) fn check_name_suggestion( + suggestion: NameSuggestion, + ra_fixture: &str, + suggested_name: &str, +) { + check_name(suggestion, ra_fixture, Some(suggested_name)); +} + +#[track_caller] +pub(crate) fn check_name_suggestion_not_applicable(suggestion: NameSuggestion, ra_fixture: &str) { + check_name(suggestion, ra_fixture, None); +} + +#[track_caller] +fn check_name(suggestion: NameSuggestion, ra_fixture: &str, expected: Option<&str>) { + let (db, file_with_carret_id, range_or_offset) = RootDatabase::with_range_or_offset(ra_fixture); + let frange = FileRange { file_id: file_with_carret_id, range: range_or_offset.into() }; + + let sema = Semantics::new(&db); + let source_file = sema.parse(frange.file_id); + let element = source_file.syntax().covering_element(frange.range); + let expr = + element.ancestors().find_map(ast::Expr::cast).expect("selection is not an expression"); + assert_eq!( + expr.syntax().text_range(), + frange.range, + "selection is not an expression(yet contained in one)" + ); + + let name = suggestion(&expr, &sema); + + match (name, expected) { + (Some(name), Some(expected_name)) => { + assert_eq_text!(&name, expected_name); + } + (Some(_), None) => panic!("name suggestion should not be applicable"), + (None, Some(_)) => panic!("name suggestion is not applicable"), + (None, None) => (), + } +} + #[test] fn assist_order_field_struct() { let before = "struct Foo { $0bar: u32 }"; diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index 880ab6fe3..62f959082 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -1,5 +1,7 @@ //! Assorted functions shared by several assists. +pub(crate) mod suggest_name; + use std::ops; use ast::TypeBoundsOwner; diff --git a/crates/ide_assists/src/utils/suggest_name.rs b/crates/ide_assists/src/utils/suggest_name.rs new file mode 100644 index 000000000..345e9af40 --- /dev/null +++ b/crates/ide_assists/src/utils/suggest_name.rs @@ -0,0 +1,770 @@ +//! This module contains functions to suggest names for expressions, functions and other items + +use hir::Semantics; +use ide_db::RootDatabase; +use itertools::Itertools; +use stdx::to_lower_snake_case; +use syntax::{ + ast::{self, NameOwner}, + match_ast, AstNode, +}; + +/// Trait names, that will be ignored when in `impl Trait` and `dyn Trait` +const USELESS_TRAITS: &[&str] = &["Send", "Sync", "Copy", "Clone", "Eq", "PartialEq"]; +/// Identifier names that won't be suggested, ever +/// +/// **NOTE**: they all must be snake lower case +const USELESS_NAMES: &[&str] = + &["new", "default", "option", "some", "none", "ok", "err", "str", "string"]; +/// Generic types replaced by their first argument +/// +/// # Examples +/// `Option` -> `Name` +/// `Result` -> `User` +const WRAPPER_TYPES: &[&str] = &["Box", "Option", "Result"]; +/// Prefixes to strip from methods names +/// +/// # Examples +/// `vec.as_slice()` -> `slice` +/// `args.into_config()` -> `config` +/// `bytes.to_vec()` -> `vec` +const USELESS_METHOD_PREFIXES: &[&str] = &["into_", "as_", "to_"]; + +/// Suggest name of variable for given expression +/// +/// **NOTE**: it is caller's responsibility to guarantee uniqueness of the name. +/// I.e. it doesn't look for names in scope. +/// +/// # Current implementation +/// +/// In current implementation, the function tries to get the name from +/// the following sources: +/// +/// * if expr is an argument to function/method, use paramter name +/// * if expr is a function/method call, use function name +/// * expression type name if it exists (E.g. `()`, `fn() -> ()` or `!` do not have names) +/// * fallback: `var_name` +/// +/// It also applies heuristics to filter out less informative names +/// +/// Currently it sticks to the first name found. +pub(crate) fn variable(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> String { + from_param(expr, sema) + .or_else(|| from_call(expr)) + .or_else(|| from_type(expr, sema)) + .unwrap_or_else(|| "var_name".to_string()) +} + +fn normalize(name: &str) -> Option { + let name = to_lower_snake_case(name); + + if USELESS_NAMES.contains(&name.as_str()) { + return None; + } + + if !is_valid_name(&name) { + return None; + } + + Some(name) +} + +fn is_valid_name(name: &str) -> bool { + match syntax::lex_single_syntax_kind(name) { + Some((syntax::SyntaxKind::IDENT, _error)) => true, + _ => false, + } +} + +fn from_call(expr: &ast::Expr) -> Option { + from_func_call(expr).or_else(|| from_method_call(expr)) +} + +fn from_func_call(expr: &ast::Expr) -> Option { + let call = match expr { + ast::Expr::CallExpr(call) => call, + _ => return None, + }; + let func = match call.expr()? { + ast::Expr::PathExpr(path) => path, + _ => return None, + }; + let ident = func.path()?.segment()?.name_ref()?.ident_token()?; + normalize(ident.text()) +} + +fn from_method_call(expr: &ast::Expr) -> Option { + let method = match expr { + ast::Expr::MethodCallExpr(call) => call, + _ => return None, + }; + let ident = method.name_ref()?.ident_token()?; + let name = normalize(ident.text())?; + + for prefix in USELESS_METHOD_PREFIXES { + if let Some(suffix) = name.strip_prefix(prefix) { + return Some(suffix.to_string()); + } + } + + Some(name) +} + +fn from_param(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> Option { + let arg_list = expr.syntax().parent().and_then(ast::ArgList::cast)?; + let args_parent = arg_list.syntax().parent()?; + let func = match_ast! { + match args_parent { + ast::CallExpr(call) => { + let func = call.expr()?; + let func_ty = sema.type_of_expr(&func)?; + func_ty.as_callable(sema.db)? + }, + ast::MethodCallExpr(method) => sema.resolve_method_call_as_callable(&method)?, + _ => return None, + } + }; + + let (idx, _) = arg_list.args().find_position(|it| it == expr).unwrap(); + let (pat, _) = func.params(sema.db).into_iter().nth(idx)?; + let pat = match pat? { + either::Either::Right(pat) => pat, + _ => return None, + }; + let name = var_name_from_pat(&pat)?; + normalize(&name.to_string()) +} + +fn var_name_from_pat(pat: &ast::Pat) -> Option { + match pat { + ast::Pat::IdentPat(var) => var.name(), + ast::Pat::RefPat(ref_pat) => var_name_from_pat(&ref_pat.pat()?), + ast::Pat::BoxPat(box_pat) => var_name_from_pat(&box_pat.pat()?), + _ => None, + } +} + +fn from_type(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> Option { + let ty = sema.type_of_expr(expr)?; + let ty = ty.remove_ref().unwrap_or(ty); + + name_of_type(&ty, sema.db) +} + +fn name_of_type(ty: &hir::Type, db: &RootDatabase) -> Option { + let name = if let Some(adt) = ty.as_adt() { + let name = adt.name(db).to_string(); + + if WRAPPER_TYPES.contains(&name.as_str()) { + let inner_ty = ty.type_parameters().next()?; + return name_of_type(&inner_ty, db); + } + + name + } else if let Some(trait_) = ty.as_dyn_trait() { + trait_name(&trait_, db)? + } else if let Some(traits) = ty.as_impl_traits(db) { + let mut iter = traits.into_iter().filter_map(|t| trait_name(&t, db)); + let name = iter.next()?; + if iter.next().is_some() { + return None; + } + name + } else { + return None; + }; + normalize(&name) +} + +fn trait_name(trait_: &hir::Trait, db: &RootDatabase) -> Option { + let name = trait_.name(db).to_string(); + if USELESS_TRAITS.contains(&name.as_str()) { + return None; + } + Some(name) +} + +#[cfg(test)] +mod tests { + use super::*; + + use crate::tests::check_name_suggestion; + + mod from_func_call { + use super::*; + + #[test] + fn no_args() { + check_name_suggestion( + |e, _| from_func_call(e), + r#" + fn foo() { + $0bar()$0 + }"#, + "bar", + ); + } + + #[test] + fn single_arg() { + check_name_suggestion( + |e, _| from_func_call(e), + r#" + fn foo() { + $0bar(1)$0 + }"#, + "bar", + ); + } + + #[test] + fn many_args() { + check_name_suggestion( + |e, _| from_func_call(e), + r#" + fn foo() { + $0bar(1, 2, 3)$0 + }"#, + "bar", + ); + } + + #[test] + fn path() { + check_name_suggestion( + |e, _| from_func_call(e), + r#" + fn foo() { + $0i32::bar(1, 2, 3)$0 + }"#, + "bar", + ); + } + + #[test] + fn generic_params() { + check_name_suggestion( + |e, _| from_func_call(e), + r#" + fn foo() { + $0bar::(1, 2, 3)$0 + }"#, + "bar", + ); + } + } + + mod from_method_call { + use super::*; + + #[test] + fn no_args() { + check_name_suggestion( + |e, _| from_method_call(e), + r#" + fn foo() { + $0bar.frobnicate()$0 + }"#, + "frobnicate", + ); + } + + #[test] + fn generic_params() { + check_name_suggestion( + |e, _| from_method_call(e), + r#" + fn foo() { + $0bar.frobnicate::()$0 + }"#, + "frobnicate", + ); + } + + #[test] + fn to_name() { + check_name_suggestion( + |e, _| from_method_call(e), + r#" + struct Args; + struct Config; + impl Args { + fn to_config(&self) -> Config {} + } + fn foo() { + $0Args.to_config()$0; + }"#, + "config", + ); + } + } + + mod from_param { + use crate::tests::check_name_suggestion_not_applicable; + + use super::*; + + #[test] + fn plain_func() { + check_name_suggestion( + from_param, + r#" + fn bar(n: i32, m: u32); + fn foo() { + bar($01$0, 2) + }"#, + "n", + ); + } + + #[test] + fn mut_param() { + check_name_suggestion( + from_param, + r#" + fn bar(mut n: i32, m: u32); + fn foo() { + bar($01$0, 2) + }"#, + "n", + ); + } + + #[test] + fn func_does_not_exist() { + check_name_suggestion_not_applicable( + from_param, + r#" + fn foo() { + bar($01$0, 2) + }"#, + ); + } + + #[test] + fn unnamed_param() { + check_name_suggestion_not_applicable( + from_param, + r#" + fn bar(_: i32, m: u32); + fn foo() { + bar($01$0, 2) + }"#, + ); + } + + #[test] + fn tuple_pat() { + check_name_suggestion_not_applicable( + from_param, + r#" + fn bar((n, k): (i32, i32), m: u32); + fn foo() { + bar($0(1, 2)$0, 3) + }"#, + ); + } + + #[test] + fn ref_pat() { + check_name_suggestion( + from_param, + r#" + fn bar(&n: &i32, m: u32); + fn foo() { + bar($0&1$0, 3) + }"#, + "n", + ); + } + + #[test] + fn box_pat() { + check_name_suggestion( + from_param, + r#" + fn bar(box n: &i32, m: u32); + fn foo() { + bar($01$0, 3) + }"#, + "n", + ); + } + + #[test] + fn param_out_of_index() { + check_name_suggestion_not_applicable( + from_param, + r#" + fn bar(n: i32, m: u32); + fn foo() { + bar(1, 2, $03$0) + }"#, + ); + } + + #[test] + fn generic_param_resolved() { + check_name_suggestion( + from_param, + r#" + fn bar(n: T, m: u32); + fn foo() { + bar($01$0, 2) + }"#, + "n", + ); + } + + #[test] + fn generic_param_unresolved() { + check_name_suggestion( + from_param, + r#" + fn bar(n: T, m: u32); + fn foo(x: T) { + bar($0x$0, 2) + }"#, + "n", + ); + } + + #[test] + fn method() { + check_name_suggestion( + from_param, + r#" + struct S; + impl S { + fn bar(&self, n: i32, m: u32); + } + fn foo() { + S.bar($01$0, 2) + }"#, + "n", + ); + } + + #[test] + fn method_ufcs() { + check_name_suggestion( + from_param, + r#" + struct S; + impl S { + fn bar(&self, n: i32, m: u32); + } + fn foo() { + S::bar(&S, $01$0, 2) + }"#, + "n", + ); + } + + #[test] + fn method_self() { + check_name_suggestion_not_applicable( + from_param, + r#" + struct S; + impl S { + fn bar(&self, n: i32, m: u32); + } + fn foo() { + S::bar($0&S$0, 1, 2) + }"#, + ); + } + + #[test] + fn method_self_named() { + check_name_suggestion( + from_param, + r#" + struct S; + impl S { + fn bar(strukt: &Self, n: i32, m: u32); + } + fn foo() { + S::bar($0&S$0, 1, 2) + }"#, + "strukt", + ); + } + } + + mod from_type { + use crate::tests::check_name_suggestion_not_applicable; + + use super::*; + + #[test] + fn i32() { + check_name_suggestion_not_applicable( + from_type, + r#" + fn foo() { + let _: i32 = $01$0; + }"#, + ); + } + + #[test] + fn u64() { + check_name_suggestion_not_applicable( + from_type, + r#" + fn foo() { + let _: u64 = $01$0; + }"#, + ); + } + + #[test] + fn bool() { + check_name_suggestion_not_applicable( + from_type, + r#" + fn foo() { + let _: bool = $0true$0; + }"#, + ); + } + + #[test] + fn struct_unit() { + check_name_suggestion( + from_type, + r#" + struct Seed; + fn foo() { + let _ = $0Seed$0; + }"#, + "seed", + ); + } + + #[test] + fn struct_unit_to_snake() { + check_name_suggestion( + from_type, + r#" + struct SeedState; + fn foo() { + let _ = $0SeedState$0; + }"#, + "seed_state", + ); + } + + #[test] + fn struct_single_arg() { + check_name_suggestion( + from_type, + r#" + struct Seed(u32); + fn foo() { + let _ = $0Seed(0)$0; + }"#, + "seed", + ); + } + + #[test] + fn struct_with_fields() { + check_name_suggestion( + from_type, + r#" + struct Seed { value: u32 } + fn foo() { + let _ = $0Seed { value: 0 }$0; + }"#, + "seed", + ); + } + + #[test] + fn enum_() { + check_name_suggestion( + from_type, + r#" + enum Kind { A, B } + fn foo() { + let _ = $0Kind::A$0; + }"#, + "kind", + ); + } + + #[test] + fn enum_generic_resolved() { + check_name_suggestion( + from_type, + r#" + enum Kind { A(T), B } + fn foo() { + let _ = $0Kind::A(1)$0; + }"#, + "kind", + ); + } + + #[test] + fn enum_generic_unresolved() { + check_name_suggestion( + from_type, + r#" + enum Kind { A(T), B } + fn foo(x: T) { + let _ = $0Kind::A(x)$0; + }"#, + "kind", + ); + } + + #[test] + fn dyn_trait() { + check_name_suggestion( + from_type, + r#" + trait DynHandler {} + fn bar() -> dyn DynHandler {} + fn foo() { + $0bar()$0; + }"#, + "dyn_handler", + ); + } + + #[test] + fn impl_trait() { + check_name_suggestion( + from_type, + r#" + trait StaticHandler {} + fn bar() -> impl StaticHandler {} + fn foo() { + $0bar()$0; + }"#, + "static_handler", + ); + } + + #[test] + fn impl_trait_plus_clone() { + check_name_suggestion( + from_type, + r#" + trait StaticHandler {} + trait Clone {} + fn bar() -> impl StaticHandler + Clone {} + fn foo() { + $0bar()$0; + }"#, + "static_handler", + ); + } + + #[test] + fn impl_trait_plus_lifetime() { + check_name_suggestion( + from_type, + r#" + trait StaticHandler {} + trait Clone {} + fn bar<'a>(&'a i32) -> impl StaticHandler + 'a {} + fn foo() { + $0bar(&1)$0; + }"#, + "static_handler", + ); + } + + #[test] + fn impl_trait_plus_trait() { + check_name_suggestion_not_applicable( + from_type, + r#" + trait Handler {} + trait StaticHandler {} + fn bar() -> impl StaticHandler + Handler {} + fn foo() { + $0bar()$0; + }"#, + ); + } + + #[test] + fn ref_value() { + check_name_suggestion( + from_type, + r#" + struct Seed; + fn bar() -> &Seed {} + fn foo() { + $0bar()$0; + }"#, + "seed", + ); + } + + #[test] + fn box_value() { + check_name_suggestion( + from_type, + r#" + struct Box(*const T); + struct Seed; + fn bar() -> Box {} + fn foo() { + $0bar()$0; + }"#, + "seed", + ); + } + + #[test] + fn box_generic() { + check_name_suggestion_not_applicable( + from_type, + r#" + struct Box(*const T); + fn bar() -> Box {} + fn foo() { + $0bar::()$0; + }"#, + ); + } + + #[test] + fn option_value() { + check_name_suggestion( + from_type, + r#" + enum Option { Some(T) } + struct Seed; + fn bar() -> Option {} + fn foo() { + $0bar()$0; + }"#, + "seed", + ); + } + + #[test] + fn result_value() { + check_name_suggestion( + from_type, + r#" + enum Result { Ok(T), Err(E) } + struct Seed; + struct Error; + fn bar() -> Result {} + fn foo() { + $0bar()$0; + }"#, + "seed", + ); + } + } +} -- cgit v1.2.3 From 7066e6b3620d06dbc2143b9dfdda4d7c97d6a8ba Mon Sep 17 00:00:00 2001 From: Vladyslav Katasonov Date: Wed, 17 Feb 2021 00:42:58 +0300 Subject: strip useless methods, and unary ops in suggest_name --- crates/ide_assists/src/utils/suggest_name.rs | 121 +++++++++++++++++++++++++-- 1 file changed, 114 insertions(+), 7 deletions(-) (limited to 'crates') diff --git a/crates/ide_assists/src/utils/suggest_name.rs b/crates/ide_assists/src/utils/suggest_name.rs index 345e9af40..d37c62642 100644 --- a/crates/ide_assists/src/utils/suggest_name.rs +++ b/crates/ide_assists/src/utils/suggest_name.rs @@ -29,6 +29,29 @@ const WRAPPER_TYPES: &[&str] = &["Box", "Option", "Result"]; /// `args.into_config()` -> `config` /// `bytes.to_vec()` -> `vec` const USELESS_METHOD_PREFIXES: &[&str] = &["into_", "as_", "to_"]; +/// Useless methods that are stripped from expression +/// +/// # Examples +/// `var.name().to_string()` -> `var.name()` +const USELESS_METHODS: &[&str] = &[ + "to_string", + "as_str", + "to_owned", + "as_ref", + "clone", + "cloned", + "expect", + "expect_none", + "unwrap", + "unwrap_none", + "unwrap_or", + "unwrap_or_default", + "unwrap_or_else", + "unwrap_unchecked", + "iter", + "into_iter", + "iter_mut", +]; /// Suggest name of variable for given expression /// @@ -49,10 +72,39 @@ const USELESS_METHOD_PREFIXES: &[&str] = &["into_", "as_", "to_"]; /// /// Currently it sticks to the first name found. pub(crate) fn variable(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> String { - from_param(expr, sema) - .or_else(|| from_call(expr)) - .or_else(|| from_type(expr, sema)) - .unwrap_or_else(|| "var_name".to_string()) + // `from_param` does not benifit from stripping + // it need the largest context possible + // so we check firstmost + if let Some(name) = from_param(expr, sema) { + return name; + } + + let mut next_expr = Some(expr.clone()); + while let Some(expr) = next_expr { + let name = from_call(&expr).or_else(|| from_type(&expr, sema)); + if let Some(name) = name { + return name; + } + + match expr { + ast::Expr::RefExpr(inner) => next_expr = inner.expr(), + ast::Expr::BoxExpr(inner) => next_expr = inner.expr(), + ast::Expr::AwaitExpr(inner) => next_expr = inner.expr(), + // ast::Expr::BlockExpr(block) => expr = block.tail_expr(), + ast::Expr::CastExpr(inner) => next_expr = inner.expr(), + ast::Expr::MethodCallExpr(method) if is_useless_method(&method) => { + next_expr = method.receiver(); + } + ast::Expr::ParenExpr(inner) => next_expr = inner.expr(), + ast::Expr::TryExpr(inner) => next_expr = inner.expr(), + ast::Expr::PrefixExpr(prefix) if prefix.op_kind() == Some(ast::PrefixOp::Deref) => { + next_expr = prefix.expr() + } + _ => break, + } + } + + "var_name".to_string() } fn normalize(name: &str) -> Option { @@ -76,6 +128,16 @@ fn is_valid_name(name: &str) -> bool { } } +fn is_useless_method(method: &ast::MethodCallExpr) -> bool { + let ident = method.name_ref().and_then(|it| it.ident_token()); + + if let Some(ident) = ident { + USELESS_METHODS.contains(&ident.text()) + } else { + false + } +} + fn from_call(expr: &ast::Expr) -> Option { from_func_call(expr).or_else(|| from_method_call(expr)) } @@ -99,15 +161,20 @@ fn from_method_call(expr: &ast::Expr) -> Option { _ => return None, }; let ident = method.name_ref()?.ident_token()?; - let name = normalize(ident.text())?; + let mut name = ident.text(); + + if USELESS_METHODS.contains(&name) { + return None; + } for prefix in USELESS_METHOD_PREFIXES { if let Some(suffix) = name.strip_prefix(prefix) { - return Some(suffix.to_string()); + name = suffix; + break; } } - Some(name) + normalize(&name) } fn from_param(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> Option { @@ -767,4 +834,44 @@ mod tests { ); } } + + mod variable { + use super::*; + + #[test] + fn ref_call() { + check_name_suggestion( + |e, c| Some(variable(e, c)), + r#" + fn foo() { + $0&bar(1, 3)$0 + }"#, + "bar", + ); + } + + #[test] + fn name_to_string() { + check_name_suggestion( + |e, c| Some(variable(e, c)), + r#" + fn foo() { + $0function.name().to_string()$0 + }"#, + "name", + ); + } + + #[test] + fn nested_useless_method() { + check_name_suggestion( + |e, c| Some(variable(e, c)), + r#" + fn foo() { + $0function.name().as_ref().unwrap().to_string()$0 + }"#, + "name", + ); + } + } } -- cgit v1.2.3