diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index d18335ef2e63a..f683e09caa234 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -3198,8 +3198,8 @@ impl<'a> Resolver<'a> { let path = if path_str.starts_with("::") { ast::Path { span, - segments: iter::once(Ident::with_dummy_span(kw::PathRoot)) - .chain(path_str.split("::").skip(1).map(Ident::from_str)) + segments: iter::once(Ident::new(kw::PathRoot, span)) + .chain(path_str.split("::").skip(1).map(|s| Ident::from_str_and_span(s, span))) .map(|i| self.new_ast_path_segment(i)) .collect(), tokens: None, @@ -3209,7 +3209,7 @@ impl<'a> Resolver<'a> { span, segments: path_str .split("::") - .map(Ident::from_str) + .map(|s| Ident::from_str_and_span(s, span)) .map(|i| self.new_ast_path_segment(i)) .collect(), tokens: None, diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index fd0dd339abdc2..f3b15a0d434ec 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -12,7 +12,7 @@ use rustc_hir::def::{ Namespace::{self, *}, PerNS, Res, }; -use rustc_hir::def_id::{CrateNum, DefId}; +use rustc_hir::def_id::DefId; use rustc_middle::ty; use rustc_resolve::ParentScope; use rustc_session::lint::{ @@ -23,7 +23,7 @@ use rustc_span::hygiene::MacroKind; use rustc_span::symbol::sym; use rustc_span::symbol::Ident; use rustc_span::symbol::Symbol; -use rustc_span::DUMMY_SP; +use rustc_span::Span; use smallvec::{smallvec, SmallVec}; use std::borrow::Cow; @@ -194,6 +194,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// [enum struct variant]: hir::VariantData::Struct fn variant_field( &self, + span: Span, path_str: &'path str, current_item: &Option, module_id: DefId, @@ -231,7 +232,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .ok_or_else(no_res)?; let ty_res = cx .enter_resolver(|resolver| { - resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id) + resolver.resolve_str_path_error(span, &path, TypeNS, module_id) }) .map(|(_, res)| res) .unwrap_or(Res::Err); @@ -289,6 +290,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// lifetimes on `&'path` will work. fn resolve_primitive_associated_item( &self, + span: Span, prim_ty: hir::PrimTy, ns: Namespace, module_id: DefId, @@ -303,12 +305,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .find_map(|&impl_| { cx.tcx .associated_items(impl_) - .find_by_name_and_namespace( - cx.tcx, - Ident::with_dummy_span(item_name), - ns, - impl_, - ) + .find_by_name_and_namespace(cx.tcx, Ident::new(item_name, span), ns, impl_) .map(|item| match item.kind { ty::AssocKind::Fn => "method", ty::AssocKind::Const => "associatedconstant", @@ -342,11 +339,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// FIXME(jynelson): Can this be unified with `resolve()`? fn resolve_macro( &self, + span: Span, path_str: &'a str, module_id: DefId, ) -> Result> { let cx = self.cx; - let path = ast::Path::from_ident(Ident::from_str(path_str)); + let path = ast::Path::from_ident(Ident::from_str_and_span(path_str, span)); cx.enter_resolver(|resolver| { // FIXME(jynelson): does this really need 3 separate lookups? if let Ok((Some(ext), res)) = resolver.resolve_macro_path( @@ -365,7 +363,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } debug!("resolving {} as a macro in the module {:?}", path_str, module_id); if let Ok((_, res)) = - resolver.resolve_str_path_error(DUMMY_SP, path_str, MacroNS, module_id) + resolver.resolve_str_path_error(span, path_str, MacroNS, module_id) { // don't resolve builtins like `#[derive]` if let Res::Def(..) = res { @@ -386,9 +384,15 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// This also handles resolving `true` and `false` as booleans. /// NOTE: `resolve_str_path_error` knows only about paths, not about types. /// Associated items will never be resolved by this function. - fn resolve_path(&self, path_str: &str, ns: Namespace, module_id: DefId) -> Option { + fn resolve_path( + &self, + span: Span, + path_str: &str, + ns: Namespace, + module_id: DefId, + ) -> Option { let result = self.cx.enter_resolver(|resolver| { - resolver.resolve_str_path_error(DUMMY_SP, &path_str, ns, module_id) + resolver.resolve_str_path_error(span, &path_str, ns, module_id) }); debug!("{} resolved to {:?} in namespace {:?}", path_str, result, ns); match result.map(|(_, res)| res) { @@ -403,6 +407,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// optional URL fragment in the case of variants and methods. fn resolve<'path>( &self, + span: Span, path_str: &'path str, ns: Namespace, // FIXME(#76467): This is for `Self`, and it's wrong. @@ -412,7 +417,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ) -> Result<(Res, Option), ErrorKind<'path>> { let cx = self.cx; - if let Some(res) = self.resolve_path(path_str, ns, module_id) { + if let Some(res) = self.resolve_path(span, path_str, ns, module_id) { match res { // FIXME(#76467): make this fallthrough to lookup the associated // item a separate function. @@ -471,13 +476,13 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // FIXME: are these both necessary? let ty_res = if let Some(ty_res) = resolve_primitive(&path_root, TypeNS) .map(|(_, res)| res) - .or_else(|| self.resolve_path(&path_root, TypeNS, module_id)) + .or_else(|| self.resolve_path(span, &path_root, TypeNS, module_id)) { ty_res } else { // FIXME: this is duplicated on the end of this function. return if ns == Namespace::ValueNS { - self.variant_field(path_str, current_item, module_id) + self.variant_field(span, path_str, current_item, module_id) } else { Err(ResolutionFailure::NotResolved { module_id, @@ -489,9 +494,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; let res = match ty_res { - Res::PrimTy(prim) => Some( - self.resolve_primitive_associated_item(prim, ns, module_id, item_name, item_str), - ), + Res::PrimTy(prim) => { + Some(self.resolve_primitive_associated_item( + span, prim, ns, module_id, item_name, item_str, + )) + } Res::Def(DefKind::Struct | DefKind::Union | DefKind::Enum | DefKind::TyAlias, did) => { debug!("looking for associated item named {} for item {:?}", item_name, did); // Checks if item_name belongs to `impl SomeItem` @@ -502,7 +509,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .flat_map(|&imp| { cx.tcx.associated_items(imp).find_by_name_and_namespace( cx.tcx, - Ident::with_dummy_span(item_name), + Ident::new(item_name, span), ns, imp, ) @@ -516,8 +523,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // To handle that properly resolve() would have to support // something like [`ambi_fn`](::ambi_fn) .or_else(|| { - let kind = - resolve_associated_trait_item(did, module_id, item_name, ns, &self.cx); + let kind = resolve_associated_trait_item( + span, did, module_id, item_name, ns, &self.cx, + ); debug!("got associated item kind {:?}", kind); kind }); @@ -585,7 +593,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { Res::Def(DefKind::Trait, did) => cx .tcx .associated_items(did) - .find_by_name_and_namespace(cx.tcx, Ident::with_dummy_span(item_name), ns, did) + .find_by_name_and_namespace(cx.tcx, Ident::new(item_name, span), ns, did) .map(|item| { let kind = match item.kind { ty::AssocKind::Const => "associatedconstant", @@ -610,7 +618,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; res.unwrap_or_else(|| { if ns == Namespace::ValueNS { - self.variant_field(path_str, current_item, module_id) + self.variant_field(span, path_str, current_item, module_id) } else { Err(ResolutionFailure::NotResolved { module_id, @@ -630,6 +638,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// (such as having invalid URL fragments or being in the wrong namespace). fn check_full_res( &self, + span: Span, ns: Namespace, path_str: &str, module_id: DefId, @@ -638,9 +647,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ) -> Option { // resolve() can't be used for macro namespace let result = match ns { - Namespace::MacroNS => self.resolve_macro(path_str, module_id).map_err(ErrorKind::from), + Namespace::MacroNS => { + self.resolve_macro(span, path_str, module_id).map_err(ErrorKind::from) + } Namespace::TypeNS | Namespace::ValueNS => self - .resolve(path_str, ns, current_item, module_id, extra_fragment) + .resolve(span, path_str, ns, current_item, module_id, extra_fragment) .map(|(res, _)| res), }; @@ -660,6 +671,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// find `std::error::Error::source` and return /// `::source`. fn resolve_associated_trait_item( + span: Span, did: DefId, module: DefId, item_name: Symbol, @@ -711,7 +723,7 @@ fn resolve_associated_trait_item( .associated_items(trait_) .find_by_name_and_namespace( cx.tcx, - Ident::with_dummy_span(item_name), + Ident::new(item_name, span), ns, trait_, ) @@ -731,7 +743,7 @@ fn resolve_associated_trait_item( candidates.extend(traits.iter().filter_map(|&trait_| { cx.tcx .associated_items(trait_) - .find_by_name_and_namespace(cx.tcx, Ident::with_dummy_span(item_name), ns, trait_) + .find_by_name_and_namespace(cx.tcx, Ident::new(item_name, span), ns, trait_) .map(|assoc| (assoc.kind, assoc.def_id)) })); } @@ -925,12 +937,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } debug!("combined_docs={}", combined_docs); - let (krate, parent_node) = if let Some(id) = attr.parent_module { + let parent_node = if let Some(id) = attr.parent_module { trace!("docs {:?} came from {:?}", attr.doc, id); - (id.krate, Some(id)) + Some(id) } else { trace!("no parent found for {:?}", attr.doc); - (item.def_id.krate, parent_node) + parent_node }; // NOTE: if there are links that start in one crate and end in another, this will not resolve them. // This is a degenerate case and it's not supported by rustdoc. @@ -941,7 +953,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { ¤t_item, parent_node, &parent_name, - krate, ori_link, link_range, ); @@ -976,7 +987,6 @@ impl LinkCollector<'_, '_> { current_item: &Option, parent_node: Option, parent_name: &Option, - krate: CrateNum, ori_link: String, link_range: Option>, ) -> Option { @@ -1041,7 +1051,7 @@ impl LinkCollector<'_, '_> { parent_node }; - let mut module_id = if let Some(id) = base_node { + let module_id = if let Some(id) = base_node { id } else { // This is a bug. @@ -1063,21 +1073,14 @@ impl LinkCollector<'_, '_> { // replace `Self` with suitable item's parent name if path_str.starts_with("Self::") { if let Some(ref name) = parent_name { - resolved_self = format!("{}::{}", name, &path_str[6..]); + resolved_self = format!("{}::{}", name, &path_str["Self::".len()..]); path_str = &resolved_self; } - } else if path_str.starts_with("crate::") { - use rustc_span::def_id::CRATE_DEF_INDEX; - - // HACK(jynelson): rustc_resolve thinks that `crate` is the crate currently being documented. - // But rustdoc wants it to mean the crate this item was originally present in. - // To work around this, remove it and resolve relative to the crate root instead. - // HACK(jynelson)(2): If we just strip `crate::` then suddenly primitives become ambiguous - // (consider `crate::char`). Instead, change it to `self::`. This works because 'self' is now the crate root. - // FIXME(#78696): This doesn't always work. - resolved_self = format!("self::{}", &path_str["crate::".len()..]); + } else if path_str.starts_with("crate::") || path_str == "crate" { + // Resolve `crate` relative to the original scope of the item, not the current scope. + // Note that this depends on giving `rustc_resolve` the correct span for the item. + resolved_self = format!("${}", path_str); // Changes `crate::` to `$crate::` path_str = &resolved_self; - module_id = DefId { krate, index: CRATE_DEF_INDEX }; } // Strip generics from the path. @@ -1242,9 +1245,11 @@ impl LinkCollector<'_, '_> { ori_link: &str, link_range: Option>, ) -> Option<(Res, Option)> { + let span = item.source.span(); + match disambiguator.map(Disambiguator::ns) { Some(ns @ (ValueNS | TypeNS)) => { - match self.resolve(path_str, ns, ¤t_item, base_node, &extra_fragment) { + match self.resolve(span, path_str, ns, ¤t_item, base_node, &extra_fragment) { Ok(res) => Some(res), Err(ErrorKind::Resolve(box mut kind)) => { // We only looked in one namespace. Try to give a better error if possible. @@ -1254,6 +1259,7 @@ impl LinkCollector<'_, '_> { // See https://github.com/rust-lang/rust/pull/76955#discussion_r493953382 for a good approach for &new_ns in &[other_ns, MacroNS] { if let Some(res) = self.check_full_res( + span, new_ns, path_str, base_node, @@ -1289,9 +1295,10 @@ impl LinkCollector<'_, '_> { // Try everything! let mut candidates = PerNS { macro_ns: self - .resolve_macro(path_str, base_node) + .resolve_macro(span, path_str, base_node) .map(|res| (res, extra_fragment.clone())), type_ns: match self.resolve( + span, path_str, TypeNS, ¤t_item, @@ -1309,6 +1316,7 @@ impl LinkCollector<'_, '_> { Err(ErrorKind::Resolve(box kind)) => Err(kind), }, value_ns: match self.resolve( + span, path_str, ValueNS, ¤t_item, @@ -1377,12 +1385,13 @@ impl LinkCollector<'_, '_> { } } Some(MacroNS) => { - match self.resolve_macro(path_str, base_node) { + match self.resolve_macro(span, path_str, base_node) { Ok(res) => Some((res, extra_fragment)), Err(mut kind) => { // `resolve_macro` only looks in the macro namespace. Try to give a better error if possible. for &ns in &[TypeNS, ValueNS] { if let Some(res) = self.check_full_res( + span, ns, path_str, base_node, @@ -1600,7 +1609,7 @@ fn report_diagnostic( item: &Item, dox: &str, link_range: &Option>, - decorate: impl FnOnce(&mut DiagnosticBuilder<'_>, Option), + decorate: impl FnOnce(&mut DiagnosticBuilder<'_>, Option), ) { let hir_id = match cx.as_local_hir_id(item.def_id) { Some(hir_id) => hir_id, @@ -1664,6 +1673,8 @@ fn resolution_failure( link_range: Option>, kinds: SmallVec<[ResolutionFailure<'_>; 3]>, ) { + let span = item.source.span(); + report_diagnostic( collector.cx, BROKEN_INTRA_DOC_LINKS, @@ -1727,7 +1738,7 @@ fn resolution_failure( name = start; for &ns in &[TypeNS, ValueNS, MacroNS] { if let Some(res) = - collector.check_full_res(ns, &start, module_id, &None, &None) + collector.check_full_res(span, ns, &start, module_id, &None, &None) { debug!("found partial_res={:?}", res); *partial_res = Some(res); @@ -1968,7 +1979,7 @@ fn suggest_disambiguator( diag: &mut DiagnosticBuilder<'_>, path_str: &str, dox: &str, - sp: Option, + sp: Option, link_range: &Option>, ) { let suggestion = disambiguator.suggestion();