Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix macro expansion in toplevel_ref_arg lint #4570

Merged
merged 3 commits into from
Sep 26, 2019
Merged

Fix macro expansion in toplevel_ref_arg lint #4570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3b9e5df
Add regression test for macro expansion
tesuji Sep 25, 2019
08ce6bc
Fix macro expansion in toplevel_ref_arg lint
tesuji Sep 26, 2019
5639639
Remove unused attribute in test
tesuji Sep 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 20 additions & 15 deletions clippy_lints/src/misc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,60 +261,65 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints {
}
}

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,
);
}
);
}
}
};
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 {} {{ {}; }}",
Expand Down
13 changes: 7 additions & 6 deletions tests/ui/toplevel_ref_arg.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +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];

// 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;
}
13 changes: 7 additions & 6 deletions tests/ui/toplevel_ref_arg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +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];

// 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;
}
32 changes: 19 additions & 13 deletions tests/ui/toplevel_ref_arg.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +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: 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:21:9
|
LL | let ref _x = vec![1, 2, 3];
| ----^^^^^^----------------- help: try: `let _x = &vec![1, 2, 3];`

error: aborting due to 5 previous errors