6670: Allow renaming between self and first param with owned parameters r=matklad a=Veykril

This fixes renaming owned SelfParams turning the parameter into a reference, as in, for a type `Foo`, `fn foo(self) {}` became `fn foo(renamed_name: &Foo) {}` prior to this.

Similarly for the other way around, we now support renaming non-ref parameters to `self`. Additionally we do more checks now than before. We check:
- that the function has an impl block
- that we are renaming the first parameter(prior we ignored which parameter was renamed and always picked the first nevertheless)
- that the parameter's type aligns with the impl block(minus one level of reference abstraction to account for `&self`/`&mut self`)

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
This commit is contained in:
bors[bot] 2020-11-29 19:07:04 +00:00 committed by GitHub
commit 68910d2f34
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -221,24 +221,47 @@ fn rename_to_self(
let source_file = sema.parse(position.file_id);
let syn = source_file.syntax();
let fn_def = find_node_at_offset::<ast::Fn>(syn, position.offset)
let (fn_def, fn_ast) = find_node_at_offset::<ast::Fn>(syn, position.offset)
.and_then(|fn_ast| sema.to_def(&fn_ast).zip(Some(fn_ast)))
.ok_or_else(|| RenameError("No surrounding method declaration found".to_string()))?;
let params =
fn_def.param_list().ok_or_else(|| RenameError("Method has no parameters".to_string()))?;
if params.self_param().is_some() {
let param_range = fn_ast
.param_list()
.and_then(|p| p.params().next())
.ok_or_else(|| RenameError("Method has no parameters".to_string()))?
.syntax()
.text_range();
if !param_range.contains(position.offset) {
return Err(RenameError("Only the first parameter can be self".to_string()));
}
let impl_block = find_node_at_offset::<ast::Impl>(syn, position.offset)
.and_then(|def| sema.to_def(&def))
.ok_or_else(|| RenameError("No impl block found for function".to_string()))?;
if fn_def.self_param(sema.db).is_some() {
return Err(RenameError("Method already has a self parameter".to_string()));
}
let params = fn_def.params(sema.db);
let first_param =
params.params().next().ok_or_else(|| RenameError("Method has no parameters".into()))?;
let mutable = match first_param.ty() {
Some(ast::Type::RefType(rt)) => rt.mut_token().is_some(),
_ => return Err(RenameError("Not renaming other types".to_string())),
params.first().ok_or_else(|| RenameError("Method has no parameters".into()))?;
let first_param_ty = first_param.ty();
let impl_ty = impl_block.target_ty(sema.db);
let (ty, self_param) = if impl_ty.remove_ref().is_some() {
// if the impl is a ref to the type we can just match the `&T` with self directly
(first_param_ty.clone(), "self")
} else {
first_param_ty.remove_ref().map_or((first_param_ty.clone(), "self"), |ty| {
(ty, if first_param_ty.is_mutable_reference() { "&mut self" } else { "&self" })
})
};
if ty != impl_ty {
return Err(RenameError("Parameter type differs from impl block type".to_string()));
}
let RangeInfo { range, info: refs } = find_all_refs(sema, position, None)
.ok_or_else(|| RenameError("No reference found at position".to_string()))?;
let param_range = first_param.syntax().text_range();
let (param_ref, usages): (Vec<Reference>, Vec<Reference>) = refs
.into_iter()
.partition(|reference| param_range.intersect(reference.file_range.range).is_some());
@ -254,10 +277,7 @@ fn rename_to_self(
edits.push(SourceFileEdit {
file_id: position.file_id,
edit: TextEdit::replace(
param_range,
String::from(if mutable { "&mut self" } else { "&self" }),
),
edit: TextEdit::replace(param_range, String::from(self_param)),
});
Ok(RangeInfo::new(range, SourceChange::from(edits)))
@ -280,7 +300,11 @@ fn text_edit_from_self_param(
let mut replacement_text = String::from(new_name);
replacement_text.push_str(": ");
replacement_text.push_str(self_param.mut_token().map_or("&", |_| "&mut "));
match (self_param.amp_token(), self_param.mut_token()) {
(None, None) => (),
(Some(_), None) => replacement_text.push('&'),
(_, Some(_)) => replacement_text.push_str("&mut "),
};
replacement_text.push_str(type_name.as_str());
Some(TextEdit::replace(self_param.syntax().text_range(), replacement_text))
@ -1080,6 +1104,95 @@ impl Foo {
self.i
}
}
"#,
);
check(
"self",
r#"
struct Foo { i: i32 }
impl Foo {
fn f(foo<|>: Foo) -> i32 {
foo.i
}
}
"#,
r#"
struct Foo { i: i32 }
impl Foo {
fn f(self) -> i32 {
self.i
}
}
"#,
);
}
#[test]
fn test_parameter_to_self_error_no_impl() {
check(
"self",
r#"
struct Foo { i: i32 }
fn f(foo<|>: &mut Foo) -> i32 {
foo.i
}
"#,
"error: No impl block found for function",
);
check(
"self",
r#"
struct Foo { i: i32 }
struct Bar;
impl Bar {
fn f(foo<|>: &mut Foo) -> i32 {
foo.i
}
}
"#,
"error: Parameter type differs from impl block type",
);
}
#[test]
fn test_parameter_to_self_error_not_first() {
check(
"self",
r#"
struct Foo { i: i32 }
impl Foo {
fn f(x: (), foo<|>: &mut Foo) -> i32 {
foo.i
}
}
"#,
"error: Only the first parameter can be self",
);
}
#[test]
fn test_parameter_to_self_impl_ref() {
check(
"self",
r#"
struct Foo { i: i32 }
impl &Foo {
fn f(foo<|>: &Foo) -> i32 {
foo.i
}
}
"#,
r#"
struct Foo { i: i32 }
impl &Foo {
fn f(self) -> i32 {
self.i
}
}
"#,
);
}
@ -1109,6 +1222,31 @@ impl Foo {
);
}
#[test]
fn test_owned_self_to_parameter() {
check(
"foo",
r#"
struct Foo { i: i32 }
impl Foo {
fn f(<|>self) -> i32 {
self.i
}
}
"#,
r#"
struct Foo { i: i32 }
impl Foo {
fn f(foo: Foo) -> i32 {
foo.i
}
}
"#,
);
}
#[test]
fn test_self_in_path_to_parameter() {
check(