Skip to content

Commit 97311f0

Browse files
JarchoJarcho
committed
Add lint manual_str_repeat 
1 parent 860cb8f commit 97311f0

File tree

10 files changed

+238
-7
lines changed

10 files changed

+238
-7
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
@@ -762,6 +762,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
762762
methods::MANUAL_FILTER_MAP,
763763
methods::MANUAL_FIND_MAP,
764764
methods::MANUAL_SATURATING_ARITHMETIC,
765+
methods::MANUAL_STR_REPEAT,
765766
methods::MAP_COLLECT_RESULT_UNIT,
766767
methods::MAP_FLATTEN,
767768
methods::MAP_UNWRAP_OR,
@@ -1298,6 +1299,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12981299
LintId::of(methods::MANUAL_FILTER_MAP),
12991300
LintId::of(methods::MANUAL_FIND_MAP),
13001301
LintId::of(methods::MANUAL_SATURATING_ARITHMETIC),
1302+
LintId::of(methods::MANUAL_STR_REPEAT),
13011303
LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
13021304
LintId::of(methods::NEW_RET_NO_SELF),
13031305
LintId::of(methods::OK_EXPECT),
@@ -1735,6 +1737,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
17351737
LintId::of(loops::NEEDLESS_COLLECT),
17361738
LintId::of(methods::EXPECT_FUN_CALL),
17371739
LintId::of(methods::ITER_NTH),
1740+
LintId::of(methods::MANUAL_STR_REPEAT),
17381741
LintId::of(methods::OR_FUN_CALL),
17391742
LintId::of(methods::SINGLE_CHAR_PATTERN),
17401743
LintId::of(misc::CMP_OWNED),

‎clippy_lints/src/mem_discriminant.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use rustc_errors::Applicability;
77
use rustc_hir::{BorrowKind, Expr, ExprKind};
88
use rustc_lint::{LateContext, LateLintPass};
99
use rustc_session::{declare_lint_pass, declare_tool_lint};
10-
use std::iter;
1110

1211
declare_clippy_lint! {
1312
/// **What it does:** Checks for calls of `mem::discriminant()` on a non-enum type.
@@ -67,7 +66,7 @@ impl<'tcx> LateLintPass<'tcx> for MemDiscriminant {
6766
}
6867
}
6968

