Auto merge of #10497 - y21:outer_doc_comment_bang, r=dswij

new lint: suspicious_doc_comments

Fixes #10485.

This PR adds a new lint (`suspicious_doc_comments`) that triggers when the user writes `///!` or `/**!`.
This is almost certainly a mistake and the user probably meant to write an inner doc comment (`//!`, `/*!`) to document the module or crate that this comment is contained in.

changelog: [`suspicious_doc_comments`]: new lint

CHANGELOG.md

@@ -4925,6 +4925,7 @@ Released 2018-09-13
 [`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl
 [`suspicious_assignment_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_assignment_formatting
 [`suspicious_command_arg_space`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_command_arg_space
+[`suspicious_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_doc_comments
 [`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting
 [`suspicious_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_map
 [`suspicious_op_assign_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_op_assign_impl

clippy_lints/src/declared_lints.rs

@@ -569,6 +569,7 @@
     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,

clippy_lints/src/lib.rs

@@ -284,6 +284,7 @@
 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;
@@ -958,6 +959,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, 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));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/suspicious_doc_comments.rs

@@ -0,0 +1,94 @@
+use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_and_then};
+use if_chain::if_chain;
+use rustc_ast::{token::CommentKind, 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*.
+    /// ```rust
+    /// pub mod util {
+    ///     ///! This module contains utility functions.
+    ///
+    ///     pub fn dummy() {}
+    /// }
+    /// ```
+    ///
+    /// Use instead:
+    /// ```rust
+    /// 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_chain! {
+                if let AttrKind::DocComment(com_kind, sym) = attr.kind;
+                if let AttrStyle::Outer = attr.style;
+                if let Some(com) = sym.as_str().strip_prefix('!');
+                then {
+                    let sugg = match com_kind {
+                        CommentKind::Line => format!("//!{com}"),
+                        CommentKind::Block => format!("/*!{com}*/")
+                    };
+                    Some((attr.span, sugg))
+                } else {
+                    None
+                }
+            }
+        })
+        .collect()
+}

tests/ui/suspicious_doc_comments.fixed

