From 98676efdc575986370dbf86f1fce0af7d72b5c23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Wed, 29 Sep 2021 14:04:32 +0200 Subject: [PATCH 1/3] Semantic getter --- crates/hir/src/lib.rs | 10 + crates/hir_ty/src/builder.rs | 4 + crates/ide/src/hover/tests.rs | 4 +- .../src/handlers/generate_function.rs | 51 ++--- .../src/handlers/generate_getter.rs | 176 ++++++++++++++++- crates/ide_assists/src/tests/generated.rs | 17 +- crates/ide_assists/src/utils.rs | 181 +++++++++++++++--- crates/ide_db/src/helpers/famous_defs.rs | 8 + crates/test_utils/src/minicore.rs | 13 +- 9 files changed, 401 insertions(+), 63 deletions(-) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index d79e93406c..b088b5df1d 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1574,6 +1574,11 @@ pub struct BuiltinType { } impl BuiltinType { + // FIXME: I'm not sure if it's the best place to put it + pub fn str() -> BuiltinType { + BuiltinType { inner: hir_def::builtin_type::BuiltinType::Str } + } + pub fn ty(self, db: &dyn HirDatabase, module: Module) -> Type { let resolver = module.id.resolver(db.upcast()); Type::new_with_resolver(db, &resolver, TyBuilder::builtin(self.inner)) @@ -2263,6 +2268,11 @@ impl Type { Type::new(db, krate, def, ty) } + // FIXME: No idea where to put it + pub fn make_slice_of(self) -> Type { + Type { krate: self.krate, env: self.env, ty: TyBuilder::slice(self.ty) } + } + pub fn is_unit(&self) -> bool { matches!(self.ty.kind(&Interner), TyKind::Tuple(0, ..)) } diff --git a/crates/hir_ty/src/builder.rs b/crates/hir_ty/src/builder.rs index bb9d84246c..add0390c94 100644 --- a/crates/hir_ty/src/builder.rs +++ b/crates/hir_ty/src/builder.rs @@ -97,6 +97,10 @@ impl TyBuilder<()> { } } + pub fn slice(argument: Ty) -> Ty { + TyKind::Slice(argument).intern(&Interner) + } + pub fn type_params_subst(db: &dyn HirDatabase, def: impl Into) -> Substitution { let params = generics(db.upcast(), def.into()); params.type_params_subst(db) diff --git a/crates/ide/src/hover/tests.rs b/crates/ide/src/hover/tests.rs index 463cd58e48..2690da65be 100644 --- a/crates/ide/src/hover/tests.rs +++ b/crates/ide/src/hover/tests.rs @@ -2268,8 +2268,8 @@ fn foo() { file_id: FileId( 1, ), - full_range: 254..436, - focus_range: 293..299, + full_range: 276..458, + focus_range: 315..321, name: "Future", kind: Trait, description: "pub trait Future", diff --git a/crates/ide_assists/src/handlers/generate_function.rs b/crates/ide_assists/src/handlers/generate_function.rs index 2f87870413..bc8cb2a337 100644 --- a/crates/ide_assists/src/handlers/generate_function.rs +++ b/crates/ide_assists/src/handlers/generate_function.rs @@ -1,3 +1,5 @@ +use rustc_hash::{FxHashMap, FxHashSet}; + use hir::{HasSource, HirDisplay, Module, ModuleDef, Semantics, TypeInfo}; use ide_db::{ base_db::FileId, @@ -5,7 +7,6 @@ use ide_db::{ helpers::SnippetCap, RootDatabase, }; -use rustc_hash::{FxHashMap, FxHashSet}; use stdx::to_lower_snake_case; use syntax::{ ast::{ @@ -17,7 +18,7 @@ use syntax::{ }; use crate::{ - utils::useless_type_special_case, + utils::convert_reference_type, utils::{find_struct_impl, render_snippet, Cursor}, AssistContext, AssistId, AssistKind, Assists, }; @@ -424,19 +425,7 @@ fn fn_args( let mut arg_types = Vec::new(); for arg in call.arg_list()?.args() { arg_names.push(fn_arg_name(&ctx.sema, &arg)); - arg_types.push(match fn_arg_type(ctx, target_module, &arg) { - Some(ty) => { - if !ty.is_empty() && ty.starts_with('&') { - match useless_type_special_case("", &ty[1..].to_owned()) { - Some((new_ty, _)) => new_ty, - None => ty, - } - } else { - ty - } - } - None => String::from("_"), - }); + arg_types.push(fn_arg_type(ctx, target_module, &arg)); } deduplicate_arg_names(&mut arg_names); let params = arg_names.into_iter().zip(arg_types).map(|(name, ty)| { @@ -511,17 +500,31 @@ fn fn_arg_name(sema: &Semantics, arg_expr: &ast::Expr) -> String { } } -fn fn_arg_type( - ctx: &AssistContext, - target_module: hir::Module, - fn_arg: &ast::Expr, -) -> Option { - let ty = ctx.sema.type_of_expr(fn_arg)?.adjusted(); - if ty.is_unknown() { - return None; +fn fn_arg_type(ctx: &AssistContext, target_module: hir::Module, fn_arg: &ast::Expr) -> String { + fn maybe_displayed_type( + ctx: &AssistContext, + target_module: hir::Module, + fn_arg: &ast::Expr, + ) -> Option { + let ty = ctx.sema.type_of_expr(fn_arg)?.adjusted(); + if ty.is_unknown() { + return None; + } + + if ty.is_reference() || ty.is_mutable_reference() { + convert_reference_type( + ty.strip_references(), + ctx, + ctx.sema.scope(fn_arg.syntax()).krate(), + ) + .map(|conversion| conversion.convert_type(ctx.db())) + .or_else(|| ty.display_source_code(ctx.db(), target_module.into()).ok()) + } else { + ty.display_source_code(ctx.db(), target_module.into()).ok() + } } - ty.display_source_code(ctx.db(), target_module.into()).ok() + maybe_displayed_type(ctx, target_module, fn_arg).unwrap_or_else(|| String::from("_")) } /// Returns the position inside the current mod or file diff --git a/crates/ide_assists/src/handlers/generate_getter.rs b/crates/ide_assists/src/handlers/generate_getter.rs index 186e01fa9d..ba0bc7fdd9 100644 --- a/crates/ide_assists/src/handlers/generate_getter.rs +++ b/crates/ide_assists/src/handlers/generate_getter.rs @@ -2,8 +2,7 @@ use stdx::{format_to, to_lower_snake_case}; use syntax::ast::{self, AstNode, HasName, HasVisibility}; use crate::{ - utils::useless_type_special_case, - utils::{find_impl_block_end, find_struct_impl, generate_impl_text}, + utils::{convert_reference_type, find_impl_block_end, find_struct_impl, generate_impl_text}, AssistContext, AssistId, AssistKind, Assists, GroupLabel, }; @@ -12,12 +11,27 @@ use crate::{ // Generate a getter method. // // ``` +// # //- minicore: as_ref +// # pub struct String; +// # impl AsRef for String { +// # fn as_ref(&self) -> &str { +// # "" +// # } +// # } +// # // struct Person { // nam$0e: String, // } // ``` // -> // ``` +// # pub struct String; +// # impl AsRef for String { +// # fn as_ref(&self) -> &str { +// # "" +// # } +// # } +// # // struct Person { // name: String, // } @@ -25,7 +39,7 @@ use crate::{ // impl Person { // /// Get a reference to the person's name. // fn $0name(&self) -> &str { -// self.name.as_str() +// self.name.as_ref() // } // } // ``` @@ -98,9 +112,23 @@ pub(crate) fn generate_getter_impl( let vis = strukt.visibility().map_or(String::new(), |v| format!("{} ", v)); let (ty, body) = if mutable { - (format!("&mut {}", field_ty), format!("&mut self.{}", field_name)) + ( + format!("&mut {}", field_ty.to_string()), + format!("&mut self.{}", field_name.to_string()), + ) } else { - useless_type_special_case(&field_name.to_string(), &field_ty.to_string()) + ctx.sema + .resolve_type(&field_ty) + .and_then(|ty| { + convert_reference_type(ty, ctx, ctx.sema.scope(field_ty.syntax()).krate()) + }) + .map(|conversion| { + cov_mark::hit!(convert_reference_type); + ( + conversion.convert_type(ctx.db()), + conversion.getter(field_name.to_string()), + ) + }) .unwrap_or_else(|| (format!("&{}", field_ty), format!("&self.{}", field_name))) }; @@ -284,30 +312,113 @@ impl Context { } #[test] - fn test_special_cases() { - cov_mark::check!(useless_type_special_case); + fn test_not_a_special_case() { + cov_mark::check_count!(convert_reference_type, 0); + // Fake string which doesn't implement AsRef check_assist( generate_getter, r#" +pub struct String; + struct S { foo: $0String } "#, r#" +pub struct String; + +struct S { foo: String } + +impl S { + /// Get a reference to the s's foo. + fn $0foo(&self) -> &String { + &self.foo + } +} +"#, + ); + } + + #[test] + fn test_convert_reference_type() { + cov_mark::check_count!(convert_reference_type, 6); + + // Copy + check_assist( + generate_getter, + r#" +//- minicore: copy +struct S { foo: $0bool } +"#, + r#" +struct S { foo: bool } + +impl S { + /// Get a reference to the s's foo. + fn $0foo(&self) -> bool { + self.foo + } +} +"#, + ); + + // AsRef + check_assist( + generate_getter, + r#" +//- minicore: as_ref +pub struct String; +impl AsRef for String { + fn as_ref(&self) -> &str { + "" + } +} + +struct S { foo: $0String } +"#, + r#" +pub struct String; +impl AsRef for String { + fn as_ref(&self) -> &str { + "" + } +} + struct S { foo: String } impl S { /// Get a reference to the s's foo. fn $0foo(&self) -> &str { - self.foo.as_str() + self.foo.as_ref() } } "#, ); + + // AsRef check_assist( generate_getter, r#" +//- minicore: as_ref +struct Sweets; + +pub struct Box(T); +impl AsRef for Box { + fn as_ref(&self) -> &T { + &self.0 + } +} + struct S { foo: $0Box } "#, r#" +struct Sweets; + +pub struct Box(T); +impl AsRef for Box { + fn as_ref(&self) -> &T { + &self.0 + } +} + struct S { foo: Box } impl S { @@ -318,28 +429,52 @@ impl S { } "#, ); + + // AsRef<[T]> check_assist( generate_getter, r#" +//- minicore: as_ref +pub struct Vec; +impl AsRef<[T]> for Vec { + fn as_ref(&self) -> &[T] { + &[] + } +} + struct S { foo: $0Vec<()> } "#, r#" +pub struct Vec; +impl AsRef<[T]> for Vec { + fn as_ref(&self) -> &[T] { + &[] + } +} + struct S { foo: Vec<()> } impl S { /// Get a reference to the s's foo. fn $0foo(&self) -> &[()] { - self.foo.as_slice() + self.foo.as_ref() } } "#, ); + + // Option check_assist( generate_getter, r#" +//- minicore: option +struct Failure; + struct S { foo: $0Option } "#, r#" +struct Failure; + struct S { foo: Option } impl S { @@ -348,6 +483,29 @@ impl S { self.foo.as_ref() } } +"#, + ); + + // Result + check_assist( + generate_getter, + r#" +//- minicore: result +struct Context { + dat$0a: Result, +} +"#, + r#" +struct Context { + data: Result, +} + +impl Context { + /// Get a reference to the context's data. + fn $0data(&self) -> Result<&bool, &i32> { + self.data.as_ref() + } +} "#, ); } diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 25acd53482..75586f62bb 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -951,11 +951,26 @@ fn doctest_generate_getter() { check_doc_test( "generate_getter", r#####" +//- minicore: as_ref +pub struct String; +impl AsRef for String { + fn as_ref(&self) -> &str { + "" + } +} + struct Person { nam$0e: String, } "#####, r#####" +pub struct String; +impl AsRef for String { + fn as_ref(&self) -> &str { + "" + } +} + struct Person { name: String, } @@ -963,7 +978,7 @@ struct Person { impl Person { /// Get a reference to the person's name. fn $0name(&self) -> &str { - self.name.as_str() + self.name.as_ref() } } "#####, diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index 6866186d34..e58f1fd50f 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -1,13 +1,14 @@ //! Assorted functions shared by several assists. -pub(crate) mod suggest_name; -mod gen_trait_fn_body; - use std::ops; -use hir::HasSource; -use ide_db::{helpers::SnippetCap, path_transform::PathTransform, RootDatabase}; use itertools::Itertools; + +pub(crate) use gen_trait_fn_body::gen_trait_fn_body; +use hir::db::HirDatabase; +use hir::{Crate, HasSource, HirDisplay}; +use ide_db::helpers::FamousDefs; +use ide_db::{helpers::SnippetCap, path_transform::PathTransform, RootDatabase}; use stdx::format_to; use syntax::{ ast::{ @@ -23,7 +24,8 @@ use syntax::{ use crate::assist_context::{AssistBuilder, AssistContext}; -pub(crate) use gen_trait_fn_body::gen_trait_fn_body; +pub(crate) mod suggest_name; +mod gen_trait_fn_body; pub(crate) fn unwrap_trivial_block(block_expr: ast::BlockExpr) -> ast::Expr { extract_trivial_expression(&block_expr) @@ -507,27 +509,156 @@ pub(crate) fn add_method_to_adt( builder.insert(start_offset, buf); } -pub fn useless_type_special_case(field_name: &str, field_ty: &String) -> Option<(String, String)> { - if field_ty == "String" { - cov_mark::hit!(useless_type_special_case); - return Some(("&str".to_string(), format!("self.{}.as_str()", field_name))); - } - if let Some(arg) = ty_ctor(field_ty, "Vec") { - return Some((format!("&[{}]", arg), format!("self.{}.as_slice()", field_name))); - } - if let Some(arg) = ty_ctor(field_ty, "Box") { - return Some((format!("&{}", arg), format!("self.{}.as_ref()", field_name))); - } - if let Some(arg) = ty_ctor(field_ty, "Option") { - return Some((format!("Option<&{}>", arg), format!("self.{}.as_ref()", field_name))); - } - None +#[derive(Debug)] +pub(crate) struct ReferenceConversion { + conversion: ReferenceConversionType, + ty: hir::Type, } -// FIXME: This should rely on semantic info. -fn ty_ctor(ty: &String, ctor: &str) -> Option { - let res = ty.to_string().strip_prefix(ctor)?.strip_prefix('<')?.strip_suffix('>')?.to_string(); - Some(res) +#[derive(Debug)] +enum ReferenceConversionType { + // reference can be stripped if the type is Copy + Copy, + // &String -> &str + AsRefStr, + // &Vec -> &[T] + AsRefSlice, + // &Box -> &T + Dereferenced, + // &Option -> Option<&T> + Option, + // &Result -> Result<&T, &E> + Result, +} + +impl ReferenceConversion { + pub(crate) fn convert_type(&self, db: &dyn HirDatabase) -> String { + match self.conversion { + ReferenceConversionType::Copy => self.ty.display(db).to_string(), + ReferenceConversionType::AsRefStr => "&str".to_string(), + ReferenceConversionType::AsRefSlice => { + let type_argument_name = + self.ty.type_arguments().next().unwrap().display(db).to_string(); + format!("&[{}]", type_argument_name) + } + ReferenceConversionType::Dereferenced => { + let type_argument_name = + self.ty.type_arguments().next().unwrap().display(db).to_string(); + format!("&{}", type_argument_name) + } + ReferenceConversionType::Option => { + let type_argument_name = + self.ty.type_arguments().next().unwrap().display(db).to_string(); + format!("Option<&{}>", type_argument_name) + } + ReferenceConversionType::Result => { + let mut type_arguments = self.ty.type_arguments(); + let first_type_argument_name = + type_arguments.next().unwrap().display(db).to_string(); + let second_type_argument_name = + type_arguments.next().unwrap().display(db).to_string(); + format!("Result<&{}, &{}>", first_type_argument_name, second_type_argument_name) + } + } + } + + pub(crate) fn getter(&self, field_name: String) -> String { + match self.conversion { + ReferenceConversionType::Copy => format!("self.{}", field_name), + ReferenceConversionType::AsRefStr + | ReferenceConversionType::AsRefSlice + | ReferenceConversionType::Dereferenced + | ReferenceConversionType::Option + | ReferenceConversionType::Result => format!("self.{}.as_ref()", field_name), + } + } +} + +// FIXME: It should return a new hir::Type, but currently constructing new types is too cumbersome +// and all users of this function operate on string type names, so they can do the conversion +// itself themselves. +// Another problem is that it probably shouldn't take AssistContext as a parameter, as +// it should be usable not only in assists. +pub(crate) fn convert_reference_type( + ty: hir::Type, + ctx: &AssistContext, + krate: Option, +) -> Option { + let sema = &ctx.sema; + let db = sema.db; + let famous_defs = &FamousDefs(sema, krate); + + handle_copy(&ty, db) + .or_else(|| handle_as_ref_str(&ty, db, famous_defs, ctx)) + .or_else(|| handle_as_ref_slice(&ty, db, famous_defs)) + .or_else(|| handle_dereferenced(&ty, db, famous_defs)) + .or_else(|| handle_option_as_ref(&ty, db, famous_defs)) + .or_else(|| handle_result_as_ref(&ty, db, famous_defs)) + .map(|conversion| ReferenceConversion { ty, conversion }) +} + +fn handle_copy(ty: &hir::Type, db: &dyn HirDatabase) -> Option { + ty.is_copy(db).then(|| ReferenceConversionType::Copy) +} + +fn handle_as_ref_str( + ty: &hir::Type, + db: &dyn HirDatabase, + famous_defs: &FamousDefs, + ctx: &AssistContext, +) -> Option { + let module = ctx.sema.to_module_def(ctx.file_id())?; + let str_type = hir::BuiltinType::str().ty(db, module); + + ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[str_type]) + .then(|| ReferenceConversionType::AsRefStr) +} + +fn handle_as_ref_slice( + ty: &hir::Type, + db: &dyn HirDatabase, + famous_defs: &FamousDefs, +) -> Option { + let type_argument = ty.type_arguments().next()?; + let slice_type = type_argument.make_slice_of(); + + ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[slice_type]) + .then(|| ReferenceConversionType::AsRefSlice) +} + +fn handle_dereferenced( + ty: &hir::Type, + db: &dyn HirDatabase, + famous_defs: &FamousDefs, +) -> Option { + let type_argument = ty.type_arguments().next()?; + + ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[type_argument]) + .then(|| ReferenceConversionType::Dereferenced) +} + +fn handle_option_as_ref( + ty: &hir::Type, + db: &dyn HirDatabase, + famous_defs: &FamousDefs, +) -> Option { + if ty.as_adt() == famous_defs.core_option_Option()?.ty(db).as_adt() { + Some(ReferenceConversionType::Option) + } else { + None + } +} + +fn handle_result_as_ref( + ty: &hir::Type, + db: &dyn HirDatabase, + famous_defs: &FamousDefs, +) -> Option { + if ty.as_adt() == famous_defs.core_result_Result()?.ty(db).as_adt() { + Some(ReferenceConversionType::Result) + } else { + None + } } pub(crate) fn get_methods(items: &ast::AssocItemList) -> Vec { diff --git a/crates/ide_db/src/helpers/famous_defs.rs b/crates/ide_db/src/helpers/famous_defs.rs index 18524986e2..b5e3907cfa 100644 --- a/crates/ide_db/src/helpers/famous_defs.rs +++ b/crates/ide_db/src/helpers/famous_defs.rs @@ -68,10 +68,18 @@ impl FamousDefs<'_, '_> { self.find_trait("core:ops:Deref") } + pub fn core_convert_AsRef(&self) -> Option { + self.find_trait("core:convert:AsRef") + } + pub fn core_ops_ControlFlow(&self) -> Option { self.find_enum("core:ops:ControlFlow") } + pub fn core_marker_Copy(&self) -> Option { + self.find_trait("core:marker:Copy") + } + pub fn alloc(&self) -> Option { self.find_crate("alloc") } diff --git a/crates/test_utils/src/minicore.rs b/crates/test_utils/src/minicore.rs index 045b4898e5..731b8b73a3 100644 --- a/crates/test_utils/src/minicore.rs +++ b/crates/test_utils/src/minicore.rs @@ -35,6 +35,7 @@ //! fmt: result //! bool_impl: option, fn //! add: +//! as_ref: sized pub mod marker { // region:sized @@ -113,8 +114,9 @@ pub mod clone { } // endregion:clone -// region:from + pub mod convert { + // region:from pub trait From: Sized { fn from(_: T) -> Self; } @@ -136,8 +138,14 @@ pub mod convert { t } } + // endregion:from + + // region:as_ref + pub trait AsRef { + fn as_ref(&self) -> &T; + } + // endregion:as_ref } -// endregion:from pub mod ops { // region:coerce_unsized @@ -609,6 +617,7 @@ pub mod prelude { cmp::{Eq, PartialEq}, // :eq cmp::{Ord, PartialOrd}, // :ord convert::{From, Into}, // :from + convert::AsRef, // :as_ref default::Default, // :default iter::{IntoIterator, Iterator}, // :iterator macros::builtin::derive, // :derive From a2242dcf1b81f96330aed7d7219d8b51c552cdb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Wed, 20 Oct 2021 21:35:35 +0200 Subject: [PATCH 2/3] Fixes --- .../src/handlers/generate_function.rs | 12 ++++------ .../src/handlers/generate_getter.rs | 11 ++++----- crates/ide_assists/src/utils.rs | 23 +++++++------------ 3 files changed, 17 insertions(+), 29 deletions(-) diff --git a/crates/ide_assists/src/handlers/generate_function.rs b/crates/ide_assists/src/handlers/generate_function.rs index bc8cb2a337..edc4697f2c 100644 --- a/crates/ide_assists/src/handlers/generate_function.rs +++ b/crates/ide_assists/src/handlers/generate_function.rs @@ -1,6 +1,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; use hir::{HasSource, HirDisplay, Module, ModuleDef, Semantics, TypeInfo}; +use ide_db::helpers::FamousDefs; use ide_db::{ base_db::FileId, defs::{Definition, NameRefClass}, @@ -512,13 +513,10 @@ fn fn_arg_type(ctx: &AssistContext, target_module: hir::Module, fn_arg: &ast::Ex } if ty.is_reference() || ty.is_mutable_reference() { - convert_reference_type( - ty.strip_references(), - ctx, - ctx.sema.scope(fn_arg.syntax()).krate(), - ) - .map(|conversion| conversion.convert_type(ctx.db())) - .or_else(|| ty.display_source_code(ctx.db(), target_module.into()).ok()) + let famous_defs = &FamousDefs(&ctx.sema, ctx.sema.scope(fn_arg.syntax()).krate()); + convert_reference_type(ty.strip_references(), ctx.db(), famous_defs) + .map(|conversion| conversion.convert_type(ctx.db())) + .or_else(|| ty.display_source_code(ctx.db(), target_module.into()).ok()) } else { ty.display_source_code(ctx.db(), target_module.into()).ok() } diff --git a/crates/ide_assists/src/handlers/generate_getter.rs b/crates/ide_assists/src/handlers/generate_getter.rs index ba0bc7fdd9..81cf72dd71 100644 --- a/crates/ide_assists/src/handlers/generate_getter.rs +++ b/crates/ide_assists/src/handlers/generate_getter.rs @@ -1,3 +1,4 @@ +use ide_db::helpers::FamousDefs; use stdx::{format_to, to_lower_snake_case}; use syntax::ast::{self, AstNode, HasName, HasVisibility}; @@ -112,16 +113,12 @@ pub(crate) fn generate_getter_impl( let vis = strukt.visibility().map_or(String::new(), |v| format!("{} ", v)); let (ty, body) = if mutable { - ( - format!("&mut {}", field_ty.to_string()), - format!("&mut self.{}", field_name.to_string()), - ) + (format!("&mut {}", field_ty), format!("&mut self.{}", field_name)) } else { + let famous_defs = &FamousDefs(&ctx.sema, ctx.sema.scope(field_ty.syntax()).krate()); ctx.sema .resolve_type(&field_ty) - .and_then(|ty| { - convert_reference_type(ty, ctx, ctx.sema.scope(field_ty.syntax()).krate()) - }) + .and_then(|ty| convert_reference_type(ty, ctx.db(), famous_defs)) .map(|conversion| { cov_mark::hit!(convert_reference_type); ( diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index e58f1fd50f..8f0e76f7b8 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -5,10 +5,10 @@ use std::ops; use itertools::Itertools; pub(crate) use gen_trait_fn_body::gen_trait_fn_body; -use hir::db::HirDatabase; -use hir::{Crate, HasSource, HirDisplay}; -use ide_db::helpers::FamousDefs; -use ide_db::{helpers::SnippetCap, path_transform::PathTransform, RootDatabase}; +use hir::{db::HirDatabase, HasSource, HirDisplay}; +use ide_db::{ + helpers::FamousDefs, helpers::SnippetCap, path_transform::PathTransform, RootDatabase, +}; use stdx::format_to; use syntax::{ ast::{ @@ -577,19 +577,13 @@ impl ReferenceConversion { // FIXME: It should return a new hir::Type, but currently constructing new types is too cumbersome // and all users of this function operate on string type names, so they can do the conversion // itself themselves. -// Another problem is that it probably shouldn't take AssistContext as a parameter, as -// it should be usable not only in assists. pub(crate) fn convert_reference_type( ty: hir::Type, - ctx: &AssistContext, - krate: Option, + db: &RootDatabase, + famous_defs: &FamousDefs, ) -> Option { - let sema = &ctx.sema; - let db = sema.db; - let famous_defs = &FamousDefs(sema, krate); - handle_copy(&ty, db) - .or_else(|| handle_as_ref_str(&ty, db, famous_defs, ctx)) + .or_else(|| handle_as_ref_str(&ty, db, famous_defs)) .or_else(|| handle_as_ref_slice(&ty, db, famous_defs)) .or_else(|| handle_dereferenced(&ty, db, famous_defs)) .or_else(|| handle_option_as_ref(&ty, db, famous_defs)) @@ -605,9 +599,8 @@ fn handle_as_ref_str( ty: &hir::Type, db: &dyn HirDatabase, famous_defs: &FamousDefs, - ctx: &AssistContext, ) -> Option { - let module = ctx.sema.to_module_def(ctx.file_id())?; + let module = famous_defs.1?.root_module(db); let str_type = hir::BuiltinType::str().ty(db, module); ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[str_type]) From 88e2f0782693b73d71d7bbf13d584c4aa37fd834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Wed, 20 Oct 2021 22:35:31 +0200 Subject: [PATCH 3/3] Fixes --- crates/hir/src/lib.rs | 6 ++---- crates/ide_assists/src/utils.rs | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index b088b5df1d..9e3dc998bf 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1574,7 +1574,6 @@ pub struct BuiltinType { } impl BuiltinType { - // FIXME: I'm not sure if it's the best place to put it pub fn str() -> BuiltinType { BuiltinType { inner: hir_def::builtin_type::BuiltinType::Str } } @@ -2268,9 +2267,8 @@ impl Type { Type::new(db, krate, def, ty) } - // FIXME: No idea where to put it - pub fn make_slice_of(self) -> Type { - Type { krate: self.krate, env: self.env, ty: TyBuilder::slice(self.ty) } + pub fn new_slice(ty: Type) -> Type { + Type { krate: ty.krate, env: ty.env, ty: TyBuilder::slice(ty.ty) } } pub fn is_unit(&self) -> bool { diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index 8f0e76f7b8..fd0ff2f5cb 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -613,7 +613,7 @@ fn handle_as_ref_slice( famous_defs: &FamousDefs, ) -> Option { let type_argument = ty.type_arguments().next()?; - let slice_type = type_argument.make_slice_of(); + let slice_type = hir::Type::new_slice(type_argument); ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[slice_type]) .then(|| ReferenceConversionType::AsRefSlice)