Skip to content

Commit 290b7d3

Browse files
pitajpitaj
committed
New lint format_add_strings 
1 parent 85b88be commit 290b7d3

13 files changed

+141
-30
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3309,6 +3309,7 @@ Released 2018-09-13
33093309
[`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy
33103310
[`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref
33113311
[`format_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#format_in_format_args
3312+
[`format_push_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#format_push_string
33123313
[`from_iter_instead_of_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect
33133314
[`from_over_into`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into
33143315
[`from_str_radix_10`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_str_radix_10

‎clippy_lints/src/format_push_string.rs

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
use clippy_utils::diagnostics::span_lint_and_help;
2+
use clippy_utils::ty::is_type_diagnostic_item;
3+
use clippy_utils::{match_def_path, paths, peel_hir_expr_refs};
4+
use rustc_hir::{BinOpKind, Expr, ExprKind};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_session::{declare_lint_pass, declare_tool_lint};
7+
use rustc_span::sym;
8+
9+
declare_clippy_lint! {
10+
/// ### What it does
11+
/// Detects cases where the result of a `format!` call is
12+
/// appended to an existing `String`.
13+
///
14+
/// ### Why is this bad?
15+
/// Introduces an extra, avoidable heap allocation.
16+
///
17+
/// ### Example
18+
/// ```rust
19+
/// let mut s = String::new();
20+
/// s += &format!("0x{:X}", 1024);
21+
/// s.push_str(&format!("0x{:X}", 1024));
22+
/// ```
23+
/// Use instead:
24+
/// ```rust
25+
/// use std::fmt::Write;
26+
///
27+
/// let mut s = String::new();
28+
/// let _ = write!(s, "0x{:X}", 1024);
29+
/// ```
30+
#[clippy::version = "1.61.0"]
31+
pub FORMAT_PUSH_STRING,
32+
perf,
33+
"`format!(..)` appended to existing `String`"
34+
}
35+
declare_lint_pass!(FormatPushString => [FORMAT_PUSH_STRING]);
36+
37+
fn is_string(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
38+
is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(e).peel_refs(), sym::String)
39+
}
40+
fn is_format(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
41+
if let Some(macro_def_id) = e.span.ctxt().outer_expn_data().macro_def_id {
42+
cx.tcx.get_diagnostic_name(macro_def_id) == Some(sym::format_macro)
43+
} else {
44+
false
45+
}
46+
}
47+
48+
impl<'tcx> LateLintPass<'tcx> for FormatPushString {
49+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
50+
let arg = match expr.kind {
51+
ExprKind::MethodCall(_, [_, arg], _) => {
52+
if let Some(fn_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) &&
53+
match_def_path(cx, fn_def_id, &paths::PUSH_STR) {
54+
arg
55+
} else {
56+
return;
57+
}
58+
}
59+
ExprKind::AssignOp(op, left, arg)
60+
if op.node == BinOpKind::Add && is_string(cx, left) => {
61+
arg
62+
},
63+
_ => return,
64+
};
65+
let (arg, _) = peel_hir_expr_refs(arg);
66+
if is_format(cx, arg) {
67+
span_lint_and_help(
68+
cx,
69+
FORMAT_PUSH_STRING,
70+
expr.span,
71+
"`format!(..)` appended to existing `String`",
72+
None,
73+
"consider using `write!` to avoid the extra allocation",
74+
);
75+
}
76+
}
77+
}

‎clippy_lints/src/inconsistent_struct_constructor.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use rustc_hir::{self as hir, ExprKind};
77
use rustc_lint::{LateContext, LateLintPass};
88
use rustc_session::{declare_lint_pass, declare_tool_lint};
99
use rustc_span::symbol::Symbol;
10+
use std::fmt::Write;
1011

