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

bad get_unwrap suggestion #3006

Closed
isonmad opened this issue Aug 5, 2018 · 10 comments
Closed

bad get_unwrap suggestion #3006

isonmad opened this issue Aug 5, 2018 · 10 comments
Labels
C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. good-first-issue These issues are a good way to get started with Clippy

Comments

@isonmad
Copy link

isonmad commented Aug 5, 2018

let v = vec!["a","b"];
let _: Vec<_> = v.get(0..1).unwrap().to_vec();
//              ^^^^^^^^^^^^^^^^^^^^ help: try this: `&v[0..1]`

The suggestion changes the result to &Vec instead of Vec, so it doesn't compile. It has to be v[0..1] or (&v[0..1]) instead.

@phansch phansch added the C-bug Category: Clippy is not doing the correct thing label Aug 5, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Aug 6, 2018

We should

  1. use the Sugg suggestion builder (easy)
  2. optimize Sugg to produce v[foo].bar() instead of (&v[foo]).bar() (medium difficulty)

@oli-obk oli-obk added good-first-issue These issues are a good way to get started with Clippy E-medium Call for participation: Medium difficulty level problem and requires some initial experience. labels Aug 6, 2018
@ms2300
Copy link
Contributor

ms2300 commented Aug 27, 2018

Can I take a shot at this issue?

@oli-obk
Copy link
Contributor

oli-obk commented Aug 27, 2018

It's all yours. Don't hesitate to ask here, on irc or on discord if you have any questions or just open a PR with any (no matter how unfinished) code that you already have.

@ms2300
Copy link
Contributor

ms2300 commented Aug 27, 2018

Awesome thank you, I'll let you know if I have any questions

@ms2300
Copy link
Contributor

ms2300 commented Aug 30, 2018

Sorry for this ticket taking a bit longer than I thought, haven't had much time to concentrate. Besides the modified / new test, the fi should probably be in clippy_lints/src/utils/sugg.rs right?

Edit: fi -> fix, I can type 95% of the time I promise

@oli-obk
Copy link
Contributor

oli-obk commented Aug 31, 2018

"fi"?

You should only have to modify sugg.rs for the second part of this issue. The first one should just use the Sugg type.

@ms2300
Copy link
Contributor

ms2300 commented Aug 31, 2018

Cool, I'll have a fix soon. Thanks for the point in the right direction!

@ms2300
Copy link
Contributor

ms2300 commented Sep 9, 2018

I haven't made any progress on this ticket b/c I honestly don't fundamentally understand how to make the modification. I'm confused as to what part of the code actually recommends the fix for this problem and therefore don't even know where to begin :/

@oli-obk
Copy link
Contributor

oli-obk commented Sep 10, 2018

ms2300 added a commit to ms2300/rust-clippy that referenced this issue Sep 13, 2018
ms2300 added a commit to ms2300/rust-clippy that referenced this issue Sep 16, 2018
ms2300 added a commit to ms2300/rust-clippy that referenced this issue Sep 16, 2018
ms2300 added a commit to ms2300/rust-clippy that referenced this issue Sep 17, 2018
ms2300 added a commit to ms2300/rust-clippy that referenced this issue Sep 19, 2018
ms2300 added a commit to ms2300/rust-clippy that referenced this issue Sep 24, 2018
@CYBAI
Copy link
Contributor

CYBAI commented Oct 26, 2018

If #3178 is merged, maybe this can be closed?

@oli-obk oli-obk closed this as completed Oct 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

No branches or pull requests

6 participants