diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index 75401f21862b6..81b32f436c8dd 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -259,6 +259,25 @@ impl Diagnostic { self } + pub fn multipart_suggestion( + &mut self, + msg: &str, + suggestion: Vec<(Span, String)>, + ) -> &mut Self { + self.suggestions.push(CodeSuggestion { + substitutions: vec![Substitution { + parts: suggestion + .into_iter() + .map(|(span, snippet)| SubstitutionPart { snippet, span }) + .collect(), + }], + msg: msg.to_owned(), + show_code_when_inline: true, + applicability: Applicability::Unspecified, + }); + self + } + /// Prints out a message with multiple suggested edits of the code. pub fn span_suggestions(&mut self, sp: Span, msg: &str, suggestions: Vec) -> &mut Self { self.suggestions.push(CodeSuggestion { diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index 7e9ca8633a53e..b813edadc577e 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/src/librustc_errors/diagnostic_builder.rs @@ -178,6 +178,11 @@ impl<'a> DiagnosticBuilder<'a> { msg: &str, suggestion: String) -> &mut Self); + forward!(pub fn multipart_suggestion( + &mut self, + msg: &str, + suggestion: Vec<(Span, String)> + ) -> &mut Self); forward!(pub fn span_suggestion(&mut self, sp: Span, msg: &str, diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index cb59af5480484..bdaee97e8f632 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -41,7 +41,7 @@ use rustc::ty; use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap}; use rustc::util::nodemap::{NodeMap, NodeSet, FxHashMap, FxHashSet, DefIdMap}; -use syntax::codemap::{BytePos, CodeMap}; +use syntax::codemap::CodeMap; use syntax::ext::hygiene::{Mark, MarkKind, SyntaxContext}; use syntax::ast::{self, Name, NodeId, Ident, FloatTy, IntTy, UintTy}; use syntax::ext::base::SyntaxExtension; @@ -210,12 +210,12 @@ fn resolve_struct_error<'sess, 'a>(resolver: &'sess Resolver, // Try to retrieve the span of the function signature and generate a new message with // a local type parameter let sugg_msg = "try using a local type parameter instead"; - if let Some((sugg_span, new_snippet)) = generate_local_type_param_snippet(cm, span) { + if let Some((sugg_span, new_snippet)) = cm.generate_local_type_param_snippet(span) { // Suggest the modification to the user err.span_suggestion(sugg_span, sugg_msg, new_snippet); - } else if let Some(sp) = generate_fn_name_span(cm, span) { + } else if let Some(sp) = cm.generate_fn_name_span(span) { err.span_label(sp, "try adding a local type parameter in this method instead"); } else { err.help("try using a local type parameter instead"); @@ -412,77 +412,6 @@ fn reduce_impl_span_to_impl_keyword(cm: &CodeMap, impl_span: Span) -> Span { impl_span } -fn generate_fn_name_span(cm: &CodeMap, span: Span) -> Option { - let prev_span = cm.span_extend_to_prev_str(span, "fn", true); - cm.span_to_snippet(prev_span).map(|snippet| { - let len = snippet.find(|c: char| !c.is_alphanumeric() && c != '_') - .expect("no label after fn"); - prev_span.with_hi(BytePos(prev_span.lo().0 + len as u32)) - }).ok() -} - -/// Take the span of a type parameter in a function signature and try to generate a span for the -/// function name (with generics) and a new snippet for this span with the pointed type parameter as -/// a new local type parameter. -/// -/// For instance: -/// ```rust,ignore (pseudo-Rust) -/// // Given span -/// fn my_function(param: T) -/// // ^ Original span -/// -/// // Result -/// fn my_function(param: T) -/// // ^^^^^^^^^^^ Generated span with snippet `my_function` -/// ``` -/// -/// Attention: The method used is very fragile since it essentially duplicates the work of the -/// parser. If you need to use this function or something similar, please consider updating the -/// codemap functions and this function to something more robust. -fn generate_local_type_param_snippet(cm: &CodeMap, span: Span) -> Option<(Span, String)> { - // Try to extend the span to the previous "fn" keyword to retrieve the function - // signature - let sugg_span = cm.span_extend_to_prev_str(span, "fn", false); - if sugg_span != span { - if let Ok(snippet) = cm.span_to_snippet(sugg_span) { - // Consume the function name - let mut offset = snippet.find(|c: char| !c.is_alphanumeric() && c != '_') - .expect("no label after fn"); - - // Consume the generics part of the function signature - let mut bracket_counter = 0; - let mut last_char = None; - for c in snippet[offset..].chars() { - match c { - '<' => bracket_counter += 1, - '>' => bracket_counter -= 1, - '(' => if bracket_counter == 0 { break; } - _ => {} - } - offset += c.len_utf8(); - last_char = Some(c); - } - - // Adjust the suggestion span to encompass the function name with its generics - let sugg_span = sugg_span.with_hi(BytePos(sugg_span.lo().0 + offset as u32)); - - // Prepare the new suggested snippet to append the type parameter that triggered - // the error in the generics of the function signature - let mut new_snippet = if last_char == Some('>') { - format!("{}, ", &snippet[..(offset - '>'.len_utf8())]) - } else { - format!("{}<", &snippet[..offset]) - }; - new_snippet.push_str(&cm.span_to_snippet(span).unwrap_or("T".to_string())); - new_snippet.push('>'); - - return Some((sugg_span, new_snippet)); - } - } - - None -} - #[derive(Copy, Clone, Debug)] struct BindingInfo { span: Span, diff --git a/src/librustc_typeck/check/compare_method.rs b/src/librustc_typeck/check/compare_method.rs index ba950f90d0a02..befe42f991fb7 100644 --- a/src/librustc_typeck/check/compare_method.rs +++ b/src/librustc_typeck/check/compare_method.rs @@ -720,7 +720,7 @@ fn compare_synthetic_generics<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, _trait_item_span: Option) // FIXME necessary? -> Result<(), ErrorReported> { // FIXME(chrisvittal) Clean up this function, list of FIXME items: - // 1. Better messages for the span lables + // 1. Better messages for the span labels // 2. Explanation as to what is going on // 3. Correct the function signature for what we actually use // If we get here, we already have the same number of generics, so the zip will @@ -751,8 +751,131 @@ fn compare_synthetic_generics<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, E0643, "method `{}` has incompatible signature for trait", trait_m.name); - err.span_label(trait_span, "annotation in trait"); - err.span_label(impl_span, "annotation in impl"); + err.span_label(trait_span, "declaration in trait here"); + match (impl_synthetic, trait_synthetic) { + // The case where the impl method uses `impl Trait` but the trait method uses + // explicit generics + (Some(hir::SyntheticTyParamKind::ImplTrait), None) => { + err.span_label(impl_span, "expected generic parameter, found `impl Trait`"); + (|| { + // try taking the name from the trait impl + // FIXME: this is obviously suboptimal since the name can already be used + // as another generic argument + let new_name = tcx + .sess + .codemap() + .span_to_snippet(trait_span) + .ok()?; + let trait_m = tcx.hir.as_local_node_id(trait_m.def_id)?; + let trait_m = tcx.hir.trait_item(hir::TraitItemId { node_id: trait_m }); + + let impl_m = tcx.hir.as_local_node_id(impl_m.def_id)?; + let impl_m = tcx.hir.impl_item(hir::ImplItemId { node_id: impl_m }); + + // in case there are no generics, take the spot between the function name + // and the opening paren of the argument list + let new_generics_span = tcx + .sess + .codemap() + .generate_fn_name_span(impl_m.span)? + .shrink_to_hi(); + // in case there are generics, just replace them + let generics_span = impl_m + .generics + .span + .substitute_dummy(new_generics_span); + // replace with the generics from the trait + let new_generics = tcx + .sess + .codemap() + .span_to_snippet(trait_m.generics.span) + .ok()?; + + err.multipart_suggestion( + "try changing the `impl Trait` argument to a generic parameter", + vec![ + // replace `impl Trait` with `T` + (impl_span, new_name), + // replace impl method generics with trait method generics + // This isn't quite right, as users might have changed the names + // of the generics, but it works for the common case + (generics_span, new_generics), + ], + ); + Some(()) + })(); + }, + // The case where the trait method uses `impl Trait`, but the impl method uses + // explicit generics. + (None, Some(hir::SyntheticTyParamKind::ImplTrait)) => { + err.span_label(impl_span, "expected `impl Trait`, found generic parameter"); + (|| { + let impl_m = tcx.hir.as_local_node_id(impl_m.def_id)?; + let impl_m = tcx.hir.impl_item(hir::ImplItemId { node_id: impl_m }); + let input_tys = match impl_m.node { + hir::ImplItemKind::Method(ref sig, _) => &sig.decl.inputs, + _ => unreachable!(), + }; + struct Visitor(Option, hir::def_id::DefId); + impl<'v> hir::intravisit::Visitor<'v> for Visitor { + fn visit_ty(&mut self, ty: &'v hir::Ty) { + hir::intravisit::walk_ty(self, ty); + match ty.node { + hir::TyPath(hir::QPath::Resolved(None, ref path)) => { + if let hir::def::Def::TyParam(def_id) = path.def { + if def_id == self.1 { + self.0 = Some(ty.span); + } + } + }, + _ => {} + } + } + fn nested_visit_map<'this>( + &'this mut self + ) -> hir::intravisit::NestedVisitorMap<'this, 'v> { + hir::intravisit::NestedVisitorMap::None + } + } + let mut visitor = Visitor(None, impl_def_id); + for ty in input_tys { + hir::intravisit::Visitor::visit_ty(&mut visitor, ty); + } + let span = visitor.0?; + + let param = impl_m.generics.params.iter().filter_map(|param| { + match param { + hir::GenericParam::Type(param) => { + if param.id == impl_node_id { + Some(param) + } else { + None + } + }, + hir::GenericParam::Lifetime(..) => None, + } + }).next()?; + let bounds = param.bounds.first()?.span().to(param.bounds.last()?.span()); + let bounds = tcx + .sess + .codemap() + .span_to_snippet(bounds) + .ok()?; + + err.multipart_suggestion( + "try removing the generic parameter and using `impl Trait` instead", + vec![ + // delete generic parameters + (impl_m.generics.span, String::new()), + // replace param usage with `impl Trait` + (span, format!("impl {}", bounds)), + ], + ); + Some(()) + })(); + }, + _ => unreachable!(), + } err.emit(); error_found = true; } diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index 73924c4270e66..fe8ce75a4b8ea 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -874,6 +874,78 @@ impl CodeMap { pub fn count_lines(&self) -> usize { self.files().iter().fold(0, |a, f| a + f.count_lines()) } + + + pub fn generate_fn_name_span(&self, span: Span) -> Option { + let prev_span = self.span_extend_to_prev_str(span, "fn", true); + self.span_to_snippet(prev_span).map(|snippet| { + let len = snippet.find(|c: char| !c.is_alphanumeric() && c != '_') + .expect("no label after fn"); + prev_span.with_hi(BytePos(prev_span.lo().0 + len as u32)) + }).ok() + } + + /// Take the span of a type parameter in a function signature and try to generate a span for the + /// function name (with generics) and a new snippet for this span with the pointed type + /// parameter as a new local type parameter. + /// + /// For instance: + /// ```rust,ignore (pseudo-Rust) + /// // Given span + /// fn my_function(param: T) + /// // ^ Original span + /// + /// // Result + /// fn my_function(param: T) + /// // ^^^^^^^^^^^ Generated span with snippet `my_function` + /// ``` + /// + /// Attention: The method used is very fragile since it essentially duplicates the work of the + /// parser. If you need to use this function or something similar, please consider updating the + /// codemap functions and this function to something more robust. + pub fn generate_local_type_param_snippet(&self, span: Span) -> Option<(Span, String)> { + // Try to extend the span to the previous "fn" keyword to retrieve the function + // signature + let sugg_span = self.span_extend_to_prev_str(span, "fn", false); + if sugg_span != span { + if let Ok(snippet) = self.span_to_snippet(sugg_span) { + // Consume the function name + let mut offset = snippet.find(|c: char| !c.is_alphanumeric() && c != '_') + .expect("no label after fn"); + + // Consume the generics part of the function signature + let mut bracket_counter = 0; + let mut last_char = None; + for c in snippet[offset..].chars() { + match c { + '<' => bracket_counter += 1, + '>' => bracket_counter -= 1, + '(' => if bracket_counter == 0 { break; } + _ => {} + } + offset += c.len_utf8(); + last_char = Some(c); + } + + // Adjust the suggestion span to encompass the function name with its generics + let sugg_span = sugg_span.with_hi(BytePos(sugg_span.lo().0 + offset as u32)); + + // Prepare the new suggested snippet to append the type parameter that triggered + // the error in the generics of the function signature + let mut new_snippet = if last_char == Some('>') { + format!("{}, ", &snippet[..(offset - '>'.len_utf8())]) + } else { + format!("{}<", &snippet[..offset]) + }; + new_snippet.push_str(&self.span_to_snippet(span).unwrap_or("T".to_string())); + new_snippet.push('>'); + + return Some((sugg_span, new_snippet)); + } + } + + None + } } impl CodeMapper for CodeMap { diff --git a/src/test/compile-fail/impl-trait/impl-generic-mismatch.rs b/src/test/ui/impl-trait/impl-generic-mismatch.rs similarity index 100% rename from src/test/compile-fail/impl-trait/impl-generic-mismatch.rs rename to src/test/ui/impl-trait/impl-generic-mismatch.rs diff --git a/src/test/ui/impl-trait/impl-generic-mismatch.stderr b/src/test/ui/impl-trait/impl-generic-mismatch.stderr new file mode 100644 index 0000000000000..e133d825ba015 --- /dev/null +++ b/src/test/ui/impl-trait/impl-generic-mismatch.stderr @@ -0,0 +1,44 @@ +error[E0643]: method `foo` has incompatible signature for trait + --> $DIR/impl-generic-mismatch.rs:18:12 + | +LL | fn foo(&self, _: &impl Debug); + | ---------- declaration in trait here +... +LL | fn foo(&self, _: &U) { } + | ^ expected `impl Trait`, found generic parameter +help: try removing the generic parameter and using `impl Trait` instead + | +LL | fn foo(&self, _: &impl Debug) { } + | + +error[E0643]: method `bar` has incompatible signature for trait + --> $DIR/impl-generic-mismatch.rs:27:23 + | +LL | fn bar(&self, _: &U); + | - declaration in trait here +... +LL | fn bar(&self, _: &impl Debug) { } + | ^^^^^^^^^^ expected generic parameter, found `impl Trait` +help: try changing the `impl Trait` argument to a generic parameter + | +LL | fn bar(&self, _: &U); +LL | } +LL | +LL | impl Bar for () { +LL | fn bar(&self, _: &U) { } + | + +error[E0643]: method `hash` has incompatible signature for trait + --> $DIR/impl-generic-mismatch.rs:38:33 + | +LL | fn hash(&self, hasher: &mut impl Hasher) {} + | ^^^^^^^^^^^ expected generic parameter, found `impl Trait` + | + ::: $SRC_DIR/libcore/hash/mod.rs:185:13 + | +LL | fn hash(&self, state: &mut H); + | - declaration in trait here + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0643`. diff --git a/src/test/ui/impl-trait/universal_wrong_bounds.rs b/src/test/ui/impl-trait/universal_wrong_bounds.rs index a5e948223fb1c..63d000c056835 100644 --- a/src/test/ui/impl-trait/universal_wrong_bounds.rs +++ b/src/test/ui/impl-trait/universal_wrong_bounds.rs @@ -13,12 +13,12 @@ use std::fmt::Display; fn foo(f: impl Display + Clone) -> String { wants_debug(f); wants_display(f); - wants_clone(f); //~ ERROR cannot find + wants_clone(f); } fn wants_debug(g: impl Debug) { } //~ ERROR cannot find fn wants_display(g: impl Debug) { } //~ ERROR cannot find -fn wants_cone(g: impl Clone) { } +fn wants_clone(g: impl Clone) { } fn main() { } diff --git a/src/test/ui/impl-trait/universal_wrong_bounds.stderr b/src/test/ui/impl-trait/universal_wrong_bounds.stderr index 02ac4707bc555..68a318e2ac4b7 100644 --- a/src/test/ui/impl-trait/universal_wrong_bounds.stderr +++ b/src/test/ui/impl-trait/universal_wrong_bounds.stderr @@ -1,9 +1,3 @@ -error[E0425]: cannot find function `wants_clone` in this scope - --> $DIR/universal_wrong_bounds.rs:16:5 - | -LL | wants_clone(f); //~ ERROR cannot find - | ^^^^^^^^^^^ did you mean `wants_cone`? - error[E0405]: cannot find trait `Debug` in this scope --> $DIR/universal_wrong_bounds.rs:19:24 | @@ -24,7 +18,6 @@ help: possible candidate is found in another module, you can import it into scop LL | use std::fmt::Debug; | -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors -Some errors occurred: E0405, E0425. -For more information about an error, try `rustc --explain E0405`. +For more information about this error, try `rustc --explain E0405`. diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 140c90aaeacc1..862c78312a013 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2861,6 +2861,15 @@ impl<'test> TestCx<'test> { let mut normalized = output.replace(&parent_dir_str, "$DIR"); + // Paths into the libstd/libcore + let src_dir = self.config.src_base.parent().unwrap().parent().unwrap(); + let src_dir_str = if json { + src_dir.display().to_string().replace("\\", "\\\\") + } else { + src_dir.display().to_string() + }; + normalized = normalized.replace(&src_dir_str, "$SRC_DIR"); + if json { // escaped newlines in json strings should be readable // in the stderr files. There's no point int being correct,