Skip to content

Commit ec824a9

Browse files
VeykrilVeykril
committed
Better handling of block doc comments
1 parent 0fbfab3 commit ec824a9

File tree

8 files changed

+158
-81
lines changed

8 files changed

+158
-81
lines changed

‎crates/hir_def/src/attr.rs

+45-38
Original file line numberDiff line numberDiff line change
@@ -77,33 +77,19 @@ impl RawAttrs {
7777
pub(crate) const EMPTY: Self = Self { entries: None };
7878

7979
pub(crate) fn new(owner: &dyn AttrsOwner, hygiene: &Hygiene) -> Self {
80-
let attrs: Vec<_> = collect_attrs(owner).collect();
81-
let entries = if attrs.is_empty() {
82-
// Avoid heap allocation
83-
None
84-
} else {
85-
Some(
86-
attrs
87-
.into_iter()
88-
.enumerate()
89-
.flat_map(|(i, attr)| match attr {
90-
Either::Left(attr) => Attr::from_src(attr, hygiene).map(|attr| (i, attr)),
91-
Either::Right(comment) => comment.doc_comment().map(|doc| {
92-
(
93-
i,
94-
Attr {
95-
index: 0,
96-
input: Some(AttrInput::Literal(SmolStr::new(doc))),
97-
path: ModPath::from(hir_expand::name!(doc)),
98-
},
99-
)
100-
}),
101-
})
102-
.map(|(i, attr)| Attr { index: i as u32, ..attr })
103-
.collect(),
104-
)
105-
};
106-
Self { entries }
80+
let entries = collect_attrs(owner)
81+
.enumerate()
82+
.flat_map(|(i, attr)| match attr {
83+
Either::Left(attr) => Attr::from_src(attr, hygiene, i as u32),
84+
Either::Right(comment) => comment.doc_comment().map(|doc| Attr {
85+
index: i as u32,
86+
input: Some(AttrInput::Literal(SmolStr::new(doc))),
87+
path: ModPath::from(hir_expand::name!(doc)),
88+
}),
89+
})
90+
.collect::<Arc<_>>();
91+
92+
Self { entries: if entries.is_empty() { None } else { Some(entries) } }
10793
}
10894

10995
fn from_attrs_owner(db: &dyn DefDatabase, owner: InFile<&dyn AttrsOwner>) -> Self {
@@ -162,7 +148,7 @@ impl RawAttrs {
162148
let attr = ast::Attr::parse(&format!("#[{}]", tree)).ok()?;
163149
// FIXME hygiene
164150
let hygiene = Hygiene::new_unhygienic();
165-
Attr::from_src(attr, &hygiene).map(|attr| Attr { index, ..attr })
151+
Attr::from_src(attr, &hygiene, index)
166152
});
167153

168154
let cfg_options = &crate_graph[krate].cfg_options;
@@ -325,15 +311,36 @@ impl Attrs {
325311
AttrInput::Literal(s) => Some(s),
326312
AttrInput::TokenTree(_) => None,
327313
});
328-
// FIXME: Replace `Itertools::intersperse` with `Iterator::intersperse[_with]` until the
329-
// libstd api gets stabilized (https://github.com/rust-lang/rust/issues/79524).
330-
let docs = Itertools::intersperse(docs, &SmolStr::new_inline("\n"))
331-
.map(|it| it.as_str())
332-
.collect::<String>();
333-
if docs.is_empty() {
314+
let indent = docs
315+
.clone()
316+
.flat_map(|s| s.lines())
317+
.filter(|line| !line.chars().all(|c| c.is_whitespace()))
318+
.map(|line| line.chars().take_while(|c| c.is_whitespace()).count())
319+
.min()
320+
.unwrap_or(0);
321+
let mut buf = String::new();
322+
for doc in docs {
323+
// str::lines doesn't yield anything for the empty string
324+
if doc.is_empty() {
325+
buf.push('\n');
326+
} else {
327+
buf.extend(Itertools::intersperse(
328+
doc.lines().map(|line| {
329+
line.char_indices()
330+
.nth(indent)
331+
.map_or(line, |(offset, _)| &line[offset..])
332+
.trim_end()
333+
}),
334+
"\n",
335+
));
336+
}
337+
buf.push('\n');
338+
}
339+
buf.pop();
340+
if buf.is_empty() {
334341
None
335342
} else {
336-
Some(Documentation(docs))
343+
Some(Documentation(buf))
337344
}
338345
}
339346
}
@@ -407,7 +414,7 @@ pub enum AttrInput {
407414
}
408415