1112
declare_clippy_lint! {
1213
/// ### What it does
@@ -89,7 +90,7 @@ impl<'tcx> LateLintPass<'tcx> for InconsistentStructConstructor {
8990
let mut fields_snippet = String::new();
9091
let (last_ident, idents) = ordered_fields.split_last().unwrap();
9192
for ident in idents {
92-
fields_snippet.push_str(&format!("{}, ", ident));
93+
let _ = write!(fields_snippet, "{}, ", ident);
9394
}
9495
fields_snippet.push_str(&last_ident.to_string());
9596

‎clippy_lints/src/lib.register_all.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
7373
LintId::of(format_args::TO_STRING_IN_FORMAT_ARGS),
7474
LintId::of(format_impl::PRINT_IN_FORMAT_IMPL),
7575
LintId::of(format_impl::RECURSIVE_FORMAT_IMPL),
76+
LintId::of(format_push_string::FORMAT_PUSH_STRING),
7677
LintId::of(formatting::POSSIBLE_MISSING_COMMA),
7778
LintId::of(formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
7879
LintId::of(formatting::SUSPICIOUS_ELSE_FORMATTING),

‎clippy_lints/src/lib.register_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ store.register_lints(&[
160160
format_args::TO_STRING_IN_FORMAT_ARGS,
161161
format_impl::PRINT_IN_FORMAT_IMPL,
162162
format_impl::RECURSIVE_FORMAT_IMPL,
163+
format_push_string::FORMAT_PUSH_STRING,
163164
formatting::POSSIBLE_MISSING_COMMA,
164165
formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
165166
formatting::SUSPICIOUS_ELSE_FORMATTING,

‎clippy_lints/src/lib.register_perf.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![
77
LintId::of(escape::BOXED_LOCAL),
88
LintId::of(format_args::FORMAT_IN_FORMAT_ARGS),
99
LintId::of(format_args::TO_STRING_IN_FORMAT_ARGS),
10+
LintId::of(format_push_string::FORMAT_PUSH_STRING),
1011
LintId::of(large_const_arrays::LARGE_CONST_ARRAYS),
1112
LintId::of(large_enum_variant::LARGE_ENUM_VARIANT),
1213
LintId::of(loops::MANUAL_MEMCPY),

‎clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ mod floating_point_arithmetic;
229229
mod format;
230230
mod format_args;
231231
mod format_impl;
232+
mod format_push_string;
232233
mod formatting;
233234
mod from_over_into;
234235
mod from_str_radix_10;
@@ -869,6 +870,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
869870
})
870871
});
871872
store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef));
873+
store.register_late_pass(|| Box::new(format_push_string::FormatPushString));
872874
// add lints here, do not remove this comment, it's used in `new_lint`
873875
}
874876

