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

[1.x] Replace GLOB_BRACE with a more robust solution #2064

Merged
merged 60 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
d6daf01
Add a rule that ensures we don't use `GLOB_BRACE`
caendesilva Dec 11, 2024
d3ec04c
Test recursive discovery
caendesilva Dec 12, 2024
dab8680
Draft new method for improved file finding
caendesilva Dec 13, 2024
f942840
Create testing helper to add several files at once
caendesilva Dec 13, 2024
7ee8393
Create FilesystemFindFilesTest.php
caendesilva Dec 13, 2024
4f73bda
Move high level tests to feature test
caendesilva Dec 13, 2024
b7389dc
Update signature to put extension first
caendesilva Dec 13, 2024
ea06af8
Implement the new file finder method
caendesilva Dec 13, 2024
ca4c0a2
Apply fixes from StyleCI
caendesilva Dec 13, 2024
beb6c72
Test edge cases
caendesilva Dec 13, 2024
077bb2c
Fix parameter order and add facade tests
caendesilva Dec 13, 2024
9da4382
Match extensions case insensitivly
caendesilva Dec 14, 2024
f6bbd94
Normalize input by removing a leading dot if present
caendesilva Dec 14, 2024
01af8d5
Sort the values to normalize between filesystems
caendesilva Dec 14, 2024
be7abcc
Cleanup and refactor the code
caendesilva Dec 14, 2024
3485774
Collect iterator directly
caendesilva Dec 14, 2024
2c3786d
Clean up the method
caendesilva Dec 14, 2024
edb397c
Inline local variable
caendesilva Dec 14, 2024
edbadae
Document the method
caendesilva Dec 14, 2024
2825c84
Return paths relative to the project root
caendesilva Dec 14, 2024
71773ff
Update the file collection class to use better file finder
caendesilva Dec 14, 2024
8cb7282
Files can now be discovered of any recursive depth
caendesilva Dec 14, 2024
4e3a4ae
Support multiple file extensions for file finder
caendesilva Dec 14, 2024
9ae2e24
Update data collection class to use better file finder feature
caendesilva Dec 14, 2024
22c09f7
Handle side effect in v1 branch
caendesilva Dec 14, 2024
f68b106
Don't throw if the directory does not exist
caendesilva Dec 14, 2024
6bae95b
Revert "Handle side effect in v1 branch"
caendesilva Dec 14, 2024
cf12d59
Apply fixes from StyleCI
StyleCIBot Dec 14, 2024
95592c6
Refactor test to not test implementations
caendesilva Dec 14, 2024
8423ea2
Refactor media file discovery to use improved file finder
caendesilva Dec 14, 2024
1212d6a
Fix test not testing the documented behavior
caendesilva Dec 14, 2024
c61ba27
Support comma separated extensions input
caendesilva Dec 14, 2024
a0e93c4
Custom media file extensions now take string config input
caendesilva Dec 14, 2024
04526e6
Apply fixes from StyleCI
StyleCIBot Dec 14, 2024
d6a26e5
Add refactoring todo
caendesilva Dec 14, 2024
e6ec49b
Move facade tests to main test
caendesilva Dec 14, 2024
2e9c404
Remove test link
caendesilva Dec 14, 2024
5971948
Extract internal class for file finder helper
caendesilva Dec 14, 2024
3d0993a
Add missing test cleanup
caendesilva Dec 14, 2024
935ab77
Dependency inject the finder so we can mock it in tests
caendesilva Dec 14, 2024
b420f7a
Improve test mocks
caendesilva Dec 15, 2024
9090e0f
Cleanup the code
caendesilva Dec 14, 2024
1e1b8a3
Clean up and merge tests
caendesilva Dec 14, 2024
3dc3383
Use better file finder instead of glob brace
caendesilva Dec 15, 2024
472d29f
Annotate the parameter type
caendesilva Dec 15, 2024
665340f
Use explicit type conversion
caendesilva Dec 15, 2024
104eb76
Explicitly annotate type as Psalm is not reading stub well
caendesilva Dec 15, 2024
cf70b97
Annotate full finder types
caendesilva Dec 15, 2024
bb4f839
Revert "Annotate full finder types"
caendesilva Dec 15, 2024
1cd16f3
Extract helper method
caendesilva Dec 15, 2024
ada1f4b
Expand mixed comma separated strings and array input
caendesilva Dec 15, 2024
be16f73
Merge code by casting to array
caendesilva Dec 15, 2024
f0b85d9
Add earlier array cast
caendesilva Dec 15, 2024
396b83d
Type the closure values
caendesilva Dec 15, 2024
3ae5df0
Expand array map shorthand due to Psalm not understanding it
caendesilva Dec 15, 2024
2aeabf7
Refactor and cleanup
caendesilva Dec 15, 2024
96ba073
Annotate return types
caendesilva Dec 15, 2024
3ef777e
Annotate type for Psalm
caendesilva Dec 15, 2024
7a86073
Clarify internal policy
caendesilva Dec 15, 2024
b41253e
Update RELEASE_NOTES.md
caendesilva Dec 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ This serves two purposes:
- Registered the `cache:clear` command to make it easier to clear the cache in https://github.com/hydephp/develop/pull/1881
- Added a new `Hyperlinks::isRemote()` helper method to check if a URL is remote in https://github.com/hydephp/develop/pull/1882
- All page types now support the `description` front matter field (used in page metadata) in https://github.com/hydephp/develop/pull/1884
- Added a new `Filesystem::findFiles()` method to find files in a directory in https://github.com/hydephp/develop/pull/2064