@@ -0,0 +1,81 @@
+// run-rustfix
+#![allow(unused)]
+#![warn(clippy::suspicious_doc_comments)]
+
+//! Real module documentation.
+//! Fake module documentation.
+fn baz() {}
+
+pub mod singleline_outer_doc {
+    //! This module contains useful functions.
+
+    pub fn bar() {}
+}
+
+pub mod singleline_inner_doc {
+    //! This module contains useful functions.
+
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc {
+    /*! This module contains useful functions.
+     */
+
+    pub fn bar() {}
+}
+
+pub mod multiline_inner_doc {
+    /*! This module contains useful functions.
+     */
+
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc2 {
+    //! This module
+    //! contains
+    //! useful functions.
+
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc3 {
+    //! a
+    //! b
+
+    /// c
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc4 {
+    //! a
+    /// b
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc_gap {
+    //! a
+
+    //! b
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc_commented {
+    /////! This outer doc comment was commented out.
+    pub fn bar() {}
+}
+
+pub mod outer_doc_macro {
+    //! Very cool macro
+    macro_rules! x {
+        () => {};
+    }
+}
+
+pub mod useless_outer_doc {
+    //! Huh.
+    use std::mem;
+}
+
+fn main() {}

tests/ui/suspicious_doc_comments.rs

@@ -0,0 +1,81 @@
+// run-rustfix
+#![allow(unused)]
+#![warn(clippy::suspicious_doc_comments)]
+
+//! Real module documentation.
+///! Fake module documentation.
+fn baz() {}
+
+pub mod singleline_outer_doc {
+    ///! This module contains useful functions.
+
+    pub fn bar() {}
+}
+
+pub mod singleline_inner_doc {
+    //! This module contains useful functions.
+
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc {
+    /**! This module contains useful functions.
+     */
+
+    pub fn bar() {}
+}
+
+pub mod multiline_inner_doc {
+    /*! This module contains useful functions.
+     */
+
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc2 {
+    ///! This module
+    ///! contains
+    ///! useful functions.
+
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc3 {
+    ///! a
+    ///! b
+
+    /// c
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc4 {
+    ///! a
+    /// b
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc_gap {
+    ///! a
+
+    ///! b
+    pub fn bar() {}
+}
+
+pub mod multiline_outer_doc_commented {
+    /////! This outer doc comment was commented out.
+    pub fn bar() {}
+}
+
+pub mod outer_doc_macro {
+    ///! Very cool macro
+    macro_rules! x {
+        () => {};
+    }
+}
+
+pub mod useless_outer_doc {
+    ///! Huh.
+    use std::mem;
+}
+
+fn main() {}

tests/ui/suspicious_doc_comments.stderr

@@ -0,0 +1,114 @@
+error: this is an outer doc comment and does not apply to the parent module or crate
+  --> $DIR/suspicious_doc_comments.rs:6:1
+   |
+LL | ///! Fake module documentation.
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::suspicious-doc-comments` implied by `-D warnings`
+help: use an inner doc comment to document the parent module or crate
+   |
+LL | //! Fake module documentation.
+   |
+
+error: this is an outer doc comment and does not apply to the parent module or crate
+  --> $DIR/suspicious_doc_comments.rs:10:5
+   |
+LL |     ///! This module contains useful functions.
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use an inner doc comment to document the parent module or crate
+   |
+LL |     //! This module contains useful functions.
+   |
+
+error: this is an outer doc comment and does not apply to the parent module or crate
+  --> $DIR/suspicious_doc_comments.rs:22:5
+   |
+LL | /     /**! This module contains useful functions.
+LL | |      */
+   | |_______^
+   |
+help: use an inner doc comment to document the parent module or crate
+   |
+LL ~     /*! This module contains useful functions.
+LL +      */
+   |
+
+error: this is an outer doc comment and does not apply to the parent module or crate
+  --> $DIR/suspicious_doc_comments.rs:36:5
+   |
+LL | /     ///! This module
+LL | |     ///! contains
+LL | |     ///! useful functions.
+   | |__________________________^
+   |
+help: use an inner doc comment to document the parent module or crate
+   |
+LL ~     //! This module
+LL ~     //! contains
+LL ~     //! useful functions.
+   |
+
+error: this is an outer doc comment and does not apply to the parent module or crate
+  --> $DIR/suspicious_doc_comments.rs:44:5
+   |
+LL | /     ///! a
+LL | |     ///! b
+   | |__________^
+   |
+help: use an inner doc comment to document the parent module or crate
+   |
+LL ~     //! a
+LL ~     //! b
+   |
+
+error: this is an outer doc comment and does not apply to the parent module or crate
+  --> $DIR/suspicious_doc_comments.rs:52:5
+   |
+LL |     ///! a
+   |     ^^^^^^
+   |
+help: use an inner doc comment to document the parent module or crate
+   |
+LL |     //! a
+   |
+
+error: this is an outer doc comment and does not apply to the parent module or crate
+  --> $DIR/suspicious_doc_comments.rs:58:5
+   |
+LL | /     ///! a
+LL | |
+LL | |     ///! b
+   | |__________^
+   |
+help: use an inner doc comment to document the parent module or crate
+   |
+LL ~     //! a
+LL | 
+LL ~     //! b
+   |
+
+error: this is an outer doc comment and does not apply to the parent module or crate
+  --> $DIR/suspicious_doc_comments.rs:70:5
+   |
+LL |     ///! Very cool macro
+   |     ^^^^^^^^^^^^^^^^^^^^
+   |
+help: use an inner doc comment to document the parent module or crate
+   |
+LL |     //! Very cool macro
+   |
+
+error: this is an outer doc comment and does not apply to the parent module or crate
+  --> $DIR/suspicious_doc_comments.rs:77:5
+   |
+LL |     ///! Huh.
+   |     ^^^^^^^^^
+   |
+help: use an inner doc comment to document the parent module or crate
+   |
+LL |     //! Huh.
+   |
+
+error: aborting due to 9 previous errors
+

tests/ui/suspicious_doc_comments_unfixable.rs

@@ -0,0 +1,16 @@
+#![allow(unused)]
+#![warn(clippy::suspicious_doc_comments)]
+
+///! a
+///! b
+/// c
+///! d
+pub fn foo() {}
+
+///! a
+///! b
+/// c
+///! d
+use std::mem;
+
+fn main() {}

tests/ui/suspicious_doc_comments_unfixable.stderr

@@ -0,0 +1,37 @@
+error: this is an outer doc comment and does not apply to the parent module or crate
+  --> $DIR/suspicious_doc_comments_unfixable.rs:4:1
+   |
+LL | / ///! a
+LL | | ///! b
+LL | | /// c
+LL | | ///! d
+   | |______^
+   |
+   = note: `-D clippy::suspicious-doc-comments` implied by `-D warnings`
+help: use an inner doc comment to document the parent module or crate
+   |
+LL + //! a
+LL + //! b
+LL | /// c
+LL + //! d
+   |
+
+error: this is an outer doc comment and does not apply to the parent module or crate
+  --> $DIR/suspicious_doc_comments_unfixable.rs:10:1
+   |
+LL | / ///! a
+LL | | ///! b
+LL | | /// c
+LL | | ///! d
+   | |______^
+   |
+help: use an inner doc comment to document the parent module or crate
+   |
+LL + //! a
+LL + //! b
+LL | /// c
+LL + //! d
+   |
+
+error: aborting due to 2 previous errors
+