From e433f5585296ba8892893e4c78f51d2d42ac7ea4 Mon Sep 17 00:00:00 2001 From: Kevin Jiang Date: Thu, 18 Mar 2021 00:15:39 -0400 Subject: [PATCH 01/13] Fixed diagnostic and added test for issue 81508 --- compiler/rustc_resolve/src/late.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 69 +++++++++++++++++++++----- src/test/ui/resolve/issue-81508.rs | 22 ++++++++ src/test/ui/resolve/issue-81508.stderr | 21 ++++++++ 4 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/resolve/issue-81508.rs create mode 100644 src/test/ui/resolve/issue-81508.stderr diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 1377bb781d008..987f1e6cc747a 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -1031,7 +1031,6 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } ItemKind::Static(ref ty, _, ref expr) | ItemKind::Const(_, ref ty, ref expr) => { - debug!("resolve_item ItemKind::Const"); self.with_item_rib(HasGenericParams::No, |this| { this.visit_ty(ty); if let Some(expr) = expr { @@ -1597,6 +1596,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { .try_resolve_as_non_binding(pat_src, pat, bmode, ident, has_sub) .unwrap_or_else(|| self.fresh_binding(ident, pat.id, pat_src, bindings)); self.r.record_partial_res(pat.id, PartialRes::new(res)); + self.r.record_local_span(pat.id, pat.span); } PatKind::TupleStruct(ref path, ref sub_patterns) => { self.smart_resolve_path( diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index fe5078f26bd98..3a8cb0fd09ad2 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -884,6 +884,10 @@ pub struct Resolver<'a> { /// "self-confirming" import resolutions during import validation. unusable_binding: Option<&'a NameBinding<'a>>, + // Spans for local variables found during resolution + // Used for suggestions during error reporting + local_span_map: NodeMap, + /// Resolutions for nodes that have a single resolution. partial_res_map: NodeMap, /// Resolutions for import nodes, which have multiple resolutions in different namespaces. @@ -1262,6 +1266,7 @@ impl<'a> Resolver<'a> { last_import_segment: false, unusable_binding: None, + local_span_map: Default::default(), partial_res_map: Default::default(), import_res_map: Default::default(), label_res_map: Default::default(), @@ -1879,7 +1884,6 @@ impl<'a> Resolver<'a> { ribs, ))); } - module = match ribs[i].kind { ModuleRibKind(module) => module, MacroDefinition(def) if def == self.macro_def(ident.span.ctxt()) => { @@ -1890,7 +1894,6 @@ impl<'a> Resolver<'a> { } _ => continue, }; - match module.kind { ModuleKind::Block(..) => {} // We can see through blocks _ => break, @@ -1909,17 +1912,19 @@ impl<'a> Resolver<'a> { return Some(LexicalScopeBinding::Item(binding)); } } + let returned_item = self + .early_resolve_ident_in_lexical_scope( + orig_ident, + ScopeSet::Late(ns, module, record_used_id), + parent_scope, + record_used, + record_used, + path_span, + ) + .ok() + .map(LexicalScopeBinding::Item); - self.early_resolve_ident_in_lexical_scope( - orig_ident, - ScopeSet::Late(ns, module, record_used_id), - parent_scope, - record_used, - record_used, - path_span, - ) - .ok() - .map(LexicalScopeBinding::Item) + returned_item } fn hygienic_lexical_parent( @@ -2386,7 +2391,40 @@ impl<'a> Resolver<'a> { .next() .map_or(false, |c| c.is_ascii_uppercase()) { - (format!("use of undeclared type `{}`", ident), None) + // Add check case for similarly named item in alternative namespace + let mut suggestion = None; + + if ribs.is_some() { + if let Some(res) = self.resolve_ident_in_lexical_scope( + ident, + ValueNS, + parent_scope, + None, + path_span, + &ribs.unwrap()[ValueNS], + ) { + let mut match_span: Option = None; + match res { + LexicalScopeBinding::Res(Res::Local(id)) => { + match_span = + Some(*self.local_span_map.get(&id).unwrap()); + } + LexicalScopeBinding::Item(name_binding) => { + match_span = Some(name_binding.span); + } + _ => (), + }; + if let Some(span) = match_span { + suggestion = Some(( + vec![(span, String::from(""))], + format!("{} is defined here, but is not a type", ident), + Applicability::MaybeIncorrect, + )); + } + } + } + + (format!("use of undeclared type `{}`", ident), suggestion) } else { (format!("use of undeclared crate or module `{}`", ident), None) } @@ -2797,6 +2835,11 @@ impl<'a> Resolver<'a> { } } + fn record_local_span(&mut self, node: NodeId, span: Span) { + debug!("(recording local) recording {:?} for {:?}", node, span); + self.local_span_map.insert(node, span); + } + fn is_accessible_from(&self, vis: ty::Visibility, module: Module<'a>) -> bool { vis.is_accessible_from(module.nearest_parent_mod, self) } diff --git a/src/test/ui/resolve/issue-81508.rs b/src/test/ui/resolve/issue-81508.rs new file mode 100644 index 0000000000000..23605cd2fd91d --- /dev/null +++ b/src/test/ui/resolve/issue-81508.rs @@ -0,0 +1,22 @@ +// Confusing diagnostic when using variable as a type: +// +// Previous warnings indicate Foo is not used, when in fact it is +// used improperly as a variable or constant. New warning points +// out user may be trying to use variable as a type. Test demonstrates +// cases for both local variable and const. + +fn main() { + let Baz: &str = ""; + + println!("{}", Baz::Bar); //~ ERROR: failed to resolve: use of undeclared type `Baz` +} + +#[allow(non_upper_case_globals)] +pub const Foo: &str = ""; + +mod submod { + use super::Foo; + fn function() { + println!("{}", Foo::Bar); //~ ERROR: failed to resolve: use of undeclared type `Foo` + } +} diff --git a/src/test/ui/resolve/issue-81508.stderr b/src/test/ui/resolve/issue-81508.stderr new file mode 100644 index 0000000000000..b0d5e1a269231 --- /dev/null +++ b/src/test/ui/resolve/issue-81508.stderr @@ -0,0 +1,21 @@ +error[E0433]: failed to resolve: use of undeclared type `Baz` + --> $DIR/issue-81508.rs:11:20 + | +LL | let Baz: &str = ""; + | --- help: Baz is defined here, but is not a type +LL | +LL | println!("{}", Baz::Bar); + | ^^^ use of undeclared type `Baz` + +error[E0433]: failed to resolve: use of undeclared type `Foo` + --> $DIR/issue-81508.rs:20:24 + | +LL | use super::Foo; + | ---------- help: Foo is defined here, but is not a type +LL | fn function() { +LL | println!("{}", Foo::Bar); + | ^^^ use of undeclared type `Foo` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0433`. From f51f25ab7de7c737ab67d79e23691b8f8f642a8c Mon Sep 17 00:00:00 2001 From: K Date: Wed, 7 Apr 2021 12:35:39 -0400 Subject: [PATCH 02/13] Added additional comments and minor edits --- compiler/rustc_resolve/src/late.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 104 +++++++++++++++---------- src/test/ui/resolve/issue-81508.stderr | 4 +- 3 files changed, 64 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 987f1e6cc747a..9321f11f65933 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -1596,7 +1596,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { .try_resolve_as_non_binding(pat_src, pat, bmode, ident, has_sub) .unwrap_or_else(|| self.fresh_binding(ident, pat.id, pat_src, bindings)); self.r.record_partial_res(pat.id, PartialRes::new(res)); - self.r.record_local_span(pat.id, pat.span); + self.r.record_pat_span(pat.id, pat.span); } PatKind::TupleStruct(ref path, ref sub_patterns) => { self.smart_resolve_path( diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 3a8cb0fd09ad2..665c32ddf069c 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -884,9 +884,9 @@ pub struct Resolver<'a> { /// "self-confirming" import resolutions during import validation. unusable_binding: Option<&'a NameBinding<'a>>, - // Spans for local variables found during resolution - // Used for suggestions during error reporting - local_span_map: NodeMap, + // Spans for local variables found during pattern resolution. + // Used for suggestions during error reporting. + pat_span_map: NodeMap, /// Resolutions for nodes that have a single resolution. partial_res_map: NodeMap, @@ -1266,7 +1266,7 @@ impl<'a> Resolver<'a> { last_import_segment: false, unusable_binding: None, - local_span_map: Default::default(), + pat_span_map: Default::default(), partial_res_map: Default::default(), import_res_map: Default::default(), label_res_map: Default::default(), @@ -1884,6 +1884,7 @@ impl<'a> Resolver<'a> { ribs, ))); } + module = match ribs[i].kind { ModuleRibKind(module) => module, MacroDefinition(def) if def == self.macro_def(ident.span.ctxt()) => { @@ -1894,6 +1895,7 @@ impl<'a> Resolver<'a> { } _ => continue, }; + match module.kind { ModuleKind::Block(..) => {} // We can see through blocks _ => break, @@ -1912,19 +1914,16 @@ impl<'a> Resolver<'a> { return Some(LexicalScopeBinding::Item(binding)); } } - let returned_item = self - .early_resolve_ident_in_lexical_scope( - orig_ident, - ScopeSet::Late(ns, module, record_used_id), - parent_scope, - record_used, - record_used, - path_span, - ) - .ok() - .map(LexicalScopeBinding::Item); - - returned_item + self.early_resolve_ident_in_lexical_scope( + orig_ident, + ScopeSet::Late(ns, module, record_used_id), + parent_scope, + record_used, + record_used, + path_span, + ) + .ok() + .map(LexicalScopeBinding::Item) } fn hygienic_lexical_parent( @@ -2391,11 +2390,9 @@ impl<'a> Resolver<'a> { .next() .map_or(false, |c| c.is_ascii_uppercase()) { - // Add check case for similarly named item in alternative namespace - let mut suggestion = None; - - if ribs.is_some() { - if let Some(res) = self.resolve_ident_in_lexical_scope( + // Check whether the name refers to an item in the value namespace. + let suggestion = if ribs.is_some() { + let match_span = match self.resolve_ident_in_lexical_scope( ident, ValueNS, parent_scope, @@ -2403,26 +2400,47 @@ impl<'a> Resolver<'a> { path_span, &ribs.unwrap()[ValueNS], ) { - let mut match_span: Option = None; - match res { - LexicalScopeBinding::Res(Res::Local(id)) => { - match_span = - Some(*self.local_span_map.get(&id).unwrap()); - } - LexicalScopeBinding::Item(name_binding) => { - match_span = Some(name_binding.span); - } - _ => (), - }; - if let Some(span) = match_span { - suggestion = Some(( - vec![(span, String::from(""))], - format!("{} is defined here, but is not a type", ident), - Applicability::MaybeIncorrect, - )); + // Name matches a local variable. For example: + // ``` + // fn f() { + // let Foo: &str = ""; + // println!("{}", Foo::Bar); // Name refers to local + // // variable `Foo`. + // } + // ``` + Some(LexicalScopeBinding::Res(Res::Local(id))) => { + Some(*self.pat_span_map.get(&id).unwrap()) } + + // Name matches item from a local name binding + // created by `use` declaration. For example: + // ``` + // pub Foo: &str = ""; + // + // mod submod { + // use super::Foo; + // println!("{}", Foo::Bar); // Name refers to local + // // binding `Foo`. + // } + // ``` + Some(LexicalScopeBinding::Item(name_binding)) => { + Some(name_binding.span) + } + _ => None, + }; + + if let Some(span) = match_span { + Some(( + vec![(span, String::from(""))], + format!("`{}` is defined here, but is not a type", ident), + Applicability::MaybeIncorrect, + )) + } else { + None } - } + } else { + None + }; (format!("use of undeclared type `{}`", ident), suggestion) } else { @@ -2835,9 +2853,9 @@ impl<'a> Resolver<'a> { } } - fn record_local_span(&mut self, node: NodeId, span: Span) { - debug!("(recording local) recording {:?} for {:?}", node, span); - self.local_span_map.insert(node, span); + fn record_pat_span(&mut self, node: NodeId, span: Span) { + debug!("(recording pat) recording {:?} for {:?}", node, span); + self.pat_span_map.insert(node, span); } fn is_accessible_from(&self, vis: ty::Visibility, module: Module<'a>) -> bool { diff --git a/src/test/ui/resolve/issue-81508.stderr b/src/test/ui/resolve/issue-81508.stderr index b0d5e1a269231..15555631b9047 100644 --- a/src/test/ui/resolve/issue-81508.stderr +++ b/src/test/ui/resolve/issue-81508.stderr @@ -2,7 +2,7 @@ error[E0433]: failed to resolve: use of undeclared type `Baz` --> $DIR/issue-81508.rs:11:20 | LL | let Baz: &str = ""; - | --- help: Baz is defined here, but is not a type + | --- help: `Baz` is defined here, but is not a type LL | LL | println!("{}", Baz::Bar); | ^^^ use of undeclared type `Baz` @@ -11,7 +11,7 @@ error[E0433]: failed to resolve: use of undeclared type `Foo` --> $DIR/issue-81508.rs:20:24 | LL | use super::Foo; - | ---------- help: Foo is defined here, but is not a type + | ---------- help: `Foo` is defined here, but is not a type LL | fn function() { LL | println!("{}", Foo::Bar); | ^^^ use of undeclared type `Foo` From d43ede10e4cc69547d1c05831ca43c9902304573 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 8 Apr 2021 10:16:15 -0700 Subject: [PATCH 03/13] Use more accurate spans for trait/impl method arg divergence --- compiler/rustc_middle/src/ty/error.rs | 13 ++- compiler/rustc_middle/src/ty/relate.rs | 6 ++ .../rustc_middle/src/ty/structural_impls.rs | 2 + .../rustc_typeck/src/check/compare_method.rs | 96 ++++--------------- src/test/ui/impl-trait/trait_type.stderr | 4 +- src/test/ui/issues/issue-20225.stderr | 12 +-- src/test/ui/issues/issue-21332.stderr | 4 +- src/test/ui/wrong-mul-method-signature.stderr | 12 +-- 8 files changed, 53 insertions(+), 96 deletions(-) diff --git a/compiler/rustc_middle/src/ty/error.rs b/compiler/rustc_middle/src/ty/error.rs index d295b17d902aa..008e6d015e879 100644 --- a/compiler/rustc_middle/src/ty/error.rs +++ b/compiler/rustc_middle/src/ty/error.rs @@ -36,6 +36,7 @@ pub enum TypeError<'tcx> { UnsafetyMismatch(ExpectedFound), AbiMismatch(ExpectedFound), Mutability, + ArgumentMutability(usize), TupleSize(ExpectedFound), FixedArraySize(ExpectedFound), ArgCount, @@ -46,6 +47,7 @@ pub enum TypeError<'tcx> { RegionsPlaceholderMismatch, Sorts(ExpectedFound>), + ArgumentSorts(ExpectedFound>, usize), IntMismatch(ExpectedFound), FloatMismatch(ExpectedFound), Traits(ExpectedFound), @@ -110,7 +112,7 @@ impl<'tcx> fmt::Display for TypeError<'tcx> { AbiMismatch(values) => { write!(f, "expected {} fn, found {} fn", values.expected, values.found) } - Mutability => write!(f, "types differ in mutability"), + ArgumentMutability(_) | Mutability => write!(f, "types differ in mutability"), TupleSize(values) => write!( f, "expected a tuple with {} element{}, \ @@ -142,7 +144,7 @@ impl<'tcx> fmt::Display for TypeError<'tcx> { br_string(br) ), RegionsPlaceholderMismatch => write!(f, "one type is more general than the other"), - Sorts(values) => ty::tls::with(|tcx| { + ArgumentSorts(values, _) | Sorts(values) => ty::tls::with(|tcx| { report_maybe_different( f, &values.expected.sort_string(tcx), @@ -199,10 +201,11 @@ impl<'tcx> TypeError<'tcx> { use self::TypeError::*; match self { CyclicTy(_) | CyclicConst(_) | UnsafetyMismatch(_) | Mismatch | AbiMismatch(_) - | FixedArraySize(_) | Sorts(_) | IntMismatch(_) | FloatMismatch(_) - | VariadicMismatch(_) | TargetFeatureCast(_) => false, + | FixedArraySize(_) | ArgumentSorts(..) | Sorts(_) | IntMismatch(_) + | FloatMismatch(_) | VariadicMismatch(_) | TargetFeatureCast(_) => false, Mutability + | ArgumentMutability(_) | TupleSize(_) | ArgCount | RegionsDoesNotOutlive(..) @@ -339,7 +342,7 @@ impl<'tcx> TyCtxt<'tcx> { use self::TypeError::*; debug!("note_and_explain_type_err err={:?} cause={:?}", err, cause); match err { - Sorts(values) => { + ArgumentSorts(values, _) | Sorts(values) => { match (values.expected.kind(), values.found.kind()) { (ty::Closure(..), ty::Closure(..)) => { db.note("no two closures, even if identical, have the same type"); diff --git a/compiler/rustc_middle/src/ty/relate.rs b/compiler/rustc_middle/src/ty/relate.rs index ca60339da0d00..b6f93c9bd59e7 100644 --- a/compiler/rustc_middle/src/ty/relate.rs +++ b/compiler/rustc_middle/src/ty/relate.rs @@ -179,6 +179,12 @@ impl<'tcx> Relate<'tcx> for ty::FnSig<'tcx> { } else { relation.relate_with_variance(ty::Contravariant, a, b) } + }) + .enumerate() + .map(|(i, r)| match r { + Err(TypeError::Sorts(exp_found)) => Err(TypeError::ArgumentSorts(exp_found, i)), + Err(TypeError::Mutability) => Err(TypeError::ArgumentMutability(i)), + r => r, }); Ok(ty::FnSig { inputs_and_output: tcx.mk_type_list(inputs_and_output)?, diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index a969626b370f5..7290c41d615df 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -587,6 +587,7 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> { UnsafetyMismatch(x) => UnsafetyMismatch(x), AbiMismatch(x) => AbiMismatch(x), Mutability => Mutability, + ArgumentMutability(i) => ArgumentMutability(i), TupleSize(x) => TupleSize(x), FixedArraySize(x) => FixedArraySize(x), ArgCount => ArgCount, @@ -607,6 +608,7 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> { CyclicTy(t) => return tcx.lift(t).map(|t| CyclicTy(t)), CyclicConst(ct) => return tcx.lift(ct).map(|ct| CyclicConst(ct)), ProjectionMismatched(x) => ProjectionMismatched(x), + ArgumentSorts(x, i) => return tcx.lift(x).map(|x| ArgumentSorts(x, i)), Sorts(x) => return tcx.lift(x).map(Sorts), ExistentialMismatch(x) => return tcx.lift(x).map(ExistentialMismatch), ConstMismatch(x) => return tcx.lift(x).map(ConstMismatch), diff --git a/compiler/rustc_typeck/src/check/compare_method.rs b/compiler/rustc_typeck/src/check/compare_method.rs index f044daa450918..3a337dadc88e9 100644 --- a/compiler/rustc_typeck/src/check/compare_method.rs +++ b/compiler/rustc_typeck/src/check/compare_method.rs @@ -278,9 +278,8 @@ fn compare_predicate_entailment<'tcx>( if let Err(terr) = sub_result { debug!("sub_types failed: impl ty {:?}, trait ty {:?}", impl_fty, trait_fty); - let (impl_err_span, trait_err_span) = extract_spans_for_error_reporting( - &infcx, param_env, &terr, &cause, impl_m, impl_sig, trait_m, trait_sig, - ); + let (impl_err_span, trait_err_span) = + extract_spans_for_error_reporting(&infcx, &terr, &cause, impl_m, trait_m); cause.make_mut().span = impl_err_span; @@ -291,7 +290,7 @@ fn compare_predicate_entailment<'tcx>( "method `{}` has an incompatible type for trait", trait_m.ident ); - if let TypeError::Mutability = terr { + if let TypeError::ArgumentMutability(_) = terr { if let Some(trait_err_span) = trait_err_span { if let Ok(trait_err_str) = tcx.sess.source_map().span_to_snippet(trait_err_span) { @@ -385,86 +384,35 @@ fn check_region_bounds_on_impl_item<'tcx>( fn extract_spans_for_error_reporting<'a, 'tcx>( infcx: &infer::InferCtxt<'a, 'tcx>, - param_env: ty::ParamEnv<'tcx>, terr: &TypeError<'_>, cause: &ObligationCause<'tcx>, impl_m: &ty::AssocItem, - impl_sig: ty::FnSig<'tcx>, trait_m: &ty::AssocItem, - trait_sig: ty::FnSig<'tcx>, ) -> (Span, Option) { let tcx = infcx.tcx; let impl_m_hir_id = tcx.hir().local_def_id_to_hir_id(impl_m.def_id.expect_local()); - let (impl_m_output, impl_m_iter) = match tcx.hir().expect_impl_item(impl_m_hir_id).kind { - ImplItemKind::Fn(ref impl_m_sig, _) => { - (&impl_m_sig.decl.output, impl_m_sig.decl.inputs.iter()) + let mut impl_args = match tcx.hir().expect_impl_item(impl_m_hir_id).kind { + ImplItemKind::Fn(ref sig, _) => { + sig.decl.inputs.iter().map(|t| t.span).chain(iter::once(sig.decl.output.span())) } _ => bug!("{:?} is not a method", impl_m), }; + let trait_args = trait_m.def_id.as_local().map(|def_id| { + let trait_m_hir_id = tcx.hir().local_def_id_to_hir_id(def_id); + match tcx.hir().expect_trait_item(trait_m_hir_id).kind { + TraitItemKind::Fn(ref sig, _) => { + sig.decl.inputs.iter().map(|t| t.span).chain(iter::once(sig.decl.output.span())) + } + _ => bug!("{:?} is not a TraitItemKind::Fn", trait_m), + } + }); match *terr { - TypeError::Mutability => { - if let Some(def_id) = trait_m.def_id.as_local() { - let trait_m_hir_id = tcx.hir().local_def_id_to_hir_id(def_id); - let trait_m_iter = match tcx.hir().expect_trait_item(trait_m_hir_id).kind { - TraitItemKind::Fn(ref trait_m_sig, _) => trait_m_sig.decl.inputs.iter(), - _ => bug!("{:?} is not a TraitItemKind::Fn", trait_m), - }; - - iter::zip(impl_m_iter, trait_m_iter) - .find(|&(ref impl_arg, ref trait_arg)| { - match (&impl_arg.kind, &trait_arg.kind) { - ( - &hir::TyKind::Rptr(_, ref impl_mt), - &hir::TyKind::Rptr(_, ref trait_mt), - ) - | (&hir::TyKind::Ptr(ref impl_mt), &hir::TyKind::Ptr(ref trait_mt)) => { - impl_mt.mutbl != trait_mt.mutbl - } - _ => false, - } - }) - .map(|(ref impl_arg, ref trait_arg)| (impl_arg.span, Some(trait_arg.span))) - .unwrap_or_else(|| (cause.span(tcx), tcx.hir().span_if_local(trait_m.def_id))) - } else { - (cause.span(tcx), tcx.hir().span_if_local(trait_m.def_id)) - } + TypeError::ArgumentMutability(i) => { + (impl_args.nth(i).unwrap(), trait_args.and_then(|mut args| args.nth(i))) } - TypeError::Sorts(ExpectedFound { .. }) => { - if let Some(def_id) = trait_m.def_id.as_local() { - let trait_m_hir_id = tcx.hir().local_def_id_to_hir_id(def_id); - let (trait_m_output, trait_m_iter) = - match tcx.hir().expect_trait_item(trait_m_hir_id).kind { - TraitItemKind::Fn(ref trait_m_sig, _) => { - (&trait_m_sig.decl.output, trait_m_sig.decl.inputs.iter()) - } - _ => bug!("{:?} is not a TraitItemKind::Fn", trait_m), - }; - - let impl_iter = impl_sig.inputs().iter(); - let trait_iter = trait_sig.inputs().iter(); - iter::zip(iter::zip(impl_iter, trait_iter), iter::zip(impl_m_iter, trait_m_iter)) - .find_map(|((&impl_arg_ty, &trait_arg_ty), (impl_arg, trait_arg))| match infcx - .at(&cause, param_env) - .sub(trait_arg_ty, impl_arg_ty) - { - Ok(_) => None, - Err(_) => Some((impl_arg.span, Some(trait_arg.span))), - }) - .unwrap_or_else(|| { - if infcx - .at(&cause, param_env) - .sup(trait_sig.output(), impl_sig.output()) - .is_err() - { - (impl_m_output.span(), Some(trait_m_output.span())) - } else { - (cause.span(tcx), tcx.hir().span_if_local(trait_m.def_id)) - } - }) - } else { - (cause.span(tcx), tcx.hir().span_if_local(trait_m.def_id)) - } + TypeError::ArgumentSorts(ExpectedFound { .. }, i) => { + (impl_args.nth(i).unwrap(), trait_args.and_then(|mut args| args.nth(i))) } _ => (cause.span(tcx), tcx.hir().span_if_local(trait_m.def_id)), } @@ -514,8 +462,7 @@ fn compare_self_type<'tcx>( tcx.sess, impl_m_span, E0185, - "method `{}` has a `{}` declaration in the impl, but \ - not in the trait", + "method `{}` has a `{}` declaration in the impl, but not in the trait", trait_m.ident, self_descr ); @@ -993,8 +940,7 @@ crate fn compare_const_impl<'tcx>( tcx.sess, cause.span, E0326, - "implemented const `{}` has an incompatible type for \ - trait", + "implemented const `{}` has an incompatible type for trait", trait_c.ident ); diff --git a/src/test/ui/impl-trait/trait_type.stderr b/src/test/ui/impl-trait/trait_type.stderr index 961bb7351181e..0bf43a65aa072 100644 --- a/src/test/ui/impl-trait/trait_type.stderr +++ b/src/test/ui/impl-trait/trait_type.stderr @@ -1,8 +1,8 @@ error[E0053]: method `fmt` has an incompatible type for trait - --> $DIR/trait_type.rs:7:4 + --> $DIR/trait_type.rs:7:21 | LL | fn fmt(&self, x: &str) -> () { } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ in mutability + | ^^^^ types differ in mutability | = note: expected fn pointer `fn(&MyType, &mut Formatter<'_>) -> Result<(), std::fmt::Error>` found fn pointer `fn(&MyType, &str)` diff --git a/src/test/ui/issues/issue-20225.stderr b/src/test/ui/issues/issue-20225.stderr index 3bcc50ded8425..756301de17c29 100644 --- a/src/test/ui/issues/issue-20225.stderr +++ b/src/test/ui/issues/issue-20225.stderr @@ -1,33 +1,33 @@ error[E0053]: method `call` has an incompatible type for trait - --> $DIR/issue-20225.rs:6:3 + --> $DIR/issue-20225.rs:6:43 | LL | impl<'a, T> Fn<(&'a T,)> for Foo { | - this type parameter LL | extern "rust-call" fn call(&self, (_,): (T,)) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&T`, found type parameter `T` + | ^^^^ expected `&T`, found type parameter `T` | = note: expected fn pointer `extern "rust-call" fn(&Foo, (&'a T,))` found fn pointer `extern "rust-call" fn(&Foo, (T,))` error[E0053]: method `call_mut` has an incompatible type for trait - --> $DIR/issue-20225.rs:11:3 + --> $DIR/issue-20225.rs:11:51 | LL | impl<'a, T> FnMut<(&'a T,)> for Foo { | - this type parameter LL | extern "rust-call" fn call_mut(&mut self, (_,): (T,)) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&T`, found type parameter `T` + | ^^^^ expected `&T`, found type parameter `T` | = note: expected fn pointer `extern "rust-call" fn(&mut Foo, (&'a T,))` found fn pointer `extern "rust-call" fn(&mut Foo, (T,))` error[E0053]: method `call_once` has an incompatible type for trait - --> $DIR/issue-20225.rs:18:3 + --> $DIR/issue-20225.rs:18:47 | LL | impl<'a, T> FnOnce<(&'a T,)> for Foo { | - this type parameter ... LL | extern "rust-call" fn call_once(self, (_,): (T,)) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&T`, found type parameter `T` + | ^^^^ expected `&T`, found type parameter `T` | = note: expected fn pointer `extern "rust-call" fn(Foo, (&'a T,))` found fn pointer `extern "rust-call" fn(Foo, (T,))` diff --git a/src/test/ui/issues/issue-21332.stderr b/src/test/ui/issues/issue-21332.stderr index 35863fbebe315..fd132687d7119 100644 --- a/src/test/ui/issues/issue-21332.stderr +++ b/src/test/ui/issues/issue-21332.stderr @@ -1,8 +1,8 @@ error[E0053]: method `next` has an incompatible type for trait - --> $DIR/issue-21332.rs:5:5 + --> $DIR/issue-21332.rs:5:27 | LL | fn next(&mut self) -> Result { Ok(7) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `Option`, found enum `Result` + | ^^^^^^^^^^^^^^^^ expected enum `Option`, found enum `Result` | = note: expected fn pointer `fn(&mut S) -> Option` found fn pointer `fn(&mut S) -> Result` diff --git a/src/test/ui/wrong-mul-method-signature.stderr b/src/test/ui/wrong-mul-method-signature.stderr index 4c367fb9e9caf..ce3528dcc9c0e 100644 --- a/src/test/ui/wrong-mul-method-signature.stderr +++ b/src/test/ui/wrong-mul-method-signature.stderr @@ -1,26 +1,26 @@ error[E0053]: method `mul` has an incompatible type for trait - --> $DIR/wrong-mul-method-signature.rs:16:5 + --> $DIR/wrong-mul-method-signature.rs:16:21 | LL | fn mul(self, s: &f64) -> Vec1 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `f64`, found `&f64` + | ^^^^ expected `f64`, found `&f64` | = note: expected fn pointer `fn(Vec1, f64) -> Vec1` found fn pointer `fn(Vec1, &f64) -> Vec1` error[E0053]: method `mul` has an incompatible type for trait - --> $DIR/wrong-mul-method-signature.rs:33:5 + --> $DIR/wrong-mul-method-signature.rs:33:21 | LL | fn mul(self, s: f64) -> Vec2 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `Vec2`, found `f64` + | ^^^ expected struct `Vec2`, found `f64` | = note: expected fn pointer `fn(Vec2, Vec2) -> f64` found fn pointer `fn(Vec2, f64) -> Vec2` error[E0053]: method `mul` has an incompatible type for trait - --> $DIR/wrong-mul-method-signature.rs:52:5 + --> $DIR/wrong-mul-method-signature.rs:52:29 | LL | fn mul(self, s: f64) -> f64 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, found `f64` + | ^^^ expected `i32`, found `f64` | = note: expected fn pointer `fn(Vec3, _) -> i32` found fn pointer `fn(Vec3, _) -> f64` From 147649d4b91b43c77f3f46c04b6daaa37f4e0955 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 8 Apr 2021 13:50:47 -0700 Subject: [PATCH 04/13] Suggest changing impl parameter types to match trait This is particularly useful for cases where arbitrary self types are used, like in custom `Future`s. --- .../rustc_typeck/src/check/compare_method.rs | 50 ++++++++++++++++--- src/test/ui/compare-method/bad-self-type.rs | 23 +++++++++ .../ui/compare-method/bad-self-type.stderr | 30 +++++++++++ .../reordered-type-param.stderr | 6 ++- .../impl-generic-mismatch-ab.stderr | 6 ++- src/test/ui/impl-trait/trait_type.stderr | 5 +- src/test/ui/issues/issue-13033.stderr | 2 +- src/test/ui/issues/issue-20225.stderr | 15 ++++-- src/test/ui/issues/issue-35869.stderr | 15 ++++-- src/test/ui/mismatched_types/E0053.stderr | 7 ++- .../trait-impl-fn-incompatibility.stderr | 7 ++- src/test/ui/wrong-mul-method-signature.stderr | 10 +++- 12 files changed, 151 insertions(+), 25 deletions(-) create mode 100644 src/test/ui/compare-method/bad-self-type.rs create mode 100644 src/test/ui/compare-method/bad-self-type.stderr diff --git a/compiler/rustc_typeck/src/check/compare_method.rs b/compiler/rustc_typeck/src/check/compare_method.rs index 3a337dadc88e9..964aa8426ea95 100644 --- a/compiler/rustc_typeck/src/check/compare_method.rs +++ b/compiler/rustc_typeck/src/check/compare_method.rs @@ -290,18 +290,55 @@ fn compare_predicate_entailment<'tcx>( "method `{}` has an incompatible type for trait", trait_m.ident ); - if let TypeError::ArgumentMutability(_) = terr { - if let Some(trait_err_span) = trait_err_span { - if let Ok(trait_err_str) = tcx.sess.source_map().span_to_snippet(trait_err_span) + match &terr { + TypeError::ArgumentMutability(0) | TypeError::ArgumentSorts(_, 0) + if trait_m.fn_has_self_parameter => + { + let ty = trait_sig.inputs()[0]; + let sugg = match ExplicitSelf::determine(ty, |_| ty == impl_trait_ref.self_ty()) { + ExplicitSelf::ByValue => "self".to_owned(), + ExplicitSelf::ByReference(_, hir::Mutability::Not) => "&self".to_owned(), + ExplicitSelf::ByReference(_, hir::Mutability::Mut) => { + "&mut self".to_owned() + } + _ => format!("self: {}", ty), + }; + + // When the `impl` receiver is an arbitrary self type, like `self: Box`, the + // span points only at the type `Box, but we want to cover the whole + // argument pattern and type. + let impl_m_hir_id = + tcx.hir().local_def_id_to_hir_id(impl_m.def_id.expect_local()); + let span = match tcx.hir().expect_impl_item(impl_m_hir_id).kind { + ImplItemKind::Fn(ref sig, body) => tcx + .hir() + .body_param_names(body) + .zip(sig.decl.inputs.iter()) + .map(|(param, ty)| param.span.to(ty.span)) + .next() + .unwrap_or(impl_err_span), + _ => bug!("{:?} is not a method", impl_m), + }; + + diag.span_suggestion( + span, + "change the self-receiver type to match the trait", + sugg, + Applicability::MachineApplicable, + ); + } + TypeError::ArgumentMutability(i) | TypeError::ArgumentSorts(_, i) => { + if let Some(trait_ty) = trait_sig.inputs().get(*i) { diag.span_suggestion( impl_err_span, - "consider changing the mutability to match the trait", - trait_err_str, + "change the parameter type to match the trait", + trait_ty.to_string(), Applicability::MachineApplicable, ); } } + _ => {} } infcx.note_type_err( @@ -482,8 +519,7 @@ fn compare_self_type<'tcx>( tcx.sess, impl_m_span, E0186, - "method `{}` has a `{}` declaration in the trait, but \ - not in the impl", + "method `{}` has a `{}` declaration in the trait, but not in the impl", trait_m.ident, self_descr ); diff --git a/src/test/ui/compare-method/bad-self-type.rs b/src/test/ui/compare-method/bad-self-type.rs new file mode 100644 index 0000000000000..9eb978664bfff --- /dev/null +++ b/src/test/ui/compare-method/bad-self-type.rs @@ -0,0 +1,23 @@ +use std::future::Future; +use std::task::{Context, Poll}; + +fn main() {} + +struct MyFuture {} + +impl Future for MyFuture { + type Output = (); + fn poll(self, _: &mut Context<'_>) -> Poll<()> { + //~^ ERROR method `poll` has an incompatible type for trait + todo!() + } +} + +trait T { + fn foo(self); +} + +impl T for MyFuture { + fn foo(self: Box) {} + //~^ ERROR method `foo` has an incompatible type for trait +} diff --git a/src/test/ui/compare-method/bad-self-type.stderr b/src/test/ui/compare-method/bad-self-type.stderr new file mode 100644 index 0000000000000..4d85ff86df5d1 --- /dev/null +++ b/src/test/ui/compare-method/bad-self-type.stderr @@ -0,0 +1,30 @@ +error[E0053]: method `poll` has an incompatible type for trait + --> $DIR/bad-self-type.rs:10:13 + | +LL | fn poll(self, _: &mut Context<'_>) -> Poll<()> { + | ^^^^ + | | + | expected struct `Pin`, found struct `MyFuture` + | help: change the self-receiver type to match the trait: `self: Pin<&mut MyFuture>` + | + = note: expected fn pointer `fn(Pin<&mut MyFuture>, &mut Context<'_>) -> Poll<_>` + found fn pointer `fn(MyFuture, &mut Context<'_>) -> Poll<_>` + +error[E0053]: method `foo` has an incompatible type for trait + --> $DIR/bad-self-type.rs:21:18 + | +LL | fn foo(self); + | ---- type in trait +... +LL | fn foo(self: Box) {} + | ------^^^^^^^^^ + | | | + | | expected struct `MyFuture`, found struct `Box` + | help: change the self-receiver type to match the trait: `self` + | + = note: expected fn pointer `fn(MyFuture)` + found fn pointer `fn(Box)` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0053`. diff --git a/src/test/ui/compare-method/reordered-type-param.stderr b/src/test/ui/compare-method/reordered-type-param.stderr index f1f8a663f2120..d581628ea48ad 100644 --- a/src/test/ui/compare-method/reordered-type-param.stderr +++ b/src/test/ui/compare-method/reordered-type-param.stderr @@ -5,8 +5,10 @@ LL | fn b(&self, x: C) -> C; | - type in trait ... LL | fn b(&self, _x: G) -> G { panic!() } - | - - ^ expected type parameter `F`, found type parameter `G` - | | | + | - - ^ + | | | | + | | | expected type parameter `F`, found type parameter `G` + | | | help: change the parameter type to match the trait: `F` | | found type parameter | expected type parameter | diff --git a/src/test/ui/impl-trait/impl-generic-mismatch-ab.stderr b/src/test/ui/impl-trait/impl-generic-mismatch-ab.stderr index 638a0093fb21d..d37670db08537 100644 --- a/src/test/ui/impl-trait/impl-generic-mismatch-ab.stderr +++ b/src/test/ui/impl-trait/impl-generic-mismatch-ab.stderr @@ -5,8 +5,10 @@ LL | fn foo(&self, a: &A, b: &impl Debug); | -- type in trait ... LL | fn foo(&self, a: &impl Debug, b: &B) { } - | - ^^^^^^^^^^^ expected type parameter `B`, found type parameter `impl Debug` - | | + | - ^^^^^^^^^^^ + | | | + | | expected type parameter `B`, found type parameter `impl Debug` + | | help: change the parameter type to match the trait: `&B` | expected type parameter | = note: expected fn pointer `fn(&(), &B, &impl Debug)` diff --git a/src/test/ui/impl-trait/trait_type.stderr b/src/test/ui/impl-trait/trait_type.stderr index 0bf43a65aa072..bea24339837a2 100644 --- a/src/test/ui/impl-trait/trait_type.stderr +++ b/src/test/ui/impl-trait/trait_type.stderr @@ -2,7 +2,10 @@ error[E0053]: method `fmt` has an incompatible type for trait --> $DIR/trait_type.rs:7:21 | LL | fn fmt(&self, x: &str) -> () { } - | ^^^^ types differ in mutability + | ^^^^ + | | + | types differ in mutability + | help: change the parameter type to match the trait: `&mut Formatter<'_>` | = note: expected fn pointer `fn(&MyType, &mut Formatter<'_>) -> Result<(), std::fmt::Error>` found fn pointer `fn(&MyType, &str)` diff --git a/src/test/ui/issues/issue-13033.stderr b/src/test/ui/issues/issue-13033.stderr index 57447fa48aacc..6c3651ff1217a 100644 --- a/src/test/ui/issues/issue-13033.stderr +++ b/src/test/ui/issues/issue-13033.stderr @@ -8,7 +8,7 @@ LL | fn bar(&mut self, other: &dyn Foo) {} | ^^^^^^^^ | | | types differ in mutability - | help: consider changing the mutability to match the trait: `&mut dyn Foo` + | help: change the parameter type to match the trait: `&mut dyn Foo` | = note: expected fn pointer `fn(&mut Baz, &mut dyn Foo)` found fn pointer `fn(&mut Baz, &dyn Foo)` diff --git a/src/test/ui/issues/issue-20225.stderr b/src/test/ui/issues/issue-20225.stderr index 756301de17c29..6f4813ca6235b 100644 --- a/src/test/ui/issues/issue-20225.stderr +++ b/src/test/ui/issues/issue-20225.stderr @@ -4,7 +4,10 @@ error[E0053]: method `call` has an incompatible type for trait LL | impl<'a, T> Fn<(&'a T,)> for Foo { | - this type parameter LL | extern "rust-call" fn call(&self, (_,): (T,)) {} - | ^^^^ expected `&T`, found type parameter `T` + | ^^^^ + | | + | expected `&T`, found type parameter `T` + | help: change the parameter type to match the trait: `(&'a T,)` | = note: expected fn pointer `extern "rust-call" fn(&Foo, (&'a T,))` found fn pointer `extern "rust-call" fn(&Foo, (T,))` @@ -15,7 +18,10 @@ error[E0053]: method `call_mut` has an incompatible type for trait LL | impl<'a, T> FnMut<(&'a T,)> for Foo { | - this type parameter LL | extern "rust-call" fn call_mut(&mut self, (_,): (T,)) {} - | ^^^^ expected `&T`, found type parameter `T` + | ^^^^ + | | + | expected `&T`, found type parameter `T` + | help: change the parameter type to match the trait: `(&'a T,)` | = note: expected fn pointer `extern "rust-call" fn(&mut Foo, (&'a T,))` found fn pointer `extern "rust-call" fn(&mut Foo, (T,))` @@ -27,7 +33,10 @@ LL | impl<'a, T> FnOnce<(&'a T,)> for Foo { | - this type parameter ... LL | extern "rust-call" fn call_once(self, (_,): (T,)) {} - | ^^^^ expected `&T`, found type parameter `T` + | ^^^^ + | | + | expected `&T`, found type parameter `T` + | help: change the parameter type to match the trait: `(&'a T,)` | = note: expected fn pointer `extern "rust-call" fn(Foo, (&'a T,))` found fn pointer `extern "rust-call" fn(Foo, (T,))` diff --git a/src/test/ui/issues/issue-35869.stderr b/src/test/ui/issues/issue-35869.stderr index f80561bf6be80..c104aa30cb03f 100644 --- a/src/test/ui/issues/issue-35869.stderr +++ b/src/test/ui/issues/issue-35869.stderr @@ -5,7 +5,10 @@ LL | fn foo(_: fn(u8) -> ()); | ------------ type in trait ... LL | fn foo(_: fn(u16) -> ()) {} - | ^^^^^^^^^^^^^ expected `u8`, found `u16` + | ^^^^^^^^^^^^^ + | | + | expected `u8`, found `u16` + | help: change the parameter type to match the trait: `fn(u8)` | = note: expected fn pointer `fn(fn(u8))` found fn pointer `fn(fn(u16))` @@ -17,7 +20,10 @@ LL | fn bar(_: Option); | ---------- type in trait ... LL | fn bar(_: Option) {} - | ^^^^^^^^^^^ expected `u8`, found `u16` + | ^^^^^^^^^^^ + | | + | expected `u8`, found `u16` + | help: change the parameter type to match the trait: `Option` | = note: expected fn pointer `fn(Option)` found fn pointer `fn(Option)` @@ -29,7 +35,10 @@ LL | fn baz(_: (u8, u16)); | --------- type in trait ... LL | fn baz(_: (u16, u16)) {} - | ^^^^^^^^^^ expected `u8`, found `u16` + | ^^^^^^^^^^ + | | + | expected `u8`, found `u16` + | help: change the parameter type to match the trait: `(u8, u16)` | = note: expected fn pointer `fn((u8, _))` found fn pointer `fn((u16, _))` diff --git a/src/test/ui/mismatched_types/E0053.stderr b/src/test/ui/mismatched_types/E0053.stderr index e0a3ce922b970..6ce8126b9f970 100644 --- a/src/test/ui/mismatched_types/E0053.stderr +++ b/src/test/ui/mismatched_types/E0053.stderr @@ -5,7 +5,10 @@ LL | fn foo(x: u16); | --- type in trait ... LL | fn foo(x: i16) { } - | ^^^ expected `u16`, found `i16` + | ^^^ + | | + | expected `u16`, found `i16` + | help: change the parameter type to match the trait: `u16` | = note: expected fn pointer `fn(u16)` found fn pointer `fn(i16)` @@ -20,7 +23,7 @@ LL | fn bar(&mut self) { } | ^^^^^^^^^ | | | types differ in mutability - | help: consider changing the mutability to match the trait: `&self` + | help: change the self-receiver type to match the trait: `self: &Bar` | = note: expected fn pointer `fn(&Bar)` found fn pointer `fn(&mut Bar)` diff --git a/src/test/ui/mismatched_types/trait-impl-fn-incompatibility.stderr b/src/test/ui/mismatched_types/trait-impl-fn-incompatibility.stderr index 161843473b6c1..2ac4d1c33a9b9 100644 --- a/src/test/ui/mismatched_types/trait-impl-fn-incompatibility.stderr +++ b/src/test/ui/mismatched_types/trait-impl-fn-incompatibility.stderr @@ -5,7 +5,10 @@ LL | fn foo(x: u16); | --- type in trait ... LL | fn foo(x: i16) { } - | ^^^ expected `u16`, found `i16` + | ^^^ + | | + | expected `u16`, found `i16` + | help: change the parameter type to match the trait: `u16` | = note: expected fn pointer `fn(u16)` found fn pointer `fn(i16)` @@ -20,7 +23,7 @@ LL | fn bar(&mut self, bar: &Bar) { } | ^^^^ | | | types differ in mutability - | help: consider changing the mutability to match the trait: `&mut Bar` + | help: change the parameter type to match the trait: `&mut Bar` | = note: expected fn pointer `fn(&mut Bar, &mut Bar)` found fn pointer `fn(&mut Bar, &Bar)` diff --git a/src/test/ui/wrong-mul-method-signature.stderr b/src/test/ui/wrong-mul-method-signature.stderr index ce3528dcc9c0e..9be07cb1a74af 100644 --- a/src/test/ui/wrong-mul-method-signature.stderr +++ b/src/test/ui/wrong-mul-method-signature.stderr @@ -2,7 +2,10 @@ error[E0053]: method `mul` has an incompatible type for trait --> $DIR/wrong-mul-method-signature.rs:16:21 | LL | fn mul(self, s: &f64) -> Vec1 { - | ^^^^ expected `f64`, found `&f64` + | ^^^^ + | | + | expected `f64`, found `&f64` + | help: change the parameter type to match the trait: `f64` | = note: expected fn pointer `fn(Vec1, f64) -> Vec1` found fn pointer `fn(Vec1, &f64) -> Vec1` @@ -11,7 +14,10 @@ error[E0053]: method `mul` has an incompatible type for trait --> $DIR/wrong-mul-method-signature.rs:33:21 | LL | fn mul(self, s: f64) -> Vec2 { - | ^^^ expected struct `Vec2`, found `f64` + | ^^^ + | | + | expected struct `Vec2`, found `f64` + | help: change the parameter type to match the trait: `Vec2` | = note: expected fn pointer `fn(Vec2, Vec2) -> f64` found fn pointer `fn(Vec2, f64) -> Vec2` From 5af3dec59f3dba741ce17cb3fcea6ddfaf058f6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 8 Apr 2021 15:27:43 -0700 Subject: [PATCH 05/13] Suggest return type --- .../rustc_typeck/src/check/compare_method.rs | 28 ++++++++++++++++++- .../defaults-specialization.stderr | 10 +++++-- src/test/ui/compare-method/bad-self-type.rs | 3 ++ .../ui/compare-method/bad-self-type.stderr | 19 +++++++++++-- src/test/ui/issues/issue-21332.stderr | 5 +++- src/test/ui/issues/issue-35869.stderr | 5 +++- src/test/ui/wrong-mul-method-signature.stderr | 5 +++- 7 files changed, 67 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_typeck/src/check/compare_method.rs b/compiler/rustc_typeck/src/check/compare_method.rs index 964aa8426ea95..9fd3a170ece46 100644 --- a/compiler/rustc_typeck/src/check/compare_method.rs +++ b/compiler/rustc_typeck/src/check/compare_method.rs @@ -329,7 +329,33 @@ fn compare_predicate_entailment<'tcx>( ); } TypeError::ArgumentMutability(i) | TypeError::ArgumentSorts(_, i) => { - if let Some(trait_ty) = trait_sig.inputs().get(*i) { + if trait_sig.inputs().len() == *i { + // Suggestion to change output type. We do not suggest in `async` functions + // to avoid complex logic or incorrect output. + let impl_m_hir_id = + tcx.hir().local_def_id_to_hir_id(impl_m.def_id.expect_local()); + match tcx.hir().expect_impl_item(impl_m_hir_id).kind { + ImplItemKind::Fn(ref sig, _) + if sig.header.asyncness == hir::IsAsync::NotAsync => + { + let (span, sugg) = match sig.decl.output { + hir::FnRetTy::DefaultReturn(sp) => { + (sp, format!(" -> {} ", trait_sig.output())) + } + hir::FnRetTy::Return(hir_ty) => { + (hir_ty.span, trait_sig.output().to_string()) + } + }; + diag.span_suggestion( + span, + "change the output type to match the trait", + sugg, + Applicability::MachineApplicable, + ); + } + _ => {} + }; + } else if let Some(trait_ty) = trait_sig.inputs().get(*i) { diag.span_suggestion( impl_err_span, "change the parameter type to match the trait", diff --git a/src/test/ui/associated-types/defaults-specialization.stderr b/src/test/ui/associated-types/defaults-specialization.stderr index 920f8322813fe..3c7dc1fc3c9b2 100644 --- a/src/test/ui/associated-types/defaults-specialization.stderr +++ b/src/test/ui/associated-types/defaults-specialization.stderr @@ -15,7 +15,10 @@ LL | fn make() -> Self::Ty { | -------- type in trait ... LL | fn make() -> u8 { 0 } - | ^^ expected associated type, found `u8` + | ^^ + | | + | expected associated type, found `u8` + | help: change the output type to match the trait: ` as Tr>::Ty` | = note: expected fn pointer `fn() -> as Tr>::Ty` found fn pointer `fn() -> u8` @@ -30,7 +33,10 @@ LL | default type Ty = bool; | ----------------------- expected this associated type LL | LL | fn make() -> bool { true } - | ^^^^ expected associated type, found `bool` + | ^^^^ + | | + | expected associated type, found `bool` + | help: change the output type to match the trait: ` as Tr>::Ty` | = note: expected fn pointer `fn() -> as Tr>::Ty` found fn pointer `fn() -> bool` diff --git a/src/test/ui/compare-method/bad-self-type.rs b/src/test/ui/compare-method/bad-self-type.rs index 9eb978664bfff..f42a9e49abdff 100644 --- a/src/test/ui/compare-method/bad-self-type.rs +++ b/src/test/ui/compare-method/bad-self-type.rs @@ -15,9 +15,12 @@ impl Future for MyFuture { trait T { fn foo(self); + fn bar(self) -> Option<()>; } impl T for MyFuture { fn foo(self: Box) {} //~^ ERROR method `foo` has an incompatible type for trait + fn bar(self) {} + //~^ ERROR method `bar` has an incompatible type for trait } diff --git a/src/test/ui/compare-method/bad-self-type.stderr b/src/test/ui/compare-method/bad-self-type.stderr index 4d85ff86df5d1..74e7c562aa76b 100644 --- a/src/test/ui/compare-method/bad-self-type.stderr +++ b/src/test/ui/compare-method/bad-self-type.stderr @@ -11,7 +11,7 @@ LL | fn poll(self, _: &mut Context<'_>) -> Poll<()> { found fn pointer `fn(MyFuture, &mut Context<'_>) -> Poll<_>` error[E0053]: method `foo` has an incompatible type for trait - --> $DIR/bad-self-type.rs:21:18 + --> $DIR/bad-self-type.rs:22:18 | LL | fn foo(self); | ---- type in trait @@ -25,6 +25,21 @@ LL | fn foo(self: Box) {} = note: expected fn pointer `fn(MyFuture)` found fn pointer `fn(Box)` -error: aborting due to 2 previous errors +error[E0053]: method `bar` has an incompatible type for trait + --> $DIR/bad-self-type.rs:24:18 + | +LL | fn bar(self) -> Option<()>; + | ---------- type in trait +... +LL | fn bar(self) {} + | ^ + | | + | expected enum `Option`, found `()` + | help: change the output type to match the trait: `-> Option<()>` + | + = note: expected fn pointer `fn(MyFuture) -> Option<()>` + found fn pointer `fn(MyFuture)` + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0053`. diff --git a/src/test/ui/issues/issue-21332.stderr b/src/test/ui/issues/issue-21332.stderr index fd132687d7119..d92966da17c4c 100644 --- a/src/test/ui/issues/issue-21332.stderr +++ b/src/test/ui/issues/issue-21332.stderr @@ -2,7 +2,10 @@ error[E0053]: method `next` has an incompatible type for trait --> $DIR/issue-21332.rs:5:27 | LL | fn next(&mut self) -> Result { Ok(7) } - | ^^^^^^^^^^^^^^^^ expected enum `Option`, found enum `Result` + | ^^^^^^^^^^^^^^^^ + | | + | expected enum `Option`, found enum `Result` + | help: change the output type to match the trait: `Option` | = note: expected fn pointer `fn(&mut S) -> Option` found fn pointer `fn(&mut S) -> Result` diff --git a/src/test/ui/issues/issue-35869.stderr b/src/test/ui/issues/issue-35869.stderr index c104aa30cb03f..71b2a9df09553 100644 --- a/src/test/ui/issues/issue-35869.stderr +++ b/src/test/ui/issues/issue-35869.stderr @@ -50,7 +50,10 @@ LL | fn qux() -> u8; | -- type in trait ... LL | fn qux() -> u16 { 5u16 } - | ^^^ expected `u8`, found `u16` + | ^^^ + | | + | expected `u8`, found `u16` + | help: change the output type to match the trait: `u8` | = note: expected fn pointer `fn() -> u8` found fn pointer `fn() -> u16` diff --git a/src/test/ui/wrong-mul-method-signature.stderr b/src/test/ui/wrong-mul-method-signature.stderr index 9be07cb1a74af..9f8896f01ee06 100644 --- a/src/test/ui/wrong-mul-method-signature.stderr +++ b/src/test/ui/wrong-mul-method-signature.stderr @@ -26,7 +26,10 @@ error[E0053]: method `mul` has an incompatible type for trait --> $DIR/wrong-mul-method-signature.rs:52:29 | LL | fn mul(self, s: f64) -> f64 { - | ^^^ expected `i32`, found `f64` + | ^^^ + | | + | expected `i32`, found `f64` + | help: change the output type to match the trait: `i32` | = note: expected fn pointer `fn(Vec3, _) -> i32` found fn pointer `fn(Vec3, _) -> f64` From bb502c488915253a261f837cce7a920bf49a1666 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 8 Apr 2021 15:54:26 -0700 Subject: [PATCH 06/13] Provide verbose suggestion for new output type --- .../rustc_typeck/src/check/compare_method.rs | 16 +++++++--------- src/test/ui/compare-method/bad-self-type.stderr | 9 +++++---- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_typeck/src/check/compare_method.rs b/compiler/rustc_typeck/src/check/compare_method.rs index 9fd3a170ece46..60ca562f99200 100644 --- a/compiler/rustc_typeck/src/check/compare_method.rs +++ b/compiler/rustc_typeck/src/check/compare_method.rs @@ -338,20 +338,18 @@ fn compare_predicate_entailment<'tcx>( ImplItemKind::Fn(ref sig, _) if sig.header.asyncness == hir::IsAsync::NotAsync => { - let (span, sugg) = match sig.decl.output { + let msg = "change the output type to match the trait"; + let ap = Applicability::MachineApplicable; + match sig.decl.output { hir::FnRetTy::DefaultReturn(sp) => { - (sp, format!(" -> {} ", trait_sig.output())) + let sugg = format!("-> {} ", trait_sig.output()); + diag.span_suggestion_verbose(sp, msg, sugg, ap); } hir::FnRetTy::Return(hir_ty) => { - (hir_ty.span, trait_sig.output().to_string()) + let sugg = trait_sig.output().to_string(); + diag.span_suggestion(hir_ty.span, msg, sugg, ap); } }; - diag.span_suggestion( - span, - "change the output type to match the trait", - sugg, - Applicability::MachineApplicable, - ); } _ => {} }; diff --git a/src/test/ui/compare-method/bad-self-type.stderr b/src/test/ui/compare-method/bad-self-type.stderr index 74e7c562aa76b..76f91fbf241d0 100644 --- a/src/test/ui/compare-method/bad-self-type.stderr +++ b/src/test/ui/compare-method/bad-self-type.stderr @@ -32,13 +32,14 @@ LL | fn bar(self) -> Option<()>; | ---------- type in trait ... LL | fn bar(self) {} - | ^ - | | - | expected enum `Option`, found `()` - | help: change the output type to match the trait: `-> Option<()>` + | ^ expected enum `Option`, found `()` | = note: expected fn pointer `fn(MyFuture) -> Option<()>` found fn pointer `fn(MyFuture)` +help: change the output type to match the trait + | +LL | fn bar(self) -> Option<()> {} + | ^^^^^^^^^^^^^ error: aborting due to 3 previous errors From 63d6e327825b8a3167a40290812ef6f7fb75e7b2 Mon Sep 17 00:00:00 2001 From: Oleksandr Povar Date: Sat, 10 Apr 2021 16:12:46 +0200 Subject: [PATCH 07/13] Bump libc dependency of std to 0.2.93 --- Cargo.lock | 4 ++-- library/std/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2b7fbf1b647d2..5504ba2732f27 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1909,9 +1909,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "libc" -version = "0.2.88" +version = "0.2.93" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "03b07a082330a35e43f63177cc01689da34fbffa0105e1246cf0311472cac73a" +checksum = "9385f66bf6105b241aa65a61cb923ef20efc665cb9f9bb50ac2f0c4b7f378d41" dependencies = [ "rustc-std-workspace-core", ] diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index 22ca7ed09b42a..84a642289212c 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -16,7 +16,7 @@ cfg-if = { version = "0.1.8", features = ['rustc-dep-of-std'] } panic_unwind = { path = "../panic_unwind", optional = true } panic_abort = { path = "../panic_abort" } core = { path = "../core" } -libc = { version = "0.2.88", default-features = false, features = ['rustc-dep-of-std'] } +libc = { version = "0.2.93", default-features = false, features = ['rustc-dep-of-std'] } compiler_builtins = { version = "0.1.39" } profiler_builtins = { path = "../profiler_builtins", optional = true } unwind = { path = "../unwind" } From c2f4a5b9f90428b804da08d554f608a9b58c3c05 Mon Sep 17 00:00:00 2001 From: Steve Klabnik Date: Sat, 10 Apr 2021 12:50:04 -0500 Subject: [PATCH 08/13] clean up example on read_to_string This is the same thing, but simpler. --- library/std/src/fs.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 860bc130b7d8b..e6120b8ee31c2 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -265,8 +265,9 @@ pub fn read>(path: P) -> io::Result> { /// ```no_run /// use std::fs; /// use std::net::SocketAddr; +/// use std::error::Error; /// -/// fn main() -> Result<(), Box> { +/// fn main() -> Result<(), Box> { /// let foo: SocketAddr = fs::read_to_string("address.txt")?.parse()?; /// Ok(()) /// } From 60780e438a8f99617ab709f28ab3d54d73ea4af6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Sun, 11 Apr 2021 00:00:00 +0000 Subject: [PATCH 09/13] Remove FixedSizeArray --- library/core/src/array/mod.rs | 36 ----------------------------------- library/core/tests/array.rs | 20 +------------------ library/core/tests/lib.rs | 1 - library/std/src/lib.rs | 1 - 4 files changed, 1 insertion(+), 57 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 8f52985d1df71..b6ce825e2477a 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -12,7 +12,6 @@ use crate::convert::{Infallible, TryFrom}; use crate::fmt; use crate::hash::{self, Hash}; use crate::iter::TrustedLen; -use crate::marker::Unsize; use crate::mem::{self, MaybeUninit}; use crate::ops::{Index, IndexMut}; use crate::slice::{Iter, IterMut}; @@ -36,41 +35,6 @@ pub fn from_mut(s: &mut T) -> &mut [T; 1] { unsafe { &mut *(s as *mut T).cast::<[T; 1]>() } } -/// Utility trait implemented only on arrays of fixed size -/// -/// This trait can be used to implement other traits on fixed-size arrays -/// without causing much metadata bloat. -/// -/// The trait is marked unsafe in order to restrict implementors to fixed-size -/// arrays. A user of this trait can assume that implementors have the exact -/// layout in memory of a fixed size array (for example, for unsafe -/// initialization). -/// -/// Note that the traits [`AsRef`] and [`AsMut`] provide similar methods for types that -/// may not be fixed-size arrays. Implementors should prefer those traits -/// instead. -#[unstable(feature = "fixed_size_array", issue = "27778")] -pub unsafe trait FixedSizeArray { - /// Converts the array to immutable slice - #[unstable(feature = "fixed_size_array", issue = "27778")] - fn as_slice(&self) -> &[T]; - /// Converts the array to mutable slice - #[unstable(feature = "fixed_size_array", issue = "27778")] - fn as_mut_slice(&mut self) -> &mut [T]; -} - -#[unstable(feature = "fixed_size_array", issue = "27778")] -unsafe impl> FixedSizeArray for A { - #[inline] - fn as_slice(&self) -> &[T] { - self - } - #[inline] - fn as_mut_slice(&mut self) -> &mut [T] { - self - } -} - /// The error type returned when a conversion from a slice to an array fails. #[stable(feature = "try_from", since = "1.34.0")] #[derive(Debug, Copy, Clone)] diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index 89c2a969c28bb..ce7480ce2ee89 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -1,24 +1,6 @@ -use core::array::{self, FixedSizeArray, IntoIter}; +use core::array::{self, IntoIter}; use core::convert::TryFrom; -#[test] -fn fixed_size_array() { - let mut array = [0; 64]; - let mut zero_sized = [(); 64]; - let mut empty_array = [0; 0]; - let mut empty_zero_sized = [(); 0]; - - assert_eq!(FixedSizeArray::as_slice(&array).len(), 64); - assert_eq!(FixedSizeArray::as_slice(&zero_sized).len(), 64); - assert_eq!(FixedSizeArray::as_slice(&empty_array).len(), 0); - assert_eq!(FixedSizeArray::as_slice(&empty_zero_sized).len(), 0); - - assert_eq!(FixedSizeArray::as_mut_slice(&mut array).len(), 64); - assert_eq!(FixedSizeArray::as_mut_slice(&mut zero_sized).len(), 64); - assert_eq!(FixedSizeArray::as_mut_slice(&mut empty_array).len(), 0); - assert_eq!(FixedSizeArray::as_mut_slice(&mut empty_zero_sized).len(), 0); -} - #[test] fn array_from_ref() { let value: String = "Hello World!".into(); diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 7dc6e220c08bc..6624fd473539a 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -28,7 +28,6 @@ #![feature(duration_zero)] #![feature(exact_size_is_empty)] #![feature(extern_types)] -#![feature(fixed_size_array)] #![feature(flt2dec)] #![feature(fmt_internals)] #![feature(hashmap_internals)] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 6baf9f2a464b2..91695ced6a962 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -214,7 +214,6 @@ feature(slice_index_methods, coerce_unsized, sgx_platform) )] #![deny(rustc::existing_doc_keyword)] -#![cfg_attr(all(test, target_vendor = "fortanix", target_env = "sgx"), feature(fixed_size_array))] // std is implemented with unstable features, many of which are internal // compiler details that will never be stable // NB: the following list is sorted to minimize merge conflicts. From d931e2b7bfd74caeb1e25128e4382ba706d4ec52 Mon Sep 17 00:00:00 2001 From: Camelid Date: Thu, 8 Apr 2021 16:06:10 -0700 Subject: [PATCH 10/13] Rename `url-improvements` test to `bare-urls` The lint used to be called `non-autolinks`, and linted more than just bare URLs. Now, it is called `bare-urls` and only lints against bare URLs. So, `bare-urls` is a better name for the test. --- .../{url-improvements.rs => bare-urls.rs} | 0 ...l-improvements.stderr => bare-urls.stderr} | 36 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) rename src/test/rustdoc-ui/{url-improvements.rs => bare-urls.rs} (100%) rename src/test/rustdoc-ui/{url-improvements.stderr => bare-urls.stderr} (84%) diff --git a/src/test/rustdoc-ui/url-improvements.rs b/src/test/rustdoc-ui/bare-urls.rs similarity index 100% rename from src/test/rustdoc-ui/url-improvements.rs rename to src/test/rustdoc-ui/bare-urls.rs diff --git a/src/test/rustdoc-ui/url-improvements.stderr b/src/test/rustdoc-ui/bare-urls.stderr similarity index 84% rename from src/test/rustdoc-ui/url-improvements.stderr rename to src/test/rustdoc-ui/bare-urls.stderr index 3d5ebd8be6b67..a4c06bbd5a57d 100644 --- a/src/test/rustdoc-ui/url-improvements.stderr +++ b/src/test/rustdoc-ui/bare-urls.stderr @@ -1,107 +1,107 @@ error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:3:5 + --> $DIR/bare-urls.rs:3:5 | LL | /// https://somewhere.com | ^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` | note: the lint level is defined here - --> $DIR/url-improvements.rs:1:9 + --> $DIR/bare-urls.rs:1:9 | LL | #![deny(rustdoc::bare_urls)] | ^^^^^^^^^^^^^^^^^^ error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:5:5 + --> $DIR/bare-urls.rs:5:5 | LL | /// https://somewhere.com/a | ^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:7:5 + --> $DIR/bare-urls.rs:7:5 | LL | /// https://www.somewhere.com | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:9:5 + --> $DIR/bare-urls.rs:9:5 | LL | /// https://www.somewhere.com/a | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:11:5 + --> $DIR/bare-urls.rs:11:5 | LL | /// https://subdomain.example.com | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:13:5 + --> $DIR/bare-urls.rs:13:5 | LL | /// https://somewhere.com? | ^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:15:5 + --> $DIR/bare-urls.rs:15:5 | LL | /// https://somewhere.com/a? | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:17:5 + --> $DIR/bare-urls.rs:17:5 | LL | /// https://somewhere.com?hello=12 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:19:5 + --> $DIR/bare-urls.rs:19:5 | LL | /// https://somewhere.com/a?hello=12 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:21:5 + --> $DIR/bare-urls.rs:21:5 | LL | /// https://example.com?hello=12#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:23:5 + --> $DIR/bare-urls.rs:23:5 | LL | /// https://example.com/a?hello=12#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:25:5 + --> $DIR/bare-urls.rs:25:5 | LL | /// https://example.com#xyz | ^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:27:5 + --> $DIR/bare-urls.rs:27:5 | LL | /// https://example.com/a#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:29:5 + --> $DIR/bare-urls.rs:29:5 | LL | /// https://somewhere.com?hello=12&bye=11 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:31:5 + --> $DIR/bare-urls.rs:31:5 | LL | /// https://somewhere.com/a?hello=12&bye=11 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:33:5 + --> $DIR/bare-urls.rs:33:5 | LL | /// https://somewhere.com?hello=12&bye=11#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:35:10 + --> $DIR/bare-urls.rs:35:10 | LL | /// hey! https://somewhere.com/a?hello=12&bye=11#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` From aabc363bc2c66d1dd31a4f7e4d27cf77303ab6d8 Mon Sep 17 00:00:00 2001 From: Camelid Date: Sat, 10 Apr 2021 18:39:05 -0700 Subject: [PATCH 11/13] Run rustfix for `bare-urls` test This will help us ensure that it emits valid suggestions. --- src/test/rustdoc-ui/bare-urls.fixed | 60 ++++++++++++++++++++++++++++ src/test/rustdoc-ui/bare-urls.rs | 2 + src/test/rustdoc-ui/bare-urls.stderr | 36 ++++++++--------- 3 files changed, 80 insertions(+), 18 deletions(-) create mode 100644 src/test/rustdoc-ui/bare-urls.fixed diff --git a/src/test/rustdoc-ui/bare-urls.fixed b/src/test/rustdoc-ui/bare-urls.fixed new file mode 100644 index 0000000000000..23aa5c44c21fd --- /dev/null +++ b/src/test/rustdoc-ui/bare-urls.fixed @@ -0,0 +1,60 @@ +// run-rustfix + +#![deny(rustdoc::bare_urls)] + +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// +//~^ ERROR this URL is not a hyperlink +/// hey! +//~^ ERROR this URL is not a hyperlink +pub fn c() {} + +/// +/// [a](http://a.com) +/// [b] +/// +/// [b]: http://b.com +/// +/// ``` +/// This link should not be linted: http://example.com +/// +/// Nor this one: or this one: [x](http://example.com) +/// ``` +/// +/// [should_not.lint](should_not.lint) +pub fn everything_is_fine_here() {} + +#[allow(rustdoc::bare_urls)] +pub mod foo { + /// https://somewhere.com/a?hello=12&bye=11#xyz + pub fn bar() {} +} diff --git a/src/test/rustdoc-ui/bare-urls.rs b/src/test/rustdoc-ui/bare-urls.rs index 43a13b02d0ab6..592f57343bc92 100644 --- a/src/test/rustdoc-ui/bare-urls.rs +++ b/src/test/rustdoc-ui/bare-urls.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![deny(rustdoc::bare_urls)] /// https://somewhere.com diff --git a/src/test/rustdoc-ui/bare-urls.stderr b/src/test/rustdoc-ui/bare-urls.stderr index a4c06bbd5a57d..6b612f81590dc 100644 --- a/src/test/rustdoc-ui/bare-urls.stderr +++ b/src/test/rustdoc-ui/bare-urls.stderr @@ -1,107 +1,107 @@ error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:3:5 + --> $DIR/bare-urls.rs:5:5 | LL | /// https://somewhere.com | ^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` | note: the lint level is defined here - --> $DIR/bare-urls.rs:1:9 + --> $DIR/bare-urls.rs:3:9 | LL | #![deny(rustdoc::bare_urls)] | ^^^^^^^^^^^^^^^^^^ error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:5:5 + --> $DIR/bare-urls.rs:7:5 | LL | /// https://somewhere.com/a | ^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:7:5 + --> $DIR/bare-urls.rs:9:5 | LL | /// https://www.somewhere.com | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:9:5 + --> $DIR/bare-urls.rs:11:5 | LL | /// https://www.somewhere.com/a | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:11:5 + --> $DIR/bare-urls.rs:13:5 | LL | /// https://subdomain.example.com | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:13:5 + --> $DIR/bare-urls.rs:15:5 | LL | /// https://somewhere.com? | ^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:15:5 + --> $DIR/bare-urls.rs:17:5 | LL | /// https://somewhere.com/a? | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:17:5 + --> $DIR/bare-urls.rs:19:5 | LL | /// https://somewhere.com?hello=12 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:19:5 + --> $DIR/bare-urls.rs:21:5 | LL | /// https://somewhere.com/a?hello=12 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:21:5 + --> $DIR/bare-urls.rs:23:5 | LL | /// https://example.com?hello=12#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:23:5 + --> $DIR/bare-urls.rs:25:5 | LL | /// https://example.com/a?hello=12#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:25:5 + --> $DIR/bare-urls.rs:27:5 | LL | /// https://example.com#xyz | ^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:27:5 + --> $DIR/bare-urls.rs:29:5 | LL | /// https://example.com/a#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:29:5 + --> $DIR/bare-urls.rs:31:5 | LL | /// https://somewhere.com?hello=12&bye=11 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:31:5 + --> $DIR/bare-urls.rs:33:5 | LL | /// https://somewhere.com/a?hello=12&bye=11 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:33:5 + --> $DIR/bare-urls.rs:35:5 | LL | /// https://somewhere.com?hello=12&bye=11#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/bare-urls.rs:35:10 + --> $DIR/bare-urls.rs:37:10 | LL | /// hey! https://somewhere.com/a?hello=12&bye=11#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` From 47d1ed96902508108a00afa1865f5e40819db5c6 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 10 Apr 2021 13:48:54 -0400 Subject: [PATCH 12/13] Preprocess intra-doc links consistently Previously, rustdoc would panic on links to external crates if they were surrounded by backticks. --- src/librustdoc/core.rs | 17 +- .../passes/collect_intra_doc_links.rs | 220 +++++++++++------- src/test/rustdoc/intra-doc/auxiliary/empty.rs | 1 + .../rustdoc/intra-doc/auxiliary/empty2.rs | 1 + .../extern-crate-only-used-in-link.rs | 13 +- 5 files changed, 160 insertions(+), 92 deletions(-) create mode 100644 src/test/rustdoc/intra-doc/auxiliary/empty.rs create mode 100644 src/test/rustdoc/intra-doc/auxiliary/empty2.rs diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index c9fdaa50534dd..1da06f78fa255 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -365,21 +365,20 @@ crate fn create_resolver<'a>( } impl ast::visit::Visitor<'_> for IntraLinkCrateLoader { fn visit_attribute(&mut self, attr: &ast::Attribute) { - use crate::html::markdown::{markdown_links, MarkdownLink}; - use crate::passes::collect_intra_doc_links::Disambiguator; + use crate::html::markdown::markdown_links; + use crate::passes::collect_intra_doc_links::preprocess_link; if let Some(doc) = attr.doc_str() { - for MarkdownLink { link, .. } in markdown_links(&doc.as_str()) { - // FIXME: this misses a *lot* of the preprocessing done in collect_intra_doc_links - // I think most of it shouldn't be necessary since we only need the crate prefix? - let path_str = match Disambiguator::from_str(&link) { - Ok(x) => x.map_or(link.as_str(), |(_, p)| p), - Err(_) => continue, + for link in markdown_links(&doc.as_str()) { + let path_str = if let Some(Ok(x)) = preprocess_link(&link) { + x.path_str + } else { + continue; }; self.resolver.borrow_mut().access(|resolver| { let _ = resolver.resolve_str_path_error( attr.span, - path_str, + &path_str, TypeNS, self.current_mod, ); diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 4bc7544e33d1e..5d280b590bdfc 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -68,7 +68,7 @@ impl<'a> From> for ErrorKind<'a> { } #[derive(Copy, Clone, Debug, Hash)] -enum Res { +crate enum Res { Def(DefKind, DefId), Primitive(PrimitiveType), } @@ -134,7 +134,7 @@ impl TryFrom for Res { /// A link failed to resolve. #[derive(Debug)] -enum ResolutionFailure<'a> { +crate enum ResolutionFailure<'a> { /// This resolved, but with the wrong namespace. WrongNamespace { /// What the link resolved to. @@ -172,7 +172,7 @@ enum ResolutionFailure<'a> { } #[derive(Debug)] -enum MalformedGenerics { +crate enum MalformedGenerics { /// This link has unbalanced angle brackets. /// /// For example, `Vec>`. @@ -224,7 +224,7 @@ impl ResolutionFailure<'a> { } } -enum AnchorFailure { +crate enum AnchorFailure { /// User error: `[std#x#y]` is not valid MultipleAnchors, /// The anchor provided by the user conflicts with Rustdoc's generated anchor. @@ -892,6 +892,117 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } +crate enum PreprocessingError<'a> { + Anchor(AnchorFailure), + Disambiguator(Range, String), + Resolution(ResolutionFailure<'a>, String, Option), +} + +impl From for PreprocessingError<'_> { + fn from(err: AnchorFailure) -> Self { + Self::Anchor(err) + } +} + +crate struct PreprocessingInfo { + crate path_str: String, + disambiguator: Option, + extra_fragment: Option, + link_text: String, +} + +/// Returns: +/// - `None` if the link should be ignored. +/// - `Some(Err)` if the link should emit an error +/// - `Some(Ok)` if the link is valid +/// +/// `link_buffer` is needed for lifetime reasons; it will always be overwritten and the contents ignored. +crate fn preprocess_link<'a>( + ori_link: &'a MarkdownLink, +) -> Option>> { + // [] is mostly likely not supposed to be a link + if ori_link.link.is_empty() { + return None; + } + + // Bail early for real links. + if ori_link.link.contains('/') { + return None; + } + + let stripped = ori_link.link.replace("`", ""); + let mut parts = stripped.split('#'); + + let link = parts.next().unwrap(); + if link.trim().is_empty() { + // This is an anchor to an element of the current page, nothing to do in here! + return None; + } + let extra_fragment = parts.next(); + if parts.next().is_some() { + // A valid link can't have multiple #'s + return Some(Err(AnchorFailure::MultipleAnchors.into())); + } + + // Parse and strip the disambiguator from the link, if present. + let (path_str, disambiguator) = match Disambiguator::from_str(&link) { + Ok(Some((d, path))) => (path.trim(), Some(d)), + Ok(None) => (link.trim(), None), + Err((err_msg, relative_range)) => { + // Only report error if we would not have ignored this link. See issue #83859. + if !should_ignore_link_with_disambiguators(link) { + let no_backticks_range = range_between_backticks(&ori_link); + let disambiguator_range = (no_backticks_range.start + relative_range.start) + ..(no_backticks_range.start + relative_range.end); + return Some(Err(PreprocessingError::Disambiguator(disambiguator_range, err_msg))); + } else { + return None; + } + } + }; + + if should_ignore_link(path_str) { + return None; + } + + // We stripped `()` and `!` when parsing the disambiguator. + // Add them back to be displayed, but not prefix disambiguators. + let link_text = + disambiguator.map(|d| d.display_for(path_str)).unwrap_or_else(|| path_str.to_owned()); + + // Strip generics from the path. + let path_str = if path_str.contains(['<', '>'].as_slice()) { + match strip_generics_from_path(&path_str) { + Ok(path) => path, + Err(err_kind) => { + debug!("link has malformed generics: {}", path_str); + return Some(Err(PreprocessingError::Resolution( + err_kind, + path_str.to_owned(), + disambiguator, + ))); + } + } + } else { + path_str.to_owned() + }; + + // Sanity check to make sure we don't have any angle brackets after stripping generics. + assert!(!path_str.contains(['<', '>'].as_slice())); + + // The link is not an intra-doc link if it still contains spaces after stripping generics. + if path_str.contains(' ') { + return None; + } + + Some(Ok(PreprocessingInfo { + path_str, + disambiguator, + extra_fragment: extra_fragment.map(String::from), + link_text, + })) +} + impl LinkCollector<'_, '_> { /// This is the entry point for resolving an intra-doc link. /// @@ -907,16 +1018,6 @@ impl LinkCollector<'_, '_> { ) -> Option { trace!("considering link '{}'", ori_link.link); - // Bail early for real links. - if ori_link.link.contains('/') { - return None; - } - - // [] is mostly likely not supposed to be a link - if ori_link.link.is_empty() { - return None; - } - let diag_info = DiagnosticInfo { item, dox, @@ -924,47 +1025,29 @@ impl LinkCollector<'_, '_> { link_range: ori_link.range.clone(), }; - let link = ori_link.link.replace("`", ""); - let no_backticks_range = range_between_backticks(&ori_link); - let parts = link.split('#').collect::>(); - let (link, extra_fragment) = if parts.len() > 2 { - // A valid link can't have multiple #'s - anchor_failure(self.cx, diag_info, AnchorFailure::MultipleAnchors); - return None; - } else if parts.len() == 2 { - if parts[0].trim().is_empty() { - // This is an anchor to an element of the current page, nothing to do in here! - return None; - } - (parts[0], Some(parts[1].to_owned())) - } else { - (parts[0], None) - }; - - // Parse and strip the disambiguator from the link, if present. - let (mut path_str, disambiguator) = match Disambiguator::from_str(&link) { - Ok(Some((d, path))) => (path.trim(), Some(d)), - Ok(None) => (link.trim(), None), - Err((err_msg, relative_range)) => { - if !should_ignore_link_with_disambiguators(link) { - // Only report error if we would not have ignored this link. - // See issue #83859. - let disambiguator_range = (no_backticks_range.start + relative_range.start) - ..(no_backticks_range.start + relative_range.end); - disambiguator_error(self.cx, diag_info, disambiguator_range, &err_msg); + let PreprocessingInfo { path_str, disambiguator, extra_fragment, link_text } = + match preprocess_link(&ori_link)? { + Ok(x) => x, + Err(err) => { + match err { + PreprocessingError::Anchor(err) => anchor_failure(self.cx, diag_info, err), + PreprocessingError::Disambiguator(range, msg) => { + disambiguator_error(self.cx, diag_info, range, &msg) + } + PreprocessingError::Resolution(err, path_str, disambiguator) => { + resolution_failure( + self, + diag_info, + &path_str, + disambiguator, + smallvec![err], + ); + } + } + return None; } - return None; - } - }; - - if should_ignore_link(path_str) { - return None; - } - - // We stripped `()` and `!` when parsing the disambiguator. - // Add them back to be displayed, but not prefix disambiguators. - let link_text = - disambiguator.map(|d| d.display_for(path_str)).unwrap_or_else(|| path_str.to_owned()); + }; + let mut path_str = &*path_str; // In order to correctly resolve intra-doc links we need to // pick a base AST node to work from. If the documentation for @@ -1029,39 +1112,12 @@ impl LinkCollector<'_, '_> { module_id = DefId { krate, index: CRATE_DEF_INDEX }; } - // Strip generics from the path. - let stripped_path_string; - if path_str.contains(['<', '>'].as_slice()) { - stripped_path_string = match strip_generics_from_path(path_str) { - Ok(path) => path, - Err(err_kind) => { - debug!("link has malformed generics: {}", path_str); - resolution_failure( - self, - diag_info, - path_str, - disambiguator, - smallvec![err_kind], - ); - return None; - } - }; - path_str = &stripped_path_string; - } - // Sanity check to make sure we don't have any angle brackets after stripping generics. - assert!(!path_str.contains(['<', '>'].as_slice())); - - // The link is not an intra-doc link if it still contains spaces after stripping generics. - if path_str.contains(' ') { - return None; - } - let (mut res, mut fragment) = self.resolve_with_disambiguator_cached( ResolutionInfo { module_id, dis: disambiguator, path_str: path_str.to_owned(), - extra_fragment, + extra_fragment: extra_fragment.map(String::from), }, diag_info.clone(), // this struct should really be Copy, but Range is not :( matches!(ori_link.kind, LinkType::Reference | LinkType::Shortcut), diff --git a/src/test/rustdoc/intra-doc/auxiliary/empty.rs b/src/test/rustdoc/intra-doc/auxiliary/empty.rs new file mode 100644 index 0000000000000..d11c69f812a8d --- /dev/null +++ b/src/test/rustdoc/intra-doc/auxiliary/empty.rs @@ -0,0 +1 @@ +// intentionally empty diff --git a/src/test/rustdoc/intra-doc/auxiliary/empty2.rs b/src/test/rustdoc/intra-doc/auxiliary/empty2.rs new file mode 100644 index 0000000000000..d11c69f812a8d --- /dev/null +++ b/src/test/rustdoc/intra-doc/auxiliary/empty2.rs @@ -0,0 +1 @@ +// intentionally empty diff --git a/src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs b/src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs index 0964c79de068f..5d8dcf8bc1d16 100644 --- a/src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs +++ b/src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs @@ -1,8 +1,19 @@ +// This test is just a little cursed. // aux-build:issue-66159-1.rs // aux-crate:priv:issue_66159_1=issue-66159-1.rs +// aux-build:empty.rs +// aux-crate:priv:empty=empty.rs +// aux-build:empty2.rs +// aux-crate:priv:empty2=empty2.rs // build-aux-docs -// compile-flags:-Z unstable-options +// compile-flags:-Z unstable-options --edition 2018 // @has extern_crate_only_used_in_link/index.html // @has - '//a[@href="../issue_66159_1/struct.Something.html"]' 'issue_66159_1::Something' //! [issue_66159_1::Something] + +// @has - '//a[@href="../empty/index.html"]' 'empty' +//! [`empty`] + +// @has - '//a[@href="../empty2/index.html"]' 'empty2' +//! [empty2] From cba50731a6998ec5a53932d6f4b891877e01a9e3 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 11 Apr 2021 09:28:03 -0400 Subject: [PATCH 13/13] Move crate loader to collect_intra_doc_links::early This groups the similar code together, and also allows making most of collect_intra_doc_links private again --- src/librustdoc/core.rs | 53 +--------------- .../passes/collect_intra_doc_links.rs | 25 ++++---- .../passes/collect_intra_doc_links/early.rs | 63 +++++++++++++++++++ 3 files changed, 80 insertions(+), 61 deletions(-) create mode 100644 src/librustdoc/passes/collect_intra_doc_links/early.rs diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 1da06f78fa255..be7bff1a29c2b 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -5,8 +5,8 @@ use rustc_driver::abort_on_err; use rustc_errors::emitter::{Emitter, EmitterWriter}; use rustc_errors::json::JsonEmitter; use rustc_feature::UnstableFeatures; -use rustc_hir::def::{Namespace::TypeNS, Res}; -use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE}; +use rustc_hir::def::Res; +use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, LOCAL_CRATE}; use rustc_hir::HirId; use rustc_hir::{ intravisit::{self, NestedVisitorMap, Visitor}, @@ -356,54 +356,7 @@ crate fn create_resolver<'a>( let (krate, resolver, _) = &*parts; let resolver = resolver.borrow().clone(); - // Letting the resolver escape at the end of the function leads to inconsistencies between the - // crates the TyCtxt sees and the resolver sees (because the resolver could load more crates - // after escaping). Hopefully `IntraLinkCrateLoader` gets all the crates we need ... - struct IntraLinkCrateLoader { - current_mod: DefId, - resolver: Rc>, - } - impl ast::visit::Visitor<'_> for IntraLinkCrateLoader { - fn visit_attribute(&mut self, attr: &ast::Attribute) { - use crate::html::markdown::markdown_links; - use crate::passes::collect_intra_doc_links::preprocess_link; - - if let Some(doc) = attr.doc_str() { - for link in markdown_links(&doc.as_str()) { - let path_str = if let Some(Ok(x)) = preprocess_link(&link) { - x.path_str - } else { - continue; - }; - self.resolver.borrow_mut().access(|resolver| { - let _ = resolver.resolve_str_path_error( - attr.span, - &path_str, - TypeNS, - self.current_mod, - ); - }); - } - } - ast::visit::walk_attribute(self, attr); - } - - fn visit_item(&mut self, item: &ast::Item) { - use rustc_ast_lowering::ResolverAstLowering; - - if let ast::ItemKind::Mod(..) = item.kind { - let new_mod = - self.resolver.borrow_mut().access(|resolver| resolver.local_def_id(item.id)); - let old_mod = mem::replace(&mut self.current_mod, new_mod.to_def_id()); - ast::visit::walk_item(self, item); - self.current_mod = old_mod; - } else { - ast::visit::walk_item(self, item); - } - } - } - let crate_id = LocalDefId { local_def_index: CRATE_DEF_INDEX }.to_def_id(); - let mut loader = IntraLinkCrateLoader { current_mod: crate_id, resolver }; + let mut loader = crate::passes::collect_intra_doc_links::IntraLinkCrateLoader::new(resolver); ast::visit::walk_crate(&mut loader, krate); loader.resolver diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 5d280b590bdfc..6342110adfe0b 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -39,13 +39,16 @@ use crate::passes::Pass; use super::span_of_attrs; +mod early; +crate use early::IntraLinkCrateLoader; + crate const COLLECT_INTRA_DOC_LINKS: Pass = Pass { name: "collect-intra-doc-links", run: collect_intra_doc_links, description: "resolves intra-doc links", }; -crate fn collect_intra_doc_links(krate: Crate, cx: &mut DocContext<'_>) -> Crate { +fn collect_intra_doc_links(krate: Crate, cx: &mut DocContext<'_>) -> Crate { LinkCollector { cx, mod_ids: Vec::new(), @@ -68,7 +71,7 @@ impl<'a> From> for ErrorKind<'a> { } #[derive(Copy, Clone, Debug, Hash)] -crate enum Res { +enum Res { Def(DefKind, DefId), Primitive(PrimitiveType), } @@ -134,7 +137,7 @@ impl TryFrom for Res { /// A link failed to resolve. #[derive(Debug)] -crate enum ResolutionFailure<'a> { +enum ResolutionFailure<'a> { /// This resolved, but with the wrong namespace. WrongNamespace { /// What the link resolved to. @@ -172,7 +175,7 @@ crate enum ResolutionFailure<'a> { } #[derive(Debug)] -crate enum MalformedGenerics { +enum MalformedGenerics { /// This link has unbalanced angle brackets. /// /// For example, `Vec>`. @@ -224,7 +227,7 @@ impl ResolutionFailure<'a> { } } -crate enum AnchorFailure { +enum AnchorFailure { /// User error: `[std#x#y]` is not valid MultipleAnchors, /// The anchor provided by the user conflicts with Rustdoc's generated anchor. @@ -892,7 +895,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } -crate enum PreprocessingError<'a> { +enum PreprocessingError<'a> { Anchor(AnchorFailure), Disambiguator(Range, String), Resolution(ResolutionFailure<'a>, String, Option), @@ -904,8 +907,8 @@ impl From for PreprocessingError<'_> { } } -crate struct PreprocessingInfo { - crate path_str: String, +struct PreprocessingInfo { + path_str: String, disambiguator: Option, extra_fragment: Option, link_text: String, @@ -917,7 +920,7 @@ crate struct PreprocessingInfo { /// - `Some(Ok)` if the link is valid /// /// `link_buffer` is needed for lifetime reasons; it will always be overwritten and the contents ignored. -crate fn preprocess_link<'a>( +fn preprocess_link<'a>( ori_link: &'a MarkdownLink, ) -> Option>> { // [] is mostly likely not supposed to be a link @@ -1494,7 +1497,7 @@ fn should_ignore_link(path_str: &str) -> bool { #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] /// Disambiguators for a link. -crate enum Disambiguator { +enum Disambiguator { /// `prim@` /// /// This is buggy, see @@ -1523,7 +1526,7 @@ impl Disambiguator { /// This returns `Ok(Some(...))` if a disambiguator was found, /// `Ok(None)` if no disambiguator was found, or `Err(...)` /// if there was a problem with the disambiguator. - crate fn from_str(link: &str) -> Result, (String, Range)> { + fn from_str(link: &str) -> Result, (String, Range)> { use Disambiguator::{Kind, Namespace as NS, Primitive}; if let Some(idx) = link.find('@') { diff --git a/src/librustdoc/passes/collect_intra_doc_links/early.rs b/src/librustdoc/passes/collect_intra_doc_links/early.rs new file mode 100644 index 0000000000000..7cba2523d1a3b --- /dev/null +++ b/src/librustdoc/passes/collect_intra_doc_links/early.rs @@ -0,0 +1,63 @@ +use rustc_ast as ast; +use rustc_hir::def::Namespace::TypeNS; +use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_INDEX}; +use rustc_interface::interface; + +use std::cell::RefCell; +use std::mem; +use std::rc::Rc; + +// Letting the resolver escape at the end of the function leads to inconsistencies between the +// crates the TyCtxt sees and the resolver sees (because the resolver could load more crates +// after escaping). Hopefully `IntraLinkCrateLoader` gets all the crates we need ... +crate struct IntraLinkCrateLoader { + current_mod: DefId, + crate resolver: Rc>, +} + +impl IntraLinkCrateLoader { + crate fn new(resolver: Rc>) -> Self { + let crate_id = LocalDefId { local_def_index: CRATE_DEF_INDEX }.to_def_id(); + Self { current_mod: crate_id, resolver } + } +} + +impl ast::visit::Visitor<'_> for IntraLinkCrateLoader { + fn visit_attribute(&mut self, attr: &ast::Attribute) { + use crate::html::markdown::markdown_links; + use crate::passes::collect_intra_doc_links::preprocess_link; + + if let Some(doc) = attr.doc_str() { + for link in markdown_links(&doc.as_str()) { + let path_str = if let Some(Ok(x)) = preprocess_link(&link) { + x.path_str + } else { + continue; + }; + self.resolver.borrow_mut().access(|resolver| { + let _ = resolver.resolve_str_path_error( + attr.span, + &path_str, + TypeNS, + self.current_mod, + ); + }); + } + } + ast::visit::walk_attribute(self, attr); + } + + fn visit_item(&mut self, item: &ast::Item) { + use rustc_ast_lowering::ResolverAstLowering; + + if let ast::ItemKind::Mod(..) = item.kind { + let new_mod = + self.resolver.borrow_mut().access(|resolver| resolver.local_def_id(item.id)); + let old_mod = mem::replace(&mut self.current_mod, new_mod.to_def_id()); + ast::visit::walk_item(self, item); + self.current_mod = old_mod; + } else { + ast::visit::walk_item(self, item); + } + } +}