Skip to content

Allow FilterEntry::filter_entry to accept a different predicate type - #214

Open
amitmishra11 wants to merge 1 commit into
BurntSushi:masterfrom
amitmishra11:fix-filter-entry-same-predicate-type
Open

Allow FilterEntry::filter_entry to accept a different predicate type#214
amitmishra11 wants to merge 1 commit into
BurntSushi:masterfrom
amitmishra11:fix-filter-entry-same-predicate-type

Conversation

@amitmishra11

Copy link
Copy Markdown

Fixes #130.

Bug

FilterEntry<I, P>::filter_entry accepted a new predicate of exactly
the same type P as the one already stored in the FilterEntry. Since
every Rust closure has a unique, anonymous type, this made it impossible
to chain two filter_entry calls with two different closures:

// error: expected closure [...], found a different closure
WalkDir::new(.)
    .into_iter()
    .filter_entry(|e| e.file_name() != foo)   // predicate type P1
    .filter_entry(|e| e.file_name() != bar)   // predicate type P2 -- rejected

Users were forced to combine the two predicates into one, or use boxed
closures / function pointers.

The second comment in #130 shows the confusing compiler error this
produces in practice.

Fix

Introduced a public SkipCurrentDir trait that abstracts the
skip_current_dir capability shared by IntoIter and FilterEntry.
Then:

  • The Iterator impl for FilterEntry<I, P> is generalized to work
    for any I: SkipCurrentDir instead of only IntoIter.
  • FilterEntry::filter_entry now introduces a fresh type parameter Q
    for the new predicate, returning FilterEntry<Self, Q>, matching the
    signature of IntoIter::filter_entry.

This is a minor semver-compatible addition (new public trait
SkipCurrentDir). The only theoretical breakage is using an empty
turbofish on FilterEntry::filter_entry::<> to observe the former lack
of type parameters, which is not a realistic usage pattern.

Test

Added filter_entry_chained in src/tests/recursive.rs that chains
two filter_entry calls with distinct closure types and verifies that
both predicates are applied correctly.

Previously FilterEntry<I, P>::filter_entry took a predicate of the same
type P as the outer FilterEntry, making it impossible to chain two
filter_entry calls with distinct closures. The compiler would reject
code like:

    WalkDir::new(.)
        .into_iter()
        .filter_entry(|e| e.file_name() != foo)
        .filter_entry(|e| e.file_name() != bar)

because the two closures are different types.

Fix this by:
  - Introducing a SkipCurrentDir trait that abstracts over IntoIter and
    FilterEntry so that the blanket Iterator impl for FilterEntry<I, P>
    can use a generic I.
  - Changing FilterEntry::filter_entry to introduce a fresh type
    parameter Q for the new predicate instead of reusing the outer P.
  - Generalising the Iterator and FusedIterator impls on FilterEntry
    to work over any I: SkipCurrentDir rather than only IntoIter.

Fixes BurntSushi#130.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FilterEntry::filter_entry has a suboptimal signature

1 participant