diff --git a/CHANGELOG.md b/CHANGELOG.md index cb149ccfeead..4909f15ecaea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5530,6 +5530,7 @@ Released 2018-09-13 [`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_code`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_link_code [`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 [`doc_nested_refdefs`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 1ce6f1256177..95dcd2c16482 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -139,6 +139,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::disallowed_types::DISALLOWED_TYPES_INFO, crate::doc::DOC_INCLUDE_WITHOUT_CFG_INFO, crate::doc::DOC_LAZY_CONTINUATION_INFO, + crate::doc::DOC_LINK_CODE_INFO, crate::doc::DOC_LINK_WITH_QUOTES_INFO, crate::doc::DOC_MARKDOWN_INFO, crate::doc::DOC_NESTED_REFDEFS_INFO, diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 15530c3dbc50..fc6d6c4282cd 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -84,6 +84,28 @@ declare_clippy_lint! { "presence of `_`, `::` or camel-case outside backticks in documentation" } +declare_clippy_lint! { + /// ### What it does + /// Checks for links with code directly adjacent to code text: + /// `` [`MyItem`]`<`[`u32`]`>` ``. + /// + /// ### Why is this bad? + /// It can be written more simply using HTML-style `` tags. + /// + /// ### Example + /// ```no_run + /// //! [`first`](x)`second` + /// ``` + /// Use instead: + /// ```no_run + /// //! [first](x)second + /// ``` + #[clippy::version = "1.86.0"] + pub DOC_LINK_CODE, + nursery, + "link with code back-to-back with other code" +} + declare_clippy_lint! { /// ### What it does /// Checks for the doc comments of publicly visible @@ -638,6 +660,7 @@ impl Documentation { } impl_lint_pass!(Documentation => [ + DOC_LINK_CODE, DOC_LINK_WITH_QUOTES, DOC_MARKDOWN, DOC_NESTED_REFDEFS, @@ -821,6 +844,21 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[ let mut cb = fake_broken_link_callback; + check_for_code_clusters( + cx, + pulldown_cmark::Parser::new_with_broken_link_callback( + &doc, + main_body_opts() - Options::ENABLE_SMART_PUNCTUATION, + Some(&mut cb), + ) + .into_offset_iter(), + &doc, + Fragments { + doc: &doc, + fragments: &fragments, + }, + ); + // disable smart punctuation to pick up ['link'] more easily let opts = main_body_opts() - Options::ENABLE_SMART_PUNCTUATION; let parser = pulldown_cmark::Parser::new_with_broken_link_callback(&doc, opts, Some(&mut cb)); @@ -844,6 +882,66 @@ enum Container { List(usize), } +/// Scan the documentation for code links that are back-to-back with code spans. +/// +/// This is done separately from the rest of the docs, because that makes it easier to produce +/// the correct messages. +fn check_for_code_clusters<'a, Events: Iterator, Range)>>( + cx: &LateContext<'_>, + events: Events, + doc: &str, + fragments: Fragments<'_>, +) { + let mut events = events.peekable(); + let mut code_starts_at = None; + let mut code_ends_at = None; + let mut code_includes_link = false; + while let Some((event, range)) = events.next() { + match event { + Start(Link { .. }) if matches!(events.peek(), Some((Code(_), _range))) => { + if code_starts_at.is_some() { + code_ends_at = Some(range.end); + } else { + code_starts_at = Some(range.start); + } + code_includes_link = true; + // skip the nested "code", because we're already handling it here + let _ = events.next(); + }, + Code(_) => { + if code_starts_at.is_some() { + code_ends_at = Some(range.end); + } else { + code_starts_at = Some(range.start); + } + }, + End(TagEnd::Link) => {}, + _ => { + if let Some(start) = code_starts_at + && let Some(end) = code_ends_at + && code_includes_link + { + if let Some(span) = fragments.span(cx, start..end) { + span_lint_and_then(cx, DOC_LINK_CODE, span, "code link adjacent to code text", |diag| { + let sugg = format!("{}", doc[start..end].replace('`', "")); + diag.span_suggestion_verbose( + span, + "wrap the entire group in `` tags", + sugg, + Applicability::MaybeIncorrect, + ); + diag.help("separate code snippets will be shown with a gap"); + }); + } + } + code_includes_link = false; + code_starts_at = None; + code_ends_at = None; + }, + } + } +} + /// Checks parsed documentation. /// This walks the "events" (think sections of markdown) produced by `pulldown_cmark`, /// so lints here will generally access that information. diff --git a/tests/ui/doc/link_adjacent.fixed b/tests/ui/doc/link_adjacent.fixed new file mode 100644 index 000000000000..0ac297a6b197 --- /dev/null +++ b/tests/ui/doc/link_adjacent.fixed @@ -0,0 +1,52 @@ +#![warn(clippy::doc_link_code)] + +//! Test case for code links that are adjacent to code text. +//! +//! This is not an example: `first``second` +//! +//! Neither is this: [`first`](x) +//! +//! Neither is this: [`first`](x) `second` +//! +//! Neither is this: [first](x)`second` +//! +//! This is: [first](x)second +//~^ ERROR: adjacent +//! +//! So is this first[second](x) +//~^ ERROR: adjacent +//! +//! So is this [first](x)[second](x) +//~^ ERROR: adjacent +//! +//! So is this [first](x)[second](x)[third](x) +//~^ ERROR: adjacent +//! +//! So is this [first](x)second[third](x) +//~^ ERROR: adjacent + +/// Test case for code links that are adjacent to code text. +/// +/// This is not an example: `first``second` arst +/// +/// Neither is this: [`first`](x) arst +/// +/// Neither is this: [`first`](x) `second` arst +/// +/// Neither is this: [first](x)`second` arst +/// +/// This is: [first](x)second arst +//~^ ERROR: adjacent +/// +/// So is this first[second](x) arst +//~^ ERROR: adjacent +/// +/// So is this [first](x)[second](x) arst +//~^ ERROR: adjacent +/// +/// So is this [first](x)[second](x)[third](x) arst +//~^ ERROR: adjacent +/// +/// So is this [first](x)second[third](x) arst +//~^ ERROR: adjacent +pub struct WithTrailing; diff --git a/tests/ui/doc/link_adjacent.rs b/tests/ui/doc/link_adjacent.rs new file mode 100644 index 000000000000..af6755eeff61 --- /dev/null +++ b/tests/ui/doc/link_adjacent.rs @@ -0,0 +1,52 @@ +#![warn(clippy::doc_link_code)] + +//! Test case for code links that are adjacent to code text. +//! +//! This is not an example: `first``second` +//! +//! Neither is this: [`first`](x) +//! +//! Neither is this: [`first`](x) `second` +//! +//! Neither is this: [first](x)`second` +//! +//! This is: [`first`](x)`second` +//~^ ERROR: adjacent +//! +//! So is this `first`[`second`](x) +//~^ ERROR: adjacent +//! +//! So is this [`first`](x)[`second`](x) +//~^ ERROR: adjacent +//! +//! So is this [`first`](x)[`second`](x)[`third`](x) +//~^ ERROR: adjacent +//! +//! So is this [`first`](x)`second`[`third`](x) +//~^ ERROR: adjacent + +/// Test case for code links that are adjacent to code text. +/// +/// This is not an example: `first``second` arst +/// +/// Neither is this: [`first`](x) arst +/// +/// Neither is this: [`first`](x) `second` arst +/// +/// Neither is this: [first](x)`second` arst +/// +/// This is: [`first`](x)`second` arst +//~^ ERROR: adjacent +/// +/// So is this `first`[`second`](x) arst +//~^ ERROR: adjacent +/// +/// So is this [`first`](x)[`second`](x) arst +//~^ ERROR: adjacent +/// +/// So is this [`first`](x)[`second`](x)[`third`](x) arst +//~^ ERROR: adjacent +/// +/// So is this [`first`](x)`second`[`third`](x) arst +//~^ ERROR: adjacent +pub struct WithTrailing; diff --git a/tests/ui/doc/link_adjacent.stderr b/tests/ui/doc/link_adjacent.stderr new file mode 100644 index 000000000000..f09762fb6a0b --- /dev/null +++ b/tests/ui/doc/link_adjacent.stderr @@ -0,0 +1,124 @@ +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:13:14 + | +LL | //! This is: [`first`](x)`second` + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap + = note: `-D clippy::doc-link-code` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::doc_link_code)]` +help: wrap the entire group in `` tags + | +LL | //! This is: [first](x)second + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:16:16 + | +LL | //! So is this `first`[`second`](x) + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap +help: wrap the entire group in `` tags + | +LL | //! So is this first[second](x) + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:19:16 + | +LL | //! So is this [`first`](x)[`second`](x) + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap +help: wrap the entire group in `` tags + | +LL | //! So is this [first](x)[second](x) + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:22:16 + | +LL | //! So is this [`first`](x)[`second`](x)[`third`](x) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap +help: wrap the entire group in `` tags + | +LL | //! So is this [first](x)[second](x)[third](x) + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:25:16 + | +LL | //! So is this [`first`](x)`second`[`third`](x) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap +help: wrap the entire group in `` tags + | +LL | //! So is this [first](x)second[third](x) + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:38:14 + | +LL | /// This is: [`first`](x)`second` arst + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap +help: wrap the entire group in `` tags + | +LL | /// This is: [first](x)second arst + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:41:16 + | +LL | /// So is this `first`[`second`](x) arst + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap +help: wrap the entire group in `` tags + | +LL | /// So is this first[second](x) arst + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:44:16 + | +LL | /// So is this [`first`](x)[`second`](x) arst + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap +help: wrap the entire group in `` tags + | +LL | /// So is this [first](x)[second](x) arst + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:47:16 + | +LL | /// So is this [`first`](x)[`second`](x)[`third`](x) arst + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap +help: wrap the entire group in `` tags + | +LL | /// So is this [first](x)[second](x)[third](x) arst + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:50:16 + | +LL | /// So is this [`first`](x)`second`[`third`](x) arst + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap +help: wrap the entire group in `` tags + | +LL | /// So is this [first](x)second[third](x) arst + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 10 previous errors +