‎clippy_lints/src/trait_bounds.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use rustc_hir::{
1414
use rustc_lint::{LateContext, LateLintPass};
1515
use rustc_session::{declare_tool_lint, impl_lint_pass};
1616
use rustc_span::Span;
17+
use std::fmt::Write;
1718

1819
declare_clippy_lint! {
1920
/// ### What it does
@@ -184,19 +185,19 @@ impl TraitBounds {
184185
for b in v.iter() {
185186
if let GenericBound::Trait(ref poly_trait_ref, _) = b {
186187
let path = &poly_trait_ref.trait_ref.path;
187-
hint_string.push_str(&format!(
188+
let _ = write!(hint_string,
188189
" {} +",
189190
snippet_with_applicability(cx, path.span, "..", &mut applicability)
190-
));
191+
);
191192
}
192193
}
193194
for b in p.bounds.iter() {
194195
if let GenericBound::Trait(ref poly_trait_ref, _) = b {
195196
let path = &poly_trait_ref.trait_ref.path;
196-
hint_string.push_str(&format!(
197+
let _ = write!(hint_string,
197198
" {} +",
198199
snippet_with_applicability(cx, path.span, "..", &mut applicability)
199-
));
200+
);
200201
}
201202
}
202203
hint_string.truncate(hint_string.len() - 2);

‎clippy_utils/src/sugg.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use rustc_span::source_map::{BytePos, CharPos, Pos, Span, SyntaxContext};
1818
use rustc_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
1919
use std::borrow::Cow;
2020
use std::convert::TryInto;
21-
use std::fmt::Display;
21+
use std::fmt::{Display, Write};
2222
use std::iter;
2323
use std::ops::{Add, Neg, Not, Sub};
2424

@@ -901,7 +901,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
901901
if cmt.place.projections.is_empty() {
902902
// handle item without any projection, that needs an explicit borrowing
903903
// i.e.: suggest `&x` instead of `x`
904-
self.suggestion_start.push_str(&format!("{}&{}", start_snip, ident_str));
904+
let _ = write!(self.suggestion_start, "{}&{}", start_snip, ident_str);
905905
} else {
906906
// cases where a parent `Call` or `MethodCall` is using the item
907907
// i.e.: suggest `.contains(&x)` for `.find(|x| [1, 2, 3].contains(x)).is_none()`
@@ -916,8 +916,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
916916
// given expression is the self argument and will be handled completely by the compiler
917917
// i.e.: `|x| x.is_something()`
918918
ExprKind::MethodCall(_, [self_expr, ..], _) if self_expr.hir_id == cmt.hir_id => {
919-
self.suggestion_start
920-
.push_str(&format!("{}{}", start_snip, ident_str_with_proj));
919+
let _ = write!(self.suggestion_start, "{}{}", start_snip, ident_str_with_proj);
921920
self.next_pos = span.hi();
922921
return;
923922
},
@@ -1025,8 +1024,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
10251024
}
10261025
}
10271026

1028-
self.suggestion_start
1029-
.push_str(&format!("{}{}", start_snip, replacement_str));
1027+
let _ = write!(self.suggestion_start, "{}{}", start_snip, replacement_str);
10301028
}
10311029
self.next_pos = span.hi();
10321030
}

‎tests/ui/format_push_string.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#![warn(clippy::format_push_string)]
2+
3+
fn main() {
4+
let mut string = String::new();
5+
string += &format!("{:?}", 1234);
6+
string.push_str(&format!("{:?}", 5678));
7+
}

