From 404e47aa84b16cab60008cb74eead08475e83960 Mon Sep 17 00:00:00 2001
From: Guillaume Gomez <guillaume1.gomez@gmail.com>
Date: Tue, 29 Oct 2024 17:53:40 +0100
Subject: [PATCH] Add new lint `doc_include_without_cfg`
---
CHANGELOG.md | 1 +
README.md | 2 +-
book/src/README.md | 2 +-
clippy_config/src/conf.rs | 2 +-
clippy_lints/src/declared_lints.rs | 1 +
.../src/doc/include_in_doc_without_cfg.rs | 45 +++++++++++++++++++
clippy_lints/src/doc/mod.rs | 28 ++++++++++++
tests/ui/doc/doc_include_without_cfg.fixed | 40 +++++++++++++++++
tests/ui/doc/doc_include_without_cfg.rs | 40 +++++++++++++++++
tests/ui/doc/doc_include_without_cfg.stderr | 17 +++++++
tests/ui/missing_doc_crate.rs | 1 +
11 files changed, 176 insertions(+), 3 deletions(-)
create mode 100644 clippy_lints/src/doc/include_in_doc_without_cfg.rs
create mode 100644 tests/ui/doc/doc_include_without_cfg.fixed
create mode 100644 tests/ui/doc/doc_include_without_cfg.rs
create mode 100644 tests/ui/doc/doc_include_without_cfg.stderr
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4bdbc91db93..1af7f349b8d 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/README.md b/README.md
index ec76a6dfb08..1690e2beb16 100644
--- a/README.md
+++ b/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.
diff --git a/book/src/README.md b/book/src/README.md
index 7bdfb97c3ac..23527ba896a 100644
--- a/book/src/README.md
+++ b/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
diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs
index b0faac6d2a8..263637aacd2 100644
--- a/clippy_config/src/conf.rs
+++ b/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 {
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index d8918d37afa..fdb778d3ac0 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/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,
diff --git a/clippy_lints/src/doc/include_in_doc_without_cfg.rs b/clippy_lints/src/doc/include_in_doc_without_cfg.rs
new file mode 100644
index 00000000000..49978d4a655
--- /dev/null
+++ b/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,
+ );
+ }
+ }
+}
diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs
index df7c37a192a..dae7fab47df 100644
--- a/clippy_lints/src/doc/mod.rs
+++ b/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;
}
diff --git a/tests/ui/doc/doc_include_without_cfg.fixed b/tests/ui/doc/doc_include_without_cfg.fixed
new file mode 100644
index 00000000000..d4ae810d738
--- /dev/null
+++ b/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
+}
diff --git a/tests/ui/doc/doc_include_without_cfg.rs b/tests/ui/doc/doc_include_without_cfg.rs
new file mode 100644
index 00000000000..c82f6bf2035
--- /dev/null
+++ b/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
+}
diff --git a/tests/ui/doc/doc_include_without_cfg.stderr b/tests/ui/doc/doc_include_without_cfg.stderr
new file mode 100644
index 00000000000..17ea53c7c31
--- /dev/null
+++ b/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
+
diff --git a/tests/ui/missing_doc_crate.rs b/tests/ui/missing_doc_crate.rs
index e00c7fbfed1..fdb23af279d 100644
--- a/tests/ui/missing_doc_crate.rs
+++ b/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() {}