Add lint `manual_str_repeat`

10 files changed, 238 insertions, 7 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 59daa074282..41af8e190dd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2493,6 +2493,7 @@ Released 2018-09-13
 [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
 [`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
 [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
+[`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat
 [`manual_strip`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip
 [`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
 [`manual_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 0e815be9bd5..e7dd3952b3a 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -762,6 +762,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         methods::MANUAL_FILTER_MAP,
         methods::MANUAL_FIND_MAP,
         methods::MANUAL_SATURATING_ARITHMETIC,
+        methods::MANUAL_STR_REPEAT,
         methods::MAP_COLLECT_RESULT_UNIT,
         methods::MAP_FLATTEN,
         methods::MAP_UNWRAP_OR,
@@ -1298,6 +1299,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(methods::MANUAL_FILTER_MAP),
         LintId::of(methods::MANUAL_FIND_MAP),
         LintId::of(methods::MANUAL_SATURATING_ARITHMETIC),
+        LintId::of(methods::MANUAL_STR_REPEAT),
         LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
         LintId::of(methods::NEW_RET_NO_SELF),
         LintId::of(methods::OK_EXPECT),
@@ -1735,6 +1737,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(loops::NEEDLESS_COLLECT),
         LintId::of(methods::EXPECT_FUN_CALL),
         LintId::of(methods::ITER_NTH),
+        LintId::of(methods::MANUAL_STR_REPEAT),
         LintId::of(methods::OR_FUN_CALL),
         LintId::of(methods::SINGLE_CHAR_PATTERN),
         LintId::of(misc::CMP_OWNED),
diff --git a/clippy_lints/src/mem_discriminant.rs b/clippy_lints/src/mem_discriminant.rs
index a735c616f6e..aca96e06ef2 100644
--- a/clippy_lints/src/mem_discriminant.rs
+++ b/clippy_lints/src/mem_discriminant.rs
@@ -7,7 +7,6 @@ use rustc_errors::Applicability;
 use rustc_hir::{BorrowKind, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use std::iter;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for calls of `mem::discriminant()` on a non-enum type.
@@ -67,7 +66,7 @@ impl<'tcx> LateLintPass<'tcx> for MemDiscriminant {
                                 }
                             }
 
-                            let derefs: String = iter::repeat('*').take(derefs_needed).collect();
+                            let derefs = "*".repeat(derefs_needed);
                             diag.span_suggestion(
                                 param.span,
                                 "try dereferencing",
diff --git a/clippy_lints/src/methods/clone_on_copy.rs b/clippy_lints/src/methods/clone_on_copy.rs
index ce2e8fa8b10..1a32af5dc7a 100644
--- a/clippy_lints/src/methods/clone_on_copy.rs
+++ b/clippy_lints/src/methods/clone_on_copy.rs
@@ -8,7 +8,6 @@ use rustc_hir::{BindingAnnotation, Expr, ExprKind, MatchSource, Node, PatKind};
 use rustc_lint::LateContext;
 use rustc_middle::ty::{self, adjustment::Adjust};
 use rustc_span::symbol::{sym, Symbol};
-use std::iter;
 
 use super::CLONE_DOUBLE_REF;
 use super::CLONE_ON_COPY;
@@ -54,8 +53,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symbol,
                             ty = inner;
                             n += 1;
                         }
-                        let refs: String = iter::repeat('&').take(n + 1).collect();
-                        let derefs: String = iter::repeat('*').take(n).collect();
+                        let refs = "&".repeat(n + 1);
+                        let derefs = "*".repeat(n);
                         let explicit = format!("<{}{}>::clone({})", refs, ty, snip);
                         diag.span_suggestion(
                             expr.span,
diff --git a/clippy_lints/src/methods/manual_str_repeat.rs b/clippy_lints/src/methods/manual_str_repeat.rs
new file mode 100644
index 00000000000..3f28412fbf7
--- /dev/null
+++ b/clippy_lints/src/methods/manual_str_repeat.rs
@@ -0,0 +1,89 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_context;
+use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type};
+use clippy_utils::{is_expr_path_def_path, paths};
+use if_chain::if_chain;
+use rustc_ast::util::parser::PREC_POSTFIX;
+use rustc_ast::LitKind;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, LangItem};
+use rustc_lint::LateContext;
+use rustc_span::symbol::{sym, Symbol};
+
+use super::MANUAL_STR_REPEAT;
+
+enum RepeatKind {
+    Str,
+    String,
+    Char,
+}
+
+fn parse_repeat_arg(cx: &LateContext<'_>, e: &Expr<'_>) -> Option<RepeatKind> {
+    if let ExprKind::Lit(lit) = &e.kind {
+        match lit.node {
+            LitKind::Str(..) => Some(RepeatKind::Str),
+            LitKind::Char(_) => Some(RepeatKind::Char),
+            _ => None,
+        }
+    } else {
+        let ty = cx.typeck_results().expr_ty(e);
+        if is_type_diagnostic_item(cx, ty, sym::string_type)
+            || is_type_lang_item(cx, ty, LangItem::OwnedBox)
+            || match_type(cx, ty, &paths::COW)
+        {
+            Some(RepeatKind::String)
+        } else {
+            let ty = ty.peel_refs();
+            (ty.is_str() || is_type_diagnostic_item(cx, ty, sym::string_type)).then(|| RepeatKind::Str)
+        }
+    }
+}
+
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    collect_expr: &Expr<'_>,
+    take_expr: &Expr<'_>,
+    take_self_arg: &Expr<'_>,
+    take_arg: &Expr<'_>,
+) {
+    if_chain! {
+        if let ExprKind::Call(repeat_fn, [repeat_arg]) = take_self_arg.kind;
+        if is_expr_path_def_path(cx, repeat_fn, &paths::ITER_REPEAT);
+        if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(collect_expr), sym::string_type);
+        if let Some(collect_id) = cx.typeck_results().type_dependent_def_id(collect_expr.hir_id);
+        if let Some(take_id) = cx.typeck_results().type_dependent_def_id(take_expr.hir_id);
+        if let Some(iter_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
+        if cx.tcx.trait_of_item(collect_id) == Some(iter_trait_id);
+        if cx.tcx.trait_of_item(take_id) == Some(iter_trait_id);
+        if let Some(repeat_kind) = parse_repeat_arg(cx, repeat_arg);
+        let ctxt = collect_expr.span.ctxt();
+        if ctxt == take_expr.span.ctxt();
+        if ctxt == take_self_arg.span.ctxt();
+        then {
+            let mut app = Applicability::MachineApplicable;
+            let (val_snip, val_is_mac) = snippet_with_context(cx, repeat_arg.span, ctxt, "..", &mut app);
+            let count_snip = snippet_with_context(cx, take_arg.span, ctxt, "..", &mut app).0;
+
+            let val_str = match repeat_kind {
+                RepeatKind::String => format!("(&{})", val_snip),
+                RepeatKind::Str if !val_is_mac && repeat_arg.precedence().order() < PREC_POSTFIX => {
+                    format!("({})", val_snip)
+                },
+                RepeatKind::Str => val_snip.into(),
+                RepeatKind::Char if val_snip == r#"'"'"# => r#""\"""#.into(),
+                RepeatKind::Char if val_snip == r#"'\''"# => r#""'""#.into(),
+                RepeatKind::Char => format!("\"{}\"", &val_snip[1..val_snip.len() - 1]),
+            };
+
+            span_lint_and_sugg(
+                cx,
+                MANUAL_STR_REPEAT,
+                collect_expr.span,
+                "manual implementation of `str::repeat` using iterators",
+                "try this",
+                format!("{}.repeat({})", val_str, count_snip),
+                app
+            )
+        }
+    }
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index a6e2e0baadb..62a56434b3c 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -32,6 +32,7 @@ mod iter_nth_zero;
 mod iter_skip_next;
 mod iterator_step_by_zero;
 mod manual_saturating_arithmetic;
+mod manual_str_repeat;
 mod map_collect_result_unit;
 mod map_flatten;
 mod map_unwrap_or;
@@ -60,9 +61,12 @@ mod wrong_self_convention;
 mod zst_offset;
 
 use bind_instead_of_map::BindInsteadOfMap;
-use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
 use clippy_utils::ty::{contains_adt_constructor, contains_ty, implements_trait, is_copy, is_type_diagnostic_item};
 use clippy_utils::{contains_return, get_trait_def_id, in_macro, iter_input_pats, paths, return_ty};
+use clippy_utils::{
+    diagnostics::{span_lint, span_lint_and_help},
+    meets_msrv, msrvs,
+};
 use if_chain::if_chain;
 use rustc_hir as hir;
 use rustc_hir::def::Res;
@@ -1664,6 +1668,27 @@ declare_clippy_lint! {
     "checks for `.splitn(0, ..)` and `.splitn(1, ..)`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for manual implementations of `str::repeat`
+    ///
+    /// **Why is this bad?** These are both harder to read, as well as less performant.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// // Bad
+    /// let x: String = std::iter::repeat('x').take(10).collect();
+    ///
+    /// // Good
+    /// let x: String = "x".repeat(10);
+    /// ```
+    pub MANUAL_STR_REPEAT,
+    perf,
+    "manual implementation of `str::repeat`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -1737,7 +1762,8 @@ impl_lint_pass!(Methods => [
     FROM_ITER_INSTEAD_OF_COLLECT,
     INSPECT_FOR_EACH,
     IMPLICIT_CLONE,
-    SUSPICIOUS_SPLITN
+    SUSPICIOUS_SPLITN,
+    MANUAL_STR_REPEAT
 ]);
 
 /// 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
                 Some(("map", [m_recv, m_arg], _)) => {
                     map_collect_result_unit::check(cx, expr, m_recv, m_arg, recv);
                 },
+                Some(("take", [take_self_arg, take_arg], _)) => {
+                    if meets_msrv(msrv, &msrvs::STR_REPEAT) {
+                        manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg);
+                    }
+                },
                 _ => {},
             },
             ("count", []) => match method_call!(recv) {
diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs
index 00df04c0144..4a9c4fd0276 100644
--- a/clippy_utils/src/msrvs.rs
+++ b/clippy_utils/src/msrvs.rs
@@ -26,4 +26,5 @@ msrv_aliases! {
     1,34,0 { TRY_FROM }
     1,30,0 { ITERATOR_FIND_MAP }
     1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST }
+    1,16,0 { STR_REPEAT }
 }
