Skip to content

Commit 7ed00ca

Browse files
estebankestebank
committed
Closure argument mismatch tweaks
 - use consistent phrasing for expected and found arguments - suggest changing arugments to tuple if possible - suggest changing single tuple argument to arguments if possible
1 parent bb345a0 commit 7ed00ca

File tree

3 files changed

+214
-177
lines changed

3 files changed

+214
-177
lines changed

‎src/librustc/traits/error_reporting.rs

+152-153
Original file line numberDiff line numberDiff line change
@@ -717,93 +717,40 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
717717
self.tcx.hir.span_if_local(did)
718718
}).map(|sp| self.tcx.sess.codemap().def_span(sp)); // the sp could be an fn def
719719

720-
let found_ty_count =
721-
match found_trait_ref.skip_binder().substs.type_at(1).sty {
722-
ty::TyTuple(ref tys, _) => tys.len(),
723-
_ => 1,
724-
};
725-
let (expected_tys, expected_ty_count) =
726-
match expected_trait_ref.skip_binder().substs.type_at(1).sty {
727-
ty::TyTuple(ref tys, _) =>
728-
(tys.iter().map(|t| &t.sty).collect(), tys.len()),
729-
ref sty => (vec![sty], 1),
730-
};
731-
if found_ty_count == expected_ty_count {
720+
let found = match found_trait_ref.skip_binder().substs.type_at(1).sty {
721+
ty::TyTuple(ref tys, _) => tys.iter()
722+
.map(|_| ArgKind::empty()).collect::<Vec<_>>(),
723+
_ => vec![ArgKind::empty()],
724+
};
725+
let expected = match expected_trait_ref.skip_binder().substs.type_at(1).sty {
726+
ty::TyTuple(ref tys, _) => tys.iter()
727+
.map(|t| match t.sty {
728+
ty::TypeVariants::TyTuple(ref tys, _) => ArgKind::Tuple(
729+
span,
730+
tys.iter()
731+
.map(|ty| ("_".to_owned(), format!("{}", ty.sty)))
732+
.collect::<Vec<_>>()
733+
),
734+
_ => ArgKind::Arg("_".to_owned(), format!("{}", t.sty)),
735+
}).collect(),
736+
ref sty => vec![ArgKind::Arg("_".to_owned(), format!("{}", sty))],
737+
};
738+
if found.len()== expected.len() {
732739
self.report_closure_arg_mismatch(span,
733740
found_span,
734741
found_trait_ref,
735742
expected_trait_ref)
736743
} else {
737-
let expected_tuple = if expected_ty_count == 1 {
738-
expected_tys.first().and_then(|t| {
739-
if let &&ty::TyTuple(ref tuptys, _) = t {
740-
Some(tuptys.len())
741-
} else {
742-
None
743-
}
744-
})
745-
} else {
746-
None
747-
};
748-
749-
// FIXME(#44150): Expand this to "N args expected but a N-tuple found."
750-
// Type of the 1st expected argument is somehow provided as type of a
751-
// found one in that case.
752-
//
753-
// ```
754-
// [1i32, 2, 3].sort_by(|(a, b)| ..)
755-
// // ^^^^^^^ --------
756-
// // expected_trait_ref: std::ops::FnMut<(&i32, &i32)>
757-
// // found_trait_ref: std::ops::FnMut<(&i32,)>
758-
// ```
759-
760-
let (closure_span, closure_args) = found_did
744+
let (closure_span, found) = found_did
761745
.and_then(|did| self.tcx.hir.get_if_local(did))
762-
.and_then(|node| {
763-
if let hir::map::NodeExpr(
764-
&hir::Expr {
765-
node: hir::ExprClosure(_, ref decl, id, span, _),
766-
..
767-
}) = node
768-
{
769-
let ty_snips = decl.inputs.iter()
770-
.map(|ty| {
771-
self.tcx.sess.codemap().span_to_snippet(ty.span).ok()
772-
.and_then(|snip| {
773-
// filter out dummy spans
774-
if snip == "," || snip == "|" {
775-
None
776-
} else {
777-
Some(snip)
778-
}
779-
})
780-
})
781-
.collect::<Vec<Option<String>>>();
782-
783-
let body = self.tcx.hir.body(id);
784-
let pat_snips = body.arguments.iter()
785-
.map(|arg|
786-
self.tcx.sess.codemap().span_to_snippet(arg.pat.span).ok())
787-
.collect::<Option<Vec<String>>>();
788-
789-
Some((span, pat_snips, ty_snips))
790-
} else {
791-
None
792-
}
793-
})
794-
.map(|(span, pat, ty)| (Some(span), Some((pat, ty))))
795-
.unwrap_or((None, None));
796-
let closure_args = closure_args.and_then(|(pat, ty)| Some((pat?, ty)));
797-
798-
self.report_arg_count_mismatch(
799-
span,
800-
closure_span.or(found_span),
801-
expected_ty_count,
802-
expected_tuple,
803-
found_ty_count,
804-
closure_args,
805-
found_trait_ty.is_closure()
806-
)
746+
.map(|node| self.get_fn_like_arguments(node))
747+
.unwrap_or((found_span.unwrap(), found));
748+
749+
self.report_arg_count_mismatch(span,
750+
closure_span,
751+
expected,
752+
found,
753+
found_trait_ty.is_closure())
807754
}
808755
}
809756

