Skip to content

Commit 88bb926

Browse files
committed
doc_comments_missing_terminal_punctuation: real md parser
1 parent 1025af5 commit 88bb926

7 files changed

+134
-232
lines changed
Lines changed: 55 additions & 180 deletions
Original file line numberDiff line numberDiff line change
@@ -1,211 +1,86 @@
1-
use rustc_ast::ast::{AttrKind, AttrStyle, Attribute};
21
use rustc_errors::Applicability;
3-
use rustc_lint::EarlyContext;
2+
use rustc_lint::LateContext;
3+
use rustc_resolve::rustdoc::main_body_opts;
44

5-
use super::DOC_COMMENTS_MISSING_TERMINAL_PUNCTUATION;
5+
use pulldown_cmark::{Event, Options, Parser, Tag, TagEnd};
6+
7+
use super::{DOC_COMMENTS_MISSING_TERMINAL_PUNCTUATION, Fragments};
68

79
const MSG: &str = "doc comments should end with a terminal punctuation mark";
810
const PUNCTUATION_SUGGESTION: char = '.';
911

10-
pub fn check(cx: &EarlyContext<'_>, attrs: &[Attribute]) {
11-
let mut doc_comment_attrs = attrs.iter().enumerate().filter(|(_, a)| is_doc_comment(a));
12-
13-
let Some((i, mut last_doc_attr)) = doc_comment_attrs.next_back() else {
14-
return;
15-
};
16-
17-
// Check that the next attribute is not a `#[doc]` attribute.
18-
if let Some(next_attr) = attrs.get(i + 1)
19-
&& is_doc_attr(next_attr)
20-
{
21-
return;
22-
}
23-
24-
// Find the last, non-blank, non-refdef line of multiline doc comments: this is enough to check that
25-
// the doc comment ends with proper punctuation.
26-
while is_doc_comment_trailer(last_doc_attr) {
27-
if let Some(doc_attr) = doc_comment_attrs.next_back() {
28-
(_, last_doc_attr) = doc_attr;
29-
} else {
30-
// The doc comment looks (functionally) empty.
31-
return;
32-
}
33-
}
34-
35-
if let Some(doc_string) = is_missing_punctuation(last_doc_attr) {
36-
let span = last_doc_attr.span;
37-
38-
if is_line_doc_comment(last_doc_attr) {
39-
let suggestion = generate_suggestion(last_doc_attr, doc_string);
40-
12+
pub fn check(cx: &LateContext<'_>, doc: &str, fragments: Fragments<'_>) {
13+
if let Some(offset) = is_missing_punctuation(doc) {
14+
if let Some(span) = fragments.span(cx, offset..offset) {
4115
clippy_utils::diagnostics::span_lint_and_sugg(
4216
cx,
4317
DOC_COMMENTS_MISSING_TERMINAL_PUNCTUATION,
4418
span,
4519
MSG,
4620
"end the doc comment with some punctuation",
47-
suggestion,
21+
PUNCTUATION_SUGGESTION.to_string(),
4822
Applicability::MaybeIncorrect,
4923
);
5024
} else {
51-
// Seems more difficult to preserve the formatting of block doc comments, so we do not provide
25+
let span = fragments.fragments.last().unwrap().span;
26+
// Seems more difficult to preserve the formatting of `#[doc]` attrs, so we do not provide
5227
// suggestions for them; they are much rarer anyway.
5328
clippy_utils::diagnostics::span_lint(cx, DOC_COMMENTS_MISSING_TERMINAL_PUNCTUATION, span, MSG);
5429
}
5530
}
5631
}
5732

5833
#[must_use]
59-
fn is_missing_punctuation(attr: &Attribute) -> Option<&str> {
34+
/// If punctuation is missing, returns the docstring and the offset
35+
/// where new punctuation should be inserted.
36+
fn is_missing_punctuation(doc_string: &str) -> Option<usize> {
6037
const TERMINAL_PUNCTUATION_MARKS: &[char] = &['.', '?', '!', '…'];
61-
const EXCEPTIONS: &[char] = &[
62-
'>', // Raw HTML or (unfortunately) Markdown autolinks.
63-
'|', // Markdown tables.
64-
];
65-
66-
let doc_string = get_doc_string(attr)?;
6738

68-
// Doc comments could have some trailing whitespace, but that is not this lint's job.
69-
let trimmed = doc_string.trim_end();
70-
71-
// Doc comments are also allowed to end with fenced code blocks.
72-
if trimmed.ends_with(TERMINAL_PUNCTUATION_MARKS) || trimmed.ends_with(EXCEPTIONS) || trimmed.ends_with("```") {
73-
return None;
74-
}
75-
76-
// Ignore single-line list items: they may not require any terminal punctuation.
77-
if looks_like_list_item(trimmed) {
78-
return None;
79-
}
80-
81-
if let Some(stripped) = strip_sentence_trailers(trimmed)
82-
&& stripped.ends_with(TERMINAL_PUNCTUATION_MARKS)
39+
let mut no_report_depth = 0;
40+
let mut text_offset = None;
41+
for (event, offset) in
42+
Parser::new_ext(doc_string, main_body_opts() - Options::ENABLE_SMART_PUNCTUATION).into_offset_iter()
8343
{
84-
return None;
85-
}
86-
87-
Some(doc_string)
88-
}
89-
90-
#[must_use]
91-
fn generate_suggestion(doc_attr: &Attribute, doc_string: &str) -> String {
92-
let doc_comment_prefix = match doc_attr.style {
93-
AttrStyle::Outer => "///",
94-
AttrStyle::Inner => "//!",
95-
};
96-
97-
let mut original_line = format!("{doc_comment_prefix}{doc_string}");
98-
99-
if let Some(stripped) = strip_sentence_trailers(doc_string) {
100-
// Insert the punctuation mark just before the sentence trailer.
101-
original_line.insert(doc_comment_prefix.len() + stripped.len(), PUNCTUATION_SUGGESTION);
102-
} else {
103-
original_line.push(PUNCTUATION_SUGGESTION);
104-
}
105-
106-
original_line
107-
}
108-
109-
/// Strips closing parentheses and Markdown emphasis delimiters.
110-
#[must_use]
111-
fn strip_sentence_trailers(string: &str) -> Option<&str> {
112-
// The std has a few occurrences of doc comments ending with a sentence in parentheses.
113-
const TRAILERS: &[char] = &[')', '*', '_'];
114-
115-
if let Some(stripped) = string.strip_suffix("**") {
116-
return Some(stripped);
117-
}
118-
119-
if let Some(stripped) = string.strip_suffix("__") {
120-
return Some(stripped);
121-
}
122-
123-
// Markdown inline links should not be mistaken for sentences in parentheses.
124-
if looks_like_inline_link(string) {
125-
return None;
126-
}
127-
128-
string.strip_suffix(TRAILERS)
129-
}
130-
131-
/// Returns whether the doc comment looks like a Markdown reference definition or a blank line.
132-
#[must_use]
133-
fn is_doc_comment_trailer(attr: &Attribute) -> bool {
134-
let Some(doc_string) = get_doc_string(attr) else {
135-
return false;
136-
};
137-
138-
super::looks_like_refdef(doc_string, 0..doc_string.len()).is_some() || doc_string.trim_end().is_empty()
139-
}
140-
141-
/// Returns whether the string looks like it ends with a Markdown inline link.
142-
#[must_use]
143-
fn looks_like_inline_link(string: &str) -> bool {
144-
let Some(sub) = string.strip_suffix(')') else {
145-
return false;
146-
};
147-
let Some((sub, _)) = sub.rsplit_once('(') else {
148-
return false;
149-
};
150-
151-
// Check whether there is closing bracket just before the opening parenthesis.
152-
sub.ends_with(']')
153-
}
154-
155-
/// Returns whether the string looks like a Markdown list item.
156-
#[must_use]
157-
fn looks_like_list_item(string: &str) -> bool {
158-
const BULLET_LIST_MARKERS: &[char] = &['-', '+', '*'];
159-
const ORDERED_LIST_MARKER_SYMBOL: &[char] = &['.', ')'];
160-
161-
let trimmed = string.trim_start();
162-
163-
if let Some(sub) = trimmed.strip_prefix(BULLET_LIST_MARKERS)
164-
&& sub.starts_with(char::is_whitespace)
165-
{
166-
return true;
167-
}
168-
169-
let mut stripped = trimmed;
170-
while let Some(sub) = stripped.strip_prefix(|c| char::is_digit(c, 10)) {
171-
stripped = sub;
172-
}
173-
if let Some(sub) = stripped.strip_prefix(ORDERED_LIST_MARKER_SYMBOL)
174-
&& sub.starts_with(char::is_whitespace)
175-
{
176-
return true;
44+
match event {
45+
Event::Start(
46+
Tag::CodeBlock(..)
47+
| Tag::FootnoteDefinition(_)
48+
| Tag::Heading { .. }
49+
| Tag::HtmlBlock
50+
| Tag::List(..)
51+
| Tag::Table(_),
52+
) => {
53+
no_report_depth += 1;
54+
},
55+
Event::End(
56+
TagEnd::CodeBlock
57+
| TagEnd::FootnoteDefinition
58+
| TagEnd::Heading(_)
59+
| TagEnd::HtmlBlock
60+
| TagEnd::List(_)
61+
| TagEnd::Table,
62+
) => {
63+
no_report_depth -= 1;
64+
},
65+
Event::InlineHtml(_) | Event::Start(Tag::Image { .. }) | Event::End(TagEnd::Image) => {
66+
text_offset = None;
67+
},
68+
Event::Text(..) | Event::Start(Tag::Link { .. }) | Event::End(TagEnd::Link)
69+
if no_report_depth == 0 && !offset.is_empty() =>
70+
{
71+
text_offset = Some(offset);
72+
},
73+
_ => {},
74+
}
17775
}
17876

179-
false
180-
}
181-
182-
#[must_use]
183-
fn is_doc_attr(attr: &Attribute) -> bool {
184-
if let AttrKind::Normal(normal_attr) = &attr.kind
185-
&& let Some(segment) = &normal_attr.item.path.segments.first()
186-
&& segment.ident.name == clippy_utils::sym::doc
77+
let text_offset = text_offset?;
78+
if doc_string[..text_offset.end]
79+
.trim_end_matches(|c: char| c.is_whitespace() || c == ')' || c == ']' || c == '}')
80+
.ends_with(TERMINAL_PUNCTUATION_MARKS)
18781
{
188-
true
189-
} else {
190-
false
191-
}
192-
}
193-
194-
#[must_use]
195-
fn get_doc_string(attr: &Attribute) -> Option<&str> {
196-
if let AttrKind::DocComment(_, symbol) = &attr.kind {
197-
Some(symbol.as_str())
198-
} else {
19982
None
83+
} else {
84+
Some(text_offset.end)
20085
}
20186
}
202-
203-
#[must_use]
204-
fn is_doc_comment(attr: &Attribute) -> bool {
205-
matches!(attr.kind, AttrKind::DocComment(_, _))
206-
}
207-
208-
#[must_use]
209-
fn is_line_doc_comment(attr: &Attribute) -> bool {
210-
matches!(attr.kind, AttrKind::DocComment(rustc_ast::token::CommentKind::Line, _))
211-
}

clippy_lints/src/doc/mod.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,6 @@ impl_lint_pass!(Documentation => [
731731
impl EarlyLintPass for Documentation {
732732
fn check_attributes(&mut self, cx: &EarlyContext<'_>, attrs: &[rustc_ast::Attribute]) {
733733
include_in_doc_without_cfg::check(cx, attrs);
734-
doc_comments_missing_terminal_punctuation::check(cx, attrs);
735734
}
736735
}
737736

@@ -898,6 +897,15 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
898897
},
899898
);
900899

900+
doc_comments_missing_terminal_punctuation::check(
901+
cx,
902+
&doc,
903+
Fragments {
904+
doc: &doc,
905+
fragments: &fragments,
906+
},
907+
);
908+
901909
// NOTE: check_doc uses it own cb function,
902910
// to avoid causing duplicated diagnostics for the broken link checker.
903911
let mut full_fake_broken_link_callback = |bl: BrokenLink<'_>| -> Option<(CowStr<'_>, CowStr<'_>)> {

tests/ui/doc/doc_comments_missing_terminal_punctuation.fixed

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,9 @@ enum Exceptions {
3636
/// HTML content is however not checked:
3737
/// <em>Raw HTML is allowed as well</em>
3838
RawHtml,
39-
/// The raw HTML exception also unfortunately results in ignoring autolinks too:
40-
/// <https://spec.commonmark.org/0.31.2/#autolinks>
39+
/// The raw HTML exception actually does the right thing to autolinks:
40+
/// <https://spec.commonmark.org/0.31.2/#autolinks>.
41+
//~^ doc_comments_missing_terminal_punctuation
4142
MarkdownAutolink,
4243
/// | Exception | Note |
4344
/// | -------------- | ----- |
@@ -77,7 +78,7 @@ mod module {
7778
enum Trailers {
7879
/// (Sometimes the last sentence is in parentheses, and that's ok.)
7980
ParensPassing,
80-
/// (But sometimes it is missing a period.)
81+
/// (But sometimes it is missing a period).
8182
//~^ doc_comments_missing_terminal_punctuation
8283
ParensFailing,
8384
/// **Sometimes the last sentence is in bold, and that's ok.**
@@ -131,12 +132,16 @@ struct TrailingBlankLine;
131132
//~^ doc_comments_missing_terminal_punctuation
132133
struct OnlyLastSentence;
133134

134-
/// Sometimes a doc attribute is used for concatenation:
135-
/// ```
136-
#[doc = ""]
137135
/// ```
138-
struct DocAttribute;
136+
struct IncompleteBlockCode;
139137

140138
#[expect(clippy::empty_docs)]
141139
///
142140
struct EmptyDocComment;
141+
142+
/**
143+
* Block doc comments work.
144+
*
145+
*/
146+
//~^^^ doc_comments_missing_terminal_punctuation
147+
struct BlockDocComment;

tests/ui/doc/doc_comments_missing_terminal_punctuation.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,9 @@ enum Exceptions {
3636
/// HTML content is however not checked:
3737
/// <em>Raw HTML is allowed as well</em>
3838
RawHtml,
39-
/// The raw HTML exception also unfortunately results in ignoring autolinks too:
39+
/// The raw HTML exception actually does the right thing to autolinks:
4040
/// <https://spec.commonmark.org/0.31.2/#autolinks>
41+
//~^ doc_comments_missing_terminal_punctuation
4142
MarkdownAutolink,
4243
/// | Exception | Note |
4344
/// | -------------- | ----- |
@@ -131,12 +132,16 @@ struct TrailingBlankLine;
131132
//~^ doc_comments_missing_terminal_punctuation
132133
struct OnlyLastSentence;
133134

134-
/// Sometimes a doc attribute is used for concatenation:
135-
/// ```
136-
#[doc = ""]
137135
/// ```
138-
struct DocAttribute;
136+
struct IncompleteBlockCode;
139137

140138
#[expect(clippy::empty_docs)]
141139
///
142140
struct EmptyDocComment;
141+
142+
/**
143+
* Block doc comments work
144+
*
145+
*/
146+
//~^^^ doc_comments_missing_terminal_punctuation
147+
struct BlockDocComment;

0 commit comments

Comments
 (0)