bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Suggest if let x = y when encountering if x = y

Browse source 
Detect potential cases where `if let` was meant but `let` was left out.

Fix #44990.
This commit is contained in:
Esteban Küber 2020-08-25 20:28:25 -07:00
parent 85fbf49ce0
commit 07112ca62d
11 changed files with 268 additions and 102 deletions
Download patch file Download diff file Expand all files Collapse all files

7
compiler/rustc_resolve/src/late.rs
View file

@ -378,6 +378,9 @@ struct DiagnosticMetadata<'ast> {
/// Only used for better errors on `let <pat>: <expr, not type>;`.
current_let_binding: Option<(Span, Option<Span>, Option<Span>)>,
/// Used to detect possible `if let` written without `let` and to provide structured suggestion.
in_if_condition: Option<&'ast Expr>,
}
struct LateResolutionVisitor<'a, 'b, 'ast> {
@ -403,7 +406,7 @@ struct LateResolutionVisitor<'a, 'b, 'ast> {
///
/// In particular, rustdoc uses this to avoid giving errors for `cfg()` items.
/// In most cases this will be `None`, in which case errors will always be reported.
/// If it is `Some(_)`, then it will be updated when entering a nested function or trait body.
/// If it is `true`, then it will be updated when entering a nested function or trait body.
in_func_body: bool,
}
@ -2199,7 +2202,9 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
ExprKind::If(ref cond, ref then, ref opt_else) => {
self.with_rib(ValueNS, NormalRibKind, |this| {
let old = this.diagnostic_metadata.in_if_condition.replace(cond);
this.visit_expr(cond);
this.diagnostic_metadata.in_if_condition = old;
this.visit_block(then);
});
if let Some(expr) = opt_else {

13
compiler/rustc_resolve/src/late/diagnostics.rs
View file

@ -176,6 +176,19 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
let code = source.error_code(res.is_some());
let mut err = self.r.session.struct_span_err_with_code(base_span, &base_msg, code);
match (source, self.diagnostic_metadata.in_if_condition) {
(PathSource::Expr(_), Some(Expr { span, kind: ExprKind::Assign(..), .. })) => {
err.span_suggestion_verbose(
span.shrink_to_lo(),
"you might have meant to use pattern matching",
"let ".to_string(),
Applicability::MaybeIncorrect,
);
self.r.session.if_let_suggestions.borrow_mut().insert(*span);
}
_ => {}
}
let is_assoc_fn = self.self_type_is_available(span);
// Emit help message for fake-self from other languages (e.g., `this` in Javascript).
if ["this", "my"].contains(&&*item_str.as_str()) && is_assoc_fn {

4
compiler/rustc_session/src/session.rs
View file

@ -213,6 +213,9 @@ pub struct Session {
known_attrs: Lock<MarkedAttrs>,
used_attrs: Lock<MarkedAttrs>,
/// `Span`s for `if` conditions that we have suggested turning into `if let`.
pub if_let_suggestions: Lock<FxHashSet<Span>>,
}
pub struct PerfStats {
@ -1354,6 +1357,7 @@ pub fn build_session(
target_features: FxHashSet::default(),
known_attrs: Lock::new(MarkedAttrs::new()),
used_attrs: Lock::new(MarkedAttrs::new()),
if_let_suggestions: Default::default(),
};
validate_commandline_args_with_session_available(&sess);

55
compiler/rustc_typeck/src/check/expr.rs
View file

@ -764,9 +764,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
rhs: &'tcx hir::Expr<'tcx>,
span: &Span,
) -> Ty<'tcx> {
let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));
let expected_ty = expected.coercion_target_type(self, expr.span);
if expected_ty == self.tcx.types.bool {
// The expected type is `bool` but this will result in `()` so we can reasonably
@ -774,20 +771,52 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// The likely cause of this is `if foo = bar { .. }`.
let actual_ty = self.tcx.mk_unit();
let mut err = self.demand_suptype_diag(expr.span, expected_ty, actual_ty).unwrap();
let msg = "try comparing for equality";
let left = self.tcx.sess.source_map().span_to_snippet(lhs.span);
let right = self.tcx.sess.source_map().span_to_snippet(rhs.span);
if let (Ok(left), Ok(right)) = (left, right) {
let help = format!("{} == {}", left, right);
err.span_suggestion(expr.span, msg, help, Applicability::MaybeIncorrect);
let lhs_ty = self.check_expr(&lhs);
let rhs_ty = self.check_expr(&rhs);
if self.can_coerce(lhs_ty, rhs_ty) {
if !lhs.is_syntactic_place_expr() {
// Do not suggest `if let x = y` as `==` is way more likely to be the intention.
if let hir::Node::Expr(hir::Expr {
kind: ExprKind::Match(_, _, hir::MatchSource::IfDesugar { .. }),
..
}) = self.tcx.hir().get(
self.tcx.hir().get_parent_node(self.tcx.hir().get_parent_node(expr.hir_id)),
) {
// Likely `if let` intended.
err.span_suggestion_verbose(
expr.span.shrink_to_lo(),
"you might have meant to use pattern matching",
"let ".to_string(),
Applicability::MaybeIncorrect,
);
}
}
err.span_suggestion_verbose(
*span,
"you might have meant to compare for equality",
"==".to_string(),
Applicability::MaybeIncorrect,
);
} else {
err.help(msg);
// Do this to cause extra errors about the assignment.
let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
let _ = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));
}
err.emit();
} else {
self.check_lhs_assignable(lhs, "E0070", span);
if self.sess().if_let_suggestions.borrow().get(&expr.span).is_some() {
// We already emitted an `if let` suggestion due to an identifier not found.
err.delay_as_bug();
} else {
err.emit();
}
return self.tcx.ty_error();
}
self.check_lhs_assignable(lhs, "E0070", span);
let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));
self.require_type_is_sized(lhs_ty, lhs.span, traits::AssignmentLhsSized);
if lhs_ty.references_error() || rhs_ty.references_error() {