409416
impl Attr {
410-
fn from_src(ast: ast::Attr, hygiene: &Hygiene) -> Option<Attr> {
417+
fn from_src(ast: ast::Attr, hygiene: &Hygiene, index: u32) -> Option<Attr> {
411418
let path = ModPath::from_src(ast.path()?, hygiene)?;
412419
let input = if let Some(lit) = ast.literal() {
413420
let value = match lit.kind() {
@@ -420,7 +427,7 @@ impl Attr {
420427
} else {
421428
None
422429
};
423-
Some(Attr { index: 0, path, input })
430+
Some(Attr { index, path, input })
424431
}
425432

426433
/// Maps this lowered `Attr` back to its original syntax node.
@@ -508,7 +515,7 @@ impl<'a> AttrQuery<'a> {
508515
self.attrs().next().is_some()
509516
}
510517

511-
pub fn attrs(self) -> impl Iterator<Item = &'a Attr> {
518+
pub fn attrs(self) -> impl Iterator<Item = &'a Attr> + Clone {
512519
let key = self.key;
513520
self.attrs
514521
.iter()

‎crates/ide/src/goto_definition.rs

+3-5
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,10 @@ fn extract_positioned_link_from_comment(
9595
let comment_range = comment.syntax().text_range();
9696
let doc_comment = comment.doc_comment()?;
9797
let def_links = extract_definitions_from_markdown(doc_comment);
98+
let start = comment_range.start() + TextSize::from(comment.prefix().len() as u32);
9899
let (def_link, ns, _) = def_links.iter().min_by_key(|(_, _, def_link_range)| {
99-
let matched_position = comment_range.start() + TextSize::from(def_link_range.start as u32);
100-
match position.offset.checked_sub(matched_position) {
101-
Some(distance) => distance,
102-
None => comment_range.end(),
103-
}
100+
let matched_position = start + TextSize::from(def_link_range.start as u32);
101+
position.offset.checked_sub(matched_position).unwrap_or_else(|| comment_range.end())
104102
})?;
105103
Some((def_link.to_string(), *ns))
106104
}

‎crates/ide/src/hover.rs

+34
Original file line numberDiff line numberDiff line change
@@ -3423,6 +3423,40 @@ mod Foo$0 {
34233423
);
34243424
}
34253425

3426+
#[test]
3427+
fn hover_doc_block_style_indentend() {
3428+
check(
3429+
r#"
3430+
/**
3431+
foo
3432+
```rust
3433+
let x = 3;
3434+
```
3435+
*/
3436+
fn foo$0() {}
3437+
"#,
3438+
expect![[r#"
3439+
*foo*
3440+
3441+
```rust
3442+
test
3443+
```
3444+
3445+
```rust
3446+
fn foo()
3447+
```
3448+
3449+
---
3450+
3451+
foo
3452+
3453+
```rust
3454+
let x = 3;
3455+
```
3456+
"#]],
3457+
);
3458+
}
3459+
34263460
#[test]
34273461
fn hover_comments_dont_highlight_parent() {
34283462
check_hover_no_result(

‎crates/ide/src/runnables.rs

+46-2
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,20 @@ fn should_have_runnable_1() {}
576576
/// ```
577577
fn should_have_runnable_2() {}
578578
579+
/**
580+
```rust
581+
let z = 55;
582+
```
583+
*/
584+
fn should_have_no_runnable_3() {}
585+
586+
/**
587+
```rust
588+
let z = 55;
589+
```
590+
*/
591+
fn should_have_no_runnable_4() {}
592+
579593
/// ```no_run
580594
/// let z = 55;
581595
/// ```
@@ -616,7 +630,7 @@ fn should_have_no_runnable_6() {}
616630
struct StructWithRunnable(String);
617631
618632
"#,
619-
&[&BIN, &DOCTEST, &DOCTEST, &DOCTEST, &DOCTEST],
633+
&[&BIN, &DOCTEST, &DOCTEST, &DOCTEST, &DOCTEST, &DOCTEST, &DOCTEST],
620634
expect![[r#"
621635
[
622636
Runnable {
@@ -682,7 +696,37 @@ struct StructWithRunnable(String);
682696
file_id: FileId(
683697
0,
684698
),
685-
full_range: 756..821,
699+
full_range: 256..320,
700+
name: "should_have_no_runnable_3",
701+
},
702+
kind: DocTest {
703+
test_id: Path(
704+
"should_have_no_runnable_3",
705+
),
706+
},
707+
cfg: None,
708+
},
709+
Runnable {
710+
nav: NavigationTarget {
711+
file_id: FileId(
712+
0,
713+
),
714+
full_range: 322..398,
715+
name: "should_have_no_runnable_4",
716+
},
717+
kind: DocTest {
718+
test_id: Path(
719+
"should_have_no_runnable_4",
720+
),
721+
},
722+
cfg: None,
723+
},
724+
Runnable {
725+
nav: NavigationTarget {
726+
file_id: FileId(
727+
0,
728+
),
729+
full_range: 900..965,
686730
name: "StructWithRunnable",
687731
},
688732
kind: DocTest {

‎crates/ide_completion/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ fn foo() {
255255
bar.fo$0;
256256
}
257257
"#,
258-
DetailAndDocumentation { detail: "fn(&self)", documentation: " Do the foo" },
258+
DetailAndDocumentation { detail: "fn(&self)", documentation: "Do the foo" },
259259
);
260260
}
261261

‎crates/syntax/src/ast.rs

+16-13
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ fn test_doc_comment_none() {
118118
.ok()
119119
.unwrap();
120120
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
121-
assert!(module.doc_comment_text().is_none());
121+
assert!(module.doc_comments().doc_comment_text().is_none());
122122
}
123123

124124
#[test]
@@ -133,7 +133,7 @@ fn test_outer_doc_comment_of_items() {
133133
.ok()
134134
.unwrap();
135135
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
136-
assert_eq!("doc", module.doc_comment_text().unwrap());
136+
assert_eq!(" doc", module.doc_comments().doc_comment_text().unwrap());
137137
}
138138

139139
#[test]
@@ -148,7 +148,7 @@ fn test_inner_doc_comment_of_items() {
148148
.ok()
149149
.unwrap();
150150
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
151-
assert!(module.doc_comment_text().is_none());
151+
assert!(module.doc_comments().doc_comment_text().is_none());
152152
}
153153

154154
#[test]
@@ -162,7 +162,7 @@ fn test_doc_comment_of_statics() {
162162
.ok()
163163
.unwrap();
164164
let st = file.syntax().descendants().find_map(Static::cast).unwrap();
165-
assert_eq!("Number of levels", st.doc_comment_text().unwrap());
165+
assert_eq!(" Number of levels", st.doc_comments().doc_comment_text().unwrap());
166166
}
167167

168168
#[test]
@@ -181,7 +181,10 @@ fn test_doc_comment_preserves_indents() {
181181
.ok()
182182
.unwrap();
183183
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
184-
assert_eq!("doc1\n```\nfn foo() {\n // ...\n}\n```", module.doc_comment_text().unwrap());
184+
assert_eq!(
185+
" doc1\n ```\n fn foo() {\n // ...\n }\n ```",
186+
module.doc_comments().doc_comment_text().unwrap()
187+
);
185188
}
186189

187190
#[test]
@@ -198,7 +201,7 @@ fn test_doc_comment_preserves_newlines() {
198201
.ok()
199202
.unwrap();
200203
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
201-
assert_eq!("this\nis\nmod\nfoo", module.doc_comment_text().unwrap());
204+
assert_eq!(" this\n is\n mod\n foo", module.doc_comments().doc_comment_text().unwrap());
202205
}
203206

204207
#[test]
@@ -212,7 +215,7 @@ fn test_doc_comment_single_line_block_strips_suffix() {
212215
.ok()
213216
.unwrap();
214217
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
215-
assert_eq!("this is mod foo", module.doc_comment_text().unwrap());
218+
assert_eq!(" this is mod foo", module.doc_comments().doc_comment_text().unwrap());
216219
}
217220

218221
#[test]
@@ -226,7 +229,7 @@ fn test_doc_comment_single_line_block_strips_suffix_whitespace() {
226229
.ok()
227230
.unwrap();
228231
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
229-
assert_eq!("this is mod foo ", module.doc_comment_text().unwrap());
232+
assert_eq!(" this is mod foo ", module.doc_comments().doc_comment_text().unwrap());
230233
}
231234

232235
#[test]
@@ -245,8 +248,8 @@ fn test_doc_comment_multi_line_block_strips_suffix() {
245248
.unwrap();
246249
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
247250
assert_eq!(
248-
" this\n is\n mod foo\n ",
249-
module.doc_comment_text().unwrap()
251+
"\n this\n is\n mod foo\n ",
252+
module.doc_comments().doc_comment_text().unwrap()
250253
);
251254
}
252255

@@ -259,8 +262,8 @@ fn test_comments_preserve_trailing_whitespace() {
259262
.unwrap();
260263
let def = file.syntax().descendants().find_map(Struct::cast).unwrap();
261264
assert_eq!(
262-
"Representation of a Realm. \nIn the specification these are called Realm Records.",
263-
def.doc_comment_text().unwrap()
265+
" Representation of a Realm. \n In the specification these are called Realm Records.",
266+
def.doc_comments().doc_comment_text().unwrap()
264267
);
265268
}
266269

@@ -276,7 +279,7 @@ fn test_four_slash_line_comment() {
276279
.ok()
277280
.unwrap();
278281
let module = file.syntax().descendants().find_map(Module::cast).unwrap();
279-
assert_eq!("doc comment", module.doc_comment_text().unwrap());
282+
assert_eq!(" doc comment", module.doc_comments().doc_comment_text().unwrap());
280283
}
281284

282285
#[test]

‎crates/syntax/src/ast/token_ext.rs

+8-11
Original file line numberDiff line numberDiff line change
@@ -33,23 +33,20 @@ impl ast::Comment {
3333
prefix
3434
}
3535

36-
/// Returns the textual content of a doc comment block as a single string.
37-
/// That is, strips leading `///` (+ optional 1 character of whitespace),
38-
/// trailing `*/`, trailing whitespace and then joins the lines.
36+
/// Returns the textual content of a doc comment node as a single string with prefix and suffix
37+
/// removed.
3938
pub fn doc_comment(&self) -> Option<&str> {
4039
let kind = self.kind();
4140
match kind {
4241
CommentKind { shape, doc: Some(_) } => {
4342
let prefix = kind.prefix();
4443
let text = &self.text()[prefix.len()..];
45-
let ws = text.chars().next().filter(|c| c.is_whitespace());
46-
let text = ws.map_or(text, |ws| &text[ws.len_utf8()..]);
47-
match shape {
48-
CommentShape::Block if text.ends_with("*/") => {
49-
Some(&text[..text.len() - "*/".len()])
50-
}
51-
_ => Some(text),
52-
}
44+
let text = if shape == CommentShape::Block {
45+
text.strip_suffix("*/").unwrap_or(text)
46+
} else {
47+
text
48+
};
49+
Some(text)
5350
}
5451
_ => None,
5552
}

0 commit comments

Comments
 (0)