Auto merge of #15705 - rmehri01:14485_fix_delegate_self_references, r=Veykril

fix: resolve Self type references in delegate method assist

This PR makes the delegate method assist resolve any `Self` type references in the parameters or return type. It also works across macros such as the `uint_impl!` macro used for `saturating_mul` in the issue example.

Closes #14485
This commit is contained in:
bors 2023-12-08 11:31:34 +00:00
commit 6bbb2ac304
4 changed files with 263 additions and 5 deletions

View File

@ -588,6 +588,14 @@ pub fn body_owner(&self) -> Option<DefWithBodyId> {
_ => None, _ => None,
}) })
} }
pub fn impl_def(&self) -> Option<ImplId> {
self.scopes().find_map(|scope| match scope {
Scope::ImplDefScope(def) => Some(*def),
_ => None,
})
}
/// `expr_id` is required to be an expression id that comes after the top level expression scope in the given resolver /// `expr_id` is required to be an expression id that comes after the top level expression scope in the given resolver
#[must_use] #[must_use]
pub fn update_to_inner_scope( pub fn update_to_inner_scope(

View File

@ -1556,6 +1556,10 @@ pub fn extern_crates(&self) -> impl Iterator<Item = (Name, Module)> + '_ {
pub fn extern_crate_decls(&self) -> impl Iterator<Item = Name> + '_ { pub fn extern_crate_decls(&self) -> impl Iterator<Item = Name> + '_ {
self.resolver.extern_crate_decls_in_scope(self.db.upcast()) self.resolver.extern_crate_decls_in_scope(self.db.upcast())
} }
pub fn has_same_self_type(&self, other: &SemanticsScope<'_>) -> bool {
self.resolver.impl_def() == other.resolver.impl_def()
}
} }
#[derive(Debug)] #[derive(Debug)]

View File

