Skip to content

Commit 86e036a

Browse files
authored
Add a doc_paragraphs_missing_punctuation lint (#15758)
This introduces a new lint that aims to check for missing terminal punctuation in doc comments. ## Lint scope and motivation It partially addresses #8371 by currently focusing on the case that is both the easiest to look for without advanced NLP capacities and still very useful: missing punctuation at the end of the last sentence of doc comments. This is particularly useful when working in a team to enforce a style guide and make sure all doc comments end with some kind of punctuation, which is often forgotten in the case of short, single-sentence doc comments. The lint is biased towards avoiding false positives so it can be easily adopted without requiring an excessive number of `#[expect]` attributes. It is currently only concerned with Latin languages, but adding support for `。` for instance should be very easy. Even if consistently ending doc comments (even very short ones) with punctuation is somewhat of an opinion, it seems sufficiently well-established, [at least in the `std`](rust-lang/rust#91886), to warrant its own lint, even if of course it would be allow-by-default. ## Lint category and possible evolution Because it is unclear how useful and more complex it would be to also look for missing punctuation at the end of *each* Markdown paragraphs, I opted for the `nursery` category for now. My understanding of [the stability guarantees](https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#stability-guarantees) is that would it not be acceptable to add such capability later if it was part of `pedantic` or `restriction` categories right away. I tried to make sure the lint name allows this kind of iterative evolution. ## Testing I ran the lint against the `core` library which led me to introduce a few special cases (which seem common enough) to avoid false positives: e.g., the last sentence being in parentheses. After excluding specific modules which should likely be excluded (e.g., `core_arch`, `intrinsincs`), it currently finds a bit more than 200 errors in the `core` library. This lint also helped find issues with a few doc comments, which [are now fixed](rust-lang/rust#146136). ## How to review this PR I suppose it is easier to have a look at the UI tests first before checking the implementation. --- *Please write a short comment explaining your change (or "none" for internal only changes)* changelog: add new lint [`doc_paragraphs_missing_punctuation`]
2 parents c8e5fff + aed57a9 commit 86e036a

10 files changed

+657
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6314,6 +6314,7 @@ Released 2018-09-13
63146314
[`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
63156315
[`doc_nested_refdefs`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs
63166316
[`doc_overindented_list_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_overindented_list_items
6317+
[`doc_paragraphs_missing_punctuation`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_paragraphs_missing_punctuation
63176318
[`doc_suspicious_footnotes`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_suspicious_footnotes
63186319
[`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons
63196320
[`double_ended_iterator_last`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_ended_iterator_last

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
118118
crate::doc::DOC_MARKDOWN_INFO,
119119
crate::doc::DOC_NESTED_REFDEFS_INFO,
120120
crate::doc::DOC_OVERINDENTED_LIST_ITEMS_INFO,
121+
crate::doc::DOC_PARAGRAPHS_MISSING_PUNCTUATION_INFO,
121122
crate::doc::DOC_SUSPICIOUS_FOOTNOTES_INFO,
122123
crate::doc::EMPTY_DOCS_INFO,
123124
crate::doc::MISSING_ERRORS_DOC_INFO,
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
use rustc_errors::Applicability;
2+
use rustc_lint::LateContext;
3+
use rustc_resolve::rustdoc::main_body_opts;
4+
5+
use rustc_resolve::rustdoc::pulldown_cmark::{Event, Options, Parser, Tag, TagEnd};
6+
7+
use super::{DOC_PARAGRAPHS_MISSING_PUNCTUATION, Fragments};
8+
9+
const MSG: &str = "doc paragraphs should end with a terminal punctuation mark";
10+
const PUNCTUATION_SUGGESTION: char = '.';
11+
12+
pub fn check(cx: &LateContext<'_>, doc: &str, fragments: Fragments<'_>) {
13+
for missing_punctuation in is_missing_punctuation(doc) {
14+
match missing_punctuation {
15+
MissingPunctuation::Fixable(offset) => {
16+
// This ignores `#[doc]` attributes, which we do not handle.
17+
if let Some(span) = fragments.span(cx, offset..offset) {
18+
clippy_utils::diagnostics::span_lint_and_sugg(
19+
cx,
20+
DOC_PARAGRAPHS_MISSING_PUNCTUATION,
21+
span,
22+
MSG,
23+
"end the paragraph with some punctuation",
24+
PUNCTUATION_SUGGESTION.to_string(),
25+
Applicability::MaybeIncorrect,
26+
);
27+
}
28+
},
29+
MissingPunctuation::Unfixable(offset) => {
30+
// This ignores `#[doc]` attributes, which we do not handle.
31+
if let Some(span) = fragments.span(cx, offset..offset) {
32+
clippy_utils::diagnostics::span_lint_and_help(
33+
cx,
34+
DOC_PARAGRAPHS_MISSING_PUNCTUATION,
35+
span,
36+
MSG,
37+
None,
38+
"end the paragraph with some punctuation",
39+
);
40+
}
41+
},
42+
}
43+
}
44+
}
45+
46+
#[must_use]
47+
/// If punctuation is missing, returns the offset where new punctuation should be inserted.
48+
fn is_missing_punctuation(doc_string: &str) -> Vec<MissingPunctuation> {
49+
// The colon is not exactly a terminal punctuation mark, but this is required for paragraphs that
50+
// introduce a table or a list for example.
51+
const TERMINAL_PUNCTUATION_MARKS: &[char] = &['.', '?', '!', '…', ':'];
52+
53+
let mut no_report_depth = 0;
54+
let mut missing_punctuation = Vec::new();
55+
let mut current_paragraph = None;
56+
57+
for (event, offset) in
58+
Parser::new_ext(doc_string, main_body_opts() - Options::ENABLE_SMART_PUNCTUATION).into_offset_iter()
59+
{
60+
match event {
61+
Event::Start(
62+
Tag::CodeBlock(..)
63+
| Tag::FootnoteDefinition(_)
64+
| Tag::Heading { .. }
65+
| Tag::HtmlBlock
66+
| Tag::List(..)
67+
| Tag::Table(_),
68+
) => {
69+
no_report_depth += 1;
70+
},
71+
Event::End(TagEnd::FootnoteDefinition) => {
72+
no_report_depth -= 1;
73+
},
74+
Event::End(
75+
TagEnd::CodeBlock | TagEnd::Heading(_) | TagEnd::HtmlBlock | TagEnd::List(_) | TagEnd::Table,
76+
) => {
77+
no_report_depth -= 1;
78+
current_paragraph = None;
79+
},
80+
Event::InlineHtml(_) | Event::Start(Tag::Image { .. }) | Event::End(TagEnd::Image) => {
81+
current_paragraph = None;
82+
},
83+
Event::End(TagEnd::Paragraph) => {
84+
if let Some(mp) = current_paragraph {
85+
missing_punctuation.push(mp);
86+
}
87+
},
88+
Event::Code(..) | Event::Start(Tag::Link { .. }) | Event::End(TagEnd::Link)
89+
if no_report_depth == 0 && !offset.is_empty() =>
90+
{
91+
if doc_string[..offset.end]
92+
.trim_end()
93+
.ends_with(TERMINAL_PUNCTUATION_MARKS)
94+
{
95+
current_paragraph = None;
96+
} else {
97+
current_paragraph = Some(MissingPunctuation::Fixable(offset.end));
98+
}
99+
},
100+
Event::Text(..) if no_report_depth == 0 && !offset.is_empty() => {
101+
let trimmed = doc_string[..offset.end].trim_end();
102+
if trimmed.ends_with(TERMINAL_PUNCTUATION_MARKS) {
103+
current_paragraph = None;
104+
} else if let Some(t) = trimmed.strip_suffix(|c| c == ')' || c == '"') {
105+
if t.ends_with(TERMINAL_PUNCTUATION_MARKS) {
106+
// Avoid false positives.
107+
current_paragraph = None;
108+
} else {
109+
current_paragraph = Some(MissingPunctuation::Unfixable(offset.end));
110+
}
111+
} else {
112+
current_paragraph = Some(MissingPunctuation::Fixable(offset.end));
113+
}
114+
},
115+
_ => {},
116+
}
117+
}
118+
119+
missing_punctuation
120+
}
121+
122+
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
123+
enum MissingPunctuation {
124+
Fixable(usize),
125+
Unfixable(usize),
126+
}

clippy_lints/src/doc/mod.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use url::Url;
2727

2828
mod broken_link;
2929
mod doc_comment_double_space_linebreaks;
30+
mod doc_paragraphs_missing_punctuation;
3031
mod doc_suspicious_footnotes;
3132
mod include_in_doc_without_cfg;
3233
mod lazy_continuation;
@@ -670,6 +671,33 @@ declare_clippy_lint! {
670671
"looks like a link or footnote ref, but with no definition"
671672
}
672673

674+
declare_clippy_lint! {
675+
/// ### What it does
676+
/// Checks for doc comments whose paragraphs do not end with a period or another punctuation mark.
677+
/// Various Markdowns constructs are taken into account to avoid false positives.
678+
///
679+
/// ### Why is this bad?
680+
/// A project may wish to enforce consistent doc comments by making sure paragraphs end with a
681+
/// punctuation mark.
682+
///
683+
/// ### Example
684+
/// ```no_run
685+
/// /// Returns a random number
686+
/// ///
687+
/// /// It was chosen by a fair dice roll
688+
/// ```
689+
/// Use instead:
690+
/// ```no_run
691+
/// /// Returns a random number.
692+
/// ///
693+
/// /// It was chosen by a fair dice roll.
694+
/// ```
695+
#[clippy::version = "1.93.0"]
696+
pub DOC_PARAGRAPHS_MISSING_PUNCTUATION,
697+
restriction,
698+
"missing terminal punctuation in doc comments"
699+
}
700+
673701
pub struct Documentation {
674702
valid_idents: FxHashSet<String>,
675703
check_private_items: bool,
@@ -704,6 +732,7 @@ impl_lint_pass!(Documentation => [
704732
DOC_INCLUDE_WITHOUT_CFG,
705733
DOC_COMMENT_DOUBLE_SPACE_LINEBREAKS,
706734
DOC_SUSPICIOUS_FOOTNOTES,
735+
DOC_PARAGRAPHS_MISSING_PUNCTUATION,
707736
]);
708737

709738
impl EarlyLintPass for Documentation {
@@ -875,6 +904,15 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
875904
},
876905
);
877906

907+
doc_paragraphs_missing_punctuation::check(
908+
cx,
909+
&doc,
910+
Fragments {
911+
doc: &doc,
912+
fragments: &fragments,
913+
},
914+
);
915+
878916
// NOTE: check_doc uses it own cb function,
879917
// to avoid causing duplicated diagnostics for the broken link checker.
880918
let mut full_fake_broken_link_callback = |bl: BrokenLink<'_>| -> Option<(CowStr<'_>, CowStr<'_>)> {

tests/ui/blanket_clippy_restriction_lints.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
#![warn(clippy::blanket_clippy_restriction_lints)]
55

6-
//! Test that the whole restriction group is not enabled
6+
//! Test that the whole restriction group is not enabled.
77
#![warn(clippy::restriction)]
88
//~^ blanket_clippy_restriction_lints
99
#![deny(clippy::restriction)]
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
#![feature(custom_inner_attributes)]
2+
#![rustfmt::skip]
3+
#![warn(clippy::doc_paragraphs_missing_punctuation)]
4+
5+
/// Returns the Answer to the Ultimate Question of Life, the Universe, and Everything.
6+
//~^ doc_paragraphs_missing_punctuation
7+
fn answer() -> i32 {
8+
42
9+
}
10+
11+
/// The `Option` type.
12+
//~^ doc_paragraphs_missing_punctuation
13+
// Triggers even in the presence of another attribute.
14+
#[derive(Debug)]
15+
enum MyOption<T> {
16+
/// No value.
17+
//~^ doc_paragraphs_missing_punctuation
18+
None,
19+
/// Some value of type `T`.
20+
Some(T),
21+
}
22+
23+
// Triggers correctly even when interleaved with other attributes.
24+
/// A multiline
25+
#[derive(Debug)]
26+
/// doc comment:
27+
/// only the last line triggers the lint.
28+
//~^ doc_paragraphs_missing_punctuation
29+
enum Exceptions {
30+
/// Question marks are fine?
31+
QuestionMark,
32+
/// Exclamation marks are fine!
33+
ExclamationMark,
34+
/// Ellipses are ok too…
35+
Ellipsis,
36+
/// HTML content is however not checked:
37+
/// <em>Raw HTML is allowed as well</em>
38+
RawHtml,
39+
/// The raw HTML exception actually does the right thing to autolinks:
40+
/// <https://spec.commonmark.org/0.31.2/#autolinks>.
41+
//~^ doc_paragraphs_missing_punctuation
42+
MarkdownAutolink,
43+
/// This table introduction ends with a colon:
44+
///
45+
/// | Exception | Note |
46+
/// | -------------- | ----- |
47+
/// | Markdown table | A-ok |
48+
MarkdownTable,
49+
/// Here is a snippet.
50+
//~^ doc_paragraphs_missing_punctuation
51+
///
52+
/// ```
53+
/// // Code blocks are no issues.
54+
/// ```
55+
CodeBlock,
56+
}
57+
58+
// Check the lint can be expected on a whole enum at once.
59+
#[expect(clippy::doc_paragraphs_missing_punctuation)]
60+
enum Char {
61+
/// U+0000
62+
Null,
63+
/// U+0001
64+
StartOfHeading,
65+
}
66+
67+
// Check the lint can be expected on a single variant without affecting others.
68+
enum Char2 {
69+
#[expect(clippy::doc_paragraphs_missing_punctuation)]
70+
/// U+0000
71+
Null,
72+
/// U+0001.
73+
//~^ doc_paragraphs_missing_punctuation
74+
StartOfHeading,
75+
}
76+
77+
mod module {
78+
//! Works on
79+
//! inner attributes too.
80+
//~^ doc_paragraphs_missing_punctuation
81+
}
82+
83+
enum Trailers {
84+
/// Sometimes the last sentence ends with parentheses (and that's ok).
85+
ParensPassing,
86+
/// (Sometimes the last sentence is in parentheses.)
87+
SentenceInParensPassing,
88+
/// **Sometimes the last sentence is in bold, and that's ok.**
89+
DoubleStarPassing,
90+
/// **But sometimes it is missing a period.**
91+
//~^ doc_paragraphs_missing_punctuation
92+
DoubleStarFailing,
93+
/// _Sometimes the last sentence is in italics, and that's ok._
94+
UnderscorePassing,
95+
/// _But sometimes it is missing a period._
96+
//~^ doc_paragraphs_missing_punctuation
97+
UnderscoreFailing,
98+
/// This comment ends with "a quote."
99+
AmericanStyleQuotePassing,
100+
/// This comment ends with "a quote".
101+
BritishStyleQuotePassing,
102+
}
103+
104+
/// Doc comments can end with an [inline link](#anchor).
105+
//~^ doc_paragraphs_missing_punctuation
106+
struct InlineLink;
107+
108+
/// Some doc comments contain [link reference definitions][spec].
109+
//~^ doc_paragraphs_missing_punctuation
110+
///
111+
/// [spec]: https://spec.commonmark.org/0.31.2/#link-reference-definitions
112+
struct LinkRefDefinition;
113+
114+
// List items do not always need to end with a period.
115+
enum UnorderedLists {
116+
/// This list has an introductory sentence:
117+
///
118+
/// - A list item
119+
Dash,
120+
/// + A list item
121+
Plus,
122+
/// * A list item
123+
Star,
124+
}
125+
126+
enum OrderedLists {
127+
/// 1. A list item
128+
Dot,
129+
/// 42) A list item
130+
Paren,
131+
}
132+
133+
/// Doc comments with trailing blank lines are supported.
134+
//~^ doc_paragraphs_missing_punctuation
135+
///
136+
struct TrailingBlankLine;
137+
138+
/// This doc comment has multiple paragraph.
139+
/// This first paragraph is missing punctuation.
140+
//~^ doc_paragraphs_missing_punctuation
141+
///
142+
/// The second one as well
143+
/// And it has multiple sentences.
144+
//~^ doc_paragraphs_missing_punctuation
145+
///
146+
/// Same for this third and last one.
147+
//~^ doc_paragraphs_missing_punctuation
148+
struct MultiParagraphDocComment;
149+
150+
/// ```
151+
struct IncompleteBlockCode;
152+
153+
/// This ends with a code `span`.
154+
//~^ doc_paragraphs_missing_punctuation
155+
struct CodeSpan;
156+
157+
#[expect(clippy::empty_docs)]
158+
///
159+
struct EmptyDocComment;
160+
161+
/**
162+
* Block doc comments work.
163+
*
164+
*/
165+
//~^^^ doc_paragraphs_missing_punctuation
166+
struct BlockDocComment;
167+
168+
/// Sometimes a doc attribute is used for concatenation
169+
/// ```
170+
#[doc = ""]
171+
/// ```
172+
struct DocAttribute;

0 commit comments

Comments
 (0)