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

There is no way to exclude path only from root directory #28158

Open
erickskrauch opened this issue Aug 8, 2018 · 28 comments · May be fixed by #54754
Open

There is no way to exclude path only from root directory #28158

erickskrauch opened this issue Aug 8, 2018 · 28 comments · May be fixed by #54754

Comments

@erickskrauch
Copy link

Symfony version(s) affected: 4.1.3

Description
I'm using PHP-CS-Fixer. It's using symfony/finder to create files list, that should be fixed. Some of my projects not using src/ directory for source files, so I need to write some exclude rules. For example:

<?php
$finder = \PhpCsFixer\Finder::create()
    ->in(__DIR__)
    ->exclude('data')
    ->exclude('docker')
    ->exclude('frontend')
    ->exclude('common/emails')
    ->exclude('common/templates')
    ->notPath('#.*/runtime#')
    ->notPath('#.*/views#')
    ->notPath('autocompletion.php')
    ->exclude('tests/codeception/_output')
    ->notPath('#tests/codeception/.*/_output#')
    ->notPath('#tests/codeception/.*/_support/_generated#')
    ->name('yii');

return \Ely\CS\Config::create()
    ->setFinder($finder);

Rule ->exclude('data') applies on any directory, so it excluding some nested folders:

api
├── config
│   ├── bootstrap.php
│   ├── config.php
│   └── params.php
├── controllers
│   ├── Controller.php
│   └── SiteController.php
├── data
│   └── test.php
├── models
├── runtime
├── views
└── web
    ├── assets
    ├── favicon.ico
    └── robots.txt
data
├── ...

In my case, the file api/data/test.php will not be fixed.

It's also strange, that ->exlude('autocompletion.php') not working, but ->notPath('autocompletion.php') works fine.

How to reproduce
Just create alternative folders structure and you will get it.

Possible Solution

  1. Make ->exclude() filter applying relatively to root folder, defined by ->in() method.
  2. Allow using absolute paths Finder exclude not working as expected with absolute paths #25945
  3. Document this behavior and add some workaround.
@nicolas-grekas
Copy link
Member

Would you like to work on it?

@erickskrauch
Copy link
Author

Yes, I can. I'll try to do PR on this weekend.

erickskrauch added a commit to erickskrauch/symfony that referenced this issue Sep 8, 2018
Apply PathFilterIterator only to directory root if path provided as non regex
@mnapoli
Copy link
Contributor

mnapoli commented Oct 15, 2018

