Auto merge of #5773 - giraffate:repeat_once, r=flip1995

Add a lint for `.repeat(1)`

changelog: New lint `repeat_once`

fix #3028.

7 files changed, 167 insertions, 0 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6261ca4879a..5d08b44ba40 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1617,6 +1617,7 @@ Released 2018-09-13
 [`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
 [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
 [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
+[`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once
 [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
 [`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
 [`result_map_or_into_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_or_into_option
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index aa911cf59ee..32e79317f82 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -281,6 +281,7 @@ mod redundant_pub_crate;
 mod redundant_static_lifetimes;
 mod reference;
 mod regex;
+mod repeat_once;
 mod returns;
 mod serde_api;
 mod shadow;
@@ -764,6 +765,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &reference::REF_IN_DEREF,
         &regex::INVALID_REGEX,
         &regex::TRIVIAL_REGEX,
+        &repeat_once::REPEAT_ONCE,
         &returns::NEEDLESS_RETURN,
         &returns::UNUSED_UNIT,
         &serde_api::SERDE_API_MISUSE,
@@ -1070,6 +1072,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box macro_use::MacroUseImports::default());
     store.register_late_pass(|| box map_identity::MapIdentity);
     store.register_late_pass(|| box pattern_type_mismatch::PatternTypeMismatch);
+    store.register_late_pass(|| box repeat_once::RepeatOnce);
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1393,6 +1396,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&reference::REF_IN_DEREF),
         LintId::of(&regex::INVALID_REGEX),
         LintId::of(&regex::TRIVIAL_REGEX),
+        LintId::of(&repeat_once::REPEAT_ONCE),
         LintId::of(&returns::NEEDLESS_RETURN),
         LintId::of(&returns::UNUSED_UNIT),
         LintId::of(&serde_api::SERDE_API_MISUSE),
@@ -1602,6 +1606,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&ranges::RANGE_ZIP_WITH_LEN),
         LintId::of(&reference::DEREF_ADDROF),
         LintId::of(&reference::REF_IN_DEREF),
+        LintId::of(&repeat_once::REPEAT_ONCE),
         LintId::of(&swap::MANUAL_SWAP),
         LintId::of(&temporary_assignment::TEMPORARY_ASSIGNMENT),
         LintId::of(&transmute::CROSSPOINTER_TRANSMUTE),
