Merge #10563

10563: feat: Make "Generate getter" assist use semantic info r=agluszak a=agluszak

This PR makes "Generate getter" assist use semantic info instead of dealing with types encoded as strings.
Getters for types which are:
- `Copy` no longer return references
- `AsRef<str>` (i.e. `String`) return `&str` (instead of `&String`)
- `AsRef<[T]>` (i.e. `Vec<T>`) return `&[T]` (instead of `&Vec<T>`)
- `AsRef<T>` (i.e. `Box<T>`) return `&T` (instead of `&Box<T>`)
- `Option<T>` return `Option<&T>` (instead of `&Option<T>`)
- `Result<T, E>` return `Result<&T, &E>` (instead of `&Result<T, E>`)

String, Vec, Box and Option were previously handled as special cases.

Closes #10295


Co-authored-by: Andrzej Głuszak <gluszak.andrzej@gmail.com>

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, ..))
     }

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)

crates/ide/src/hover/tests.rs

@@ -2268,8 +2268,8 @@ fn foo() {
                                     file_id: FileId(
                                         1,
                                     ),
-                                    full_range: 254..436,
+                                    full_range: 276..458,
-                                    focus_range: 293..299,
+                                    focus_range: 315..321,
                                     name: "Future",
                                     kind: Trait,
                                     description: "pub trait Future",

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) {
+        arg_types.push(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("_"),
-        });
     }
     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<RootDatabase>, arg_expr: &ast::Expr) -> String {
     }
 }
 
-fn fn_arg_type(
+fn fn_arg_type(ctx: &AssistContext, target_module: hir::Module, fn_arg: &ast::Expr) -> String {
-    ctx: &AssistContext,
+    fn maybe_displayed_type(
-    target_module: hir::Module,
+        ctx: &AssistContext,
-    fn_arg: &ast::Expr,
+        target_module: hir::Module,
-) -> Option<String> {
+        fn_arg: &ast::Expr,
-    let ty = ctx.sema.type_of_expr(fn_arg)?.adjusted();
+    ) -> Option<String> {
-    if ty.is_unknown() {
+        let ty = ctx.sema.type_of_expr(fn_arg)?.adjusted();
-        return None;
+        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

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::{convert_reference_type, find_impl_block_end, find_struct_impl, generate_impl_text},
-    utils::{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<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 +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() {
+    fn test_not_a_special_case() {
-        cov_mark::check!(useless_type_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 +426,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 {
@@ -348,6 +480,29 @@ impl S {
         self.foo.as_ref()
     }
 }
+"#,
+        );
+
+        // 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()
+    }
+}
 "#,
         );
     }

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()
     }
 }
 "#####,

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)> {
+#[derive(Debug)]
-    if field_ty == "String" {
+pub(crate) struct ReferenceConversion {
-        cov_mark::hit!(useless_type_special_case);
+    conversion: ReferenceConversionType,
-        return Some(("&str".to_string(), format!("self.{}.as_str()", field_name)));
+    ty: hir::Type,
-    }
-    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
 }
 
-// FIXME: This should rely on semantic info.
+#[derive(Debug)]
-fn ty_ctor(ty: &String, ctor: &str) -> Option<String> {
+enum ReferenceConversionType {
-    let res = ty.to_string().strip_prefix(ctor)?.strip_prefix('<')?.strip_suffix('>')?.to_string();
+    // reference can be stripped if the type is Copy
-    Some(res)
+    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)
+            }
+        }
+    }
+
+    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<ReferenceConversion> {
+    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<ReferenceConversionType> {
+    ty.is_copy(db).then(|| ReferenceConversionType::Copy)
+}
+
+fn handle_as_ref_str(
+    ty: &hir::Type,
+    db: &dyn HirDatabase,
+    famous_defs: &FamousDefs,
+) -> Option<ReferenceConversionType> {
+    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<ReferenceConversionType> {
+    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<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
+    }
+}
+
+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> {

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")
     }

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<T>: Sized {
         fn from(_: T) -> Self;
     }
@@ -140,8 +142,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
@@ -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