From b4dd4752570d5f1ba24f7ffda69be4b1c935cd04 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 29 Apr 2020 14:49:54 +0200 Subject: More principled approach for finding From trait --- .../src/handlers/add_from_impl_for_enum.rs | 70 +++++++++++----------- crates/ra_assists/src/marks.rs | 1 + crates/ra_assists/src/utils.rs | 60 ++++++++++++++++++- 3 files changed, 94 insertions(+), 37 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs index 03806724a..49deb6701 100644 --- a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs +++ b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs @@ -1,11 +1,12 @@ +use ra_ide_db::RootDatabase; use ra_syntax::{ ast::{self, AstNode, NameOwner}, TextSize, }; use stdx::format_to; -use crate::{Assist, AssistCtx, AssistId}; -use ra_ide_db::RootDatabase; +use crate::{utils::FamousDefs, Assist, AssistCtx, AssistId}; +use test_utils::tested_by; // Assist add_from_impl_for_enum // @@ -41,7 +42,8 @@ pub(crate) fn add_from_impl_for_enum(ctx: AssistCtx) -> Option { _ => return None, }; - if already_has_from_impl(ctx.sema, &variant) { + if existing_from_impl(ctx.sema, &variant).is_some() { + tested_by!(test_add_from_impl_already_exists); return None; } @@ -70,41 +72,33 @@ impl From<{0}> for {1} {{ ) } -fn already_has_from_impl( +fn existing_from_impl( sema: &'_ hir::Semantics<'_, RootDatabase>, variant: &ast::EnumVariant, -) -> bool { - let scope = sema.scope(&variant.syntax()); +) -> Option<()> { + let variant = sema.to_def(variant)?; + let enum_ = variant.parent_enum(sema.db); + let krate = enum_.module(sema.db).krate(); - let from_path = ast::make::path_from_text("From"); - let from_hir_path = match hir::Path::from_ast(from_path) { - Some(p) => p, - None => return false, - }; - let from_trait = match scope.resolve_hir_path(&from_hir_path) { - Some(hir::PathResolution::Def(hir::ModuleDef::Trait(t))) => t, - _ => return false, - }; + let from_trait = FamousDefs(sema, krate).core_convert_From()?; - let e: hir::Enum = match sema.to_def(&variant.parent_enum()) { - Some(e) => e, - None => return false, - }; - let e_ty = e.ty(sema.db); + let enum_type = enum_.ty(sema.db); - let hir_enum_var: hir::EnumVariant = match sema.to_def(variant) { - Some(ev) => ev, - None => return false, - }; - let var_ty = hir_enum_var.fields(sema.db)[0].signature_ty(sema.db); + let wrapped_type = variant.fields(sema.db).get(0)?.signature_ty(sema.db); - e_ty.impls_trait(sema.db, from_trait, &[var_ty]) + if enum_type.impls_trait(sema.db, from_trait, &[wrapped_type]) { + Some(()) + } else { + None + } } #[cfg(test)] mod tests { use super::*; + use crate::helpers::{check_assist, check_assist_not_applicable}; + use test_utils::covers; #[test] fn test_add_from_impl_for_enum() { @@ -136,36 +130,40 @@ mod tests { ); } + fn check_not_applicable(ra_fixture: &str) { + let fixture = + format!("//- main.rs crate:main deps:core\n{}\n{}", ra_fixture, FamousDefs::FIXTURE); + check_assist_not_applicable(add_from_impl_for_enum, &fixture) + } + #[test] fn test_add_from_impl_no_element() { - check_assist_not_applicable(add_from_impl_for_enum, "enum A { <|>One }"); + check_not_applicable("enum A { <|>One }"); } #[test] fn test_add_from_impl_more_than_one_element_in_tuple() { - check_assist_not_applicable(add_from_impl_for_enum, "enum A { <|>One(u32, String) }"); + check_not_applicable("enum A { <|>One(u32, String) }"); } #[test] fn test_add_from_impl_struct_variant() { - check_assist_not_applicable(add_from_impl_for_enum, "enum A { <|>One { x: u32 } }"); + check_not_applicable("enum A { <|>One { x: u32 } }"); } #[test] fn test_add_from_impl_already_exists() { - check_assist_not_applicable( - add_from_impl_for_enum, - r#"enum A { <|>One(u32), } + covers!(test_add_from_impl_already_exists); + check_not_applicable( + r#" +enum A { <|>One(u32), } impl From for A { fn from(v: u32) -> Self { A::One(v) } } - -pub trait From { - fn from(T) -> Self; -}"#, +"#, ); } diff --git a/crates/ra_assists/src/marks.rs b/crates/ra_assists/src/marks.rs index 6c2a2b8b6..8d910205f 100644 --- a/crates/ra_assists/src/marks.rs +++ b/crates/ra_assists/src/marks.rs @@ -8,4 +8,5 @@ test_utils::marks![ test_not_inline_mut_variable test_not_applicable_if_variable_unused change_visibility_field_false_positive + test_add_from_impl_already_exists ]; diff --git a/crates/ra_assists/src/utils.rs b/crates/ra_assists/src/utils.rs index 61f8bd1dc..efd988697 100644 --- a/crates/ra_assists/src/utils.rs +++ b/crates/ra_assists/src/utils.rs @@ -3,7 +3,7 @@ pub(crate) mod insert_use; use std::iter; -use hir::{Adt, Semantics, Type}; +use hir::{Adt, Crate, Semantics, Trait, Type}; use ra_ide_db::RootDatabase; use ra_syntax::{ ast::{self, make, NameOwner}, @@ -149,3 +149,61 @@ impl TryEnum { } } } + +/// Helps with finding well-know things inside the standard library. This is +/// somewhat similar to the known paths infra inside hir, but it different; We +/// want to make sure that IDE specific paths don't become interesting inside +/// the compiler itself as well. +pub(crate) struct FamousDefs<'a, 'b>(pub(crate) &'a Semantics<'b, RootDatabase>, pub(crate) Crate); + +#[allow(non_snake_case)] +impl FamousDefs<'_, '_> { + #[cfg(test)] + pub(crate) const FIXTURE: &'static str = r#" +//- /libcore.rs crate:core +pub mod convert{ + pub trait From { + fn from(T) -> Self; + } +} + +pub mod prelude { pub use crate::convert::From } +#[prelude_import] +pub use prelude::*; +"#; + + pub(crate) fn core_convert_From(&self) -> Option { + self.find_trait("core:convert:From") + } + + fn find_trait(&self, path: &str) -> Option { + let db = self.0.db; + let mut path = path.split(':'); + let trait_ = path.next_back()?; + let std_crate = path.next()?; + let std_crate = self + .1 + .dependencies(db) + .into_iter() + .find(|dep| &dep.name.to_string() == std_crate)? + .krate; + + let mut module = std_crate.root_module(db)?; + for segment in path { + module = module.children(db).find_map(|child| { + let name = child.name(db)?; + if &name.to_string() == segment { + Some(child) + } else { + None + } + })?; + } + let def = + module.scope(db, None).into_iter().find(|(name, _def)| &name.to_string() == trait_)?.1; + match def { + hir::ScopeDef::ModuleDef(hir::ModuleDef::Trait(it)) => Some(it), + _ => None, + } + } +} -- cgit v1.2.3