From 3b9e5dfda565224e6f007f379ca1cea4623422d9 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Wed, 25 Sep 2019 11:06:52 +0700 Subject: [PATCH 1/3] Add regression test for macro expansion --- tests/ui/toplevel_ref_arg.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ui/toplevel_ref_arg.rs b/tests/ui/toplevel_ref_arg.rs index 7d5eaa940e1..e11c40966e4 100644 --- a/tests/ui/toplevel_ref_arg.rs +++ b/tests/ui/toplevel_ref_arg.rs @@ -19,6 +19,8 @@ fn main() { let (ref x, _) = (1, 2); // ok, not top level println!("The answer is {}.", x); + let ref x = vec![1, 2, 3]; + // Make sure that allowing the lint works #[allow(clippy::toplevel_ref_arg)] let ref mut x = 1_234_543; From 08ce6bc6d9532191e59a49786811a0735ea35f5a Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Thu, 26 Sep 2019 08:46:51 +0700 Subject: [PATCH 2/3] Fix macro expansion in toplevel_ref_arg lint --- clippy_lints/src/misc.rs | 35 ++++++++++++++++++-------------- tests/ui/toplevel_ref_arg.fixed | 2 ++ tests/ui/toplevel_ref_arg.stderr | 8 +++++++- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 38fdabaaf7a..980c4caf2c5 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -261,40 +261,45 @@ fn check_fn( } } - fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, s: &'tcx Stmt) { + fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt) { if_chain! { - if let StmtKind::Local(ref l) = s.node; - if let PatKind::Binding(an, .., i, None) = l.pat.node; - if let Some(ref init) = l.init; + if let StmtKind::Local(ref local) = stmt.node; + if let PatKind::Binding(an, .., name, None) = local.pat.node; + if let Some(ref init) = local.init; then { if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut { - let sugg_init = Sugg::hir(cx, init, ".."); - let (mutopt,initref) = if an == BindingAnnotation::RefMut { + let sugg_init = if init.span.from_expansion() { + Sugg::hir_with_macro_callsite(cx, init, "..") + } else { + Sugg::hir(cx, init, "..") + }; + let (mutopt, initref) = if an == BindingAnnotation::RefMut { ("mut ", sugg_init.mut_addr()) } else { ("", sugg_init.addr()) }; - let tyopt = if let Some(ref ty) = l.ty { + let tyopt = if let Some(ref ty) = local.ty { format!(": &{mutopt}{ty}", mutopt=mutopt, ty=snippet(cx, ty.span, "_")) } else { String::new() }; - span_lint_hir_and_then(cx, + span_lint_hir_and_then( + cx, TOPLEVEL_REF_ARG, init.hir_id, - l.pat.span, + local.pat.span, "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead", |db| { db.span_suggestion( - s.span, + stmt.span, "try", format!( "let {name}{tyopt} = {initref};", - name=snippet(cx, i.span, "_"), + name=snippet(cx, name.span, "_"), tyopt=tyopt, initref=initref, ), - Applicability::MachineApplicable, // snippet + Applicability::MachineApplicable, ); } ); @@ -302,19 +307,19 @@ fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, s: &'tcx Stmt) { } }; if_chain! { - if let StmtKind::Semi(ref expr) = s.node; + if let StmtKind::Semi(ref expr) = stmt.node; if let ExprKind::Binary(ref binop, ref a, ref b) = expr.node; if binop.node == BinOpKind::And || binop.node == BinOpKind::Or; if let Some(sugg) = Sugg::hir_opt(cx, a); then { span_lint_and_then(cx, SHORT_CIRCUIT_STATEMENT, - s.span, + stmt.span, "boolean short circuit operator in statement may be clearer using an explicit test", |db| { let sugg = if binop.node == BinOpKind::Or { !sugg } else { sugg }; db.span_suggestion( - s.span, + stmt.span, "replace it with", format!( "if {} {{ {}; }}", diff --git a/tests/ui/toplevel_ref_arg.fixed b/tests/ui/toplevel_ref_arg.fixed index 8a4bf530173..df3c15ed59e 100644 --- a/tests/ui/toplevel_ref_arg.fixed +++ b/tests/ui/toplevel_ref_arg.fixed @@ -19,6 +19,8 @@ fn main() { let (ref x, _) = (1, 2); // ok, not top level println!("The answer is {}.", x); + let x = &vec![1, 2, 3]; + // Make sure that allowing the lint works #[allow(clippy::toplevel_ref_arg)] let ref mut x = 1_234_543; diff --git a/tests/ui/toplevel_ref_arg.stderr b/tests/ui/toplevel_ref_arg.stderr index de80a9614de..882e9e558b2 100644 --- a/tests/ui/toplevel_ref_arg.stderr +++ b/tests/ui/toplevel_ref_arg.stderr @@ -24,5 +24,11 @@ error: `ref` on an entire `let` pattern is discouraged, take a reference with `& LL | let ref mut z = 1 + 2; | ----^^^^^^^^^--------- help: try: `let z = &mut (1 + 2);` -error: aborting due to 4 previous errors +error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead + --> $DIR/toplevel_ref_arg.rs:22:9 + | +LL | let ref x = vec![1, 2, 3]; + | ----^^^^^----------------- help: try: `let x = &vec![1, 2, 3];` + +error: aborting due to 5 previous errors From 5639639d3530529d88b9e2ec5c9468f4fb7acec1 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Thu, 26 Sep 2019 01:53:39 +0000 Subject: [PATCH 3/3] Remove unused attribute in test --- tests/ui/toplevel_ref_arg.fixed | 13 ++++++------- tests/ui/toplevel_ref_arg.rs | 13 ++++++------- tests/ui/toplevel_ref_arg.stderr | 30 +++++++++++++++--------------- 3 files changed, 27 insertions(+), 29 deletions(-) diff --git a/tests/ui/toplevel_ref_arg.fixed b/tests/ui/toplevel_ref_arg.fixed index df3c15ed59e..9438abbe330 100644 --- a/tests/ui/toplevel_ref_arg.fixed +++ b/tests/ui/toplevel_ref_arg.fixed @@ -1,27 +1,26 @@ // run-rustfix #![warn(clippy::toplevel_ref_arg)] -#![allow(unused)] fn main() { // Closures should not warn let y = |ref x| println!("{:?}", x); y(1u8); - let x = &1; + let _x = &1; - let y: &(&_, u8) = &(&1, 2); + let _y: &(&_, u8) = &(&1, 2); - let z = &(1 + 2); + let _z = &(1 + 2); - let z = &mut (1 + 2); + let _z = &mut (1 + 2); let (ref x, _) = (1, 2); // ok, not top level println!("The answer is {}.", x); - let x = &vec![1, 2, 3]; + let _x = &vec![1, 2, 3]; // Make sure that allowing the lint works #[allow(clippy::toplevel_ref_arg)] - let ref mut x = 1_234_543; + let ref mut _x = 1_234_543; } diff --git a/tests/ui/toplevel_ref_arg.rs b/tests/ui/toplevel_ref_arg.rs index e11c40966e4..ee630f12a60 100644 --- a/tests/ui/toplevel_ref_arg.rs +++ b/tests/ui/toplevel_ref_arg.rs @@ -1,27 +1,26 @@ // run-rustfix #![warn(clippy::toplevel_ref_arg)] -#![allow(unused)] fn main() { // Closures should not warn let y = |ref x| println!("{:?}", x); y(1u8); - let ref x = 1; + let ref _x = 1; - let ref y: (&_, u8) = (&1, 2); + let ref _y: (&_, u8) = (&1, 2); - let ref z = 1 + 2; + let ref _z = 1 + 2; - let ref mut z = 1 + 2; + let ref mut _z = 1 + 2; let (ref x, _) = (1, 2); // ok, not top level println!("The answer is {}.", x); - let ref x = vec![1, 2, 3]; + let ref _x = vec![1, 2, 3]; // Make sure that allowing the lint works #[allow(clippy::toplevel_ref_arg)] - let ref mut x = 1_234_543; + let ref mut _x = 1_234_543; } diff --git a/tests/ui/toplevel_ref_arg.stderr b/tests/ui/toplevel_ref_arg.stderr index 882e9e558b2..19d69496709 100644 --- a/tests/ui/toplevel_ref_arg.stderr +++ b/tests/ui/toplevel_ref_arg.stderr @@ -1,34 +1,34 @@ error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead - --> $DIR/toplevel_ref_arg.rs:11:9 + --> $DIR/toplevel_ref_arg.rs:10:9 | -LL | let ref x = 1; - | ----^^^^^----- help: try: `let x = &1;` +LL | let ref _x = 1; + | ----^^^^^^----- help: try: `let _x = &1;` | = note: `-D clippy::toplevel-ref-arg` implied by `-D warnings` error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead - --> $DIR/toplevel_ref_arg.rs:13:9 + --> $DIR/toplevel_ref_arg.rs:12:9 | -LL | let ref y: (&_, u8) = (&1, 2); - | ----^^^^^--------------------- help: try: `let y: &(&_, u8) = &(&1, 2);` +LL | let ref _y: (&_, u8) = (&1, 2); + | ----^^^^^^--------------------- help: try: `let _y: &(&_, u8) = &(&1, 2);` error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead - --> $DIR/toplevel_ref_arg.rs:15:9 + --> $DIR/toplevel_ref_arg.rs:14:9 | -LL | let ref z = 1 + 2; - | ----^^^^^--------- help: try: `let z = &(1 + 2);` +LL | let ref _z = 1 + 2; + | ----^^^^^^--------- help: try: `let _z = &(1 + 2);` error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead - --> $DIR/toplevel_ref_arg.rs:17:9 + --> $DIR/toplevel_ref_arg.rs:16:9 | -LL | let ref mut z = 1 + 2; - | ----^^^^^^^^^--------- help: try: `let z = &mut (1 + 2);` +LL | let ref mut _z = 1 + 2; + | ----^^^^^^^^^^--------- help: try: `let _z = &mut (1 + 2);` error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead - --> $DIR/toplevel_ref_arg.rs:22:9 + --> $DIR/toplevel_ref_arg.rs:21:9 | -LL | let ref x = vec![1, 2, 3]; - | ----^^^^^----------------- help: try: `let x = &vec![1, 2, 3];` +LL | let ref _x = vec![1, 2, 3]; + | ----^^^^^^----------------- help: try: `let _x = &vec![1, 2, 3];` error: aborting due to 5 previous errors