From 8a2c5d215bbef35688d47d851569843e61c1a0ba Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Fri, 23 Jun 2023 01:27:19 +0200 Subject: [PATCH] Still in need of more test cases --- .../src/handlers/add_missing_impl_members.rs | 44 ++++++++++++++--- .../replace_derive_with_manual_impl.rs | 14 +++++- crates/ide-assists/src/utils.rs | 47 ++++++------------- 3 files changed, 65 insertions(+), 40 deletions(-) diff --git a/crates/ide-assists/src/handlers/add_missing_impl_members.rs b/crates/ide-assists/src/handlers/add_missing_impl_members.rs index ecf1abc277d..818ec3de337 100644 --- a/crates/ide-assists/src/handlers/add_missing_impl_members.rs +++ b/crates/ide-assists/src/handlers/add_missing_impl_members.rs @@ -3,7 +3,10 @@ use syntax::ast::{self, make, AstNode}; use crate::{ assist_context::{AssistContext, Assists}, - utils::{add_trait_assoc_items_to_impl, filter_assoc_items, gen_trait_fn_body, DefaultMethods}, + utils::{ + add_trait_assoc_items_to_impl, filter_assoc_items, gen_trait_fn_body, DefaultMethods, + IgnoreAssocItems, + }, AssistId, AssistKind, }; @@ -43,7 +46,7 @@ pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext<'_ acc, ctx, DefaultMethods::No, - true, + IgnoreAssocItems::HiddenDocAttrPresent, "add_impl_missing_members", "Implement missing members", ) @@ -88,7 +91,7 @@ pub(crate) fn add_missing_default_members( acc, ctx, DefaultMethods::Only, - true, + IgnoreAssocItems::HiddenDocAttrPresent, "add_impl_default_members", "Implement default members", ) @@ -98,7 +101,7 @@ fn add_missing_impl_members_inner( acc: &mut Assists, ctx: &AssistContext<'_>, mode: DefaultMethods, - ignore_hidden: bool, + ignore_items: IgnoreAssocItems, assist_id: &'static str, label: &'static str, ) -> Option<()> { @@ -118,11 +121,22 @@ fn add_missing_impl_members_inner( let trait_ref = impl_.trait_ref(ctx.db())?; let trait_ = trait_ref.trait_(); + let mut ign_item = ignore_items; + + if let IgnoreAssocItems::HiddenDocAttrPresent = ignore_items { + // Relax condition for local crates. + + let db = ctx.db(); + if trait_.module(db).krate().origin(db).is_local() { + ign_item = IgnoreAssocItems::No; + } + } + let missing_items = filter_assoc_items( &ctx.sema, &ide_db::traits::get_missing_assoc_items(&ctx.sema, &impl_def), mode, - ignore_hidden, + ign_item, ); if missing_items.is_empty() { @@ -1999,10 +2013,28 @@ trait Trait { } } impl Trait for Foo { - $0fn another_default_impl() -> u32 { + $0fn func_with_default_impl() -> u32 { + 42 + } + + fn another_default_impl() -> u32 { 43 } }"#, ) } + + #[test] + fn doc_hidden_default_impls_extern_crates() { + // Not applicable because Eq has a single method and this has a #[doc(hidden)] attr set. + check_assist_not_applicable( + add_missing_default_members, + r#" +//- minicore: eq +use core::cmp::Eq; +struct Foo; +impl E$0q for Foo { /* $0 */ } +"#, + ) + } } diff --git a/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs index 4fa3394233f..48a68b6bea6 100644 --- a/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs +++ b/crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs @@ -10,7 +10,7 @@ use crate::{ assist_context::{AssistContext, Assists, SourceChangeBuilder}, utils::{ add_trait_assoc_items_to_impl, filter_assoc_items, gen_trait_fn_body, - generate_trait_impl_text, render_snippet, Cursor, DefaultMethods, + generate_trait_impl_text, render_snippet, Cursor, DefaultMethods, IgnoreAssocItems, }, AssistId, AssistKind, }; @@ -172,7 +172,17 @@ fn impl_def_from_trait( ) -> Option<(ast::Impl, ast::AssocItem)> { let trait_ = trait_?; let target_scope = sema.scope(annotated_name.syntax())?; - let trait_items = filter_assoc_items(sema, &trait_.items(sema.db), DefaultMethods::No, true); + + // Keep assoc items of local crates even if they have #[doc(hidden)] attr. + let ignore_items = if trait_.module(sema.db).krate().origin(sema.db).is_local() { + IgnoreAssocItems::No + } else { + IgnoreAssocItems::HiddenDocAttrPresent + }; + + let trait_items = + filter_assoc_items(sema, &trait_.items(sema.db), DefaultMethods::No, ignore_items); + if trait_items.is_empty() { return None; } diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index e4d242815ef..e09af5ce302 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -84,6 +84,12 @@ pub fn test_related_attribute(fn_def: &ast::Fn) -> Option { }) } +#[derive(Clone, Copy, PartialEq)] +pub enum IgnoreAssocItems { + HiddenDocAttrPresent, + No, +} + #[derive(Copy, Clone, PartialEq)] pub enum DefaultMethods { Only, @@ -94,48 +100,25 @@ pub fn filter_assoc_items( sema: &Semantics<'_, RootDatabase>, items: &[hir::AssocItem], default_methods: DefaultMethods, - ignore_hidden: bool, + ignore_items: IgnoreAssocItems, ) -> Vec> { - // FIXME : How to use the closure below this so I can write sth like - // sema.source(hide(it)?)?... - - // let hide = |it: impl HasAttrs| { - // if default_methods == DefaultMethods::IgnoreHidden && it.attrs(sema.db).has_doc_hidden() { - // None; - // } - // Some(it) - // }; - return items .iter() - // Note: This throws away items with no source. .copied() + .filter(|assoc_item| { + !(ignore_items == IgnoreAssocItems::HiddenDocAttrPresent + && assoc_item.attrs(sema.db).has_doc_hidden()) + }) .filter_map(|assoc_item| { let item = match assoc_item { - hir::AssocItem::Function(it) => { - if ignore_hidden && it.attrs(sema.db).has_doc_hidden() { - return None; - } - sema.source(it)?.map(ast::AssocItem::Fn) - } - hir::AssocItem::TypeAlias(it) => { - if ignore_hidden && it.attrs(sema.db).has_doc_hidden() { - return None; - } - - sema.source(it)?.map(ast::AssocItem::TypeAlias) - } - hir::AssocItem::Const(it) => { - if ignore_hidden && it.attrs(sema.db).has_doc_hidden() { - return None; - } - - sema.source(it)?.map(ast::AssocItem::Const) - } + hir::AssocItem::Function(it) => sema.source(it)?.map(ast::AssocItem::Fn), + hir::AssocItem::TypeAlias(it) => sema.source(it)?.map(ast::AssocItem::TypeAlias), + hir::AssocItem::Const(it) => sema.source(it)?.map(ast::AssocItem::Const), }; Some(item) }) .filter(has_def_name) + // Note: This throws away items with no source. .filter(|it| match &it.value { ast::AssocItem::Fn(def) => matches!( (default_methods, def.body()),