Skip to content

Commit 2d894e8

Browse files
JarchoJarcho
committed
Add lint manual_str_repeat 
1 parent 029c326 commit 2d894e8

File tree

10 files changed

+234
-5
lines changed

10 files changed

+234
-5
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2493,6 +2493,7 @@ Released 2018-09-13
24932493
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
24942494
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
24952495
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
2496+
[`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat
24962497
[`manual_strip`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip
24972498
[`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
24982499
[`manual_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or

‎clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
779779
methods::MANUAL_FILTER_MAP,
780780
methods::MANUAL_FIND_MAP,
781781
methods::MANUAL_SATURATING_ARITHMETIC,
782+
methods::MANUAL_STR_REPEAT,
782783
methods::MAP_COLLECT_RESULT_UNIT,
783784
methods::MAP_FLATTEN,
784785
methods::MAP_UNWRAP_OR,
@@ -1317,6 +1318,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13171318
LintId::of(methods::MANUAL_FILTER_MAP),
13181319
LintId::of(methods::MANUAL_FIND_MAP),
13191320
LintId::of(methods::MANUAL_SATURATING_ARITHMETIC),
1321+
LintId::of(methods::MANUAL_STR_REPEAT),
13201322
LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
13211323
LintId::of(methods::NEW_RET_NO_SELF),
13221324
LintId::of(methods::OK_EXPECT),
@@ -1752,6 +1754,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
17521754
LintId::of(loops::NEEDLESS_COLLECT),
17531755
LintId::of(methods::EXPECT_FUN_CALL),
17541756
LintId::of(methods::ITER_NTH),
1757+
LintId::of(methods::MANUAL_STR_REPEAT),
17551758
LintId::of(methods::OR_FUN_CALL),
17561759
LintId::of(methods::SINGLE_CHAR_PATTERN),
17571760
LintId::of(misc::CMP_OWNED),

‎clippy_lints/src/mem_discriminant.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ impl<'tcx> LateLintPass<'tcx> for MemDiscriminant {
6767
}
6868
}
6969

70-
let derefs: String = iter::repeat('*').take(derefs_needed).collect();
70+
let derefs = "*".repeat(derefs_needed);
7171
diag.span_suggestion(
7272
param.span,
7373
"try dereferencing",

‎clippy_lints/src/methods/clone_on_copy.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symbol,
5454
ty = inner;
5555
n += 1;
5656
}
57-
let refs: String = iter::repeat('&').take(n + 1).collect();
58-
let derefs: String = iter::repeat('*').take(n).collect();
57+
let refs = "&".repeat(n + 1);
58+
let derefs = "*".repeat(n);
5959
let explicit = format!("<{}{}>::clone({})", refs, ty, snip);
6060
diag.span_suggestion(
6161
expr.span,

‎clippy_lints/src/methods/manual_str_repeat.rs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet_with_context;
3+
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type};
4+
use clippy_utils::{is_expr_path_def_path, paths};
5+
use if_chain::if_chain;
6+
use rustc_ast::util::parser::PREC_POSTFIX;
7+
use rustc_ast::LitKind;
8+
use rustc_errors::Applicability;
9+
use rustc_hir::{Expr, ExprKind, LangItem};
10+
use rustc_lint::LateContext;
11+
use rustc_span::symbol::{sym, Symbol};
12+
13+
use super::MANUAL_STR_REPEAT;
14+
15+
enum RepeatKind {
16+
Str,
17+
String,
18+
Char,
19+
}
20+
21+
fn parse_repeat_arg(cx: &LateContext<'_>, e: &Expr<'_>) -> Option<RepeatKind> {
22+
if let ExprKind::Lit(lit) = &e.kind {
23+
match lit.node {
24+
LitKind::Str(..) => Some(RepeatKind::Str),
25+
LitKind::Char(_) => Some(RepeatKind::Char),
26+
_ => None,
27+
}
28+
} else {
29+
let ty = cx.typeck_results().expr_ty(e);
30+
if is_type_diagnostic_item(cx, ty, sym::string_type)
31+
|| is_type_lang_item(cx, ty, LangItem::OwnedBox)
32+
|| match_type(cx, ty, &paths::COW)
33+
{
34+
Some(RepeatKind::String)
35+
} else {
36+
let ty = ty.peel_refs();
37+
(ty.is_str() || is_type_diagnostic_item(cx, ty, sym::string_type)).then(|| RepeatKind::Str)
38+
}
39+
}
40+
}
41+
42+
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symbol, args: &[Expr<'_>]) {
43+
let ctxt = expr.span.ctxt();
44+
if_chain! {
45+
if let [take_expr] = args;
46+
if method_name.as_str() == "collect";
47+
if let ExprKind::MethodCall(take_name, _, [repeat_expr, take_arg], _) = take_expr.kind;
48+
if take_name.ident.as_str() == "take";
49+
if let ExprKind::Call(repeat_fn, [repeat_arg]) = repeat_expr.kind;
50+
if is_expr_path_def_path(cx, repeat_fn, &paths::ITER_REPEAT);
51+
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::string_type);
52+
if let Some(collect_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
53+
if let Some(take_id) = cx.typeck_results().type_dependent_def_id(take_expr.hir_id);
54+
if let Some(iter_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
55+
if cx.tcx.trait_of_item(collect_id) == Some(iter_trait_id);
56+
if cx.tcx.trait_of_item(take_id) == Some(iter_trait_id);
57+
if let Some(repeat_kind) = parse_repeat_arg(cx, repeat_arg);
58+
if ctxt == take_expr.span.ctxt();
59+
if ctxt == repeat_expr.span.ctxt();
60+
then {
61+
let mut app = Applicability::MachineApplicable;
62+
let (val_snip, val_is_mac) = snippet_with_context(cx, repeat_arg.span, ctxt, "..", &mut app);
63+
let count_snip = snippet_with_context(cx, take_arg.span, ctxt, "..", &mut app).0;
64+
65+
let val_str = match repeat_kind {
66+
RepeatKind::String => format!("(&{})", val_snip),
67+
RepeatKind::Str if !val_is_mac && repeat_arg.precedence().order() < PREC_POSTFIX => {
68+
format!("({})", val_snip)
69+
},
70+
RepeatKind::Str => val_snip.into(),
71+
RepeatKind::Char if val_snip == r#"'"'"# => r#""\"""#.into(),
72+
RepeatKind::Char if val_snip == r#"'\''"# => r#""'""#.into(),
73+
RepeatKind::Char => format!("\"{}\"", &val_snip[1..val_snip.len() - 1]),
74+
};
75+
76+
span_lint_and_sugg(
77+
cx,
78+
MANUAL_STR_REPEAT,
79+
expr.span,
80+
"manual implementation of `str::repeat` using iterators",
81+
"try this",
82+
format!("{}.repeat({})", val_str, count_snip),
83+
app
84+
)
85+
}
86+
}
87+
}

‎clippy_lints/src/methods/mod.rs

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ mod iter_nth_zero;
3232
mod iter_skip_next;
3333
mod iterator_step_by_zero;
3434
mod manual_saturating_arithmetic;
35+
mod manual_str_repeat;
3536
mod map_collect_result_unit;
3637
mod map_flatten;
3738
mod map_unwrap_or;
@@ -59,9 +60,12 @@ mod wrong_self_convention;
5960
mod zst_offset;
6061

6162
use bind_instead_of_map::BindInsteadOfMap;
62-
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
6363
use clippy_utils::ty::{contains_adt_constructor, contains_ty, implements_trait, is_copy, is_type_diagnostic_item};
6464
use clippy_utils::{contains_return, get_trait_def_id, in_macro, iter_input_pats, paths, return_ty};
65+
use clippy_utils::{
66+
diagnostics::{span_lint, span_lint_and_help},
67+
meets_msrv, msrvs,
68+
};
6569
use if_chain::if_chain;
6670
use rustc_hir as hir;
6771
use rustc_hir::def::Res;
@@ -1657,6 +1661,27 @@ declare_clippy_lint! {
16571661
"replace `.iter().count()` with `.len()`"
16581662
}
16591663

1664+
declare_clippy_lint! {
1665+
/// **What it does:** Checks for manual implementations of `str::repeat`
1666+
///
1667+
/// **Why is this bad?** These are both harder to read, as well as less performant.
1668+
///
1669+
/// **Known problems:** None.
1670+
///
1671+
/// **Example:**
1672+
///
1673+
/// ```rust
1674+
/// let x: String = std::iter::repeat('x').take(10).collect();
1675+
/// ```
1676+
/// Use instead:
1677+
/// ```rust
1678+
/// let x: String = "x".repeat(10);
1679+
/// ```
1680+
pub MANUAL_STR_REPEAT,
1681+
perf,
1682+
"manual implementation of `str::repeat`"
1683+
}
1684+
16601685
pub struct Methods {
16611686
msrv: Option<RustcVersion>,
16621687
}
@@ -1726,7 +1751,8 @@ impl_lint_pass!(Methods => [
17261751
MAP_COLLECT_RESULT_UNIT,
17271752
FROM_ITER_INSTEAD_OF_COLLECT,
17281753
INSPECT_FOR_EACH,
1729-
IMPLICIT_CLONE
1754+
IMPLICIT_CLONE,
1755+
MANUAL_STR_REPEAT
17301756
]);
17311757

17321758
/// Extracts a method call name, args, and `Span` of the method name.
@@ -1769,6 +1795,9 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
17691795
single_char_add_str::check(cx, expr, args);
17701796
into_iter_on_ref::check(cx, expr, *method_span, method_call.ident.name, args);
17711797
single_char_pattern::check(cx, expr, method_call.ident.name, args);
1798+
if meets_msrv(self.msrv.as_ref(), &msrvs::STR_REPEAT) {
1799+
manual_str_repeat::check(cx, expr, method_call.ident.name, args);
1800+
}
17721801
},
17731802
hir::ExprKind::Binary(op, lhs, rhs) if op.node == hir::BinOpKind::Eq || op.node == hir::BinOpKind::Ne => {
17741803
let mut info = BinaryExprInfo {

‎clippy_utils/src/msrvs.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,5 @@ msrv_aliases! {
2626
1,34,0 { TRY_FROM }
2727
1,30,0 { ITERATOR_FIND_MAP }
2828
1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST }
29+
1,16,0 { STR_REPEAT }
2930
}

‎tests/ui/manual_str_repeat.fixed

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::manual_str_repeat)]
4+
5+
use std::iter::repeat;
6+
7+
fn main() {
8+
let _: String = "test".repeat(10);
9+
let _: String = "x".repeat(10);
10+
let _: String = "'".repeat(10);
11+
let _: String = "\"".repeat(10);
12+
13+
let x = "test";
14+
let count = 10;
15+
let _ = x.repeat(count + 2);
16+
17+
macro_rules! m {
18+
($e:expr) => {{ $e }};
19+
}
20+
21+
let _: String = m!("test").repeat(m!(count));
22+
23+
let x = &x;
24+
let _: String = (*x).repeat(count);
25+
26+
macro_rules! repeat_m {
27+
($e:expr) => {{ repeat($e) }};
28+
}
29+
// Don't lint, repeat is from a macro.
30+
let _: String = repeat_m!("test").take(count).collect();
31+
}

