diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index d79e93406c..9e3dc998bf 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1574,6 +1574,10 @@ pub struct BuiltinType { } impl BuiltinType { + 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 +2267,10 @@ impl Type { Type::new(db, krate, def, 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 { 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 e684b77c4f..b6cec50038 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..edc4697f2c 100644 --- a/crates/ide_assists/src/handlers/generate_function.rs +++ b/crates/ide_assists/src/handlers/generate_function.rs @@ -1,11 +1,13 @@ +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}, helpers::SnippetCap, RootDatabase, }; -use rustc_hash::{FxHashMap, FxHashSet}; use stdx::to_lower_snake_case; use syntax::{ ast::{ @@ -17,7 +19,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 +426,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 +501,28 @@ 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() { + 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() + } } - 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..81cf72dd71 100644 --- a/crates/ide_assists/src/handlers/generate_getter.rs +++ b/crates/ide_assists/src/handlers/generate_getter.rs @@ -1,9 +1,9 @@ +use ide_db::helpers::FamousDefs; 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 +12,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 +40,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() // } // } // ``` @@ -100,7 +115,17 @@ pub(crate) fn generate_getter_impl( let (ty, body) = if mutable { (format!("&mut {}", field_ty), format!("&mut self.{}", field_name)) } else { - useless_type_special_case(&field_name.to_string(), &field_ty.to_string()) + 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.db(), famous_defs)) + .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 +309,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 +426,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 +480,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 539f81622b..6c5eaf310c 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..fd0ff2f5cb 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, HasSource, HirDisplay}; +use ide_db::{ + helpers::FamousDefs, 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,149 @@ 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. +pub(crate) fn convert_reference_type( + ty: hir::Type, + db: &RootDatabase, + famous_defs: &FamousDefs, +) -> Option { + handle_copy(&ty, db) + .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)) + .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, +) -> Option { + 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]) + .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 = hir::Type::new_slice(type_argument); + + 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 ef38a194bc..8801567fa5 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 @@ -117,8 +118,9 @@ pub mod clone { } // endregion:clone -// region:from + pub mod convert { + // region:from pub trait From: Sized { fn from(_: T) -> Self; } @@ -140,8 +142,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 @@ -613,6 +621,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