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

Extend useless conversion #5631

Merged
merged 3 commits into from
May 27, 2020
Merged

Conversation

ThibsG
Copy link
Contributor

@ThibsG ThibsG commented May 22, 2020

This PR extends useless_conversion lint with TryFrom and TryInto

fixes: #5344

changelog: Extend useless_conversion with TryFrom and TryInto

@phansch phansch added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 22, 2020
@flip1995
Copy link
Member

r? @matthiaskrgr

@ThibsG ThibsG force-pushed the ExtendUselessConversion branch from a4c684a to 705bfdc Compare May 25, 2020 18:03
Copy link
Member

@matthiaskrgr matthiaskrgr left a comment

Choose a reason for hiding this comment

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

Hi,
I would also like to see these additional tests, just for some additional coverage:

    let _: String = "".to_owned().try_into().unwrap();
    // FIXME this is a false negative
    if String::from("a") == TryInto::<String>::try_into(String::from("a")).unwrap() {}
    let _: String = match String::from("_").try_into() {
        Ok(a) => a,
        Err(_) => "".into(),
    };

The TryInto::<String>::try_into(String::from("a")).unwrap() does currently not warn while it should, but it's not very important, we can just leave this as a note if someone wants to improve it further. :)

clippy_lints/src/useless_conversion.rs Outdated Show resolved Hide resolved
clippy_lints/src/useless_conversion.rs Outdated Show resolved Hide resolved
clippy_lints/src/useless_conversion.rs Outdated Show resolved Hide resolved
clippy_lints/src/useless_conversion.rs Outdated Show resolved Hide resolved
clippy_lints/src/useless_conversion.rs Outdated Show resolved Hide resolved
@matthiaskrgr matthiaskrgr 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 26, 2020
Copy link
Member

@matthiaskrgr matthiaskrgr left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks you!

@matthiaskrgr
Copy link
Member

matthiaskrgr commented May 27, 2020

@bors +

edit: oops

@matthiaskrgr matthiaskrgr added S-waiting-on-bors Status: The marked PR was approved and is only waiting bors and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels May 27, 2020
@matthiaskrgr
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 27, 2020

📌 Commit 1801841 has been approved by matthiaskrgr

@bors
Copy link
Contributor

bors commented May 27, 2020

⌛ Testing commit 1801841 with merge ee3088f...

@bors
Copy link
Contributor

bors commented May 27, 2020

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

@bors bors merged commit ee3088f into rust-lang:master May 27, 2020
@ThibsG ThibsG deleted the ExtendUselessConversion branch May 27, 2020 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: The marked PR was approved and is only waiting bors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lint identity_conversion could be extended to work with try_from/try_into
5 participants