BurntSushi / walkdir

Rust library for walking directories recursively.
The Unlicense
1.21k stars 106 forks source link

FilterEntry::filter_entry has a suboptimal signature #130

Open CAD97 opened 4 years ago

CAD97 commented 4 years ago
impl<P> FilterEntry<IntoIter, P> where P: FnMut(&DirEntry) -> bool {
    pub fn filter_entry(self, predicate: P) -> FilterEntry<Self, P> {
        FilterEntry { it: self, predicate: predicate }
    }
}

This requires the P for the filter_entry to be the same as the one already applied. So this requires a second application of filter_entry to use the same type as the first. This means it's not possible to call filter_entry on a WalkDir twice with two different closures; instead, function pointers (or boxed closures) need to be used instead.

I find it amusing it took so long to find this 😆 I guess 99.99% of use cases only need one filter. My workaround is to combine the two filters I was applying, though readability suffers for it.

The snippet in question ```rust let dir = WalkDir::new(manifest_dir.join(attr.dir.value())); let tests: Result, _> = dir .contents_first(true) .into_iter() .filter_entry(|entry: &DirEntry| entry.file_type().is_file()) .filter_entry(|entry: &DirEntry| { ext.is_none() || entry.path().extension().unwrap_or(OsStr::new("")) == &**ext.as_ref().unwrap() }) .collect(); ```

The better impl would give FilterEntry::filter_entry the same signature as IntoIter::filter_entry:

impl<P> FilterEntry<IntoIter, P> where P: FnMut(&DirEntry) -> bool {
    pub fn filter_entry<Q>(self, predicate: Q) -> FilterEntry<Self, Q>
    where Q: FnMut(&DirEntry) -> bool
    {
        FilterEntry { it: self, predicate: predicate }
    }
}

IIUC this is a "major breaking" change as someone could have used an empty turbofish (e.g. let _: fn(_, fn(&_) -> _) -> _ = walkdir::FilterEntry::filter_entry::<>;) to observe the lack of generic arguments.

BurntSushi commented 4 years ago

Aye, yeah, this is unfortunate. I'll mark this for walkdir 3. (I started on walkdir 3 mid last year. The big change is jettisoning std's read_dir implementation in favor of a home-grown one. But I ran out of steam.)

tv42 commented 3 years ago

I just stumbled on this. The error is most newbie-unfriendly. Adding it here as google fodder:

error[E0308]: mismatched types
  --> server/src/file_scanner.rs:23:27
   |
22 |               .filter_entry(|e| !is_hidden(e))
   |                             ----------------- the expected closure
23 |               .filter_entry(|entry: &DirEntry| match entry.file_name().to_str() {
   |  ___________________________^
24 | |                 None => {
25 | |                     log::warn!("ignoring non-UTF-8 filename: {:?}", entry.path());
26 | |                     false
27 | |                 }
28 | |                 Some(_) => true,
29 | |             })
   | |_____________^ expected closure, found a different closure
   |
   = note: expected closure `[closure@server/src/file_scanner.rs:22:27: 22:44]`
              found closure `[closure@server/src/file_scanner.rs:23:27: 29:14]`
   = note: no two closures, even if identical, have the same type
   = help: consider boxing your closure and/or using it as a trait object

error[E0599]: the method `filter_map` exists for struct `FilterEntry<FilterEntry<walkdir::IntoIter, [closure@server/src/file_scanner.rs:22:27: 22:44]>, [closure@server/src/file_scanner.rs:22:27: 22:44]>`, but its trait bounds were not satisfied
   --> server/src/file_scanner.rs:30:14
    |
30  |             .filter_map(|result| match result {
    |              ^^^^^^^^^^ method cannot be called on `FilterEntry<FilterEntry<walkdir::IntoIter, [closure@server/src/file_scanner.rs:22:27: 22:44]>, [closure@server/src/file_scanner.rs:22:27: 22:44]>` due to unsatisfied trait bounds
    | 
   ::: /home/tv/.cargo/registry/src/github.com-1ecc6299db9ec823/walkdir-2.3.1/src/lib.rs:991:1
    |
991 | pub struct FilterEntry<I, P> {
    | ---------------------------- doesn't satisfy `_: Iterator`
    |
    = note: the following trait bounds were not satisfied:
            `FilterEntry<FilterEntry<walkdir::IntoIter, [closure@server/src/file_scanner.rs:22:27: 22:44]>, [closure@server/src/file_scanner.rs:22:27: 22:44]>: Iterator`
            which is required by `&mut FilterEntry<FilterEntry<walkdir::IntoIter, [closure@server/src/file_scanner.rs:22:27: 22:44]>, [closure@server/src/file_scanner.rs:22:27: 22:44]>: Iterator`

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0308, E0423, E0599.
For more information about an error, try `rustc --explain E0308`.
error: could not compile `blah`