### Changed
- Changed the `Hyde` facade to use a `@mixin` annotation instead of single method annotations in https://github.com/hydephp/develop/pull/1919
Expand All @@ -24,6 +25,7 @@ This serves two purposes:
- The `torchlight:install` command is now hidden from the command list as it's already installed in https://github.com/hydephp/develop/pull/1879
- Updated the home page fallback link in the 404 template to lead to the site root in https://github.com/hydephp/develop/pull/1880 (fixes https://github.com/hydephp/develop/issues/1781)
- Normalized remote URL checks so that protocol relative URLs `//` are consistently considered to be remote in all places in https://github.com/hydephp/develop/pull/1882 (fixes https://github.com/hydephp/develop/issues/1788)
- Replaced internal usages of glob functions with our improved file finder in https://github.com/hydephp/develop/pull/2064
- Updated to HydeFront v3.4 in https://github.com/hydephp/develop/pull/1803
- Realtime Compiler: Virtual routes are now managed through the service container in https://github.com/hydephp/develop/pull/1858
- Realtime Compiler: Improved the dashboard layout in https://github.com/hydephp/develop/pull/1866
Expand All @@ -44,6 +46,8 @@ This serves two purposes:
- Fixed routing issues with nested 404 pages where an index page does not exist https://github.com/hydephp/develop/issues/1781 in https://github.com/hydephp/develop/pull/1880
- Fixed URL metadata for blog posts not using customized post output directories in https://github.com/hydephp/develop/pull/1889
- Improved printed documentation views in https://github.com/hydephp/develop/pull/2005
- Fixed "BuildService finding non-existent files to copy in Debian" https://github.com/hydephp/framework/issues/662 in https://github.com/hydephp/develop/pull/2064
- Fixed "Undefined constant `Hyde\Foundation\Kernel\GLOB_BRACE`" https://github.com/hydephp/hyde/issues/270 in https://github.com/hydephp/develop/pull/2064
- Realtime Compiler: Updated the exception handler to match HTTP exception codes when sending error responses in https://github.com/hydephp/develop/pull/1853
- Realtime Compiler: Improved routing for nested index pages in https://github.com/hydephp/develop/pull/1852
- Realtime Compiler: Improved the dashboard https://github.com/hydephp/develop/pull/1866 fixing https://github.com/hydephp/realtime-compiler/issues/22 and https://github.com/hydephp/realtime-compiler/issues/29
Expand Down
22 changes: 22 additions & 0 deletions monorepo/HydeStan/HydeStan.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ final class HydeStan
private const FILE_ANALYSERS = [
NoFixMeAnalyser::class,
UnImportedFunctionAnalyser::class,
NoGlobBraceAnalyser::class,
];

private const TEST_FILE_ANALYSERS = [
Expand Down Expand Up @@ -370,6 +371,27 @@ public function run(string $file, string $contents): void
}
}

class NoGlobBraceAnalyser extends FileAnalyser
{
public function run(string $file, string $contents): void
{
$lines = explode("\n", $contents);

foreach ($lines as $lineNumber => $line) {
AnalysisStatisticsContainer::analysedExpression();

if (str_contains($line, 'GLOB_BRACE')) {
$this->fail(sprintf('Usage of `GLOB_BRACE` found in %s at line %d. This feature is not supported on all systems and should be avoided.',
realpath(BASE_PATH.'/'.$file),
$lineNumber + 1
));

HydeStan::addActionsMessage('error', $file, $lineNumber + 1, 'HydeStan: NoGlobBraceError', '`GLOB_BRACE` is not supported on all systems. Consider refactoring to avoid it.');
}
}
}
}

