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

Finder name glob does not behave as documented #42675

Closed
Bilge opened this issue Aug 21, 2021 · 12 comments
Closed

Finder name glob does not behave as documented #42675

Bilge opened this issue Aug 21, 2021 · 12 comments

Comments

@Bilge
Copy link
Contributor

Bilge commented Aug 21, 2021

Symfony version(s) affected: 5.3.6

Description
According to the Finder::name() docblock, the following two calling conventions are equivalent:

* $finder->name('*.php')
* $finder->name('/\.php$/') // same as above

However, this is demonstrably false for dot-files.

How to reproduce

Given the file .foo.php and $finder is called with ignoreDotFiles(false):

  • $finder->name('*.php') will not match.
  • $finder->name('/\.php$/') will match.

This is because *.php is actually translated to the following regular expression: #^(?=[^\.])[^/]*\.php$#. The assertion prevents the file starting with a dot (.). (Aside: that assertion should probably have been written as (?!\.) for clarity).

It is unclear what is the intended behaviour here. By default, shells like Bash will not match dotfiles with a glob like this, although they can be set to match dot-files (with shopt -s dotglob). In any case, Finder's own documentation claims that the glob should match because the glob is equivalent to a suffix-only match, failing to mention that dot-files are implicitly excluded from such globs.

Possible Solution
Either:

  • Allow globs to match dot-files.
  • Change the documentation to stop pretending the glob and simple suffix-only regex are equivalent.

My view is that, from the perspective of Finder, the only correct solution can be to allow globs to match dot-files, because Finder has its own way of excluding dot-files (with ignoreDotFiles()). But then we are stuck with the problem that fixing the issue is a breaking change within Finder.

@Bilge Bilge added the Bug label Aug 21, 2021
@Bilge Bilge changed the title Finder name glob does not behave as expected Finder name glob does not behave as documented Aug 21, 2021
@xabbuh xabbuh added the Finder label Sep 9, 2021
@carsonbot
Copy link

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
Copy link

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

@carsonbot carsonbot removed the Stalled label Mar 24, 2022
@stof
Copy link
Member

stof commented Mar 24, 2022

I suggest updating the doc to explain the current behavior instead of doing a BC break

@carsonbot
Copy link

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?

@Bilge
Copy link
Contributor Author

Bilge commented Oct 29, 2022

Still relevant.

@carsonbot carsonbot removed the Stalled label Oct 29, 2022
@VincentLanglet
Copy link
Contributor

I suggest updating the doc to explain the current behavior instead of doing a BC break

Calling this behavior a BC break is debatable.
I just spend half an hour to understand why

Finder::create()->files()->name('*.twig')->ignoreDotFiles(false);

as expected.

Changing the behavior

  • Won't change anything for people writing things like Finder::create()->files()->name('*.twig'), because by default ignoring dot files is set to true.
  • Will allow code like Finder::create()->files()->name('*.twig')->ignoreDotFiles(false) to NOT ignore dot files, which is exactly what people wants.

To me, it could be considered as a bugfix.

@carsonbot
Copy link

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?

@VincentLanglet
Copy link
Contributor

Still relevant

@carsonbot carsonbot removed the Stalled label Jul 4, 2023
@nicolas-grekas
Copy link
Member

The phpdoc should be changed. *.php shouldn't match dot-files. PR welcome.

@VincentLanglet
Copy link
Contributor

I opened #50921 then

nicolas-grekas added a commit that referenced this issue Jul 13, 2023
This PR was merged into the 5.4 branch.

Discussion
----------

[Finder] Fix phpdoc about glob syntax

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes ? no ? just phpdoc
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #42675
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

Closes #42675 cc `@nicolas`-grekas

Commits
-------

2340a75 Fix Finder phpdoc
@Bilge
Copy link
Contributor Author

Bilge commented Jul 13, 2023

When a bug becomes a feature.

@VincentLanglet
Copy link
Contributor

When a bug becomes a feature.

In php glob('*') doesn't return the dot files ; glob('.*') does

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants