Skip to content

Commit 1b4c423

Browse files
committed
Auto merge of #11021 - y21:issue9493, r=llogiq
[`format_push_string`]: look through `match` and `if` expressions Closes #9493. changelog: [`format_push_string`]: look through `match` and `if` expressions
2 parents 3cee98d + fe856d3 commit 1b4c423

File tree

3 files changed

+83
-6
lines changed

3 files changed

+83
-6
lines changed

clippy_lints/src/format_push_string.rs

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
22
use clippy_utils::ty::is_type_lang_item;
3-
use clippy_utils::{match_def_path, paths, peel_hir_expr_refs};
4-
use rustc_hir::{BinOpKind, Expr, ExprKind, LangItem};
3+
use clippy_utils::{higher, match_def_path, paths};
4+
use rustc_hir::{BinOpKind, Expr, ExprKind, LangItem, MatchSource};
55
use rustc_lint::{LateContext, LateLintPass};
66
use rustc_session::{declare_lint_pass, declare_tool_lint};
77
use rustc_span::sym;
@@ -44,10 +44,24 @@ fn is_string(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
4444
is_type_lang_item(cx, cx.typeck_results().expr_ty(e).peel_refs(), LangItem::String)
4545
}
4646
fn is_format(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
47-
if let Some(macro_def_id) = e.span.ctxt().outer_expn_data().macro_def_id {
47+
let e = e.peel_blocks().peel_borrows();
48+
49+
if e.span.from_expansion()
50+
&& let Some(macro_def_id) = e.span.ctxt().outer_expn_data().macro_def_id
51+
{
4852
cx.tcx.get_diagnostic_name(macro_def_id) == Some(sym::format_macro)
53+
} else if let Some(higher::If { then, r#else, .. }) = higher::If::hir(e) {
54+
is_format(cx, then) || r#else.is_some_and(|e| is_format(cx, e))
4955
} else {
50-
false
56+
match higher::IfLetOrMatch::parse(cx, e) {
57+
Some(higher::IfLetOrMatch::Match(_, arms, MatchSource::Normal)) => {
58+
arms.iter().any(|arm| is_format(cx, arm.body))
59+
},
60+
Some(higher::IfLetOrMatch::IfLet(_, _, then, r#else)) => {
61+
is_format(cx, then) ||r#else.is_some_and(|e| is_format(cx, e))
62+
},
63+
_ => false,
64+
}
5165
}
5266
}
5367

@@ -68,7 +82,6 @@ impl<'tcx> LateLintPass<'tcx> for FormatPushString {
6882
},
6983
_ => return,
7084
};
71-
let (arg, _) = peel_hir_expr_refs(arg);
7285
if is_format(cx, arg) {
7386
span_lint_and_help(
7487
cx,

tests/ui/format_push_string.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,32 @@ fn main() {
55
string += &format!("{:?}", 1234);
66
string.push_str(&format!("{:?}", 5678));
77
}
8+
9+
mod issue9493 {
10+
pub fn u8vec_to_hex(vector: &Vec<u8>, upper: bool) -> String {
11+
let mut hex = String::with_capacity(vector.len() * 2);
12+
for byte in vector {
13+
hex += &(if upper {
14+
format!("{byte:02X}")
15+
} else {
16+
format!("{byte:02x}")
17+
});
18+
}
19+
hex
20+
}
21+
22+
pub fn other_cases() {
23+
let mut s = String::new();
24+
// if let
25+
s += &(if let Some(_a) = Some(1234) {
26+
format!("{}", 1234)
27+
} else {
28+
format!("{}", 1234)
29+
});
30+
// match
31+
s += &(match Some(1234) {
32+
Some(_) => format!("{}", 1234),
33+
None => format!("{}", 1234),
34+
});
35+
}
36+
}

tests/ui/format_push_string.stderr

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,40 @@ LL | string.push_str(&format!("{:?}", 5678));
1515
|
1616
= help: consider using `write!` to avoid the extra allocation
1717

18-
error: aborting due to 2 previous errors
18+
error: `format!(..)` appended to existing `String`
19+
--> $DIR/format_push_string.rs:13:13
20+
|
21+
LL | / hex += &(if upper {
22+
LL | | format!("{byte:02X}")
23+
LL | | } else {
24+
LL | | format!("{byte:02x}")
25+
LL | | });
26+
| |______________^
27+
|
28+
= help: consider using `write!` to avoid the extra allocation
29+
30+
error: `format!(..)` appended to existing `String`
31+
--> $DIR/format_push_string.rs:25:9
32+
|
33+
LL | / s += &(if let Some(_a) = Some(1234) {
34+
LL | | format!("{}", 1234)
35+
LL | | } else {
36+
LL | | format!("{}", 1234)
37+
LL | | });
38+
| |__________^
39+
|
40+
= help: consider using `write!` to avoid the extra allocation
41+
42+
error: `format!(..)` appended to existing `String`
43+
--> $DIR/format_push_string.rs:31:9
44+
|
45+
LL | / s += &(match Some(1234) {
46+
LL | | Some(_) => format!("{}", 1234),
47+
LL | | None => format!("{}", 1234),
48+
LL | | });
49+
| |__________^
50+
|
51+
= help: consider using `write!` to avoid the extra allocation
52+
53+
error: aborting due to 5 previous errors
1954

0 commit comments

Comments
 (0)