Skip to content

fix: resolve doc path from parent module if outer comments exist on module #19507

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: resolve doc path from parent module if outer comments exist on module #19507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 52 additions & 54 deletions crates/hir-expand/src/attrs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,8 @@ impl RawAttrs {
.cloned()
.chain(b.slice.iter().map(|it| {
let mut it = it.clone();
it.id.id = (it.id.ast_index() as u32 + last_ast_index)
| ((it.id.cfg_attr_index().unwrap_or(0) as u32)
<< AttrId::AST_INDEX_BITS);
let id = it.id.ast_index() as u32 + last_ast_index;
it.id = AttrId::new(id as usize, it.id.is_inner_attr());
it
}))
.collect::<Vec<_>>();
Expand All @@ -128,40 +127,38 @@ impl RawAttrs {
}

let cfg_options = krate.cfg_options(db);
let new_attrs =
self.iter()
.flat_map(|attr| -> SmallVec<[_; 1]> {
let is_cfg_attr =
attr.path.as_ident().is_some_and(|name| *name == sym::cfg_attr.clone());
if !is_cfg_attr {
return smallvec![attr.clone()];
}
let new_attrs = self
.iter()
.flat_map(|attr| -> SmallVec<[_; 1]> {
let is_cfg_attr =
attr.path.as_ident().is_some_and(|name| *name == sym::cfg_attr.clone());
if !is_cfg_attr {
return smallvec![attr.clone()];
}

let subtree = match attr.token_tree_value() {
Some(it) => it,
_ => return smallvec![attr.clone()],
};

let (cfg, parts) = match parse_cfg_attr_input(subtree) {
Some(it) => it,
None => return smallvec![attr.clone()],
};
let index = attr.id;
let attrs = parts.enumerate().take(1 << AttrId::CFG_ATTR_BITS).filter_map(
|(idx, attr)| Attr::from_tt(db, attr, index.with_cfg_attr(idx)),
);

let cfg = TopSubtree::from_token_trees(subtree.top_subtree().delimiter, cfg);
let cfg = CfgExpr::parse(&cfg);
if cfg_options.check(&cfg) == Some(false) {
smallvec![]
} else {
cov_mark::hit!(cfg_attr_active);

attrs.collect()
}
})
.collect::<Vec<_>>();
let subtree = match attr.token_tree_value() {
Some(it) => it,
_ => return smallvec![attr.clone()],
};

let (cfg, parts) = match parse_cfg_attr_input(subtree) {
Some(it) => it,
None => return smallvec![attr.clone()],
};
let index = attr.id;
let attrs = parts.filter_map(|attr| Attr::from_tt(db, attr, index));

let cfg = TopSubtree::from_token_trees(subtree.top_subtree().delimiter, cfg);
let cfg = CfgExpr::parse(&cfg);
if cfg_options.check(&cfg) == Some(false) {
smallvec![]
} else {
cov_mark::hit!(cfg_attr_active);

attrs.collect()
}
})
.collect::<Vec<_>>();
let entries = if new_attrs.is_empty() {
None
} else {
Expand All @@ -183,25 +180,21 @@ pub struct AttrId {
// FIXME: This only handles a single level of cfg_attr nesting
// that is `#[cfg_attr(all(), cfg_attr(all(), cfg(any())))]` breaks again
impl AttrId {
const CFG_ATTR_BITS: usize = 7;
const AST_INDEX_MASK: usize = 0x00FF_FFFF;
const AST_INDEX_BITS: usize = Self::AST_INDEX_MASK.count_ones() as usize;
const CFG_ATTR_SET_BITS: u32 = 1 << 31;
const INNER_ATTR_BIT: usize = 1 << 31;

pub fn ast_index(&self) -> usize {
self.id as usize & Self::AST_INDEX_MASK
pub fn new(id: usize, is_inner: bool) -> Self {
let id = id & Self::AST_INDEX_MASK;
let id = if is_inner { id | Self::INNER_ATTR_BIT } else { id };
Self { id: id as u32 }
}

pub fn cfg_attr_index(&self) -> Option<usize> {
if self.id & Self::CFG_ATTR_SET_BITS == 0 {
None
} else {
Some(self.id as usize >> Self::AST_INDEX_BITS)
}
pub fn ast_index(&self) -> usize {
self.id as usize & Self::AST_INDEX_MASK
}

pub fn with_cfg_attr(self, idx: usize) -> AttrId {
AttrId { id: self.id | ((idx as u32) << Self::AST_INDEX_BITS) | Self::CFG_ATTR_SET_BITS }
pub fn is_inner_attr(&self) -> bool {
(self.id as usize) & Self::INNER_ATTR_BIT != 0
}
}

Expand Down Expand Up @@ -439,13 +432,18 @@ fn unescape(s: &str) -> Option<Cow<'_, str>> {
pub fn collect_attrs(
owner: &dyn ast::HasAttrs,
) -> impl Iterator<Item = (AttrId, Either<ast::Attr, ast::Comment>)> {
let inner_attrs = inner_attributes(owner.syntax()).into_iter().flatten();
let outer_attrs =
ast::AttrDocCommentIter::from_syntax_node(owner.syntax()).filter(|el| match el {
let inner_attrs =
inner_attributes(owner.syntax()).into_iter().flatten().map(|attr| (attr, true));
let outer_attrs = ast::AttrDocCommentIter::from_syntax_node(owner.syntax())
.filter(|el| match el {
Either::Left(attr) => attr.kind().is_outer(),
Either::Right(comment) => comment.is_outer(),
});
outer_attrs.chain(inner_attrs).enumerate().map(|(id, attr)| (AttrId { id: id as u32 }, attr))
})
.map(|attr| (attr, false));
outer_attrs
.chain(inner_attrs)
.enumerate()
.map(|(id, (attr, is_inner))| (AttrId::new(id, is_inner), attr))
}

fn inner_attributes(
Expand Down
17 changes: 14 additions & 3 deletions crates/hir/src/attrs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,32 @@ impl HasAttrs for crate::Crate {
/// Resolves the item `link` points to in the scope of `def`.
pub fn resolve_doc_path_on(
db: &dyn HirDatabase,
def: impl HasAttrs,
def: impl HasAttrs + Copy,
link: &str,
ns: Option<Namespace>,
) -> Option<DocLinkDef> {
resolve_doc_path_on_(db, link, def.attr_id(), ns)
let is_inner =
def.attrs(db).by_key(&intern::sym::doc).attrs().all(|attr| attr.id.is_inner_attr());
resolve_doc_path_on_(db, link, def.attr_id(), ns, is_inner)
}

fn resolve_doc_path_on_(
db: &dyn HirDatabase,
link: &str,
attr_id: AttrDefId,
ns: Option<Namespace>,
is_inner: bool,
) -> Option<DocLinkDef> {
let resolver = match attr_id {
AttrDefId::ModuleId(it) => it.resolver(db),
AttrDefId::ModuleId(it) => {
if is_inner {
it.resolver(db)
} else if let Some(parent) = Module::from(it).parent(db) {
parent.id.resolver(db)
} else {
it.resolver(db)
}
}
AttrDefId::FieldId(it) => it.parent.resolver(db),
AttrDefId::AdtId(it) => it.resolver(db),
AttrDefId::FunctionId(it) => it.resolver(db),
Expand Down
57 changes: 57 additions & 0 deletions crates/ide/src/doc_links/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,40 @@ struct S$0(i32);
);
}

#[test]
fn doc_links_module() {
check_doc_links(
r#"
/// [`M`]
/// [`M::f`]
mod M$0 {
//^ M
#![doc = "inner_item[`M::S`]"]

pub fn f() {}
//^ M::f
pub struct S;
//^ M::S
}
"#,
);

check_doc_links(
r#"
mod M$0 {
//^ super::M
//! [`super::M`]
//! [`super::M::f`]
//! [`super::M::S`]
pub fn f() {}
//^ super::M::f
pub struct S;
//^ super::M::S
}
"#,
);
}

#[test]
fn rewrite_html_root_url() {
check_rewrite(
Expand Down Expand Up @@ -690,6 +724,29 @@ fn rewrite_intra_doc_link_with_anchor() {
);
}

#[test]
fn rewrite_module() {
check_rewrite(
r#"
//- /main.rs crate:foo
/// [Foo]
pub mod $0Foo{
};
"#,
expect![[r#"[Foo](https://docs.rs/foo/*/foo/Foo/index.html)"#]],
);

check_rewrite(
r#"
//- /main.rs crate:foo
pub mod $0Foo{
//! [super::Foo]
};
"#,
expect![[r#"[super::Foo](https://docs.rs/foo/*/foo/Foo/index.html)"#]],
);
}

#[test]
fn rewrite_intra_doc_link_to_associated_item() {
check_rewrite(
Expand Down
4 changes: 2 additions & 2 deletions crates/ide/src/syntax_highlighting/test_data/highlight_module_docs_inline.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@
.invalid_escape_sequence { color: #FC5555; text-decoration: wavy underline; }
.unresolved_reference { color: #FC5555; text-decoration: wavy underline; }
</style>
<pre><code><span class="comment documentation">//! </span><span class="struct documentation injected intra_doc_link">[Struct]</span>
<pre><code><span class="comment documentation">//! </span><span class="struct documentation injected intra_doc_link">[foo::Struct]</span>
<span class="comment documentation">//! This is an intra doc injection test for modules</span>
<span class="comment documentation">//! </span><span class="struct documentation injected intra_doc_link">[Struct]</span>
<span class="comment documentation">//! </span><span class="struct documentation injected intra_doc_link">[foo::Struct]</span>
<span class="comment documentation">//! This is an intra doc injection test for modules</span>

<span class="keyword">pub</span> <span class="keyword">struct</span> <span class="struct declaration public">Struct</span><span class="semicolon">;</span>
Expand Down
8 changes: 4 additions & 4 deletions crates/ide/src/syntax_highlighting/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1072,9 +1072,9 @@ fn test_mod_hl_injection() {
check_highlighting(
r##"
//- /foo.rs
//! [Struct]
//! [foo::Struct]
//! This is an intra doc injection test for modules
//! [Struct]
//! [foo::Struct]
//! This is an intra doc injection test for modules

pub struct Struct;
Expand All @@ -1097,9 +1097,9 @@ mod foo;
/// This is an intra doc injection test for modules
mod foo;
//- /foo.rs
//! [Struct]
//! [foo::Struct]
//! This is an intra doc injection test for modules
//! [Struct]
//! [foo::Struct]
//! This is an intra doc injection test for modules

pub struct Struct;
Expand Down