mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #80381 - rust-lang:revert-80244-spans, r=GuillaumeGomez

Browse Source 
Revert "Cleanup markdown span handling"

Reverts rust-lang/rust#80244. This caused a diagnostic regression, originally it was:

```
warning: unresolved link to `std::process::Comman`
 --> link.rs:3:10
  |
3 | //! [a]: std::process::Comman
  |          ^^^^^^^^^^^^^^^^^^^^ no item named `Comman` in module `process`
  |
  = note: `#[warn(broken_intra_doc_links)]` on by default
```
but after that PR rustdoc now displays
```
warning: unresolved link to `std::process::Comman`
 --> link.rs:1:14
  |
1 | //! Links to [a] [link][a]
  |              ^^^ no item named `Comman` in module `process`
  |
  = note: `#[warn(broken_intra_doc_links)]` on by default
```
which IMO is much less clear.

cc `@bugadani,` thanks for catching this in https://github.com/rust-lang/rust/pull/77859.
r? `@GuillaumeGomez`
This commit is contained in:
Yuki Okushi 2020-12-30 22:49:23 +09:00 committed by GitHub
commit 7494aef979
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 123 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

122
src/librustdoc/html/markdown.rs
View File

@ -418,7 +418,7 @@ fn next(&mut self) -> Option<Self::Item> {
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,
}
@ -428,10 +428,8 @@ fn new(iter: I, toc: Option<&'b mut TocBuilder>, ids: &'ids mut IdMap) -> Self {
}
}
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() {
@ -439,29 +437,31 @@ fn next(&mut self) -> Option<Self::Item> {
}
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\">\
@ -469,7 +469,7 @@ fn next(&mut self) -> Option<Self::Item> {
id = id,
level = level
);
return Some((Event::Html(start_tags.into()), 0..0));
return Some(Event::Html(start_tags.into()));
}
event
}
@ -560,23 +560,23 @@ fn get_entry(&mut self, key: &str) -> &mut (Vec<Event<'a>>, u16) {
}
}
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;
}
@ -607,7 +607,7 @@ fn next(&mut self) -> Option<Self::Item> {
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;
}
@ -917,14 +917,13 @@ pub fn into_string(self) -> String {
};
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
@ -935,7 +934,7 @@ 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);
@ -943,8 +942,8 @@ impl MarkdownWithToc<'_> {
{
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);
}
@ -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
@ -1125,45 +1124,50 @@ fn push(s: &mut String, text_length: &mut usize, text: &str) {
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
}
// 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));
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));
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));
// 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),
});
}
}
}

60
src/librustdoc/passes/collect_intra_doc_links.rs
View File

@ -245,7 +245,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)]
@ -982,7 +982,7 @@ fn resolve_link(
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);
@ -1628,7 +1628,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) {
@ -1646,26 +1646,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("");
let span = link_range
.as_ref()
.and_then(|range| super::source_span_for_markdown_range(cx, dox, range, attrs));
// 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(),
));
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);
@ -1685,7 +1690,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]>,
) {
let tcx = collector.cx.tcx;
@ -1909,7 +1914,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 {
@ -1934,7 +1939,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);
@ -1983,12 +1988,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 {
@ -2007,7 +2013,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 {

7
src/test/rustdoc-ui/reference-links.rs Normal file
View File

@ -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

20
src/test/rustdoc-ui/reference-links.stderr Normal file
View File

@ -0,0 +1,20 @@
error: unresolved link to `std::process::Comman`
--> $DIR/reference-links.rs:5:10
|
LL | //! [a]: std::process::Comman
| ^^^^^^^^^^^^^^^^^^^^ no item named `Comman` in module `process`
|
note: the lint level is defined here
--> $DIR/reference-links.rs:2:9
|
LL | #![deny(broken_intra_doc_links)]
| ^^^^^^^^^^^^^^^^^^^^^^
error: unresolved link to `std::process::Comman`
--> $DIR/reference-links.rs:5:10
|
LL | //! [a]: std::process::Comman
| ^^^^^^^^^^^^^^^^^^^^ no item named `Comman` in module `process`
error: aborting due to 2 previous errors