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

search_is_some fixit hint suggests invalid code #4033

Closed
matthiaskrgr opened this issue Apr 25, 2019 · 1 comment · Fixed by #4049
Closed

search_is_some fixit hint suggests invalid code #4033

matthiaskrgr opened this issue Apr 25, 2019 · 1 comment · Fixed by #4049
Labels
good-first-issue These issues are a good way to get started with Clippy I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@matthiaskrgr
Copy link
Member

clippy 0.0.212 (9897442 2019-04-23)

in this code

    // https://docs.rs/walkdir/2.2.7/walkdir/#example-skip-hidden-files-and-directories-on-unix
    fn is_git(entry: &walkdir::DirEntry) -> bool {
        entry
            .path()
            .components()
            .find(|&path_elm| path_elm == std::path::Component::Normal(OsStr::new(".git")))
            .is_some()
    }

clippy suggests:

warning: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
   --> src/main.rs:352:9
    |
352 | /         entry
353 | |             .path()
354 | |             .components()
355 | |             .find(|&path_elm| path_elm == std::path::Component::Normal(OsStr::new(".git")))
356 | |             .is_some()
    | |______________________^
    |
    = note: #[warn(clippy::search_is_some)] on by default
    = note: replace `find(|&path_elm| path_elm == std::path::Component::Normal(OsStr::new(".git"))).is_some()` with `any(|&path_elm| path_elm == std::path::Component::Normal(OsStr::new(".git")))`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some

if I apply the suggestion

    fn is_git(entry: &walkdir::DirEntry) -> bool {
        entry
            .path()
            .components()
            .any(|&path_elm| path_elm == std::path::Component::Normal(OsStr::new(".git")))
    }

the code no longer compiles

    Checking raca v0.1.0 (/home/matthias/vcs/github/raca)
error[E0308]: mismatched types
   --> src/main.rs:355:19
    |
355 |             .any(|&path_elm| path_elm == std::path::Component::Normal(OsStr::new(".git")))
    |                   ^^^^^^^^^ expected enum `std::path::Component`, found reference
    |
    = note: expected type `std::path::Component<'_>`
               found type `&_`
    = help: did you mean `path_elm: &std::path::Component<'_>`?

error: aborting due to previous error

The correct code is

.any(|path_elm| ...

instead of

.any(|&path_elm| ...
@matthiaskrgr matthiaskrgr added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Apr 25, 2019
@flip1995
Copy link
Member

FnDecl of any:

fn any<F>(&mut self, f: F) -> bool 
    where F: FnMut(Self::Item) -> bool,

FnDecl of find:

fn find<P>(&mut self, predicate: P) -> Option<Self::Item> 
    where P: FnMut(&Self::Item) -> bool, 

The closure of findtakes the element by ref, while the closure of any takes the element by value. So this should be easily fixable by checking if we match on |&_| in find and if so only use |_| in the suggestion.

@flip1995 flip1995 added L-suggestion Lint: Improving, adding or fixing lint suggestions good-first-issue These issues are a good way to get started with Clippy labels Apr 26, 2019
bors added a commit that referenced this issue May 2, 2019
Fix #4033 search_is_some

Fixes #4033.

Suggest `any(|x| ..)` instead of `any(|&x| ..)` for `find(|&x| ..).is_some()` (Lint [search_is_some](https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some))

FnDecl of `find`:

```rust
fn find<P>(&mut self, mut p: P) -> Option<Self::Item> where
    P: FnMut(&Self::Item) -> bool
```

FnDecl of `any`:

```rust
fn any<F>(&mut self, mut f: F) -> bool where
    F: FnMut(Self::Item) -> bool
```

If match on `|&_|` in closure of `find`, only use `|_|` in the suggestion.

PS. It's the first time that I have used the `hir` API, please correct me if there is any mistake 😺
@bors bors closed this as completed in #4049 May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue These issues are a good way to get started with Clippy I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants