Skip to content

Commit b4463d7

Browse files
borsbors
committed
Auto merge of #50943 - oli-obk:cleanups, r=estebank
impl Trait diagnostic/test cleanups
2 parents 242195d + 849c565 commit b4463d7

File tree

10 files changed

+282
-88
lines changed

10 files changed

+282
-88
lines changed

‎src/librustc_errors/diagnostic.rs

+19
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,25 @@ impl Diagnostic {
259259
self
260260
}
261261

262+
pub fn multipart_suggestion(
263+
&mut self,
264+
msg: &str,
265+
suggestion: Vec<(Span, String)>,
266+
) -> &mut Self {
267+
self.suggestions.push(CodeSuggestion {
268+
substitutions: vec![Substitution {
269+
parts: suggestion
270+
.into_iter()
271+
.map(|(span, snippet)| SubstitutionPart { snippet, span })
272+
.collect(),
273+
}],
274+
msg: msg.to_owned(),
275+
show_code_when_inline: true,
276+
applicability: Applicability::Unspecified,
277+
});
278+
self
279+
}
280+
262281
/// Prints out a message with multiple suggested edits of the code.
263282
pub fn span_suggestions(&mut self, sp: Span, msg: &str, suggestions: Vec<String>) -> &mut Self {
264283
self.suggestions.push(CodeSuggestion {

‎src/librustc_errors/diagnostic_builder.rs

+5
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,11 @@ impl<'a> DiagnosticBuilder<'a> {
178178
msg: &str,
179179
suggestion: String)
180180
-> &mut Self);
181+
forward!(pub fn multipart_suggestion(
182+
&mut self,
183+
msg: &str,
184+
suggestion: Vec<(Span, String)>
185+
) -> &mut Self);
181186
forward!(pub fn span_suggestion(&mut self,
182187
sp: Span,
183188
msg: &str,

‎src/librustc_resolve/lib.rs

+3-74
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ use rustc::ty;
4242
use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap};
4343
use rustc::util::nodemap::{NodeMap, NodeSet, FxHashMap, FxHashSet, DefIdMap};
4444

45-
use syntax::codemap::{BytePos, CodeMap};
45+
use syntax::codemap::CodeMap;
4646
use syntax::ext::hygiene::{Mark, MarkKind, SyntaxContext};
4747
use syntax::ast::{self, Name, NodeId, Ident, FloatTy, IntTy, UintTy};
4848
use syntax::ext::base::SyntaxExtension;
@@ -211,12 +211,12 @@ fn resolve_struct_error<'sess, 'a>(resolver: &'sess Resolver,
211211
// Try to retrieve the span of the function signature and generate a new message with
212212
// a local type parameter
213213
let sugg_msg = "try using a local type parameter instead";
214-
if let Some((sugg_span, new_snippet)) = generate_local_type_param_snippet(cm, span) {
214+
if let Some((sugg_span, new_snippet)) = cm.generate_local_type_param_snippet(span) {
215215
// Suggest the modification to the user
216216
err.span_suggestion(sugg_span,
217217
sugg_msg,
218218
new_snippet);
219-
} else if let Some(sp) = generate_fn_name_span(cm, span) {
219+
} else if let Some(sp) = cm.generate_fn_name_span(span) {
220220
err.span_label(sp, "try adding a local type parameter in this method instead");
221221
} else {
222222
err.help("try using a local type parameter instead");
@@ -413,77 +413,6 @@ fn reduce_impl_span_to_impl_keyword(cm: &CodeMap, impl_span: Span) -> Span {
413413
impl_span
414414
}
415415

416-
fn generate_fn_name_span(cm: &CodeMap, span: Span) -> Option<Span> {
417-
let prev_span = cm.span_extend_to_prev_str(span, "fn", true);
418-
cm.span_to_snippet(prev_span).map(|snippet| {
419-
let len = snippet.find(|c: char| !c.is_alphanumeric() && c != '_')
420-
.expect("no label after fn");
421-
prev_span.with_hi(BytePos(prev_span.lo().0 + len as u32))
422-
}).ok()
423-
}
424-
425-
/// Take the span of a type parameter in a function signature and try to generate a span for the
426-
/// function name (with generics) and a new snippet for this span with the pointed type parameter as
427-
/// a new local type parameter.
428-
///
429-
/// For instance:
430-
/// ```rust,ignore (pseudo-Rust)
431-
/// // Given span
432-
/// fn my_function(param: T)
433-
/// // ^ Original span
434-
///
435-
/// // Result
436-
/// fn my_function(param: T)
437-
/// // ^^^^^^^^^^^ Generated span with snippet `my_function<T>`
438-
/// ```
439-
///
440-
/// Attention: The method used is very fragile since it essentially duplicates the work of the
441-
/// parser. If you need to use this function or something similar, please consider updating the
442-
/// codemap functions and this function to something more robust.
443-
fn generate_local_type_param_snippet(cm: &CodeMap, span: Span) -> Option<(Span, String)> {
444-
// Try to extend the span to the previous "fn" keyword to retrieve the function
445-
// signature
446-
let sugg_span = cm.span_extend_to_prev_str(span, "fn", false);
447-
if sugg_span != span {
448-
if let Ok(snippet) = cm.span_to_snippet(sugg_span) {
449-
// Consume the function name
450-
let mut offset = snippet.find(|c: char| !c.is_alphanumeric() && c != '_')
451-
.expect("no label after fn");
452-
453-
// Consume the generics part of the function signature
454-
let mut bracket_counter = 0;
455-
let mut last_char = None;
456-
for c in snippet[offset..].chars() {
457-
match c {
458-
'<' => bracket_counter += 1,
459-
'>' => bracket_counter -= 1,
460-
'(' => if bracket_counter == 0 { break; }
461-
_ => {}
462-
}
463-
offset += c.len_utf8();
464-
last_char = Some(c);
465-
}
466-
467-
// Adjust the suggestion span to encompass the function name with its generics
468-
let sugg_span = sugg_span.with_hi(BytePos(sugg_span.lo().0 + offset as u32));
469-
470-
// Prepare the new suggested snippet to append the type parameter that triggered
471-
// the error in the generics of the function signature
472-
let mut new_snippet = if last_char == Some('>') {
473-
format!("{}, ", &snippet[..(offset - '>'.len_utf8())])
474-
} else {
475-
format!("{}<", &snippet[..offset])
476-
};
477-
new_snippet.push_str(&cm.span_to_snippet(span).unwrap_or("T".to_string()));
478-
new_snippet.push('>');
479-
480-
return Some((sugg_span, new_snippet));
481-
}
482-
}
483-
484-
None
485-
}
486-
487416
#[derive(Copy, Clone, Debug)]
488417
struct BindingInfo {
489418
span: Span,

‎src/librustc_typeck/check/compare_method.rs

+126-3
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,7 @@ fn compare_synthetic_generics<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
720720
_trait_item_span: Option<Span>) // FIXME necessary?
721721
-> Result<(), ErrorReported> {
722722
// FIXME(chrisvittal) Clean up this function, list of FIXME items:
723-
// 1. Better messages for the span lables
723+
// 1. Better messages for the span labels
724724
// 2. Explanation as to what is going on
725725
// 3. Correct the function signature for what we actually use
726726
// 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>,
751751
E0643,
752752
"method `{}` has incompatible signature for trait",
753753
trait_m.name);
754-
err.span_label(trait_span, "annotation in trait");
755-
err.span_label(impl_span, "annotation in impl");
754+
err.span_label(trait_span, "declaration in trait here");
755+
match (impl_synthetic, trait_synthetic) {
756+
// The case where the impl method uses `impl Trait` but the trait method uses
757+
// explicit generics
758+
(Some(hir::SyntheticTyParamKind::ImplTrait), None) => {
759+
err.span_label(impl_span, "expected generic parameter, found `impl Trait`");
760+
(|| {
761+
// try taking the name from the trait impl
762+
// FIXME: this is obviously suboptimal since the name can already be used
763+
// as another generic argument
764+
let new_name = tcx
765+
.sess
766+
.codemap()
767+
.span_to_snippet(trait_span)
768+
.ok()?;
769+
let trait_m = tcx.hir.as_local_node_id(trait_m.def_id)?;
770+
let trait_m = tcx.hir.trait_item(hir::TraitItemId { node_id: trait_m });
771+
772+
let impl_m = tcx.hir.as_local_node_id(impl_m.def_id)?;
773+
let impl_m = tcx.hir.impl_item(hir::ImplItemId { node_id: impl_m });
774+
775+
// in case there are no generics, take the spot between the function name
776+
// and the opening paren of the argument list
777+
let new_generics_span = tcx
778+
.sess
779+
.codemap()
780+
.generate_fn_name_span(impl_m.span)?
781+
.shrink_to_hi();
782+
// in case there are generics, just replace them
783+
let generics_span = impl_m
784+
.generics
785+
.span
786+
.substitute_dummy(new_generics_span);
787+
// replace with the generics from the trait
788+
let new_generics = tcx
789+
.sess
790+
.codemap()
791+
.span_to_snippet(trait_m.generics.span)
792+
.ok()?;
793+
794+
err.multipart_suggestion(
795+
"try changing the `impl Trait` argument to a generic parameter",
796+
vec![
797+
// replace `impl Trait` with `T`
798+
(impl_span, new_name),
799+
// replace impl method generics with trait method generics
800+
// This isn't quite right, as users might have changed the names
801+
// of the generics, but it works for the common case
802+
(generics_span, new_generics),
803+
],
804+
);
805+
Some(())
806+
})();
807+
},
808+
// The case where the trait method uses `impl Trait`, but the impl method uses
809+
// explicit generics.
810+
(None, Some(hir::SyntheticTyParamKind::ImplTrait)) => {
811+
err.span_label(impl_span, "expected `impl Trait`, found generic parameter");
812+
(|| {
813+
let impl_m = tcx.hir.as_local_node_id(impl_m.def_id)?;
814+
let impl_m = tcx.hir.impl_item(hir::ImplItemId { node_id: impl_m });
815+
let input_tys = match impl_m.node {
816+
hir::ImplItemKind::Method(ref sig, _) => &sig.decl.inputs,
817+
_ => unreachable!(),
818+
};
819+
struct Visitor(Option<Span>, hir::def_id::DefId);
820+
impl<'v> hir::intravisit::Visitor<'v> for Visitor {
821+
fn visit_ty(&mut self, ty: &'v hir::Ty) {
822+
hir::intravisit::walk_ty(self, ty);
823+
match ty.node {
824+
hir::TyPath(hir::QPath::Resolved(None, ref path)) => {
825+
if let hir::def::Def::TyParam(def_id) = path.def {
826+
if def_id == self.1 {
827+
self.0 = Some(ty.span);
828+
}
829+
}
830+
},
831+
_ => {}
832+
}
833+
}
834+
fn nested_visit_map<'this>(
835+
&'this mut self
836+
) -> hir::intravisit::NestedVisitorMap<'this, 'v> {
837+
hir::intravisit::NestedVisitorMap::None
838+
}
839+
}
840+
let mut visitor = Visitor(None, impl_def_id);
841+
for ty in input_tys {
842+
hir::intravisit::Visitor::visit_ty(&mut visitor, ty);
843+
}
844+
let span = visitor.0?;
845+
846+
let param = impl_m.generics.params.iter().filter_map(|param| {
847+
match param {
848+
hir::GenericParam::Type(param) => {
849+
if param.id == impl_node_id {
850+
Some(param)
851+
} else {
852+
None
853+
}
854+
},
855+
hir::GenericParam::Lifetime(..) => None,
856+
}
857+
}).next()?;
858+
let bounds = param.bounds.first()?.span().to(param.bounds.last()?.span());
859+
let bounds = tcx
860+
.sess
861+
.codemap()
862+
.span_to_snippet(bounds)
863+
.ok()?;
864+
865+
err.multipart_suggestion(
866+
"try removing the generic parameter and using `impl Trait` instead",
867+
vec![
868+
// delete generic parameters
869+
(impl_m.generics.span, String::new()),
870+
// replace param usage with `impl Trait`
871+
(span, format!("impl {}", bounds)),
872+
],
873+
);
874+
Some(())
875+
})();
876+
},
877+
_ => unreachable!(),
878+
}
756879
err.emit();
757880
error_found = true;
758881
}

‎src/libsyntax/codemap.rs

+72
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,78 @@ impl CodeMap {
874874
pub fn count_lines(&self) -> usize {
875875
self.files().iter().fold(0, |a, f| a + f.count_lines())
876876
}
877+
878+
879+
pub fn generate_fn_name_span(&self, span: Span) -> Option<Span> {
880+
let prev_span = self.span_extend_to_prev_str(span, "fn", true);
881+
self.span_to_snippet(prev_span).map(|snippet| {
882+
let len = snippet.find(|c: char| !c.is_alphanumeric() && c != '_')
883+
.expect("no label after fn");
884+
prev_span.with_hi(BytePos(prev_span.lo().0 + len as u32))
885+
}).ok()
886+
}
887+
888+
/// Take the span of a type parameter in a function signature and try to generate a span for the
889+
/// function name (with generics) and a new snippet for this span with the pointed type
890+
/// parameter as a new local type parameter.
891+
///
892+
/// For instance:
893+
/// ```rust,ignore (pseudo-Rust)
894+
/// // Given span
895+
/// fn my_function(param: T)
896+
/// // ^ Original span
897+
///
898+
/// // Result
899+
/// fn my_function(param: T)
900+
/// // ^^^^^^^^^^^ Generated span with snippet `my_function<T>`
901+
/// ```
902+
///
903+
/// Attention: The method used is very fragile since it essentially duplicates the work of the
904+
/// parser. If you need to use this function or something similar, please consider updating the
905+
/// codemap functions and this function to something more robust.
906+
pub fn generate_local_type_param_snippet(&self, span: Span) -> Option<(Span, String)> {
907+
// Try to extend the span to the previous "fn" keyword to retrieve the function
908+
// signature
909+
let sugg_span = self.span_extend_to_prev_str(span, "fn", false);
910+
if sugg_span != span {
911+
if let Ok(snippet) = self.span_to_snippet(sugg_span) {
912+
// Consume the function name
913+
let mut offset = snippet.find(|c: char| !c.is_alphanumeric() && c != '_')
914+
.expect("no label after fn");
915+
916+
// Consume the generics part of the function signature
917+
let mut bracket_counter = 0;
918+
let mut last_char = None;
919+
for c in snippet[offset..].chars() {
920+
match c {
921+
'<' => bracket_counter += 1,
922+
'>' => bracket_counter -= 1,
923+
'(' => if bracket_counter == 0 { break; }
924+
_ => {}
925+
}
926+
offset += c.len_utf8();
927+
last_char = Some(c);
928+
}
929+
930+
// Adjust the suggestion span to encompass the function name with its generics
931+
let sugg_span = sugg_span.with_hi(BytePos(sugg_span.lo().0 + offset as u32));
932+
933+
// Prepare the new suggested snippet to append the type parameter that triggered
934+
// the error in the generics of the function signature
935+
let mut new_snippet = if last_char == Some('>') {
936+
format!("{}, ", &snippet[..(offset - '>'.len_utf8())])
937+
} else {
938+
format!("{}<", &snippet[..offset])
939+
};
940+
new_snippet.push_str(&self.span_to_snippet(span).unwrap_or("T".to_string()));
941+
new_snippet.push('>');
942+
943+
return Some((sugg_span, new_snippet));
944+
}
945+
}
946+
947+
None
948+
}
877949
}
878950

879951
impl CodeMapper for CodeMap {

‎src/test/compile-fail/impl-trait/impl-generic-mismatch.rs renamed to ‎src/test/ui/impl-trait/impl-generic-mismatch.rs

File renamed without changes.

0 commit comments

Comments
 (0)