Unnamed repository; edit this file 'description' to name the repository.
Semantic getter
| -rw-r--r-- | crates/hir/src/lib.rs | 10 | ||||
| -rw-r--r-- | crates/hir_ty/src/builder.rs | 4 | ||||
| -rw-r--r-- | crates/ide/src/hover/tests.rs | 4 | ||||
| -rw-r--r-- | crates/ide_assists/src/handlers/generate_function.rs | 51 | ||||
| -rw-r--r-- | crates/ide_assists/src/handlers/generate_getter.rs | 176 | ||||
| -rw-r--r-- | crates/ide_assists/src/tests/generated.rs | 17 | ||||
| -rw-r--r-- | crates/ide_assists/src/utils.rs | 175 | ||||
| -rw-r--r-- | crates/ide_db/src/helpers/famous_defs.rs | 8 | ||||
| -rw-r--r-- | crates/test_utils/src/minicore.rs | 13 |
9 files changed, 398 insertions, 60 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<GenericDefId>) -> 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<RootDatabase>, arg_expr: &ast::Expr) -> String { } } -fn fn_arg_type( - ctx: &AssistContext, - target_module: hir::Module, - fn_arg: &ast::Expr, -) -> Option<String> { - 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<String> { + 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<str> for String { +// # fn as_ref(&self) -> &str { +// # "" +// # } +// # } +// # // struct Person { // nam$0e: String, // } // ``` // -> // ``` +// # pub struct String; +// # impl AsRef<str> 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<str> 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<str> + check_assist( + generate_getter, + r#" +//- minicore: as_ref +pub struct String; +impl AsRef<str> for String { + fn as_ref(&self) -> &str { + "" + } +} + +struct S { foo: $0String } +"#, + r#" +pub struct String; +impl AsRef<str> 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<T> check_assist( generate_getter, r#" +//- minicore: as_ref +struct Sweets; + +pub struct Box<T>(T); +impl<T> AsRef<T> for Box<T> { + fn as_ref(&self) -> &T { + &self.0 + } +} + struct S { foo: $0Box<Sweets> } "#, r#" +struct Sweets; + +pub struct Box<T>(T); +impl<T> AsRef<T> for Box<T> { + fn as_ref(&self) -> &T { + &self.0 + } +} + struct S { foo: Box<Sweets> } impl S { @@ -318,28 +429,52 @@ impl S { } "#, ); + + // AsRef<[T]> check_assist( generate_getter, r#" +//- minicore: as_ref +pub struct Vec<T>; +impl<T> AsRef<[T]> for Vec<T> { + fn as_ref(&self) -> &[T] { + &[] + } +} + struct S { foo: $0Vec<()> } "#, r#" +pub struct Vec<T>; +impl<T> AsRef<[T]> for Vec<T> { + 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<Failure> } "#, r#" +struct Failure; + struct S { foo: Option<Failure> } impl S { @@ -350,5 +485,28 @@ impl S { } "#, ); + + // Result + check_assist( + generate_getter, + r#" +//- minicore: result +struct Context { + dat$0a: Result<bool, i32>, +} +"#, + r#" +struct Context { + data: Result<bool, i32>, +} + +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<str> for String { + fn as_ref(&self) -> &str { + "" + } +} + struct Person { nam$0e: String, } "#####, r#####" +pub struct String; +impl AsRef<str> 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))); +#[derive(Debug)] +pub(crate) struct ReferenceConversion { + conversion: ReferenceConversionType, + ty: hir::Type, +} + +#[derive(Debug)] +enum ReferenceConversionType { + // reference can be stripped if the type is Copy + Copy, + // &String -> &str + AsRefStr, + // &Vec<T> -> &[T] + AsRefSlice, + // &Box<T> -> &T + Dereferenced, + // &Option<T> -> Option<&T> + Option, + // &Result<T, E> -> 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) + } + } } - if let Some(arg) = ty_ctor(field_ty, "Box") { - return Some((format!("&{}", arg), format!("self.{}.as_ref()", field_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), + } } - if let Some(arg) = ty_ctor(field_ty, "Option") { - return Some((format!("Option<&{}>", arg), 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<Crate>, +) -> Option<ReferenceConversion> { + 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<ReferenceConversionType> { + ty.is_copy(db).then(|| ReferenceConversionType::Copy) +} + +fn handle_as_ref_str( + ty: &hir::Type, + db: &dyn HirDatabase, + famous_defs: &FamousDefs, + ctx: &AssistContext, +) -> Option<ReferenceConversionType> { + 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<ReferenceConversionType> { + 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<ReferenceConversionType> { + 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<ReferenceConversionType> { + if ty.as_adt() == famous_defs.core_option_Option()?.ty(db).as_adt() { + Some(ReferenceConversionType::Option) + } else { + None } - None } -// FIXME: This should rely on semantic info. -fn ty_ctor(ty: &String, ctor: &str) -> Option<String> { - let res = ty.to_string().strip_prefix(ctor)?.strip_prefix('<')?.strip_suffix('>')?.to_string(); - Some(res) +fn handle_result_as_ref( + ty: &hir::Type, + db: &dyn HirDatabase, + famous_defs: &FamousDefs, +) -> Option<ReferenceConversionType> { + 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<ast::Fn> { 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<Trait> { + self.find_trait("core:convert:AsRef") + } + pub fn core_ops_ControlFlow(&self) -> Option<Enum> { self.find_enum("core:ops:ControlFlow") } + pub fn core_marker_Copy(&self) -> Option<Trait> { + self.find_trait("core:marker:Copy") + } + pub fn alloc(&self) -> Option<Crate> { 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<T>: Sized { fn from(_: T) -> Self; } @@ -136,8 +138,14 @@ pub mod convert { t } } + // endregion:from + + // region:as_ref + pub trait AsRef<T: ?Sized> { + 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 |