Add new lint doc_include_without_cfg (#13625)

It's becoming more and more common to see people including markdown
files in their code using `doc = include_str!("...")`, which is great.
However, often there is no condition on this include, which is not great
because it slows down compilation and might trigger recompilation if
these files are updated.

This lint aims at fixing this situation.

changelog: Add new lint `doc_include_without_cfg`

CHANGELOG.md

@@ -5441,6 +5441,7 @@ Released 2018-09-13
 [`disallowed_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_type
 [`disallowed_types`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types
 [`diverging_sub_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression
+[`doc_include_without_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_include_without_cfg
 [`doc_lazy_continuation`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_lazy_continuation
 [`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

clippy_config/src/conf.rs

@@ -181,7 +181,7 @@ macro_rules! define_Conf {
     )*) => {
         /// Clippy lint configuration
         pub struct Conf {
-            $($(#[doc = $doc])+ pub $name: $ty,)*
+            $($(#[cfg_attr(doc, doc = $doc)])+ pub $name: $ty,)*
         }
 
         mod defaults {

clippy_lints/src/declared_lints.rs

@@ -135,6 +135,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::disallowed_names::DISALLOWED_NAMES_INFO,
     crate::disallowed_script_idents::DISALLOWED_SCRIPT_IDENTS_INFO,
     crate::disallowed_types::DISALLOWED_TYPES_INFO,
+    crate::doc::DOC_INCLUDE_WITHOUT_CFG_INFO,
     crate::doc::DOC_LAZY_CONTINUATION_INFO,
     crate::doc::DOC_LINK_WITH_QUOTES_INFO,
     crate::doc::DOC_MARKDOWN_INFO,

clippy_lints/src/doc/include_in_doc_without_cfg.rs

@@ -0,0 +1,45 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_opt;
+use rustc_ast::{AttrArgs, AttrArgsEq, AttrKind, AttrStyle, Attribute};
+use rustc_errors::Applicability;
+use rustc_lint::LateContext;
+use rustc_span::sym;
+
+use super::DOC_INCLUDE_WITHOUT_CFG;
+
+pub fn check(cx: &LateContext<'_>, attrs: &[Attribute]) {
+    for attr in attrs {
+        if !attr.span.from_expansion()
+            && let AttrKind::Normal(ref normal) = attr.kind
+            && normal.item.path == sym::doc
+            && let AttrArgs::Eq(_, AttrArgsEq::Hir(ref meta)) = normal.item.args
+            && !attr.span.contains(meta.span)
+            // Since the `include_str` is already expanded at this point, we can only take the
+            // whole attribute snippet and then modify for our suggestion.
+            && let Some(snippet) = snippet_opt(cx, attr.span)
+            // We cannot remove this because a `#[doc = include_str!("...")]` attribute can occupy
+            // several lines.
+            && let Some(start) = snippet.find('[')
+            && let Some(end) = snippet.rfind(']')
+            && let snippet = &snippet[start + 1..end]
+            // We check that the expansion actually comes from `include_str!` and not just from
+            // another macro.
+            && let Some(sub_snippet) = snippet.trim().strip_prefix("doc")
+            && let Some(sub_snippet) = sub_snippet.trim().strip_prefix("=")
+            && sub_snippet.trim().starts_with("include_str!")
+        {
+            span_lint_and_sugg(
+                cx,
+                DOC_INCLUDE_WITHOUT_CFG,
+                attr.span,
+                "included a file in documentation unconditionally",
+                "use `cfg_attr(doc, doc = \"...\")`",
+                format!(
+                    "#{}[cfg_attr(doc, {snippet})]",
+                    if attr.style == AttrStyle::Inner { "!" } else { "" }
+                ),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+}

clippy_lints/src/doc/mod.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::lint_without_lint_pass)]
+
 mod lazy_continuation;
 mod too_long_first_doc_paragraph;
 
@@ -33,6 +35,7 @@ use std::ops::Range;
 use url::Url;
 
 mod empty_line_after;
+mod include_in_doc_without_cfg;
 mod link_with_quotes;
 mod markdown;
 mod missing_headers;
@@ -532,6 +535,29 @@ declare_clippy_lint! {
     "empty line after doc comments"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks if included files in doc comments are included only for `cfg(doc)`.
+    ///
+    /// ### Why is this bad?
+    /// These files are not useful for compilation but will still be included.
+    /// Also, if any of these non-source code file is updated, it will trigger a
+    /// recompilation.
+    ///
+    /// ### Example
+    /// ```ignore
+    /// #![doc = include_str!("some_file.md")]
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// #![cfg_attr(doc, doc = include_str!("some_file.md"))]
+    /// ```
+    #[clippy::version = "1.84.0"]
+    pub DOC_INCLUDE_WITHOUT_CFG,
+    pedantic,
+    "check if files included in documentation are behind `cfg(doc)`"
+}
+
 pub struct Documentation {
     valid_idents: FxHashSet<String>,
     check_private_items: bool,
@@ -561,6 +587,7 @@ impl_lint_pass!(Documentation => [
     EMPTY_LINE_AFTER_OUTER_ATTR,
     EMPTY_LINE_AFTER_DOC_COMMENTS,
     TOO_LONG_FIRST_DOC_PARAGRAPH,
+    DOC_INCLUDE_WITHOUT_CFG,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Documentation {
@@ -690,6 +717,7 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
         Some(("fake".into(), "fake".into()))
     }
 
+    include_in_doc_without_cfg::check(cx, attrs);
     if suspicious_doc_comments::check(cx, attrs) || empty_line_after::check(cx, attrs) || is_doc_hidden(attrs) {
         return None;
     }

tests/ui/doc/doc_include_without_cfg.fixed

@@ -0,0 +1,40 @@
+#![warn(clippy::doc_include_without_cfg)]
+// Should not lint.
+#![doc(html_playground_url = "https://playground.example.com/")]
+#![cfg_attr(doc, doc = include_str!("../approx_const.rs"))] //~ doc_include_without_cfg
+// Should not lint.
+#![cfg_attr(feature = "whatever", doc = include_str!("../approx_const.rs"))]
+#![cfg_attr(doc, doc = include_str!("../approx_const.rs"))]
+#![doc = "some doc"]
+//! more doc
+
+macro_rules! man_link {
+    ($a:literal, $b:literal) => {
+        concat!($a, $b)
+    };
+}
+
+// Should not lint!
+macro_rules! tst {
+    ($(#[$attr:meta])*) => {
+        $(#[$attr])*
+        fn blue() {
+            println!("Hello, world!");
+        }
+    }
+}
+
+tst! {
+    /// This is a test with no included file
+}
+
+#[cfg_attr(doc, doc = include_str!("../approx_const.rs"))] //~ doc_include_without_cfg
+// Should not lint.
+#[doc = man_link!("bla", "blob")]
+#[cfg_attr(feature = "whatever", doc = include_str!("../approx_const.rs"))]
+#[cfg_attr(doc, doc = include_str!("../approx_const.rs"))]
+#[doc = "some doc"]
+/// more doc
+fn main() {
+    // test code goes here
+}

tests/ui/doc/doc_include_without_cfg.rs

@@ -0,0 +1,40 @@
+#![warn(clippy::doc_include_without_cfg)]
+// Should not lint.
+#![doc(html_playground_url = "https://playground.example.com/")]
+#![doc = include_str!("../approx_const.rs")] //~ doc_include_without_cfg
+// Should not lint.
+#![cfg_attr(feature = "whatever", doc = include_str!("../approx_const.rs"))]
+#![cfg_attr(doc, doc = include_str!("../approx_const.rs"))]
+#![doc = "some doc"]
+//! more doc
+
+macro_rules! man_link {
+    ($a:literal, $b:literal) => {
+        concat!($a, $b)
+    };
+}
+
+// Should not lint!
+macro_rules! tst {
+    ($(#[$attr:meta])*) => {
+        $(#[$attr])*
+        fn blue() {
+            println!("Hello, world!");
+        }
+    }
+}
+
+tst! {
+    /// This is a test with no included file
+}
+
+#[doc = include_str!("../approx_const.rs")] //~ doc_include_without_cfg
+// Should not lint.
+#[doc = man_link!("bla", "blob")]
+#[cfg_attr(feature = "whatever", doc = include_str!("../approx_const.rs"))]
+#[cfg_attr(doc, doc = include_str!("../approx_const.rs"))]
+#[doc = "some doc"]
+/// more doc
+fn main() {
+    // test code goes here
+}

tests/ui/doc/doc_include_without_cfg.stderr

@@ -0,0 +1,17 @@
+error: included a file in documentation unconditionally
+  --> tests/ui/doc/doc_include_without_cfg.rs:4:1
+   |
+LL | #![doc = include_str!("../approx_const.rs")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `cfg_attr(doc, doc = "...")`: `#![cfg_attr(doc, doc = include_str!("../approx_const.rs"))]`
+   |
+   = note: `-D clippy::doc-include-without-cfg` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::doc_include_without_cfg)]`
+
+error: included a file in documentation unconditionally
+  --> tests/ui/doc/doc_include_without_cfg.rs:31:1
+   |
+LL | #[doc = include_str!("../approx_const.rs")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `cfg_attr(doc, doc = "...")`: `#[cfg_attr(doc, doc = include_str!("../approx_const.rs"))]`
+
+error: aborting due to 2 previous errors
+

tests/ui/missing_doc_crate.rs

@@ -1,4 +1,5 @@
 #![warn(clippy::missing_docs_in_private_items)]
+#![allow(clippy::doc_include_without_cfg)]
 #![doc = include_str!("../../README.md")]
 
 fn main() {}