diff --git a/Cargo.lock b/Cargo.lock index 4cb2a584e6ab..295a5b43a6f3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -558,7 +558,6 @@ dependencies = [ "declare_clippy_lint", "if_chain", "itertools", - "pulldown-cmark", "quine-mc_cluskey", "regex", "regex-syntax 0.7.2", diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs index ba7417b6ddaf..7c41c32d0222 100644 --- a/compiler/rustc_resolve/src/rustdoc.rs +++ b/compiler/rustc_resolve/src/rustdoc.rs @@ -2,9 +2,11 @@ use pulldown_cmark::{BrokenLink, CowStr, Event, LinkType, Options, Parser, Tag}; use rustc_ast as ast; use rustc_ast::util::comments::beautify_doc_string; use rustc_data_structures::fx::FxHashMap; +use rustc_middle::ty::TyCtxt; use rustc_span::def_id::DefId; use rustc_span::symbol::{kw, sym, Symbol}; -use rustc_span::Span; +use rustc_span::{InnerSpan, Span, DUMMY_SP}; +use std::ops::Range; use std::{cmp, mem}; #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -462,3 +464,88 @@ fn collect_link_data<'input, 'callback>( display_text.map(String::into_boxed_str) } + +/// Returns a span encompassing all the document fragments. +pub fn span_of_fragments(fragments: &[DocFragment]) -> Option { + if fragments.is_empty() { + return None; + } + let start = fragments[0].span; + if start == DUMMY_SP { + return None; + } + let end = fragments.last().expect("no doc strings provided").span; + Some(start.to(end)) +} + +/// Attempts to match a range of bytes from parsed markdown to a `Span` in the source code. +/// +/// This method will return `None` if we cannot construct a span from the source map or if the +/// fragments are not all sugared doc comments. It's difficult to calculate the correct span in +/// that case due to escaping and other source features. +pub fn source_span_for_markdown_range( + tcx: TyCtxt<'_>, + markdown: &str, + md_range: &Range, + fragments: &[DocFragment], +) -> Option { + let is_all_sugared_doc = fragments.iter().all(|frag| frag.kind == DocFragmentKind::SugaredDoc); + + if !is_all_sugared_doc { + return None; + } + + let snippet = tcx.sess.source_map().span_to_snippet(span_of_fragments(fragments)?).ok()?; + + let starting_line = markdown[..md_range.start].matches('\n').count(); + let ending_line = starting_line + markdown[md_range.start..md_range.end].matches('\n').count(); + + // We use `split_terminator('\n')` instead of `lines()` when counting bytes so that we treat + // CRLF and LF line endings the same way. + let mut src_lines = snippet.split_terminator('\n'); + let md_lines = markdown.split_terminator('\n'); + + // The number of bytes from the source span to the markdown span that are not part + // of the markdown, like comment markers. + let mut start_bytes = 0; + let mut end_bytes = 0; + + 'outer: for (line_no, md_line) in md_lines.enumerate() { + loop { + let source_line = src_lines.next()?; + match source_line.find(md_line) { + Some(offset) => { + if line_no == starting_line { + start_bytes += offset; + + if starting_line == ending_line { + break 'outer; + } + } else if line_no == ending_line { + end_bytes += offset; + break 'outer; + } else if line_no < starting_line { + start_bytes += source_line.len() - md_line.len(); + } else { + end_bytes += source_line.len() - md_line.len(); + } + break; + } + None => { + // Since this is a source line that doesn't include a markdown line, + // we have to count the newline that we split from earlier. + if line_no <= starting_line { + start_bytes += source_line.len() + 1; + } else { + end_bytes += source_line.len() + 1; + } + } + } + } + } + + Some(span_of_fragments(fragments)?.from_inner(InnerSpan::new( + md_range.start + start_bytes, + md_range.end + start_bytes + end_bytes, + ))) +} diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index ff158fa8b4cb..b276745f3170 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -25,7 +25,9 @@ use rustc_index::IndexVec; use rustc_metadata::rendered_const; use rustc_middle::ty::fast_reject::SimplifiedType; use rustc_middle::ty::{self, TyCtxt, Visibility}; -use rustc_resolve::rustdoc::{add_doc_fragment, attrs_to_doc_fragments, inner_docs, DocFragment}; +use rustc_resolve::rustdoc::{ + add_doc_fragment, attrs_to_doc_fragments, inner_docs, span_of_fragments, DocFragment, +}; use rustc_session::Session; use rustc_span::hygiene::MacroKind; use rustc_span::symbol::{kw, sym, Ident, Symbol}; @@ -397,7 +399,7 @@ impl Item { } pub(crate) fn attr_span(&self, tcx: TyCtxt<'_>) -> rustc_span::Span { - crate::passes::span_of_attrs(&self.attrs) + span_of_fragments(&self.attrs.doc_strings) .unwrap_or_else(|| self.span(tcx).map_or(rustc_span::DUMMY_SP, |span| span.inner())) } diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index bb7d26c541b7..ea87268877fb 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -10,6 +10,7 @@ use rustc_middle::hir::nested_filter; use rustc_middle::ty::TyCtxt; use rustc_parse::maybe_new_parser_from_source_str; use rustc_parse::parser::attr::InnerAttrPolicy; +use rustc_resolve::rustdoc::span_of_fragments; use rustc_session::config::{self, CrateType, ErrorOutputType}; use rustc_session::parse::ParseSess; use rustc_session::{lint, EarlyErrorHandler, Session}; @@ -33,7 +34,6 @@ use crate::clean::{types::AttributesExt, Attributes}; use crate::config::Options as RustdocOptions; use crate::html::markdown::{self, ErrorCodes, Ignore, LangString}; use crate::lint::init_lints; -use crate::passes::span_of_attrs; /// Options that apply to all doctests in a crate or Markdown file (for `rustdoc foo.md`). #[derive(Clone, Default)] @@ -1241,7 +1241,7 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> { Some(&crate::html::markdown::ExtraInfo::new( self.tcx, def_id.to_def_id(), - span_of_attrs(&attrs).unwrap_or(sp), + span_of_fragments(&attrs.doc_strings).unwrap_or(sp), )), ); } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 7b0a7a90d316..0db846bfc7e3 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -16,7 +16,9 @@ use rustc_hir::Mutability; use rustc_middle::ty::{Ty, TyCtxt}; use rustc_middle::{bug, span_bug, ty}; use rustc_resolve::rustdoc::{has_primitive_or_keyword_docs, prepare_to_doc_link_resolution}; -use rustc_resolve::rustdoc::{strip_generics_from_path, MalformedGenerics}; +use rustc_resolve::rustdoc::{ + source_span_for_markdown_range, strip_generics_from_path, MalformedGenerics, +}; use rustc_session::lint::Lint; use rustc_span::hygiene::MacroKind; use rustc_span::symbol::{sym, Ident, Symbol}; @@ -1211,11 +1213,11 @@ impl LinkCollector<'_, '_> { ori_link: &MarkdownLinkRange, item: &Item, ) { - let span = super::source_span_for_markdown_range( + let span = source_span_for_markdown_range( self.cx.tcx, dox, ori_link.inner_range(), - &item.attrs, + &item.attrs.doc_strings, ) .unwrap_or_else(|| item.attr_span(self.cx.tcx)); rustc_session::parse::feature_err( @@ -1702,26 +1704,27 @@ fn report_diagnostic( let (span, link_range) = match link_range { MarkdownLinkRange::Destination(md_range) => { let mut md_range = md_range.clone(); - let sp = super::source_span_for_markdown_range(tcx, dox, &md_range, &item.attrs) - .map(|mut sp| { - while dox.as_bytes().get(md_range.start) == Some(&b' ') - || dox.as_bytes().get(md_range.start) == Some(&b'`') - { - md_range.start += 1; - sp = sp.with_lo(sp.lo() + BytePos(1)); - } - while dox.as_bytes().get(md_range.end - 1) == Some(&b' ') - || dox.as_bytes().get(md_range.end - 1) == Some(&b'`') - { - md_range.end -= 1; - sp = sp.with_hi(sp.hi() - BytePos(1)); - } - sp - }); + let sp = + source_span_for_markdown_range(tcx, dox, &md_range, &item.attrs.doc_strings) + .map(|mut sp| { + while dox.as_bytes().get(md_range.start) == Some(&b' ') + || dox.as_bytes().get(md_range.start) == Some(&b'`') + { + md_range.start += 1; + sp = sp.with_lo(sp.lo() + BytePos(1)); + } + while dox.as_bytes().get(md_range.end - 1) == Some(&b' ') + || dox.as_bytes().get(md_range.end - 1) == Some(&b'`') + { + md_range.end -= 1; + sp = sp.with_hi(sp.hi() - BytePos(1)); + } + sp + }); (sp, MarkdownLinkRange::Destination(md_range)) } MarkdownLinkRange::WholeLink(md_range) => ( - super::source_span_for_markdown_range(tcx, dox, &md_range, &item.attrs), + source_span_for_markdown_range(tcx, dox, &md_range, &item.attrs.doc_strings), link_range.clone(), ), }; diff --git a/src/librustdoc/passes/lint/bare_urls.rs b/src/librustdoc/passes/lint/bare_urls.rs index 97078a5cb161..0c5cfffe1be5 100644 --- a/src/librustdoc/passes/lint/bare_urls.rs +++ b/src/librustdoc/passes/lint/bare_urls.rs @@ -4,11 +4,11 @@ use crate::clean::*; use crate::core::DocContext; use crate::html::markdown::main_body_opts; -use crate::passes::source_span_for_markdown_range; use core::ops::Range; use pulldown_cmark::{Event, Parser, Tag}; use regex::Regex; use rustc_errors::Applicability; +use rustc_resolve::rustdoc::source_span_for_markdown_range; use std::mem; use std::sync::LazyLock; @@ -21,8 +21,9 @@ pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item) { if !dox.is_empty() { let report_diag = |cx: &DocContext<'_>, msg: &'static str, url: &str, range: Range| { - let sp = source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs) - .unwrap_or_else(|| item.attr_span(cx.tcx)); + let sp = + source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs.doc_strings) + .unwrap_or_else(|| item.attr_span(cx.tcx)); cx.tcx.struct_span_lint_hir(crate::lint::BARE_URLS, hir_id, sp, msg, |lint| { lint.note("bare URLs are not automatically turned into clickable links") .span_suggestion( diff --git a/src/librustdoc/passes/lint/check_code_block_syntax.rs b/src/librustdoc/passes/lint/check_code_block_syntax.rs index 37e28e1fbcab..316b1a41c7da 100644 --- a/src/librustdoc/passes/lint/check_code_block_syntax.rs +++ b/src/librustdoc/passes/lint/check_code_block_syntax.rs @@ -6,6 +6,7 @@ use rustc_errors::{ Applicability, Diagnostic, Handler, LazyFallbackBundle, }; use rustc_parse::parse_stream_from_source_str; +use rustc_resolve::rustdoc::source_span_for_markdown_range; use rustc_session::parse::ParseSess; use rustc_span::hygiene::{AstPass, ExpnData, ExpnKind, LocalExpnId}; use rustc_span::source_map::{FilePathMapping, SourceMap}; @@ -14,7 +15,6 @@ use rustc_span::{FileName, InnerSpan, DUMMY_SP}; use crate::clean; use crate::core::DocContext; use crate::html::markdown::{self, RustCodeBlock}; -use crate::passes::source_span_for_markdown_range; pub(crate) fn visit_item(cx: &DocContext<'_>, item: &clean::Item) { if let Some(dox) = &item.opt_doc_value() { @@ -77,11 +77,15 @@ fn check_rust_syntax( let is_ignore = code_block.lang_string.ignore != markdown::Ignore::None; // The span and whether it is precise or not. - let (sp, precise_span) = - match source_span_for_markdown_range(cx.tcx, dox, &code_block.range, &item.attrs) { - Some(sp) => (sp, true), - None => (item.attr_span(cx.tcx), false), - }; + let (sp, precise_span) = match source_span_for_markdown_range( + cx.tcx, + dox, + &code_block.range, + &item.attrs.doc_strings, + ) { + Some(sp) => (sp, true), + None => (item.attr_span(cx.tcx), false), + }; let msg = if buffer.has_errors { "could not parse code block as Rust code" diff --git a/src/librustdoc/passes/lint/html_tags.rs b/src/librustdoc/passes/lint/html_tags.rs index c135c584cec4..79fc599e1763 100644 --- a/src/librustdoc/passes/lint/html_tags.rs +++ b/src/librustdoc/passes/lint/html_tags.rs @@ -2,9 +2,9 @@ use crate::clean::*; use crate::core::DocContext; use crate::html::markdown::main_body_opts; -use crate::passes::source_span_for_markdown_range; use pulldown_cmark::{BrokenLink, Event, LinkType, Parser, Tag}; +use rustc_resolve::rustdoc::source_span_for_markdown_range; use std::iter::Peekable; use std::ops::Range; @@ -20,7 +20,8 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) { let dox = item.doc_value(); if !dox.is_empty() { let report_diag = |msg: String, range: &Range, is_open_tag: bool| { - let sp = match source_span_for_markdown_range(tcx, &dox, range, &item.attrs) { + let sp = match source_span_for_markdown_range(tcx, &dox, range, &item.attrs.doc_strings) + { Some(sp) => sp, None => item.attr_span(tcx), }; @@ -60,7 +61,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) { tcx, &dox, &(generics_start..generics_end), - &item.attrs, + &item.attrs.doc_strings, ) { Some(sp) => sp, None => item.attr_span(tcx), diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs index 67cd2cc9732b..0c15bf5f764f 100644 --- a/src/librustdoc/passes/lint/redundant_explicit_links.rs +++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs @@ -6,6 +6,7 @@ use rustc_errors::SuggestionStyle; use rustc_hir::def::{DefKind, DocLinkResMap, Namespace, Res}; use rustc_hir::HirId; use rustc_lint_defs::Applicability; +use rustc_resolve::rustdoc::source_span_for_markdown_range; use rustc_span::Symbol; use crate::clean::utils::find_nearest_parent_module; @@ -13,7 +14,6 @@ use crate::clean::utils::inherits_doc_hidden; use crate::clean::Item; use crate::core::DocContext; use crate::html::markdown::main_body_opts; -use crate::passes::source_span_for_markdown_range; #[derive(Debug)] struct LinkData { @@ -160,16 +160,21 @@ fn check_inline_or_reference_unknown_redundancy( (find_resolution(resolutions, &dest)?, find_resolution(resolutions, resolvable_link)?); if dest_res == display_res { - let link_span = source_span_for_markdown_range(cx.tcx, &doc, &link_range, &item.attrs) - .unwrap_or(item.attr_span(cx.tcx)); + let link_span = + source_span_for_markdown_range(cx.tcx, &doc, &link_range, &item.attrs.doc_strings) + .unwrap_or(item.attr_span(cx.tcx)); let explicit_span = source_span_for_markdown_range( cx.tcx, &doc, &offset_explicit_range(doc, link_range, open, close), - &item.attrs, + &item.attrs.doc_strings, + )?; + let display_span = source_span_for_markdown_range( + cx.tcx, + &doc, + &resolvable_link_range, + &item.attrs.doc_strings, )?; - let display_span = - source_span_for_markdown_range(cx.tcx, &doc, &resolvable_link_range, &item.attrs)?; cx.tcx.struct_span_lint_hir(crate::lint::REDUNDANT_EXPLICIT_LINKS, hir_id, explicit_span, "redundant explicit link target", |lint| { lint.span_label(explicit_span, "explicit target is redundant") @@ -201,21 +206,26 @@ fn check_reference_redundancy( (find_resolution(resolutions, &dest)?, find_resolution(resolutions, resolvable_link)?); if dest_res == display_res { - let link_span = source_span_for_markdown_range(cx.tcx, &doc, &link_range, &item.attrs) - .unwrap_or(item.attr_span(cx.tcx)); + let link_span = + source_span_for_markdown_range(cx.tcx, &doc, &link_range, &item.attrs.doc_strings) + .unwrap_or(item.attr_span(cx.tcx)); let explicit_span = source_span_for_markdown_range( cx.tcx, &doc, &offset_explicit_range(doc, link_range.clone(), b'[', b']'), - &item.attrs, + &item.attrs.doc_strings, + )?; + let display_span = source_span_for_markdown_range( + cx.tcx, + &doc, + &resolvable_link_range, + &item.attrs.doc_strings, )?; - let display_span = - source_span_for_markdown_range(cx.tcx, &doc, &resolvable_link_range, &item.attrs)?; let def_span = source_span_for_markdown_range( cx.tcx, &doc, &offset_reference_def_range(doc, dest, link_range), - &item.attrs, + &item.attrs.doc_strings, )?; cx.tcx.struct_span_lint_hir(crate::lint::REDUNDANT_EXPLICIT_LINKS, hir_id, explicit_span, "redundant explicit link target", |lint| { diff --git a/src/librustdoc/passes/lint/unescaped_backticks.rs b/src/librustdoc/passes/lint/unescaped_backticks.rs index 256958d71601..8b7fdd6ab1f9 100644 --- a/src/librustdoc/passes/lint/unescaped_backticks.rs +++ b/src/librustdoc/passes/lint/unescaped_backticks.rs @@ -3,10 +3,10 @@ use crate::clean::Item; use crate::core::DocContext; use crate::html::markdown::main_body_opts; -use crate::passes::source_span_for_markdown_range; use pulldown_cmark::{BrokenLink, Event, Parser}; use rustc_errors::DiagnosticBuilder; use rustc_lint_defs::Applicability; +use rustc_resolve::rustdoc::source_span_for_markdown_range; use std::ops::Range; pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) { @@ -52,7 +52,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) { tcx, &dox, &(backtick_index..backtick_index + 1), - &item.attrs, + &item.attrs.doc_strings, ) .unwrap_or_else(|| item.attr_span(tcx)); @@ -378,9 +378,12 @@ fn suggest_insertion( /// Maximum bytes of context to show around the insertion. const CONTEXT_MAX_LEN: usize = 80; - if let Some(span) = - source_span_for_markdown_range(cx.tcx, &dox, &(insert_index..insert_index), &item.attrs) - { + if let Some(span) = source_span_for_markdown_range( + cx.tcx, + &dox, + &(insert_index..insert_index), + &item.attrs.doc_strings, + ) { lint.span_suggestion(span, message, suggestion, Applicability::MaybeIncorrect); } else { let line_start = dox[..insert_index].rfind('\n').map_or(0, |idx| idx + 1); diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 4b1ff68df502..bb678e338888 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -1,11 +1,6 @@ //! Contains information about "passes", used to modify crate information during the documentation //! process. -use rustc_middle::ty::TyCtxt; -use rustc_resolve::rustdoc::DocFragmentKind; -use rustc_span::{InnerSpan, Span, DUMMY_SP}; -use std::ops::Range; - use self::Condition::*; use crate::clean; use crate::core::DocContext; @@ -115,89 +110,3 @@ impl ConditionalPass { pub(crate) fn defaults(show_coverage: bool) -> &'static [ConditionalPass] { if show_coverage { COVERAGE_PASSES } else { DEFAULT_PASSES } } - -/// Returns a span encompassing all the given attributes. -pub(crate) fn span_of_attrs(attrs: &clean::Attributes) -> Option { - if attrs.doc_strings.is_empty() { - return None; - } - let start = attrs.doc_strings[0].span; - if start == DUMMY_SP { - return None; - } - let end = attrs.doc_strings.last().expect("no doc strings provided").span; - Some(start.to(end)) -} - -/// Attempts to match a range of bytes from parsed markdown to a `Span` in the source code. -/// -/// This method will return `None` if we cannot construct a span from the source map or if the -/// attributes are not all sugared doc comments. It's difficult to calculate the correct span in -/// that case due to escaping and other source features. -pub(crate) fn source_span_for_markdown_range( - tcx: TyCtxt<'_>, - markdown: &str, - md_range: &Range, - attrs: &clean::Attributes, -) -> Option { - let is_all_sugared_doc = - attrs.doc_strings.iter().all(|frag| frag.kind == DocFragmentKind::SugaredDoc); - - if !is_all_sugared_doc { - return None; - } - - let snippet = tcx.sess.source_map().span_to_snippet(span_of_attrs(attrs)?).ok()?; - - let starting_line = markdown[..md_range.start].matches('\n').count(); - let ending_line = starting_line + markdown[md_range.start..md_range.end].matches('\n').count(); - - // We use `split_terminator('\n')` instead of `lines()` when counting bytes so that we treat - // CRLF and LF line endings the same way. - let mut src_lines = snippet.split_terminator('\n'); - let md_lines = markdown.split_terminator('\n'); - - // The number of bytes from the source span to the markdown span that are not part - // of the markdown, like comment markers. - let mut start_bytes = 0; - let mut end_bytes = 0; - - 'outer: for (line_no, md_line) in md_lines.enumerate() { - loop { - let source_line = src_lines.next()?; - match source_line.find(md_line) { - Some(offset) => { - if line_no == starting_line { - start_bytes += offset; - - if starting_line == ending_line { - break 'outer; - } - } else if line_no == ending_line { - end_bytes += offset; - break 'outer; - } else if line_no < starting_line { - start_bytes += source_line.len() - md_line.len(); - } else { - end_bytes += source_line.len() - md_line.len(); - } - break; - } - None => { - // Since this is a source line that doesn't include a markdown line, - // we have to count the newline that we split from earlier. - if line_no <= starting_line { - start_bytes += source_line.len() + 1; - } else { - end_bytes += source_line.len() + 1; - } - } - } - } - } - - Some(span_of_attrs(attrs)?.from_inner(InnerSpan::new( - md_range.start + start_bytes, - md_range.end + start_bytes + end_bytes, - ))) -} diff --git a/src/tools/clippy/clippy_lints/Cargo.toml b/src/tools/clippy/clippy_lints/Cargo.toml index c0a9f466e7b0..dcd9a4adcbd4 100644 --- a/src/tools/clippy/clippy_lints/Cargo.toml +++ b/src/tools/clippy/clippy_lints/Cargo.toml @@ -15,7 +15,6 @@ clippy_utils = { path = "../clippy_utils" } declare_clippy_lint = { path = "../declare_clippy_lint" } if_chain = "1.0" itertools = "0.10.1" -pulldown-cmark = { version = "0.9", default-features = false } quine-mc_cluskey = "0.2" regex-syntax = "0.7" serde = { version = "1.0", features = ["derive"] } diff --git a/src/tools/clippy/clippy_lints/src/doc.rs b/src/tools/clippy/clippy_lints/src/doc.rs index e29ab634c979..d8f2fbcea002 100644 --- a/src/tools/clippy/clippy_lints/src/doc.rs +++ b/src/tools/clippy/clippy_lints/src/doc.rs @@ -1,18 +1,16 @@ use clippy_utils::attrs::is_doc_hidden; use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_note, span_lint_and_then}; use clippy_utils::macros::{is_panic, root_macro_call_first_node}; -use clippy_utils::source::{first_line_of_span, snippet_with_applicability}; +use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; use clippy_utils::{is_entrypoint_fn, method_chain_args, return_ty}; use if_chain::if_chain; -use itertools::Itertools; use pulldown_cmark::Event::{ Code, End, FootnoteReference, HardBreak, Html, Rule, SoftBreak, Start, TaskListMarker, Text, }; use pulldown_cmark::Tag::{CodeBlock, Heading, Item, Link, Paragraph}; use pulldown_cmark::{BrokenLink, CodeBlockKind, CowStr, Options}; -use rustc_ast::ast::{Async, AttrKind, Attribute, Fn, FnRetTy, ItemKind}; -use rustc_ast::token::CommentKind; +use rustc_ast::ast::{Async, Attribute, Fn, FnRetTy, ItemKind}; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; use rustc_errors::emitter::EmitterWriter; @@ -26,6 +24,9 @@ use rustc_middle::lint::in_external_macro; use rustc_middle::ty; use rustc_parse::maybe_new_parser_from_source_str; use rustc_parse::parser::ForceCollect; +use rustc_resolve::rustdoc::{ + add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range, DocFragment, +}; use rustc_session::parse::ParseSess; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::edition::Edition; @@ -450,53 +451,16 @@ fn lint_for_missing_headers( } } -/// Cleanup documentation decoration. -/// -/// We can't use `rustc_ast::attr::AttributeMethods::with_desugared_doc` or -/// `rustc_ast::parse::lexer::comments::strip_doc_comment_decoration` because we -/// need to keep track of -/// the spans but this function is inspired from the later. -#[expect(clippy::cast_possible_truncation)] -#[must_use] -pub fn strip_doc_comment_decoration(doc: &str, comment_kind: CommentKind, span: Span) -> (String, Vec<(usize, Span)>) { - // one-line comments lose their prefix - if comment_kind == CommentKind::Line { - let mut doc = doc.to_owned(); - doc.push('\n'); - let len = doc.len(); - // +3 skips the opening delimiter - return (doc, vec![(len, span.with_lo(span.lo() + BytePos(3)))]); - } +#[derive(Copy, Clone)] +struct Fragments<'a> { + doc: &'a str, + fragments: &'a [DocFragment], +} - let mut sizes = vec![]; - let mut contains_initial_stars = false; - for line in doc.lines() { - let offset = line.as_ptr() as usize - doc.as_ptr() as usize; - debug_assert_eq!(offset as u32 as usize, offset); - contains_initial_stars |= line.trim_start().starts_with('*'); - // +1 adds the newline, +3 skips the opening delimiter - sizes.push((line.len() + 1, span.with_lo(span.lo() + BytePos(3 + offset as u32)))); - } - if !contains_initial_stars { - return (doc.to_string(), sizes); - } - // remove the initial '*'s if any - let mut no_stars = String::with_capacity(doc.len()); - for line in doc.lines() { - let mut chars = line.chars(); - for c in &mut chars { - if c.is_whitespace() { - no_stars.push(c); - } else { - no_stars.push(if c == '*' { ' ' } else { c }); - break; - } - } - no_stars.push_str(chars.as_str()); - no_stars.push('\n'); +impl Fragments<'_> { + fn span(self, cx: &LateContext<'_>, range: Range) -> Option { + source_span_for_markdown_range(cx.tcx, &self.doc, &range, &self.fragments) } - - (no_stars, sizes) } #[derive(Copy, Clone, Default)] @@ -515,27 +479,12 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[ Some(("fake".into(), "fake".into())) } + let (fragments, _) = attrs_to_doc_fragments(attrs.iter().map(|attr| (attr, None)), true); let mut doc = String::new(); - let mut spans = vec![]; - - for attr in attrs { - if let AttrKind::DocComment(comment_kind, comment) = attr.kind { - let (comment, current_spans) = strip_doc_comment_decoration(comment.as_str(), comment_kind, attr.span); - spans.extend_from_slice(¤t_spans); - doc.push_str(&comment); - } else if attr.has_name(sym::doc) { - // ignore mix of sugared and non-sugared doc - // don't trigger the safety or errors check - return None; - } - } - - let mut current = 0; - for &mut (ref mut offset, _) in &mut spans { - let offset_copy = *offset; - *offset = current; - current += offset_copy; + for fragment in &fragments { + add_doc_fragment(&mut doc, fragment); } + doc.pop(); if doc.is_empty() { return Some(DocHeaders::default()); @@ -543,23 +492,19 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[ let mut cb = fake_broken_link_callback; - let parser = - pulldown_cmark::Parser::new_with_broken_link_callback(&doc, Options::empty(), Some(&mut cb)).into_offset_iter(); - // Iterate over all `Events` and combine consecutive events into one - let events = parser.coalesce(|previous, current| { - let previous_range = previous.1; - let current_range = current.1; - - match (previous.0, current.0) { - (Text(previous), Text(current)) => { - let mut previous = previous.to_string(); - previous.push_str(¤t); - Ok((Text(previous.into()), previous_range)) - }, - (previous, current) => Err(((previous, previous_range), (current, current_range))), - } - }); - Some(check_doc(cx, valid_idents, events, &spans)) + // 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)); + + Some(check_doc( + cx, + valid_idents, + parser.into_offset_iter(), + Fragments { + fragments: &fragments, + doc: &doc, + }, + )) } const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail"]; @@ -568,7 +513,7 @@ fn check_doc<'a, Events: Iterator, Range, valid_idents: &FxHashSet, events: Events, - spans: &[(usize, Span)], + fragments: Fragments<'_>, ) -> DocHeaders { // true if a safety header was found let mut headers = DocHeaders::default(); @@ -579,8 +524,8 @@ fn check_doc<'a, Events: Iterator, Range, Span)> = Vec::new(); - let mut paragraph_span = spans.get(0).expect("function isn't called if doc comment is empty").1; + let mut text_to_check: Vec<(CowStr<'_>, Range)> = Vec::new(); + let mut paragraph_range = 0..0; for (event, range) in events { match event { Start(CodeBlock(ref kind)) => { @@ -613,25 +558,28 @@ fn check_doc<'a, Events: Iterator, Range { if let End(Heading(_, _, _)) = event { in_heading = false; } - if ticks_unbalanced { + if ticks_unbalanced + && let Some(span) = fragments.span(cx, paragraph_range.clone()) + { span_lint_and_help( cx, DOC_MARKDOWN, - paragraph_span, + span, "backticks are unbalanced", None, "a backtick may be missing a pair", ); } else { - for (text, span) in text_to_check { - check_text(cx, valid_idents, &text, span); + for (text, range) in text_to_check { + if let Some(span) = fragments.span(cx, range) { + check_text(cx, valid_idents, &text, span); + } } } text_to_check = Vec::new(); @@ -640,8 +588,7 @@ fn check_doc<'a, Events: Iterator, Range (), // HTML is weird, just ignore it SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (), FootnoteReference(text) | Text(text) => { - let (begin, span) = get_current_span(spans, range.start); - paragraph_span = paragraph_span.with_hi(span.hi()); + paragraph_range.end = range.end; ticks_unbalanced |= text.contains('`') && !in_code; if Some(&text) == in_link.as_ref() || ticks_unbalanced { // Probably a link of the form `` @@ -658,19 +605,19 @@ fn check_doc<'a, Events: Iterator, Range, Range, - in_link: bool, - trimmed_text: &str, - span: Span, - range: &Range, - begin: usize, - text_len: usize, -) { - if in_link && trimmed_text.starts_with('\'') && trimmed_text.ends_with('\'') { - // fix the span to only point at the text within the link - let lo = span.lo() + BytePos::from_usize(range.start - begin); +fn check_link_quotes(cx: &LateContext<'_>, trimmed_text: &str, range: Range, fragments: Fragments<'_>) { + if trimmed_text.starts_with('\'') + && trimmed_text.ends_with('\'') + && let Some(span) = fragments.span(cx, range) + { span_lint( cx, DOC_LINK_WITH_QUOTES, - span.with_lo(lo).with_hi(lo + BytePos::from_usize(text_len)), + span, "possible intra-doc link using quotes instead of backticks", ); } } -fn get_current_span(spans: &[(usize, Span)], idx: usize) -> (usize, Span) { - let index = match spans.binary_search_by(|c| c.0.cmp(&idx)) { - Ok(o) => o, - Err(e) => e - 1, - }; - spans[index] -} - -fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) { +fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, range: Range, fragments: Fragments<'_>) { fn has_needless_main(code: String, edition: Edition) -> bool { rustc_driver::catch_fatal_errors(|| { rustc_span::create_session_globals_then(edition, || { @@ -774,12 +706,13 @@ fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) { .unwrap_or_default() } + let trailing_whitespace = text.len() - text.trim_end().len(); + // Because of the global session, we need to create a new session in a different thread with // the edition we need. let text = text.to_owned(); - if thread::spawn(move || has_needless_main(text, edition)) - .join() - .expect("thread::spawn failed") + if thread::spawn(move || has_needless_main(text, edition)).join().expect("thread::spawn failed") + && let Some(span) = fragments.span(cx, range.start..range.end - trailing_whitespace) { span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest"); } diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index ab71bfbdc587..20fdf26dc521 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -21,6 +21,7 @@ // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) +extern crate pulldown_cmark; extern crate rustc_arena; extern crate rustc_ast; extern crate rustc_ast_pretty; @@ -37,6 +38,7 @@ extern crate rustc_lexer; extern crate rustc_lint; extern crate rustc_middle; extern crate rustc_parse; +extern crate rustc_resolve; extern crate rustc_session; extern crate rustc_span; extern crate rustc_target; diff --git a/src/tools/clippy/tests/compile-test.rs b/src/tools/clippy/tests/compile-test.rs index e329a94ff6a1..c1c3d4a868f9 100644 --- a/src/tools/clippy/tests/compile-test.rs +++ b/src/tools/clippy/tests/compile-test.rs @@ -130,7 +130,9 @@ fn base_config(test_dir: &str) -> (Config, Args) { }; let mut config = Config { - mode: Mode::Yolo { rustfix: RustfixMode::Everything }, + mode: Mode::Yolo { + rustfix: RustfixMode::Everything, + }, stderr_filters: vec![(Match::PathBackslash, b"/")], stdout_filters: vec![], output_conflict_handling: if bless { diff --git a/src/tools/clippy/tests/ui/doc/doc-fixable.fixed b/src/tools/clippy/tests/ui/doc/doc-fixable.fixed index f7c2f14a4824..47b56960a00f 100644 --- a/src/tools/clippy/tests/ui/doc/doc-fixable.fixed +++ b/src/tools/clippy/tests/ui/doc/doc-fixable.fixed @@ -198,6 +198,16 @@ fn pulldown_cmark_crash() {} /// [plain text][path::to::item] fn intra_doc_link() {} +/// Ignore escaped\_underscores +/// +/// \\[ +/// \\prod\_{x\\in X} p\_x +/// \\] +fn issue_2581() {} + +/// Foo \[bar\] \[baz\] \[qux\]. `DocMarkdownLint` +fn lint_after_escaped_chars() {} + // issue #7033 - generic_const_exprs ICE struct S where [(); N.checked_next_power_of_two().unwrap()]: { diff --git a/src/tools/clippy/tests/ui/doc/doc-fixable.rs b/src/tools/clippy/tests/ui/doc/doc-fixable.rs index 51961e75b84c..4d9a4eafa5fc 100644 --- a/src/tools/clippy/tests/ui/doc/doc-fixable.rs +++ b/src/tools/clippy/tests/ui/doc/doc-fixable.rs @@ -198,6 +198,16 @@ fn pulldown_cmark_crash() {} /// [plain text][path::to::item] fn intra_doc_link() {} +/// Ignore escaped\_underscores +/// +/// \\[ +/// \\prod\_{x\\in X} p\_x +/// \\] +fn issue_2581() {} + +/// Foo \[bar\] \[baz\] \[qux\]. DocMarkdownLint +fn lint_after_escaped_chars() {} + // issue #7033 - generic_const_exprs ICE struct S where [(); N.checked_next_power_of_two().unwrap()]: { diff --git a/src/tools/clippy/tests/ui/doc/doc-fixable.stderr b/src/tools/clippy/tests/ui/doc/doc-fixable.stderr index a30ded81385f..4c9ff41d9183 100644 --- a/src/tools/clippy/tests/ui/doc/doc-fixable.stderr +++ b/src/tools/clippy/tests/ui/doc/doc-fixable.stderr @@ -308,5 +308,16 @@ help: try LL | /// An iterator over `mycrate::Collection`'s values. | ~~~~~~~~~~~~~~~~~~~~~ -error: aborting due to 28 previous errors +error: item in documentation is missing backticks + --> $DIR/doc-fixable.rs:208:34 + | +LL | /// Foo \[bar\] \[baz\] \[qux\]. DocMarkdownLint + | ^^^^^^^^^^^^^^^ + | +help: try + | +LL | /// Foo \[bar\] \[baz\] \[qux\]. `DocMarkdownLint` + | ~~~~~~~~~~~~~~~~~ + +error: aborting due to 29 previous errors diff --git a/src/tools/clippy/tests/ui/doc/unbalanced_ticks.rs b/src/tools/clippy/tests/ui/doc/unbalanced_ticks.rs index 4a1711f79a02..6f7bab72040c 100644 --- a/src/tools/clippy/tests/ui/doc/unbalanced_ticks.rs +++ b/src/tools/clippy/tests/ui/doc/unbalanced_ticks.rs @@ -48,4 +48,4 @@ fn other_markdown() {} /// /// `lol` /// pub struct Struct; /// ``` -fn iss_7421() {} +fn issue_7421() {} diff --git a/src/tools/clippy/tests/ui/doc/unbalanced_ticks.stderr b/src/tools/clippy/tests/ui/doc/unbalanced_ticks.stderr index 92b6f8536c88..89ad8db3916c 100644 --- a/src/tools/clippy/tests/ui/doc/unbalanced_ticks.stderr +++ b/src/tools/clippy/tests/ui/doc/unbalanced_ticks.stderr @@ -1,7 +1,8 @@ error: backticks are unbalanced - --> $DIR/unbalanced_ticks.rs:7:1 + --> $DIR/unbalanced_ticks.rs:7:5 | -LL | / /// This is a doc comment with `unbalanced_tick marks and several words that +LL | /// This is a doc comment with `unbalanced_tick marks and several words that + | _____^ LL | | LL | | /// should be `encompassed_by` tick marks because they `contain_underscores`. LL | | /// Because of the initial `unbalanced_tick` pair, the error message is @@ -13,10 +14,10 @@ LL | | /// very `confusing_and_misleading`. = help: to override `-D warnings` add `#[allow(clippy::doc_markdown)]` error: backticks are unbalanced - --> $DIR/unbalanced_ticks.rs:14:1 + --> $DIR/unbalanced_ticks.rs:14:5 | LL | /// This paragraph has `unbalanced_tick marks and should stop_linting. - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: a backtick may be missing a pair @@ -32,10 +33,10 @@ LL | /// This paragraph is fine and `should_be` linted normally. | ~~~~~~~~~~~ error: backticks are unbalanced - --> $DIR/unbalanced_ticks.rs:20:1 + --> $DIR/unbalanced_ticks.rs:20:5 | LL | /// Double unbalanced backtick from ``here to here` should lint. - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: a backtick may be missing a pair @@ -51,18 +52,18 @@ LL | /// ## `not_fine` | ~~~~~~~~~~ error: backticks are unbalanced - --> $DIR/unbalanced_ticks.rs:37:1 + --> $DIR/unbalanced_ticks.rs:37:5 | LL | /// ### `unbalanced - | ^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ | = help: a backtick may be missing a pair error: backticks are unbalanced - --> $DIR/unbalanced_ticks.rs:40:1 + --> $DIR/unbalanced_ticks.rs:40:5 | LL | /// - This `item has unbalanced tick marks - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: a backtick may be missing a pair diff --git a/src/tools/clippy/tests/ui/doc_errors.rs b/src/tools/clippy/tests/ui/doc_errors.rs index 86721f61d199..d661231c59ea 100644 --- a/src/tools/clippy/tests/ui/doc_errors.rs +++ b/src/tools/clippy/tests/ui/doc_errors.rs @@ -85,6 +85,22 @@ impl Struct1 { async fn async_priv_method_missing_errors_header() -> Result<(), ()> { unimplemented!(); } + + /** + # Errors + A description of the errors goes here. + */ + fn block_comment() -> Result<(), ()> { + unimplemented!(); + } + + /** + * # Errors + * A description of the errors goes here. + */ + fn block_comment_leading_asterisks() -> Result<(), ()> { + unimplemented!(); + } } pub trait Trait1 { diff --git a/src/tools/clippy/tests/ui/doc_errors.stderr b/src/tools/clippy/tests/ui/doc_errors.stderr index 9cea864f1d86..28b2db2440b7 100644 --- a/src/tools/clippy/tests/ui/doc_errors.stderr +++ b/src/tools/clippy/tests/ui/doc_errors.stderr @@ -38,7 +38,7 @@ LL | pub async fn async_pub_method_missing_errors_header() -> Result<(), ()> | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:92:5 + --> $DIR/doc_errors.rs:108:5 | LL | fn trait_method_missing_errors_header() -> Result<(), ()>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/tools/clippy/tests/ui/needless_doc_main.rs b/src/tools/clippy/tests/ui/needless_doc_main.rs index d31b9047eaae..fee05926ce4f 100644 --- a/src/tools/clippy/tests/ui/needless_doc_main.rs +++ b/src/tools/clippy/tests/ui/needless_doc_main.rs @@ -10,7 +10,7 @@ /// unimplemented!(); /// } /// ``` -/// +/// /// With an explicit return type it should lint too /// ```edition2015 /// fn main() -> () { @@ -18,7 +18,7 @@ /// unimplemented!(); /// } /// ``` -/// +/// /// This should, too. /// ```rust /// fn main() { @@ -26,11 +26,12 @@ /// unimplemented!(); /// } /// ``` -/// +/// /// This one too. /// ```no_run -/// fn main() { +/// // the fn is not always the first line //~^ ERROR: needless `fn main` in doctest +/// fn main() { /// unimplemented!(); /// } /// ``` diff --git a/src/tools/clippy/tests/ui/needless_doc_main.stderr b/src/tools/clippy/tests/ui/needless_doc_main.stderr index 8dd93bb05dd5..84256548671d 100644 --- a/src/tools/clippy/tests/ui/needless_doc_main.stderr +++ b/src/tools/clippy/tests/ui/needless_doc_main.stderr @@ -1,29 +1,47 @@ error: needless `fn main` in doctest - --> $DIR/needless_doc_main.rs:7:4 + --> $DIR/needless_doc_main.rs:7:5 | -LL | /// fn main() { - | ^^^^^^^^^^^^ +LL | /// fn main() { + | _____^ +LL | | +LL | | +LL | | /// unimplemented!(); +LL | | /// } + | |_____^ | = note: `-D clippy::needless-doctest-main` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::needless_doctest_main)]` error: needless `fn main` in doctest - --> $DIR/needless_doc_main.rs:16:4 + --> $DIR/needless_doc_main.rs:16:5 | -LL | /// fn main() -> () { - | ^^^^^^^^^^^^^^^^^^ +LL | /// fn main() -> () { + | _____^ +LL | | +LL | | /// unimplemented!(); +LL | | /// } + | |_____^ error: needless `fn main` in doctest - --> $DIR/needless_doc_main.rs:24:4 + --> $DIR/needless_doc_main.rs:24:5 | -LL | /// fn main() { - | ^^^^^^^^^^^^ +LL | /// fn main() { + | _____^ +LL | | +LL | | /// unimplemented!(); +LL | | /// } + | |_____^ error: needless `fn main` in doctest - --> $DIR/needless_doc_main.rs:32:4 + --> $DIR/needless_doc_main.rs:32:5 | -LL | /// fn main() { - | ^^^^^^^^^^^^ +LL | /// // the fn is not always the first line + | _____^ +LL | | +LL | | /// fn main() { +LL | | /// unimplemented!(); +LL | | /// } + | |_____^ error: aborting due to 4 previous errors