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

adding condition for map_clone message #8688

Merged
merged 2 commits into from
Apr 12, 2022

Conversation

kyoto7250
Copy link
Contributor

This PR fixes the message about map_clone.

if msrv >= 1.36, the message is correct.

$ cat main.rs 
fn main() {
  let x: Vec<&i32> = vec![&1, &2];
  let y: Vec<_>  = x.iter().map(|i| *i).collect();
  println!("{:?}", y);
}

$ cargo clippy
warning: you are using an explicit closure for copying elements
 --> main.rs:3:20
  |
3 |   let y: Vec<_>  = x.iter().map(|i| *i).collect();
  |                    ^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.iter().copied()`
  |
  = note: `#[warn(clippy::map_clone)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_clone

warning: `test` (build script) generated 1 warning
warning: `test` (bin "test") generated 1 warning (1 duplicate)
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s

but, if msrv < 1.36, the suggestion is cloned, but the message is copying.

$ cat clippy.toml 
msrv = "1.35"
 
$ cargo clippy
warning: you are using an explicit closure for copying elements
 --> main.rs:3:20
  |
3 |   let y: Vec<_>  = x.iter().map(|i| *i).collect();
  |                    ^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.iter().cloned()`

I think the separation of messages will make it more user-friendly.

thank you in advance.

changelog: Fixed a message in map_clone.

if msrv < 1.36, the message tells , but the suggestion is
@rust-highfive
Copy link

r? @llogiq

(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 Apr 11, 2022
@@ -143,7 +143,7 @@ fn lint_needless_cloning(cx: &LateContext<'_>, root: Span, receiver: Span) {
impl MapClone {
fn lint_explicit_closure(&self, cx: &LateContext<'_>, replace: Span, root: Span, is_copy: bool) {
let mut applicability = Applicability::MachineApplicable;
let message = if is_copy {
let message = if is_copy && meets_msrv(self.msrv.as_ref(), &msrvs::ITERATOR_COPIED) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nits: Since it's the same as sugg_method, so it would be better to put them into one like this:

        let (message, sugg_method) = if is_copy && meets_msrv(self.msrv.as_ref(), &msrvs::ITERATOR_COPIED) {
            ("you are using an explicit closure for copying elements", "copied")
        } else {
            ("you are using an explicit closure for cloning elements", "cloned")
        };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This way is more cooler.
Thank you!

@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Apr 12, 2022

📌 Commit 40224f4 has been approved by giraffate

@bors
Copy link
Collaborator

bors commented Apr 12, 2022

⌛ Testing commit 40224f4 with merge bc069ef...

@bors
Copy link
Collaborator

bors commented Apr 12, 2022

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

@bors bors merged commit bc069ef into rust-lang:master Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants