[Data Loader] [Data Locator] [Dependency Injection] Pass root paths to FileSystemLocator during construction

[rpkamp]

Q	A
Branch?	2.0
Bug fix?	no
New feature?	no
BC breaks?	yes
Deprecations?	no
Tests pass?	yes
Fixed tickets	discussion in #908 (comment)
License	MIT

The point of this PR is to require a fully instantiated Locator in FileSystemLoader.
Before, you would need to pass a LocatorInterface and rootpaths to the FilesystemLoader, and the FilesystemLoader would then pass the rootpaths to the LocatorInterface, making the FilesystemLoader aware of the inner workings of LocatorInterface which it should not be.

By requiring a LocatorInterface with rootpaths set, this knowledge about inner workings of the LocatorInterface is removed.

This also removed the need for setOptions on the LocatorInterface, which did not really belong the main purpose of the interface (locating files) anyway.

It also removed the dependency on OptionsResolver from FileSystemLocator since the rootpaths can now be passed directly to the constructor, making the rootpaths a hard dependency instead of an optional one - which was incorrect.

Update by @robfrawley: Ran php-cs-fixer using new style rules and added types to locator interface/class method signatures.

[robfrawley]

You need to create a PR against the 1.x branch with appropriate deprecations. Done in #932.

[robfrawley]

Remove unused use statements from FileSystemLocator, specifically:

Symfony\Component\OptionsResolver\Exception\ExceptionInterface;
Symfony\Component\OptionsResolver\OptionsResolver;

Using PHP CS Fixer would resolve this as well: #932 (review).

[robfrawley]

Aside from the requested changes, this looks great!

[rpkamp]

This PR has been open for a long time (since May of this year).
Anything I can do to advance the merging of this PR?

[robfrawley]

@rpkamp Your merge commit is very broken; please review. Better yet, don't use merges to re-align with upstream: use rebase! :-)

Merging is awfully error-prone, as it results in "other people's code" being merged into "your code", which causes you to handle merge conflicts for "other people's code". Inversely, rebasing results in "your code" being merged into "other people's code", causing all merge conflicts to be directly related to your code as it applies on top of the upstream HEAD.

This PR is planned for the 2.0.0 release (scheduled to be tagged March 12), assuming conflicts are resolved.

[rpkamp]

I know, I usually use rebasing, but I thought I'd use github's merge conflict editor instead this time. Turns out that doesn't work very well for me 😉

I'll trash that merge commit and do a manual rebase onto 2.0 instead.

[rpkamp]

Rebased against 2.0 and all tests are green again 😃

[robfrawley]

Our root namespace usages are resolved inline, ie use \ReflectionProperty when used and remove use statement.

[robfrawley]

Thanks @rpkamp!