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.

Sign up for GitHub

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

Revert "Cleanup markdown span handling" #80381

Merged
merged 1 commit into from
Dec 30, 2020
Merged

Revert "Cleanup markdown span handling" #80381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
126 changes: 65 additions & 61 deletions src/librustdoc/html/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for LinkReplacer<'a, I> {
struct HeadingLinks<'a, 'b, 'ids, I> {
inner: I,
toc: Option<&'b mut TocBuilder>,
buf: VecDeque<(Event<'a>, Range<usize>)>,
buf: VecDeque<Event<'a>>,
id_map: &'ids mut IdMap,
}

Expand All @@ -428,48 +428,48 @@ impl<'a, 'b, 'ids, I> HeadingLinks<'a, 'b, 'ids, I> {
}
}

impl<'a, 'b, 'ids, I: Iterator<Item = (Event<'a>, Range<usize>)>> Iterator
for HeadingLinks<'a, 'b, 'ids, I>
{
type Item = (Event<'a>, Range<usize>);
impl<'a, 'b, 'ids, I: Iterator<Item = Event<'a>>> Iterator for HeadingLinks<'a, 'b, 'ids, I> {
type Item = Event<'a>;

fn next(&mut self) -> Option<Self::Item> {
if let Some(e) = self.buf.pop_front() {
return Some(e);
}

let event = self.inner.next();
if let Some((Event::Start(Tag::Heading(level)), _)) = event {
if let Some(Event::Start(Tag::Heading(level))) = event {
let mut id = String::new();
for event in &mut self.inner {
match &event.0 {
match &event {
Event::End(Tag::Heading(..)) => break,
Event::Start(Tag::Link(_, _, _)) | Event::End(Tag::Link(..)) => {}
Event::Text(text) | Event::Code(text) => {
id.extend(text.chars().filter_map(slugify));
self.buf.push_back(event);
}
_ => self.buf.push_back(event),
_ => {}
}
match event {
Event::Start(Tag::Link(_, _, _)) | Event::End(Tag::Link(..)) => {}
event => self.buf.push_back(event),
}
}
let id = self.id_map.derive(id);

if let Some(ref mut builder) = self.toc {
let mut html_header = String::new();
html::push_html(&mut html_header, self.buf.iter().map(|(ev, _)| ev.clone()));
html::push_html(&mut html_header, self.buf.iter().cloned());
let sec = builder.push(level as u32, html_header, id.clone());
self.buf.push_front((Event::Html(format!("{} ", sec).into()), 0..0));
self.buf.push_front(Event::Html(format!("{} ", sec).into()));
}

self.buf.push_back((Event::Html(format!("</a></h{}>", level).into()), 0..0));
self.buf.push_back(Event::Html(format!("</a></h{}>", level).into()));

let start_tags = format!(
"<h{level} id=\"{id}\" class=\"section-header\">\
<a href=\"#{id}\">",
id = id,
level = level
);
return Some((Event::Html(start_tags.into()), 0..0));
return Some(Event::Html(start_tags.into()));
}
event
}
Expand Down Expand Up @@ -560,23 +560,23 @@ impl<'a, I> Footnotes<'a, I> {
}
}

impl<'a, I: Iterator<Item = (Event<'a>, Range<usize>)>> Iterator for Footnotes<'a, I> {
type Item = (Event<'a>, Range<usize>);
impl<'a, I: Iterator<Item = Event<'a>>> Iterator for Footnotes<'a, I> {
type Item = Event<'a>;

fn next(&mut self) -> Option<Self::Item> {
loop {
match self.inner.next() {
Some((Event::FootnoteReference(ref reference), range)) => {
Some(Event::FootnoteReference(ref reference)) => {
let entry = self.get_entry(&reference);
let reference = format!(
"<sup id=\"fnref{0}\"><a href=\"#fn{0}\">{0}</a></sup>",
(*entry).1
);
return Some((Event::Html(reference.into()), range));
return Some(Event::Html(reference.into()));
}
Some((Event::Start(Tag::FootnoteDefinition(def)), _)) => {
Some(Event::Start(Tag::FootnoteDefinition(def))) => {
let mut content = Vec::new();
for (event, _) in &mut self.inner {
for event in &mut self.inner {
if let Event::End(Tag::FootnoteDefinition(..)) = event {
break;
}
Expand Down Expand Up @@ -607,7 +607,7 @@ impl<'a, I: Iterator<Item = (Event<'a>, Range<usize>)>> Iterator for Footnotes<'
ret.push_str("</li>");
}
ret.push_str("</ol></div>");
return Some((Event::Html(ret.into()), 0..0));
return Some(Event::Html(ret.into()));
} else {
return None;
}
Expand Down Expand Up @@ -917,14 +917,13 @@ impl Markdown<'_> {
};

let p = Parser::new_with_broken_link_callback(md, opts(), Some(&mut replacer));
let p = p.into_offset_iter();

let mut s = String::with_capacity(md.len() * 3 / 2);

let p = HeadingLinks::new(p, None, &mut ids);
let p = Footnotes::new(p);
let p = LinkReplacer::new(p.map(|(ev, _)| ev), links);
let p = LinkReplacer::new(p, links);
let p = CodeBlocks::new(p, codes, edition, playground);
let p = Footnotes::new(p);
html::push_html(&mut s, p);

s
Expand All @@ -935,16 +934,16 @@ impl MarkdownWithToc<'_> {
crate fn into_string(self) -> String {
let MarkdownWithToc(md, mut ids, codes, edition, playground) = self;

let p = Parser::new_ext(md, opts()).into_offset_iter();
let p = Parser::new_ext(md, opts());

let mut s = String::with_capacity(md.len() * 3 / 2);

let mut toc = TocBuilder::new();

{
let p = HeadingLinks::new(p, Some(&mut toc), &mut ids);
let p = CodeBlocks::new(p, codes, edition, playground);
let p = Footnotes::new(p);
let p = CodeBlocks::new(p.map(|(ev, _)| ev), codes, edition, playground);
html::push_html(&mut s, p);
}

Expand All @@ -960,19 +959,19 @@ impl MarkdownHtml<'_> {
if md.is_empty() {
return String::new();
}
let p = Parser::new_ext(md, opts()).into_offset_iter();
let p = Parser::new_ext(md, opts());

// Treat inline HTML as plain text.
let p = p.map(|event| match event.0 {
Event::Html(text) => (Event::Text(text), event.1),
let p = p.map(|event| match event {
Event::Html(text) => Event::Text(text),
_ => event,
});

let mut s = String::with_capacity(md.len() * 3 / 2);

let p = HeadingLinks::new(p, None, &mut ids);
let p = CodeBlocks::new(p, codes, edition, playground);
let p = Footnotes::new(p);
let p = CodeBlocks::new(p.map(|(ev, _)| ev), codes, edition, playground);
html::push_html(&mut s, p);

s
Expand Down Expand Up @@ -1125,45 +1124,50 @@ crate fn plain_text_summary(md: &str) -> String {
s
}

crate fn markdown_links(md: &str) -> Vec<(String, Range<usize>)> {
crate fn markdown_links(md: &str) -> Vec<(String, Option<Range<usize>>)> {
if md.is_empty() {
return vec![];
}

let mut links = vec![];
// Used to avoid mutable borrow issues in the `push` closure
// Probably it would be more efficient to use a `RefCell` but it doesn't seem worth the churn.
let mut shortcut_links = vec![];

let span_for_link = |link: &str, span: Range<usize>| {
// Pulldown includes the `[]` as well as the URL. Only highlight the relevant span.
// NOTE: uses `rfind` in case the title and url are the same: `[Ok][Ok]`
match md[span.clone()].rfind(link) {
Some(start) => {
let start = span.start + start;
start..start + link.len()
{
let locate = |s: &str| unsafe {
let s_start = s.as_ptr();
let s_end = s_start.add(s.len());
let md_start = md.as_ptr();
let md_end = md_start.add(md.len());
if md_start <= s_start && s_end <= md_end {
let start = s_start.offset_from(md_start) as usize;
let end = s_end.offset_from(md_start) as usize;
Some(start..end)
} else {
None
}
};

let mut push = |link: BrokenLink<'_>| {
// FIXME: use `link.span` instead of `locate`
// (doing it now includes the `[]` as well as the text)
shortcut_links.push((link.reference.to_owned(), locate(link.reference)));
None
};
let p = Parser::new_with_broken_link_callback(md, opts(), Some(&mut push));

// There's no need to thread an IdMap through to here because
// the IDs generated aren't going to be emitted anywhere.
let mut ids = IdMap::new();
let iter = Footnotes::new(HeadingLinks::new(p, None, &mut ids));

for ev in iter {
if let Event::Start(Tag::Link(_, dest, _)) = ev {
debug!("found link: {}", dest);
links.push(match dest {
CowStr::Borrowed(s) => (s.to_owned(), locate(s)),
s @ (CowStr::Boxed(..) | CowStr::Inlined(..)) => (s.into_string(), None),
});
}
// This can happen for things other than intra-doc links, like `#1` expanded to `https://github.com/rust-lang/rust/issues/1`.
None => span,
}
};
let mut push = |link: BrokenLink<'_>| {
let span = span_for_link(link.reference, link.span);
shortcut_links.push((link.reference.to_owned(), span));
None
};
let p = Parser::new_with_broken_link_callback(md, opts(), Some(&mut push));

// There's no need to thread an IdMap through to here because
// the IDs generated aren't going to be emitted anywhere.
let mut ids = IdMap::new();
let iter = Footnotes::new(HeadingLinks::new(p.into_offset_iter(), None, &mut ids));

for ev in iter {
if let Event::Start(Tag::Link(_, dest, _)) = ev.0 {
debug!("found link: {}", dest);
let span = span_for_link(&dest, ev.1);
links.push((dest.into_string(), span));
}
}

Expand Down
62 changes: 34 additions & 28 deletions src/librustdoc/passes/collect_intra_doc_links.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ struct DiagnosticInfo<'a> {
item: &'a Item,
dox: &'a str,
ori_link: &'a str,
link_range: Range<usize>,
link_range: Option<Range<usize>>,
}

#[derive(Clone, Debug, Hash)]
Expand Down Expand Up @@ -920,7 +920,7 @@ impl LinkCollector<'_, '_> {
parent_node: Option<DefId>,
krate: CrateNum,
ori_link: String,
link_range: Range<usize>,
link_range: Option<Range<usize>>,
) -> Option<ItemLink> {
trace!("considering link '{}'", ori_link);

Expand Down Expand Up @@ -1566,7 +1566,7 @@ fn report_diagnostic(
msg: &str,
item: &Item,
dox: &str,
link_range: &Range<usize>,
link_range: &Option<Range<usize>>,
decorate: impl FnOnce(&mut DiagnosticBuilder<'_>, Option<rustc_span::Span>),
) {
let hir_id = match cx.as_local_hir_id(item.def_id) {
Expand All @@ -1584,26 +1584,31 @@ fn report_diagnostic(
cx.tcx.struct_span_lint_hir(lint, hir_id, sp, |lint| {
let mut diag = lint.build(msg);

let span = super::source_span_for_markdown_range(cx, dox, link_range, attrs);
if let Some(sp) = span {
diag.set_span(sp);
} else {
// blah blah blah\nblah\nblah [blah] blah blah\nblah blah
// ^ ~~~~
// | link_range
// last_new_line_offset
let last_new_line_offset = dox[..link_range.start].rfind('\n').map_or(0, |n| n + 1);
let line = dox[last_new_line_offset..].lines().next().unwrap_or("");

// Print the line containing the `link_range` and manually mark it with '^'s.
diag.note(&format!(
"the link appears in this line:\n\n{line}\n\
{indicator: <before$}{indicator:^<found$}",
line = line,
indicator = "",
before = link_range.start - last_new_line_offset,
found = link_range.len(),
));
let span = link_range
.as_ref()
.and_then(|range| super::source_span_for_markdown_range(cx, dox, range, attrs));

if let Some(link_range) = link_range {
if let Some(sp) = span {
diag.set_span(sp);
} else {
// blah blah blah\nblah\nblah [blah] blah blah\nblah blah
// ^ ~~~~
// | link_range
// last_new_line_offset
let last_new_line_offset = dox[..link_range.start].rfind('\n').map_or(0, |n| n + 1);
let line = dox[last_new_line_offset..].lines().next().unwrap_or("");

// Print the line containing the `link_range` and manually mark it with '^'s.
diag.note(&format!(
"the link appears in this line:\n\n{line}\n\
{indicator: <before$}{indicator:^<found$}",
line = line,
indicator = "",
before = link_range.start - last_new_line_offset,
found = link_range.len(),
));
}
}

decorate(&mut diag, span);
Expand All @@ -1623,7 +1628,7 @@ fn resolution_failure(
path_str: &str,
disambiguator: Option<Disambiguator>,
dox: &str,
link_range: Range<usize>,
link_range: Option<Range<usize>>,
kinds: SmallVec<[ResolutionFailure<'_>; 3]>,
) {
report_diagnostic(
Expand Down Expand Up @@ -1857,7 +1862,7 @@ fn anchor_failure(
item: &Item,
path_str: &str,
dox: &str,
link_range: Range<usize>,
link_range: Option<Range<usize>>,
failure: AnchorFailure,
) {
let msg = match failure {
Expand All @@ -1882,7 +1887,7 @@ fn ambiguity_error(
item: &Item,
path_str: &str,
dox: &str,
link_range: Range<usize>,
link_range: Option<Range<usize>>,
candidates: Vec<Res>,
) {
let mut msg = format!("`{}` is ", path_str);
Expand Down Expand Up @@ -1931,12 +1936,13 @@ fn suggest_disambiguator(
path_str: &str,
dox: &str,
sp: Option<rustc_span::Span>,
link_range: &Range<usize>,
link_range: &Option<Range<usize>>,
) {
let suggestion = disambiguator.suggestion();
let help = format!("to link to the {}, {}", disambiguator.descr(), suggestion.descr());

if let Some(sp) = sp {
let link_range = link_range.as_ref().expect("must have a link range if we have a span");
let msg = if dox.bytes().nth(link_range.start) == Some(b'`') {
format!("`{}`", suggestion.as_help(path_str))
} else {
Expand All @@ -1955,7 +1961,7 @@ fn privacy_error(
item: &Item,
path_str: &str,
dox: &str,
link_range: Range<usize>,
link_range: Option<Range<usize>>,
) {
let sym;
let item_name = match item.name {
Expand Down
7 changes: 7 additions & 0 deletions src/test/rustdoc-ui/reference-links.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Test that errors point to the reference, not to the title text.
#![deny(broken_intra_doc_links)]
//! Links to [a] [link][a]
//!
//! [a]: std::process::Comman
//~^ ERROR unresolved
//~| ERROR unresolved
Loading