Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Remove unnecessary .as_ref() in generate getter assist #16487

Merged
merged 4 commits into from
Feb 8, 2024
Merged

fix: Remove unnecessary .as_ref() in generate getter assist #16487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
97ecfe4
Remove unnecessary `.as_ref()` in generate getter assist
ShoyuVanilla Feb 4, 2024
f0428e5
Simplify redundant coercion
ShoyuVanilla Feb 5, 2024
983cce8
Typo fix
ShoyuVanilla Feb 5, 2024
26715a8
cargo fmt
ShoyuVanilla Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 16 additions & 2 deletions crates/ide-assists/src/handlers/generate_getter_or_setter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,21 @@ pub(crate) fn generate_setter(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt
// Generate a getter method.
//
// ```
// # //- minicore: as_ref
// # //- minicore: as_ref, deref
// # pub struct String;
// # impl AsRef<str> for String {
// # fn as_ref(&self) -> &str {
// # ""
// # }
// # }
// #
// # impl core::ops::Deref for String {
// # type Target = str;
// # fn deref(&self) -> &Self::Target {
// # ""
// # }
// # }
// #
// struct Person {
// nam$0e: String,
// }
Expand All @@ -96,13 +103,20 @@ pub(crate) fn generate_setter(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt
// # }
// # }
// #
// # impl core::ops::Deref for String {
// # type Target = str;
// # fn deref(&self) -> &Self::Target {
// # ""
// # }
// # }
// #
// struct Person {
// name: String,
// }
//
// impl Person {
// fn $0name(&self) -> &str {
// self.name.as_ref()
// &self.name
// }
// }
// ```
Expand Down
18 changes: 16 additions & 2 deletions crates/ide-assists/src/tests/generated.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1429,14 +1429,21 @@ fn doctest_generate_getter() {
check_doc_test(
"generate_getter",
r#####"
//- minicore: as_ref
//- minicore: as_ref, deref
pub struct String;
impl AsRef<str> for String {
fn as_ref(&self) -> &str {
""
}
}

impl core::ops::Deref for String {
type Target = str;
fn deref(&self) -> &Self::Target {
""
}
}

struct Person {
nam$0e: String,
}
Expand All @@ -1449,13 +1456,20 @@ impl AsRef<str> for String {
}
}

impl core::ops::Deref for String {
type Target = str;
fn deref(&self) -> &Self::Target {
""
}
}

struct Person {
name: String,
}

impl Person {
fn $0name(&self) -> &str {
self.name.as_ref()
&self.name
}
}
"#####,
Expand Down
51 changes: 34 additions & 17 deletions crates/ide-assists/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,7 @@ pub(crate) fn add_method_to_adt(
pub(crate) struct ReferenceConversion {
conversion: ReferenceConversionType,
ty: hir::Type,
impls_deref: bool,
}

#[derive(Debug)]
Expand Down Expand Up @@ -656,7 +657,13 @@ impl ReferenceConversion {
| ReferenceConversionType::AsRefSlice
| ReferenceConversionType::Dereferenced
| ReferenceConversionType::Option
| ReferenceConversionType::Result => format!("self.{field_name}.as_ref()"),
| ReferenceConversionType::Result => {
if self.impls_deref {
format!("&self.{field_name}")
} else {
format!("self.{field_name}.as_ref()")
}
}
}
}
}
Expand All @@ -675,54 +682,64 @@ pub(crate) fn convert_reference_type(
.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 })
.map(|(conversion, impls_deref)| ReferenceConversion { ty, conversion, impls_deref })
}

fn could_deref_to_target(ty: &hir::Type, target: &hir::Type, db: &dyn HirDatabase) -> bool {
let ty_ref = hir::Type::reference(ty, hir::Mutability::Shared);
let target_ref = hir::Type::reference(target, hir::Mutability::Shared);
ty_ref.could_coerce_to(db, &target_ref)
}

fn handle_copy(ty: &hir::Type, db: &dyn HirDatabase) -> Option<ReferenceConversionType> {
ty.is_copy(db).then_some(ReferenceConversionType::Copy)
fn handle_copy(ty: &hir::Type, db: &dyn HirDatabase) -> Option<(ReferenceConversionType, bool)> {
ty.is_copy(db).then_some((ReferenceConversionType::Copy, true))
}

fn handle_as_ref_str(
ty: &hir::Type,
db: &dyn HirDatabase,
famous_defs: &FamousDefs<'_, '_>,
) -> Option<ReferenceConversionType> {
) -> Option<(ReferenceConversionType, bool)> {
let str_type = hir::BuiltinType::str().ty(db);

ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[str_type])
.then_some(ReferenceConversionType::AsRefStr)
ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[str_type.clone()])
.then_some((ReferenceConversionType::AsRefStr, could_deref_to_target(ty, &str_type, db)))
}

fn handle_as_ref_slice(
ty: &hir::Type,
db: &dyn HirDatabase,
famous_defs: &FamousDefs<'_, '_>,
) -> Option<ReferenceConversionType> {
) -> Option<(ReferenceConversionType, bool)> {
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_some(ReferenceConversionType::AsRefSlice)
ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[slice_type.clone()]).then_some((
ReferenceConversionType::AsRefSlice,
could_deref_to_target(ty, &slice_type, db),
))
}

fn handle_dereferenced(
ty: &hir::Type,
db: &dyn HirDatabase,
famous_defs: &FamousDefs<'_, '_>,
) -> Option<ReferenceConversionType> {
) -> Option<(ReferenceConversionType, bool)> {
let type_argument = ty.type_arguments().next()?;

ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[type_argument])
.then_some(ReferenceConversionType::Dereferenced)
ty.impls_trait(db, famous_defs.core_convert_AsRef()?, &[type_argument.clone()]).then_some((
ReferenceConversionType::Dereferenced,
could_deref_to_target(ty, &type_argument, db),
))
}

fn handle_option_as_ref(
ty: &hir::Type,
db: &dyn HirDatabase,
famous_defs: &FamousDefs<'_, '_>,
) -> Option<ReferenceConversionType> {
) -> Option<(ReferenceConversionType, bool)> {
if ty.as_adt() == famous_defs.core_option_Option()?.ty(db).as_adt() {
Some(ReferenceConversionType::Option)
Some((ReferenceConversionType::Option, false))
} else {
None
}
Expand All @@ -732,9 +749,9 @@ fn handle_result_as_ref(
ty: &hir::Type,
db: &dyn HirDatabase,
famous_defs: &FamousDefs<'_, '_>,
) -> Option<ReferenceConversionType> {
) -> Option<(ReferenceConversionType, bool)> {
if ty.as_adt() == famous_defs.core_result_Result()?.ty(db).as_adt() {
Some(ReferenceConversionType::Result)
Some((ReferenceConversionType::Result, false))
} else {
None
}
Expand Down