1
Fork 0

Suggest calling when operator types mismatch

This commit is contained in:
Michael Goulet 2022-08-28 01:08:24 +00:00
parent 2f78dd15a6
commit 18b640aee5
8 changed files with 199 additions and 164 deletions

View file

@ -1248,9 +1248,13 @@ impl HandlerInner {
}
fn treat_err_as_bug(&self) -> bool {
self.flags
.treat_err_as_bug
.map_or(false, |c| self.err_count() + self.lint_err_count >= c.get())
self.flags.treat_err_as_bug.map_or(false, |c| {
self.err_count()
+ self.lint_err_count
+ self.delayed_span_bugs.len()
+ self.delayed_good_path_bugs.len()
>= c.get()
})
}
fn print_error_count(&mut self, registry: &Registry) {
@ -1406,7 +1410,14 @@ impl HandlerInner {
// This is technically `self.treat_err_as_bug()` but `delay_span_bug` is called before
// incrementing `err_count` by one, so we need to +1 the comparing.
// FIXME: Would be nice to increment err_count in a more coherent way.
if self.flags.treat_err_as_bug.map_or(false, |c| self.err_count() + 1 >= c.get()) {
if self.flags.treat_err_as_bug.map_or(false, |c| {
self.err_count()
+ self.lint_err_count
+ self.delayed_span_bugs.len()
+ self.delayed_good_path_bugs.len()
+ 1
>= c.get()
}) {
// FIXME: don't abort here if report_delayed_bugs is off
self.span_bug(sp, msg);
}

View file

@ -41,7 +41,8 @@ macro_rules! pluralize {
/// All suggestions are marked with an `Applicability`. Tools use the applicability of a suggestion
/// to determine whether it should be automatically applied or if the user should be consulted
/// before applying the suggestion.
#[derive(Copy, Clone, Debug, PartialEq, Hash, Encodable, Decodable, Serialize, Deserialize)]
#[derive(Copy, Clone, Debug, Hash, Encodable, Decodable, Serialize, Deserialize)]
#[derive(PartialEq, Eq, PartialOrd, Ord)]
pub enum Applicability {
/// The suggestion is definitely what the user intended, or maintains the exact meaning of the code.
/// This suggestion should be automatically applied.

View file

@ -76,10 +76,68 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
found: Ty<'tcx>,
can_satisfy: impl FnOnce(Ty<'tcx>) -> bool,
) -> bool {
enum DefIdOrName {
DefId(DefId),
Name(&'static str),
let Some((def_id_or_name, output, num_inputs)) = self.extract_callable_info(expr, found)
else { return false; };
if can_satisfy(output) {
let (sugg_call, mut applicability) = match num_inputs {
0 => ("".to_string(), Applicability::MachineApplicable),
1..=4 => (
(0..num_inputs).map(|_| "_").collect::<Vec<_>>().join(", "),
Applicability::MachineApplicable,
),
_ => ("...".to_string(), Applicability::HasPlaceholders),
};
let msg = match def_id_or_name {
DefIdOrName::DefId(def_id) => match self.tcx.def_kind(def_id) {
DefKind::Ctor(CtorOf::Struct, _) => "instantiate this tuple struct".to_string(),
DefKind::Ctor(CtorOf::Variant, _) => {
"instantiate this tuple variant".to_string()
}
kind => format!("call this {}", kind.descr(def_id)),
},
DefIdOrName::Name(name) => format!("call this {name}"),
};
let sugg = match expr.kind {
hir::ExprKind::Call(..)
| hir::ExprKind::Path(..)
| hir::ExprKind::Index(..)
| hir::ExprKind::Lit(..) => {
vec![(expr.span.shrink_to_hi(), format!("({sugg_call})"))]
}
hir::ExprKind::Closure { .. } => {
// Might be `{ expr } || { bool }`
applicability = Applicability::MaybeIncorrect;
vec![
(expr.span.shrink_to_lo(), "(".to_string()),
(expr.span.shrink_to_hi(), format!(")({sugg_call})")),
]
}
_ => {
vec![
(expr.span.shrink_to_lo(), "(".to_string()),
(expr.span.shrink_to_hi(), format!(")({sugg_call})")),
]
}
};
err.multipart_suggestion_verbose(
format!("use parentheses to {msg}"),
sugg,
applicability,
);
return true;
}
false
}
fn extract_callable_info(
&self,
expr: &Expr<'_>,
found: Ty<'tcx>,
) -> Option<(DefIdOrName, Ty<'tcx>, usize)> {
// Autoderef is useful here because sometimes we box callables, etc.
let Some((def_id_or_name, output, inputs)) = self.autoderef(expr.span, found).silence_errors().find_map(|(found, _)| {
match *found.kind() {
@ -148,67 +206,83 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
_ => None,
}
}) else { return false; };
}) else { return None; };
let output = self.replace_bound_vars_with_fresh_vars(expr.span, infer::FnCall, output);
// We don't want to register any extra obligations, which should be
// implied by wf, but also because that would possibly result in
// erroneous errors later on.
let infer::InferOk { value: output, obligations: _ } =
self.normalize_associated_types_in_as_infer_ok(expr.span, output);
if !output.is_ty_var() && can_satisfy(output) {
let (sugg_call, mut applicability) = match inputs {
0 => ("".to_string(), Applicability::MachineApplicable),
1..=4 => (
(0..inputs).map(|_| "_").collect::<Vec<_>>().join(", "),
Applicability::MachineApplicable,
),
_ => ("...".to_string(), Applicability::HasPlaceholders),
};
let msg = match def_id_or_name {
DefIdOrName::DefId(def_id) => match self.tcx.def_kind(def_id) {
DefKind::Ctor(CtorOf::Struct, _) => "instantiate this tuple struct".to_string(),
DefKind::Ctor(CtorOf::Variant, _) => {
"instantiate this tuple variant".to_string()
if output.is_ty_var() { None } else { Some((def_id_or_name, output, inputs)) }
}
pub fn suggest_two_fn_call(
&self,
err: &mut Diagnostic,
lhs_expr: &'tcx hir::Expr<'tcx>,
lhs_ty: Ty<'tcx>,
rhs_expr: &'tcx hir::Expr<'tcx>,
rhs_ty: Ty<'tcx>,
can_satisfy: impl FnOnce(Ty<'tcx>, Ty<'tcx>) -> bool,
) -> bool {
let Some((_, lhs_output_ty, num_lhs_inputs)) = self.extract_callable_info(lhs_expr, lhs_ty)
else { return false; };
let Some((_, rhs_output_ty, num_rhs_inputs)) = self.extract_callable_info(rhs_expr, rhs_ty)
else { return false; };
if can_satisfy(lhs_output_ty, rhs_output_ty) {
let mut sugg = vec![];
let mut applicability = Applicability::MachineApplicable;
for (expr, num_inputs) in [(lhs_expr, num_lhs_inputs), (rhs_expr, num_rhs_inputs)] {
let (sugg_call, this_applicability) = match num_inputs {
0 => ("".to_string(), Applicability::MachineApplicable),
1..=4 => (
(0..num_inputs).map(|_| "_").collect::<Vec<_>>().join(", "),
Applicability::MachineApplicable,
),
_ => ("...".to_string(), Applicability::HasPlaceholders),
};
applicability = applicability.max(this_applicability);
match expr.kind {
hir::ExprKind::Call(..)
| hir::ExprKind::Path(..)
| hir::ExprKind::Index(..)
| hir::ExprKind::Lit(..) => {
sugg.extend([(expr.span.shrink_to_hi(), format!("({sugg_call})"))]);
}
hir::ExprKind::Closure { .. } => {
// Might be `{ expr } || { bool }`
applicability = Applicability::MaybeIncorrect;
sugg.extend([
(expr.span.shrink_to_lo(), "(".to_string()),
(expr.span.shrink_to_hi(), format!(")({sugg_call})")),
]);
}
_ => {
sugg.extend([
(expr.span.shrink_to_lo(), "(".to_string()),
(expr.span.shrink_to_hi(), format!(")({sugg_call})")),
]);
}
kind => format!("call this {}", kind.descr(def_id)),
},
DefIdOrName::Name(name) => format!("call this {name}"),
};
let sugg = match expr.kind {
hir::ExprKind::Call(..)
| hir::ExprKind::Path(..)
| hir::ExprKind::Index(..)
| hir::ExprKind::Lit(..) => {
vec![(expr.span.shrink_to_hi(), format!("({sugg_call})"))]
}
hir::ExprKind::Closure { .. } => {
// Might be `{ expr } || { bool }`
applicability = Applicability::MaybeIncorrect;
vec![
(expr.span.shrink_to_lo(), "(".to_string()),
(expr.span.shrink_to_hi(), format!(")({sugg_call})")),
]
}
_ => {
vec![
(expr.span.shrink_to_lo(), "(".to_string()),
(expr.span.shrink_to_hi(), format!(")({sugg_call})")),
]
}
};
}
err.multipart_suggestion_verbose(
format!("use parentheses to {msg}"),
format!("use parentheses to call these"),
sugg,
applicability,
);
return true;
true
} else {
false
}
false
}
pub fn suggest_deref_ref_or_into(
@ -959,3 +1033,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
}
enum DefIdOrName {
DefId(DefId),
Name(&'static str),
}

View file

@ -410,26 +410,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};
let mut err = struct_span_err!(self.tcx.sess, op.span, E0369, "{message}");
if !lhs_expr.span.eq(&rhs_expr.span) {
self.add_type_neq_err_label(
&mut err,
lhs_expr.span,
lhs_ty,
rhs_ty,
rhs_expr,
op,
is_assign,
expected,
);
self.add_type_neq_err_label(
&mut err,
rhs_expr.span,
rhs_ty,
lhs_ty,
lhs_expr,
op,
is_assign,
expected,
);
err.span_label(lhs_expr.span, lhs_ty.to_string());
err.span_label(rhs_expr.span, rhs_ty.to_string());
}
self.note_unmet_impls_on_type(&mut err, errors);
(err, missing_trait, use_output)
@ -468,17 +450,50 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
};
let is_compatible = |lhs_ty, rhs_ty| {
self.lookup_op_method(
lhs_ty,
Some(rhs_ty),
Some(rhs_expr),
Op::Binary(op, is_assign),
expected,
)
.is_ok()
};
// We should suggest `a + b` => `*a + b` if `a` is copy, and suggest
// `a += b` => `*a += b` if a is a mut ref.
if is_assign == IsAssign::Yes
&& let Some(lhs_deref_ty) = self.deref_once_mutably_for_diagnostic(lhs_ty) {
suggest_deref_binop(lhs_deref_ty);
if !op.span.can_be_used_for_suggestions() {
// Suppress suggestions when lhs and rhs are not in the same span as the error
} else if is_assign == IsAssign::Yes
&& let Some(lhs_deref_ty) = self.deref_once_mutably_for_diagnostic(lhs_ty)
{
suggest_deref_binop(lhs_deref_ty);
} else if is_assign == IsAssign::No
&& let Ref(_, lhs_deref_ty, _) = lhs_ty.kind() {
if self.type_is_copy_modulo_regions(self.param_env, *lhs_deref_ty, lhs_expr.span) {
&& let Ref(_, lhs_deref_ty, _) = lhs_ty.kind()
{
if self.type_is_copy_modulo_regions(
self.param_env,
*lhs_deref_ty,
lhs_expr.span,
) {
suggest_deref_binop(*lhs_deref_ty);
}
} else if self.suggest_fn_call(&mut err, lhs_expr, lhs_ty, |lhs_ty| {
is_compatible(lhs_ty, rhs_ty)
}) || self.suggest_fn_call(&mut err, rhs_expr, rhs_ty, |rhs_ty| {
is_compatible(lhs_ty, rhs_ty)
}) || self.suggest_two_fn_call(
&mut err,
rhs_expr,
rhs_ty,
lhs_expr,
lhs_ty,
|lhs_ty, rhs_ty| is_compatible(lhs_ty, rhs_ty),
) {
// Cool
}
if let Some(missing_trait) = missing_trait {
let mut visitor = TypeParamVisitor(vec![]);
visitor.visit_ty(lhs_ty);
@ -548,69 +563,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(lhs_ty, rhs_ty, return_ty)
}
/// If one of the types is an uncalled function and calling it would yield the other type,
/// suggest calling the function. Returns `true` if suggestion would apply (even if not given).
fn add_type_neq_err_label(
&self,
err: &mut Diagnostic,
span: Span,
ty: Ty<'tcx>,
other_ty: Ty<'tcx>,
other_expr: &'tcx hir::Expr<'tcx>,
op: hir::BinOp,
is_assign: IsAssign,
expected: Expectation<'tcx>,
) -> bool /* did we suggest to call a function because of missing parentheses? */ {
err.span_label(span, ty.to_string());
if let FnDef(def_id, _) = *ty.kind() {
if !self.tcx.has_typeck_results(def_id) {
return false;
}
// FIXME: Instead of exiting early when encountering bound vars in
// the function signature, consider keeping the binder here and
// propagating it downwards.
let Some(fn_sig) = self.tcx.fn_sig(def_id).no_bound_vars() else {
return false;
};
let other_ty = if let FnDef(def_id, _) = *other_ty.kind() {
if !self.tcx.has_typeck_results(def_id) {
return false;
}
// We're emitting a suggestion, so we can just ignore regions
self.tcx.fn_sig(def_id).skip_binder().output()
} else {
other_ty
};
if self
.lookup_op_method(
fn_sig.output(),
Some(other_ty),
Some(other_expr),
Op::Binary(op, is_assign),
expected,
)
.is_ok()
{
let (variable_snippet, applicability) = if !fn_sig.inputs().is_empty() {
("( /* arguments */ )", Applicability::HasPlaceholders)
} else {
("()", Applicability::MaybeIncorrect)
};
err.span_suggestion_verbose(
span.shrink_to_hi(),
"you might have forgotten to call this function",
variable_snippet,
applicability,
);
return true;
}
}
false
}
/// Provide actionable suggestions when trying to add two strings with incorrect types,
/// like `&str + &str`, `String + String` and `&str + &String`.
///

View file

@ -5,6 +5,11 @@ LL | if foo == y {}
| --- ^^ - _
| |
| for<'r> fn(&'r i32) -> &'r i32 {foo}
|
help: use parentheses to call this function
|
LL | if foo(_) == y {}
| +++
error: aborting due to previous error

View file

@ -6,14 +6,10 @@ LL | let x = f == g;
| |
| fn() {f}
|
help: you might have forgotten to call this function
help: use parentheses to call these
|
LL | let x = f() == g;
| ++
help: you might have forgotten to call this function
|
LL | let x = f == g();
| ++
LL | let x = f() == g();
| ++ ++
error[E0308]: mismatched types
--> $DIR/fn-compare-mismatch.rs:4:18

View file

@ -6,7 +6,7 @@ LL | foo > 12;
| |
| fn() -> i32 {foo}
|
help: you might have forgotten to call this function
help: use parentheses to call this function
|
LL | foo() > 12;
| ++
@ -28,10 +28,10 @@ LL | bar > 13;
| |
| fn(i64) -> i64 {bar}
|
help: you might have forgotten to call this function
help: use parentheses to call this function
|
LL | bar( /* arguments */ ) > 13;
| +++++++++++++++++++
LL | bar(_) > 13;
| +++
error[E0308]: mismatched types
--> $DIR/issue-59488.rs:18:11
@ -50,14 +50,10 @@ LL | foo > foo;
| |
| fn() -> i32 {foo}
|
help: you might have forgotten to call this function
help: use parentheses to call these
|
LL | foo() > foo;
| ++
help: you might have forgotten to call this function
|
LL | foo > foo();
| ++
LL | foo() > foo();
| ++ ++
error[E0369]: binary operation `>` cannot be applied to type `fn() -> i32 {foo}`
--> $DIR/issue-59488.rs:25:9

View file

@ -8,11 +8,6 @@ LL | assert_eq!(a, 0);
| {integer}
|
= note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)
help: you might have forgotten to call this function
--> $SRC_DIR/core/src/macros/mod.rs:LL:COL
|
LL | if !(*left_val() == *right_val) {
| ++
error[E0308]: mismatched types
--> $DIR/issue-70724-add_type_neq_err_label-unwrap.rs:6:5
@ -21,7 +16,7 @@ LL | assert_eq!(a, 0);
| ^^^^^^^^^^^^^^^^ expected fn item, found integer
|
= note: expected fn item `fn() -> i32 {a}`
found type `i32`
found type `{integer}`
= note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0277]: `fn() -> i32 {a}` doesn't implement `Debug`