Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new lint doc_include_without_cfg #13625

Merged
merged 1 commit into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion book/src/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
45 changes: 45 additions & 0 deletions clippy_lints/src/doc/include_in_doc_without_cfg.rs
Original file line number Diff line number Diff line change
@@ -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)
blyxyas marked this conversation as resolved.
Show resolved Hide resolved
// We cannot remove this because a `#[doc = include_str!("...")]` attribute can occupy
// several lines.
&& let Some(start) = snippet.find('[')
&& let Some(end) = snippet.rfind(']')
blyxyas marked this conversation as resolved.
Show resolved Hide resolved
&& 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,
);
}
}
}
28 changes: 28 additions & 0 deletions clippy_lints/src/doc/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::lint_without_lint_pass)]

mod lazy_continuation;
mod too_long_first_doc_paragraph;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down
40 changes: 40 additions & 0 deletions tests/ui/doc/doc_include_without_cfg.fixed
Original file line number Diff line number Diff line change
@@ -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
}
40 changes: 40 additions & 0 deletions tests/ui/doc/doc_include_without_cfg.rs
Original file line number Diff line number Diff line change
@@ -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
}
17 changes: 17 additions & 0 deletions tests/ui/doc/doc_include_without_cfg.stderr
Original file line number Diff line number Diff line change
@@ -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

1 change: 1 addition & 0 deletions tests/ui/missing_doc_crate.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![warn(clippy::missing_docs_in_private_items)]
#![allow(clippy::doc_include_without_cfg)]
#![doc = include_str!("../../README.md")]

fn main() {}