Skip to content

Commit 49ca220

Browse files
llogiqllogiq
authored
Fix multiple problems in #15063 (#15070)
Closes #15063 ---- changelog: [`op_ref`]: fix wrongly showed macro definition in suggestions changelog: [`needless_bool_assign`]: fix missing curlies when on else if
2 parents 012e5f5 + f29aa69 commit 49ca220

File tree

8 files changed

+144
-18
lines changed

8 files changed

+144
-18
lines changed

‎clippy_lints/src/needless_bool.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,11 +199,16 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBool {
199199
let mut applicability = Applicability::MachineApplicable;
200200
let cond = Sugg::hir_with_applicability(cx, cond, "..", &mut applicability);
201201
let lhs = snippet_with_applicability(cx, lhs_a.span, "..", &mut applicability);
202-
let sugg = if a == b {
202+
let mut sugg = if a == b {
203203
format!("{cond}; {lhs} = {a:?};")
204204
} else {
205205
format!("{lhs} = {};", if a { cond } else { !cond })
206206
};
207+
208+
if is_else_clause(cx.tcx, e) {
209+
sugg = format!("{{ {sugg} }}");
210+
}
211+
207212
span_lint_and_sugg(
208213
cx,
209214
NEEDLESS_BOOL_ASSIGN,

‎clippy_lints/src/operators/op_ref.rs‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::get_enclosing_block;
3-
use clippy_utils::source::snippet;
3+
use clippy_utils::source::snippet_with_context;
44
use clippy_utils::ty::{implements_trait, is_copy};
55
use rustc_errors::Applicability;
66
use rustc_hir::def::Res;
@@ -61,12 +61,13 @@ pub(crate) fn check<'tcx>(
6161
e.span,
6262
"needlessly taken reference of both operands",
6363
|diag| {
64-
let lsnip = snippet(cx, l.span, "...").to_string();
65-
let rsnip = snippet(cx, r.span, "...").to_string();
64+
let mut applicability = Applicability::MachineApplicable;
65+
let (lsnip, _) = snippet_with_context(cx, l.span, e.span.ctxt(), "...", &mut applicability);
66+
let (rsnip, _) = snippet_with_context(cx, r.span, e.span.ctxt(), "...", &mut applicability);
6667
diag.multipart_suggestion(
6768
"use the values directly",
68-
vec![(left.span, lsnip), (right.span, rsnip)],
69-
Applicability::MachineApplicable,
69+
vec![(left.span, lsnip.to_string()), (right.span, rsnip.to_string())],
70+
applicability,
7071
);
7172
},
7273
);
@@ -80,13 +81,9 @@ pub(crate) fn check<'tcx>(
8081
e.span,
8182
"needlessly taken reference of left operand",
8283
|diag| {
83-
let lsnip = snippet(cx, l.span, "...").to_string();
84-
diag.span_suggestion(
85-
left.span,
86-
"use the left value directly",
87-
lsnip,
88-
Applicability::MachineApplicable,
89-
);
84+
let mut applicability = Applicability::MachineApplicable;
85+
let (lsnip, _) = snippet_with_context(cx, l.span, e.span.ctxt(), "...", &mut applicability);
86+
diag.span_suggestion(left.span, "use the left value directly", lsnip, applicability);
9087
},
9188
);
9289
} else if !lcpy
@@ -99,7 +96,8 @@ pub(crate) fn check<'tcx>(
9996
e.span,
10097
"needlessly taken reference of right operand",
10198
|diag| {
102-
let rsnip = snippet(cx, r.span, "...").to_string();
99+
let mut applicability = Applicability::MachineApplicable;
100+
let (rsnip, _) = snippet_with_context(cx, r.span, e.span.ctxt(), "...", &mut applicability);
103101
diag.span_suggestion(
104102
right.span,
105103
"use the right value directly",
@@ -131,7 +129,8 @@ pub(crate) fn check<'tcx>(
131129
e.span,
132130
"needlessly taken reference of left operand",
133131
|diag| {
134-
let lsnip = snippet(cx, l.span, "...").to_string();
132+
let mut applicability = Applicability::MachineApplicable;
133+
let (lsnip, _) = snippet_with_context(cx, l.span, e.span.ctxt(), "...", &mut applicability);
135134
diag.span_suggestion(
136135
left.span,
137136
"use the left value directly",
@@ -158,7 +157,8 @@ pub(crate) fn check<'tcx>(
158157
&& implements_trait(cx, cx.typeck_results().expr_ty(left), trait_id, &[rty.into()])
159158
{
160159
span_lint_and_then(cx, OP_REF, e.span, "taken reference of right operand", |diag| {
161-
let rsnip = snippet(cx, r.span, "...").to_string();
160+
let mut applicability = Applicability::MachineApplicable;
161+
let (rsnip, _) = snippet_with_context(cx, r.span, e.span.ctxt(), "...", &mut applicability);
162162
diag.span_suggestion(
163163
right.span,
164164
"use the right value directly",

‎tests/ui/needless_bool_assign.fixed‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,12 @@ fn main() {
3333
b = true;
3434
}
3535
}
36+
37+
fn issue15063(x: bool, y: bool) {
38+
let mut z = false;
39+
40+
if x && y {
41+
todo!()
42+
} else { z = x || y; }
43+
//~^^^^^ needless_bool_assign
44+
}

‎tests/ui/needless_bool_assign.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,16 @@ fn main() {
4545
b = true;
4646
}
4747
}
48+
49+
fn issue15063(x: bool, y: bool) {
50+
let mut z = false;
51+
52+
if x && y {
53+
todo!()
54+
} else if x || y {
55+
z = true;
56+
} else {
57+
z = false;
58+
}
59+
//~^^^^^ needless_bool_assign
60+
}

‎tests/ui/needless_bool_assign.stderr‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,16 @@ LL | | }
5151
= note: `-D clippy::if-same-then-else` implied by `-D warnings`
5252
= help: to override `-D warnings` add `#[allow(clippy::if_same_then_else)]`
5353

54-
error: aborting due to 4 previous errors
54+
error: this if-then-else expression assigns a bool literal
55+
--> tests/ui/needless_bool_assign.rs:54:12
56+
|
57+
LL | } else if x || y {
58+
| ____________^
59+
LL | | z = true;
60+
LL | | } else {
61+
LL | | z = false;
62+
LL | | }
63+
| |_____^ help: you can reduce it to: `{ z = x || y; }`
64+
65+
error: aborting due to 5 previous errors
5566

‎tests/ui/op_ref.fixed‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,37 @@ mod issue_2597 {
110110
&array[idx] < val
111111
}
112112
}
113+
114+
#[allow(clippy::needless_if)]
115+
fn issue15063() {
116+
use std::ops::BitAnd;
117+
118+
macro_rules! mac {
119+
($e:expr) => {
120+
$e.clone()
121+
};
122+
}
123+
124+
let x = 1;
125+
if x == mac!(1) {}
126+
//~^ op_ref
127+
128+
#[derive(Copy, Clone)]
129+
struct Y(i32);
130+
impl BitAnd for Y {
131+
type Output = Y;
132+
fn bitand(self, rhs: Y) -> Y {
133+
Y(self.0 & rhs.0)
134+
}
135+
}
136+
impl<'a> BitAnd<&'a Y> for Y {
137+
type Output = Y;
138+
fn bitand(self, rhs: &'a Y) -> Y {
139+
Y(self.0 & rhs.0)
140+
}
141+
}
142+
let x = Y(1);
143+
let y = Y(2);
144+
let z = x & mac!(y);
145+
//~^ op_ref
146+
}

‎tests/ui/op_ref.rs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,37 @@ mod issue_2597 {
110110
&array[idx] < val
111111
}
112112
}
113+
114+
#[allow(clippy::needless_if)]
115+
fn issue15063() {
116+
use std::ops::BitAnd;
117+
118+
macro_rules! mac {
119+
($e:expr) => {
120+
$e.clone()
121+
};
122+
}
123+
124+
let x = 1;
125+
if &x == &mac!(1) {}
126+
//~^ op_ref
127+
128+
#[derive(Copy, Clone)]
129+
struct Y(i32);
130+
impl BitAnd for Y {
131+
type Output = Y;
132+
fn bitand(self, rhs: Y) -> Y {
133+
Y(self.0 & rhs.0)
134+
}
135+
}
136+
impl<'a> BitAnd<&'a Y> for Y {
137+
type Output = Y;
138+
fn bitand(self, rhs: &'a Y) -> Y {
139+
Y(self.0 & rhs.0)
140+
}
141+
}
142+
let x = Y(1);
143+
let y = Y(2);
144+
let z = x & &mac!(y);
145+
//~^ op_ref
146+
}

‎tests/ui/op_ref.stderr‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,25 @@ LL | let _ = two + &three;
3636
| |
3737
| help: use the right value directly: `three`
3838

39-
error: aborting due to 4 previous errors
39+
error: needlessly taken reference of both operands
40+
--> tests/ui/op_ref.rs:125:8
41+
|
42+
LL | if &x == &mac!(1) {}
43+
| ^^^^^^^^^^^^^^
44+
|
45+
help: use the values directly
46+
|
47+
LL - if &x == &mac!(1) {}
48+
LL + if x == mac!(1) {}
49+
|
50+
51+
error: taken reference of right operand
52+
--> tests/ui/op_ref.rs:144:13
53+
|
54+
LL | let z = x & &mac!(y);
55+
| ^^^^--------
56+
| |
57+
| help: use the right value directly: `mac!(y)`
58+
59+
error: aborting due to 6 previous errors
4060

0 commit comments

Comments
 (0)