From d89890d154f4d5ab148312421d1cb2022809fab4 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Mon, 13 Nov 2023 13:44:00 +0100 Subject: [PATCH] move `suspicious_doc_comments` to doc pass --- clippy_lints/src/declared_lints.rs | 2 +- clippy_lints/src/doc.rs | 89 ++++++++++++++++++++ clippy_lints/src/lib.rs | 2 - clippy_lints/src/suspicious_doc_comments.rs | 92 --------------------- 4 files changed, 90 insertions(+), 95 deletions(-) delete mode 100644 clippy_lints/src/suspicious_doc_comments.rs diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 04dcccede..ecfe8e158 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -139,6 +139,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::doc::MISSING_PANICS_DOC_INFO, crate::doc::MISSING_SAFETY_DOC_INFO, crate::doc::NEEDLESS_DOCTEST_MAIN_INFO, + crate::doc::SUSPICIOUS_DOC_COMMENTS_INFO, crate::doc::UNNECESSARY_SAFETY_DOC_INFO, crate::double_parens::DOUBLE_PARENS_INFO, crate::drop_forget_ref::DROP_NON_DROP_INFO, @@ -626,7 +627,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::strings::STR_TO_STRING_INFO, crate::strings::TRIM_SPLIT_WHITESPACE_INFO, crate::strlen_on_c_strings::STRLEN_ON_C_STRINGS_INFO, - crate::suspicious_doc_comments::SUSPICIOUS_DOC_COMMENTS_INFO, crate::suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS_INFO, crate::suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL_INFO, crate::suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL_INFO, diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 6a12baa42..632bbacb6 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -10,6 +10,8 @@ use pulldown_cmark::Event::{ use pulldown_cmark::Tag::{CodeBlock, Heading, Item, Link, Paragraph}; use pulldown_cmark::{BrokenLink, CodeBlockKind, CowStr, Options}; use rustc_ast::ast::{Async, Attribute, Fn, FnRetTy, ItemKind}; +use rustc_ast::token::CommentKind; +use rustc_ast::{AttrKind, AttrStyle}; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; use rustc_errors::emitter::EmitterWriter; @@ -260,6 +262,53 @@ declare_clippy_lint! { "`pub fn` or `pub trait` with `# Safety` docs" } +declare_clippy_lint! { + /// ### What it does + /// Detects the use of outer doc comments (`///`, `/**`) followed by a bang (`!`): `///!` + /// + /// ### Why is this bad? + /// Triple-slash comments (known as "outer doc comments") apply to items that follow it. + /// An outer doc comment followed by a bang (i.e. `///!`) has no specific meaning. + /// + /// The user most likely meant to write an inner doc comment (`//!`, `/*!`), which + /// applies to the parent item (i.e. the item that the comment is contained in, + /// usually a module or crate). + /// + /// ### Known problems + /// Inner doc comments can only appear before items, so there are certain cases where the suggestion + /// made by this lint is not valid code. For example: + /// ```rs + /// fn foo() {} + /// ///! + /// fn bar() {} + /// ``` + /// This lint detects the doc comment and suggests changing it to `//!`, but an inner doc comment + /// is not valid at that position. + /// + /// ### Example + /// In this example, the doc comment is attached to the *function*, rather than the *module*. + /// ```no_run + /// pub mod util { + /// ///! This module contains utility functions. + /// + /// pub fn dummy() {} + /// } + /// ``` + /// + /// Use instead: + /// ```no_run + /// pub mod util { + /// //! This module contains utility functions. + /// + /// pub fn dummy() {} + /// } + /// ``` + #[clippy::version = "1.70.0"] + pub SUSPICIOUS_DOC_COMMENTS, + suspicious, + "suspicious usage of (outer) doc comments" +} + #[expect(clippy::module_name_repetitions)] #[derive(Clone)] pub struct DocMarkdown { @@ -284,6 +333,7 @@ impl_lint_pass!(DocMarkdown => [ MISSING_PANICS_DOC, NEEDLESS_DOCTEST_MAIN, UNNECESSARY_SAFETY_DOC, + SUSPICIOUS_DOC_COMMENTS ]); impl<'tcx> LateLintPass<'tcx> for DocMarkdown { @@ -478,6 +528,8 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[ return None; } + check_almost_inner_doc(cx, attrs); + let (fragments, _) = attrs_to_doc_fragments(attrs.iter().map(|attr| (attr, None)), true); let mut doc = String::new(); for fragment in &fragments { @@ -506,6 +558,43 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[ )) } +/// Looks for `///!` and `/**!` comments, which were probably meant to be `//!` and `/*!` +fn check_almost_inner_doc(cx: &LateContext<'_>, attrs: &[Attribute]) { + let replacements: Vec<_> = attrs + .iter() + .filter_map(|attr| { + if let AttrKind::DocComment(com_kind, sym) = attr.kind + && let AttrStyle::Outer = attr.style + && let Some(com) = sym.as_str().strip_prefix('!') + { + let sugg = match com_kind { + CommentKind::Line => format!("//!{com}"), + CommentKind::Block => format!("/*!{com}*/"), + }; + Some((attr.span, sugg)) + } else { + None + } + }) + .collect(); + + if let Some((&(lo_span, _), &(hi_span, _))) = replacements.first().zip(replacements.last()) { + span_lint_and_then( + cx, + SUSPICIOUS_DOC_COMMENTS, + lo_span.to(hi_span), + "this is an outer doc comment and does not apply to the parent module or crate", + |diag| { + diag.multipart_suggestion( + "use an inner doc comment to document the parent module or crate", + replacements, + Applicability::MaybeIncorrect, + ); + }, + ); + } +} + const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail"]; #[allow(clippy::too_many_lines)] // Only a big match statement diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 609ff3d05..463f880aa 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -307,7 +307,6 @@ mod slow_vector_initialization; mod std_instead_of_core; mod strings; mod strlen_on_c_strings; -mod suspicious_doc_comments; mod suspicious_operation_groupings; mod suspicious_trait_impl; mod suspicious_xor_used_as_pow; @@ -1000,7 +999,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(lines_filter_map_ok::LinesFilterMapOk)); store.register_late_pass(|_| Box::new(tests_outside_test_module::TestsOutsideTestModule)); store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation)); - store.register_early_pass(|| Box::new(suspicious_doc_comments::SuspiciousDocComments)); store.register_early_pass(move || { Box::new(excessive_nesting::ExcessiveNesting { excessive_nesting_threshold, diff --git a/clippy_lints/src/suspicious_doc_comments.rs b/clippy_lints/src/suspicious_doc_comments.rs deleted file mode 100644 index cbd6e7458..000000000 --- a/clippy_lints/src/suspicious_doc_comments.rs +++ /dev/null @@ -1,92 +0,0 @@ -use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_and_then}; -use rustc_ast::token::CommentKind; -use rustc_ast::{AttrKind, AttrStyle, Attribute, Item}; -use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::Span; - -declare_clippy_lint! { - /// ### What it does - /// Detects the use of outer doc comments (`///`, `/**`) followed by a bang (`!`): `///!` - /// - /// ### Why is this bad? - /// Triple-slash comments (known as "outer doc comments") apply to items that follow it. - /// An outer doc comment followed by a bang (i.e. `///!`) has no specific meaning. - /// - /// The user most likely meant to write an inner doc comment (`//!`, `/*!`), which - /// applies to the parent item (i.e. the item that the comment is contained in, - /// usually a module or crate). - /// - /// ### Known problems - /// Inner doc comments can only appear before items, so there are certain cases where the suggestion - /// made by this lint is not valid code. For example: - /// ```rs - /// fn foo() {} - /// ///! - /// fn bar() {} - /// ``` - /// This lint detects the doc comment and suggests changing it to `//!`, but an inner doc comment - /// is not valid at that position. - /// - /// ### Example - /// In this example, the doc comment is attached to the *function*, rather than the *module*. - /// ```no_run - /// pub mod util { - /// ///! This module contains utility functions. - /// - /// pub fn dummy() {} - /// } - /// ``` - /// - /// Use instead: - /// ```no_run - /// pub mod util { - /// //! This module contains utility functions. - /// - /// pub fn dummy() {} - /// } - /// ``` - #[clippy::version = "1.70.0"] - pub SUSPICIOUS_DOC_COMMENTS, - suspicious, - "suspicious usage of (outer) doc comments" -} -declare_lint_pass!(SuspiciousDocComments => [SUSPICIOUS_DOC_COMMENTS]); - -const WARNING: &str = "this is an outer doc comment and does not apply to the parent module or crate"; -const HELP: &str = "use an inner doc comment to document the parent module or crate"; - -impl EarlyLintPass for SuspiciousDocComments { - fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { - let replacements = collect_doc_comment_replacements(&item.attrs); - - if let Some(((lo_span, _), (hi_span, _))) = replacements.first().zip(replacements.last()) { - let span = lo_span.to(*hi_span); - - span_lint_and_then(cx, SUSPICIOUS_DOC_COMMENTS, span, WARNING, |diag| { - multispan_sugg_with_applicability(diag, HELP, Applicability::MaybeIncorrect, replacements); - }); - } - } -} - -fn collect_doc_comment_replacements(attrs: &[Attribute]) -> Vec<(Span, String)> { - attrs - .iter() - .filter_map(|attr| { - if let AttrKind::DocComment(com_kind, sym) = attr.kind - && let AttrStyle::Outer = attr.style - && let Some(com) = sym.as_str().strip_prefix('!') - { - let sugg = match com_kind { - CommentKind::Line => format!("//!{com}"), - CommentKind::Block => format!("/*!{com}*/"), - }; - Some((attr.span, sugg)) - } else { - None - } - }) - .collect() -}