class NoTestReferenceAnalyser extends LineAnalyser
{
public function run(string $file, int $lineNumber, string $line): void
Expand Down
15 changes: 15 additions & 0 deletions packages/framework/src/Facades/Filesystem.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,21 @@ public static function smartGlob(string $pattern, int $flags = 0): Collection
return self::kernel()->filesystem()->smartGlob($pattern, $flags);
}

/**
* Find files in the project's directory, with optional filtering by extension and recursion.
*
* The returned collection will be a list of paths relative to the project root.
*
* @param string $directory
* @param string|array<string>|false $matchExtensions The file extension(s) to match, or false to match all files.
* @param bool $recursive Whether to search recursively or not.
* @return \Illuminate\Support\Collection<int, string>
*/
public static function findFiles(string $directory, string|array|false $matchExtensions = false, bool $recursive = false): Collection
{
return self::kernel()->filesystem()->findFiles($directory, $matchExtensions, $recursive);
}

/**
* Touch one or more files in the project's directory.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ protected function runExtensionHandlers(): void
protected function discoverFilesFor(string $pageClass): void
{
// Scan the source directory, and directories therein, for files that match the model's file extension.
foreach (Filesystem::smartGlob($pageClass::sourcePath('{*,**/*}'), GLOB_BRACE) as $path) {
foreach (Filesystem::findFiles($pageClass::sourceDirectory(), $pageClass::fileExtension(), true) as $path) {
if (! str_starts_with(basename((string) $path), '_')) {
$this->addFile(SourceFile::make($path, $pageClass));
}
Expand Down
13 changes: 13 additions & 0 deletions packages/framework/src/Foundation/Kernel/Filesystem.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Hyde\Foundation\HydeKernel;
use Hyde\Foundation\PharSupport;
use Illuminate\Support\Collection;
use Hyde\Framework\Actions\Internal\FileFinder;

use function collect;
use function Hyde\normalize_slashes;
Expand Down Expand Up @@ -183,4 +184,16 @@ public function smartGlob(string $pattern, int $flags = 0): Collection

return $files->map(fn (string $path): string => $this->pathToRelative($path));
}

/**
* @param string|array<string>|false $matchExtensions
* @return \Illuminate\Support\Collection<int, string>
*/
public function findFiles(string $directory, string|array|false $matchExtensions = false, bool $recursive = false): Collection
{
/** @var \Hyde\Framework\Actions\Internal\FileFinder $finder */
$finder = app(FileFinder::class);

return $finder->handle($directory, $matchExtensions, $recursive);
}
}
66 changes: 66 additions & 0 deletions packages/framework/src/Framework/Actions/Internal/FileFinder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

declare(strict_types=1);

namespace Hyde\Framework\Actions\Internal;

use Hyde\Facades\Filesystem;
use Hyde\Hyde;
use Illuminate\Support\Collection;
use SplFileInfo;
use Symfony\Component\Finder\Finder;

