From cdeb1b2c78e07d326871bf5ee2bcebdf01806750 Mon Sep 17 00:00:00 2001 From: Steven Joruk Date: Tue, 1 Mar 2022 13:22:21 +0000 Subject: [PATCH 1/6] chore: fill_match_arms was renamed - update its usage in a comment --- crates/ide_assists/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 067f4d8e14..64d93083f8 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -42,7 +42,7 @@ //! useful and (worse) less predictable. The user should have a clear //! intuition when each particular assist is available. //! * Make small assists, which compose. Example: rather than auto-importing -//! enums in `fill_match_arms`, we use fully-qualified names. There's a +//! enums in `add_missing_match_arms`, we use fully-qualified names. There's a //! separate assist to shorten a fully-qualified name. //! * Distinguish between assists and fixits for diagnostics. Internally, fixits //! and assists are equivalent. They have the same "show a list + invoke a From 5b712bd8217f974e96019910aace4c66547fbacc Mon Sep 17 00:00:00 2001 From: Steven Joruk Date: Tue, 1 Mar 2022 13:03:51 +0000 Subject: [PATCH 2/6] feat: Add an assist for inlining type aliases This intends to lead to a more useful assist to replace all users of an alias with its definition. --- .../src/handlers/inline_type_alias.rs | 714 ++++++++++++++++++ crates/ide_assists/src/lib.rs | 2 + crates/ide_assists/src/tests/generated.rs | 21 + crates/syntax/src/ast/node_ext.rs | 9 + 4 files changed, 746 insertions(+) create mode 100644 crates/ide_assists/src/handlers/inline_type_alias.rs diff --git a/crates/ide_assists/src/handlers/inline_type_alias.rs b/crates/ide_assists/src/handlers/inline_type_alias.rs new file mode 100644 index 0000000000..f6c5762d0d --- /dev/null +++ b/crates/ide_assists/src/handlers/inline_type_alias.rs @@ -0,0 +1,714 @@ +// Some ideas for future improvements: +// - Support replacing aliases which are used in expressions, e.g. `A::new()`. +// - "inline_alias_to_users" assist #10881. +// - Remove unused aliases if there are no longer any users, see inline_call.rs. + +use hir::PathResolution; +use itertools::Itertools; +use std::collections::HashMap; +use syntax::{ + ast::{ + self, + make::{self}, + HasGenericParams, HasName, + }, + ted::{self}, + AstNode, NodeOrToken, SyntaxKind, SyntaxNode, +}; + +use crate::{ + assist_context::{AssistContext, Assists}, + AssistId, AssistKind, +}; + +// Assist: inline_type_alias +// +// Replace a type alias with its concrete type. +// +// ``` +// type A = Vec; +// +// fn main() { +// let a: $0A; +// } +// ``` +// -> +// ``` +// type A = Vec; +// +// fn main() { +// let a: Vec; +// } +// ``` +pub(crate) fn inline_type_alias(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let alias_instance = ctx.find_node_at_offset::()?; + let alias = get_type_alias(&ctx, &alias_instance)?; + let concrete_type = alias.ty()?; + + let replacement = if let Some(alias_generics) = alias.generic_param_list() { + get_replacement_for_generic_alias( + alias_instance.syntax().descendants().find_map(ast::GenericArgList::cast), + alias_generics, + &concrete_type, + )? + } else { + concrete_type.to_string() + }; + + let target = alias_instance.syntax().text_range(); + + acc.add( + AssistId("inline_type_alias", AssistKind::RefactorInline), + "Inline type alias", + target, + |builder| { + builder.replace(target, replacement); + }, + ) +} + +/// This doesn't attempt to ensure specified generics are compatible with those +/// required by the type alias, other than lifetimes which must either all be +/// specified or all omitted. It will replace TypeArgs with ConstArgs and vice +/// versa if they're in the wrong position. It supports partially specified +/// generics. +/// +/// 1. Map the provided instance's generic args to the type alias's generic +/// params: +/// +/// ``` +/// type A<'a, const N: usize, T = u64> = &'a [T; N]; +/// ^ alias generic params +/// let a: A<100>; +/// ^ instance generic args +/// ``` +/// +/// generic['a] = '_ due to omission +/// generic[N] = 100 due to the instance arg +/// generic[T] = u64 due to the default param +/// +/// 2. Copy the concrete type and substitute in each found mapping: +/// +/// &'_ [u64; 100] +/// +/// 3. Remove wildcard lifetimes entirely: +/// +/// &[u64; 100] +fn get_replacement_for_generic_alias( + instance_generic_args_list: Option, + alias_generics: ast::GenericParamList, + concrete_type: &ast::Type, +) -> Option { + if alias_generics.generic_params().count() == 0 { + cov_mark::hit!(no_generics_params); + return None; + } + + let mut lifetime_mappings = HashMap::<&str, ast::Lifetime>::new(); + let mut other_mappings = HashMap::::new(); + + let wildcard_lifetime = make::lifetime("'_"); + let alias_lifetimes = alias_generics.lifetime_params().map(|l| l.to_string()).collect_vec(); + for lifetime in &alias_lifetimes { + lifetime_mappings.insert(lifetime, wildcard_lifetime.clone()); + } + + if let Some(ref instance_generic_args_list) = instance_generic_args_list { + for (index, lifetime) in instance_generic_args_list + .lifetime_args() + .map(|arg| arg.lifetime().expect("LifetimeArg has a Lifetime")) + .enumerate() + { + if index >= alias_lifetimes.len() { + cov_mark::hit!(too_many_lifetimes); + return None; + } + + let key = &alias_lifetimes[index]; + + lifetime_mappings.insert(key, lifetime); + } + } + + let instance_generics = generic_args_to_other_generics(instance_generic_args_list); + let alias_generics = generic_param_list_to_other_generics(&alias_generics); + + if instance_generics.len() > alias_generics.len() { + cov_mark::hit!(too_many_generic_args); + return None; + } + + // Any declaration generics that don't have a default value must have one + // provided by the instance. + for (i, declaration_generic) in alias_generics.iter().enumerate() { + let key = declaration_generic.replacement_key(); + + if let Some(instance_generic) = instance_generics.get(i) { + other_mappings.insert(key, instance_generic.replacement_value()?); + } else if let Some(value) = declaration_generic.replacement_value() { + other_mappings.insert(key, value); + } else { + cov_mark::hit!(missing_replacement_param); + return None; + } + } + + let updated_concrete_type = concrete_type.clone_for_update(); + let mut replacements = Vec::new(); + let mut removals = Vec::new(); + + for syntax in updated_concrete_type.syntax().descendants() { + let syntax_string = syntax.to_string(); + let syntax_str = syntax_string.as_str(); + + if syntax.kind() == SyntaxKind::LIFETIME { + let new = lifetime_mappings.get(syntax_str).expect("lifetime is mapped"); + if new.text() == "'_" { + removals.push(NodeOrToken::Node(syntax.clone())); + + if let Some(ws) = syntax.next_sibling_or_token() { + removals.push(ws.clone()); + } + + continue; + } + + replacements.push((syntax.clone(), new.syntax().clone_for_update())); + } else if let Some(replacement_syntax) = other_mappings.get(syntax_str) { + let new_string = replacement_syntax.to_string(); + let new = if new_string == "_" { + make::wildcard_pat().syntax().clone_for_update() + } else { + replacement_syntax.clone_for_update() + }; + + replacements.push((syntax.clone(), new)); + } + } + + for (old, new) in replacements { + ted::replace(old, new); + } + + for syntax in removals { + ted::remove(syntax); + } + + Some(updated_concrete_type.to_string()) +} + +fn get_type_alias(ctx: &AssistContext, path: &ast::PathType) -> Option { + let resolved_path = ctx.sema.resolve_path(&path.path()?)?; + + // We need the generics in the correct order to be able to map any provided + // instance generics to declaration generics. The `hir::TypeAlias` doesn't + // keep the order, so we must get the `ast::TypeAlias` from the hir + // definition. + if let PathResolution::Def(hir::ModuleDef::TypeAlias(ta)) = resolved_path { + ast::TypeAlias::cast(ctx.sema.source(ta)?.syntax().value.clone()) + } else { + None + } +} + +enum OtherGeneric { + ConstArg(ast::ConstArg), + TypeArg(ast::TypeArg), + ConstParam(ast::ConstParam), + TypeParam(ast::TypeParam), +} + +impl OtherGeneric { + fn replacement_key(&self) -> String { + // Only params are used as replacement keys. + match self { + OtherGeneric::ConstArg(_) => unreachable!(), + OtherGeneric::TypeArg(_) => unreachable!(), + OtherGeneric::ConstParam(cp) => cp.name().expect("ConstParam has a name").to_string(), + OtherGeneric::TypeParam(tp) => tp.name().expect("TypeParam has a name").to_string(), + } + } + + fn replacement_value(&self) -> Option { + Some(match self { + OtherGeneric::ConstArg(ca) => ca.expr()?.syntax().clone(), + OtherGeneric::TypeArg(ta) => ta.syntax().clone(), + OtherGeneric::ConstParam(cp) => cp.default_val()?.syntax().clone(), + OtherGeneric::TypeParam(tp) => tp.default_type()?.syntax().clone(), + }) + } +} + +fn generic_param_list_to_other_generics(generics: &ast::GenericParamList) -> Vec { + let mut others = Vec::new(); + + for param in generics.generic_params() { + match param { + ast::GenericParam::LifetimeParam(_) => {} + ast::GenericParam::ConstParam(cp) => { + others.push(OtherGeneric::ConstParam(cp)); + } + ast::GenericParam::TypeParam(tp) => others.push(OtherGeneric::TypeParam(tp)), + } + } + + others +} + +fn generic_args_to_other_generics(generics: Option) -> Vec { + let mut others = Vec::new(); + + // It's fine for there to be no instance generics because the declaration + // might have default values or they might be inferred. + if let Some(generics) = generics { + for arg in generics.generic_args() { + match arg { + ast::GenericArg::TypeArg(ta) => { + others.push(OtherGeneric::TypeArg(ta)); + } + ast::GenericArg::ConstArg(ca) => { + others.push(OtherGeneric::ConstArg(ca)); + } + _ => {} + } + } + } + + others +} + +#[cfg(test)] +mod test { + use super::*; + use crate::tests::{check_assist, check_assist_not_applicable}; + + #[test] + fn empty_generic_params() { + cov_mark::check!(no_generics_params); + check_assist_not_applicable( + inline_type_alias, + r#" +type A<> = T; +fn main() { + let a: $0A; +} + "#, + ); + } + + #[test] + fn too_many_generic_args() { + cov_mark::check!(too_many_generic_args); + check_assist_not_applicable( + inline_type_alias, + r#" +type A = T; +fn main() { + let a: $0A; +} + "#, + ); + } + + #[test] + fn too_many_lifetimes() { + cov_mark::check!(too_many_lifetimes); + check_assist_not_applicable( + inline_type_alias, + r#" +type A<'a> = &'a &'b u32; +fn f<'a>() { + let a: $0A<'a, 'b> = 0; +} +"#, + ); + } + + // This must be supported in order to support "inline_alias_to_users" or + // whatever it will be called. + #[test] + fn alias_as_expression_ignored() { + check_assist_not_applicable( + inline_type_alias, + r#" +type A = Vec; +fn main() { + let a: A = $0A::new(); +} +"#, + ); + } + + #[test] + fn primitive_arg() { + check_assist( + inline_type_alias, + r#" +type A = T; +fn main() { + let a: $0A = 0; +} +"#, + r#" +type A = T; +fn main() { + let a: u32 = 0; +} +"#, + ); + } + + #[test] + fn no_generic_replacements() { + check_assist( + inline_type_alias, + r#" +type A = Vec; +fn main() { + let a: $0A; +} +"#, + r#" +type A = Vec; +fn main() { + let a: Vec; +} +"#, + ); + } + + #[test] + fn param_expression() { + check_assist( + inline_type_alias, + r#" +type A = [u32; N]; +fn main() { + let a: $0A; +} +"#, + r#" +type A = [u32; N]; +fn main() { + let a: [u32; { 1 }]; +} +"#, + ); + } + + #[test] + fn param_default_value() { + check_assist( + inline_type_alias, + r#" +type A = [u32; N]; +fn main() { + let a: $0A; +} +"#, + r#" +type A = [u32; N]; +fn main() { + let a: [u32; 1]; +} +"#, + ); + } + + #[test] + fn all_param_types() { + check_assist( + inline_type_alias, + r#" +struct Struct; +type A<'inner1, 'outer1, Outer1, const INNER1: usize, Inner1: Clone, const OUTER1: usize> = (Struct, Struct, Outer1, &'inner1 (), Inner1, &'outer1 ()); +fn foo<'inner2, 'outer2, Outer2, const INNER2: usize, Inner2, const OUTER2: usize>() { + let a: $0A<'inner2, 'outer2, Outer2, INNER2, Inner2, OUTER2>; +} +"#, + r#" +struct Struct; +type A<'inner1, 'outer1, Outer1, const INNER1: usize, Inner1: Clone, const OUTER1: usize> = (Struct, Struct, Outer1, &'inner1 (), Inner1, &'outer1 ()); +fn foo<'inner2, 'outer2, Outer2, const INNER2: usize, Inner2, const OUTER2: usize>() { + let a: (Struct, Struct, Outer2, &'inner2 (), Inner2, &'outer2 ()); +} +"#, + ); + } + + #[test] + fn omitted_lifetimes() { + check_assist( + inline_type_alias, + r#" +type A<'l, 'r> = &'l &'r u32; +fn main() { + let a: $0A; +} +"#, + r#" +type A<'l, 'r> = &'l &'r u32; +fn main() { + let a: &&u32; +} +"#, + ); + } + + #[test] + fn omitted_type() { + check_assist( + inline_type_alias, + r#" +type A<'r, 'l, T = u32> = &'l std::collections::HashMap<&'r str, T>; +fn main() { + let a: $0A<'_, '_>; +} +"#, + r#" +type A<'r, 'l, T = u32> = &'l std::collections::HashMap<&'r str, T>; +fn main() { + let a: &std::collections::HashMap<&str, u32>; +} +"#, + ); + } + + #[test] + fn omitted_everything() { + check_assist( + inline_type_alias, + r#" +type A<'r, 'l, T = u32> = &'l std::collections::HashMap<&'r str, T>; +fn main() { + let v = std::collections::HashMap<&str, u32>; + let a: $0A = &v; +} +"#, + r#" +type A<'r, 'l, T = u32> = &'l std::collections::HashMap<&'r str, T>; +fn main() { + let v = std::collections::HashMap<&str, u32>; + let a: &std::collections::HashMap<&str, u32> = &v; +} +"#, + ); + } + + // This doesn't actually cause the GenericArgsList to contain a AssocTypeArg. + #[test] + fn arg_associated_type() { + check_assist( + inline_type_alias, + r#" +trait Tra { type Assoc; fn a(); } +struct Str {} +impl Tra for Str { + type Assoc = u32; + fn a() { + type A = Vec; + let a: $0A; + } +} +"#, + r#" +trait Tra { type Assoc; fn a(); } +struct Str {} +impl Tra for Str { + type Assoc = u32; + fn a() { + type A = Vec; + let a: Vec; + } +} +"#, + ); + } + + #[test] + fn param_default_associated_type() { + check_assist( + inline_type_alias, + r#" +trait Tra { type Assoc; fn a() } +struct Str {} +impl Tra for Str { + type Assoc = u32; + fn a() { + type A = Vec; + let a: $0A; + } +} +"#, + r#" +trait Tra { type Assoc; fn a() } +struct Str {} +impl Tra for Str { + type Assoc = u32; + fn a() { + type A = Vec; + let a: Vec; + } +} +"#, + ); + } + + #[test] + fn function_pointer() { + check_assist( + inline_type_alias, + r#" +type A = fn(u32); +fn foo(a: u32) {} +fn main() { + let a: $0A = foo; +} +"#, + r#" +type A = fn(u32); +fn foo(a: u32) {} +fn main() { + let a: fn(u32) = foo; +} +"#, + ); + } + + #[test] + fn closure() { + check_assist( + inline_type_alias, + r#" +type A = Box u32>; +fn main() { + let a: $0A = Box::new(|_| 0); +} +"#, + r#" +type A = Box u32>; +fn main() { + let a: Box u32> = Box::new(|_| 0); +} +"#, + ); + } + + // Type aliases can't be used in traits, but someone might use the assist to + // fix the error. + #[test] + fn bounds() { + check_assist( + inline_type_alias, + r#"type A = std::io::Write; fn f() where T: $0A {}"#, + r#"type A = std::io::Write; fn f() where T: std::io::Write {}"#, + ); + } + + #[test] + fn function_parameter() { + check_assist( + inline_type_alias, + r#" +type A = std::io::Write; +fn f(a: impl $0A) {} +"#, + r#" +type A = std::io::Write; +fn f(a: impl std::io::Write) {} +"#, + ); + } + + #[test] + fn arg_expression() { + check_assist( + inline_type_alias, + r#" +type A = [u32; N]; +fn main() { + let a: $0A<{ 1 + 1 }>; +} +"#, + r#" +type A = [u32; N]; +fn main() { + let a: [u32; { 1 + 1 }]; +} +"#, + ) + } + + #[test] + fn alias_instance_generic_path() { + check_assist( + inline_type_alias, + r#" +type A = [u32; N]; +fn main() { + let a: $0A; +} +"#, + r#" +type A = [u32; N]; +fn main() { + let a: [u32; u32::MAX]; +} +"#, + ) + } + + #[test] + fn generic_type() { + check_assist( + inline_type_alias, + r#" +type A = String; +fn f(a: Vec<$0A>) {} +"#, + r#" +type A = String; +fn f(a: Vec) {} +"#, + ); + } + + #[test] + fn missing_replacement_param() { + cov_mark::check!(missing_replacement_param); + check_assist_not_applicable( + inline_type_alias, + r#" +type A = Vec; +fn main() { + let a: $0A; +} +"#, + ); + } + + #[test] + fn imported_external() { + check_assist( + inline_type_alias, + r#" +mod foo { + type A = String; +} +fn main() { + use foo::A; + let a: $0A; +} +"#, + r#" +mod foo { + type A = String; +} +fn main() { + use foo::A; + let a: String; +} +"#, + ); + } +} diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 64d93083f8..6eff8871e8 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -150,6 +150,7 @@ mod handlers { mod add_return_type; mod inline_call; mod inline_local_variable; + mod inline_type_alias; mod introduce_named_lifetime; mod invert_if; mod merge_imports; @@ -231,6 +232,7 @@ mod handlers { inline_call::inline_call, inline_call::inline_into_callers, inline_local_variable::inline_local_variable, + inline_type_alias::inline_type_alias, introduce_named_generic::introduce_named_generic, introduce_named_lifetime::introduce_named_lifetime, invert_if::invert_if, diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 485b807d05..84343daa6a 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -1239,6 +1239,27 @@ fn main() { ) } +#[test] +fn doctest_inline_type_alias() { + check_doc_test( + "inline_type_alias", + r#####" +type A = Vec; + +fn main() { + let a: $0A; +} +"#####, + r#####" +type A = Vec; + +fn main() { + let a: Vec; +} +"#####, + ) +} + #[test] fn doctest_introduce_named_generic() { check_doc_test( diff --git a/crates/syntax/src/ast/node_ext.rs b/crates/syntax/src/ast/node_ext.rs index 333ee35d63..e1d4addb52 100644 --- a/crates/syntax/src/ast/node_ext.rs +++ b/crates/syntax/src/ast/node_ext.rs @@ -764,6 +764,15 @@ impl ast::Meta { } } +impl ast::GenericArgList { + pub fn lifetime_args(&self) -> impl Iterator { + self.generic_args().filter_map(|arg| match arg { + ast::GenericArg::LifetimeArg(it) => Some(it), + _ => None, + }) + } +} + impl ast::GenericParamList { pub fn lifetime_params(&self) -> impl Iterator { self.generic_params().filter_map(|param| match param { From d7517d83232082b1d6c0ce7cdea7ba5fe3c70745 Mon Sep 17 00:00:00 2001 From: Steven Joruk Date: Sat, 12 Mar 2022 15:10:16 +0000 Subject: [PATCH 3/6] refactor: Veykril's code review suggestions --- .../src/handlers/inline_type_alias.rs | 217 ++++++++++-------- 1 file changed, 126 insertions(+), 91 deletions(-) diff --git a/crates/ide_assists/src/handlers/inline_type_alias.rs b/crates/ide_assists/src/handlers/inline_type_alias.rs index f6c5762d0d..e80e09260d 100644 --- a/crates/ide_assists/src/handlers/inline_type_alias.rs +++ b/crates/ide_assists/src/handlers/inline_type_alias.rs @@ -46,11 +46,19 @@ pub(crate) fn inline_type_alias(acc: &mut Assists, ctx: &AssistContext) -> Optio let concrete_type = alias.ty()?; let replacement = if let Some(alias_generics) = alias.generic_param_list() { - get_replacement_for_generic_alias( - alias_instance.syntax().descendants().find_map(ast::GenericArgList::cast), - alias_generics, + if alias_generics.generic_params().next().is_none() { + cov_mark::hit!(no_generics_params); + return None; + } + + let instance_args = + alias_instance.syntax().descendants().find_map(ast::GenericArgList::cast); + + create_replacement( + &LifetimeMap::new(&instance_args, &alias_generics)?, + &ConstAndTypeMap::new(&instance_args, &alias_generics)?, &concrete_type, - )? + ) } else { concrete_type.to_string() }; @@ -67,6 +75,83 @@ pub(crate) fn inline_type_alias(acc: &mut Assists, ctx: &AssistContext) -> Optio ) } +struct LifetimeMap(HashMap); + +impl LifetimeMap { + fn new( + instance_args: &Option, + alias_generics: &ast::GenericParamList, + ) -> Option { + let mut inner = HashMap::new(); + + let wildcard_lifetime = make::lifetime("'_"); + let lifetimes = alias_generics + .lifetime_params() + .filter_map(|lp| lp.lifetime()) + .map(|l| l.to_string()) + .collect_vec(); + + for lifetime in &lifetimes { + inner.insert(lifetime.to_string(), wildcard_lifetime.clone()); + } + + if let Some(instance_generic_args_list) = &instance_args { + for (index, lifetime) in instance_generic_args_list + .lifetime_args() + .filter_map(|arg| arg.lifetime()) + .enumerate() + { + let key = match lifetimes.get(index) { + Some(key) => key, + None => { + cov_mark::hit!(too_many_lifetimes); + return None; + } + }; + + inner.insert(key.clone(), lifetime); + } + } + + Some(Self(inner)) + } +} + +struct ConstAndTypeMap(HashMap); + +impl ConstAndTypeMap { + fn new( + instance_args: &Option, + alias_generics: &ast::GenericParamList, + ) -> Option { + let mut inner = HashMap::new(); + let instance_generics = generic_args_to_const_and_type_generics(instance_args); + let alias_generics = generic_param_list_to_const_and_type_generics(&alias_generics); + + if instance_generics.len() > alias_generics.len() { + cov_mark::hit!(too_many_generic_args); + return None; + } + + // Any declaration generics that don't have a default value must have one + // provided by the instance. + for (i, declaration_generic) in alias_generics.iter().enumerate() { + let key = declaration_generic.replacement_key()?; + + if let Some(instance_generic) = instance_generics.get(i) { + inner.insert(key, instance_generic.replacement_value()?); + } else if let Some(value) = declaration_generic.replacement_value() { + inner.insert(key, value); + } else { + cov_mark::hit!(missing_replacement_param); + return None; + } + } + + Some(Self(inner)) + } +} + /// This doesn't attempt to ensure specified generics are compatible with those /// required by the type alias, other than lifetimes which must either all be /// specified or all omitted. It will replace TypeArgs with ConstArgs and vice @@ -94,65 +179,11 @@ pub(crate) fn inline_type_alias(acc: &mut Assists, ctx: &AssistContext) -> Optio /// 3. Remove wildcard lifetimes entirely: /// /// &[u64; 100] -fn get_replacement_for_generic_alias( - instance_generic_args_list: Option, - alias_generics: ast::GenericParamList, +fn create_replacement( + lifetime_map: &LifetimeMap, + const_and_type_map: &ConstAndTypeMap, concrete_type: &ast::Type, -) -> Option { - if alias_generics.generic_params().count() == 0 { - cov_mark::hit!(no_generics_params); - return None; - } - - let mut lifetime_mappings = HashMap::<&str, ast::Lifetime>::new(); - let mut other_mappings = HashMap::::new(); - - let wildcard_lifetime = make::lifetime("'_"); - let alias_lifetimes = alias_generics.lifetime_params().map(|l| l.to_string()).collect_vec(); - for lifetime in &alias_lifetimes { - lifetime_mappings.insert(lifetime, wildcard_lifetime.clone()); - } - - if let Some(ref instance_generic_args_list) = instance_generic_args_list { - for (index, lifetime) in instance_generic_args_list - .lifetime_args() - .map(|arg| arg.lifetime().expect("LifetimeArg has a Lifetime")) - .enumerate() - { - if index >= alias_lifetimes.len() { - cov_mark::hit!(too_many_lifetimes); - return None; - } - - let key = &alias_lifetimes[index]; - - lifetime_mappings.insert(key, lifetime); - } - } - - let instance_generics = generic_args_to_other_generics(instance_generic_args_list); - let alias_generics = generic_param_list_to_other_generics(&alias_generics); - - if instance_generics.len() > alias_generics.len() { - cov_mark::hit!(too_many_generic_args); - return None; - } - - // Any declaration generics that don't have a default value must have one - // provided by the instance. - for (i, declaration_generic) in alias_generics.iter().enumerate() { - let key = declaration_generic.replacement_key(); - - if let Some(instance_generic) = instance_generics.get(i) { - other_mappings.insert(key, instance_generic.replacement_value()?); - } else if let Some(value) = declaration_generic.replacement_value() { - other_mappings.insert(key, value); - } else { - cov_mark::hit!(missing_replacement_param); - return None; - } - } - +) -> String { let updated_concrete_type = concrete_type.clone_for_update(); let mut replacements = Vec::new(); let mut removals = Vec::new(); @@ -161,20 +192,21 @@ fn get_replacement_for_generic_alias( let syntax_string = syntax.to_string(); let syntax_str = syntax_string.as_str(); - if syntax.kind() == SyntaxKind::LIFETIME { - let new = lifetime_mappings.get(syntax_str).expect("lifetime is mapped"); - if new.text() == "'_" { - removals.push(NodeOrToken::Node(syntax.clone())); + if let Some(old_lifetime) = ast::Lifetime::cast(syntax.clone()) { + if let Some(new_lifetime) = lifetime_map.0.get(&old_lifetime.to_string()) { + if new_lifetime.text() == "'_" { + removals.push(NodeOrToken::Node(syntax.clone())); - if let Some(ws) = syntax.next_sibling_or_token() { - removals.push(ws.clone()); + if let Some(ws) = syntax.next_sibling_or_token() { + removals.push(ws.clone()); + } + + continue; } - continue; + replacements.push((syntax.clone(), new_lifetime.syntax().clone_for_update())); } - - replacements.push((syntax.clone(), new.syntax().clone_for_update())); - } else if let Some(replacement_syntax) = other_mappings.get(syntax_str) { + } else if let Some(replacement_syntax) = const_and_type_map.0.get(syntax_str) { let new_string = replacement_syntax.to_string(); let new = if new_string == "_" { make::wildcard_pat().syntax().clone_for_update() @@ -194,7 +226,7 @@ fn get_replacement_for_generic_alias( ted::remove(syntax); } - Some(updated_concrete_type.to_string()) + updated_concrete_type.to_string() } fn get_type_alias(ctx: &AssistContext, path: &ast::PathType) -> Option { @@ -205,57 +237,60 @@ fn get_type_alias(ctx: &AssistContext, path: &ast::PathType) -> Option String { +impl ConstOrTypeGeneric { + fn replacement_key(&self) -> Option { // Only params are used as replacement keys. match self { - OtherGeneric::ConstArg(_) => unreachable!(), - OtherGeneric::TypeArg(_) => unreachable!(), - OtherGeneric::ConstParam(cp) => cp.name().expect("ConstParam has a name").to_string(), - OtherGeneric::TypeParam(tp) => tp.name().expect("TypeParam has a name").to_string(), + ConstOrTypeGeneric::ConstParam(cp) => Some(cp.name()?.to_string()), + ConstOrTypeGeneric::TypeParam(tp) => Some(tp.name()?.to_string()), + _ => None, } } fn replacement_value(&self) -> Option { Some(match self { - OtherGeneric::ConstArg(ca) => ca.expr()?.syntax().clone(), - OtherGeneric::TypeArg(ta) => ta.syntax().clone(), - OtherGeneric::ConstParam(cp) => cp.default_val()?.syntax().clone(), - OtherGeneric::TypeParam(tp) => tp.default_type()?.syntax().clone(), + ConstOrTypeGeneric::ConstArg(ca) => ca.expr()?.syntax().clone(), + ConstOrTypeGeneric::TypeArg(ta) => ta.syntax().clone(), + ConstOrTypeGeneric::ConstParam(cp) => cp.default_val()?.syntax().clone(), + ConstOrTypeGeneric::TypeParam(tp) => tp.default_type()?.syntax().clone(), }) } } -fn generic_param_list_to_other_generics(generics: &ast::GenericParamList) -> Vec { +fn generic_param_list_to_const_and_type_generics( + generics: &ast::GenericParamList, +) -> Vec { let mut others = Vec::new(); for param in generics.generic_params() { match param { ast::GenericParam::LifetimeParam(_) => {} ast::GenericParam::ConstParam(cp) => { - others.push(OtherGeneric::ConstParam(cp)); + others.push(ConstOrTypeGeneric::ConstParam(cp)); } - ast::GenericParam::TypeParam(tp) => others.push(OtherGeneric::TypeParam(tp)), + ast::GenericParam::TypeParam(tp) => others.push(ConstOrTypeGeneric::TypeParam(tp)), } } others } -fn generic_args_to_other_generics(generics: Option) -> Vec { +fn generic_args_to_const_and_type_generics( + generics: &Option, +) -> Vec { let mut others = Vec::new(); // It's fine for there to be no instance generics because the declaration @@ -264,10 +299,10 @@ fn generic_args_to_other_generics(generics: Option) -> Vec< for arg in generics.generic_args() { match arg { ast::GenericArg::TypeArg(ta) => { - others.push(OtherGeneric::TypeArg(ta)); + others.push(ConstOrTypeGeneric::TypeArg(ta)); } ast::GenericArg::ConstArg(ca) => { - others.push(OtherGeneric::ConstArg(ca)); + others.push(ConstOrTypeGeneric::ConstArg(ca)); } _ => {} } From 2e00fa208fa9552ac8ddb56c253b89901d9515f9 Mon Sep 17 00:00:00 2001 From: Steven Joruk Date: Sat, 12 Mar 2022 15:59:00 +0000 Subject: [PATCH 4/6] chore: Remove unused import --- crates/ide_assists/src/handlers/inline_type_alias.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ide_assists/src/handlers/inline_type_alias.rs b/crates/ide_assists/src/handlers/inline_type_alias.rs index e80e09260d..ed8f22dd66 100644 --- a/crates/ide_assists/src/handlers/inline_type_alias.rs +++ b/crates/ide_assists/src/handlers/inline_type_alias.rs @@ -13,7 +13,7 @@ use syntax::{ HasGenericParams, HasName, }, ted::{self}, - AstNode, NodeOrToken, SyntaxKind, SyntaxNode, + AstNode, NodeOrToken, SyntaxNode, }; use crate::{ From 1981a3dc655b742c8b5b681abbca3545a2be485c Mon Sep 17 00:00:00 2001 From: Steven Joruk Date: Sun, 13 Mar 2022 13:25:06 +0000 Subject: [PATCH 5/6] test: Make imported_external test that the full path is replaced --- crates/ide_assists/src/handlers/inline_type_alias.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/ide_assists/src/handlers/inline_type_alias.rs b/crates/ide_assists/src/handlers/inline_type_alias.rs index ed8f22dd66..0ffa339cda 100644 --- a/crates/ide_assists/src/handlers/inline_type_alias.rs +++ b/crates/ide_assists/src/handlers/inline_type_alias.rs @@ -723,24 +723,22 @@ fn main() { } #[test] - fn imported_external() { + fn full_path_type_is_replaced() { check_assist( inline_type_alias, r#" mod foo { - type A = String; + pub type A = String; } fn main() { - use foo::A; - let a: $0A; + let a: foo::$0A; } "#, r#" mod foo { - type A = String; + pub type A = String; } fn main() { - use foo::A; let a: String; } "#, From 1381a2de230e19944a71e8176936ac90764fbd6e Mon Sep 17 00:00:00 2001 From: Steven Joruk Date: Sun, 20 Mar 2022 20:53:03 +0000 Subject: [PATCH 6/6] refactor: Do more work in inline_type_alias closure --- .../src/handlers/inline_type_alias.rs | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/crates/ide_assists/src/handlers/inline_type_alias.rs b/crates/ide_assists/src/handlers/inline_type_alias.rs index 0ffa339cda..eeb2e2e667 100644 --- a/crates/ide_assists/src/handlers/inline_type_alias.rs +++ b/crates/ide_assists/src/handlers/inline_type_alias.rs @@ -45,6 +45,11 @@ pub(crate) fn inline_type_alias(acc: &mut Assists, ctx: &AssistContext) -> Optio let alias = get_type_alias(&ctx, &alias_instance)?; let concrete_type = alias.ty()?; + enum Replacement { + Generic { lifetime_map: LifetimeMap, const_and_type_map: ConstAndTypeMap }, + Plain, + } + let replacement = if let Some(alias_generics) = alias.generic_param_list() { if alias_generics.generic_params().next().is_none() { cov_mark::hit!(no_generics_params); @@ -54,13 +59,12 @@ pub(crate) fn inline_type_alias(acc: &mut Assists, ctx: &AssistContext) -> Optio let instance_args = alias_instance.syntax().descendants().find_map(ast::GenericArgList::cast); - create_replacement( - &LifetimeMap::new(&instance_args, &alias_generics)?, - &ConstAndTypeMap::new(&instance_args, &alias_generics)?, - &concrete_type, - ) + Replacement::Generic { + lifetime_map: LifetimeMap::new(&instance_args, &alias_generics)?, + const_and_type_map: ConstAndTypeMap::new(&instance_args, &alias_generics)?, + } } else { - concrete_type.to_string() + Replacement::Plain }; let target = alias_instance.syntax().text_range(); @@ -70,7 +74,14 @@ pub(crate) fn inline_type_alias(acc: &mut Assists, ctx: &AssistContext) -> Optio "Inline type alias", target, |builder| { - builder.replace(target, replacement); + let replacement_text = match replacement { + Replacement::Generic { lifetime_map, const_and_type_map } => { + create_replacement(&lifetime_map, &const_and_type_map, &concrete_type) + } + Replacement::Plain => concrete_type.to_string(), + }; + + builder.replace(target, replacement_text); }, ) }