Skip to content

Commit e8de57c

Browse files
borsbors
committed
Auto merge of rust-lang#6212 - ThibsG:MacroTopLevelRefArg, r=flip1995
No lint in macro for `toplevel_ref_arg` Do not lint when the span is from a macro. Question: shouldn't we extend this for external macros also ? Fixes: rust-lang#5849 changelog: none
2 parents 6d5cd6e + bab3386 commit e8de57c

7 files changed

+119
-12
lines changed

‎clippy_lints/src/misc.rs

+11-4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use rustc_hir::{
77
StmtKind, TyKind, UnOp,
88
};
99
use rustc_lint::{LateContext, LateLintPass};
10+
use rustc_middle::lint::in_external_macro;
1011
use rustc_middle::ty::{self, Ty};
1112
use rustc_session::{declare_lint_pass, declare_tool_lint};
1213
use rustc_span::hygiene::DesugaringKind;
@@ -271,13 +272,16 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints {
271272
k: FnKind<'tcx>,
272273
decl: &'tcx FnDecl<'_>,
273274
body: &'tcx Body<'_>,
274-
_: Span,
275+
span: Span,
275276
_: HirId,
276277
) {
277278
if let FnKind::Closure(_) = k {
278279
// Does not apply to closures
279280
return;
280281
}
282+
if in_external_macro(cx.tcx.sess, span) {
283+
return;
284+
}
281285
for arg in iter_input_pats(decl, body) {
282286
if let PatKind::Binding(BindingAnnotation::Ref | BindingAnnotation::RefMut, ..) = arg.pat.kind {
283287
span_lint(
@@ -293,13 +297,16 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints {
293297

294298
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
295299
if_chain! {
300+
if !in_external_macro(cx.tcx.sess, stmt.span);
296301
if let StmtKind::Local(ref local) = stmt.kind;
297302
if let PatKind::Binding(an, .., name, None) = local.pat.kind;
298303
if let Some(ref init) = local.init;
299304
if !higher::is_from_for_desugar(local);
300305
then {
301306
if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut {
302-
let sugg_init = if init.span.from_expansion() {
307+
// use the macro callsite when the init span (but not the whole local span)
308+
// comes from an expansion like `vec![1, 2, 3]` in `let ref _ = vec![1, 2, 3];`
309+
let sugg_init = if init.span.from_expansion() && !local.span.from_expansion() {
303310
Sugg::hir_with_macro_callsite(cx, init, "..")
304311
} else {
305312
Sugg::hir(cx, init, "..")
@@ -310,7 +317,7 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints {
310317
("", sugg_init.addr())
311318
};
312319
let tyopt = if let Some(ref ty) = local.ty {
313-
format!(": &{mutopt}{ty}", mutopt=mutopt, ty=snippet(cx, ty.span, "_"))
320+
format!(": &{mutopt}{ty}", mutopt=mutopt, ty=snippet(cx, ty.span, ".."))
314321
} else {
315322
String::new()
316323
};
@@ -326,7 +333,7 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints {
326333
"try",
327334
format!(
328335
"let {name}{tyopt} = {initref};",
329-
name=snippet(cx, name.span, "_"),
336+
name=snippet(cx, name.span, ".."),
330337
tyopt=tyopt,
331338
initref=initref,
332339
),

‎tests/ui/auxiliary/macro_rules.rs

+14
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,17 @@ macro_rules! option_env_unwrap_external {
5656
option_env!($env).expect($message)
5757
};
5858
}
59+
60+
#[macro_export]
61+
macro_rules! ref_arg_binding {
62+
() => {
63+
let ref _y = 42;
64+
};
65+
}
66+
67+
#[macro_export]
68+
macro_rules! ref_arg_function {
69+
() => {
70+
fn fun_example(ref _x: usize) {}
71+
};
72+
}

‎tests/ui/toplevel_ref_arg.fixed

+21
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
11
// run-rustfix
2+
// aux-build:macro_rules.rs
23

34
#![warn(clippy::toplevel_ref_arg)]
45

6+
#[macro_use]
7+
extern crate macro_rules;
8+
9+
macro_rules! gen_binding {
10+
() => {
11+
let _y = &42;
12+
};
13+
}
14+
515
fn main() {
616
// Closures should not warn
717
let y = |ref x| println!("{:?}", x);
@@ -26,4 +36,15 @@ fn main() {
2636

2737
// ok
2838
for ref _x in 0..10 {}
39+
40+
// lint in macro
41+
#[allow(unused)]
42+
{
43+
gen_binding!();
44+
}
45+
46+
// do not lint in external macro
47+
{
48+
ref_arg_binding!();
49+
}
2950
}

‎tests/ui/toplevel_ref_arg.rs

+21
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
11
// run-rustfix
2+
// aux-build:macro_rules.rs
23

34
#![warn(clippy::toplevel_ref_arg)]
45

6+
#[macro_use]
7+
extern crate macro_rules;
8+
9+
macro_rules! gen_binding {
10+
() => {
11+
let ref _y = 42;
12+
};
13+
}
14+
515
fn main() {
616
// Closures should not warn
717
let y = |ref x| println!("{:?}", x);
@@ -26,4 +36,15 @@ fn main() {
2636

2737
// ok
2838
for ref _x in 0..10 {}
39+
40+
// lint in macro
41+
#[allow(unused)]
42+
{
43+
gen_binding!();
44+
}
45+
46+
// do not lint in external macro
47+
{
48+
ref_arg_binding!();
49+
}
2950
}

‎tests/ui/toplevel_ref_arg.stderr

+17-6
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,45 @@
11
error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead
2-
--> $DIR/toplevel_ref_arg.rs:10:9
2+
--> $DIR/toplevel_ref_arg.rs:20:9
33
|
44
LL | let ref _x = 1;
55
| ----^^^^^^----- help: try: `let _x = &1;`
66
|
77
= note: `-D clippy::toplevel-ref-arg` implied by `-D warnings`
88

99
error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead
10-
--> $DIR/toplevel_ref_arg.rs:12:9
10+
--> $DIR/toplevel_ref_arg.rs:22:9
1111
|
1212
LL | let ref _y: (&_, u8) = (&1, 2);
1313
| ----^^^^^^--------------------- help: try: `let _y: &(&_, u8) = &(&1, 2);`
1414

1515
error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead
16-
--> $DIR/toplevel_ref_arg.rs:14:9
16+
--> $DIR/toplevel_ref_arg.rs:24:9
1717
|
1818
LL | let ref _z = 1 + 2;
1919
| ----^^^^^^--------- help: try: `let _z = &(1 + 2);`
2020

2121
error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead
22-
--> $DIR/toplevel_ref_arg.rs:16:9
22+
--> $DIR/toplevel_ref_arg.rs:26:9
2323
|
2424
LL | let ref mut _z = 1 + 2;
2525
| ----^^^^^^^^^^--------- help: try: `let _z = &mut (1 + 2);`
2626

2727
error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead
28-
--> $DIR/toplevel_ref_arg.rs:21:9
28+
--> $DIR/toplevel_ref_arg.rs:31:9
2929
|
3030
LL | let ref _x = vec![1, 2, 3];
3131
| ----^^^^^^----------------- help: try: `let _x = &vec![1, 2, 3];`
3232

33-
error: aborting due to 5 previous errors
33+
error: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead
34+
--> $DIR/toplevel_ref_arg.rs:11:13
35+
|
36+
LL | let ref _y = 42;
37+
| ----^^^^^^------ help: try: `let _y = &42;`
38+
...
39+
LL | gen_binding!();
40+
| --------------- in this macro invocation
41+
|
42+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
43+
44+
error: aborting due to 6 previous errors
3445

‎tests/ui/toplevel_ref_arg_non_rustfix.rs

+22
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,33 @@
1+
// aux-build:macro_rules.rs
2+
13
#![warn(clippy::toplevel_ref_arg)]
24
#![allow(unused)]
35

6+
#[macro_use]
7+
extern crate macro_rules;
8+
49
fn the_answer(ref mut x: u8) {
510
*x = 42;
611
}
712

13+
macro_rules! gen_function {
14+
() => {
15+
fn fun_example(ref _x: usize) {}
16+
};
17+
}
18+
819
fn main() {
920
let mut x = 0;
1021
the_answer(x);
22+
23+
// lint in macro
24+
#[allow(unused)]
25+
{
26+
gen_function!();
27+
}
28+
29+
// do not lint in external macro
30+
{
31+
ref_arg_function!();
32+
}
1133
}

‎tests/ui/toplevel_ref_arg_non_rustfix.stderr

+13-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,21 @@
11
error: `ref` directly on a function argument is ignored. Consider using a reference type instead.
2-
--> $DIR/toplevel_ref_arg_non_rustfix.rs:4:15
2+
--> $DIR/toplevel_ref_arg_non_rustfix.rs:9:15
33
|
44
LL | fn the_answer(ref mut x: u8) {
55
| ^^^^^^^^^
66
|
77
= note: `-D clippy::toplevel-ref-arg` implied by `-D warnings`
88

9-
error: aborting due to previous error
9+
error: `ref` directly on a function argument is ignored. Consider using a reference type instead.
10+
--> $DIR/toplevel_ref_arg_non_rustfix.rs:15:24
11+
|
12+
LL | fn fun_example(ref _x: usize) {}
13+
| ^^^^^^
14+
...
15+
LL | gen_function!();
16+
| ---------------- in this macro invocation
17+
|
18+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
19+
20+
error: aborting due to 2 previous errors
1021

0 commit comments

Comments
 (0)