From 39ffabbb40e91f50fb20243b2fd6198b446663c6 Mon Sep 17 00:00:00 2001 From: Yiming Lei Date: Tue, 23 Aug 2022 19:30:04 -0700 Subject: [PATCH] Point at the string inside literal and mention if we need string interpolation modified: compiler/rustc_passes/src/liveness.rs new file: src/test/ui/type/issue-100584.rs new file: src/test/ui/type/issue-100584.stderr --- compiler/rustc_passes/src/liveness.rs | 82 +++++++++++++++++++++++---- src/test/ui/type/issue-100584.rs | 15 +++++ src/test/ui/type/issue-100584.stderr | 44 ++++++++++++++ 3 files changed, 130 insertions(+), 11 deletions(-) create mode 100644 src/test/ui/type/issue-100584.rs create mode 100644 src/test/ui/type/issue-100584.stderr diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index 7124b84bfef..0c3281fbf06 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -188,6 +188,19 @@ enum VarKind { Upvar(HirId, Symbol), } +struct CollectLitsVisitor<'tcx> { + lit_exprs: Vec<&'tcx hir::Expr<'tcx>>, +} + +impl<'tcx> Visitor<'tcx> for CollectLitsVisitor<'tcx> { + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + if let hir::ExprKind::Lit(_) = expr.kind { + self.lit_exprs.push(expr); + } + intravisit::walk_expr(self, expr); + } +} + struct IrMaps<'tcx> { tcx: TyCtxt<'tcx>, live_node_map: HirIdMap, @@ -1342,7 +1355,7 @@ fn warn_about_unreachable( impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> { fn visit_local(&mut self, local: &'tcx hir::Local<'tcx>) { - self.check_unused_vars_in_pat(&local.pat, None, |spans, hir_id, ln, var| { + self.check_unused_vars_in_pat(&local.pat, None, None, |spans, hir_id, ln, var| { if local.init.is_some() { self.warn_about_dead_assign(spans, hir_id, ln, var); } @@ -1357,7 +1370,7 @@ fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { } fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) { - self.check_unused_vars_in_pat(&arm.pat, None, |_, _, _, _| {}); + self.check_unused_vars_in_pat(&arm.pat, None, None, |_, _, _, _| {}); intravisit::walk_arm(self, arm); } } @@ -1396,7 +1409,7 @@ fn check_expr<'tcx>(this: &mut Liveness<'_, 'tcx>, expr: &'tcx Expr<'tcx>) { } hir::ExprKind::Let(let_expr) => { - this.check_unused_vars_in_pat(let_expr.pat, None, |_, _, _, _| {}); + this.check_unused_vars_in_pat(let_expr.pat, None, None, |_, _, _, _| {}); } // no correctness conditions related to liveness @@ -1517,13 +1530,18 @@ fn warn_about_unused_upvars(&self, entry_ln: LiveNode) { fn warn_about_unused_args(&self, body: &hir::Body<'_>, entry_ln: LiveNode) { for p in body.params { - self.check_unused_vars_in_pat(&p.pat, Some(entry_ln), |spans, hir_id, ln, var| { - if !self.live_on_entry(ln, var) { - self.report_unused_assign(hir_id, spans, var, |name| { - format!("value passed to `{}` is never read", name) - }); - } - }); + self.check_unused_vars_in_pat( + &p.pat, + Some(entry_ln), + Some(body), + |spans, hir_id, ln, var| { + if !self.live_on_entry(ln, var) { + self.report_unused_assign(hir_id, spans, var, |name| { + format!("value passed to `{}` is never read", name) + }); + } + }, + ); } } @@ -1531,6 +1549,7 @@ fn check_unused_vars_in_pat( &self, pat: &hir::Pat<'_>, entry_ln: Option, + opt_body: Option<&hir::Body<'_>>, on_used_on_entry: impl Fn(Vec, HirId, LiveNode, Variable), ) { // In an or-pattern, only consider the variable; any later patterns must have the same @@ -1558,7 +1577,7 @@ fn check_unused_vars_in_pat( hir_ids_and_spans.into_iter().map(|(_, _, ident_span)| ident_span).collect(); on_used_on_entry(spans, id, ln, var); } else { - self.report_unused(hir_ids_and_spans, ln, var, can_remove); + self.report_unused(hir_ids_and_spans, ln, var, can_remove, pat, opt_body); } } } @@ -1570,6 +1589,8 @@ fn report_unused( ln: LiveNode, var: Variable, can_remove: bool, + pat: &hir::Pat<'_>, + opt_body: Option<&hir::Body<'_>>, ) { let first_hir_id = hir_ids_and_spans[0].0; @@ -1673,6 +1694,9 @@ fn report_unused( .collect::>(), |lint| { let mut err = lint.build(&format!("unused variable: `{}`", name)); + if self.has_added_lit_match_name_span(&name, opt_body, &mut err) { + err.span_label(pat.span, "unused variable"); + } err.multipart_suggestion( "if this is intentional, prefix it with an underscore", non_shorthands, @@ -1686,6 +1710,42 @@ fn report_unused( } } + fn has_added_lit_match_name_span( + &self, + name: &str, + opt_body: Option<&hir::Body<'_>>, + err: &mut rustc_errors::DiagnosticBuilder<'_, ()>, + ) -> bool { + let mut has_litstring = false; + let Some(opt_body) = opt_body else {return false;}; + let mut visitor = CollectLitsVisitor { lit_exprs: vec![] }; + intravisit::walk_body(&mut visitor, opt_body); + for lit_expr in visitor.lit_exprs { + let hir::ExprKind::Lit(litx) = &lit_expr.kind else { continue }; + let rustc_ast::LitKind::Str(syb, _) = litx.node else{ continue; }; + let name_str: &str = syb.as_str(); + let mut name_pa = String::from("{"); + name_pa.push_str(&name); + name_pa.push('}'); + if name_str.contains(&name_pa) { + err.span_label( + lit_expr.span, + "you might have meant to use string interpolation in this string literal", + ); + err.multipart_suggestion( + "string interpolation only works in `format!` invocations", + vec![ + (lit_expr.span.shrink_to_lo(), "format!(".to_string()), + (lit_expr.span.shrink_to_hi(), ")".to_string()), + ], + Applicability::MachineApplicable, + ); + has_litstring = true; + } + } + has_litstring + } + fn warn_about_dead_assign(&self, spans: Vec, hir_id: HirId, ln: LiveNode, var: Variable) { if !self.live_on_exit(ln, var) { self.report_unused_assign(hir_id, spans, var, |name| { diff --git a/src/test/ui/type/issue-100584.rs b/src/test/ui/type/issue-100584.rs new file mode 100644 index 00000000000..10284656323 --- /dev/null +++ b/src/test/ui/type/issue-100584.rs @@ -0,0 +1,15 @@ +#![deny(unused)] +fn foo(xyza: &str) { +//~^ ERROR unused variable: `xyza` + let _ = "{xyza}"; +} + +fn foo3(xyza: &str) { +//~^ ERROR unused variable: `xyza` + let _ = "aaa{xyza}bbb"; +} + +fn main() { + foo("x"); + foo3("xx"); +} diff --git a/src/test/ui/type/issue-100584.stderr b/src/test/ui/type/issue-100584.stderr new file mode 100644 index 00000000000..e1db14d1f00 --- /dev/null +++ b/src/test/ui/type/issue-100584.stderr @@ -0,0 +1,44 @@ +error: unused variable: `xyza` + --> $DIR/issue-100584.rs:2:8 + | +LL | fn foo(xyza: &str) { + | ^^^^ unused variable +LL | +LL | let _ = "{xyza}"; + | -------- you might have meant to use string interpolation in this string literal + | +note: the lint level is defined here + --> $DIR/issue-100584.rs:1:9 + | +LL | #![deny(unused)] + | ^^^^^^ + = note: `#[deny(unused_variables)]` implied by `#[deny(unused)]` +help: string interpolation only works in `format!` invocations + | +LL | let _ = format!("{xyza}"); + | ++++++++ + +help: if this is intentional, prefix it with an underscore + | +LL | fn foo(_xyza: &str) { + | ~~~~~ + +error: unused variable: `xyza` + --> $DIR/issue-100584.rs:7:9 + | +LL | fn foo3(xyza: &str) { + | ^^^^ unused variable +LL | +LL | let _ = "aaa{xyza}bbb"; + | -------------- you might have meant to use string interpolation in this string literal + | +help: string interpolation only works in `format!` invocations + | +LL | let _ = format!("aaa{xyza}bbb"); + | ++++++++ + +help: if this is intentional, prefix it with an underscore + | +LL | fn foo3(_xyza: &str) { + | ~~~~~ + +error: aborting due to 2 previous errors +