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`
This commit is contained in:
bors 2023-08-01 08:38:36 +00:00
commit efc5a813de
3 changed files with 208 additions and 5 deletions

View File

@ -3,7 +3,10 @@
use crate::{ use crate::{
assist_context::{AssistContext, Assists}, 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, AssistId, AssistKind,
}; };
@ -43,6 +46,7 @@ pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext<'_
acc, acc,
ctx, ctx,
DefaultMethods::No, DefaultMethods::No,
IgnoreAssocItems::DocHiddenAttrPresent,
"add_impl_missing_members", "add_impl_missing_members",
"Implement missing members", "Implement missing members",
) )
@ -87,6 +91,7 @@ pub(crate) fn add_missing_default_members(
acc, acc,
ctx, ctx,
DefaultMethods::Only, DefaultMethods::Only,
IgnoreAssocItems::DocHiddenAttrPresent,
"add_impl_default_members", "add_impl_default_members",
"Implement default members", "Implement default members",
) )
@ -96,6 +101,7 @@ fn add_missing_impl_members_inner(
acc: &mut Assists, acc: &mut Assists,
ctx: &AssistContext<'_>, ctx: &AssistContext<'_>,
mode: DefaultMethods, mode: DefaultMethods,
ignore_items: IgnoreAssocItems,
assist_id: &'static str, assist_id: &'static str,
label: &'static str, label: &'static str,
) -> Option<()> { ) -> Option<()> {
@ -115,10 +121,21 @@ fn add_missing_impl_members_inner(
let trait_ref = impl_.trait_ref(ctx.db())?; let trait_ref = impl_.trait_ref(ctx.db())?;
let trait_ = trait_ref.trait_(); 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( let missing_items = filter_assoc_items(
&ctx.sema, &ctx.sema,
&ide_db::traits::get_missing_assoc_items(&ctx.sema, &impl_def), &ide_db::traits::get_missing_assoc_items(&ctx.sema, &impl_def),
mode, mode,
ign_item,
); );
if missing_items.is_empty() { 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!()
}
}
"#,
)
}
} }

View File

@ -10,7 +10,7 @@
assist_context::{AssistContext, Assists, SourceChangeBuilder}, assist_context::{AssistContext, Assists, SourceChangeBuilder},
utils::{ utils::{
add_trait_assoc_items_to_impl, filter_assoc_items, gen_trait_fn_body, 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, AssistId, AssistKind,
}; };
@ -172,7 +172,17 @@ fn impl_def_from_trait(
) -> Option<(ast::Impl, ast::AssocItem)> { ) -> Option<(ast::Impl, ast::AssocItem)> {
let trait_ = trait_?; let trait_ = trait_?;
let target_scope = sema.scope(annotated_name.syntax())?; 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() { if trait_items.is_empty() {
return None; return None;
} }

View File

@ -3,7 +3,7 @@
use std::ops; use std::ops;
pub(crate) use gen_trait_fn_body::gen_trait_fn_body; 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::{ use ide_db::{
famous_defs::FamousDefs, path_transform::PathTransform, famous_defs::FamousDefs, path_transform::PathTransform,
syntax_helpers::insert_whitespace_into_node::insert_ws_into, RootDatabase, SnippetCap, 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)] #[derive(Copy, Clone, PartialEq)]
pub enum DefaultMethods { pub enum DefaultMethods {
Only, Only,
@ -94,11 +100,16 @@ pub fn filter_assoc_items(
sema: &Semantics<'_, RootDatabase>, sema: &Semantics<'_, RootDatabase>,
items: &[hir::AssocItem], items: &[hir::AssocItem],
default_methods: DefaultMethods, default_methods: DefaultMethods,
ignore_items: IgnoreAssocItems,
) -> Vec<InFile<ast::AssocItem>> { ) -> Vec<InFile<ast::AssocItem>> {
return items return items
.iter() .iter()
// Note: This throws away items with no source.
.copied() .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| { .filter_map(|assoc_item| {
let item = match assoc_item { let item = match assoc_item {
hir::AssocItem::Function(it) => sema.source(it)?.map(ast::AssocItem::Fn), hir::AssocItem::Function(it) => sema.source(it)?.map(ast::AssocItem::Fn),