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 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