From f455587feec19b459c5eb75c9dc1995b93bb24f6 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 9 Jul 2024 16:00:06 +0200 Subject: [PATCH] Set the limit of characters to 200 and don't run the lint on private items unless config allows it --- clippy_lints/src/doc/mod.rs | 13 +++++++++---- .../src/doc/too_long_first_doc_paragraph.rs | 17 +++++++++++------ tests/ui/too_long_first_doc_paragraph-fix.fixed | 5 +++-- tests/ui/too_long_first_doc_paragraph-fix.rs | 5 +++-- .../ui/too_long_first_doc_paragraph-fix.stderr | 3 ++- tests/ui/too_long_first_doc_paragraph.rs | 12 +++++++++--- 6 files changed, 37 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 5e9fb0162bf..6debcd9b6a0 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -493,7 +493,13 @@ fn check_attributes(&mut self, cx: &LateContext<'tcx>, attrs: &'tcx [Attribute]) match cx.tcx.hir_node(cx.last_node_with_lint_attrs) { Node::Item(item) => { - too_long_first_doc_paragraph::check(cx, attrs, item.kind, headers.first_paragraph_len); + too_long_first_doc_paragraph::check( + cx, + item, + attrs, + headers.first_paragraph_len, + self.check_private_items, + ); match item.kind { ItemKind::Fn(sig, _, body_id) => { if !(is_entrypoint_fn(cx, item.owner_id.to_def_id()) @@ -627,9 +633,8 @@ fn fake_broken_link_callback<'a>(_: BrokenLink<'_>) -> Option<(CowStr<'a>, CowSt acc }); doc.pop(); - let doc = doc.trim(); - if doc.is_empty() { + if doc.trim().is_empty() { if let Some(span) = span_of_fragments(&fragments) { span_lint_and_help( cx, @@ -653,7 +658,7 @@ fn fake_broken_link_callback<'a>(_: BrokenLink<'_>) -> Option<(CowStr<'a>, CowSt cx, valid_idents, parser.into_offset_iter(), - doc, + &doc, Fragments { fragments: &fragments, doc: &doc, diff --git a/clippy_lints/src/doc/too_long_first_doc_paragraph.rs b/clippy_lints/src/doc/too_long_first_doc_paragraph.rs index a5a58b44401..a1b714b7d00 100644 --- a/clippy_lints/src/doc/too_long_first_doc_paragraph.rs +++ b/clippy_lints/src/doc/too_long_first_doc_paragraph.rs @@ -1,6 +1,6 @@ use rustc_ast::ast::Attribute; use rustc_errors::Applicability; -use rustc_hir::ItemKind; +use rustc_hir::{Item, ItemKind}; use rustc_lint::LateContext; use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; @@ -10,13 +10,17 @@ pub(super) fn check( cx: &LateContext<'_>, + item: &Item<'_>, attrs: &[Attribute], - item_kind: ItemKind<'_>, mut first_paragraph_len: usize, + check_private_items: bool, ) { - if first_paragraph_len <= 100 + if !check_private_items && !cx.effective_visibilities.is_exported(item.owner_id.def_id) { + return; + } + if first_paragraph_len <= 200 || !matches!( - item_kind, + item.kind, ItemKind::Static(..) | ItemKind::Const(..) | ItemKind::Fn(..) @@ -32,6 +36,7 @@ pub(super) fn check( { return; } + let mut spans = Vec::new(); let mut should_suggest_empty_doc = false; @@ -42,7 +47,7 @@ pub(super) fn check( let doc = doc.trim(); if spans.len() == 1 { // We make this suggestion only if the first doc line ends with a punctuation - // because if might just need to add an empty line with `///`. + // because it might just need to add an empty line with `///`. should_suggest_empty_doc = doc.ends_with('.') || doc.ends_with('!') || doc.ends_with('?'); } let len = doc.chars().count(); @@ -68,7 +73,7 @@ pub(super) fn check( |diag| { diag.span_suggestion( new_span, - "add", + "add an empty line", format!("{snippet}///\n"), Applicability::MachineApplicable, ); diff --git a/tests/ui/too_long_first_doc_paragraph-fix.fixed b/tests/ui/too_long_first_doc_paragraph-fix.fixed index b3f66f52793..d4a0cdf3447 100644 --- a/tests/ui/too_long_first_doc_paragraph-fix.fixed +++ b/tests/ui/too_long_first_doc_paragraph-fix.fixed @@ -4,5 +4,6 @@ /// /// A much longer explanation that goes into a lot more detail about /// how the thing works, possibly with doclinks and so one, -/// and probably spanning a many rows. -struct Foo; +/// and probably spanning a many rows. Blablabla, it needs to be over +/// 200 characters so I needed to write something longeeeeeeer. +pub struct Foo; diff --git a/tests/ui/too_long_first_doc_paragraph-fix.rs b/tests/ui/too_long_first_doc_paragraph-fix.rs index f0ece9523de..5a3b6c42a32 100644 --- a/tests/ui/too_long_first_doc_paragraph-fix.rs +++ b/tests/ui/too_long_first_doc_paragraph-fix.rs @@ -3,5 +3,6 @@ /// A very short summary. /// A much longer explanation that goes into a lot more detail about /// how the thing works, possibly with doclinks and so one, -/// and probably spanning a many rows. -struct Foo; +/// and probably spanning a many rows. Blablabla, it needs to be over +/// 200 characters so I needed to write something longeeeeeeer. +pub struct Foo; diff --git a/tests/ui/too_long_first_doc_paragraph-fix.stderr b/tests/ui/too_long_first_doc_paragraph-fix.stderr index 00949f405d5..6403265a39c 100644 --- a/tests/ui/too_long_first_doc_paragraph-fix.stderr +++ b/tests/ui/too_long_first_doc_paragraph-fix.stderr @@ -4,7 +4,8 @@ error: first doc comment paragraph is too long LL | / /// A very short summary. LL | | /// A much longer explanation that goes into a lot more detail about LL | | /// how the thing works, possibly with doclinks and so one, -LL | | /// and probably spanning a many rows. +LL | | /// and probably spanning a many rows. Blablabla, it needs to be over +LL | | /// 200 characters so I needed to write something longeeeeeeer. | |_ | = note: `-D clippy::too-long-first-doc-paragraph` implied by `-D warnings` diff --git a/tests/ui/too_long_first_doc_paragraph.rs b/tests/ui/too_long_first_doc_paragraph.rs index 88a8f6d3831..1042249c5b7 100644 --- a/tests/ui/too_long_first_doc_paragraph.rs +++ b/tests/ui/too_long_first_doc_paragraph.rs @@ -19,7 +19,7 @@ impl Bar {} /// Nunc turpis nunc, lacinia /// a dolor in, pellentesque aliquet enim. Cras nec maximus sem. Mauris arcu libero, /// gravida non lacinia at, rhoncus eu lacus. -enum Enum { +pub enum Enum { A, } @@ -27,7 +27,7 @@ enum Enum { /// ipsum dolor sit amet, consectetur adipiscing elit. Nunc turpis nunc, lacinia /// a dolor in, pellentesque aliquet enim. Cras nec maximus sem. Mauris arcu libero, /// gravida non lacinia at, rhoncus eu lacus. -union Union { +pub union Union { a: u8, b: u8, } @@ -37,11 +37,17 @@ union Union { /// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc turpis nunc, lacinia /// a dolor in, pellentesque aliquet enim. Cras nec maximus sem. Mauris arcu libero, /// gravida non lacinia at, rhoncus eu lacus. -union Union2 { +pub union Union2 { a: u8, b: u8, } +// Should not warn! (not public) +/// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc turpis nunc, lacinia +/// a dolor in, pellentesque aliquet enim. Cras nec maximus sem. Mauris arcu libero, +/// gravida non lacinia at, rhoncus eu lacus. +fn f() {} + fn main() { // test code goes here }