70-
let derefs: String = iter::repeat('*').take(derefs_needed).collect();
69+
let derefs = "*".repeat(derefs_needed);
7170
diag.span_suggestion(
7271
param.span,
7372
"try dereferencing",

‎clippy_lints/src/methods/clone_on_copy.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ use rustc_hir::{BindingAnnotation, Expr, ExprKind, MatchSource, Node, PatKind};
88
use rustc_lint::LateContext;
99
use rustc_middle::ty::{self, adjustment::Adjust};
1010
use rustc_span::symbol::{sym, Symbol};
11-
use std::iter;
1211

1312
use super::CLONE_DOUBLE_REF;
1413
use super::CLONE_ON_COPY;
@@ -54,8 +53,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symbol,
5453
ty = inner;
5554
n += 1;
5655
}
57-
let refs: String = iter::repeat('&').take(n + 1).collect();
58-
let derefs: String = iter::repeat('*').take(n).collect();
56+
let refs = "&".repeat(n + 1);
57+
let derefs = "*".repeat(n);
5958
let explicit = format!("<{}{}>::clone({})", refs, ty, snip);
6059
diag.span_suggestion(
6160
expr.span,

‎clippy_lints/src/methods/manual_str_repeat.rs

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
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(
43+
cx: &LateContext<'_>,
44+
collect_expr: &Expr<'_>,
45+
take_expr: &Expr<'_>,
46+
take_self_arg: &Expr<'_>,
47+
take_arg: &Expr<'_>,
48+
) {
49+
if_chain! {
50+
if let ExprKind::Call(repeat_fn, [repeat_arg]) = take_self_arg.kind;
51+
if is_expr_path_def_path(cx, repeat_fn, &paths::ITER_REPEAT);
52+
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(collect_expr), sym::string_type);
53+
if let Some(collect_id) = cx.typeck_results().type_dependent_def_id(collect_expr.hir_id);
54+
if let Some(take_id) = cx.typeck_results().type_dependent_def_id(take_expr.hir_id);
55+
if let Some(iter_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
56+
if cx.tcx.trait_of_item(collect_id) == Some(iter_trait_id);
57+
if cx.tcx.trait_of_item(take_id) == Some(iter_trait_id);
58+
if let Some(repeat_kind) = parse_repeat_arg(cx, repeat_arg);
59+
let ctxt = collect_expr.span.ctxt();
60+
if ctxt == take_expr.span.ctxt();
61+
if ctxt == take_self_arg.span.ctxt();
62+
then {
63+
let mut app = Applicability::MachineApplicable;
64+
let (val_snip, val_is_mac) = snippet_with_context(cx, repeat_arg.span, ctxt, "..", &mut app);
65+
let count_snip = snippet_with_context(cx, take_arg.span, ctxt, "..", &mut app).0;
66+
67+
let val_str = match repeat_kind {
68+
RepeatKind::String => format!("(&{})", val_snip),
69+
RepeatKind::Str if !val_is_mac && repeat_arg.precedence().order() < PREC_POSTFIX => {
70+
format!("({})", val_snip)
71+
},
72+
RepeatKind::Str => val_snip.into(),
73+
RepeatKind::Char if val_snip == r#"'"'"# => r#""\"""#.into(),
74+
RepeatKind::Char if val_snip == r#"'\''"# => r#""'""#.into(),
75+
RepeatKind::Char => format!("\"{}\"", &val_snip[1..val_snip.len() - 1]),
76+
};
77+
78+
span_lint_and_sugg(
79+
cx,
80+
MANUAL_STR_REPEAT,
81+
collect_expr.span,
82+
"manual implementation of `str::repeat` using iterators",
83+
"try this",
84+
format!("{}.repeat({})", val_str, count_snip),
85+
app
86+
)
87+
}
88+
}
89+
}

‎clippy_lints/src/methods/mod.rs

Lines changed: 33 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;
@@ -60,9 +61,12 @@ mod wrong_self_convention;
6061
mod zst_offset;
6162

6263
use bind_instead_of_map::BindInsteadOfMap;
63-
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
6464
use clippy_utils::ty::{contains_adt_constructor, contains_ty, implements_trait, is_copy, is_type_diagnostic_item};
6565
use clippy_utils::{contains_return, get_trait_def_id, in_macro, iter_input_pats, paths, return_ty};
66+
use clippy_utils::{
67+
diagnostics::{span_lint, span_lint_and_help},
68+
meets_msrv, msrvs,
69+
};
6670
use if_chain::if_chain;
6771
use rustc_hir as hir;
6872
use rustc_hir::def::Res;
@@ -1664,6 +1668,27 @@ declare_clippy_lint! {
16641668
"checks for `.splitn(0, ..)` and `.splitn(1, ..)`"
16651669
}
16661670

1671+
declare_clippy_lint! {
1672+
/// **What it does:** Checks for manual implementations of `str::repeat`
1673+
///
1674+
/// **Why is this bad?** These are both harder to read, as well as less performant.
1675+
///
1676+
/// **Known problems:** None.
1677+
///
1678+
/// **Example:**
1679+
///
1680+
/// ```rust
1681+
/// // Bad
1682+
/// let x: String = std::iter::repeat('x').take(10).collect();
1683+
///
1684+
/// // Good
1685+
/// let x: String = "x".repeat(10);
1686+
/// ```
1687+
pub MANUAL_STR_REPEAT,
1688+
perf,
1689+
"manual implementation of `str::repeat`"
1690+
}
1691+
16671692
pub struct Methods {
16681693
avoid_breaking_exported_api: bool,
16691694
msrv: Option<RustcVersion>,
@@ -1737,7 +1762,8 @@ impl_lint_pass!(Methods => [
17371762
FROM_ITER_INSTEAD_OF_COLLECT,
17381763
INSPECT_FOR_EACH,
17391764
IMPLICIT_CLONE,
1740-
SUSPICIOUS_SPLITN
1765+
SUSPICIOUS_SPLITN,
1766+
MANUAL_STR_REPEAT
17411767
]);
17421768

17431769
/// Extracts a method call name, args, and `Span` of the method name.
@@ -1981,6 +2007,11 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
19812007
Some(("map", [m_recv, m_arg], _)) => {
19822008
map_collect_result_unit::check(cx, expr, m_recv, m_arg, recv);
19832009
},
2010+
Some(("take", [take_self_arg, take_arg], _)) => {
2011+
if meets_msrv(msrv, &msrvs::STR_REPEAT) {
2012+
manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg);
2013+
}
2014+
},
19842015
_ => {},
19852016
},
19862017
("count", []) => match method_call!(recv) {

‎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)