Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #15050 - alibektas:14957, r=Veykril
bugfix : skip doc(hidden) default members
fixes #14957 . I have two questions :
1. I am definitely looking for a more idiomatic way for the things I added in `crates/ide-assists/src/utils.rs`. See `FIXME` in that file.
2. Would it be actually better to change `DefaultMethods` to something like
```rust
enum DefaultMethods {
Only( IgnoreHidden ( bool ) ) ,
None
}
```
instead of adding a boolean to every function that calls `crates/ide-assists/src/utils.rs::filter_assoc_items`
| -rw-r--r-- | crates/ide-assists/src/handlers/add_missing_impl_members.rs | 184 | ||||
| -rw-r--r-- | crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs | 14 | ||||
| -rw-r--r-- | crates/ide-assists/src/utils.rs | 15 |
3 files changed, 208 insertions, 5 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 d07c637262..6aca716bb6 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,6 +46,7 @@ pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext<'_ acc, ctx, DefaultMethods::No, + IgnoreAssocItems::DocHiddenAttrPresent, "add_impl_missing_members", "Implement missing members", ) @@ -87,6 +91,7 @@ pub(crate) fn add_missing_default_members( acc, ctx, DefaultMethods::Only, + IgnoreAssocItems::DocHiddenAttrPresent, "add_impl_default_members", "Implement default members", ) @@ -96,6 +101,7 @@ fn add_missing_impl_members_inner( acc: &mut Assists, ctx: &AssistContext<'_>, mode: DefaultMethods, + ignore_items: IgnoreAssocItems, assist_id: &'static str, label: &'static str, ) -> Option<()> { @@ -115,10 +121,21 @@ 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::DocHiddenAttrPresent = 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, + ign_item, ); if missing_items.is_empty() { @@ -1966,4 +1983,169 @@ impl AnotherTrait<i32> for () { "#, ); } + + #[test] + fn doc_hidden_default_impls_ignored() { + // doc(hidden) attr is ignored trait and impl both belong to the local crate. + check_assist( + add_missing_default_members, + r#" +struct Foo; +trait Trait { + #[doc(hidden)] + fn func_with_default_impl() -> u32 { + 42 + } + fn another_default_impl() -> u32 { + 43 + } +} +impl Tra$0it for Foo {}"#, + r#" +struct Foo; +trait Trait { + #[doc(hidden)] + fn func_with_default_impl() -> u32 { + 42 + } + fn another_default_impl() -> u32 { + 43 + } +} +impl Trait for Foo { + $0fn func_with_default_impl() -> u32 { + 42 + } + + fn another_default_impl() -> u32 { + 43 + } +}"#, + ) + } + + #[test] + fn doc_hidden_default_impls_lang_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 */ } +"#, + ) + } + + #[test] + fn doc_hidden_default_impls_lib_crates() { + check_assist( + add_missing_default_members, + r#" + //- /main.rs crate:a deps:b + struct B; + impl b::Exte$0rnTrait for B {} + //- /lib.rs crate:b new_source_root:library + pub trait ExternTrait { + #[doc(hidden)] + fn hidden_default() -> Option<()> { + todo!() + } + + fn unhidden_default() -> Option<()> { + todo!() + } + + fn unhidden_nondefault() -> Option<()>; + } + "#, + r#" + struct B; + impl b::ExternTrait for B { + $0fn unhidden_default() -> Option<()> { + todo!() + } + } + "#, + ) + } + + #[test] + fn doc_hidden_default_impls_local_crates() { + check_assist( + add_missing_default_members, + r#" +trait LocalTrait { + #[doc(hidden)] + fn no_skip_default() -> Option<()> { + todo!() + } + fn no_skip_default_2() -> Option<()> { + todo!() + } +} + +struct B; +impl Loc$0alTrait for B {} + "#, + r#" +trait LocalTrait { + #[doc(hidden)] + fn no_skip_default() -> Option<()> { + todo!() + } + fn no_skip_default_2() -> Option<()> { + todo!() + } +} + +struct B; +impl LocalTrait for B { + $0fn no_skip_default() -> Option<()> { + todo!() + } + + fn no_skip_default_2() -> Option<()> { + todo!() + } +} + "#, + ) + } + + #[test] + fn doc_hidden_default_impls_workspace_crates() { + check_assist( + add_missing_default_members, + r#" +//- /lib.rs crate:b new_source_root:local +trait LocalTrait { + #[doc(hidden)] + fn no_skip_default() -> Option<()> { + todo!() + } + fn no_skip_default_2() -> Option<()> { + todo!() + } +} + +//- /main.rs crate:a deps:b +struct B; +impl b::Loc$0alTrait for B {} + "#, + r#" +struct B; +impl b::LocalTrait for B { + $0fn no_skip_default() -> Option<()> { + todo!() + } + + fn no_skip_default_2() -> Option<()> { + todo!() + } +} + "#, + ) + } } 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 c03bc2f41d..ac45581b7b 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); + + // 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::DocHiddenAttrPresent + }; + + 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 03d8553506..a262570d94 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -3,7 +3,7 @@ use std::ops; pub(crate) use gen_trait_fn_body::gen_trait_fn_body; -use hir::{db::HirDatabase, HirDisplay, InFile, Semantics}; +use hir::{db::HirDatabase, HasAttrs as HirHasAttrs, HirDisplay, InFile, Semantics}; use ide_db::{ famous_defs::FamousDefs, path_transform::PathTransform, syntax_helpers::insert_whitespace_into_node::insert_ws_into, RootDatabase, SnippetCap, @@ -84,6 +84,12 @@ pub fn test_related_attribute(fn_def: &ast::Fn) -> Option<ast::Attr> { }) } +#[derive(Clone, Copy, PartialEq)] +pub enum IgnoreAssocItems { + DocHiddenAttrPresent, + No, +} + #[derive(Copy, Clone, PartialEq)] pub enum DefaultMethods { Only, @@ -94,11 +100,16 @@ pub fn filter_assoc_items( sema: &Semantics<'_, RootDatabase>, items: &[hir::AssocItem], default_methods: DefaultMethods, + ignore_items: IgnoreAssocItems, ) -> Vec<InFile<ast::AssocItem>> { return items .iter() - // Note: This throws away items with no source. .copied() + .filter(|assoc_item| { + !(ignore_items == IgnoreAssocItems::DocHiddenAttrPresent + && assoc_item.attrs(sema.db).has_doc_hidden()) + }) + // Note: This throws away items with no source. .filter_map(|assoc_item| { let item = match assoc_item { hir::AssocItem::Function(it) => sema.source(it)?.map(ast::AssocItem::Fn), |