‎tests/ui/format_push_string.stderr

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
error: `format!(..)` appended to existing `String`
2+
--> $DIR/format_push_string.rs:5:5
3+
|
4+
LL | string += &format!("{:?}", 1234);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::format-push-string` implied by `-D warnings`
8+
= help: consider using `write!` to avoid the extra allocation
9+
10+
error: `format!(..)` appended to existing `String`
11+
--> $DIR/format_push_string.rs:6:5
12+
|
13+
LL | string.push_str(&format!("{:?}", 5678));
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= help: consider using `write!` to avoid the extra allocation
17+
18+
error: aborting due to 2 previous errors
19+

‎tests/ui/identity_op.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::fmt::Write;
2+
13
const ONE: i64 = 1;
24
const NEG_ONE: i64 = -1;
35
const ZERO: i64 = 0;
@@ -7,7 +9,7 @@ struct A(String);
79
impl std::ops::Shl<i32> for A {
810
type Output = A;
911
fn shl(mut self, other: i32) -> Self {
10-
self.0.push_str(&format!("{}", other));
12+
let _ = write!(self.0, "{}", other);
1113
self
1214
}
1315
}

‎tests/ui/identity_op.stderr

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,109 +1,109 @@
11
error: the operation is ineffective. Consider reducing it to `x`
2-
--> $DIR/identity_op.rs:37:5
2+
--> $DIR/identity_op.rs:39:5
33
|
44
LL | x + 0;
55
| ^^^^^
66
|
77
= note: `-D clippy::identity-op` implied by `-D warnings`
88

99
error: the operation is ineffective. Consider reducing it to `x`
10-
--> $DIR/identity_op.rs:38:5
10+
--> $DIR/identity_op.rs:40:5
1111
|
1212
LL | x + (1 - 1);
1313
| ^^^^^^^^^^^
1414

1515
error: the operation is ineffective. Consider reducing it to `x`
16-
--> $DIR/identity_op.rs:40:5
16+
--> $DIR/identity_op.rs:42:5
1717
|
1818
LL | 0 + x;
1919
| ^^^^^
2020

2121
error: the operation is ineffective. Consider reducing it to `x`
22-
--> $DIR/identity_op.rs:43:5
22+
--> $DIR/identity_op.rs:45:5
2323
|
2424
LL | x | (0);
2525
| ^^^^^^^
2626

2727
error: the operation is ineffective. Consider reducing it to `x`
28-
--> $DIR/identity_op.rs:46:5
28+
--> $DIR/identity_op.rs:48:5
2929
|
3030
LL | x * 1;
3131
| ^^^^^
3232

3333
error: the operation is ineffective. Consider reducing it to `x`
34-
--> $DIR/identity_op.rs:47:5
34+
--> $DIR/identity_op.rs:49:5
3535
|
3636
LL | 1 * x;
3737
| ^^^^^
3838

3939
error: the operation is ineffective. Consider reducing it to `x`
40-
--> $DIR/identity_op.rs:53:5
40+
--> $DIR/identity_op.rs:55:5
4141
|
4242
LL | -1 & x;
4343
| ^^^^^^
4444

4545
error: the operation is ineffective. Consider reducing it to `u`
46-
--> $DIR/identity_op.rs:56:5
46+
--> $DIR/identity_op.rs:58:5
4747
|
4848
LL | u & 255;
4949
| ^^^^^^^
5050

5151
error: the operation is ineffective. Consider reducing it to `42`
52-
--> $DIR/identity_op.rs:59:5
52+
--> $DIR/identity_op.rs:61:5
5353
|
5454
LL | 42 << 0;
5555
| ^^^^^^^
5656

5757
error: the operation is ineffective. Consider reducing it to `1`
58-
--> $DIR/identity_op.rs:60:5
58+
--> $DIR/identity_op.rs:62:5
5959
|
6060
LL | 1 >> 0;
6161
| ^^^^^^
6262

6363
error: the operation is ineffective. Consider reducing it to `42`
64-
--> $DIR/identity_op.rs:61:5
64+
--> $DIR/identity_op.rs:63:5
6565
|
6666
LL | 42 >> 0;
6767
| ^^^^^^^
6868

6969
error: the operation is ineffective. Consider reducing it to `&x`
70-
--> $DIR/identity_op.rs:62:5
70+
--> $DIR/identity_op.rs:64:5
7171
|
7272
LL | &x >> 0;
7373
| ^^^^^^^
7474

7575
error: the operation is ineffective. Consider reducing it to `x`
76-
--> $DIR/identity_op.rs:63:5
76+
--> $DIR/identity_op.rs:65:5
7777
|
7878
LL | x >> &0;
7979
| ^^^^^^^
8080

8181
error: the operation is ineffective. Consider reducing it to `2`
82-
--> $DIR/identity_op.rs:70:5
82+
--> $DIR/identity_op.rs:72:5
8383
|
8484
LL | 2 % 3;
8585
| ^^^^^
8686

8787
error: the operation is ineffective. Consider reducing it to `-2`
88-
--> $DIR/identity_op.rs:71:5
88+
--> $DIR/identity_op.rs:73:5
8989
|
9090
LL | -2 % 3;
9191
| ^^^^^^
9292

9393
error: the operation is ineffective. Consider reducing it to `2`
94-
--> $DIR/identity_op.rs:72:5
94+
--> $DIR/identity_op.rs:74:5
9595
|
9696
LL | 2 % -3 + x;
9797
| ^^^^^^
9898

9999
error: the operation is ineffective. Consider reducing it to `-2`
100-
--> $DIR/identity_op.rs:73:5
100+
--> $DIR/identity_op.rs:75:5
101101
|
102102
LL | -2 % -3 + x;
103103
| ^^^^^^^
104104

105105
error: the operation is ineffective. Consider reducing it to `1`
106-
--> $DIR/identity_op.rs:74:9
106+
--> $DIR/identity_op.rs:76:9
107107
|
108108
LL | x + 1 % 3;
109109
| ^^^^^

0 commit comments

Comments
 (0)