diff --git a/tests/ui/manual_str_repeat.fixed b/tests/ui/manual_str_repeat.fixed
new file mode 100644
index 00000000000..62225e7a7f8
--- /dev/null
+++ b/tests/ui/manual_str_repeat.fixed
@@ -0,0 +1,31 @@
+// run-rustfix
+
+#![warn(clippy::manual_str_repeat)]
+
+use std::iter::repeat;
+
+fn main() {
+    let _: String = "test".repeat(10);
+    let _: String = "x".repeat(10);
+    let _: String = "'".repeat(10);
+    let _: String = "\"".repeat(10);
+
+    let x = "test";
+    let count = 10;
+    let _ = x.repeat(count + 2);
+
+    macro_rules! m {
+        ($e:expr) => {{ $e }};
+    }
+
+    let _: String = m!("test").repeat(m!(count));
+
+    let x = &x;
+    let _: String = (*x).repeat(count);
+
+    macro_rules! repeat_m {
+        ($e:expr) => {{ repeat($e) }};
+    }
+    // Don't lint, repeat is from a macro.
+    let _: String = repeat_m!("test").take(count).collect();
+}
diff --git a/tests/ui/manual_str_repeat.rs b/tests/ui/manual_str_repeat.rs
new file mode 100644
index 00000000000..1acd66da275
--- /dev/null
+++ b/tests/ui/manual_str_repeat.rs
@@ -0,0 +1,31 @@
+// run-rustfix
+
+#![warn(clippy::manual_str_repeat)]
+
+use std::iter::repeat;
+
+fn main() {
+    let _: String = std::iter::repeat("test").take(10).collect();
+    let _: String = std::iter::repeat('x').take(10).collect();
+    let _: String = std::iter::repeat('\'').take(10).collect();
+    let _: String = std::iter::repeat('"').take(10).collect();
+
+    let x = "test";
+    let count = 10;
+    let _ = repeat(x).take(count + 2).collect::<String>();
+
+    macro_rules! m {
+        ($e:expr) => {{ $e }};
+    }
+
+    let _: String = repeat(m!("test")).take(m!(count)).collect();
+
+    let x = &x;
+    let _: String = repeat(*x).take(count).collect();
+
+    macro_rules! repeat_m {
+        ($e:expr) => {{ repeat($e) }};
+    }
+    // Don't lint, repeat is from a macro.
+    let _: String = repeat_m!("test").take(count).collect();
+}
diff --git a/tests/ui/manual_str_repeat.stderr b/tests/ui/manual_str_repeat.stderr
new file mode 100644
index 00000000000..ef67ad2a1fd
--- /dev/null
+++ b/tests/ui/manual_str_repeat.stderr
@@ -0,0 +1,46 @@
+error: manual implementation of `str::repeat` using iterators
+  --> $DIR/manual_str_repeat.rs:8:21
+   |
+LL |     let _: String = std::iter::repeat("test").take(10).collect();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"test".repeat(10)`
+   |
+   = note: `-D clippy::manual-str-repeat` implied by `-D warnings`
+
+error: manual implementation of `str::repeat` using iterators
+  --> $DIR/manual_str_repeat.rs:9:21
+   |
+LL |     let _: String = std::iter::repeat('x').take(10).collect();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"x".repeat(10)`
+
+error: manual implementation of `str::repeat` using iterators
+  --> $DIR/manual_str_repeat.rs:10:21
+   |
+LL |     let _: String = std::iter::repeat('/'').take(10).collect();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"'".repeat(10)`
+
+error: manual implementation of `str::repeat` using iterators
+  --> $DIR/manual_str_repeat.rs:11:21
+   |
+LL |     let _: String = std::iter::repeat('"').take(10).collect();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"/"".repeat(10)`
+
+error: manual implementation of `str::repeat` using iterators
+  --> $DIR/manual_str_repeat.rs:15:13
+   |
+LL |     let _ = repeat(x).take(count + 2).collect::<String>();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `x.repeat(count + 2)`
+
+error: manual implementation of `str::repeat` using iterators
+  --> $DIR/manual_str_repeat.rs:21:21
+   |
+LL |     let _: String = repeat(m!("test")).take(m!(count)).collect();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `m!("test").repeat(m!(count))`
+
+error: manual implementation of `str::repeat` using iterators
+  --> $DIR/manual_str_repeat.rs:24:21
+   |
+LL |     let _: String = repeat(*x).take(count).collect();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(*x).repeat(count)`
+
+error: aborting due to 7 previous errors
+