Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lint manual_str_repeat #7265

Merged
merged 2 commits into from
Jun 1, 2021
Merged

Add lint manual_str_repeat #7265

merged 2 commits into from
Jun 1, 2021

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented May 22, 2021

fixes: #7260

There's a similar function for slices. Should this be renamed to include it, or should that be a separate lint? If we are going to have them as one lint a better name will be needed. manual_repeat isn't exactly clear as it's replacing a call to iter::repeat.

changelog: Add new lint manual_str_repeat

@rust-highfive
Copy link

r? @giraffate

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 22, 2021
@Jarcho Jarcho force-pushed the manual_str_repeat branch 3 times, most recently from 2d894e8 to d6c3273 Compare May 22, 2021 21:40
@ghost
Copy link

ghost commented May 23, 2021

I tested this lint against a lot of crates and it worked perfectly.

Some of the type checks could be tightened up through because it can get confused by custom implementations of FromIterator. An example is below. I couldn't find an actual example of this in real code.

#![warn(clippy::manual_str_repeat)]

use core::iter;
use core::iter::FromIterator;

#[derive(Clone)]
struct S {}

impl FromIterator<Box<S>> for String {
    fn from_iter<I: IntoIterator<Item = Box<S>>>(_: I) -> String {
        "abc".to_string()
    }
}

fn main() {
    let _: String = iter::repeat(Box::new(S {})).take(3).collect();
    //Bad suggestion:
    //let _ = (&Box::new(S {})).repeat(3);
}

@Jarcho
Copy link
Contributor Author

Jarcho commented May 23, 2021

I forgot you could do that with Box. Fixed.

@Jarcho Jarcho force-pushed the manual_str_repeat branch from 1f57e21 to e99e947 Compare May 23, 2021 17:28
Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this lint to the lint documentation in clippy_lints/src/utils/conf.rs, and tests for MSRV?

clippy_lints/src/methods/manual_str_repeat.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/manual_str_repeat.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/manual_str_repeat.rs Outdated Show resolved Hide resolved
@giraffate giraffate added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels May 25, 2021
@Jarcho Jarcho force-pushed the manual_str_repeat branch from 7d1ae3c to d028d8b Compare May 27, 2021 14:13
@Jarcho
Copy link
Contributor Author

Jarcho commented May 27, 2021

What change do you want with the documentation?

@bors
Copy link
Contributor

bors commented May 30, 2021

☔ The latest upstream changes (presumably #7292) made this pull request unmergeable. Please resolve the merge conflicts.

@giraffate
Copy link
Contributor

What change do you want with the documentation?

Here:

/// Lint: CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE. The minimum rust version that the project supports

@Jarcho Jarcho force-pushed the manual_str_repeat branch 2 times, most recently from dfdc009 to 855b93b Compare May 31, 2021 03:31
@Jarcho Jarcho force-pushed the manual_str_repeat branch from 855b93b to cfddf09 Compare May 31, 2021 13:38
@giraffate
Copy link
Contributor

@bors r+

It looks good, thanks!

@bors
Copy link
Contributor

bors commented Jun 1, 2021

📌 Commit cfddf09 has been approved by giraffate

@bors
Copy link
Contributor

bors commented Jun 1, 2021

⌛ Testing commit cfddf09 with merge ca570f9...

@bors
Copy link
Contributor

bors commented Jun 1, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing ca570f9 to master...

@bors bors merged commit ca570f9 into rust-lang:master Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggest to change std::iter::repeat(s).take(n).collect::<String>() to s.repeat(n)
6 participants