We are seeing this bug too in Bref (brefphp/bref#51). #28410 fixes it but is considered a BC break: is it really considered a break if the behavior it's fixing is broken?

By "broken" I mean it's not respecting the original contract which is:

Directories passed as argument must be relative to the ones defined with the in() method.

(https://api.symfony.com/4.1/Symfony/Component/Finder/Finder.html#method_exclude)

@phil-davis
Copy link
Contributor

It's also strange, that ->exclude('autocompletion.php') not working, but ->notPath('autocompletion.php') works fine.

exclude() is just for directories
notPath() with just a file name will exclude just that file at the top level in() directory
notName() with a file name will exclude all occurrences of that file name in the directory tree

@devnix
Copy link

devnix commented Dec 27, 2018

I understand the whole argument about the behaviour of (for example) notPath(), but I still don't find the documentation well explained, nor the behaviour of the method.

For example: I'm building a custom gettext string scanner for my company with the Console component. Based on old .po files I'm getting the scan paths, and the exclusions, so I'm making an array with the following format:

$paths = [
    BASE_PATH.'/admin',
    BASE_PATH.'/public',
    '!admin/DontTranslate.php'
    '!admin/excluded_folder'
];

I'm iterating the array and getting the paths starting with an exclamation as exclusions. This would be the function that lists all the files I want to be scanned, iterating through the files and folders, taking in consideration the exclusions:

private function scanRecursively(array $items)
{
    $results = [];
    $exclusions = [];

    foreach ($items as $key => $pattern) {
        if ($pattern[0] === '!') {
            $exclusions[] = substr($pattern, 1);
            unset($items[$key]);
        }
    }

    foreach ($items as $key => $pattern) {
        $path = realpath($pattern);

        // Si el elemento es una ruta a un fichero directamente, añadimos a resultados.
        if ($path !== false && !is_dir($path)) {
            $results[] = $path;
            unset($items[$key]);
        }
    }

    $finder = new Finder();
    $finder->files()->in($items)->name('*.php')->name('*.js');

    foreach ($exclusions as $exclusion) {
        $finder->notPath($exclusion);
    }

    foreach ($finder as $file) {
        $results[] = $file->getPathname();
    }

    return $results;
}

The thing is: if I'm providing full paths to multiple inclusions, and relative paths for the exclusions ($finder->files()->in('/var/www/html/admin')->in('/var/www/html/public'), and $finder->notPath('admin/DontTranslate.php')), I find kinda logic to exclude the file or the path from the scan.

Right now, I'm forced to write the array like this:

$paths = [
    BASE_PATH.'/admin',
    BASE_PATH.'/public',
    '!DontTranslate.php'
    '!excluded_folder'
];

Which seems to work wonders, but if there is a DontTranslate.php file in /public that I don't want to be excluded, then I'm done.

matt-allan added a commit to matt-allan/laravel-code-style that referenced this issue Jul 13, 2019
The Symfony Finder component only allows excluding folders via relative
paths, meaning we can only exclude the `database/migrations` folder if
we exclude all folders named `migrations`. Instead we can skip including
the entire `database` path and only include the known subfolders we want
to format. See symfony/symfony#28158.
matt-allan added a commit to matt-allan/laravel-code-style that referenced this issue Jul 16, 2019
The Symfony Finder component only allows excluding folders via relative
paths, meaning we can only exclude the `database/migrations` folder if
we exclude all folders named `migrations`. Instead we can skip including
the entire `database` path and only include the known subfolders we want
to format. See symfony/symfony#28158.
@nicolas-grekas
Copy link
Member

I closed the linked PR, but I'm wondering: can't this be solved already by using a regexp?

@ogizanagi
Copy link
Member

@nicolas-grekas : I think the ExcludeDirectoryFilterIterator does not allow a regexp.

Still I agree the current behavior is misleading. A workaround is to use ->notPath, but the performances can be drastically affected.

Could it behave like .gitignore files, 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.

@c33s
Copy link

c33s commented Dec 12, 2020

->notPath nor ->exclude() it not working for me, both strip out deeper matches and don't accept prefixing vendor with / like /vendor to only match the relative "root" directory.

would call this more a bug than a feature request assuming that only the first level is removed can lead to data loss if this would be used in a backup call.

        $finder
            ->in(getcwd())
            ->notPath('vendor')
            ->notPath('tests')
        ;

where the directory structure is:

.git
.idea
.robo
src
	Bundle
	ParameterObjects
	vendor
		bar.txt
tests
vendor
.composer_version
.editorconfig
.gitignore
.gitlab-ci.yml
.php_cs.dist
.phpcs.xml.dist
codeception.yml
composer.json
composer.lock
composer.yaml
LICENSE
phpstan.neon
README.md
RoboFile.php
^ "codeception.yml"
^ "composer.json"
^ "composer.lock"
^ "composer.yaml"
^ "LICENSE"
^ "phpstan.neon"
^ "README.md"
^ "RoboFile.php"
^ "src"
^ "src\Bundle"
^ "src\Bundle\C33sParameterObjectsBundle.php"
^ "src\Bundle\DependencyInjection"
^ "src\Bundle\DependencyInjection\C33sParameterObjectsExtension.php"
^ "src\Bundle\Resources"
^ "src\Bundle\Resources\config"
^ "src\Bundle\Resources\config\services.yaml"
^ "src\ParameterObjects"
^ "src\ParameterObjects\KernelCacheDir.php"
^ "src\ParameterObjects\KernelCharset.php"
^ "src\ParameterObjects\KernelContainerClass.php"
^ "src\ParameterObjects\KernelDebug.php"
^ "src\ParameterObjects\KernelDefaultLocale.php"
^ "src\ParameterObjects\KernelEnvironment.php"
^ "src\ParameterObjects\KernelErrorController.php"
^ "src\ParameterObjects\KernelHttpMethodOverride.php"
^ "src\ParameterObjects\KernelLogsDir.php"
^ "src\ParameterObjects\KernelName.php"
^ "src\ParameterObjects\KernelProjectDir.php"
^ "src\ParameterObjects\KernelRootDir.php"
^ "src\ParameterObjects\KernelSecret.php"
^ "src\ParameterObjects\KernelTrustedHosts.php"
^ "src\ParameterObjects\Locale.php"
^ "src\ParameterObjects\Traits"
^ "src\ParameterObjects\Traits\ArrayValueObjectTrait.php"
^ "src\ParameterObjects\Traits\BooleanValueObjectTrait.php"
^ "src\ParameterObjects\Traits\StringValueObjectTrait.php"

so bar.txt is stripped out

src
	vendor
		bar.txt

currently the "workaround" is using ->ignoreVCSIgnored(true) (you can create a temporary .gitignore file with everything you want to ignore) as this allows to match /vendor/ and keeps bar.txt in my example

@bytestream
Copy link
Contributor

@c33s you should be able to use notPath('/^vendor/') which only strips out the top level vendor, and not those in sub directories?

@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@erickskrauch
Copy link
Author

Yep, I'm still here.

@carsonbot carsonbot removed the Stalled label Jul 9, 2021
@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@c33s
Copy link

c33s commented Jan 10, 2022

yes still looking for it

@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@carsonbot
Copy link

Just a quick reminder to make a comment on this. If I don't hear anything I'll close this.

@carsonbot carsonbot removed the Stalled label Aug 28, 2022
@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@carsonbot
Copy link

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

@devnix
Copy link

devnix commented Mar 16, 2023

This would be nice to have @carsonbot

@carsonbot carsonbot removed the Stalled label Mar 16, 2023
@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@c33s
Copy link

c33s commented Sep 23, 2023

@carsonbot yes

@carsonbot carsonbot removed the Stalled label Sep 23, 2023
@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@erickskrauch
Copy link
Author

@carsonbot, yep.

@carsonbot carsonbot removed the Stalled label Mar 24, 2024
@wouterj
Copy link
Member

wouterj commented Mar 24, 2024

Anyone wants to work on a PR that implements this in a non-BC breaking way? (e.g. maybe using #28158 (comment) suggestion of doing root-only matches when the path is prefixed by / or ./, similar to how gitignore works)

Otherwise, I'm afraid we'll have to still close this as Stalled. This issue has not received any updates for the past 3 years, which makes me feel that unless someone invests time into this, we'll be at the same state 3 years from now.

@c33s
Copy link

c33s commented Mar 24, 2024

@wouterj please keep it open, i will have a look at it in my summer holidays (currently my workload is too high).

@wouterj
Copy link
Member

wouterj commented Mar 25, 2024

Thanks @c33s. We'll leave this one open.
If there is no activity, Carson will remind us in 6 months again. But no pressure, if this gets closed as stalled, we can always reopen if there is activity again.

@joshtrichards
Copy link

This seems similar territory as #47431.

@joshtrichards
Copy link

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

phil-davis pushed a commit to phil-davis/symfony that referenced this issue Apr 27, 2024
phil-davis pushed a commit to phil-davis/symfony that referenced this issue Apr 27, 2024
phil-davis pushed a commit to phil-davis/symfony that referenced this issue Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.