Skip to content

Commit 1f57e21

Browse files
JarchoJarcho
committed
Fix type checks for manual_str_repeat 
1 parent d6c3273 commit 1f57e21

File tree

4 files changed

+86
-14
lines changed

4 files changed

+86
-14
lines changed

‎clippy_lints/src/methods/manual_str_repeat.rs

+12-4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use rustc_ast::LitKind;
88
use rustc_errors::Applicability;
99
use rustc_hir::{Expr, ExprKind, LangItem};
1010
use rustc_lint::LateContext;
11+
use rustc_middle::ty::{self, Ty};
1112
use rustc_span::symbol::{sym, Symbol};
1213

1314
use super::MANUAL_STR_REPEAT;
@@ -18,6 +19,14 @@ enum RepeatKind {
1819
Char,
1920
}
2021

22+
fn get_ty_param(ty: Ty<'_>) -> Option<Ty<'_>> {
23+
if let ty::Adt(_, subs) = ty.kind() {
24+
subs.types().next()
25+
} else {
26+
None
27+
}
28+
}
29+
2130
fn parse_repeat_arg(cx: &LateContext<'_>, e: &Expr<'_>) -> Option<RepeatKind> {
2231
if let ExprKind::Lit(lit) = &e.kind {
2332
match lit.node {
@@ -28,8 +37,8 @@ fn parse_repeat_arg(cx: &LateContext<'_>, e: &Expr<'_>) -> Option<RepeatKind> {
2837
} else {
2938
let ty = cx.typeck_results().expr_ty(e);
3039
if is_type_diagnostic_item(cx, ty, sym::string_type)
31-
|| is_type_lang_item(cx, ty, LangItem::OwnedBox)
32-
|| match_type(cx, ty, &paths::COW)
40+
|| (is_type_lang_item(cx, ty, LangItem::OwnedBox) && get_ty_param(ty).map_or(false, |ty| ty.is_str()))
41+
|| (match_type(cx, ty, &paths::COW) && get_ty_param(ty).map_or(false, |ty| ty.is_str()))
3342
{
3443
Some(RepeatKind::String)
3544
} else {
@@ -63,11 +72,10 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symbol,
6372
let count_snip = snippet_with_context(cx, take_arg.span, ctxt, "..", &mut app).0;
6473

6574
let val_str = match repeat_kind {
66-
RepeatKind::String => format!("(&{})", val_snip),
6775
RepeatKind::Str if !val_is_mac && repeat_arg.precedence().order() < PREC_POSTFIX => {
6876
format!("({})", val_snip)
6977
},
70-
RepeatKind::Str => val_snip.into(),
78+
RepeatKind::Str | RepeatKind::String => val_snip.into(),
7179
RepeatKind::Char if val_snip == r#"'"'"# => r#""\"""#.into(),
7280
RepeatKind::Char if val_snip == r#"'\''"# => r#""'""#.into(),
7381
RepeatKind::Char => format!("\"{}\"", &val_snip[1..val_snip.len() - 1]),

‎tests/ui/manual_str_repeat.fixed

+24-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
#![warn(clippy::manual_str_repeat)]
44

5-
use std::iter::repeat;
5+
use std::borrow::Cow;
6+
use std::iter::{repeat, FromIterator};
67

78
fn main() {
89
let _: String = "test".repeat(10);
@@ -28,4 +29,26 @@ fn main() {
2829
}
2930
// Don't lint, repeat is from a macro.
3031
let _: String = repeat_m!("test").take(count).collect();
32+
33+
let x: Box<str> = Box::from("test");
34+
let _: String = x.repeat(count);
35+
36+
#[derive(Clone)]
37+
struct S;
38+
impl FromIterator<Box<S>> for String {
39+
fn from_iter<T: IntoIterator<Item = Box<S>>>(_: T) -> Self {
40+
Self::new()
41+
}
42+
}
43+
// Don't lint, wrong box type
44+
let _: String = repeat(Box::new(S)).take(count).collect();
45+
46+
let _: String = Cow::Borrowed("test").repeat(count);
47+
48+
let x = "x".to_owned();
49+
let _: String = x.repeat(count);
50+
51+
let x = 'x';
52+
// Don't lint, not char literal
53+
let _: String = repeat(x).take(count).collect();
3154
}

‎tests/ui/manual_str_repeat.rs

+24-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
#![warn(clippy::manual_str_repeat)]
44

5-
use std::iter::repeat;
5+
use std::borrow::Cow;
6+
use std::iter::{repeat, FromIterator};
67

78
fn main() {
89
let _: String = std::iter::repeat("test").take(10).collect();
@@ -28,4 +29,26 @@ fn main() {
2829
}
2930
// Don't lint, repeat is from a macro.
3031
let _: String = repeat_m!("test").take(count).collect();
32+
33+
let x: Box<str> = Box::from("test");
34+
let _: String = repeat(x).take(count).collect();
35+
36+
#[derive(Clone)]
37+
struct S;
38+
impl FromIterator<Box<S>> for String {
39+
fn from_iter<T: IntoIterator<Item = Box<S>>>(_: T) -> Self {
40+
Self::new()
41+
}
42+
}
43+
// Don't lint, wrong box type
44+
let _: String = repeat(Box::new(S)).take(count).collect();
45+
46+
let _: String = repeat(Cow::Borrowed("test")).take(count).collect();
47+
48+
let x = "x".to_owned();
49+
let _: String = repeat(x).take(count).collect();
50+
51+
let x = 'x';
52+
// Don't lint, not char literal
53+
let _: String = repeat(x).take(count).collect();
3154
}

‎tests/ui/manual_str_repeat.stderr

+26-8
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,64 @@
11
error: manual implementation of `str::repeat` using iterators
2-
--> $DIR/manual_str_repeat.rs:8:21
2+
--> $DIR/manual_str_repeat.rs:9:21
33
|
44
LL | let _: String = std::iter::repeat("test").take(10).collect();
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"test".repeat(10)`
66
|
77
= note: `-D clippy::manual-str-repeat` implied by `-D warnings`
88

99
error: manual implementation of `str::repeat` using iterators
10-
--> $DIR/manual_str_repeat.rs:9:21
10+
--> $DIR/manual_str_repeat.rs:10:21
1111
|
1212
LL | let _: String = std::iter::repeat('x').take(10).collect();
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"x".repeat(10)`
1414

1515
error: manual implementation of `str::repeat` using iterators
16-
--> $DIR/manual_str_repeat.rs:10:21
16+
--> $DIR/manual_str_repeat.rs:11:21
1717
|
1818
LL | let _: String = std::iter::repeat('/'').take(10).collect();
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"'".repeat(10)`
2020

2121
error: manual implementation of `str::repeat` using iterators
22-
--> $DIR/manual_str_repeat.rs:11:21
22+
--> $DIR/manual_str_repeat.rs:12:21
2323
|
2424
LL | let _: String = std::iter::repeat('"').take(10).collect();
2525
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"/"".repeat(10)`
2626

2727
error: manual implementation of `str::repeat` using iterators
28-
--> $DIR/manual_str_repeat.rs:15:13
28+
--> $DIR/manual_str_repeat.rs:16:13
2929
|
3030
LL | let _ = repeat(x).take(count + 2).collect::<String>();
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `x.repeat(count + 2)`
3232

3333
error: manual implementation of `str::repeat` using iterators
34-
--> $DIR/manual_str_repeat.rs:21:21
34+
--> $DIR/manual_str_repeat.rs:22:21
3535
|
3636
LL | let _: String = repeat(m!("test")).take(m!(count)).collect();
3737
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `m!("test").repeat(m!(count))`
3838

3939
error: manual implementation of `str::repeat` using iterators
40-
--> $DIR/manual_str_repeat.rs:24:21
40+
--> $DIR/manual_str_repeat.rs:25:21
4141
|
4242
LL | let _: String = repeat(*x).take(count).collect();
4343
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(*x).repeat(count)`
4444

45-
error: aborting due to 7 previous errors
45+
error: manual implementation of `str::repeat` using iterators
46+
--> $DIR/manual_str_repeat.rs:34:21
47+
|
48+
LL | let _: String = repeat(x).take(count).collect();
49+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `x.repeat(count)`
50+
51+
error: manual implementation of `str::repeat` using iterators
52+
--> $DIR/manual_str_repeat.rs:46:21
53+
|
54+
LL | let _: String = repeat(Cow::Borrowed("test")).take(count).collect();
55+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `Cow::Borrowed("test").repeat(count)`
56+
57+
error: manual implementation of `str::repeat` using iterators
58+
--> $DIR/manual_str_repeat.rs:49:21
59+
|
60+
LL | let _: String = repeat(x).take(count).collect();
61+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `x.repeat(count)`
62+
63+
error: aborting due to 10 previous errors
4664

0 commit comments

Comments
 (0)