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

Add new lint doc_overindented_list_items #13711

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5533,6 +5533,7 @@ Released 2018-09-13
[`doc_link_with_quotes`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_link_with_quotes
[`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
[`doc_nested_refdefs`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs
[`doc_overindented_list_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_overindented_list_items
[`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons
[`double_ended_iterator_last`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_ended_iterator_last
[`double_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_must_use
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,11 @@ line. (You can swap `clippy::all` with the specific lint category you are target
You can add options to your code to `allow`/`warn`/`deny` Clippy lints:

* the whole set of `Warn` lints using the `clippy` lint group (`#![deny(clippy::all)]`).
Note that `rustc` has additional [lint groups](https://doc.rust-lang.org/rustc/lints/groups.html).
Note that `rustc` has additional [lint groups](https://doc.rust-lang.org/rustc/lints/groups.html).

* all lints using both the `clippy` and `clippy::pedantic` lint groups (`#![deny(clippy::all)]`,
`#![deny(clippy::pedantic)]`). Note that `clippy::pedantic` contains some very aggressive
lints prone to false positives.
`#![deny(clippy::pedantic)]`). Note that `clippy::pedantic` contains some very aggressive
lints prone to false positives.

* only some lints (`#![deny(clippy::single_match, clippy::box_vec)]`, etc.)

Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::doc::DOC_LINK_WITH_QUOTES_INFO,
crate::doc::DOC_MARKDOWN_INFO,
crate::doc::DOC_NESTED_REFDEFS_INFO,
crate::doc::DOC_OVERINDENTED_LIST_ITEMS_INFO,
crate::doc::EMPTY_DOCS_INFO,
crate::doc::EMPTY_LINE_AFTER_DOC_COMMENTS_INFO,
crate::doc::EMPTY_LINE_AFTER_OUTER_ATTR_INFO,
Expand Down
112 changes: 73 additions & 39 deletions clippy_lints/src/doc/lazy_continuation.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh this could also be renamed but tbh I am not sure to what.. It's fine if it's kept as is

Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ use itertools::Itertools;
use rustc_errors::Applicability;
use rustc_lint::LateContext;
use rustc_span::{BytePos, Span};
use std::cmp::Ordering;
use std::ops::Range;

use super::DOC_LAZY_CONTINUATION;
use super::{DOC_LAZY_CONTINUATION, DOC_OVERINDENTED_LIST_ITEMS};

fn map_container_to_text(c: &super::Container) -> &'static str {
match c {
Expand All @@ -28,12 +29,57 @@ pub(super) fn check(
return;
}

// Blockquote
let ccount = doc[range.clone()].chars().filter(|c| *c == '>').count();
let blockquote_level = containers
.iter()
.filter(|c| matches!(c, super::Container::Blockquote))
.count();
let lcount = doc[range.clone()].chars().filter(|c| *c == ' ').count();
if ccount < blockquote_level {
span_lint_and_then(
cx,
DOC_LAZY_CONTINUATION,
span,
"doc quote line without `>` marker",
|diag| {
let mut doc_start_range = &doc[range];
let mut suggested = String::new();
for c in containers {
let text = map_container_to_text(c);
if doc_start_range.starts_with(text) {
doc_start_range = &doc_start_range[text.len()..];
span = span.with_lo(
span.lo() + BytePos(u32::try_from(text.len()).expect("text is not 2**32 or bigger")),
);
} else if matches!(c, super::Container::Blockquote)
&& let Some(i) = doc_start_range.find('>')
{
doc_start_range = &doc_start_range[i + 1..];
span = span
.with_lo(span.lo() + BytePos(u32::try_from(i).expect("text is not 2**32 or bigger") + 1));
} else {
suggested.push_str(text);
}
}
diag.span_suggestion_verbose(
span,
"add markers to start of line",
suggested,
Applicability::MachineApplicable,
);
diag.help("if this not intended to be a quote at all, escape it with `\\>`");
},
);
return;
}

if ccount != 0 && blockquote_level != 0 {
// If this doc is a blockquote, we don't go further.
return;
}

// List
let leading_spaces = doc[range].chars().filter(|c| *c == ' ').count();
let list_indentation = containers
.iter()
.map(|c| {
Expand All @@ -44,50 +90,38 @@ pub(super) fn check(
}
})
.sum();
if ccount < blockquote_level || lcount < list_indentation {
let msg = if ccount < blockquote_level {
"doc quote line without `>` marker"
} else {
"doc list item without indentation"
};
span_lint_and_then(cx, DOC_LAZY_CONTINUATION, span, msg, |diag| {
if ccount == 0 && blockquote_level == 0 {
match leading_spaces.cmp(&list_indentation) {
Ordering::Less => span_lint_and_then(
cx,
DOC_LAZY_CONTINUATION,
span,
"doc list item without indentation",
|diag| {
// simpler suggestion style for indentation
let indent = list_indentation - lcount;
let indent = list_indentation - leading_spaces;
diag.span_suggestion_verbose(
span.shrink_to_hi(),
"indent this line",
std::iter::repeat(" ").take(indent).join(""),
Applicability::MaybeIncorrect,
);
diag.help("if this is supposed to be its own paragraph, add a blank line");
return;
}
let mut doc_start_range = &doc[range];
let mut suggested = String::new();
for c in containers {
let text = map_container_to_text(c);
if doc_start_range.starts_with(text) {
doc_start_range = &doc_start_range[text.len()..];
span = span
.with_lo(span.lo() + BytePos(u32::try_from(text.len()).expect("text is not 2**32 or bigger")));
} else if matches!(c, super::Container::Blockquote)
&& let Some(i) = doc_start_range.find('>')
{
doc_start_range = &doc_start_range[i + 1..];
span =
span.with_lo(span.lo() + BytePos(u32::try_from(i).expect("text is not 2**32 or bigger") + 1));
} else {
suggested.push_str(text);
}
}
diag.span_suggestion_verbose(
span,
"add markers to start of line",
suggested,
Applicability::MachineApplicable,
);
diag.help("if this not intended to be a quote at all, escape it with `\\>`");
});
},
),
Ordering::Greater => span_lint_and_then(
cx,
DOC_OVERINDENTED_LIST_ITEMS,
span,
"doc list item overindented",
|diag| {
diag.span_suggestion_verbose(
span,
"remove unnecessary spaces",
std::iter::repeat(" ").take(list_indentation).join(""),
Applicability::MaybeIncorrect,
);
},
),
Ordering::Equal => {},
}
}
34 changes: 34 additions & 0 deletions clippy_lints/src/doc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,39 @@ declare_clippy_lint! {
"require every line of a paragraph to be indented and marked"
}

declare_clippy_lint! {
/// ### What it does
///
/// Detects overindented list items in doc comments where the continuation
/// lines are indented more than necessary.
///
/// ### Why is this bad?
///
/// Overindented list items in doc comments can lead to inconsistent and
/// poorly formatted documentation when rendered. Excessive indentation may
/// cause the text to be misinterpreted as a nested list item or code block,
/// affecting readability and the overall structure of the documentation.
///
/// ### Example
///
/// ```no_run
/// /// - This is the first item in a list
/// /// and this line is overindented.
/// # fn foo() {}
/// ```
///
/// Fixes this into:
/// ```no_run
/// /// - This is the first item in a list
/// /// and this line is overindented.
/// # fn foo() {}
/// ```
#[clippy::version = "1.80.0"]
pub DOC_OVERINDENTED_LIST_ITEMS,
style,
"ensure list items are not overindented"
}

declare_clippy_lint! {
/// ### What it does
/// Checks if the first paragraph in the documentation of items listed in the module page is too long.
Expand Down Expand Up @@ -617,6 +650,7 @@ impl_lint_pass!(Documentation => [
SUSPICIOUS_DOC_COMMENTS,
EMPTY_DOCS,
DOC_LAZY_CONTINUATION,
DOC_OVERINDENTED_LIST_ITEMS,
EMPTY_LINE_AFTER_OUTER_ATTR,
EMPTY_LINE_AFTER_DOC_COMMENTS,
TOO_LONG_FIRST_DOC_PARAGRAPH,
Expand Down
1 change: 1 addition & 0 deletions tests/ui/doc/doc_lazy_list.fixed
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![warn(clippy::doc_lazy_continuation)]
#![allow(clippy::doc_overindented_list_items)]

/// 1. nest here
/// lazy continuation
Expand Down
1 change: 1 addition & 0 deletions tests/ui/doc/doc_lazy_list.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![warn(clippy::doc_lazy_continuation)]
#![allow(clippy::doc_overindented_list_items)]

/// 1. nest here
/// lazy continuation
Expand Down
22 changes: 11 additions & 11 deletions tests/ui/doc/doc_lazy_list.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: doc list item without indentation
--> tests/ui/doc/doc_lazy_list.rs:4:5
--> tests/ui/doc/doc_lazy_list.rs:5:5
|
LL | /// lazy continuation
| ^
Expand All @@ -13,7 +13,7 @@ LL | /// lazy continuation
| +++

error: doc list item without indentation
--> tests/ui/doc/doc_lazy_list.rs:9:5
--> tests/ui/doc/doc_lazy_list.rs:10:5
|
LL | /// lazy list continuations don't make warnings with this lint
| ^
Expand All @@ -25,7 +25,7 @@ LL | /// lazy list continuations don't make warnings with this lint
| +++

error: doc list item without indentation
--> tests/ui/doc/doc_lazy_list.rs:11:5
--> tests/ui/doc/doc_lazy_list.rs:12:5
|
LL | /// because they don't have the
| ^
Expand All @@ -37,7 +37,7 @@ LL | /// because they don't have the
| +++

error: doc list item without indentation
--> tests/ui/doc/doc_lazy_list.rs:16:5
--> tests/ui/doc/doc_lazy_list.rs:17:5
|
LL | /// lazy continuation
| ^
Expand All @@ -49,7 +49,7 @@ LL | /// lazy continuation
| ++++

error: doc list item without indentation
--> tests/ui/doc/doc_lazy_list.rs:21:5
--> tests/ui/doc/doc_lazy_list.rs:22:5
|
LL | /// lazy list continuations don't make warnings with this lint
| ^
Expand All @@ -61,7 +61,7 @@ LL | /// lazy list continuations don't make warnings with this lint
| ++++

error: doc list item without indentation
--> tests/ui/doc/doc_lazy_list.rs:23:5
--> tests/ui/doc/doc_lazy_list.rs:24:5
|
LL | /// because they don't have the
| ^
Expand All @@ -73,7 +73,7 @@ LL | /// because they don't have the
| ++++

error: doc list item without indentation
--> tests/ui/doc/doc_lazy_list.rs:28:5
--> tests/ui/doc/doc_lazy_list.rs:29:5
|
LL | /// lazy continuation
| ^
Expand All @@ -85,7 +85,7 @@ LL | /// lazy continuation
| ++++

error: doc list item without indentation
--> tests/ui/doc/doc_lazy_list.rs:33:5
--> tests/ui/doc/doc_lazy_list.rs:34:5
|
LL | /// this will warn on the lazy continuation
| ^
Expand All @@ -97,7 +97,7 @@ LL | /// this will warn on the lazy continuation
| ++++++

error: doc list item without indentation
--> tests/ui/doc/doc_lazy_list.rs:35:5
--> tests/ui/doc/doc_lazy_list.rs:36:5
|
LL | /// and so should this
| ^^^^
Expand All @@ -109,7 +109,7 @@ LL | /// and so should this
| ++

error: doc list item without indentation
--> tests/ui/doc/doc_lazy_list.rs:56:5
--> tests/ui/doc/doc_lazy_list.rs:57:5
|
LL | /// 'protocol_descriptors': [
| ^
Expand All @@ -121,7 +121,7 @@ LL | /// 'protocol_descriptors': [
| +

error: doc list item without indentation
--> tests/ui/doc/doc_lazy_list.rs:75:5
--> tests/ui/doc/doc_lazy_list.rs:76:5
|
LL | /// ]
| ^
Expand Down
28 changes: 28 additions & 0 deletions tests/ui/doc/doc_overindented_list_items.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#![warn(clippy::doc_overindented_list_items)]

#[rustfmt::skip]
/// - first list item
/// overindented line
//~^ ERROR: doc list item overindented
/// this is overindented line too
//~^ ERROR: doc list item overindented
/// - second list item
fn foo() {}

#[rustfmt::skip]
/// - first list item
/// overindented line
//~^ ERROR: doc list item overindented
/// this is overindented line too
//~^ ERROR: doc list item overindented
/// - second list item
fn bar() {}

#[rustfmt::skip]
/// * first list item
/// overindented line
//~^ ERROR: doc list item overindented
/// this is overindented line too
//~^ ERROR: doc list item overindented
/// * second list item
fn baz() {}
28 changes: 28 additions & 0 deletions tests/ui/doc/doc_overindented_list_items.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#![warn(clippy::doc_overindented_list_items)]

#[rustfmt::skip]
/// - first list item
/// overindented line
//~^ ERROR: doc list item overindented
/// this is overindented line too
//~^ ERROR: doc list item overindented
/// - second list item
fn foo() {}

#[rustfmt::skip]
/// - first list item
/// overindented line
//~^ ERROR: doc list item overindented
/// this is overindented line too
//~^ ERROR: doc list item overindented
/// - second list item
fn bar() {}

#[rustfmt::skip]
/// * first list item
/// overindented line
//~^ ERROR: doc list item overindented
/// this is overindented line too
//~^ ERROR: doc list item overindented
/// * second list item
fn baz() {}
Loading
Loading