From 33442b133cef78e8f08271cd863051e0de997b69 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Thu, 28 Feb 2019 13:54:19 -0500 Subject: [PATCH 1/3] overhaul intra-doc-link ambiguity warning - Makes the warning part of the `intra_doc_link_resolution_failure` lint. - Tightens the span to just the ambiguous link. - Reports ambiguities across all three namespaces. - Uses structured suggestions for disambiguation. - Adds a test for the warnings. --- .../passes/collect_intra_doc_links.rs | 240 +++++++++++------- src/test/rustdoc-ui/intra-links-ambiguity.rs | 36 +++ .../rustdoc-ui/intra-links-ambiguity.stderr | 82 ++++++ src/test/rustdoc/intra-links.rs | 3 +- 4 files changed, 266 insertions(+), 95 deletions(-) create mode 100644 src/test/rustdoc-ui/intra-links-ambiguity.rs create mode 100644 src/test/rustdoc-ui/intra-links-ambiguity.stderr diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index fefff1f3a7593..36bfe2fb2c3da 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1,7 +1,8 @@ -use rustc::lint as lint; -use rustc::hir; +use errors::Applicability; use rustc::hir::def::Def; use rustc::hir::def_id::DefId; +use rustc::hir; +use rustc::lint as lint; use rustc::ty; use syntax; use syntax::ast::{self, Ident}; @@ -53,6 +54,13 @@ struct LinkCollector<'a, 'tcx> { is_nightly_build: bool, } +#[derive(Debug, Copy, Clone)] +enum Namespace { + Type, + Value, + Macro, +} + impl<'a, 'tcx> LinkCollector<'a, 'tcx> { fn new(cx: &'a DocContext<'tcx>) -> Self { LinkCollector { @@ -345,57 +353,52 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } PathKind::Unknown => { // Try everything! + let mut candidates = vec![]; + if let Some(macro_def) = macro_resolve(cx, path_str) { - if let Ok(type_def) = - self.resolve(path_str, false, ¤t_item, parent_node) - { - let (type_kind, article, type_disambig) - = type_ns_kind(type_def.0, path_str); - ambiguity_error(cx, &item.attrs, path_str, - article, type_kind, &type_disambig, - "a", "macro", &format!("macro@{}", path_str)); - continue; - } else if let Ok(value_def) = - self.resolve(path_str, true, ¤t_item, parent_node) - { - let (value_kind, value_disambig) - = value_ns_kind(value_def.0, path_str) - .expect("struct and mod cases should have been \ - caught in previous branch"); - ambiguity_error(cx, &item.attrs, path_str, - "a", value_kind, &value_disambig, - "a", "macro", &format!("macro@{}", path_str)); - } - (macro_def, None) - } else if let Ok(type_def) = + candidates.push(((macro_def, None), Namespace::Macro)); + } + + if let Ok(type_def) = self.resolve(path_str, false, ¤t_item, parent_node) { - // It is imperative we search for not-a-value first - // Otherwise we will find struct ctors for when we are looking - // for structs, and the link won't work if there is something in - // both namespaces. - if let Ok(value_def) = - self.resolve(path_str, true, ¤t_item, parent_node) - { - let kind = value_ns_kind(value_def.0, path_str); - if let Some((value_kind, value_disambig)) = kind { - let (type_kind, article, type_disambig) - = type_ns_kind(type_def.0, path_str); - ambiguity_error(cx, &item.attrs, path_str, - article, type_kind, &type_disambig, - "a", value_kind, &value_disambig); - continue; - } - } - type_def - } else if let Ok(value_def) = + candidates.push((type_def, Namespace::Type)); + } + + if let Ok(value_def) = self.resolve(path_str, true, ¤t_item, parent_node) { - value_def - } else { + // Structs, variants, and mods exist in both namespaces, skip them. + match value_def.0 { + Def::StructCtor(..) + | Def::Mod(..) + | Def::Variant(..) + | Def::VariantCtor(..) + | Def::SelfCtor(..) => (), + _ => candidates.push((value_def, Namespace::Value)), + } + } + + if candidates.len() == 1 { + candidates.remove(0).0 + } else if candidates.is_empty() { resolution_failure(cx, &item.attrs, path_str, &dox, link_range); // this could just be a normal link continue; + } else { + let candidates = candidates.into_iter().map(|((def, _), ns)| { + (def, ns) + }).collect::>(); + + ambiguity_error( + cx, + &item.attrs, + path_str, + &dox, + link_range, + &candidates, + ); + continue; } } PathKind::Macro => { @@ -505,59 +508,108 @@ fn resolution_failure( diag.emit(); } -fn ambiguity_error(cx: &DocContext<'_>, attrs: &Attributes, - path_str: &str, - article1: &str, kind1: &str, disambig1: &str, - article2: &str, kind2: &str, disambig2: &str) { +fn ambiguity_error( + cx: &DocContext<'_>, + attrs: &Attributes, + path_str: &str, + dox: &str, + link_range: Option>, + candidates: &[(Def, Namespace)], +) { let sp = span_of_attrs(attrs); - cx.sess() - .struct_span_warn(sp, - &format!("`{}` is both {} {} and {} {}", - path_str, article1, kind1, - article2, kind2)) - .help(&format!("try `{}` if you want to select the {}, \ - or `{}` if you want to \ - select the {}", - disambig1, kind1, disambig2, - kind2)) - .emit(); -} -/// Given a def, returns its name and disambiguator -/// for a value namespace. -/// -/// Returns `None` for things which cannot be ambiguous since -/// they exist in both namespaces (structs and modules). -fn value_ns_kind(def: Def, path_str: &str) -> Option<(&'static str, String)> { - match def { - // Structs, variants, and mods exist in both namespaces; skip them. - Def::StructCtor(..) | Def::Mod(..) | Def::Variant(..) | - Def::VariantCtor(..) | Def::SelfCtor(..) - => None, - Def::Fn(..) - => Some(("function", format!("{}()", path_str))), - Def::Method(..) - => Some(("method", format!("{}()", path_str))), - Def::Const(..) - => Some(("const", format!("const@{}", path_str))), - Def::Static(..) - => Some(("static", format!("static@{}", path_str))), - _ => Some(("value", format!("value@{}", path_str))), + let mut msg = format!("`{}` is ", path_str); + + match candidates { + [(first_def, _), (second_def, _)] => { + msg += &format!( + "both {} {} and {} {}", + first_def.article(), + first_def.kind_name(), + second_def.article(), + second_def.kind_name(), + ); + } + _ => { + let mut candidates = candidates.iter().peekable(); + while let Some((def, _)) = candidates.next() { + if candidates.peek().is_some() { + msg += &format!("{} {}, ", def.article(), def.kind_name()); + } else { + msg += &format!("and {} {}", def.article(), def.kind_name()); + } + } + } } -} -/// Given a def, returns its name, the article to be used, and a disambiguator -/// for the type namespace. -fn type_ns_kind(def: Def, path_str: &str) -> (&'static str, &'static str, String) { - let (kind, article) = match def { - // We can still have non-tuple structs. - Def::Struct(..) => ("struct", "a"), - Def::Enum(..) => ("enum", "an"), - Def::Trait(..) => ("trait", "a"), - Def::Union(..) => ("union", "a"), - _ => ("type", "a"), - }; - (kind, article, format!("{}@{}", kind, path_str)) + let mut diag = cx.tcx.struct_span_lint_hir( + lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE, + hir::CRATE_HIR_ID, + sp, + &msg, + ); + + if let Some(link_range) = link_range { + if let Some(sp) = super::source_span_for_markdown_range(cx, dox, &link_range, attrs) { + diag.set_span(sp); + diag.span_label(sp, "ambiguous link"); + + for (def, ns) in candidates { + let (action, mut suggestion) = match def { + Def::Method(..) | Def::Fn(..) => { + ("add parentheses", format!("{}()", path_str)) + } + _ => { + let type_ = match (def, ns) { + (Def::Const(..), _) => "const", + (Def::Static(..), _) => "static", + (Def::Struct(..), _) => "struct", + (Def::Enum(..), _) => "enum", + (Def::Union(..), _) => "union", + (Def::Trait(..), _) => "trait", + (Def::Mod(..), _) => "module", + (_, Namespace::Type) => "type", + (_, Namespace::Value) => "value", + (_, Namespace::Macro) => "macro", + }; + + // FIXME: if this is an implied shortcut link, it's bad style to suggest `@` + ("prefix with the item type", format!("{}@{}", type_, path_str)) + } + }; + + if dox.bytes().nth(link_range.start) == Some(b'`') { + suggestion = format!("`{}`", suggestion); + } + + diag.span_suggestion( + sp, + &format!("to link to the {}, {}", def.kind_name(), action), + suggestion, + Applicability::MaybeIncorrect, + ); + } + } 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: {} } + +#[allow(non_camel_case_types)] +pub struct multi_conflict {} + +pub fn multi_conflict() {} + +pub mod type_and_value {} + +pub const type_and_value: i32 = 0; + +pub mod foo { + pub enum bar {} + + pub fn bar() {} +} + +/// [`ambiguous`] is ambiguous. //~ERROR `ambiguous` +/// +/// [ambiguous] is ambiguous. //~ERROR ambiguous +/// +/// [`multi_conflict`] is a three-way conflict. //~ERROR `multi_conflict` +/// +/// Ambiguous [type_and_value]. //~ERROR type_and_value +/// +/// Ambiguous non-implied shortcut link [`foo::bar`]. //~ERROR `foo::bar` +pub struct Docs {} diff --git a/src/test/rustdoc-ui/intra-links-ambiguity.stderr b/src/test/rustdoc-ui/intra-links-ambiguity.stderr new file mode 100644 index 0000000000000..3506e7f29c413 --- /dev/null +++ b/src/test/rustdoc-ui/intra-links-ambiguity.stderr @@ -0,0 +1,82 @@ +error: `ambiguous` is both a struct and a function + --> $DIR/intra-links-ambiguity.rs:27:6 + | +LL | /// [`ambiguous`] is ambiguous. + | ^^^^^^^^^^^ ambiguous link + | +note: lint level defined here + --> $DIR/intra-links-ambiguity.rs:1:9 + | +LL | #![deny(intra_doc_link_resolution_failure)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the struct, prefix with the item type + | +LL | /// [`struct@ambiguous`] is ambiguous. + | ^^^^^^^^^^^^^^^^^^ +help: to link to the function, add parentheses + | +LL | /// [`ambiguous()`] is ambiguous. + | ^^^^^^^^^^^^^ + +error: `ambiguous` is both a struct and a function + --> $DIR/intra-links-ambiguity.rs:29:6 + | +LL | /// [ambiguous] is ambiguous. + | ^^^^^^^^^ ambiguous link +help: to link to the struct, prefix with the item type + | +LL | /// [struct@ambiguous] is ambiguous. + | ^^^^^^^^^^^^^^^^ +help: to link to the function, add parentheses + | +LL | /// [ambiguous()] is ambiguous. + | ^^^^^^^^^^^ + +error: `multi_conflict` is a macro, a struct, and a function + --> $DIR/intra-links-ambiguity.rs:31:6 + | +LL | /// [`multi_conflict`] is a three-way conflict. + | ^^^^^^^^^^^^^^^^ ambiguous link +help: to link to the macro, prefix with the item type + | +LL | /// [`macro@multi_conflict`] is a three-way conflict. + | ^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the struct, prefix with the item type + | +LL | /// [`struct@multi_conflict`] is a three-way conflict. + | ^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the function, add parentheses + | +LL | /// [`multi_conflict()`] is a three-way conflict. + | ^^^^^^^^^^^^^^^^^^ + +error: `type_and_value` is both a module and a constant + --> $DIR/intra-links-ambiguity.rs:33:16 + | +LL | /// Ambiguous [type_and_value]. + | ^^^^^^^^^^^^^^ ambiguous link +help: to link to the module, prefix with the item type + | +LL | /// Ambiguous [module@type_and_value]. + | ^^^^^^^^^^^^^^^^^^^^^ +help: to link to the constant, prefix with the item type + | +LL | /// Ambiguous [const@type_and_value]. + | ^^^^^^^^^^^^^^^^^^^^ + +error: `foo::bar` is both an enum and a function + --> $DIR/intra-links-ambiguity.rs:35:42 + | +LL | /// Ambiguous non-implied shortcut link [`foo::bar`]. + | ^^^^^^^^^^ ambiguous link +help: to link to the enum, prefix with the item type + | +LL | /// Ambiguous non-implied shortcut link [`enum@foo::bar`]. + | ^^^^^^^^^^^^^^^ +help: to link to the function, add parentheses + | +LL | /// Ambiguous non-implied shortcut link [`foo::bar()`]. + | ^^^^^^^^^^^^ + +error: aborting due to 5 previous errors + diff --git a/src/test/rustdoc/intra-links.rs b/src/test/rustdoc/intra-links.rs index 9139fc51b092e..c356ab3a8ac52 100644 --- a/src/test/rustdoc/intra-links.rs +++ b/src/test/rustdoc/intra-links.rs @@ -22,6 +22,7 @@ //! * [`ThisType::this_method`](ThisType::this_method) //! * [`ThisEnum`](ThisEnum) //! * [`ThisEnum::ThisVariant`](ThisEnum::ThisVariant) +//! * [`ThisEnum::ThisVariantCtor`](ThisEnum::ThisVariantCtor) //! * [`ThisTrait`](ThisTrait) //! * [`ThisTrait::this_associated_method`](ThisTrait::this_associated_method) //! * [`ThisTrait::ThisAssociatedType`](ThisTrait::ThisAssociatedType) @@ -50,7 +51,7 @@ pub struct ThisType; impl ThisType { pub fn this_method() {} } -pub enum ThisEnum { ThisVariant, } +pub enum ThisEnum { ThisVariant, ThisVariantCtor(u32), } pub trait ThisTrait { type ThisAssociatedType; const THIS_ASSOCIATED_CONST: u8; From 150b49a582f06b26391a6de542e9d4bc12c9aa2d Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Sat, 9 Mar 2019 16:25:12 -0500 Subject: [PATCH 2/3] replace ad-hoc namespace enums --- .../passes/collect_intra_doc_links.rs | 165 +++++++----------- .../rustdoc-ui/intra-links-ambiguity.stderr | 10 +- 2 files changed, 72 insertions(+), 103 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 36bfe2fb2c3da..b105e0a0b6fcc 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1,5 +1,5 @@ use errors::Applicability; -use rustc::hir::def::Def; +use rustc::hir::def::{Def, Namespace::{self, *}, PerNS}; use rustc::hir::def_id::DefId; use rustc::hir; use rustc::lint as lint; @@ -36,29 +36,9 @@ pub fn collect_intra_doc_links(krate: Crate, cx: &DocContext<'_>) -> Crate { } } -#[derive(Debug)] -enum PathKind { - /// Either a value or type, but not a macro - Unknown, - /// Macro - Macro, - /// Values, functions, consts, statics (everything in the value namespace) - Value, - /// Types, traits (everything in the type namespace) - Type, -} - struct LinkCollector<'a, 'tcx> { cx: &'a DocContext<'tcx>, mod_ids: Vec, - is_nightly_build: bool, -} - -#[derive(Debug, Copy, Clone)] -enum Namespace { - Type, - Value, - Macro, } impl<'a, 'tcx> LinkCollector<'a, 'tcx> { @@ -66,16 +46,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { LinkCollector { cx, mod_ids: Vec::new(), - is_nightly_build: UnstableFeatures::from_environment().is_nightly_build(), } } - /// Resolves a given string as a path, along with whether or not it is - /// in the value namespace. Also returns an optional URL fragment in the case - /// of variants and methods. + /// Resolves a string as a path within a particular namespace. Also returns an optional + /// URL fragment in the case of variants and methods. fn resolve(&self, path_str: &str, - is_val: bool, + ns: Namespace, current_item: &Option, parent_id: Option) -> Result<(Def, Option), ()> @@ -86,11 +64,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // path. if let Some(id) = parent_id.or(self.mod_ids.last().cloned()) { // FIXME: `with_scope` requires the `NodeId` of a module. - let result = cx.enter_resolver(|resolver| resolver.with_scope(id, - |resolver| { - resolver.resolve_str_path_error(DUMMY_SP, - &path_str, is_val) - })); + let result = cx.enter_resolver(|resolver| { + resolver.with_scope(id, |resolver| { + resolver.resolve_str_path_error(DUMMY_SP, &path_str, ns == ValueNS) + }) + }); if let Ok(result) = result { // In case this is a trait item, skip the @@ -103,16 +81,16 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { _ => return Ok((result.def, None)) }; - if value != is_val { + if value != (ns == ValueNS) { return Err(()) } - } else if let Some(prim) = is_primitive(path_str, is_val) { + } else if let Some(prim) = is_primitive(path_str, ns) { return Ok((prim, Some(path_str.to_owned()))) } else { // If resolution failed, it may still be a method // because methods are not handled by the resolver // If so, bail when we're not looking for a value. - if !is_val { + if ns != ValueNS { return Err(()) } } @@ -136,7 +114,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { path = name.clone(); } } - if let Some(prim) = is_primitive(&path, false) { + if let Some(prim) = is_primitive(&path, TypeNS) { let did = primitive_impl(cx, &path).ok_or(())?; return cx.tcx.associated_items(did) .find(|item| item.ident.name == item_name) @@ -160,8 +138,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .find(|item| item.ident.name == item_name); if let Some(item) = item { let out = match item.kind { - ty::AssociatedKind::Method if is_val => "method", - ty::AssociatedKind::Const if is_val => "associatedconstant", + ty::AssociatedKind::Method if ns == ValueNS => "method", + ty::AssociatedKind::Const if ns == ValueNS => "associatedconstant", _ => return Err(()) }; Ok((ty.def, Some(format!("{}.{}", out, item_name)))) @@ -198,9 +176,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .find(|item| item.ident.name == item_name); if let Some(item) = item { let kind = match item.kind { - ty::AssociatedKind::Const if is_val => "associatedconstant", - ty::AssociatedKind::Type if !is_val => "associatedtype", - ty::AssociatedKind::Method if is_val => { + ty::AssociatedKind::Const if ns == ValueNS => "associatedconstant", + ty::AssociatedKind::Type if ns == TypeNS => "associatedtype", + ty::AssociatedKind::Method if ns == ValueNS => { if item.defaultness.has_value() { "method" } else { @@ -287,10 +265,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { look_for_tests(&cx, &dox, &item, true); - if !self.is_nightly_build { - return None; - } - for (ori_link, link_range) in markdown_links(&dox) { // Bail early for real links. if ori_link.contains('/') { @@ -298,28 +272,28 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } let link = ori_link.replace("`", ""); let (def, fragment) = { - let mut kind = PathKind::Unknown; + let mut kind = None; let path_str = if let Some(prefix) = ["struct@", "enum@", "type@", "trait@", "union@"].iter() .find(|p| link.starts_with(**p)) { - kind = PathKind::Type; + kind = Some(TypeNS); link.trim_start_matches(prefix) } else if let Some(prefix) = ["const@", "static@", "value@", "function@", "mod@", "fn@", "module@", "method@"] .iter().find(|p| link.starts_with(**p)) { - kind = PathKind::Value; + kind = Some(ValueNS); link.trim_start_matches(prefix) } else if link.ends_with("()") { - kind = PathKind::Value; + kind = Some(ValueNS); link.trim_end_matches("()") } else if link.starts_with("macro@") { - kind = PathKind::Macro; + kind = Some(MacroNS); link.trim_start_matches("macro@") } else if link.ends_with('!') { - kind = PathKind::Macro; + kind = Some(MacroNS); link.trim_end_matches('!') } else { &link[..] @@ -331,8 +305,8 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } match kind { - PathKind::Value => { - if let Ok(def) = self.resolve(path_str, true, ¤t_item, parent_node) { + Some(ns @ ValueNS) => { + if let Ok(def) = self.resolve(path_str, ns, ¤t_item, parent_node) { def } else { resolution_failure(cx, &item.attrs, path_str, &dox, link_range); @@ -342,8 +316,8 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { continue; } } - PathKind::Type => { - if let Ok(def) = self.resolve(path_str, false, ¤t_item, parent_node) { + Some(ns @ TypeNS) => { + if let Ok(def) = self.resolve(path_str, ns, ¤t_item, parent_node) { def } else { resolution_failure(cx, &item.attrs, path_str, &dox, link_range); @@ -351,57 +325,49 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { continue; } } - PathKind::Unknown => { + None => { // Try everything! - let mut candidates = vec![]; - - if let Some(macro_def) = macro_resolve(cx, path_str) { - candidates.push(((macro_def, None), Namespace::Macro)); - } - - if let Ok(type_def) = - self.resolve(path_str, false, ¤t_item, parent_node) - { - candidates.push((type_def, Namespace::Type)); - } - - if let Ok(value_def) = - self.resolve(path_str, true, ¤t_item, parent_node) - { - // Structs, variants, and mods exist in both namespaces, skip them. - match value_def.0 { - Def::StructCtor(..) - | Def::Mod(..) - | Def::Variant(..) - | Def::VariantCtor(..) - | Def::SelfCtor(..) => (), - _ => candidates.push((value_def, Namespace::Value)), - } - } + let candidates = PerNS { + macro_ns: macro_resolve(cx, path_str).map(|def| (def, None)), + type_ns: self + .resolve(path_str, TypeNS, ¤t_item, parent_node) + .ok(), + value_ns: self + .resolve(path_str, ValueNS, ¤t_item, parent_node) + .ok() + .and_then(|(def, fragment)| { + // Constructors are picked up in the type namespace. + match def { + Def::StructCtor(..) + | Def::VariantCtor(..) + | Def::SelfCtor(..) => None, + _ => Some((def, fragment)) + } + }), + }; - if candidates.len() == 1 { - candidates.remove(0).0 - } else if candidates.is_empty() { + if candidates.is_empty() { resolution_failure(cx, &item.attrs, path_str, &dox, link_range); // this could just be a normal link continue; - } else { - let candidates = candidates.into_iter().map(|((def, _), ns)| { - (def, ns) - }).collect::>(); + } + let is_unambiguous = candidates.clone().present_items().count() == 1; + if is_unambiguous { + candidates.present_items().next().unwrap() + } else { ambiguity_error( cx, &item.attrs, path_str, &dox, link_range, - &candidates, + candidates.map(|candidate| candidate.map(|(def, _)| def)), ); continue; } } - PathKind::Macro => { + Some(MacroNS) => { if let Some(def) = macro_resolve(cx, path_str) { (def, None) } else { @@ -514,13 +480,16 @@ fn ambiguity_error( path_str: &str, dox: &str, link_range: Option>, - candidates: &[(Def, Namespace)], + candidates: PerNS>, ) { let sp = span_of_attrs(attrs); let mut msg = format!("`{}` is ", path_str); - match candidates { + let candidates = [TypeNS, ValueNS, MacroNS].iter().filter_map(|&ns| { + candidates[ns].map(|def| (def, ns)) + }).collect::>(); + match candidates.as_slice() { [(first_def, _), (second_def, _)] => { msg += &format!( "both {} {} and {} {}", @@ -568,9 +537,9 @@ fn ambiguity_error( (Def::Union(..), _) => "union", (Def::Trait(..), _) => "trait", (Def::Mod(..), _) => "module", - (_, Namespace::Type) => "type", - (_, Namespace::Value) => "value", - (_, Namespace::Macro) => "macro", + (_, TypeNS) => "type", + (_, ValueNS) => "value", + (_, MacroNS) => "macro", }; // FIXME: if this is an implied shortcut link, it's bad style to suggest `@` @@ -646,11 +615,11 @@ const PRIMITIVES: &[(&str, Def)] = &[ ("char", Def::PrimTy(hir::PrimTy::Char)), ]; -fn is_primitive(path_str: &str, is_val: bool) -> Option { - if is_val { - None - } else { +fn is_primitive(path_str: &str, ns: Namespace) -> Option { + if ns == TypeNS { PRIMITIVES.iter().find(|x| x.0 == path_str).map(|x| x.1) + } else { + None } } diff --git a/src/test/rustdoc-ui/intra-links-ambiguity.stderr b/src/test/rustdoc-ui/intra-links-ambiguity.stderr index 3506e7f29c413..9793cd06fb0e8 100644 --- a/src/test/rustdoc-ui/intra-links-ambiguity.stderr +++ b/src/test/rustdoc-ui/intra-links-ambiguity.stderr @@ -32,15 +32,11 @@ help: to link to the function, add parentheses LL | /// [ambiguous()] is ambiguous. | ^^^^^^^^^^^ -error: `multi_conflict` is a macro, a struct, and a function +error: `multi_conflict` is a struct, a function, and a macro --> $DIR/intra-links-ambiguity.rs:31:6 | LL | /// [`multi_conflict`] is a three-way conflict. | ^^^^^^^^^^^^^^^^ ambiguous link -help: to link to the macro, prefix with the item type - | -LL | /// [`macro@multi_conflict`] is a three-way conflict. - | ^^^^^^^^^^^^^^^^^^^^^^ help: to link to the struct, prefix with the item type | LL | /// [`struct@multi_conflict`] is a three-way conflict. @@ -49,6 +45,10 @@ help: to link to the function, add parentheses | LL | /// [`multi_conflict()`] is a three-way conflict. | ^^^^^^^^^^^^^^^^^^ +help: to link to the macro, prefix with the item type + | +LL | /// [`macro@multi_conflict`] is a three-way conflict. + | ^^^^^^^^^^^^^^^^^^^^^^ error: `type_and_value` is both a module and a constant --> $DIR/intra-links-ambiguity.rs:33:16 From 7c66ae2fc5eb15a5f9b3c7a0b2e18528c54e88a6 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Sat, 9 Mar 2019 16:32:07 -0500 Subject: [PATCH 3/3] use `!` in macro disambiguation suggestion --- src/librustdoc/passes/collect_intra_doc_links.rs | 3 +++ src/test/rustdoc-ui/intra-links-ambiguity.stderr | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index b105e0a0b6fcc..0fa6b6baec79d 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -528,6 +528,9 @@ fn ambiguity_error( Def::Method(..) | Def::Fn(..) => { ("add parentheses", format!("{}()", path_str)) } + Def::Macro(..) => { + ("add an exclamation mark", format!("{}!", path_str)) + } _ => { let type_ = match (def, ns) { (Def::Const(..), _) => "const", diff --git a/src/test/rustdoc-ui/intra-links-ambiguity.stderr b/src/test/rustdoc-ui/intra-links-ambiguity.stderr index 9793cd06fb0e8..5d66cc1364c5f 100644 --- a/src/test/rustdoc-ui/intra-links-ambiguity.stderr +++ b/src/test/rustdoc-ui/intra-links-ambiguity.stderr @@ -45,10 +45,10 @@ help: to link to the function, add parentheses | LL | /// [`multi_conflict()`] is a three-way conflict. | ^^^^^^^^^^^^^^^^^^ -help: to link to the macro, prefix with the item type +help: to link to the macro, add an exclamation mark | -LL | /// [`macro@multi_conflict`] is a three-way conflict. - | ^^^^^^^^^^^^^^^^^^^^^^ +LL | /// [`multi_conflict!`] is a three-way conflict. + | ^^^^^^^^^^^^^^^^^ error: `type_and_value` is both a module and a constant --> $DIR/intra-links-ambiguity.rs:33:16