Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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 deptrac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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/.* }
Expand Down Expand Up @@ -89,6 +92,7 @@ deptrac:
- Cache
- Document
- Domain
- Filesystem # PhpDirectoryReader: the one directory walk
- Parser
- Repository
Repository:
Expand Down
6 changes: 4 additions & 2 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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/*
Expand Down Expand Up @@ -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:
Expand Down
19 changes: 19 additions & 0 deletions src/Filesystem/DirectoryListing.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

declare(strict_types=1);

namespace Firehed\PhpLsp\Filesystem;

final readonly class DirectoryListing
{
/**
* @param list<string> $files Full paths of the PHP files directly inside
* @param list<string> $directories Full paths of the directories directly inside
*/
public function __construct(
public string $path,
public array $files,
public array $directories,
) {
}
}
67 changes: 67 additions & 0 deletions src/Filesystem/PhpDirectoryReader.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?php

declare(strict_types=1);

namespace Firehed\PhpLsp\Filesystem;

use FilesystemIterator;
use SplFileInfo;

final class PhpDirectoryReader

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably leverage the native directory iteration tooling

{
/** @var list<non-empty-string> */
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<DirectoryListing> 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);
}
}
}
33 changes: 7 additions & 26 deletions src/Knowledge/ComposerMapBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -64,6 +65,7 @@ public function __construct(
private readonly ComposerAutoloadMapReader $mapReader,
private readonly DocumentSourceInterface $documents,
private readonly DeclarationSourceInterface $declarations,
private readonly PhpDirectoryReader $directories,
) {
}

Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -357,31 +359,10 @@ private static function relativePhpPath(string $directory, string $file): ?strin
/**
* @return iterable<string> 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;
}
}
}
3 changes: 2 additions & 1 deletion src/Knowledge/KnowledgeStack.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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()),
Expand Down
70 changes: 70 additions & 0 deletions tests/Filesystem/PhpDirectoryReaderTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?php

declare(strict_types=1);

namespace Firehed\PhpLsp\Tests\Filesystem;

use Firehed\PhpLsp\Filesystem\DirectoryListing;
use Firehed\PhpLsp\Filesystem\PhpDirectoryReader;
use Firehed\PhpLsp\Tests\LoadsFixturesTrait;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\TestCase;

#[CoversClass(PhpDirectoryReader::class)]
#[CoversClass(DirectoryListing::class)]
final class PhpDirectoryReaderTest extends TestCase
{
use LoadsFixturesTrait;

public function testReadListsPhpFilesAndDirectoriesOneLevelDown(): void
{
$root = rtrim($this->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',
);
}
}
4 changes: 4 additions & 0 deletions tests/Knowledge/ComposerMapBackendTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -73,6 +74,7 @@ public function testLookupClassLikeReadsAnOpenFileFromItsBuffer(): void
])),
new CompositeDocumentSource($open, $this->reader),
$this->declarations,
new PhpDirectoryReader(),
);

self::assertNotNull(
Expand Down Expand Up @@ -333,6 +335,7 @@ classMap: ['Fixtures\Domain\User' => $this->fixturesRoot . '/src/Domain/User.php
$mapReader,
$this->reader,
$this->declarations,
new PhpDirectoryReader(),
);

self::assertContains(
Expand Down Expand Up @@ -693,6 +696,7 @@ private function backendForMap(ComposerAutoloadMap $map): ComposerMapBackend
ComposerAutoloadMapReader::fromMap($map),
$this->reader,
$this->declarations,
new PhpDirectoryReader(),
);
}

Expand Down
8 changes: 7 additions & 1 deletion tests/Knowledge/CompositeInvalidatableTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Loading