diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a081bb85fe..c5bbaac0df6 100644 --- 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 index fe34e4390d6..0f362dbf86b 100644 --- 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, ®ex::INVALID_REGEX, ®ex::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(®ex::INVALID_REGEX), LintId::of(®ex::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..af3c948ec82 --- /dev/null +++ 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 index e681f47f949..078924d3f9b 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1879,6 +1879,13 @@ pub static ref ALL_LINTS: Vec = 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 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 +