@ -1,6 +1,7 @@
use std::collections::HashSet; use std::collections::HashSet;
use hir::{self, HasCrate, HasSource, HasVisibility}; use hir::{self, HasCrate, HasVisibility};
use ide_db::path_transform::PathTransform;
use syntax::{ use syntax::{
ast::{ ast::{
self, edit_in_place::Indent, make, AstNode, HasGenericParams, HasName, HasVisibility as _, self, edit_in_place::Indent, make, AstNode, HasGenericParams, HasName, HasVisibility as _,
@ -105,7 +106,7 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<'
target, target,
|edit| { |edit| {
// Create the function // Create the function
let method_source = match method.source(ctx.db()) { let method_source = match ctx.sema.source(method) {
Some(source) => source.value, Some(source) => source.value,
None => return, None => return,
}; };
@ -130,7 +131,7 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<'
vis, vis,
fn_name, fn_name,
type_params, type_params,
None, method_source.where_clause(),
params, params,
body, body,
ret_type, ret_type,
@ -183,6 +184,12 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<'
let assoc_items = impl_def.get_or_create_assoc_item_list(); let assoc_items = impl_def.get_or_create_assoc_item_list();
assoc_items.add_item(f.clone().into()); assoc_items.add_item(f.clone().into());
if let Some((target, source)) =
ctx.sema.scope(strukt.syntax()).zip(ctx.sema.scope(method_source.syntax()))
{
PathTransform::generic_transformation(&target, &source).apply(f.syntax());
}
if let Some(cap) = ctx.config.snippet_cap { if let Some(cap) = ctx.config.snippet_cap {
edit.add_tabstop_before(cap, f) edit.add_tabstop_before(cap, f)
} }
@ -454,6 +461,209 @@ impl Person {
); );
} }
#[test]
fn test_preserve_where_clause() {
check_assist(
generate_delegate_methods,
r#"
struct Inner<T>(T);
impl<T> Inner<T> {
fn get(&self) -> T
where
T: Copy,
T: PartialEq,
{
self.0
}
}
struct Struct<T> {
$0field: Inner<T>,
}
"#,
r#"
struct Inner<T>(T);
impl<T> Inner<T> {
fn get(&self) -> T
where
T: Copy,
T: PartialEq,
{
self.0
}
}
struct Struct<T> {
field: Inner<T>,
}
impl<T> Struct<T> {
$0fn get(&self) -> T where
T: Copy,
T: PartialEq, {
self.field.get()
}
}
"#,
);
}
#[test]
fn test_fixes_basic_self_references() {
check_assist(
generate_delegate_methods,
r#"
struct Foo {
field: $0Bar,
}
struct Bar;
impl Bar {
fn bar(&self, other: Self) -> Self {
other
}
}
"#,
r#"
struct Foo {
field: Bar,
}
impl Foo {
$0fn bar(&self, other: Bar) -> Bar {
self.field.bar(other)
}
}
struct Bar;
impl Bar {
fn bar(&self, other: Self) -> Self {
other
}
}
"#,
);
}
#[test]
fn test_fixes_nested_self_references() {
check_assist(
generate_delegate_methods,
r#"
struct Foo {
field: $0Bar,
}
struct Bar;
impl Bar {
fn bar(&mut self, a: (Self, [Self; 4]), b: Vec<Self>) {}
}
"#,
r#"
struct Foo {
field: Bar,
}
impl Foo {
$0fn bar(&mut self, a: (Bar, [Bar; 4]), b: Vec<Bar>) {
self.field.bar(a, b)
}
}
struct Bar;
impl Bar {
fn bar(&mut self, a: (Self, [Self; 4]), b: Vec<Self>) {}
}
"#,
);
}
#[test]
fn test_fixes_self_references_with_lifetimes_and_generics() {
check_assist(
generate_delegate_methods,
r#"
struct Foo<'a, T> {
$0field: Bar<'a, T>,
}
struct Bar<'a, T>(&'a T);
impl<'a, T> Bar<'a, T> {
fn bar(self, mut b: Vec<&'a Self>) -> &'a Self {
b.pop().unwrap()
}
}
"#,
r#"
struct Foo<'a, T> {
field: Bar<'a, T>,
}
impl<'a, T> Foo<'a, T> {
$0fn bar(self, mut b: Vec<&'a Bar<'_, T>>) -> &'a Bar<'_, T> {
self.field.bar(b)
}
}
struct Bar<'a, T>(&'a T);
impl<'a, T> Bar<'a, T> {
fn bar(self, mut b: Vec<&'a Self>) -> &'a Self {
b.pop().unwrap()
}
}
"#,
);
}
#[test]
fn test_fixes_self_references_across_macros() {
check_assist(
generate_delegate_methods,
r#"
//- /bar.rs
macro_rules! test_method {
() => {
pub fn test(self, b: Bar) -> Self {
self
}
};
}
pub struct Bar;
impl Bar {
test_method!();
}
//- /main.rs
mod bar;
struct Foo {
$0bar: bar::Bar,
}
"#,
r#"
mod bar;
struct Foo {
bar: bar::Bar,
}
impl Foo {
$0pub fn test(self,b:bar::Bar) ->bar::Bar {
self.bar.test(b)
}
}
"#,
);
}
#[test] #[test]
fn test_generate_delegate_visibility() { fn test_generate_delegate_visibility() {
check_assist_not_applicable( check_assist_not_applicable(

View File

@ -2,7 +2,7 @@
use crate::helpers::mod_path_to_ast; use crate::helpers::mod_path_to_ast;
use either::Either; use either::Either;
use hir::{AsAssocItem, HirDisplay, SemanticsScope}; use hir::{AsAssocItem, HirDisplay, ModuleDef, SemanticsScope};
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use syntax::{ use syntax::{
ast::{self, make, AstNode}, ast::{self, make, AstNode},
@ -183,6 +183,7 @@ fn build_ctx(&self) -> Ctx<'a> {
lifetime_substs, lifetime_substs,
target_module, target_module,
source_scope: self.source_scope, source_scope: self.source_scope,
same_self_type: self.target_scope.has_same_self_type(self.source_scope),
}; };
ctx.transform_default_values(defaulted_params); ctx.transform_default_values(defaulted_params);
ctx ctx
@ -195,6 +196,7 @@ struct Ctx<'a> {
lifetime_substs: FxHashMap<LifetimeName, ast::Lifetime>, lifetime_substs: FxHashMap<LifetimeName, ast::Lifetime>,
target_module: hir::Module, target_module: hir::Module,
source_scope: &'a SemanticsScope<'a>, source_scope: &'a SemanticsScope<'a>,
same_self_type: bool,
} }
fn postorder(item: &SyntaxNode) -> impl Iterator<Item = SyntaxNode> { fn postorder(item: &SyntaxNode) -> impl Iterator<Item = SyntaxNode> {
@ -332,8 +334,42 @@ fn transform_path(&self, path: ast::Path) -> Option<()> {
ted::replace(path.syntax(), subst.clone_subtree().clone_for_update()); ted::replace(path.syntax(), subst.clone_subtree().clone_for_update());
} }
} }
hir::PathResolution::SelfType(imp) => {
// keep Self type if it does not need to be replaced
if self.same_self_type {
return None;
}
let ty = imp.self_ty(self.source_scope.db);
let ty_str = &ty
.display_source_code(
self.source_scope.db,
self.source_scope.module().into(),
true,
)
.ok()?;
let ast_ty = make::ty(&ty_str).clone_for_update();
if let Some(adt) = ty.as_adt() {
if let ast::Type::PathType(path_ty) = &ast_ty {
let found_path = self.target_module.find_use_path(
self.source_scope.db.upcast(),
ModuleDef::from(adt),
false,
true,
)?;
if let Some(qual) = mod_path_to_ast(&found_path).qualifier() {
let res = make::path_concat(qual, path_ty.path()?).clone_for_update();
ted::replace(path.syntax(), res.syntax());
return Some(());
}
}
}
ted::replace(path.syntax(), ast_ty.syntax());
}
hir::PathResolution::Local(_) hir::PathResolution::Local(_)
| hir::PathResolution::SelfType(_)
| hir::PathResolution::Def(_) | hir::PathResolution::Def(_)
| hir::PathResolution::BuiltinAttr(_) | hir::PathResolution::BuiltinAttr(_)
| hir::PathResolution::ToolModule(_) | hir::PathResolution::ToolModule(_)