@@ -845,94 +792,135 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
845792
}
846793
}
847794

795+
fn get_fn_like_arguments(&self, node: hir::map::Node) -> (Span, Vec<ArgKind>) {
796+
if let hir::map::NodeExpr(&hir::Expr {
797+
node: hir::ExprClosure(_, ref _decl, id, span, _),
798+
..
799+
}) = node {
800+
(self.tcx.sess.codemap().def_span(span), self.tcx.hir.body(id).arguments.iter()
801+
.map(|arg| {
802+
if let hir::Pat {
803+
node: hir::PatKind::Tuple(args, _),
804+
span,
805+
..
806+
} = arg.pat.clone().into_inner() {
807+
ArgKind::Tuple(
808+
span,
809+
args.iter().map(|pat| {
810+
let snippet = self.tcx.sess.codemap()
811+
.span_to_snippet(pat.span).unwrap();
812+
(snippet, "_".to_owned())
813+
}).collect::<Vec<_>>(),
814+
)
815+
} else {
816+
let name = self.tcx.sess.codemap().span_to_snippet(arg.pat.span).unwrap();
817+
ArgKind::Arg(name, "_".to_owned())
818+
}
819+
})
820+
.collect::<Vec<ArgKind>>())
821+
} else if let hir::map::NodeItem(&hir::Item {
822+
span,
823+
node: hir::ItemFn(ref decl, ..),
824+
..
825+
}) = node {
826+
(self.tcx.sess.codemap().def_span(span), decl.inputs.iter()
827+
.map(|arg| match arg.clone().into_inner().node {
828+
hir::TyTup(ref tys) => ArgKind::Tuple(
829+
arg.span,
830+
tys.iter()
831+
.map(|_| ("_".to_owned(), "_".to_owned()))
832+
.collect::<Vec<_>>(),
833+
),
834+
_ => ArgKind::Arg("_".to_owned(), "_".to_owned())
835+
}).collect::<Vec<ArgKind>>())
836+
} else {
837+
panic!("non-FnLike node found: {:?}", node);
838+
}
839+
}
840+
848841
fn report_arg_count_mismatch(
849842
&self,
850843
span: Span,
851-
found_span: Option<Span>,
852-
expected: usize,
853-
expected_tuple: Option<usize>,
854-
found: usize,
855-
closure_args: Option<(Vec<String>, Vec<Option<String>>)>,
856-
is_closure: bool
844+
found_span: Span,
845+
expected_args: Vec<ArgKind>,
846+
found_args: Vec<ArgKind>,
847+
is_closure: bool,
857848
) -> DiagnosticBuilder<'tcx> {
858-
use std::borrow::Cow;
859-
860849
let kind = if is_closure { "closure" } else { "function" };
861850