‎tests/ui/manual_str_repeat.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::manual_str_repeat)]
4+
5+
use std::iter::repeat;
6+
7+
fn main() {
8+
let _: String = std::iter::repeat("test").take(10).collect();
9+
let _: String = std::iter::repeat('x').take(10).collect();
10+
let _: String = std::iter::repeat('\'').take(10).collect();
11+
let _: String = std::iter::repeat('"').take(10).collect();
12+
13+
let x = "test";
14+
let count = 10;
15+
let _ = repeat(x).take(count + 2).collect::<String>();
16+
17+
macro_rules! m {
18+
($e:expr) => {{ $e }};
19+
}
20+
21+
let _: String = repeat(m!("test")).take(m!(count)).collect();
22+
23+
let x = &x;
24+
let _: String = repeat(*x).take(count).collect();
25+
26+
macro_rules! repeat_m {
27+
($e:expr) => {{ repeat($e) }};
28+
}
29+
// Don't lint, repeat is from a macro.
30+
let _: String = repeat_m!("test").take(count).collect();
31+
}

‎tests/ui/manual_str_repeat.stderr

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
error: manual implementation of `str::repeat` using iterators
2+
--> $DIR/manual_str_repeat.rs:8:21
3+
|
4+
LL | let _: String = std::iter::repeat("test").take(10).collect();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"test".repeat(10)`
6+
|
7+
= note: `-D clippy::manual-str-repeat` implied by `-D warnings`
8+
9+
error: manual implementation of `str::repeat` using iterators
10+
--> $DIR/manual_str_repeat.rs:9:21
11+
|
12+
LL | let _: String = std::iter::repeat('x').take(10).collect();
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"x".repeat(10)`
14+
15+
error: manual implementation of `str::repeat` using iterators
16+
--> $DIR/manual_str_repeat.rs:10:21
17+
|
18+
LL | let _: String = std::iter::repeat('/'').take(10).collect();
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"'".repeat(10)`
20+
21+
error: manual implementation of `str::repeat` using iterators
22+
--> $DIR/manual_str_repeat.rs:11:21
23+
|
24+
LL | let _: String = std::iter::repeat('"').take(10).collect();
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"/"".repeat(10)`
26+
27+
error: manual implementation of `str::repeat` using iterators
28+
--> $DIR/manual_str_repeat.rs:15:13
29+
|
30+
LL | let _ = repeat(x).take(count + 2).collect::<String>();
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `x.repeat(count + 2)`
32+
33+
error: manual implementation of `str::repeat` using iterators
34+
--> $DIR/manual_str_repeat.rs:21:21
35+
|
36+
LL | let _: String = repeat(m!("test")).take(m!(count)).collect();
37+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `m!("test").repeat(m!(count))`
38+
39+
error: manual implementation of `str::repeat` using iterators
40+
--> $DIR/manual_str_repeat.rs:24:21
41+
|
42+
LL | let _: String = repeat(*x).take(count).collect();
43+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(*x).repeat(count)`
44+
45+
error: aborting due to 7 previous errors
46+

0 commit comments

Comments
 (0)