diff --git a/deptrac.yaml b/deptrac.yaml index 6868734f..758d0b2d 100644 --- a/deptrac.yaml +++ b/deptrac.yaml @@ -27,6 +27,9 @@ deptrac: - name: Domain collectors: - { type: directory, value: src/Domain/.* } + - name: Filesystem + collectors: + - { type: directory, value: src/Filesystem/.* } - name: Handler collectors: - { type: directory, value: src/Handler/.* } @@ -89,6 +92,7 @@ deptrac: - Cache - Document - Domain + - Filesystem # PhpDirectoryReader: the one directory walk - Parser - Repository Repository: diff --git a/phpstan.neon b/phpstan.neon index 889cd298..b9c1f1db 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -267,9 +267,10 @@ parameters: - 'disk_total_space()' - 'parse_ini_file()' - 'move_uploaded_file()' - message: 'filesystem access is confined: SourceFileReader reads source, ComposerAutoloadMap and ComposerMapBackend read Composer metadata, transport owns its streams' + message: 'filesystem access is confined: SourceFileReader reads source, PhpDirectoryReader lists directories, ComposerAutoloadMap and ComposerMapBackend read Composer metadata, transport owns its streams' allowIn: - src/Document/SourceFileReader.php + - src/Filesystem/PhpDirectoryReader.php - src/Domain/ComposerAutoloadMap.php - src/Knowledge/ComposerMapBackend.php - src/Transport/* @@ -307,8 +308,9 @@ parameters: - 'SplFileInfo' - 'SplFileObject' - 'SplTempFileObject' - message: 'filesystem access is confined, and its readers (SourceFileReader, ComposerAutoloadMap, ComposerMapBackend, transport) reach the disk through functions rather than through a file object' + message: 'filesystem access is confined: SourceFileReader reads source, PhpDirectoryReader lists directories, ComposerAutoloadMap and ComposerMapBackend read Composer metadata, transport owns its streams' allowIn: + - src/Filesystem/PhpDirectoryReader.php - tests/* disallowedMethodCalls: diff --git a/src/Filesystem/DirectoryListing.php b/src/Filesystem/DirectoryListing.php new file mode 100644 index 00000000..a6965748 --- /dev/null +++ b/src/Filesystem/DirectoryListing.php @@ -0,0 +1,19 @@ + $files Full paths of the PHP files directly inside + * @param list $directories Full paths of the directories directly inside + */ + public function __construct( + public string $path, + public array $files, + public array $directories, + ) { + } +} diff --git a/src/Filesystem/PhpDirectoryReader.php b/src/Filesystem/PhpDirectoryReader.php new file mode 100644 index 00000000..3d0d3c2c --- /dev/null +++ b/src/Filesystem/PhpDirectoryReader.php @@ -0,0 +1,67 @@ + */ + private array $extensions; + + public function __construct() + { + // Future scope: allow other extensions via constructor injection + $this->extensions = ['php']; + } + + public function read(string $directory): ?DirectoryListing + { + if (!is_dir($directory)) { + return null; + } + + $files = []; + $directories = []; + foreach (new FilesystemIterator($directory, FilesystemIterator::SKIP_DOTS) as $entry) { + \assert($entry instanceof SplFileInfo); + $path = $entry->getPathname(); + if ($entry->isDir()) { + $directories[] = $path; + } elseif ($entry->isFile() && $this->hasWatchedExtension($entry->getFilename())) { + $files[] = $path; + } + } + + return new DirectoryListing($directory, $files, $directories); + } + + private function hasWatchedExtension(string $entry): bool + { + foreach ($this->extensions as $extension) { + if (str_ends_with($entry, '.' . $extension)) { + return true; + } + } + return false; + } + + /** + * @return iterable The directory, then every directory beneath it + */ + public function walk(string $directory): iterable + { + $listing = $this->read($directory); + if ($listing === null) { + return; + } + + yield $listing; + foreach ($listing->directories as $child) { + yield from $this->walk($child); + } + } +} diff --git a/src/Knowledge/ComposerMapBackend.php b/src/Knowledge/ComposerMapBackend.php index fbb6350e..274ef597 100644 --- a/src/Knowledge/ComposerMapBackend.php +++ b/src/Knowledge/ComposerMapBackend.php @@ -20,6 +20,7 @@ use Firehed\PhpLsp\Domain\Symbol; use Firehed\PhpLsp\Domain\SymbolInfoInterface; use Firehed\PhpLsp\Domain\SymbolKind; +use Firehed\PhpLsp\Filesystem\PhpDirectoryReader; /** * A {@see SymbolSourceInterface} over PHP files on disk, resolved through @@ -64,6 +65,7 @@ public function __construct( private readonly ComposerAutoloadMapReader $mapReader, private readonly DocumentSourceInterface $documents, private readonly DeclarationSourceInterface $declarations, + private readonly PhpDirectoryReader $directories, ) { } @@ -179,7 +181,7 @@ private function buildIndex(ComposerAutoloadMap $map): void foreach (self::orderedByPrefixLength($map->psr4Prefixes()) as $prefix => $directories) { $prefixTrimmed = trim($prefix, '\\'); foreach ($directories as $directory) { - foreach (self::walkPhpFiles($directory) as $file) { + foreach ($this->walkPhpFiles($directory) as $file) { if (array_key_exists($file, $this->fqnByWalkedPath)) { continue; } @@ -194,7 +196,7 @@ private function buildIndex(ComposerAutoloadMap $map): void foreach (self::orderedByPrefixLength($map->psr0Prefixes()) as $prefix => $directories) { $prefixTrimmed = trim($prefix, '\\'); foreach ($directories as $directory) { - foreach (self::walkPhpFiles($directory) as $file) { + foreach ($this->walkPhpFiles($directory) as $file) { if (array_key_exists($file, $this->fqnByWalkedPath)) { continue; } @@ -357,31 +359,10 @@ private static function relativePhpPath(string $directory, string $file): ?strin /** * @return iterable Real paths of every `.php` file under $directory. */ - private static function walkPhpFiles(string $directory): iterable + private function walkPhpFiles(string $directory): iterable { - if (!is_dir($directory)) { - return; - } - - $entries = scandir($directory); - if ($entries === false) { - // @codeCoverageIgnoreStart - return; - // @codeCoverageIgnoreEnd - } - - foreach ($entries as $entry) { - if ($entry === '.' || $entry === '..') { - continue; - } - $path = $directory . '/' . $entry; - if (is_dir($path)) { - yield from self::walkPhpFiles($path); - continue; - } - if (str_ends_with($entry, '.php') && is_file($path)) { - yield $path; - } + foreach ($this->directories->walk($directory) as $listing) { + yield from $listing->files; } } } diff --git a/src/Knowledge/KnowledgeStack.php b/src/Knowledge/KnowledgeStack.php index ad51673c..18c592c6 100644 --- a/src/Knowledge/KnowledgeStack.php +++ b/src/Knowledge/KnowledgeStack.php @@ -7,6 +7,7 @@ use Firehed\PhpLsp\Cache\CacheFactory; use Firehed\PhpLsp\Cache\InvalidatableInterface; use Firehed\PhpLsp\Document\DocumentSourceInterface; +use Firehed\PhpLsp\Filesystem\PhpDirectoryReader; use Firehed\PhpLsp\Parser\SyntaxSource\SyntaxSourceInterface; /** @@ -56,7 +57,7 @@ public static function forProject( $openDocuments = new OpenDocumentBackend($declarations); $autoloadFiles = new AutoloadFilesBackend($mapReader, $reader, $declarations); - $composerMap = new ComposerMapBackend($mapReader, $reader, $declarations); + $composerMap = new ComposerMapBackend($mapReader, $reader, $declarations, new PhpDirectoryReader()); return new self( new CompositeSymbolSource($openDocuments, $autoloadFiles, $composerMap, new BuiltinBackend()), diff --git a/tests/Filesystem/PhpDirectoryReaderTest.php b/tests/Filesystem/PhpDirectoryReaderTest.php new file mode 100644 index 00000000..20f8704e --- /dev/null +++ b/tests/Filesystem/PhpDirectoryReaderTest.php @@ -0,0 +1,70 @@ +fixturePath(''), '/'); + + $listing = (new PhpDirectoryReader())->read($root); + + self::assertNotNull($listing, 'the fixtures root is a directory'); + self::assertSame($root, $listing->path, 'a listing names the directory it describes'); + self::assertContains($root . '/NoNamespace.php', $listing->files, 'a PHP file is listed by its full path'); + self::assertNotContains($root . '/composer.json', $listing->files, 'only PHP files are listed'); + self::assertContains($root . '/src', $listing->directories, 'a directory is listed by its full path'); + self::assertNotContains( + $root . '/src/Domain/User.php', + $listing->files, + 'a listing describes one directory, not what is beneath it', + ); + } + + public function testReadOfSomethingThatIsNotADirectoryIsNull(): void + { + $reader = new PhpDirectoryReader(); + + self::assertNull($reader->read($this->fixturePath('no/such/directory')), 'a missing path has no listing'); + self::assertNull($reader->read($this->fixturePath('NoNamespace.php')), 'a file has no listing'); + } + + public function testWalkReachesEveryDirectoryBeneathTheRoot(): void + { + $root = $this->fixturePath('src'); + + $paths = []; + $files = []; + foreach ((new PhpDirectoryReader())->walk($root) as $listing) { + $paths[] = $listing->path; + $files = [...$files, ...$listing->files]; + } + + self::assertSame($root, $paths[0], 'the walk starts with the root itself'); + self::assertContains($root . '/Domain', $paths, 'a directory beneath the root is walked'); + self::assertContains($root . '/Domain/User.php', $files, 'a file in a nested directory is reached'); + self::assertSame(array_unique($paths), $paths, 'no directory is walked twice'); + } + + public function testWalkOfSomethingThatIsNotADirectoryIsEmpty(): void + { + self::assertSame( + [], + iterator_to_array((new PhpDirectoryReader())->walk($this->fixturePath('no/such/directory')), false), + 'an autoload root that does not exist contributes nothing', + ); + } +} diff --git a/tests/Knowledge/ComposerMapBackendTest.php b/tests/Knowledge/ComposerMapBackendTest.php index 911ace2e..a1abd342 100644 --- a/tests/Knowledge/ComposerMapBackendTest.php +++ b/tests/Knowledge/ComposerMapBackendTest.php @@ -14,6 +14,7 @@ use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Domain\NamespaceContents; use Firehed\PhpLsp\Domain\NamespaceName; +use Firehed\PhpLsp\Filesystem\PhpDirectoryReader; use Firehed\PhpLsp\Knowledge\ComposerAutoloadMapReader; use Firehed\PhpLsp\Knowledge\ComposerMapBackend; use Firehed\PhpLsp\Knowledge\ParsedDeclarationSource; @@ -73,6 +74,7 @@ public function testLookupClassLikeReadsAnOpenFileFromItsBuffer(): void ])), new CompositeDocumentSource($open, $this->reader), $this->declarations, + new PhpDirectoryReader(), ); self::assertNotNull( @@ -333,6 +335,7 @@ classMap: ['Fixtures\Domain\User' => $this->fixturesRoot . '/src/Domain/User.php $mapReader, $this->reader, $this->declarations, + new PhpDirectoryReader(), ); self::assertContains( @@ -693,6 +696,7 @@ private function backendForMap(ComposerAutoloadMap $map): ComposerMapBackend ComposerAutoloadMapReader::fromMap($map), $this->reader, $this->declarations, + new PhpDirectoryReader(), ); } diff --git a/tests/Knowledge/CompositeInvalidatableTest.php b/tests/Knowledge/CompositeInvalidatableTest.php index 03a76439..43ce6579 100644 --- a/tests/Knowledge/CompositeInvalidatableTest.php +++ b/tests/Knowledge/CompositeInvalidatableTest.php @@ -9,6 +9,7 @@ use Firehed\PhpLsp\Domain\FunctionName; use Firehed\PhpLsp\Domain\NameKind; use Firehed\PhpLsp\Domain\Symbol; +use Firehed\PhpLsp\Filesystem\PhpDirectoryReader; use Firehed\PhpLsp\Knowledge\AutoloadFilesBackend; use Firehed\PhpLsp\Knowledge\ComposerAutoloadMapReader; use Firehed\PhpLsp\Knowledge\ComposerMapBackend; @@ -126,7 +127,12 @@ private function composerMapBackend(ComposerAutoloadMapReader $mapReader): Compo { $production = ProductionSyntaxSource::create(); - return new ComposerMapBackend($mapReader, $production->reader, $production->declarations); + return new ComposerMapBackend( + $mapReader, + $production->reader, + $production->declarations, + new PhpDirectoryReader(), + ); } private function autoloadFilesBackend(ComposerAutoloadMapReader $mapReader): AutoloadFilesBackend