862-
let args_str = |n, distinct| format!(
863-
"{} {}argument{}",
864-
n,
865-
if distinct && n >= 2 { "distinct " } else { "" },
866-
if n == 1 { "" } else { "s" },
867-
);
868-
869-
let expected_str = if let Some(n) = expected_tuple {
870-
assert!(expected == 1);
871-
if closure_args.as_ref().map(|&(ref pats, _)| pats.len()) == Some(n) {
872-
Cow::from("a single tuple as argument")
873-
} else {
874-
// be verbose when numbers differ
875-
Cow::from(format!("a single {}-tuple as argument", n))
851+
let args_str = |arguments: &Vec<ArgKind>, other: &Vec<ArgKind>| {
852+
let arg_length = arguments.len();
853+
let distinct = match &other[..] {
854+
&[ArgKind::Tuple(..)] => true,
855+
_ => false,
856+
};
857+
match (arg_length, arguments.get(0)) {
858+
(1, Some(&ArgKind::Tuple(_, ref fields))) => {
859+
format!("a single {}-tuple as argument", fields.len())
860+
}
861+
_ => format!("{} {}argument{}",
862+
arg_length,
863+
if distinct && arg_length > 1 { "distinct " } else { "" },
864+
if arg_length == 1 { "" } else { "s" }),
876865
}
877-
} else {
878-
Cow::from(args_str(expected, false))
879-
};
880-
881-
let found_str = if expected_tuple.is_some() {
882-
args_str(found, true)
883-
} else {
884-
args_str(found, false)
885866
};
886867

868+
let expected_str = args_str(&expected_args, &found_args);
869+
let found_str = args_str(&found_args, &expected_args);
887870

888-
let mut err = struct_span_err!(self.tcx.sess, span, E0593,
871+
let mut err = struct_span_err!(
872+
self.tcx.sess,
873+
span,
874+
E0593,
889875
"{} is expected to take {}, but it takes {}",
890876
kind,
891877
expected_str,
892878
found_str,
893879
);
894880

895-
err.span_label(
896-
span,
897-
format!(
898-
"expected {} that takes {}",
899-
kind,
900-
expected_str,
901-
)
902-
);
903-
904-
if let Some(span) = found_span {
905-
if let (Some(expected_tuple), Some((pats, tys))) = (expected_tuple, closure_args) {
906-
if expected_tuple != found || pats.len() != found {
907-
err.span_label(span, format!("takes {}", found_str));
908-
} else {
909-
let sugg = format!(
910-
"|({}){}|",
911-
pats.join(", "),
912-
913-
// add type annotations if available
914-
if tys.iter().any(|ty| ty.is_some()) {
915-
Cow::from(format!(
916-
": ({})",
917-
tys.into_iter().map(|ty| if let Some(ty) = ty {
918-
ty
919-
} else {
920-
"_".to_string()
921-
}).collect::<Vec<String>>().join(", ")
922-
))
923-
} else {
924-
Cow::from("")
925-
},
926-
);
927-
928-
err.span_suggestion(
929-
span,
930-
"consider changing the closure to accept a tuple",
931-
sugg
932-
);
933-
}
934-
} else {
935-
err.span_label(span, format!("takes {}", found_str));
881+
err.span_label(span, format!( "expected {} that takes {}", kind, expected_str));
882+
err.span_label(found_span, format!("takes {}", found_str));
883+
884+
if let &[ArgKind::Tuple(_, ref fields)] = &found_args[..] {
885+
if fields.len() == expected_args.len() {
886+
let sugg = fields.iter()
887+
.map(|(name, _)| name.to_owned())
888+
.collect::<Vec<String>>().join(", ");
889+
err.span_suggestion(found_span,
890+
"change the closure to take multiple arguments instead of \
891+
a single tuple",
892+
format!("|{}|", sugg));
893+
}
894+
}
895+
if let &[ArgKind::Tuple(_, ref fields)] = &expected_args[..] {
896+
if fields.len() == found_args.len() && is_closure {
897+
let sugg = format!(
898+
"|({}){}|",
899+
found_args.iter()
900+
.map(|arg| match arg {
901+
ArgKind::Arg(name, _) => name.to_owned(),
902+
_ => "_".to_owned(),
903+
})
904+
.collect::<Vec<String>>()
905+
.join(", "),
906+
// add type annotations if available
907+
if found_args.iter().any(|arg| match arg {
908+
ArgKind::Arg(_, ty) => ty != "_",
909+
_ => false,
910+
}) {
911+
format!(": ({})",
912+
fields.iter()
913+
.map(|(_, ty)| ty.to_owned())
914+
.collect::<Vec<String>>()
915+
.join(", "))
916+
} else {
917+
"".to_owned()
918+
},
919+
);
920+
err.span_suggestion(found_span,
921+
"change the closure to accept a tuple instead of individual \
922+
arguments",
923+
sugg);
936924
}
937925
}
938926

@@ -1325,3 +1313,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
13251313
suggested_limit));
13261314
}
13271315
}
1316+
1317+
enum ArgKind {
1318+
Arg(String, String),
1319+
Tuple(Span, Vec<(String, String)>),
1320+
}
1321+
1322+
impl ArgKind {
1323+
fn empty() -> ArgKind {
1324+
ArgKind::Arg("_".to_owned(), "_".to_owned())
1325+
}
1326+
}

‎src/test/ui/mismatched_types/closure-arg-count.rs

+5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ fn main() {
1818
//~^ ERROR closure is expected to take
1919
[1, 2, 3].sort_by(|(tuple, tuple2)| panic!());
2020
//~^ ERROR closure is expected to take
21+
[1, 2, 3].sort_by(|(tuple, tuple2): (usize, _)| panic!());
22+
//~^ ERROR closure is expected to take
2123
f(|| panic!());
2224
//~^ ERROR closure is expected to take
2325

@@ -32,6 +34,9 @@ fn main() {
3234
let bar = |i, x, y| i;
3335
let _it = vec![1, 2, 3].into_iter().enumerate().map(bar);
3436
//~^ ERROR closure is expected to take
37+
let _it = vec![1, 2, 3].into_iter().enumerate().map(qux);
38+
//~^ ERROR function is expected to take
3539
}
3640

3741
fn foo() {}
42+
fn qux(x: usize, y: usize) {}

0 commit comments

Comments
 (0)