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.

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

bugfix : skip doc(hidden) default members #15050

Merged
merged 4 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
184 changes: 183 additions & 1 deletion crates/ide-assists/src/handlers/add_missing_impl_members.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -43,6 +46,7 @@ pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext<'_
acc,
ctx,
DefaultMethods::No,
IgnoreAssocItems::HiddenDocAttrPresent,
"add_impl_missing_members",
"Implement missing members",
)
Expand Down Expand Up @@ -87,6 +91,7 @@ pub(crate) fn add_missing_default_members(
acc,
ctx,
DefaultMethods::Only,
IgnoreAssocItems::HiddenDocAttrPresent,
"add_impl_default_members",
"Implement default members",
)
Expand All @@ -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<()> {
Expand All @@ -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::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,
ign_item,
);

if missing_items.is_empty() {
Expand Down Expand Up @@ -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!()
}
}
"#,
)
}
}
14 changes: 12 additions & 2 deletions crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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::HiddenDocAttrPresent
};

let trait_items =
filter_assoc_items(sema, &trait_.items(sema.db), DefaultMethods::No, ignore_items);

if trait_items.is_empty() {
return None;
}
Expand Down
15 changes: 13 additions & 2 deletions crates/ide-assists/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -84,6 +84,12 @@ pub fn test_related_attribute(fn_def: &ast::Fn) -> Option<ast::Attr> {
})
}

#[derive(Clone, Copy, PartialEq)]
pub enum IgnoreAssocItems {
HiddenDocAttrPresent,
alibektas marked this conversation as resolved.
Show resolved Hide resolved
No,
}

#[derive(Copy, Clone, PartialEq)]
pub enum DefaultMethods {
Only,
Expand All @@ -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::HiddenDocAttrPresent
&& 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),
Expand Down