Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Also inspect doc block comments #10783

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion clippy_lints/src/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,23 @@ fn lint_for_missing_headers(
#[expect(clippy::cast_possible_truncation)]
#[must_use]
pub fn strip_doc_comment_decoration(doc: &str, comment_kind: CommentKind, span: Span) -> (String, Vec<(usize, Span)>) {
fn get_line_indentation(line: &Vec<char>) -> u32 {
let mut indent = 0;
let mut smallest_indent = 0;
for c in line {
if *c == ' ' {
indent += 1;
} else {
if smallest_indent > indent {
smallest_indent = indent;
}
indent = 0;
continue;
}
}
indent
}

// one-line comments lose their prefix
if comment_kind == CommentKind::Line {
let mut doc = doc.to_owned();
Expand All @@ -465,15 +482,33 @@ pub fn strip_doc_comment_decoration(doc: &str, comment_kind: CommentKind, span:

let mut sizes = vec![];
let mut contains_initial_stars = false;
let mut least_indented = 10; // (Hoping the document has less indentation than 10)
for line in doc.lines() {
let offset = line.as_ptr() as usize - doc.as_ptr() as usize;
debug_assert_eq!(offset as u32 as usize, offset);
if !line.is_empty() {
// Empty lines don't have indentation
let line_indent = get_line_indentation(&line.chars().collect::<Vec<char>>());
if line_indent < least_indented {
least_indented = line_indent;
};
};
contains_initial_stars |= line.trim_start().starts_with('*');
// +1 adds the newline, +3 skips the opening delimiter
sizes.push((line.len() + 1, span.with_lo(span.lo() + BytePos(3 + offset as u32))));
}
if !contains_initial_stars {
return (doc.to_string(), sizes);
let mut new_doc = Vec::new();
for line in doc.lines() {
if line.len() >= least_indented as usize {

new_doc.push(&line[least_indented as usize..]); // Sometimes users start the comment the same line the doc comment is defined.
// /** Example of this behaviour
// (Some description)
// */
};
}
return (new_doc.join("\n"), sizes);
}
// remove the initial '*'s if any
let mut no_stars = String::with_capacity(doc.len());
Expand Down
44 changes: 43 additions & 1 deletion tests/ui/doc_errors.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![warn(clippy::missing_errors_doc)]
#![warn(clippy::missing_errors_doc, clippy::doc_markdown)]
#![allow(clippy::result_unit_err)]
#![allow(clippy::unnecessary_wraps)]

Expand Down Expand Up @@ -63,6 +63,48 @@ impl Struct1 {
unimplemented!();
}

/**
Before text
```
Indentation block
```
# Errors
Hi
*/
pub fn pub_method_with_block_errors_header() -> Result<(), ()> {
unimplemented!();
}

/**
This is not sufficiently documented.
*/
pub fn pub_method_missing_block_errors_header() -> Result<(), ()> {
unimplemented!();
}

/**
# Errors
This is sufficiently documented.

This function is a test for this lint's capabilities with other
lints, such as `doc_markdown`
std::str;
*/
pub fn pub_method_block_errors_header_doc_markdown() -> Result<(), ()> {
unimplemented!();
}

/**
This is not sufficiently documented.

This function is also a test for this lint's capabilities with other
lints, such as `doc_markdown`
std::str;
*/
pub fn pub_method_missing_block_errors_header_doc_markdown() -> Result<(), ()> {
unimplemented!();
}

/// # Errors
/// A description of the errors goes here.
pub async fn async_pub_method_with_errors_header() -> Result<(), ()> {
Expand Down
16 changes: 14 additions & 2 deletions tests/ui/doc_errors.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,22 @@ LL | pub async fn async_pub_method_missing_errors_header() -> Result<(), ()>
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: docs for function returning `Result` missing `# Errors` section
--> $DIR/doc_errors.rs:85:5
--> $DIR/doc_errors.rs:81:5
|
LL | pub fn pub_method_missing_block_errors_header() -> Result<(), ()> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: docs for function returning `Result` missing `# Errors` section
--> $DIR/doc_errors.rs:104:5
|
LL | pub fn pub_method_missing_block_errors_header_doc_markdown() -> Result<(), ()> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: docs for function returning `Result` missing `# Errors` section
--> $DIR/doc_errors.rs:127:5
|
LL | fn trait_method_missing_errors_header() -> Result<(), ()>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 7 previous errors
error: aborting due to 9 previous errors

15 changes: 15 additions & 0 deletions tests/ui/missing_panics_doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,21 @@ pub fn todo() {
todo!()
}

/**
# Panics
A reason
*/
pub fn panic_documented_in_block() {
panic!();
}

/**
<Cool documentation here>
*/
pub fn panic_undocumented_in_block() {
panic!();
}

/// This is okay because it is private
fn unwrap_private() {
let result = Err("Hi");
Expand Down
38 changes: 25 additions & 13 deletions tests/ui/missing_panics_doc.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -72,76 +72,88 @@ LL | assert_ne!(x, 0);
| ^^^^^^^^^^^^^^^^

error: docs for function which may panic missing `# Panics` section
--> $DIR/missing_panics_doc.rs:151:5
--> $DIR/missing_panics_doc.rs:126:1
|
LL | pub fn panic_undocumented_in_block() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: first possible panic found here
--> $DIR/missing_panics_doc.rs:127:5
|
LL | panic!();
| ^^^^^^^^

error: docs for function which may panic missing `# Panics` section
--> $DIR/missing_panics_doc.rs:166:5
|
LL | pub fn option_unwrap<T>(v: &[T]) -> &T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: first possible panic found here
--> $DIR/missing_panics_doc.rs:153:9
--> $DIR/missing_panics_doc.rs:168:9
|
LL | o.unwrap()
| ^^^^^^^^^^

error: docs for function which may panic missing `# Panics` section
--> $DIR/missing_panics_doc.rs:156:5
--> $DIR/missing_panics_doc.rs:171:5
|
LL | pub fn option_expect<T>(v: &[T]) -> &T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: first possible panic found here
--> $DIR/missing_panics_doc.rs:158:9
--> $DIR/missing_panics_doc.rs:173:9
|
LL | o.expect("passed an empty thing")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: docs for function which may panic missing `# Panics` section
--> $DIR/missing_panics_doc.rs:161:5
--> $DIR/missing_panics_doc.rs:176:5
|
LL | pub fn result_unwrap<T>(v: &[T]) -> &T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: first possible panic found here
--> $DIR/missing_panics_doc.rs:163:9
--> $DIR/missing_panics_doc.rs:178:9
|
LL | res.unwrap()
| ^^^^^^^^^^^^

error: docs for function which may panic missing `# Panics` section
--> $DIR/missing_panics_doc.rs:166:5
--> $DIR/missing_panics_doc.rs:181:5
|
LL | pub fn result_expect<T>(v: &[T]) -> &T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: first possible panic found here
--> $DIR/missing_panics_doc.rs:168:9
--> $DIR/missing_panics_doc.rs:183:9
|
LL | res.expect("passed an empty thing")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: docs for function which may panic missing `# Panics` section
--> $DIR/missing_panics_doc.rs:171:5
--> $DIR/missing_panics_doc.rs:186:5
|
LL | pub fn last_unwrap(v: &[u32]) -> u32 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: first possible panic found here
--> $DIR/missing_panics_doc.rs:172:10
--> $DIR/missing_panics_doc.rs:187:10
|
LL | *v.last().unwrap()
| ^^^^^^^^^^^^^^^^^

error: docs for function which may panic missing `# Panics` section
--> $DIR/missing_panics_doc.rs:175:5
--> $DIR/missing_panics_doc.rs:190:5
|
LL | pub fn last_expect(v: &[u32]) -> u32 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: first possible panic found here
--> $DIR/missing_panics_doc.rs:176:10
--> $DIR/missing_panics_doc.rs:191:10
|
LL | *v.last().expect("passed an empty thing")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 12 previous errors
error: aborting due to 13 previous errors

Loading