Add new lint doc_include_without_cfg

Author: GuillaumeGomez

CHANGELOG.md

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

README.md

@@ -5,7 +5,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
 You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.

book/src/README.md

@@ -6,7 +6,7 @@
 A collection of lints to catch common mistakes and improve your
 [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint
 level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how

clippy_config/src/conf.rs

@@ -154,7 +154,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

@@ -134,6 +134,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() {}