From ec824a92d05caa1908cb25cbd5b969c8e995aaa7 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 17 Mar 2021 14:38:11 +0100 Subject: Better handling of block doc comments --- crates/hir_def/src/attr.rs | 83 +++++++++++++++++++++----------------- crates/ide/src/goto_definition.rs | 8 ++-- crates/ide/src/hover.rs | 34 ++++++++++++++++ crates/ide/src/runnables.rs | 48 +++++++++++++++++++++- crates/ide_completion/src/lib.rs | 2 +- crates/syntax/src/ast.rs | 29 +++++++------ crates/syntax/src/ast/token_ext.rs | 19 ++++----- crates/syntax/src/ast/traits.rs | 16 +++----- 8 files changed, 158 insertions(+), 81 deletions(-) (limited to 'crates') diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index 7ba53ee5c..aeeb2c5cf 100644 --- a/crates/hir_def/src/attr.rs +++ b/crates/hir_def/src/attr.rs @@ -77,33 +77,19 @@ 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 } + 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::>(); + + Self { entries: if entries.is_empty() { None } else { Some(entries) } } } fn from_attrs_owner(db: &dyn DefDatabase, owner: InFile<&dyn AttrsOwner>) -> Self { @@ -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; @@ -325,15 +311,36 @@ 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::(); - 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.push('\n'); + } else { + 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 +414,7 @@ pub enum AttrInput { } impl Attr { - fn from_src(ast: ast::Attr, hygiene: &Hygiene) -> Option { + fn from_src(ast: ast::Attr, hygiene: &Hygiene, index: u32) -> Option { let path = ModPath::from_src(ast.path()?, hygiene)?; let input = if let Some(lit) = ast.literal() { let value = match lit.kind() { @@ -420,7 +427,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. @@ -508,7 +515,7 @@ impl<'a> AttrQuery<'a> { self.attrs().next().is_some() } - pub fn attrs(self) -> impl Iterator { + pub fn attrs(self) -> impl Iterator + Clone { let key = self.key; self.attrs .iter() diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs index b71f4917c..5072ecea0 100644 --- a/crates/ide/src/goto_definition.rs +++ b/crates/ide/src/goto_definition.rs @@ -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)) } diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 325014622..cc2b79124 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -3423,6 +3423,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( diff --git a/crates/ide/src/runnables.rs b/crates/ide/src/runnables.rs index 397e2126b..bea020b06 100644 --- a/crates/ide/src/runnables.rs +++ b/crates/ide/src/runnables.rs @@ -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 { diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs index 5b7ad38d5..d9ea7b7ea 100644 --- a/crates/ide_completion/src/lib.rs +++ b/crates/ide_completion/src/lib.rs @@ -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" }, ); } diff --git a/crates/syntax/src/ast.rs b/crates/syntax/src/ast.rs index 19261686c..38e0b04ef 100644 --- a/crates/syntax/src/ast.rs +++ b/crates/syntax/src/ast.rs @@ -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] diff --git a/crates/syntax/src/ast/token_ext.rs b/crates/syntax/src/ast/token_ext.rs index 977eb8181..6c242d126 100644 --- a/crates/syntax/src/ast/token_ext.rs +++ b/crates/syntax/src/ast/token_ext.rs @@ -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, } diff --git a/crates/syntax/src/ast/traits.rs b/crates/syntax/src/ast/traits.rs index 96d4cc997..ddd213637 100644 --- a/crates/syntax/src/ast/traits.rs +++ b/crates/syntax/src/ast/traits.rs @@ -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 { - 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 { - 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 { -- cgit v1.2.3