mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge #8065

Browse Source 
8065: Better handling of block doc comments r=Veykril a=Veykril

Moves doc string processing to `Attrs::docs`, as we need the indent info from all comments before being able to know how much to strip

Closes #7774

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
This commit is contained in:
bors[bot] 2021-03-17 16:00:43 +00:00 committed by GitHub
commit edf11480ce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 235 additions and 140 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
crates/hir_def/src/attr.rs
View File

@ -76,37 +76,23 @@ impl ops::Deref for Attrs {
impl RawAttrs {
pub(crate) const EMPTY: Self = Self { entries: None };
pub(crate) fn new(owner: &dyn AttrsOwner, hygiene: &Hygiene) -> Self {
let attrs: Vec<_> = collect_attrs(owner).collect();
let entries = if attrs.is_empty() {
// Avoid heap allocation
None
} else {
Some(
attrs
.into_iter()
.enumerate()
.flat_map(|(i, attr)| match attr {
Either::Left(attr) => Attr::from_src(attr, hygiene).map(|attr| (i, attr)),
Either::Right(comment) => comment.doc_comment().map(|doc| {
(
i,
Attr {
index: 0,
input: Some(AttrInput::Literal(SmolStr::new(doc))),
path: ModPath::from(hir_expand::name!(doc)),
},
)
}),
})
.map(|(i, attr)| Attr { index: i as u32, ..attr })
.collect(),
)
};
Self { entries }
pub(crate) fn new(owner: &dyn ast::AttrsOwner, hygiene: &Hygiene) -> Self {
let entries = collect_attrs(owner)
.enumerate()
.flat_map(|(i, attr)| match attr {
Either::Left(attr) => Attr::from_src(attr, hygiene, i as u32),
Either::Right(comment) => comment.doc_comment().map(|doc| Attr {
index: i as u32,
input: Some(AttrInput::Literal(SmolStr::new(doc))),
path: ModPath::from(hir_expand::name!(doc)),
}),
})
.collect::<Arc<_>>();
Self { entries: if entries.is_empty() { None } else { Some(entries) } }
}
fn from_attrs_owner(db: &dyn DefDatabase, owner: InFile<&dyn AttrsOwner>) -> Self {
fn from_attrs_owner(db: &dyn DefDatabase, owner: InFile<&dyn ast::AttrsOwner>) -> Self {
let hygiene = Hygiene::new(db.upcast(), owner.file_id);
Self::new(owner.value, &hygiene)
}
@ -162,7 +148,7 @@ impl RawAttrs {
let attr = ast::Attr::parse(&format!("#[{}]", tree)).ok()?;
// FIXME hygiene
let hygiene = Hygiene::new_unhygienic();
Attr::from_src(attr, &hygiene).map(|attr| Attr { index, ..attr })
Attr::from_src(attr, &hygiene, index)
});
let cfg_options = &crate_graph[krate].cfg_options;
@ -192,7 +178,7 @@ impl Attrs {
Some(it) => {
let raw_attrs = RawAttrs::from_attrs_owner(
db,
it.as_ref().map(|it| it as &dyn AttrsOwner),
it.as_ref().map(|it| it as &dyn ast::AttrsOwner),
);
match mod_data.definition_source(db) {
InFile { file_id, value: ModuleSource::SourceFile(file) } => raw_attrs
@ -203,9 +189,9 @@ impl Attrs {
None => RawAttrs::from_attrs_owner(
db,
mod_data.definition_source(db).as_ref().map(|src| match src {
ModuleSource::SourceFile(file) => file as &dyn AttrsOwner,
ModuleSource::Module(module) => module as &dyn AttrsOwner,
ModuleSource::BlockExpr(block) => block as &dyn AttrsOwner,
ModuleSource::SourceFile(file) => file as &dyn ast::AttrsOwner,
ModuleSource::Module(module) => module as &dyn ast::AttrsOwner,
ModuleSource::BlockExpr(block) => block as &dyn ast::AttrsOwner,
}),
),
}
@ -263,7 +249,7 @@ impl Attrs {
let mut res = ArenaMap::default();
for (id, var) in src.value.iter() {
let attrs = RawAttrs::from_attrs_owner(db, src.with_value(var as &dyn AttrsOwner))
let attrs = RawAttrs::from_attrs_owner(db, src.with_value(var as &dyn ast::AttrsOwner))
.filter(db, krate);
res.insert(id, attrs)
@ -297,7 +283,7 @@ impl Attrs {
/// Constructs a map that maps the lowered `Attr`s in this `Attrs` back to its original syntax nodes.
///
/// `owner` must be the original owner of the attributes.
pub fn source_map(&self, owner: &dyn AttrsOwner) -> AttrSourceMap {
pub fn source_map(&self, owner: &dyn ast::AttrsOwner) -> AttrSourceMap {
AttrSourceMap { attrs: collect_attrs(owner).collect() }
}
@ -325,15 +311,34 @@ impl Attrs {
AttrInput::Literal(s) => Some(s),
AttrInput::TokenTree(_) => None,
});
// FIXME: Replace `Itertools::intersperse` with `Iterator::intersperse[_with]` until the
// libstd api gets stabilized (https://github.com/rust-lang/rust/issues/79524).
let docs = Itertools::intersperse(docs, &SmolStr::new_inline("\n"))
.map(|it| it.as_str())
.collect::<String>();
if docs.is_empty() {
let indent = docs
.clone()
.flat_map(|s| s.lines())
.filter(|line| !line.chars().all(|c| c.is_whitespace()))
.map(|line| line.chars().take_while(|c| c.is_whitespace()).count())
.min()
.unwrap_or(0);
let mut buf = String::new();
for doc in docs {
// str::lines doesn't yield anything for the empty string
if !doc.is_empty() {
buf.extend(Itertools::intersperse(
doc.lines().map(|line| {
line.char_indices()
.nth(indent)
.map_or(line, |(offset, _)| &line[offset..])
.trim_end()
}),
"\n",
));
}
buf.push('\n');
}
buf.pop();
if buf.is_empty() {
None
} else {
Some(Documentation(docs))
Some(Documentation(buf))
}
}
}
@ -407,7 +412,7 @@ pub enum AttrInput {
}
impl Attr {
fn from_src(ast: ast::Attr, hygiene: &Hygiene) -> Option<Attr> {
fn from_src(ast: ast::Attr, hygiene: &Hygiene, index: u32) -> Option<Attr> {
let path = ModPath::from_src(ast.path()?, hygiene)?;
let input = if let Some(lit) = ast.literal() {
let value = match lit.kind() {
@ -420,7 +425,7 @@ impl Attr {
} else {
None
};
Some(Attr { index: 0, path, input })
Some(Attr { index, path, input })
}
/// Maps this lowered `Attr` back to its original syntax node.
@ -429,7 +434,7 @@ impl Attr {
///
/// Note that the returned syntax node might be a `#[cfg_attr]`, or a doc comment, instead of
/// the attribute represented by `Attr`.
pub fn to_src(&self, owner: &dyn AttrsOwner) -> Either<ast::Attr, ast::Comment> {
pub fn to_src(&self, owner: &dyn ast::AttrsOwner) -> Either<ast::Attr, ast::Comment> {
collect_attrs(owner).nth(self.index as usize).unwrap_or_else(|| {
panic!("cannot find `Attr` at index {} in {}", self.index, owner.syntax())
})
@ -508,7 +513,7 @@ impl<'a> AttrQuery<'a> {
self.attrs().next().is_some()
}
pub fn attrs(self) -> impl Iterator<Item = &'a Attr> {
pub fn attrs(self) -> impl Iterator<Item = &'a Attr> + Clone {
let key = self.key;
self.attrs
.iter()
@ -521,7 +526,7 @@ where
N: ast::AttrsOwner,
{
let src = InFile::new(src.file_id, src.to_node(db.upcast()));
RawAttrs::from_attrs_owner(db, src.as_ref().map(|it| it as &dyn AttrsOwner))
RawAttrs::from_attrs_owner(db, src.as_ref().map(|it| it as &dyn ast::AttrsOwner))
}
fn attrs_from_item_tree<N: ItemTreeNode>(id: ItemTreeId<N>, db: &dyn DefDatabase) -> RawAttrs {
@ -530,7 +535,9 @@ fn attrs_from_item_tree<N: ItemTreeNode>(id: ItemTreeId<N>, db: &dyn DefDatabase
tree.raw_attrs(mod_item.into()).clone()
}
fn collect_attrs(owner: &dyn AttrsOwner) -> impl Iterator<Item = Either<ast::Attr, ast::Comment>> {
fn collect_attrs(
owner: &dyn ast::AttrsOwner,
) -> impl Iterator<Item = Either<ast::Attr, ast::Comment>> {
let (inner_attrs, inner_docs) = inner_attributes(owner.syntax())
.map_or((None, None), |(attrs, docs)| ((Some(attrs), Some(docs))));

8
crates/ide/src/goto_definition.rs
View File

@ -95,12 +95,10 @@ fn extract_positioned_link_from_comment(
let comment_range = comment.syntax().text_range();
let doc_comment = comment.doc_comment()?;
let def_links = extract_definitions_from_markdown(doc_comment);
let start = comment_range.start() + TextSize::from(comment.prefix().len() as u32);
let (def_link, ns, _) = def_links.iter().min_by_key(|(_, _, def_link_range)| {
let matched_position = comment_range.start() + TextSize::from(def_link_range.start as u32);
match position.offset.checked_sub(matched_position) {
Some(distance) => distance,
None => comment_range.end(),
}
let matched_position = start + TextSize::from(def_link_range.start as u32);
position.offset.checked_sub(matched_position).unwrap_or_else(|| comment_range.end())
})?;
Some((def_link.to_string(), *ns))
}

60
crates/ide/src/hover.rs
View File

@ -1533,12 +1533,21 @@ fn my() {}
fn test_hover_struct_doc_comment() {
check(
r#"
/// bar docs
/// This is an example
/// multiline doc
///
/// # Example
///
/// ```
/// let five = 5;
///
/// assert_eq!(6, my_crate::add_one(5));
/// ```
struct Bar;
fn foo() { let bar = Ba$0r; }
"#,
expect![[r#"
expect![[r##"
*Bar*
```rust
@ -1551,8 +1560,17 @@ fn foo() { let bar = Ba$0r; }
---
bar docs
"#]],
This is an example
multiline doc
# Example
```
let five = 5;
assert_eq!(6, my_crate::add_one(5));
```
"##]],
);
}
@ -3423,6 +3441,40 @@ mod Foo$0 {
);
}
#[test]
fn hover_doc_block_style_indentend() {
check(
r#"
/**
foo
```rust
let x = 3;
```
*/
fn foo$0() {}
"#,
expect![[r#"
*foo*
```rust
test
```
```rust
fn foo()
```
---
foo
```rust
let x = 3;
```
"#]],
);
}
#[test]
fn hover_comments_dont_highlight_parent() {
check_hover_no_result(

48
crates/ide/src/runnables.rs
View File

@ -576,6 +576,20 @@ fn should_have_runnable_1() {}
/// ```
fn should_have_runnable_2() {}
/**
```rust
let z = 55;
```
*/
fn should_have_no_runnable_3() {}
/**
```rust
let z = 55;
```
*/
fn should_have_no_runnable_4() {}
/// ```no_run
/// let z = 55;
/// ```
@ -616,7 +630,7 @@ fn should_have_no_runnable_6() {}
struct StructWithRunnable(String);
"#,
&[&BIN, &DOCTEST, &DOCTEST, &DOCTEST, &DOCTEST],
&[&BIN, &DOCTEST, &DOCTEST, &DOCTEST, &DOCTEST, &DOCTEST, &DOCTEST],
expect![[r#"
[
Runnable {
@ -682,7 +696,37 @@ struct StructWithRunnable(String);
file_id: FileId(
0,
),
full_range: 756..821,
full_range: 256..320,
name: "should_have_no_runnable_3",
},
kind: DocTest {
test_id: Path(
"should_have_no_runnable_3",
),
},
cfg: None,
},
Runnable {
nav: NavigationTarget {
file_id: FileId(
0,
),
full_range: 322..398,
name: "should_have_no_runnable_4",
},
kind: DocTest {
test_id: Path(
"should_have_no_runnable_4",
),
},
cfg: None,
},
Runnable {
nav: NavigationTarget {
file_id: FileId(
0,
),
full_range: 900..965,
name: "StructWithRunnable",
},
kind: DocTest {

2
crates/ide_completion/src/lib.rs
View File

@ -255,7 +255,7 @@ fn foo() {
bar.fo$0;
}
"#,
DetailAndDocumentation { detail: "fn(&self)", documentation: " Do the foo" },
DetailAndDocumentation { detail: "fn(&self)", documentation: "Do the foo" },
);
}

6
crates/ide_db/src/call_info.rs
View File

@ -53,15 +53,15 @@ pub fn call_info(db: &RootDatabase, position: FilePosition) -> Option<CallInfo>
match callable.kind() {
hir::CallableKind::Function(func) => {
res.doc = func.docs(db).map(|it| it.as_str().to_string());
res.doc = func.docs(db).map(|it| it.into());
format_to!(res.signature, "fn {}", func.name(db));
}
hir::CallableKind::TupleStruct(strukt) => {
res.doc = strukt.docs(db).map(|it| it.as_str().to_string());
res.doc = strukt.docs(db).map(|it| it.into());
format_to!(res.signature, "struct {}", strukt.name(db));
}
hir::CallableKind::TupleEnumVariant(variant) => {
res.doc = variant.docs(db).map(|it| it.as_str().to_string());
res.doc = variant.docs(db).map(|it| it.into());
format_to!(
res.signature,
"enum {}::{}",

82
crates/ide_db/src/call_info/tests.rs
View File

@ -220,11 +220,11 @@ fn bar() {
}
"#,
expect![[r#"
test
------
fn foo(j: u32) -> u32
(<j: u32>)
"#]],
test
------
fn foo(j: u32) -> u32
(<j: u32>)
"#]],
);
}
@ -249,19 +249,19 @@ pub fn do() {
add_one($0
}"#,
expect![[r##"
Adds one to the number given.
Adds one to the number given.
# Examples
# Examples
```
let five = 5;
```
let five = 5;
assert_eq!(6, my_crate::add_one(5));
```
------
fn add_one(x: i32) -> i32
(<x: i32>)
"##]],
assert_eq!(6, my_crate::add_one(5));
```
------
fn add_one(x: i32) -> i32
(<x: i32>)
"##]],
);
}
@ -291,19 +291,19 @@ pub fn do_it() {
}
"#,
expect![[r##"
Adds one to the number given.
Adds one to the number given.
# Examples
# Examples
```
let five = 5;
```
let five = 5;
assert_eq!(6, my_crate::add_one(5));
```
------
fn add_one(x: i32) -> i32
(<x: i32>)
"##]],
assert_eq!(6, my_crate::add_one(5));
```
------
fn add_one(x: i32) -> i32
(<x: i32>)
"##]],
);
}
@ -335,13 +335,13 @@ pub fn foo(mut r: WriteHandler<()>) {
}
"#,
expect![[r#"
Method is called when writer finishes.
Method is called when writer finishes.
By default this method stops actor's `Context`.
------
fn finished(&mut self, ctx: &mut {unknown})
(<ctx: &mut {unknown}>)
"#]],
By default this method stops actor's `Context`.
------
fn finished(&mut self, ctx: &mut {unknown})
(<ctx: &mut {unknown}>)
"#]],
);
}
@ -389,11 +389,11 @@ fn main() {
}
"#,
expect![[r#"
A cool tuple struct
------
struct S(u32, i32)
(u32, <i32>)
"#]],
A cool tuple struct
------
struct S(u32, i32)
(u32, <i32>)
"#]],
);
}
@ -431,11 +431,11 @@ fn main() {
}
"#,
expect![[r#"
A Variant
------
enum E::A(i32)
(<i32>)
"#]],
A Variant
------
enum E::A(i32)
(<i32>)
"#]],
);
}

29
crates/syntax/src/ast.rs
View File

@ -118,7 +118,7 @@ fn test_doc_comment_none() {
.ok()
.unwrap();
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
assert!(module.doc_comment_text().is_none());
assert!(module.doc_comments().doc_comment_text().is_none());
}
#[test]
@ -133,7 +133,7 @@ fn test_outer_doc_comment_of_items() {
.ok()
.unwrap();
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
assert_eq!("doc", module.doc_comment_text().unwrap());
assert_eq!(" doc", module.doc_comments().doc_comment_text().unwrap());
}
#[test]
@ -148,7 +148,7 @@ fn test_inner_doc_comment_of_items() {
.ok()
.unwrap();
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
assert!(module.doc_comment_text().is_none());
assert!(module.doc_comments().doc_comment_text().is_none());
}
#[test]
@ -162,7 +162,7 @@ fn test_doc_comment_of_statics() {
.ok()
.unwrap();
let st = file.syntax().descendants().find_map(Static::cast).unwrap();
assert_eq!("Number of levels", st.doc_comment_text().unwrap());
assert_eq!(" Number of levels", st.doc_comments().doc_comment_text().unwrap());
}
#[test]
@ -181,7 +181,10 @@ fn test_doc_comment_preserves_indents() {
.ok()
.unwrap();
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
assert_eq!("doc1\n```\nfn foo() {\n // ...\n}\n```", module.doc_comment_text().unwrap());
assert_eq!(
" doc1\n ```\n fn foo() {\n // ...\n }\n ```",
module.doc_comments().doc_comment_text().unwrap()
);
}
#[test]
@ -198,7 +201,7 @@ fn test_doc_comment_preserves_newlines() {
.ok()
.unwrap();
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
assert_eq!("this\nis\nmod\nfoo", module.doc_comment_text().unwrap());
assert_eq!(" this\n is\n mod\n foo", module.doc_comments().doc_comment_text().unwrap());
}
#[test]
@ -212,7 +215,7 @@ fn test_doc_comment_single_line_block_strips_suffix() {
.ok()
.unwrap();
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
assert_eq!("this is mod foo", module.doc_comment_text().unwrap());
assert_eq!(" this is mod foo", module.doc_comments().doc_comment_text().unwrap());
}
#[test]
@ -226,7 +229,7 @@ fn test_doc_comment_single_line_block_strips_suffix_whitespace() {
.ok()
.unwrap();
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
assert_eq!("this is mod foo ", module.doc_comment_text().unwrap());
assert_eq!(" this is mod foo ", module.doc_comments().doc_comment_text().unwrap());
}
#[test]
@ -245,8 +248,8 @@ fn test_doc_comment_multi_line_block_strips_suffix() {
.unwrap();
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
assert_eq!(
" this\n is\n mod foo\n ",
module.doc_comment_text().unwrap()
"\n this\n is\n mod foo\n ",
module.doc_comments().doc_comment_text().unwrap()
);
}
@ -259,8 +262,8 @@ fn test_comments_preserve_trailing_whitespace() {
.unwrap();
let def = file.syntax().descendants().find_map(Struct::cast).unwrap();
assert_eq!(
"Representation of a Realm. \nIn the specification these are called Realm Records.",
def.doc_comment_text().unwrap()
" Representation of a Realm. \n In the specification these are called Realm Records.",
def.doc_comments().doc_comment_text().unwrap()
);
}
@ -276,7 +279,7 @@ fn test_four_slash_line_comment() {
.ok()
.unwrap();
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
assert_eq!("doc comment", module.doc_comment_text().unwrap());
assert_eq!(" doc comment", module.doc_comments().doc_comment_text().unwrap());
}
#[test]

19
crates/syntax/src/ast/token_ext.rs
View File

@ -33,23 +33,20 @@ impl ast::Comment {
prefix
}
/// Returns the textual content of a doc comment block as a single string.
/// That is, strips leading `///` (+ optional 1 character of whitespace),
/// trailing `*/`, trailing whitespace and then joins the lines.
/// Returns the textual content of a doc comment node as a single string with prefix and suffix
/// removed.
pub fn doc_comment(&self) -> Option<&str> {
let kind = self.kind();
match kind {
CommentKind { shape, doc: Some(_) } => {
let prefix = kind.prefix();
let text = &self.text()[prefix.len()..];
let ws = text.chars().next().filter(|c| c.is_whitespace());
let text = ws.map_or(text, |ws| &text[ws.len_utf8()..]);
match shape {
CommentShape::Block if text.ends_with("*/") => {
Some(&text[..text.len() - "*/".len()])
}
_ => Some(text),
}
let text = if shape == CommentShape::Block {
text.strip_suffix("*/").unwrap_or(text)
} else {
text
};
Some(text)
}
_ => None,
}

16
crates/syntax/src/ast/traits.rs
View File

@ -1,8 +1,6 @@
//! Various traits that are implemented by ast nodes.
//!
//! The implementations are usually trivial, and live in generated.rs
use itertools::Itertools;
use crate::{
ast::{self, support, AstChildren, AstNode, AstToken},
syntax_node::SyntaxElementChildren,
@ -76,10 +74,6 @@ pub trait DocCommentsOwner: AttrsOwner {
fn doc_comments(&self) -> CommentIter {
CommentIter { iter: self.syntax().children_with_tokens() }
}
fn doc_comment_text(&self) -> Option<String> {
self.doc_comments().doc_comment_text()
}
}
impl CommentIter {
@ -87,12 +81,12 @@ impl CommentIter {
CommentIter { iter: syntax_node.children_with_tokens() }
}
/// Returns the textual content of a doc comment block as a single string.
/// That is, strips leading `///` (+ optional 1 character of whitespace),
/// trailing `*/`, trailing whitespace and then joins the lines.
#[cfg(test)]
pub fn doc_comment_text(self) -> Option<String> {
let docs =
self.filter_map(|comment| comment.doc_comment().map(ToOwned::to_owned)).join("\n");
let docs = itertools::Itertools::join(
&mut self.filter_map(|comment| comment.doc_comment().map(ToOwned::to_owned)),
"\n",
);
if docs.is_empty() {
None
} else {