Skip to content

Commit 8298da7

Browse files
blyxyasblyxyas
authored
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`
2 parents a19d69d + 404e47a commit 8298da7

9 files changed

+174
-1
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5441,6 +5441,7 @@ Released 2018-09-13
54415441
[`disallowed_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_type
54425442
[`disallowed_types`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types
54435443
[`diverging_sub_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression
5444+
[`doc_include_without_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_include_without_cfg
54445445
[`doc_lazy_continuation`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_lazy_continuation
54455446
[`doc_link_with_quotes`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_link_with_quotes
54465447
[`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown

‎clippy_config/src/conf.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ macro_rules! define_Conf {
181181
)*) => {
182182
/// Clippy lint configuration
183183
pub struct Conf {
184-
$($(#[doc = $doc])+ pub $name: $ty,)*
184+
$($(#[cfg_attr(doc, doc = $doc)])+ pub $name: $ty,)*
185185
}
186186

187187
mod defaults {

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
135135
crate::disallowed_names::DISALLOWED_NAMES_INFO,
136136
crate::disallowed_script_idents::DISALLOWED_SCRIPT_IDENTS_INFO,
137137
crate::disallowed_types::DISALLOWED_TYPES_INFO,
138+
crate::doc::DOC_INCLUDE_WITHOUT_CFG_INFO,
138139
crate::doc::DOC_LAZY_CONTINUATION_INFO,
139140
crate::doc::DOC_LINK_WITH_QUOTES_INFO,
140141
crate::doc::DOC_MARKDOWN_INFO,

‎clippy_lints/src/doc/include_in_doc_without_cfg.rs

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet_opt;
3+
use rustc_ast::{AttrArgs, AttrArgsEq, AttrKind, AttrStyle, Attribute};
4+
use rustc_errors::Applicability;
5+
use rustc_lint::LateContext;
6+
use rustc_span::sym;
7+
8+
use super::DOC_INCLUDE_WITHOUT_CFG;
9+
10+
pub fn check(cx: &LateContext<'_>, attrs: &[Attribute]) {
11+
for attr in attrs {
12+
if !attr.span.from_expansion()
13+
&& let AttrKind::Normal(ref normal) = attr.kind
14+
&& normal.item.path == sym::doc
15+
&& let AttrArgs::Eq(_, AttrArgsEq::Hir(ref meta)) = normal.item.args
16+
&& !attr.span.contains(meta.span)
17+
// Since the `include_str` is already expanded at this point, we can only take the
18+
// whole attribute snippet and then modify for our suggestion.
19+
&& let Some(snippet) = snippet_opt(cx, attr.span)
20+
// We cannot remove this because a `#[doc = include_str!("...")]` attribute can occupy
21+
// several lines.
22+
&& let Some(start) = snippet.find('[')
23+
&& let Some(end) = snippet.rfind(']')
24+
&& let snippet = &snippet[start + 1..end]
25+
// We check that the expansion actually comes from `include_str!` and not just from
26+
// another macro.
27+
&& let Some(sub_snippet) = snippet.trim().strip_prefix("doc")
28+
&& let Some(sub_snippet) = sub_snippet.trim().strip_prefix("=")
29+
&& sub_snippet.trim().starts_with("include_str!")
30+
{
31+
span_lint_and_sugg(
32+
cx,
33+
DOC_INCLUDE_WITHOUT_CFG,
34+
attr.span,
35+
"included a file in documentation unconditionally",
36+
"use `cfg_attr(doc, doc = \"...\")`",
37+
format!(
38+
"#{}[cfg_attr(doc, {snippet})]",
39+
if attr.style == AttrStyle::Inner { "!" } else { "" }
40+
),
41+
Applicability::MachineApplicable,
42+
);
43+
}
44+
}
45+
}

‎clippy_lints/src/doc/mod.rs

+28
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#![allow(clippy::lint_without_lint_pass)]
2+
13
mod lazy_continuation;
24
mod too_long_first_doc_paragraph;
35

@@ -33,6 +35,7 @@ use std::ops::Range;
3335
use url::Url;
3436

3537
mod empty_line_after;
38+
mod include_in_doc_without_cfg;
3639
mod link_with_quotes;
3740
mod markdown;
3841
mod missing_headers;
@@ -532,6 +535,29 @@ declare_clippy_lint! {
532535
"empty line after doc comments"
533536
}
534537

538+
declare_clippy_lint! {
539+
/// ### What it does
540+
/// Checks if included files in doc comments are included only for `cfg(doc)`.
541+
///
542+
/// ### Why is this bad?
543+
/// These files are not useful for compilation but will still be included.
544+
/// Also, if any of these non-source code file is updated, it will trigger a
545+
/// recompilation.
546+
///
547+
/// ### Example
548+
/// ```ignore
549+
/// #![doc = include_str!("some_file.md")]
550+
/// ```
551+
/// Use instead:
552+
/// ```no_run
553+
/// #![cfg_attr(doc, doc = include_str!("some_file.md"))]
554+
/// ```
555+
#[clippy::version = "1.84.0"]
556+
pub DOC_INCLUDE_WITHOUT_CFG,
557+
pedantic,
558+
"check if files included in documentation are behind `cfg(doc)`"
559+
}
560+
535561
pub struct Documentation {
536562
valid_idents: FxHashSet<String>,
537563
check_private_items: bool,
@@ -561,6 +587,7 @@ impl_lint_pass!(Documentation => [
561587
EMPTY_LINE_AFTER_OUTER_ATTR,
562588
EMPTY_LINE_AFTER_DOC_COMMENTS,
563589
TOO_LONG_FIRST_DOC_PARAGRAPH,
590+
DOC_INCLUDE_WITHOUT_CFG,
564591
]);
565592

566593
impl<'tcx> LateLintPass<'tcx> for Documentation {
@@ -690,6 +717,7 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
690717
Some(("fake".into(), "fake".into()))
691718
}
692719

720+
include_in_doc_without_cfg::check(cx, attrs);
693721
if suspicious_doc_comments::check(cx, attrs) || empty_line_after::check(cx, attrs) || is_doc_hidden(attrs) {
694722
return None;
695723
}

‎tests/ui/doc/doc_include_without_cfg.fixed

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
#![warn(clippy::doc_include_without_cfg)]
2+
// Should not lint.
3+
#![doc(html_playground_url = "https://playground.example.com/")]
4+
#![cfg_attr(doc, doc = include_str!("../approx_const.rs"))] //~ doc_include_without_cfg
5+
// Should not lint.
6+
#![cfg_attr(feature = "whatever", doc = include_str!("../approx_const.rs"))]
7+
#![cfg_attr(doc, doc = include_str!("../approx_const.rs"))]
8+
#![doc = "some doc"]
9+
//! more doc
10+
11+
macro_rules! man_link {
12+
($a:literal, $b:literal) => {
13+
concat!($a, $b)
14+
};
15+
}
16+
17+
// Should not lint!
18+
macro_rules! tst {
19+
($(#[$attr:meta])*) => {
20+
$(#[$attr])*
21+
fn blue() {
22+
println!("Hello, world!");
23+
}
24+
}
25+
}
26+
27+
tst! {
28+
/// This is a test with no included file
29+
}
30+
31+
#[cfg_attr(doc, doc = include_str!("../approx_const.rs"))] //~ doc_include_without_cfg
32+
// Should not lint.
33+
#[doc = man_link!("bla", "blob")]
34+
#[cfg_attr(feature = "whatever", doc = include_str!("../approx_const.rs"))]
35+
#[cfg_attr(doc, doc = include_str!("../approx_const.rs"))]
36+
#[doc = "some doc"]
37+
/// more doc
38+
fn main() {
39+
// test code goes here
40+
}

‎tests/ui/doc/doc_include_without_cfg.rs

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
#![warn(clippy::doc_include_without_cfg)]
2+
// Should not lint.
3+
#![doc(html_playground_url = "https://playground.example.com/")]
4+
#![doc = include_str!("../approx_const.rs")] //~ doc_include_without_cfg
5+
// Should not lint.
6+
#![cfg_attr(feature = "whatever", doc = include_str!("../approx_const.rs"))]
7+
#![cfg_attr(doc, doc = include_str!("../approx_const.rs"))]
8+
#![doc = "some doc"]
9+
//! more doc
10+
11+
macro_rules! man_link {
12+
($a:literal, $b:literal) => {
13+
concat!($a, $b)
14+
};
15+
}
16+
17+
// Should not lint!
18+
macro_rules! tst {
19+
($(#[$attr:meta])*) => {
20+
$(#[$attr])*
21+
fn blue() {
22+
println!("Hello, world!");
23+
}
24+
}
25+
}
26+
27+
tst! {
28+
/// This is a test with no included file
29+
}
30+
31+
#[doc = include_str!("../approx_const.rs")] //~ doc_include_without_cfg
32+
// Should not lint.
33+
#[doc = man_link!("bla", "blob")]
34+
#[cfg_attr(feature = "whatever", doc = include_str!("../approx_const.rs"))]
35+
#[cfg_attr(doc, doc = include_str!("../approx_const.rs"))]
36+
#[doc = "some doc"]
37+
/// more doc
38+
fn main() {
39+
// test code goes here
40+
}

‎tests/ui/doc/doc_include_without_cfg.stderr

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
error: included a file in documentation unconditionally
2+
--> tests/ui/doc/doc_include_without_cfg.rs:4:1
3+
|
4+
LL | #![doc = include_str!("../approx_const.rs")]
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `cfg_attr(doc, doc = "...")`: `#![cfg_attr(doc, doc = include_str!("../approx_const.rs"))]`
6+
|
7+
= note: `-D clippy::doc-include-without-cfg` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::doc_include_without_cfg)]`
9+
10+
error: included a file in documentation unconditionally
11+
--> tests/ui/doc/doc_include_without_cfg.rs:31:1
12+
|
13+
LL | #[doc = include_str!("../approx_const.rs")]
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `cfg_attr(doc, doc = "...")`: `#[cfg_attr(doc, doc = include_str!("../approx_const.rs"))]`
15+
16+
error: aborting due to 2 previous errors
17+

‎tests/ui/missing_doc_crate.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![warn(clippy::missing_docs_in_private_items)]
2+
#![allow(clippy::doc_include_without_cfg)]
23
#![doc = include_str!("../../README.md")]
34

45
fn main() {}

0 commit comments

Comments
 (0)