Skip to content

Commit

Permalink
bug #22985 [Config] Allow empty globs (nicolas-grekas)
Browse files Browse the repository at this point in the history
This PR was merged into the 3.3 branch.

Discussion
----------

[Config] Allow empty globs

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22950
| License       | MIT
| Doc PR        | -

This considers globs as valid even if they return no matches when they have a prefix (and when that prefix exists, according to `$ignoreErrors`).
This means there is an edge-case:
`*.abc` with at least one match is OK, but when it has no match, it falls back to regular import, then usually will fil.
But rewriting this to `./*.abc` resolves the ambiguity and turns this into a glob that won't fail if no matches are found.

This should provide the expected behavior in most cases (but ambiguous described one of course).

Commits
-------

c5b9c1a [Config] Allow empty globs
  • Loading branch information
fabpot committed May 31, 2017
2 parents e879945 + c5b9c1a commit b136719
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
17 changes: 11 additions & 6 deletions src/Symfony/Component/Config/Loader/FileLoader.php
Expand Up @@ -83,13 +83,18 @@ public function getLocator()
*/
public function import($resource, $type = null, $ignoreErrors = false, $sourceResource = null)
{
if (is_string($resource) && false !== strpbrk($resource, '*?{[')) {
if (is_string($resource) && strlen($resource) !== $i = strcspn($resource, '*?{[')) {
$ret = array();
foreach ($this->glob($resource, false, $_, true) as $path => $info) {
$ret[] = $this->doImport($path, $type, $ignoreErrors, $sourceResource);
$isSubpath = 0 !== $i && false !== strpos(substr($resource, 0, $i), '/');
foreach ($this->glob($resource, false, $_, $ignoreErrors || !$isSubpath) as $path => $info) {
if (null !== $res = $this->doImport($path, $type, $ignoreErrors, $sourceResource)) {
$ret[] = $res;
}
$isSubpath = true;
}
if ($ret) {
return count($ret) > 1 ? $ret : $ret[0];

if ($isSubpath) {
return isset($ret[1]) ? $ret : (isset($ret[0]) ? $ret[0] : null);
}
}

Expand All @@ -104,7 +109,7 @@ protected function glob($pattern, $recursive, &$resource = null, $ignoreErrors =
if (strlen($pattern) === $i = strcspn($pattern, '*?{[')) {
$prefix = $pattern;
$pattern = '';
} elseif (0 === $i) {
} elseif (0 === $i || false === strpos(substr($pattern, 0, $i), '/')) {
$prefix = '.';
$pattern = '/'.$pattern;
} else {
Expand Down
16 changes: 16 additions & 0 deletions src/Symfony/Component/Config/Tests/Loader/FileLoaderTest.php
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Config\Tests\Loader;

use PHPUnit\Framework\TestCase;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\Config\Loader\FileLoader;
use Symfony\Component\Config\Loader\LoaderResolver;

Expand Down Expand Up @@ -74,6 +75,21 @@ public function testImportWithGlobLikeResource()

$this->assertSame('[foo]', $loader->import('[foo]'));
}

public function testImportWithNoGlobMatch()
{
$locatorMock = $this->getMockBuilder('Symfony\Component\Config\FileLocatorInterface')->getMock();
$loader = new TestFileLoader($locatorMock);

$this->assertNull($loader->import('./*.abc'));
}

public function testImportWithSimpleGlob()
{
$loader = new TestFileLoader(new FileLocator(__DIR__));

$this->assertSame(__FILE__, strtr($loader->import('FileLoaderTest.*'), '/', DIRECTORY_SEPARATOR));
}
}

class TestFileLoader extends FileLoader
Expand Down

0 comments on commit b136719

Please sign in to comment.