diff --git a/clippy_lints/src/repeat_once.rs b/clippy_lints/src/repeat_once.rs
new file mode 100644
index 00000000000..a3af369e41e
--- /dev/null
+++ b/clippy_lints/src/repeat_once.rs
@@ -0,0 +1,82 @@
+use crate::consts::{constant_context, Constant};
+use crate::utils::{in_macro, is_type_diagnostic_item, snippet, span_lint_and_sugg, walk_ptrs_ty};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `.repeat(1)` and suggest the following method for each types.
+    /// - `.to_string()` for `str`
+    /// - `.clone()` for `String`
+    /// - `.to_vec()` for `slice`
+    ///
+    /// **Why is this bad?** For example, `String.repeat(1)` is equivalent to `.clone()`. If cloning the string is the intention behind this, `clone()` should be used.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// fn main() {
+    ///     let x = String::from("hello world").repeat(1);
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn main() {
+    ///     let x = String::from("hello world").clone();
+    /// }
+    /// ```
+    pub REPEAT_ONCE,
+    complexity,
+    "using `.repeat(1)` instead of `String.clone()`, `str.to_string()` or `slice.to_vec()` "
+}
+
+declare_lint_pass!(RepeatOnce => [REPEAT_ONCE]);
+
+impl<'tcx> LateLintPass<'tcx> for RepeatOnce {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'tcx Expr<'_>) {
+        if_chain! {
+            if let ExprKind::MethodCall(ref path, _, ref args, _) = expr.kind;
+            if path.ident.name == sym!(repeat);
+            if let Some(Constant::Int(1)) = constant_context(cx, cx.tables()).expr(&args[1]);
+            if !in_macro(args[0].span);
+            then {
+                let ty = walk_ptrs_ty(cx.tables().expr_ty(&args[0]));
+                if ty.is_str() {
+                    span_lint_and_sugg(
+                        cx,
+                        REPEAT_ONCE,
+                        expr.span,
+                        "calling `repeat(1)` on str",
+                        "consider using `.to_string()` instead",
+                        format!("{}.to_string()", snippet(cx, args[0].span, r#""...""#)),
+                        Applicability::MachineApplicable,
+                    );
+                } else if ty.builtin_index().is_some() {
+                    span_lint_and_sugg(
+                        cx,
+                        REPEAT_ONCE,
+                        expr.span,
+                        "calling `repeat(1)` on slice",
+                        "consider using `.to_vec()` instead",
+                        format!("{}.to_vec()", snippet(cx, args[0].span, r#""...""#)),
+                        Applicability::MachineApplicable,
+                    );
+                } else if is_type_diagnostic_item(cx, ty, sym!(string_type)) {
+                    span_lint_and_sugg(
+                        cx,
+                        REPEAT_ONCE,
+                        expr.span,
+                        "calling `repeat(1)` on a string literal",
+                        "consider using `.clone()` instead",
+                        format!("{}.clone()", snippet(cx, args[0].span, r#""...""#)),
+                        Applicability::MachineApplicable,
+                    );
+                }
+            }
+        }
+    }
+}
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index 0c77e07c872..b89a8712862 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -1887,6 +1887,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         module: "reference",
     },
     Lint {
+        name: "repeat_once",
+        group: "complexity",
+        desc: "using `.repeat(1)` instead of `String.clone()`, `str.to_string()` or `slice.to_vec()` ",
+        deprecation: None,
+        module: "repeat_once",
+    },
+    Lint {
         name: "rest_pat_in_fully_bound_structs",
         group: "restriction",
         desc: "a match on a struct that binds all fields but still uses the wildcard pattern",
diff --git a/tests/ui/repeat_once.fixed b/tests/ui/repeat_once.fixed
new file mode 100644
index 00000000000..a637c22fbcd
--- /dev/null
+++ b/tests/ui/repeat_once.fixed
@@ -0,0 +1,16 @@
+// run-rustfix
+#![warn(clippy::repeat_once)]
+#[allow(unused, clippy::many_single_char_names, clippy::redundant_clone)]
+fn main() {
+    const N: usize = 1;
+    let s = "str";
+    let string = "String".to_string();
+    let slice = [1; 5];
+
+    let a = [1; 5].to_vec();
+    let b = slice.to_vec();
+    let c = "hello".to_string();
+    let d = "hi".to_string();
+    let e = s.to_string();
+    let f = string.clone();
+}
diff --git a/tests/ui/repeat_once.rs b/tests/ui/repeat_once.rs
new file mode 100644
index 00000000000..d99ca1b5b55
--- /dev/null
+++ b/tests/ui/repeat_once.rs
@@ -0,0 +1,16 @@
+// run-rustfix
+#![warn(clippy::repeat_once)]
+#[allow(unused, clippy::many_single_char_names, clippy::redundant_clone)]
+fn main() {
+    const N: usize = 1;
+    let s = "str";
+    let string = "String".to_string();
+    let slice = [1; 5];
+
+    let a = [1; 5].repeat(1);
+    let b = slice.repeat(1);
+    let c = "hello".repeat(N);
+    let d = "hi".repeat(1);
+    let e = s.repeat(1);
+    let f = string.repeat(1);
+}
diff --git a/tests/ui/repeat_once.stderr b/tests/ui/repeat_once.stderr
new file mode 100644
index 00000000000..915eea3bfc6
--- /dev/null
+++ b/tests/ui/repeat_once.stderr
@@ -0,0 +1,40 @@
+error: calling `repeat(1)` on slice
+  --> $DIR/repeat_once.rs:10:13
+   |
+LL |     let a = [1; 5].repeat(1);
+   |             ^^^^^^^^^^^^^^^^ help: consider using `.to_vec()` instead: `[1; 5].to_vec()`
+   |
+   = note: `-D clippy::repeat-once` implied by `-D warnings`
+
+error: calling `repeat(1)` on slice
+  --> $DIR/repeat_once.rs:11:13
+   |
+LL |     let b = slice.repeat(1);
+   |             ^^^^^^^^^^^^^^^ help: consider using `.to_vec()` instead: `slice.to_vec()`
+
+error: calling `repeat(1)` on str
+  --> $DIR/repeat_once.rs:12:13
+   |
+LL |     let c = "hello".repeat(N);
+   |             ^^^^^^^^^^^^^^^^^ help: consider using `.to_string()` instead: `"hello".to_string()`
+
+error: calling `repeat(1)` on str
+  --> $DIR/repeat_once.rs:13:13
+   |
+LL |     let d = "hi".repeat(1);
+   |             ^^^^^^^^^^^^^^ help: consider using `.to_string()` instead: `"hi".to_string()`
+
+error: calling `repeat(1)` on str
+  --> $DIR/repeat_once.rs:14:13
+   |
+LL |     let e = s.repeat(1);
+   |             ^^^^^^^^^^^ help: consider using `.to_string()` instead: `s.to_string()`
+
+error: calling `repeat(1)` on a string literal
+  --> $DIR/repeat_once.rs:15:13
+   |
+LL |     let f = string.repeat(1);
+   |             ^^^^^^^^^^^^^^^^ help: consider using `.clone()` instead: `string.clone()`
+
+error: aborting due to 6 previous errors
+