diff --git a/clippy_lints/src/doc/doc_link_code.rs b/clippy_lints/src/doc/doc_link_code.rs new file mode 100644 index 000000000000..95499c5c6353 --- /dev/null +++ b/clippy_lints/src/doc/doc_link_code.rs @@ -0,0 +1,90 @@ +use std::mem; +use std::ops::Range; + +use clippy_utils::diagnostics::span_lint_and_then; +use rustc_errors::Applicability; +use rustc_lint::LateContext; +use rustc_resolve::rustdoc::pulldown_cmark::{Event, Tag, TagEnd}; + +use crate::doc::Fragments; + +use super::DOC_LINK_CODE; + +struct PendingLink { + range: Range, + seen_code: bool, +} + +#[derive(Default)] +pub(super) struct LinkCode { + start: Option, + end: Option, + includes_link: bool, + pending_link: Option, +} + +impl LinkCode { + pub fn check( + &mut self, + cx: &LateContext<'_>, + event: &Event<'_>, + range: Range, + doc: &str, + fragments: Fragments<'_>, + ) { + match event { + Event::Start(Tag::Link { .. }) => { + self.pending_link = Some(PendingLink { + range, + seen_code: false, + }); + }, + Event::End(TagEnd::Link) => { + if let Some(PendingLink { range, seen_code: true }) = self.pending_link.take() { + if self.start.is_some() { + self.end = Some(range.end); + } else { + self.start = Some(range.start); + } + self.includes_link = true; + } + }, + _ if let Some(pending_link) = &mut self.pending_link => { + if matches!(event, Event::Code(_)) && !pending_link.seen_code { + pending_link.seen_code = true; + } else { + self.consume(cx, fragments, doc); + } + }, + Event::Code(_) => { + if self.start.is_some() { + self.end = Some(range.end); + } else { + self.start = Some(range.start); + } + }, + _ => self.consume(cx, fragments, doc), + } + } + + fn consume(&mut self, cx: &LateContext<'_>, fragments: Fragments<'_>, doc: &str) { + if let LinkCode { + start: Some(start), + end: Some(end), + includes_link: true, + pending_link: _, + } = mem::take(self) + && let Some(span) = fragments.span(cx, start..end) + { + span_lint_and_then(cx, DOC_LINK_CODE, span, "code link adjacent to code text", |diag| { + diag.span_suggestion_verbose( + span, + "wrap the entire group in `` tags", + format!("{}", doc[start..end].replace('`', "")), + Applicability::MaybeIncorrect, + ); + diag.help("separate code snippets will be shown with a gap"); + }); + } + } +} diff --git a/clippy_lints/src/doc/doc_paragraphs_missing_punctuation.rs b/clippy_lints/src/doc/doc_paragraphs_missing_punctuation.rs index a8f734637672..5ff1b8755025 100644 --- a/clippy_lints/src/doc/doc_paragraphs_missing_punctuation.rs +++ b/clippy_lints/src/doc/doc_paragraphs_missing_punctuation.rs @@ -1,62 +1,31 @@ +use clippy_utils::diagnostics::span_lint_and_then; use rustc_errors::Applicability; use rustc_lint::LateContext; -use rustc_resolve::rustdoc::main_body_opts; - -use rustc_resolve::rustdoc::pulldown_cmark::{Event, Options, Parser, Tag, TagEnd}; +use rustc_resolve::rustdoc::pulldown_cmark::{Event, Tag, TagEnd}; +use rustc_span::Span; +use std::ops::Range; use super::{DOC_PARAGRAPHS_MISSING_PUNCTUATION, Fragments}; -const MSG: &str = "doc paragraphs should end with a terminal punctuation mark"; -const PUNCTUATION_SUGGESTION: char = '.'; - -pub fn check(cx: &LateContext<'_>, doc: &str, fragments: Fragments<'_>) { - for missing_punctuation in is_missing_punctuation(doc) { - match missing_punctuation { - MissingPunctuation::Fixable(offset) => { - // This ignores `#[doc]` attributes, which we do not handle. - if let Some(span) = fragments.span(cx, offset..offset) { - clippy_utils::diagnostics::span_lint_and_sugg( - cx, - DOC_PARAGRAPHS_MISSING_PUNCTUATION, - span, - MSG, - "end the paragraph with some punctuation", - PUNCTUATION_SUGGESTION.to_string(), - Applicability::MaybeIncorrect, - ); - } - }, - MissingPunctuation::Unfixable(offset) => { - // This ignores `#[doc]` attributes, which we do not handle. - if let Some(span) = fragments.span(cx, offset..offset) { - clippy_utils::diagnostics::span_lint_and_help( - cx, - DOC_PARAGRAPHS_MISSING_PUNCTUATION, - span, - MSG, - None, - "end the paragraph with some punctuation", - ); - } - }, - } - } +#[derive(Default)] +pub(super) struct MissingPunctuation { + no_report_depth: u32, + current_paragraph: Option, } -#[must_use] -/// If punctuation is missing, returns the offset where new punctuation should be inserted. -fn is_missing_punctuation(doc_string: &str) -> Vec { - // The colon is not exactly a terminal punctuation mark, but this is required for paragraphs that - // introduce a table or a list for example. - const TERMINAL_PUNCTUATION_MARKS: &[char] = &['.', '?', '!', '…', ':']; +impl MissingPunctuation { + pub fn check( + &mut self, + cx: &LateContext<'_>, + event: &Event<'_>, + range: Range, + doc: &str, + fragments: Fragments<'_>, + ) { + // The colon is not exactly a terminal punctuation mark, but this is required for paragraphs that + // introduce a table or a list for example. + const TERMINAL_PUNCTUATION_MARKS: &[char] = &['.', '?', '!', '…', ':']; - let mut no_report_depth = 0; - let mut missing_punctuation = Vec::new(); - let mut current_paragraph = None; - - for (event, offset) in - Parser::new_ext(doc_string, main_body_opts() - Options::ENABLE_SMART_PUNCTUATION).into_offset_iter() - { match event { Event::Start( Tag::CodeBlock(..) @@ -66,61 +35,82 @@ fn is_missing_punctuation(doc_string: &str) -> Vec { | Tag::List(..) | Tag::Table(_), ) => { - no_report_depth += 1; + self.no_report_depth += 1; }, Event::End(TagEnd::FootnoteDefinition) => { - no_report_depth -= 1; + self.no_report_depth -= 1; }, Event::End( TagEnd::CodeBlock | TagEnd::Heading(_) | TagEnd::HtmlBlock | TagEnd::List(_) | TagEnd::Table, ) => { - no_report_depth -= 1; - current_paragraph = None; + self.no_report_depth -= 1; + self.current_paragraph = None; }, Event::InlineHtml(_) | Event::Start(Tag::Image { .. }) | Event::End(TagEnd::Image) => { - current_paragraph = None; + self.current_paragraph = None; }, Event::End(TagEnd::Paragraph) => { - if let Some(mp) = current_paragraph { - missing_punctuation.push(mp); + if let Some(position) = self.current_paragraph + && let Some(span) = position.span(cx, fragments) + { + span_lint_and_then( + cx, + DOC_PARAGRAPHS_MISSING_PUNCTUATION, + span, + "doc paragraphs should end with a terminal punctuation mark", + |diag| { + if matches!(position, Position::Fixable(_)) { + diag.span_suggestion( + span, + "end the paragraph with some punctuation", + '.', + Applicability::MaybeIncorrect, + ); + } else { + diag.help("end the paragraph with some punctuation"); + } + }, + ); } }, Event::Code(..) | Event::Start(Tag::Link { .. }) | Event::End(TagEnd::Link) - if no_report_depth == 0 && !offset.is_empty() => + if self.no_report_depth == 0 && !range.is_empty() => { - if doc_string[..offset.end] - .trim_end() - .ends_with(TERMINAL_PUNCTUATION_MARKS) - { - current_paragraph = None; + if doc[..range.end].trim_end().ends_with(TERMINAL_PUNCTUATION_MARKS) { + self.current_paragraph = None; } else { - current_paragraph = Some(MissingPunctuation::Fixable(offset.end)); + self.current_paragraph = Some(Position::Fixable(range.end)); } }, - Event::Text(..) if no_report_depth == 0 && !offset.is_empty() => { - let trimmed = doc_string[..offset.end].trim_end(); + Event::Text(..) if self.no_report_depth == 0 && !range.is_empty() => { + let trimmed = doc[..range.end].trim_end(); if trimmed.ends_with(TERMINAL_PUNCTUATION_MARKS) { - current_paragraph = None; + self.current_paragraph = None; } else if let Some(t) = trimmed.strip_suffix(|c| c == ')' || c == '"') { if t.ends_with(TERMINAL_PUNCTUATION_MARKS) { // Avoid false positives. - current_paragraph = None; + self.current_paragraph = None; } else { - current_paragraph = Some(MissingPunctuation::Unfixable(offset.end)); + self.current_paragraph = Some(Position::Unfixable(range.end)); } } else { - current_paragraph = Some(MissingPunctuation::Fixable(offset.end)); + self.current_paragraph = Some(Position::Fixable(range.end)); } }, _ => {}, } } - - missing_punctuation } #[derive(Debug, Copy, Clone, PartialEq, Eq)] -enum MissingPunctuation { +enum Position { Fixable(usize), Unfixable(usize), } + +impl Position { + fn span(self, cx: &LateContext<'_>, fragments: Fragments<'_>) -> Option { + let (Position::Fixable(pos) | Position::Unfixable(pos)) = self; + fragments.span(cx, pos..pos) + } +} diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 120da92da944..296acf79ebb9 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -25,8 +25,12 @@ use rustc_span::Span; use std::ops::Range; use url::Url; +use doc_link_code::LinkCode; +use doc_paragraphs_missing_punctuation::MissingPunctuation; + mod broken_link; mod doc_comment_double_space_linebreaks; +mod doc_link_code; mod doc_paragraphs_missing_punctuation; mod doc_suspicious_footnotes; mod include_in_doc_without_cfg; @@ -847,14 +851,6 @@ struct DocHeaders { /// back in the various late lint pass methods if they need the final doc headers, like "Safety" or /// "Panics" sections. fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[Attribute]) -> Option { - // We don't want the parser to choke on intra doc links. Since we don't - // actually care about rendering them, just pretend that all broken links - // point to a fake address. - #[expect(clippy::unnecessary_wraps)] // we're following a type signature - fn fake_broken_link_callback<'a>(_: BrokenLink<'_>) -> Option<(CowStr<'a>, CowStr<'a>)> { - Some(("fake".into(), "fake".into())) - } - if suspicious_doc_comments::check(cx, attrs) || is_doc_hidden(attrs) { return None; } @@ -889,41 +885,16 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[ return Some(DocHeaders::default()); } - check_for_code_clusters( - cx, - pulldown_cmark::Parser::new_with_broken_link_callback( - &doc, - main_body_opts() - Options::ENABLE_SMART_PUNCTUATION, - Some(&mut fake_broken_link_callback), - ) - .into_offset_iter(), - &doc, - Fragments { - doc: &doc, - fragments: &fragments, - }, - ); - - doc_paragraphs_missing_punctuation::check( - cx, - &doc, - Fragments { - doc: &doc, - fragments: &fragments, - }, - ); - // NOTE: check_doc uses it own cb function, // to avoid causing duplicated diagnostics for the broken link checker. - let mut full_fake_broken_link_callback = |bl: BrokenLink<'_>| -> Option<(CowStr<'_>, CowStr<'_>)> { + let mut broken_link_callback = |bl: BrokenLink<'_>| -> Option<(CowStr<'_>, CowStr<'_>)> { broken_link::check(cx, &bl, &doc, &fragments); Some(("fake".into(), "fake".into())) }; // 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 full_fake_broken_link_callback)); + let parser = pulldown_cmark::Parser::new_with_broken_link_callback(&doc, opts, Some(&mut broken_link_callback)); Some(check_doc( cx, @@ -943,65 +914,6 @@ 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 - && 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; - }, - } - } -} - #[derive(Clone, Copy)] #[expect(clippy::struct_excessive_bools)] struct CodeTags { @@ -1079,6 +991,9 @@ fn check_doc<'a, Events: Iterator, Range, attrs: &[Attribute], ) -> DocHeaders { + let mut missing_punctuation = MissingPunctuation::default(); + let mut link_code = LinkCode::default(); + // true if a safety header was found let mut headers = DocHeaders::default(); let mut code = None; @@ -1098,6 +1013,9 @@ fn check_doc<'a, Events: Iterator, Range { if tag.starts_with("[first](x)second //~^ ERROR: adjacent //! @@ -24,6 +28,9 @@ //! //! So is this [first](x)second[third](x) //~^ ERROR: adjacent +//! +//! first[second](x)[third](x) +//~^ ERROR: adjacent /// Test case for code links that are adjacent to code text. /// @@ -35,6 +42,10 @@ /// /// Neither is this: [first](x)`second` arst /// +/// Neither is this: `first`[](x) arst +/// +/// Neither is this: `first`[`second` not code](x) arst +/// /// This is: [first](x)second arst //~^ ERROR: adjacent /// @@ -49,4 +60,7 @@ /// /// So is this [first](x)second[third](x) arst //~^ ERROR: adjacent +/// +/// first[second](x)[third](x) arst +//~^ ERROR: adjacent pub struct WithTrailing; diff --git a/tests/ui/doc/link_adjacent.rs b/tests/ui/doc/link_adjacent.rs index af6755eeff61..0642e43cd06d 100644 --- a/tests/ui/doc/link_adjacent.rs +++ b/tests/ui/doc/link_adjacent.rs @@ -10,6 +10,10 @@ //! //! Neither is this: [first](x)`second` //! +//! Neither is this: `first`[](x) +//! +//! Neither is this: `first`[`second` not code](x) +//! //! This is: [`first`](x)`second` //~^ ERROR: adjacent //! @@ -24,6 +28,9 @@ //! //! So is this [`first`](x)`second`[`third`](x) //~^ ERROR: adjacent +//! +//! `first`[`second`](x)[third](x) +//~^ ERROR: adjacent /// Test case for code links that are adjacent to code text. /// @@ -35,6 +42,10 @@ /// /// Neither is this: [first](x)`second` arst /// +/// Neither is this: `first`[](x) arst +/// +/// Neither is this: `first`[`second` not code](x) arst +/// /// This is: [`first`](x)`second` arst //~^ ERROR: adjacent /// @@ -49,4 +60,7 @@ /// /// So is this [`first`](x)`second`[`third`](x) arst //~^ ERROR: adjacent +/// +/// `first`[`second`](x)[third](x) arst +//~^ ERROR: adjacent pub struct WithTrailing; diff --git a/tests/ui/doc/link_adjacent.stderr b/tests/ui/doc/link_adjacent.stderr index 27e8a73c7083..fae642e19166 100644 --- a/tests/ui/doc/link_adjacent.stderr +++ b/tests/ui/doc/link_adjacent.stderr @@ -1,5 +1,5 @@ error: code link adjacent to code text - --> tests/ui/doc/link_adjacent.rs:13:14 + --> tests/ui/doc/link_adjacent.rs:17:14 | LL | //! This is: [`first`](x)`second` | ^^^^^^^^^^^^^^^^^^^^ @@ -14,7 +14,7 @@ LL + //! This is: [first](x)second | error: code link adjacent to code text - --> tests/ui/doc/link_adjacent.rs:16:16 + --> tests/ui/doc/link_adjacent.rs:20:16 | LL | //! So is this `first`[`second`](x) | ^^^^^^^^^^^^^^^^^^^^ @@ -27,7 +27,7 @@ LL + //! So is this first[second](x) | error: code link adjacent to code text - --> tests/ui/doc/link_adjacent.rs:19:16 + --> tests/ui/doc/link_adjacent.rs:23:16 | LL | //! So is this [`first`](x)[`second`](x) | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -40,7 +40,7 @@ LL + //! So is this [first](x)[second](x) | error: code link adjacent to code text - --> tests/ui/doc/link_adjacent.rs:22:16 + --> tests/ui/doc/link_adjacent.rs:26:16 | LL | //! So is this [`first`](x)[`second`](x)[`third`](x) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -53,7 +53,7 @@ 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 + --> tests/ui/doc/link_adjacent.rs:29:16 | LL | //! So is this [`first`](x)`second`[`third`](x) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -66,7 +66,20 @@ LL + //! So is this [first](x)second[third](x) | error: code link adjacent to code text - --> tests/ui/doc/link_adjacent.rs:38:14 + --> tests/ui/doc/link_adjacent.rs:32:5 + | +LL | //! `first`[`second`](x)[third](x) + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap +help: wrap the entire group in `` tags + | +LL - //! `first`[`second`](x)[third](x) +LL + //! first[second](x)[third](x) + | + +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:49:14 | LL | /// This is: [`first`](x)`second` arst | ^^^^^^^^^^^^^^^^^^^^ @@ -79,7 +92,7 @@ LL + /// This is: [first](x)second arst | error: code link adjacent to code text - --> tests/ui/doc/link_adjacent.rs:41:16 + --> tests/ui/doc/link_adjacent.rs:52:16 | LL | /// So is this `first`[`second`](x) arst | ^^^^^^^^^^^^^^^^^^^^ @@ -92,7 +105,7 @@ LL + /// So is this first[second](x) arst | error: code link adjacent to code text - --> tests/ui/doc/link_adjacent.rs:44:16 + --> tests/ui/doc/link_adjacent.rs:55:16 | LL | /// So is this [`first`](x)[`second`](x) arst | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -105,7 +118,7 @@ LL + /// So is this [first](x)[second](x) arst | error: code link adjacent to code text - --> tests/ui/doc/link_adjacent.rs:47:16 + --> tests/ui/doc/link_adjacent.rs:58:16 | LL | /// So is this [`first`](x)[`second`](x)[`third`](x) arst | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -118,7 +131,7 @@ 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 + --> tests/ui/doc/link_adjacent.rs:61:16 | LL | /// So is this [`first`](x)`second`[`third`](x) arst | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -130,5 +143,18 @@ LL - /// So is this [`first`](x)`second`[`third`](x) arst LL + /// So is this [first](x)second[third](x) arst | -error: aborting due to 10 previous errors +error: code link adjacent to code text + --> tests/ui/doc/link_adjacent.rs:64:5 + | +LL | /// `first`[`second`](x)[third](x) arst + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: separate code snippets will be shown with a gap +help: wrap the entire group in `` tags + | +LL - /// `first`[`second`](x)[third](x) arst +LL + /// first[second](x)[third](x) arst + | + +error: aborting due to 12 previous errors