Skip to content

Commit

Permalink
Add a lint for .repeat(1)
Browse files Browse the repository at this point in the history
  • Loading branch information
giraffate committed Jul 7, 2020
1 parent ac85692 commit 5307cb5
Show file tree
Hide file tree
Showing 7 changed files with 211 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
126 changes: 126 additions & 0 deletions clippy_lints/src/repeat_once.rs
Original file line number Diff line number Diff line change
@@ -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,
}
}
7 changes: 7 additions & 0 deletions src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/repeat_once.fixed
Original file line number Diff line number Diff line change
@@ -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();
}
16 changes: 16 additions & 0 deletions tests/ui/repeat_once.rs
Original file line number Diff line number Diff line change
@@ -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);
}
40 changes: 40 additions & 0 deletions tests/ui/repeat_once.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 5307cb5

Please sign in to comment.