Add a lint for .repeat(1)

giraffate · giraffate · commit 5307cb5614e7 · 2020-07-07T23:13:39.000+09:00
fix rust-lang#3028.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1616,6 +1616,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
@@ -282,6 +282,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,
@@ -1071,6 +1073,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
@@ -0,0 +1,126 @@
+use crate::consts::{miri_to_const, 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_ast::ast::LitKind;
+use rustc_errors::Applicability;
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::{self, Ty};
+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 thi, `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 is_once(cx, &args[1]) && !in_macro(args[0].span);
+            then {
+                let ty = walk_ptrs_ty(cx.tables().expr_ty(&args[0]));
+                if is_str(ty){
+                    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 is_slice(ty) {
+                    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,
+                    );
+                }
+            }
+        }
+    }
+}
+
+fn is_once<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> bool {
+    match expr.kind {
+        ExprKind::Lit(ref lit) => {
+            if let LitKind::Int(ref lit_content, _) = lit.node {
+                *lit_content == 1
+            } else {
+                false
+            }
+        },
+        ExprKind::Path(rustc_hir::QPath::Resolved(None, path)) => {
+            if let Res::Def(DefKind::Const, def_id) = path.res {
+                let ty = cx.tcx.type_of(def_id);
+                let con = cx
+                    .tcx
+                    .const_eval_poly(def_id)
+                    .ok()
+                    .map(|val| rustc_middle::ty::Const::from_value(cx.tcx, val, ty))
+                    .unwrap();
+                let con = miri_to_const(con);
+                con == Some(Constant::Int(1))
+            } else {
+                false
+            }
+        },
+        _ => false,
+    }
+}
+
+fn is_str(ty: Ty<'_>) -> bool {
+    match ty.kind {
+        ty::Str => true,
+        _ => false,
+    }
+}
+
+fn is_slice(ty: Ty<'_>) -> bool {
+    match ty.kind {
+        ty::Slice(..) | ty::Array(..) => true,
+        _ => false,
+    }
+}
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
@@ -1879,6 +1879,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         deprecation: None,
         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",
diff --git a/tests/ui/repeat_once.fixed 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
@@ -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
@@ -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
+
