Finder excluded does NOT exclude relative directories

[raveren]

Symfony version(s) affected

5.4+

Description

I'm configuring php-cs-fixer which uses the symfony/finder component and noticed it does not work as advertised.

$finder = PhpCsFixer\Finder::create()
    ->in(__DIR__)
    ->exclude('storage')
;

Will exclude all files that have /storage/ in its path, such as "resources/lang/de/storage/index.php".

Looking at the source it's quite obvious:

        $iterator = new Iterator\RecursiveDirectoryIterator($dir, $flags, $this->ignoreUnreadableDirs);

        if ($exclude) {
            $iterator = new Iterator\ExcludeDirectoryFilterIterator($iterator, $exclude);
        }

https://github.com/symfony/symfony/blob/6.1/src/Symfony/Component/Finder/Finder.php#L735

and going inside:

        $this->isRecursive = $iterator instanceof \RecursiveIterator;
        // ...
		foreach ($directories as $directory) {
            $directory = rtrim($directory, '/');
            if (!$this->isRecursive || str_contains($directory, '/')) { // <---- $this->isRecursive is always true
                $patterns[] = preg_quote($directory, '#');
            } else {
                $this->excludedDirs[$directory] = true;
            }
        }

How to reproduce

Have dir structure

/ex/should-be-excluded.php
/not/ex/should-be-included.php
.php-cs-fixer.php

# .php-cs-fixer.php
$finder = PhpCsFixer\Finder::create()
    ->in(__DIR__)
    ->exclude('ex')
;

$a = [];
foreach ($finder as $item) {
    $a[] = $item->getRelativePathname();
}
saged($a, count($a));

Possible Solution

No response

Additional Context

symfony/finder in my vendors directory reports itself as v5.4

relevant docs:
https://symfony.com/doc/current/components/finder.html#location

[xabbuh]

@raveren Would you like to send a request to the documentation repository to update the docs accordingly?

[raveren]

Okay here's the full reproduceable file, need to run it on linux/wsl in a symfony/laravel root directory (next to vendor)

<?php

require __DIR__ . '/../vendor/autoload.php';

$dir = __DIR__;

exec("mkdir -p {$dir}/ex");
exec("mkdir -p {$dir}/not");
exec("mkdir -p {$dir}/not/ex");

exec("touch {$dir}/ex/should-be-excluded.php");
exec("touch {$dir}/not/ex/should-be-included.php");


$finder = Symfony\Component\Finder\Finder::create() // https://symfony.com/doc/current/components/finder.html
    ->in($dir)
    ->exclude('ex')
;

$parsedFiles = [];
foreach ($finder as $item) {
    $parsedFiles[] = $item->getRelativePathname();
}

dd(
    $parsedFiles,

    'this must be true:',
    file_exists("{$dir}/not/ex/should-be-included.php"),
    'this must be also true:',
    in_array("{$dir}/not/ex/should-be-included.php", $parsedFiles)
);

output:

The actually sad thing is, I can't figure out how to exclude a specific directory from Finder!

[alamirault]

I think updating documentation is not enough and there is a bug or bad behavior.

$finder = new Finder();
$finder
    ->in(__DIR__)
    ->exclude('foo');

_DIR_ content

.
├── bar/
│   ├── foo/
│   │   └── test-3.txt
│   └── test-2.txt
└── foo/
    └── test-1.txt

Actual result is

bar
bar/test-2.txt

I think expected result should be

bar
bar/test-2.txt
bar/foo/
bar/foo/test-3.txt

Finder exclude folder in all path and is not relative.

I think exclude MUST be relative and if we want exclude in all path, it must be explicit, WDYT ?

(If I'm not wrong, currently there is no way to exclude "foo" only at first level)

[carsonbot]

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

[raveren]

The reproduceable file is still wielding the same wrong results: when pasting the file contents into asd/sad.php and launching php sad.php:

There's no clean "workarounds", just highly-situation-specific hacks to produce expected behaviour

[carsonbot]

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

[carsonbot]

Hello? This issue is about to be closed if nobody replies.

[raveren]

That's a nice bot, here's a ⚙, good boy, carson!

The feature is still relevant and unsolved :)

[xabbuh]

Nothing will change that if nobody's gonna do that change though. PR welcome :)

[raveren]

The thing is, there is no concensus what must be done in this case, it's quite a big BC break if we just "fix" it. What do you think? Should a new, 'correctly' working method be added? I don't use Symfony, what is the regular policy on?

[joshtrichards]

This seems to be similar territory as #28158.

[joshtrichards]

The non-BC way suggested approach in #28158 was:

where paths starting with / would only exclude the files from the content root (->in() in our case)? If not starting with /, the behavior stays the same as today and will exclude any matching subdir.

[joshtrichards]

PR with a proposed implementation is in #54752 if anyone feels like testing/reviewing.