From ea02ce05fa37d5b4544a6b5aafcc6ce052224e68 Mon Sep 17 00:00:00 2001 From: mamazu <14860264+mamazu@users.noreply.github.com> Date: Thu, 14 Mar 2024 23:32:23 +0100 Subject: [PATCH] Include excluded files (#2593) * Take hirarchy into account * Simplifying the implemenation * Adding test for default behaviour * Fixing the path sorting * CS * Adding comments to explain the behaviour. --- CHANGELOG.md | 5 +- lib/Filesystem/Domain/FileList.php | 60 ++++++--- .../Tests/Unit/Domain/FileListTest.php | 119 +++++++++++++++++- .../Filesystem/FilesystemFileListProvider.php | 13 +- lib/Indexer/Extension/IndexerExtension.php | 4 +- 5 files changed, 164 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e2bd84a79..7900890856 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ Improvements: - Basic support for `array_reduce` stub #2576 +Bug fixes: + + - Fixing include and exclude patterns #2593 @mamazu ## 2024-03-09 @@ -58,7 +61,7 @@ Bug fixes: Documentation: - - Added Helix LSP instructions #2581 @lens0021 + - Added Helix LSP instructions #2581 @lens0021 - Fix typos in Behat #2534 @vuon9 - Fix broken external links #2500 @einenlum diff --git a/lib/Filesystem/Domain/FileList.php b/lib/Filesystem/Domain/FileList.php index 41d556becd..5b2c792668 100644 --- a/lib/Filesystem/Domain/FileList.php +++ b/lib/Filesystem/Domain/FileList.php @@ -86,30 +86,51 @@ public function byExtensions(array $extensions): self } /** - * @param string[] $globPatterns + * @param string[] $includePatterns + * @param string[] $excludePatterns */ - public function excludePatterns(array $globPatterns): self + public function includeAndExclude(array $includePatterns = [], array $excludePatterns = []): self { - return $this->filter(function (SplFileInfo $info) use ($globPatterns) { - foreach ($globPatterns as $pattern) { - if (Glob::match($info->getPathname(), $pattern)) { - return false; - } + $inclusionMap = []; + if ($includePatterns === []) { + $inclusionMap['/**/*'] = true; + } + + foreach ($includePatterns as $includePattern) { + $inclusionMap[$includePattern] = true; + } + foreach ($excludePatterns as $excludePattern) { + $inclusionMap[$excludePattern] = false; + } + + // Sort map by keys so that more specific paths are getting matched first + uksort($inclusionMap, function (string $a, string $b) { + $partsA = explode(DIRECTORY_SEPARATOR, $a); + $partsB = explode(DIRECTORY_SEPARATOR, $b); + $countDiff = count($partsA) <=> count($partsB); + if ($countDiff !== 0) { + // Longer paths should come first + return -$countDiff; } - return true; + foreach ($partsA as $i => $pathPartA) { + if ($pathPartA === '**' || $pathPartA === '*') { + return 1; + } + + $compare = strcmp($pathPartA, $partsB[$i]); + if($compare !== 0) { + return $compare; + } + } + // If none of the path segments were different, then they must be equal + return 0; }); - } - /** - * @param string[] $globPatterns - */ - public function includePatterns(array $globPatterns): self - { - return $this->filter(function (SplFileInfo $info) use ($globPatterns) { - foreach ($globPatterns as $pattern) { - if (Glob::match($info->getPathname(), $pattern)) { - return true; + return $this->filter(function (SplFileInfo $info) use ($inclusionMap): bool { + foreach($inclusionMap as $glob => $isIncluded) { + if (Glob::match($info->getPathname(), $glob)) { + return $isIncluded; } } @@ -133,6 +154,9 @@ public function named(string $name): self ))); } + /** + * @param Closure(SplFileInfo): bool $closure + */ public function filter(Closure $closure): self { return new self(new CallbackFilterIterator($this->iterator, $closure)); diff --git a/lib/Filesystem/Tests/Unit/Domain/FileListTest.php b/lib/Filesystem/Tests/Unit/Domain/FileListTest.php index 14dba9aab6..1f440050b9 100644 --- a/lib/Filesystem/Tests/Unit/Domain/FileListTest.php +++ b/lib/Filesystem/Tests/Unit/Domain/FileListTest.php @@ -2,6 +2,7 @@ namespace Phpactor\Filesystem\Tests\Unit\Domain; +use Generator; use Phpactor\Filesystem\Domain\FileList; use Phpactor\Filesystem\Domain\FilePath; use Phpactor\Filesystem\Tests\IntegrationTestCase; @@ -121,9 +122,16 @@ public function testExcludesFilesMatchingPatterns(): void FilePath::fromString('/vendor/foo/bar/src/foo.php'), ]); - self::assertCount(2, $list->excludePatterns([ - '/vendor/**/tests/*', - ])); + self::assertEquals( + [ + FilePath::fromString('/vendor/foo/bar/src/bar.php'), + FilePath::fromString('/vendor/foo/bar/src/foo.php'), + ], + iterator_to_array($list->includeAndExclude( + includePatterns: ['/**/*'], + excludePatterns: [ '/vendor/**/tests/*'] + )) + ); } public function testIncldesFilesMatchingPatterns(): void @@ -135,9 +143,108 @@ public function testIncldesFilesMatchingPatterns(): void FilePath::fromString('/vendor/foo/bar/src/foo.php'), ]); - self::assertCount(2, $list->excludePatterns([ - '/vendor/**/tests/*', - ])); + self::assertEquals( + [ + FilePath::fromString('/vendor/foo/bar/tests/bartest.php'), + FilePath::fromString('/vendor/foo/bar/tests/footest.php'), + ], + iterator_to_array($list->includeAndExclude( + includePatterns: [ '/vendor/**/tests/*'], + )) + ); + } + + public function testIncludesEverythingByDefault(): void + { + $list = FileList::fromFilePaths([ + FilePath::fromString('/vendor/cache/important/bartest.php'), + FilePath::fromString('/vendor/cache/important/footest.php'), + FilePath::fromString('/vendor/cache/bar.php'), + FilePath::fromString('/vendor/cache/foo.php'), + ])->includeAndExclude( + includePatterns: [], + excludePatterns: [], + ); + + self::assertEquals( + [ + FilePath::fromString('/vendor/cache/important/bartest.php'), + FilePath::fromString('/vendor/cache/important/footest.php'), + FilePath::fromString('/vendor/cache/bar.php'), + FilePath::fromString('/vendor/cache/foo.php'), + ], + iterator_to_array($list) + ); + } + + public function testIncludesExcludePatterns(): void + { + $list = FileList::fromFilePaths([ + FilePath::fromString('/vendor/cache/important/bartest.php'), + FilePath::fromString('/vendor/cache/important/footest.php'), + FilePath::fromString('/vendor/cache/bar.php'), + FilePath::fromString('/vendor/cache/foo.php'), + ])->includeAndExclude( + includePatterns: [ '/src/**/*', '/vendor/cache/important/**/*'], + excludePatterns: [ '/vendor/**/*' ], + ); + + self::assertEquals( + [ + FilePath::fromString('/vendor/cache/important/bartest.php'), + FilePath::fromString('/vendor/cache/important/footest.php'), + ], + iterator_to_array($list) + ); + } + + /** + * @param array $fileList + * @param array $includePatterns + * @param array $excludePatterns + * @param array $expected + * + * @dataProvider provideExcludesWithShortFolderName + */ + public function testExcludesWithShortFolderName( + array $fileList, + array $includePatterns, + array $excludePatterns, + array $expected, + ): void { + $list = FileList::fromFilePaths($fileList)->includeAndExclude( + includePatterns:$includePatterns, + excludePatterns: $excludePatterns + ); + + self::assertEquals($expected, array_map(fn (FilePath $x) => (string) $x, iterator_to_array($list))); + } + + public function provideExcludesWithShortFolderName(): Generator + { + yield 'ascii file name' => [ + [ + FilePath::fromString('/src/package/test.php'), + FilePath::fromString('/src/a/test.php'), + ], + [ '/src/**/*'], + [ '/src/a/*' ], + [ + '/src/package/test.php', + ], + ]; + + yield 'unicode file name' => [ + [ + FilePath::fromString('/src/package/test.php'), + FilePath::fromString('/src/ü/test.php'), + ], + [ '/src/**/*'], + [ '/src/ü/*' ], + [ + '/src/package/test.php', + ], + ]; } public function testContainingString(): void diff --git a/lib/Indexer/Adapter/Filesystem/FilesystemFileListProvider.php b/lib/Indexer/Adapter/Filesystem/FilesystemFileListProvider.php index 7140e61993..40fe59c094 100644 --- a/lib/Indexer/Adapter/Filesystem/FilesystemFileListProvider.php +++ b/lib/Indexer/Adapter/Filesystem/FilesystemFileListProvider.php @@ -30,15 +30,10 @@ public function provideFileList(Index $index, ?string $subPath = null): FileList return FileList::fromSingleFilePath($subPath); } - $files = $this->filesystem->fileList()->byExtensions($this->supportedExtensions); - - if ($this->includePatterns) { - $files = $files->includePatterns($this->includePatterns); - } - - if ($this->excludePatterns) { - $files = $files->excludePatterns($this->excludePatterns); - } + $files = $this->filesystem->fileList() + ->byExtensions($this->supportedExtensions) + ->includeAndExclude($this->includePatterns, $this->excludePatterns) + ; if ($subPath) { $files = $files->within(FilePath::fromString($subPath)); diff --git a/lib/Indexer/Extension/IndexerExtension.php b/lib/Indexer/Extension/IndexerExtension.php index 703a63237e..18571d4de6 100644 --- a/lib/Indexer/Extension/IndexerExtension.php +++ b/lib/Indexer/Extension/IndexerExtension.php @@ -224,9 +224,7 @@ private function registerModel(ContainerBuilder $container): void ->setIncludePatterns($container->get(self::SERVICE_INDEXER_INCLUDE_PATTERNS)) /** @phpstan-ignore-next-line */ ->setSupportedExtensions($container->parameter(self::PARAM_SUPPORTED_EXTENSIONS)->value()) - ->setFollowSymlinks( - (bool) $container->getParameter(self::PARAM_INDEXER_FOLLOW_SYMLINKS), - ) + ->setFollowSymlinks($container->parameter(self::PARAM_INDEXER_FOLLOW_SYMLINKS)->bool()) ->setStubPaths($container->getParameter(self::PARAM_STUB_PATHS)); });