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

Avoid mutating the Finder when building the iterator #30313

Merged
merged 1 commit into from Feb 20, 2019

Conversation

Projects
None yet
4 participants
@stof
Copy link
Member

stof commented Feb 20, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

When excluding dot files or vcs files (which is done by default), the Finder object was mutated each time searchInDirectory was called to register the extra exclusions. This leads to registering them multiple times when the method is called multiple times (which happens either because you have multiple directories in ->in() or because you call getIterator multiple times, for instance because of using hasResults or count).

This mutation create bugs if the Finder object is reconfigured between the 2 calls to getIterator to disable some of these ignore rules, as they would already be registered in the other config properties. New tests have been added to reproduce these bugs and prevent regressions.

This mutation is now avoided by using a local array for the final configuration, preserving the user configuration.

@stof stof added the Finder label Feb 20, 2019

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Feb 20, 2019

@fabpot

fabpot approved these changes Feb 20, 2019

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Feb 20, 2019

Thank you @stof.

@fabpot fabpot merged commit 94989fe into symfony:3.4 Feb 20, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Feb 20, 2019

bug #30313 Avoid mutating the Finder when building the iterator (stof)
This PR was merged into the 3.4 branch.

Discussion
----------

Avoid mutating the Finder when building the iterator

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

When excluding dot files or vcs files (which is done by default), the Finder object was mutated each time `searchInDirectory` was called to register the extra exclusions. This leads to registering them multiple times when the method is called multiple times (which happens either because you have multiple directories in `->in()` or because you call `getIterator` multiple times, for instance because of using `hasResults` or `count`).

This mutation create bugs if the Finder object is reconfigured between the 2 calls to `getIterator` to disable some of these ignore rules, as they would already be registered in the other config properties. New tests have been added to reproduce these bugs and prevent regressions.

This mutation is now avoided by using a local array for the final configuration, preserving the user configuration.

Commits
-------

94989fe Avoid mutating the Finder when building the iterator

@stof stof deleted the stof:clean_finder branch Feb 20, 2019

This was referenced Mar 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.