From aa03f1f5e3791f1ff07d414ba003f395ad6538d8 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 28 Sep 2019 02:30:48 +0200 Subject: [PATCH] Improve diagnostic for `let A = 0;` where `A` is a constant, not a new variable. --- src/librustc/hir/map/mod.rs | 8 ++++ src/librustc_mir/hair/pattern/check_match.rs | 47 +++++++++++++++---- src/librustc_typeck/astconv.rs | 7 +-- src/librustc_typeck/check/callee.rs | 11 +---- .../consts/const-pattern-irrefutable.stderr | 24 ++++++++-- .../const-pat-non-exaustive-let-new-var.rs | 10 ++++ ...const-pat-non-exaustive-let-new-var.stderr | 15 ++++++ 7 files changed, 96 insertions(+), 26 deletions(-) create mode 100644 src/test/ui/suggestions/const-pat-non-exaustive-let-new-var.rs create mode 100644 src/test/ui/suggestions/const-pat-non-exaustive-let-new-var.stderr diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index d4efe0297b6..e7f4c5982f6 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -1064,6 +1064,14 @@ impl<'hir> Map<'hir> { self.as_local_hir_id(id).map(|id| self.span(id)) } + pub fn res_span(&self, res: Res) -> Option { + match res { + Res::Err => None, + Res::Local(id) => Some(self.span(id)), + res => self.span_if_local(res.opt_def_id()?), + } + } + pub fn node_to_string(&self, id: HirId) -> String { hir_id_to_string(self, id, true) } diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index 4572519683d..3a8e5f0930c 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -270,20 +270,51 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { "refutable pattern in {}: {} not covered", origin, joined_patterns ); - err.span_label(pat.span, match &pat.kind { + match &pat.kind { hir::PatKind::Path(hir::QPath::Resolved(None, path)) - if path.segments.len() == 1 && path.segments[0].args.is_none() => { - format!("interpreted as {} {} pattern, not new variable", - path.res.article(), path.res.descr()) + if path.segments.len() == 1 && path.segments[0].args.is_none() => + { + const_not_var(&mut err, cx.tcx, pat, path); } - _ => pattern_not_convered_label(&witnesses, &joined_patterns), - }); + _ => { + err.span_label( + pat.span, + pattern_not_covered_label(&witnesses, &joined_patterns), + ); + } + } + adt_defined_here(cx, &mut err, pattern_ty, &witnesses); err.emit(); }); } } +/// A path pattern was interpreted as a constant, not a new variable. +/// This caused an irrefutable match failure in e.g. `let`. +fn const_not_var(err: &mut DiagnosticBuilder<'_>, tcx: TyCtxt<'_>, pat: &Pat, path: &hir::Path) { + let descr = path.res.descr(); + err.span_label(pat.span, format!( + "interpreted as {} {} pattern, not a new variable", + path.res.article(), + descr, + )); + + err.span_suggestion( + pat.span, + "introduce a variable instead", + format!("{}_var", path.segments[0].ident).to_lowercase(), + // Cannot use `MachineApplicable` as it's not really *always* correct + // because there may be such an identifier in scope or the user maybe + // really wanted to match against the constant. This is quite unlikely however. + Applicability::MaybeIncorrect, + ); + + if let Some(span) = tcx.hir().res_span(path.res) { + err.span_label(span, format!("{} defined here", descr)); + } +} + fn check_for_bindings_named_same_as_variants(cx: &MatchVisitor<'_, '_>, pat: &Pat) { pat.walk(|p| { if let hir::PatKind::Binding(_, _, ident, None) = p.kind { @@ -449,7 +480,7 @@ fn check_exhaustive<'tcx>( cx.tcx.sess, sp, format!("non-exhaustive patterns: {} not covered", joined_patterns), ); - err.span_label(sp, pattern_not_convered_label(&witnesses, &joined_patterns)); + err.span_label(sp, pattern_not_covered_label(&witnesses, &joined_patterns)); adt_defined_here(cx, &mut err, scrut_ty, &witnesses); err.help( "ensure that all possible cases are being handled, \ @@ -475,7 +506,7 @@ fn joined_uncovered_patterns(witnesses: &[super::Pat<'_>]) -> String { } } -fn pattern_not_convered_label(witnesses: &[super::Pat<'_>], joined_patterns: &str) -> String { +fn pattern_not_covered_label(witnesses: &[super::Pat<'_>], joined_patterns: &str) -> String { format!("pattern{} {} not covered", rustc_errors::pluralise!(witnesses.len()), joined_patterns) } diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 6f1d854481a..5606f36632e 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -1368,11 +1368,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { span, format!("associated type `{}` must be specified", assoc_item.ident), ); - if item_def_id.is_local() { - err.span_label( - tcx.def_span(*item_def_id), - format!("`{}` defined here", assoc_item.ident), - ); + if let Some(sp) = tcx.hir().span_if_local(*item_def_id) { + err.span_label(sp, format!("`{}` defined here", assoc_item.ident)); } if suggest { if let Ok(snippet) = tcx.sess.source_map().span_to_snippet( diff --git a/src/librustc_typeck/check/callee.rs b/src/librustc_typeck/check/callee.rs index 4d8ec6fb0b8..7e0a3e78188 100644 --- a/src/librustc_typeck/check/callee.rs +++ b/src/librustc_typeck/check/callee.rs @@ -351,16 +351,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.span_label(call_expr.span, "call expression requires function"); - let def_span = match def { - Res::Err => None, - Res::Local(id) => { - Some(self.tcx.hir().span(id)) - }, - _ => def - .opt_def_id() - .and_then(|did| self.tcx.hir().span_if_local(did)), - }; - if let Some(span) = def_span { + if let Some(span) = self.tcx.hir().res_span(def) { let label = match (unit_variant, inner_callee_path) { (Some(path), _) => format!("`{}` defined here", path), (_, Some(hir::QPath::Resolved(_, path))) => format!( diff --git a/src/test/ui/consts/const-pattern-irrefutable.stderr b/src/test/ui/consts/const-pattern-irrefutable.stderr index 06f5e90d2f1..4814aa9a5b2 100644 --- a/src/test/ui/consts/const-pattern-irrefutable.stderr +++ b/src/test/ui/consts/const-pattern-irrefutable.stderr @@ -1,20 +1,38 @@ error[E0005]: refutable pattern in local binding: `0u8..=1u8` and `3u8..=std::u8::MAX` not covered --> $DIR/const-pattern-irrefutable.rs:12:9 | +LL | const a: u8 = 2; + | ---------------- constant defined here +... LL | let a = 4; - | ^ interpreted as a constant pattern, not new variable + | ^ + | | + | interpreted as a constant pattern, not a new variable + | help: introduce a variable instead: `a_var` error[E0005]: refutable pattern in local binding: `0u8..=1u8` and `3u8..=std::u8::MAX` not covered --> $DIR/const-pattern-irrefutable.rs:13:9 | +LL | pub const b: u8 = 2; + | -------------------- constant defined here +... LL | let c = 4; - | ^ interpreted as a constant pattern, not new variable + | ^ + | | + | interpreted as a constant pattern, not a new variable + | help: introduce a variable instead: `c_var` error[E0005]: refutable pattern in local binding: `0u8..=1u8` and `3u8..=std::u8::MAX` not covered --> $DIR/const-pattern-irrefutable.rs:14:9 | +LL | pub const d: u8 = 2; + | -------------------- constant defined here +... LL | let d = 4; - | ^ interpreted as a constant pattern, not new variable + | ^ + | | + | interpreted as a constant pattern, not a new variable + | help: introduce a variable instead: `d_var` error: aborting due to 3 previous errors diff --git a/src/test/ui/suggestions/const-pat-non-exaustive-let-new-var.rs b/src/test/ui/suggestions/const-pat-non-exaustive-let-new-var.rs new file mode 100644 index 00000000000..2a11871db8e --- /dev/null +++ b/src/test/ui/suggestions/const-pat-non-exaustive-let-new-var.rs @@ -0,0 +1,10 @@ +fn main() { + let A = 3; + //~^ ERROR refutable pattern in local binding: `std::i32::MIN..=1i32` and + //~| interpreted as a constant pattern, not a new variable + //~| HELP introduce a variable instead + //~| SUGGESTION a_var + + const A: i32 = 2; + //~^ constant defined here +} diff --git a/src/test/ui/suggestions/const-pat-non-exaustive-let-new-var.stderr b/src/test/ui/suggestions/const-pat-non-exaustive-let-new-var.stderr new file mode 100644 index 00000000000..fc17199bf91 --- /dev/null +++ b/src/test/ui/suggestions/const-pat-non-exaustive-let-new-var.stderr @@ -0,0 +1,15 @@ +error[E0005]: refutable pattern in local binding: `std::i32::MIN..=1i32` and `3i32..=std::i32::MAX` not covered + --> $DIR/const-pat-non-exaustive-let-new-var.rs:2:9 + | +LL | let A = 3; + | ^ + | | + | interpreted as a constant pattern, not a new variable + | help: introduce a variable instead: `a_var` +... +LL | const A: i32 = 2; + | ----------------- constant defined here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0005`.