From bc20eb6fbc1109462147fbd5d0680b6015f71e94 Mon Sep 17 00:00:00 2001 From: kennytm Date: Mon, 28 May 2018 01:46:59 +0800 Subject: [PATCH 1/2] Point to the rustdoc attribute where intralink resolution failed. --- src/librustdoc/clean/mod.rs | 25 +++++++++++++------ src/test/rustdoc-ui/intra-links-warning.rs | 4 ++- .../rustdoc-ui/intra-links-warning.stderr | 18 +++++++++++++ 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index a57f3a42939..7c3df329bb7 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -954,12 +954,20 @@ fn type_ns_kind(def: Def, path_str: &str) -> (&'static str, &'static str, String (kind, article, format!("{}@{}", kind, path_str)) } +fn span_of_attrs(attrs: &Attributes) -> syntax_pos::Span { + if attrs.doc_strings.is_empty() { + return DUMMY_SP; + } + let start = attrs.doc_strings[0].span(); + let end = attrs.doc_strings.last().unwrap().span(); + start.to(end) +} + fn ambiguity_error(cx: &DocContext, attrs: &Attributes, path_str: &str, article1: &str, kind1: &str, disambig1: &str, article2: &str, kind2: &str, disambig2: &str) { - let sp = attrs.doc_strings.first() - .map_or(DUMMY_SP, |a| a.span()); + let sp = span_of_attrs(attrs); cx.sess() .struct_span_warn(sp, &format!("`{}` is both {} {} and {} {}", @@ -1174,8 +1182,9 @@ enum PathKind { Type, } -fn resolution_failure(cx: &DocContext, path_str: &str) { - cx.sess().warn(&format!("[{}] cannot be resolved, ignoring it...", path_str)); +fn resolution_failure(cx: &DocContext, attrs: &Attributes, path_str: &str) { + let sp = span_of_attrs(attrs); + cx.sess().span_warn(sp, &format!("[{}] cannot be resolved, ignoring it...", path_str)); } impl Clean for [ast::Attribute] { @@ -1228,7 +1237,7 @@ fn clean(&self, cx: &DocContext) -> Attributes { if let Ok(def) = resolve(cx, path_str, true) { def } else { - resolution_failure(cx, path_str); + resolution_failure(cx, &attrs, path_str); // this could just be a normal link or a broken link // we could potentially check if something is // "intra-doc-link-like" and warn in that case @@ -1239,7 +1248,7 @@ fn clean(&self, cx: &DocContext) -> Attributes { if let Ok(def) = resolve(cx, path_str, false) { def } else { - resolution_failure(cx, path_str); + resolution_failure(cx, &attrs, path_str); // this could just be a normal link continue; } @@ -1284,7 +1293,7 @@ fn clean(&self, cx: &DocContext) -> Attributes { } else if let Ok(value_def) = resolve(cx, path_str, true) { value_def } else { - resolution_failure(cx, path_str); + resolution_failure(cx, &attrs, path_str); // this could just be a normal link continue; } @@ -1293,7 +1302,7 @@ fn clean(&self, cx: &DocContext) -> Attributes { if let Some(def) = macro_resolve(cx, path_str) { (def, None) } else { - resolution_failure(cx, path_str); + resolution_failure(cx, &attrs, path_str); continue } } diff --git a/src/test/rustdoc-ui/intra-links-warning.rs b/src/test/rustdoc-ui/intra-links-warning.rs index 2a00d31e3d7..830aaabf9d2 100644 --- a/src/test/rustdoc-ui/intra-links-warning.rs +++ b/src/test/rustdoc-ui/intra-links-warning.rs @@ -10,7 +10,9 @@ // compile-pass -//! Test with [Foo::baz], [Bar::foo], [Uniooon::X] +//! Test with [Foo::baz], [Bar::foo], ... +//! +//! and [Uniooon::X]. pub struct Foo { pub bar: usize, diff --git a/src/test/rustdoc-ui/intra-links-warning.stderr b/src/test/rustdoc-ui/intra-links-warning.stderr index 67d7bdd02b3..539ae94c3a9 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -1,6 +1,24 @@ warning: [Foo::baz] cannot be resolved, ignoring it... + --> $DIR/intra-links-warning.rs:13:1 + | +13 | / //! Test with [Foo::baz], [Bar::foo], ... +14 | | //! +15 | | //! and [Uniooon::X]. + | |_____________________^ warning: [Bar::foo] cannot be resolved, ignoring it... + --> $DIR/intra-links-warning.rs:13:1 + | +13 | / //! Test with [Foo::baz], [Bar::foo], ... +14 | | //! +15 | | //! and [Uniooon::X]. + | |_____________________^ warning: [Uniooon::X] cannot be resolved, ignoring it... + --> $DIR/intra-links-warning.rs:13:1 + | +13 | / //! Test with [Foo::baz], [Bar::foo], ... +14 | | //! +15 | | //! and [Uniooon::X]. + | |_____________________^ From 2886aca232361519a1a8c142b2c670fed1bd03b6 Mon Sep 17 00:00:00 2001 From: kennytm Date: Sun, 3 Jun 2018 18:22:24 +0800 Subject: [PATCH 2/2] Show which line the link is coming from. --- src/librustdoc/clean/mod.rs | 45 ++++++++++++++++--- src/librustdoc/html/markdown.rs | 25 +++++++++-- src/librustdoc/lib.rs | 1 + .../rustdoc-ui/intra-links-warning.stderr | 15 +++++++ 4 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 7c3df329bb7..1c1ba208678 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -57,6 +57,7 @@ use std::cell::RefCell; use std::sync::Arc; use std::u32; +use std::ops::Range; use core::{self, DocContext}; use doctree; @@ -1182,9 +1183,39 @@ enum PathKind { Type, } -fn resolution_failure(cx: &DocContext, attrs: &Attributes, path_str: &str) { +fn resolution_failure( + cx: &DocContext, + attrs: &Attributes, + path_str: &str, + dox: &str, + link_range: Option>, +) { let sp = span_of_attrs(attrs); - cx.sess().span_warn(sp, &format!("[{}] cannot be resolved, ignoring it...", path_str)); + let mut diag = cx.sess() + .struct_span_warn(sp, &format!("[{}] cannot be resolved, ignoring it...", path_str)); + + if let Some(link_range) = link_range { + // 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: for [ast::Attribute] { @@ -1193,7 +1224,7 @@ fn clean(&self, cx: &DocContext) -> Attributes { if UnstableFeatures::from_environment().is_nightly_build() { let dox = attrs.collapsed_doc_value().unwrap_or_else(String::new); - for ori_link in markdown_links(&dox) { + for (ori_link, link_range) in markdown_links(&dox) { // bail early for real links if ori_link.contains('/') { continue; @@ -1237,7 +1268,7 @@ fn clean(&self, cx: &DocContext) -> Attributes { if let Ok(def) = resolve(cx, path_str, true) { def } else { - resolution_failure(cx, &attrs, path_str); + resolution_failure(cx, &attrs, path_str, &dox, link_range); // this could just be a normal link or a broken link // we could potentially check if something is // "intra-doc-link-like" and warn in that case @@ -1248,7 +1279,7 @@ fn clean(&self, cx: &DocContext) -> Attributes { if let Ok(def) = resolve(cx, path_str, false) { def } else { - resolution_failure(cx, &attrs, path_str); + resolution_failure(cx, &attrs, path_str, &dox, link_range); // this could just be a normal link continue; } @@ -1293,7 +1324,7 @@ fn clean(&self, cx: &DocContext) -> Attributes { } else if let Ok(value_def) = resolve(cx, path_str, true) { value_def } else { - resolution_failure(cx, &attrs, path_str); + resolution_failure(cx, &attrs, path_str, &dox, link_range); // this could just be a normal link continue; } @@ -1302,7 +1333,7 @@ fn clean(&self, cx: &DocContext) -> Attributes { if let Some(def) = macro_resolve(cx, path_str) { (def, None) } else { - resolution_failure(cx, &attrs, path_str); + resolution_failure(cx, &attrs, path_str, &dox, link_range); continue } } diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index c09bd4cc84a..7088104cd7a 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -32,6 +32,8 @@ use std::collections::{HashMap, VecDeque}; use std::default::Default; use std::fmt::{self, Write}; +use std::borrow::Cow; +use std::ops::Range; use std::str; use syntax::feature_gate::UnstableFeatures; use syntax::codemap::Span; @@ -747,7 +749,7 @@ fn next(&mut self) -> Option { s } -pub fn markdown_links(md: &str) -> Vec { +pub fn markdown_links(md: &str) -> Vec<(String, Option>)> { if md.is_empty() { return vec![]; } @@ -760,8 +762,22 @@ pub fn markdown_links(md: &str) -> Vec { let shortcut_links = RefCell::new(vec![]); { + 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 push = |_: &str, s: &str| { - shortcut_links.borrow_mut().push(s.to_owned()); + shortcut_links.borrow_mut().push((s.to_owned(), locate(s))); None }; let p = Parser::new_with_broken_link_callback(md, opts, @@ -772,7 +788,10 @@ pub fn markdown_links(md: &str) -> Vec { for ev in iter { if let Event::Start(Tag::Link(dest, _)) = ev { debug!("found link: {}", dest); - links.push(dest.into_owned()); + links.push(match dest { + Cow::Borrowed(s) => (s.to_owned(), locate(s)), + Cow::Owned(s) => (s, None), + }); } } } diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 1b713a446a0..97c84d8348f 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -23,6 +23,7 @@ #![feature(test)] #![feature(vec_remove_item)] #![feature(entry_and_modify)] +#![feature(ptr_offset_from)] #![recursion_limit="256"] diff --git a/src/test/rustdoc-ui/intra-links-warning.stderr b/src/test/rustdoc-ui/intra-links-warning.stderr index 539ae94c3a9..1e8e9f04c26 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -5,6 +5,11 @@ warning: [Foo::baz] cannot be resolved, ignoring it... 14 | | //! 15 | | //! and [Uniooon::X]. | |_____________________^ + | + = note: the link appears in this line: + + Test with [Foo::baz], [Bar::foo], ... + ^^^^^^^^ warning: [Bar::foo] cannot be resolved, ignoring it... --> $DIR/intra-links-warning.rs:13:1 @@ -13,6 +18,11 @@ warning: [Bar::foo] cannot be resolved, ignoring it... 14 | | //! 15 | | //! and [Uniooon::X]. | |_____________________^ + | + = note: the link appears in this line: + + Test with [Foo::baz], [Bar::foo], ... + ^^^^^^^^ warning: [Uniooon::X] cannot be resolved, ignoring it... --> $DIR/intra-links-warning.rs:13:1 @@ -21,4 +31,9 @@ warning: [Uniooon::X] cannot be resolved, ignoring it... 14 | | //! 15 | | //! and [Uniooon::X]. | |_____________________^ + | + = note: the link appears in this line: + + and [Uniooon::X]. + ^^^^^^^^^^