From 2ab79c6f4d972796d3fd9f4b7fae3342a551f5e8 Mon Sep 17 00:00:00 2001 From: Jess Balint Date: Wed, 20 May 2020 18:37:09 -0500 Subject: [PATCH 1/7] Assist: replace anonymous lifetime with a named one (fixes #4523) --- .../handlers/change_lifetime_anon_to_named.rs | 123 ++++++++++++++++++ crates/ra_assists/src/lib.rs | 2 + docs/user/assists.md | 24 ++++ 3 files changed, 149 insertions(+) create mode 100644 crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs diff --git a/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs new file mode 100644 index 0000000000..63f0a7dabf --- /dev/null +++ b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs @@ -0,0 +1,123 @@ +use crate::{AssistContext, AssistId, Assists}; +use ra_syntax::{ast, ast::{TypeParamsOwner}, AstNode, SyntaxKind}; + +/// Assist: change_lifetime_anon_to_named +/// +/// Change an anonymous lifetime to a named lifetime. +/// +/// ``` +/// impl Cursor<'_<|>> { +/// fn node(self) -> &SyntaxNode { +/// match self { +/// Cursor::Replace(node) | Cursor::Before(node) => node, +/// } +/// } +/// } +/// ``` +/// -> +/// ``` +/// impl<'a> Cursor<'a> { +/// fn node(self) -> &SyntaxNode { +/// match self { +/// Cursor::Replace(node) | Cursor::Before(node) => node, +/// } +/// } +/// } +/// ``` +// TODO : How can we handle renaming any one of multiple anonymous lifetimes? +pub(crate) fn change_lifetime_anon_to_named(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let lifetime_token = ctx.find_token_at_offset(SyntaxKind::LIFETIME)?; + let lifetime_arg = ast::LifetimeArg::cast(lifetime_token.parent())?; + if lifetime_arg.syntax().text() != "'_" { + return None; + } + let next_token = lifetime_token.next_token()?; + if next_token.kind() != SyntaxKind::R_ANGLE { + // only allow naming the last anonymous lifetime + return None; + } + match lifetime_arg.syntax().ancestors().find_map(ast::ImplDef::cast) { + Some(impl_def) => { + // get the `impl` keyword so we know where to add the lifetime argument + let impl_kw = impl_def.syntax().first_child_or_token()?.into_token()?; + if impl_kw.kind() != SyntaxKind::IMPL_KW { + return None; + } + acc.add( + AssistId("change_lifetime_anon_to_named"), + "Give anonymous lifetime a name", + lifetime_arg.syntax().text_range(), + |builder| { + match impl_def.type_param_list() { + Some(type_params) => { + builder.insert( + (u32::from(type_params.syntax().text_range().end()) - 1).into(), + ", 'a", + ); + }, + None => { + builder.insert( + impl_kw.text_range().end(), + "<'a>", + ); + }, + } + builder.replace(lifetime_arg.syntax().text_range(), "'a"); + }, + ) + } + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests::{check_assist, check_assist_not_applicable}; + + #[test] + fn test_example_case() { + check_assist( + change_lifetime_anon_to_named, + r#"impl Cursor<'_<|>> { + fn node(self) -> &SyntaxNode { + match self { + Cursor::Replace(node) | Cursor::Before(node) => node, + } + } + }"#, + r#"impl<'a> Cursor<'a> { + fn node(self) -> &SyntaxNode { + match self { + Cursor::Replace(node) | Cursor::Before(node) => node, + } + } + }"#, + ); + } + + #[test] + fn test_example_case_simplified() { + check_assist( + change_lifetime_anon_to_named, + r#"impl Cursor<'_<|>> {"#, + r#"impl<'a> Cursor<'a> {"#, + ); + } + + #[test] + fn test_not_applicable() { + check_assist_not_applicable(change_lifetime_anon_to_named, r#"impl Cursor<'_><|> {"#); + check_assist_not_applicable(change_lifetime_anon_to_named, r#"impl Cursor<|><'_> {"#); + check_assist_not_applicable(change_lifetime_anon_to_named, r#"impl Cursor<'a<|>> {"#); + } + + #[test] + fn test_with_type_parameter() { + check_assist( + change_lifetime_anon_to_named, + r#"impl Cursor>"#, + r#"impl Cursor"#, + ); + } +} diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 464bc03dde..3f8f7ffbfc 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -112,6 +112,7 @@ mod handlers { mod add_turbo_fish; mod apply_demorgan; mod auto_import; + mod change_lifetime_anon_to_named; mod change_return_type_to_result; mod change_visibility; mod early_return; @@ -151,6 +152,7 @@ mod handlers { add_turbo_fish::add_turbo_fish, apply_demorgan::apply_demorgan, auto_import::auto_import, + change_lifetime_anon_to_named::change_lifetime_anon_to_named, change_return_type_to_result::change_return_type_to_result, change_visibility::change_visibility, early_return::convert_to_guarded_return, diff --git a/docs/user/assists.md b/docs/user/assists.md index 4ad7ea59d2..29b64330e9 100644 --- a/docs/user/assists.md +++ b/docs/user/assists.md @@ -259,6 +259,30 @@ fn main() { } ``` +## `change_lifetime_anon_to_named` + +Change an anonymous lifetime to a named lifetime. + +```rust +// BEFORE +impl Cursor<'_<|>> { + fn node(self) -> &SyntaxNode { + match self { + Cursor::Replace(node) | Cursor::Before(node) => node, + } + } +} + +// AFTER +impl<'a> Cursor<'a> { + fn node(self) -> &SyntaxNode { + match self { + Cursor::Replace(node) | Cursor::Before(node) => node, + } + } +} +``` + ## `change_return_type_to_result` Change the function's return type to Result. From 1fae96a8d4fb452216906f8909d421ad884fd929 Mon Sep 17 00:00:00 2001 From: Jess Balint Date: Fri, 22 May 2020 08:51:37 -0500 Subject: [PATCH 2/7] handle the case of conflicting lifetime param name - and clean/format code --- .../handlers/change_lifetime_anon_to_named.rs | 43 ++++++++++++++++--- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs index 63f0a7dabf..8d8f7833f2 100644 --- a/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs +++ b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs @@ -1,5 +1,6 @@ use crate::{AssistContext, AssistId, Assists}; -use ra_syntax::{ast, ast::{TypeParamsOwner}, AstNode, SyntaxKind}; +use ra_syntax::{ast, ast::TypeParamsOwner, AstNode, SyntaxKind}; +use std::collections::HashSet; /// Assist: change_lifetime_anon_to_named /// @@ -24,7 +25,7 @@ use ra_syntax::{ast, ast::{TypeParamsOwner}, AstNode, SyntaxKind}; /// } /// } /// ``` -// TODO : How can we handle renaming any one of multiple anonymous lifetimes? +// FIXME: How can we handle renaming any one of multiple anonymous lifetimes? pub(crate) fn change_lifetime_anon_to_named(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let lifetime_token = ctx.find_token_at_offset(SyntaxKind::LIFETIME)?; let lifetime_arg = ast::LifetimeArg::cast(lifetime_token.parent())?; @@ -43,6 +44,22 @@ pub(crate) fn change_lifetime_anon_to_named(acc: &mut Assists, ctx: &AssistConte if impl_kw.kind() != SyntaxKind::IMPL_KW { return None; } + let new_lifetime_param = match impl_def.type_param_list() { + Some(type_params) => { + let used_lifetime_params: HashSet<_> = type_params + .lifetime_params() + .map(|p| { + let mut param_name = p.syntax().text().to_string(); + param_name.remove(0); + param_name + }) + .collect(); + "abcdefghijklmnopqrstuvwxyz" + .chars() + .find(|c| !used_lifetime_params.contains(&c.to_string()))? + } + None => 'a', + }; acc.add( AssistId("change_lifetime_anon_to_named"), "Give anonymous lifetime a name", @@ -52,17 +69,20 @@ pub(crate) fn change_lifetime_anon_to_named(acc: &mut Assists, ctx: &AssistConte Some(type_params) => { builder.insert( (u32::from(type_params.syntax().text_range().end()) - 1).into(), - ", 'a", + format!(", '{}", new_lifetime_param), ); - }, + } None => { builder.insert( impl_kw.text_range().end(), - "<'a>", + format!("<'{}>", new_lifetime_param), ); - }, + } } - builder.replace(lifetime_arg.syntax().text_range(), "'a"); + builder.replace( + lifetime_arg.syntax().text_range(), + format!("'{}", new_lifetime_param), + ); }, ) } @@ -120,4 +140,13 @@ mod tests { r#"impl Cursor"#, ); } + + #[test] + fn test_with_existing_lifetime_name_conflict() { + check_assist( + change_lifetime_anon_to_named, + r#"impl<'a, 'b> Cursor<'a, 'b, '_<|>>"#, + r#"impl<'a, 'b, 'c> Cursor<'a, 'b, 'c>"#, + ); + } } From 6594235dd80d337dd7bfaa8be876ba5e111526b1 Mon Sep 17 00:00:00 2001 From: Jess Balint Date: Fri, 22 May 2020 09:20:43 -0500 Subject: [PATCH 3/7] Remove doc using `cargo xtask codegen`. --- docs/user/assists.md | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/docs/user/assists.md b/docs/user/assists.md index 29b64330e9..4ad7ea59d2 100644 --- a/docs/user/assists.md +++ b/docs/user/assists.md @@ -259,30 +259,6 @@ fn main() { } ``` -## `change_lifetime_anon_to_named` - -Change an anonymous lifetime to a named lifetime. - -```rust -// BEFORE -impl Cursor<'_<|>> { - fn node(self) -> &SyntaxNode { - match self { - Cursor::Replace(node) | Cursor::Before(node) => node, - } - } -} - -// AFTER -impl<'a> Cursor<'a> { - fn node(self) -> &SyntaxNode { - match self { - Cursor::Replace(node) | Cursor::Before(node) => node, - } - } -} -``` - ## `change_return_type_to_result` Change the function's return type to Result. From 1f9e02c74e7822a490466a605fa384ce1b5e3afe Mon Sep 17 00:00:00 2001 From: Jess Balint Date: Fri, 22 May 2020 09:25:55 -0500 Subject: [PATCH 4/7] fix generated docs issue --- .../handlers/change_lifetime_anon_to_named.rs | 46 +++++++++---------- crates/ra_assists/src/tests/generated.rs | 25 ++++++++++ docs/user/assists.md | 24 ++++++++++ 3 files changed, 72 insertions(+), 23 deletions(-) diff --git a/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs index 8d8f7833f2..7101d9aaff 100644 --- a/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs +++ b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs @@ -2,29 +2,29 @@ use crate::{AssistContext, AssistId, Assists}; use ra_syntax::{ast, ast::TypeParamsOwner, AstNode, SyntaxKind}; use std::collections::HashSet; -/// Assist: change_lifetime_anon_to_named -/// -/// Change an anonymous lifetime to a named lifetime. -/// -/// ``` -/// impl Cursor<'_<|>> { -/// fn node(self) -> &SyntaxNode { -/// match self { -/// Cursor::Replace(node) | Cursor::Before(node) => node, -/// } -/// } -/// } -/// ``` -/// -> -/// ``` -/// impl<'a> Cursor<'a> { -/// fn node(self) -> &SyntaxNode { -/// match self { -/// Cursor::Replace(node) | Cursor::Before(node) => node, -/// } -/// } -/// } -/// ``` +// Assist: change_lifetime_anon_to_named +// +// Change an anonymous lifetime to a named lifetime. +// +// ``` +// impl Cursor<'_<|>> { +// fn node(self) -> &SyntaxNode { +// match self { +// Cursor::Replace(node) | Cursor::Before(node) => node, +// } +// } +// } +// ``` +// -> +// ``` +// impl<'a> Cursor<'a> { +// fn node(self) -> &SyntaxNode { +// match self { +// Cursor::Replace(node) | Cursor::Before(node) => node, +// } +// } +// } +// ``` // FIXME: How can we handle renaming any one of multiple anonymous lifetimes? pub(crate) fn change_lifetime_anon_to_named(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { let lifetime_token = ctx.find_token_at_offset(SyntaxKind::LIFETIME)?; diff --git a/crates/ra_assists/src/tests/generated.rs b/crates/ra_assists/src/tests/generated.rs index 250e56a696..abffbf97cf 100644 --- a/crates/ra_assists/src/tests/generated.rs +++ b/crates/ra_assists/src/tests/generated.rs @@ -268,6 +268,31 @@ pub mod std { pub mod collections { pub struct HashMap { } } } ) } +#[test] +fn doctest_change_lifetime_anon_to_named() { + check_doc_test( + "change_lifetime_anon_to_named", + r#####" +impl Cursor<'_<|>> { + fn node(self) -> &SyntaxNode { + match self { + Cursor::Replace(node) | Cursor::Before(node) => node, + } + } +} +"#####, + r#####" +impl<'a> Cursor<'a> { + fn node(self) -> &SyntaxNode { + match self { + Cursor::Replace(node) | Cursor::Before(node) => node, + } + } +} +"#####, + ) +} + #[test] fn doctest_change_return_type_to_result() { check_doc_test( diff --git a/docs/user/assists.md b/docs/user/assists.md index 4ad7ea59d2..a1058ecde9 100644 --- a/docs/user/assists.md +++ b/docs/user/assists.md @@ -259,6 +259,30 @@ fn main() { } ``` +## `change_lifetime_anon_to_named` + +Change an anonymous lifetime to a named lifetime. + +```rust +// BEFORE +impl Cursor<'_┃> { + fn node(self) -> &SyntaxNode { + match self { + Cursor::Replace(node) | Cursor::Before(node) => node, + } + } +} + +// AFTER +impl<'a> Cursor<'a> { + fn node(self) -> &SyntaxNode { + match self { + Cursor::Replace(node) | Cursor::Before(node) => node, + } + } +} +``` + ## `change_return_type_to_result` Change the function's return type to Result. From d42fd8efb6019c08371007df68ef3a38c2e86a45 Mon Sep 17 00:00:00 2001 From: Jess Balint Date: Fri, 22 May 2020 10:15:59 -0500 Subject: [PATCH 5/7] use char range --- .../ra_assists/src/handlers/change_lifetime_anon_to_named.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs index 7101d9aaff..67d50cfda5 100644 --- a/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs +++ b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs @@ -54,8 +54,8 @@ pub(crate) fn change_lifetime_anon_to_named(acc: &mut Assists, ctx: &AssistConte param_name }) .collect(); - "abcdefghijklmnopqrstuvwxyz" - .chars() + (b'a'..=b'z') + .map(char::from) .find(|c| !used_lifetime_params.contains(&c.to_string()))? } None => 'a', From 4967b811dda4eae3910514ea8788f0fab256ad8c Mon Sep 17 00:00:00 2001 From: Jess Balint Date: Fri, 22 May 2020 19:09:37 -0500 Subject: [PATCH 6/7] tweak syntax --- .../handlers/change_lifetime_anon_to_named.rs | 91 +++++++++---------- 1 file changed, 41 insertions(+), 50 deletions(-) diff --git a/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs index 67d50cfda5..9222136070 100644 --- a/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs +++ b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs @@ -37,57 +37,48 @@ pub(crate) fn change_lifetime_anon_to_named(acc: &mut Assists, ctx: &AssistConte // only allow naming the last anonymous lifetime return None; } - match lifetime_arg.syntax().ancestors().find_map(ast::ImplDef::cast) { - Some(impl_def) => { - // get the `impl` keyword so we know where to add the lifetime argument - let impl_kw = impl_def.syntax().first_child_or_token()?.into_token()?; - if impl_kw.kind() != SyntaxKind::IMPL_KW { - return None; - } - let new_lifetime_param = match impl_def.type_param_list() { - Some(type_params) => { - let used_lifetime_params: HashSet<_> = type_params - .lifetime_params() - .map(|p| { - let mut param_name = p.syntax().text().to_string(); - param_name.remove(0); - param_name - }) - .collect(); - (b'a'..=b'z') - .map(char::from) - .find(|c| !used_lifetime_params.contains(&c.to_string()))? - } - None => 'a', - }; - acc.add( - AssistId("change_lifetime_anon_to_named"), - "Give anonymous lifetime a name", - lifetime_arg.syntax().text_range(), - |builder| { - match impl_def.type_param_list() { - Some(type_params) => { - builder.insert( - (u32::from(type_params.syntax().text_range().end()) - 1).into(), - format!(", '{}", new_lifetime_param), - ); - } - None => { - builder.insert( - impl_kw.text_range().end(), - format!("<'{}>", new_lifetime_param), - ); - } - } - builder.replace( - lifetime_arg.syntax().text_range(), - format!("'{}", new_lifetime_param), - ); - }, - ) - } - _ => None, + let impl_def = lifetime_arg.syntax().ancestors().find_map(ast::ImplDef::cast)?; + // get the `impl` keyword so we know where to add the lifetime argument + let impl_kw = impl_def.syntax().first_child_or_token()?.into_token()?; + if impl_kw.kind() != SyntaxKind::IMPL_KW { + return None; } + let new_lifetime_param = match impl_def.type_param_list() { + Some(type_params) => { + let used_lifetime_params: HashSet<_> = type_params + .lifetime_params() + .map(|p| { + let mut param_name = p.syntax().text().to_string(); + param_name.remove(0); + param_name + }) + .collect(); + (b'a'..=b'z') + .map(char::from) + .find(|c| !used_lifetime_params.contains(&c.to_string()))? + } + None => 'a', + }; + acc.add( + AssistId("change_lifetime_anon_to_named"), + "Give anonymous lifetime a name", + lifetime_arg.syntax().text_range(), + |builder| { + match impl_def.type_param_list() { + Some(type_params) => { + builder.insert( + (u32::from(type_params.syntax().text_range().end()) - 1).into(), + format!(", '{}", new_lifetime_param), + ); + } + None => { + builder + .insert(impl_kw.text_range().end(), format!("<'{}>", new_lifetime_param)); + } + } + builder.replace(lifetime_arg.syntax().text_range(), format!("'{}", new_lifetime_param)); + }, + ) } #[cfg(test)] From bd8aa04bae5280c9b5248413f2370f0773ac73aa Mon Sep 17 00:00:00 2001 From: Jess Balint Date: Thu, 28 May 2020 14:20:26 -0500 Subject: [PATCH 7/7] add support for naming anon lifetimes in function return type --- .../handlers/change_lifetime_anon_to_named.rs | 264 ++++++++++++++---- 1 file changed, 212 insertions(+), 52 deletions(-) diff --git a/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs index 9222136070..999aec421b 100644 --- a/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs +++ b/crates/ra_assists/src/handlers/change_lifetime_anon_to_named.rs @@ -1,6 +1,10 @@ -use crate::{AssistContext, AssistId, Assists}; -use ra_syntax::{ast, ast::TypeParamsOwner, AstNode, SyntaxKind}; -use std::collections::HashSet; +use crate::{assist_context::AssistBuilder, AssistContext, AssistId, Assists}; +use ast::{NameOwner, ParamList, TypeAscriptionOwner, TypeParamList, TypeRef}; +use ra_syntax::{ast, ast::TypeParamsOwner, AstNode, SyntaxKind, TextRange, TextSize}; +use rustc_hash::FxHashSet; + +static ASSIST_NAME: &str = "change_lifetime_anon_to_named"; +static ASSIST_LABEL: &str = "Give anonymous lifetime a name"; // Assist: change_lifetime_anon_to_named // @@ -26,59 +30,117 @@ use std::collections::HashSet; // } // ``` // FIXME: How can we handle renaming any one of multiple anonymous lifetimes? +// FIXME: should also add support for the case fun(f: &Foo) -> &<|>Foo pub(crate) fn change_lifetime_anon_to_named(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let lifetime_token = ctx.find_token_at_offset(SyntaxKind::LIFETIME)?; - let lifetime_arg = ast::LifetimeArg::cast(lifetime_token.parent())?; - if lifetime_arg.syntax().text() != "'_" { - return None; - } - let next_token = lifetime_token.next_token()?; - if next_token.kind() != SyntaxKind::R_ANGLE { + let lifetime_token = ctx + .find_token_at_offset(SyntaxKind::LIFETIME) + .filter(|lifetime| lifetime.text() == "'_")?; + if let Some(fn_def) = lifetime_token.ancestors().find_map(ast::FnDef::cast) { + generate_fn_def_assist(acc, &fn_def, lifetime_token.text_range()) + } else if let Some(impl_def) = lifetime_token.ancestors().find_map(ast::ImplDef::cast) { // only allow naming the last anonymous lifetime - return None; + lifetime_token.next_token().filter(|tok| tok.kind() == SyntaxKind::R_ANGLE)?; + generate_impl_def_assist(acc, &impl_def, lifetime_token.text_range()) + } else { + None } - let impl_def = lifetime_arg.syntax().ancestors().find_map(ast::ImplDef::cast)?; - // get the `impl` keyword so we know where to add the lifetime argument - let impl_kw = impl_def.syntax().first_child_or_token()?.into_token()?; - if impl_kw.kind() != SyntaxKind::IMPL_KW { - return None; - } - let new_lifetime_param = match impl_def.type_param_list() { - Some(type_params) => { - let used_lifetime_params: HashSet<_> = type_params - .lifetime_params() - .map(|p| { - let mut param_name = p.syntax().text().to_string(); - param_name.remove(0); - param_name - }) - .collect(); - (b'a'..=b'z') - .map(char::from) - .find(|c| !used_lifetime_params.contains(&c.to_string()))? +} + +/// Generate the assist for the fn def case +fn generate_fn_def_assist( + acc: &mut Assists, + fn_def: &ast::FnDef, + lifetime_loc: TextRange, +) -> Option<()> { + let param_list: ParamList = fn_def.param_list()?; + let new_lifetime_param = generate_unique_lifetime_param_name(&fn_def.type_param_list())?; + let end_of_fn_ident = fn_def.name()?.ident_token()?.text_range().end(); + let self_param = + // use the self if it's a reference and has no explicit lifetime + param_list.self_param().filter(|p| p.lifetime_token().is_none() && p.amp_token().is_some()); + // compute the location which implicitly has the same lifetime as the anonymous lifetime + let loc_needing_lifetime = if let Some(self_param) = self_param { + // if we have a self reference, use that + Some(self_param.self_token()?.text_range().start()) + } else { + // otherwise, if there's a single reference parameter without a named liftime, use that + let fn_params_without_lifetime: Vec<_> = param_list + .params() + .filter_map(|param| match param.ascribed_type() { + Some(TypeRef::ReferenceType(ascribed_type)) + if ascribed_type.lifetime_token() == None => + { + Some(ascribed_type.amp_token()?.text_range().end()) + } + _ => None, + }) + .collect(); + match fn_params_without_lifetime.len() { + 1 => Some(fn_params_without_lifetime.into_iter().nth(0)?), + 0 => None, + // multiple unnnamed is invalid. assist is not applicable + _ => return None, } - None => 'a', }; - acc.add( - AssistId("change_lifetime_anon_to_named"), - "Give anonymous lifetime a name", - lifetime_arg.syntax().text_range(), - |builder| { - match impl_def.type_param_list() { - Some(type_params) => { - builder.insert( - (u32::from(type_params.syntax().text_range().end()) - 1).into(), - format!(", '{}", new_lifetime_param), - ); - } - None => { - builder - .insert(impl_kw.text_range().end(), format!("<'{}>", new_lifetime_param)); - } - } - builder.replace(lifetime_arg.syntax().text_range(), format!("'{}", new_lifetime_param)); - }, - ) + acc.add(AssistId(ASSIST_NAME), ASSIST_LABEL, lifetime_loc, |builder| { + add_lifetime_param(fn_def, builder, end_of_fn_ident, new_lifetime_param); + builder.replace(lifetime_loc, format!("'{}", new_lifetime_param)); + loc_needing_lifetime.map(|loc| builder.insert(loc, format!("'{} ", new_lifetime_param))); + }) +} + +/// Generate the assist for the impl def case +fn generate_impl_def_assist( + acc: &mut Assists, + impl_def: &ast::ImplDef, + lifetime_loc: TextRange, +) -> Option<()> { + let new_lifetime_param = generate_unique_lifetime_param_name(&impl_def.type_param_list())?; + let end_of_impl_kw = impl_def.impl_token()?.text_range().end(); + acc.add(AssistId(ASSIST_NAME), ASSIST_LABEL, lifetime_loc, |builder| { + add_lifetime_param(impl_def, builder, end_of_impl_kw, new_lifetime_param); + builder.replace(lifetime_loc, format!("'{}", new_lifetime_param)); + }) +} + +/// Given a type parameter list, generate a unique lifetime parameter name +/// which is not in the list +fn generate_unique_lifetime_param_name( + existing_type_param_list: &Option, +) -> Option { + match existing_type_param_list { + Some(type_params) => { + let used_lifetime_params: FxHashSet<_> = type_params + .lifetime_params() + .map(|p| p.syntax().text().to_string()[1..].to_owned()) + .collect(); + (b'a'..=b'z').map(char::from).find(|c| !used_lifetime_params.contains(&c.to_string())) + } + None => Some('a'), + } +} + +/// Add the lifetime param to `builder`. If there are type parameters in `type_params_owner`, add it to the end. Otherwise +/// add new type params brackets with the lifetime parameter at `new_type_params_loc`. +fn add_lifetime_param( + type_params_owner: &TypeParamsOwner, + builder: &mut AssistBuilder, + new_type_params_loc: TextSize, + new_lifetime_param: char, +) { + match type_params_owner.type_param_list() { + // add the new lifetime parameter to an existing type param list + Some(type_params) => { + builder.insert( + (u32::from(type_params.syntax().text_range().end()) - 1).into(), + format!(", '{}", new_lifetime_param), + ); + } + // create a new type param list containing only the new lifetime parameter + None => { + builder.insert(new_type_params_loc, format!("<'{}>", new_lifetime_param)); + } + } } #[cfg(test)] @@ -117,10 +179,36 @@ mod tests { } #[test] - fn test_not_applicable() { + fn test_example_case_cursor_after_tick() { + check_assist( + change_lifetime_anon_to_named, + r#"impl Cursor<'<|>_> {"#, + r#"impl<'a> Cursor<'a> {"#, + ); + } + + #[test] + fn test_example_case_cursor_before_tick() { + check_assist( + change_lifetime_anon_to_named, + r#"impl Cursor<<|>'_> {"#, + r#"impl<'a> Cursor<'a> {"#, + ); + } + + #[test] + fn test_not_applicable_cursor_position() { check_assist_not_applicable(change_lifetime_anon_to_named, r#"impl Cursor<'_><|> {"#); check_assist_not_applicable(change_lifetime_anon_to_named, r#"impl Cursor<|><'_> {"#); + } + + #[test] + fn test_not_applicable_lifetime_already_name() { check_assist_not_applicable(change_lifetime_anon_to_named, r#"impl Cursor<'a<|>> {"#); + check_assist_not_applicable( + change_lifetime_anon_to_named, + r#"fn my_fun<'a>() -> X<'a<|>>"#, + ); } #[test] @@ -140,4 +228,76 @@ mod tests { r#"impl<'a, 'b, 'c> Cursor<'a, 'b, 'c>"#, ); } + + #[test] + fn test_function_return_value_anon_lifetime_param() { + check_assist( + change_lifetime_anon_to_named, + r#"fn my_fun() -> X<'_<|>>"#, + r#"fn my_fun<'a>() -> X<'a>"#, + ); + } + + #[test] + fn test_function_return_value_anon_reference_lifetime() { + check_assist( + change_lifetime_anon_to_named, + r#"fn my_fun() -> &'_<|> X"#, + r#"fn my_fun<'a>() -> &'a X"#, + ); + } + + #[test] + fn test_function_param_anon_lifetime() { + check_assist( + change_lifetime_anon_to_named, + r#"fn my_fun(x: X<'_<|>>)"#, + r#"fn my_fun<'a>(x: X<'a>)"#, + ); + } + + #[test] + fn test_function_add_lifetime_to_params() { + check_assist( + change_lifetime_anon_to_named, + r#"fn my_fun(f: &Foo) -> X<'_<|>>"#, + r#"fn my_fun<'a>(f: &'a Foo) -> X<'a>"#, + ); + } + + #[test] + fn test_function_add_lifetime_to_params_in_presence_of_other_lifetime() { + check_assist( + change_lifetime_anon_to_named, + r#"fn my_fun<'other>(f: &Foo, b: &'other Bar) -> X<'_<|>>"#, + r#"fn my_fun<'other, 'a>(f: &'a Foo, b: &'other Bar) -> X<'a>"#, + ); + } + + #[test] + fn test_function_not_applicable_without_self_and_multiple_unnamed_param_lifetimes() { + // this is not permitted under lifetime elision rules + check_assist_not_applicable( + change_lifetime_anon_to_named, + r#"fn my_fun(f: &Foo, b: &Bar) -> X<'_<|>>"#, + ); + } + + #[test] + fn test_function_add_lifetime_to_self_ref_param() { + check_assist( + change_lifetime_anon_to_named, + r#"fn my_fun<'other>(&self, f: &Foo, b: &'other Bar) -> X<'_<|>>"#, + r#"fn my_fun<'other, 'a>(&'a self, f: &Foo, b: &'other Bar) -> X<'a>"#, + ); + } + + #[test] + fn test_function_add_lifetime_to_param_with_non_ref_self() { + check_assist( + change_lifetime_anon_to_named, + r#"fn my_fun<'other>(self, f: &Foo, b: &'other Bar) -> X<'_<|>>"#, + r#"fn my_fun<'other, 'a>(self, f: &'a Foo, b: &'other Bar) -> X<'a>"#, + ); + } }