-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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<_>>(); | ||||||
|
@@ -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 { | ||||||
|
@@ -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 | ||||||
} | ||||||
} | ||||||
|
||||||
|
@@ -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)); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
style nit |
||||||
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)); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
outer_attrs | ||||||
.chain(inner_attrs) | ||||||
.enumerate() | ||||||
.map(|(id, (attr, is_inner))| (AttrId::new(id, is_inner), attr)) | ||||||
} | ||||||
|
||||||
fn inner_attributes( | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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()); | ||
Comment on lines
+112
to
+113
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a parameter of |
||
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), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need the mask anymore, we just need to make sure to strip the inner attribute bit when using the index as an actual index