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

Overindented lines lint (similar to doc_lazy_continuation) #13601

Open
ojeda opened this issue Oct 24, 2024 · 4 comments · May be fixed by #13711
Open

Overindented lines lint (similar to doc_lazy_continuation) #13601

ojeda opened this issue Oct 24, 2024 · 4 comments · May be fixed by #13711
Labels
A-lint Area: New lints

Comments

@ojeda
Copy link
Contributor

ojeda commented Oct 24, 2024

What it does

The lint reports overindented lines, for instance in bullet lists (and potentially elsewhere?).

This could perhaps be considered a false negative of doc_lazy_continuation, but "lazy continuation lines" only cover removing indentation, not adding it.

Advantage

The advantages would be similar to doc_lazy_continuation:

  • Clearer (easier to read) documentation in plain text form, i.e. unrendered, which is important in some projects.

  • More consistency in documentation comments across a project (and across different projects, for those that enable the lint).

Drawbacks

No response

Example

/// - a
///    b
pub fn f() {}

Could be written as:

/// - a
///   b
pub fn f() {}
@ohno418
Copy link

ohno418 commented Nov 4, 2024

Just a quick report for future work.

I was working on this for a bit, and found some difficulties in automatically applying this rule.

For example, doc like

/// - First line. Here's an sample object:
///   {
///      'protocol': 25,
///      'data': 0xffff
///   }
/// - Second line.

would be fixed into:

/// - First line. Here's an sample object:
///   {
///   'protocol': 25,
///   'data': 0xffff
///   }
/// - Second line.

Similar UI test exists here:

/// * `protocol_descriptors`: A Json Representation of the ProtocolDescriptors
/// to set up. Example:
/// 'protocol_descriptors': [
//~^ ERROR: doc list item without indentation
/// {
/// 'protocol': 25, # u64 Representation of ProtocolIdentifier::AVDTP
/// 'params': [
/// {
/// 'data': 0x0103 # to indicate 1.3
/// },
/// {
/// 'data': 0x0105 # to indicate 1.5
/// }
/// ]
/// },
/// {
/// 'protocol': 1, # u64 Representation of ProtocolIdentifier::SDP
/// 'params': [{
/// 'data': 0x0019
/// }]
/// }
/// ]
//~^ ERROR: doc list item without indentation

@ojeda
Copy link
Contributor Author

ojeda commented Nov 4, 2024

Thanks for taking a look!

For example, doc like

If I understand correctly, that doc is wrong but for other reasons, which makes it hard to offer fixes for; i.e. it should be e.g.

/// - First line. Here's an sample object:
///   ```json
///   {
///      'protocol': 25,
///      'data': 0xffff
///   }
///   ```
/// - Second line.

In any case, please note that even if the lint does not offer fixes (automatic or not), it would still be valuable to have it!

For instance, here it would have detected that broken documentation text.

@ohno418
Copy link

ohno418 commented Nov 5, 2024

doc is wrong but for other reasons

Yeah, you're right.
So, it seems like doc in tests/ui/doc/doc_lazy_list.rs test is also wrong, and we should modify it together.

In any case, please note that even if the lint does not offer fixes (automatic or not), it would still be valuable to have it!

Agreed.

I'll probably take another look later. Thanks!

@ojeda
Copy link
Contributor Author

ojeda commented Nov 5, 2024

Yeah, that last example in tests/ui/doc/doc_lazy_list.rs looks broken to me; however, it seems it was added to test for an ICE (from the comment linked there), i.e. it may be intentionally broken, which would be fine since it is a test. Thanks!

@ohno418 ohno418 linked a pull request Nov 20, 2024 that will close this issue
ojeda pushed a commit to Rust-for-Linux/linux that referenced this issue Dec 18, 2024
Align bullet points and improve indentation in the `Invariants` section
of the `GenDisk` struct documentation for better readability.

[ Yutaro is also working on implementing the lint we suggested to catch
  this sort of issue in upstream Rust:

    rust-lang/rust-clippy#13601
    rust-lang/rust-clippy#13711

  Thanks a lot! - Miguel ]

Fixes: 3253aba ("rust: block: introduce `kernel::block::mq` module")
Signed-off-by: Yutaro Ohno <[email protected]>
Reviewed-by: Boqun Feng <[email protected]>
Acked-by: Andreas Hindborg <[email protected]>
Link: https://lore.kernel.org/r/ZxkcU5yTFCagg_lX@ohnotp
Signed-off-by: Miguel Ojeda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants