Skip to content

Reuse rustdoc's doc comment handling in Clippy #115689

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

Merged
merged 1 commit into from
Sep 12, 2023
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: 0 additions & 1 deletion Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,6 @@ dependencies = [
"declare_clippy_lint",
"if_chain",
"itertools",
"pulldown-cmark",
"quine-mc_cluskey",
"regex",
"regex-syntax 0.7.2",
Expand Down
89 changes: 88 additions & 1 deletion compiler/rustc_resolve/src/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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<Span> {
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<usize>,
fragments: &[DocFragment],
) -> Option<Span> {
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,
)))
}
6 changes: 4 additions & 2 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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()))
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/doctest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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)]
Expand Down Expand Up @@ -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),
)),
);
}
Expand Down
43 changes: 23 additions & 20 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(),
),
};
Expand Down
7 changes: 4 additions & 3 deletions src/librustdoc/passes/lint/bare_urls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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<usize>| {
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(
Expand Down
16 changes: 10 additions & 6 deletions src/librustdoc/passes/lint/check_code_block_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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() {
Expand Down Expand Up @@ -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"
Expand Down
7 changes: 4 additions & 3 deletions src/librustdoc/passes/lint/html_tags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<usize>, 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),
};
Expand Down Expand Up @@ -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),
Expand Down
34 changes: 22 additions & 12 deletions src/librustdoc/passes/lint/redundant_explicit_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ 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;
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 {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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| {
Expand Down
Loading