/**
* @interal This class is used internally by the framework and is not part of the public API, unless that is requested on GitHub with a valid use case.
*/
class FileFinder
{
/**
* @param array<string>|string|false $matchExtensions
* @return \Illuminate\Support\Collection<int, string>
*/
public static function handle(string $directory, array|string|false $matchExtensions = false, bool $recursive = false): Collection
{
if (! Filesystem::isDirectory($directory)) {
return collect();
}

$finder = Finder::create()->files()->in(Hyde::path($directory));

if ($recursive === false) {
$finder->depth('== 0');
}

if ($matchExtensions !== false) {
$finder->name(static::buildFileExtensionPattern((array) $matchExtensions));
}

return collect($finder)->map(function (SplFileInfo $file): string {
return Hyde::pathToRelative($file->getPathname());
})->sort()->values();
}

/** @param array<string> $extensions */
protected static function buildFileExtensionPattern(array $extensions): string
{
$extensions = self::expandCommaSeparatedValues($extensions);

return '/\.('.self::normalizeExtensionForRegexPattern($extensions).')$/i';
}

/** @param array<string> $extensions */
private static function expandCommaSeparatedValues(array $extensions): array
{
return array_merge(...array_map(function (string $item): array {
return array_map(fn (string $item): string => trim($item), explode(',', $item));
}, $extensions));
}

/** @param array<string> $extensions */
private static function normalizeExtensionForRegexPattern(array $extensions): string
{
return implode('|', array_map(function (string $extension): string {
return preg_quote(ltrim($extension, '.'), '/');
}, $extensions));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use Hyde\Framework\Features\BuildTasks\PreBuildTask;

use function basename;
use function glob;
use function in_array;
use function sprintf;

Expand All @@ -21,7 +20,7 @@ class CleanSiteDirectory extends PreBuildTask
public function handle(): void
{
if ($this->isItSafeToCleanOutputDirectory()) {
Filesystem::unlink(glob(Hyde::sitePath('*.{html,json}'), GLOB_BRACE));
Filesystem::unlink(Filesystem::findFiles(Hyde::sitePath(), ['html', 'json'])->all());
Filesystem::cleanDirectory(Hyde::siteMediaPath());
}
}
Expand Down
7 changes: 2 additions & 5 deletions packages/framework/src/Support/DataCollections.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@
use Illuminate\Support\Collection;
use Illuminate\Support\Str;

use function implode;
use function Hyde\path_join;
use function json_decode;
use function sprintf;
use function Hyde\unslash;
use function str_starts_with;

Expand Down Expand Up @@ -102,9 +101,7 @@ public static function json(string $name, bool $asArray = false): static

protected static function findFiles(string $name, array|string $extensions): Collection
{
return Filesystem::smartGlob(sprintf('%s/%s/*.{%s}',
static::$sourceDirectory, $name, implode(',', (array) $extensions)
), GLOB_BRACE);
return Filesystem::findFiles(path_join(static::$sourceDirectory, $name), $extensions);
}

protected static function makeIdentifier(string $path): string
Expand Down
17 changes: 9 additions & 8 deletions packages/framework/src/Support/Filesystem/MediaFile.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Hyde\Support\Filesystem;

use Hyde\Facades\Filesystem;
use Hyde\Hyde;
use Hyde\Facades\Config;
use Hyde\Framework\Exceptions\FileNotFoundException;
Expand All @@ -14,12 +15,9 @@
use function array_merge;
use function array_keys;
use function filesize;
use function implode;
use function pathinfo;
use function collect;
use function is_file;
use function sprintf;
use function glob;

/**
* File abstraction for a project media file.
Expand Down Expand Up @@ -104,15 +102,18 @@ protected static function discoverMediaAssetFiles(): array
})->all();
}

/** @return array<string> */
protected static function getMediaAssetFiles(): array
{
return glob(Hyde::path(static::getMediaGlobPattern()), GLOB_BRACE) ?: [];
return Filesystem::findFiles(Hyde::getMediaDirectory(), static::getMediaFileExtensions(), true)->all();
}

protected static function getMediaGlobPattern(): string
/** @return array<string>|string */
protected static function getMediaFileExtensions(): array|string
{
return sprintf(Hyde::getMediaDirectory().'/{*,**/*,**/*/*}.{%s}', implode(',',
Config::getArray('hyde.media_extensions', self::EXTENSIONS)
));
/** @var array<string>|string $config */
$config = Config::get('hyde.media_extensions', self::EXTENSIONS);

return $config;
}
}
2 changes: 1 addition & 1 deletion packages/framework/tests/Feature/DiscoveryServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public function testMediaAssetExtensionsCanBeAddedByCommaSeparatedValues()

$this->assertSame([], MediaFile::files());

self::mockConfig(['hyde.media_extensions' => ['1,2,3']]);
self::mockConfig(['hyde.media_extensions' => '1,2,3']);
$this->assertSame(['test.1', 'test.2', 'test.3'], MediaFile::files());
}

Expand Down
16 changes: 16 additions & 0 deletions packages/framework/tests/Feature/FileCollectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,20 @@ public function testDocumentationPagesAreDiscovered()
$this->assertArrayHasKey('_docs/foo.md', $collection->toArray());
$this->assertEquals(new SourceFile('_docs/foo.md', DocumentationPage::class), $collection->get('_docs/foo.md'));
}

public function testDiscoverFilesForRecursivelyDiscoversFilesInSubdirectories()
{
$this->file('_pages/foo.md');
$this->file('_pages/foo/bar.md');
$this->file('_pages/foo/bar/baz.md');
$collection = FileCollection::init(Hyde::getInstance())->boot();

$this->assertArrayHasKey('_pages/foo.md', $collection->toArray());
$this->assertArrayHasKey('_pages/foo/bar.md', $collection->toArray());
$this->assertArrayHasKey('_pages/foo/bar/baz.md', $collection->toArray());

$this->assertEquals(new SourceFile('_pages/foo.md', MarkdownPage::class), $collection->get('_pages/foo.md'));
$this->assertEquals(new SourceFile('_pages/foo/bar.md', MarkdownPage::class), $collection->get('_pages/foo/bar.md'));
$this->assertEquals(new SourceFile('_pages/foo/bar/baz.md', MarkdownPage::class), $collection->get('_pages/foo/bar/baz.md'));
}
}
Loading
Loading