From 2f939f260b24eab0e801f1711d2af0c37daeef9a Mon Sep 17 00:00:00 2001 From: mamazu <14860264+mamazu@users.noreply.github.com> Date: Sun, 24 Mar 2024 14:24:17 +0100 Subject: [PATCH 1/5] Adding better types --- phpstan-baseline.neon | 65 ------------------- .../Application/Finder/FileFinderTest.php | 11 +++- .../Logger/SymfonyConsoleMoveLoggerTest.php | 4 +- .../ClassMover/Rpc/ClassCopyHandlerTest.php | 5 +- .../ClassMover/Rpc/ClassMoveHandlerTest.php | 2 +- .../ClassMover/Rpc/ReferencesHandlerTest.php | 4 +- 6 files changed, 18 insertions(+), 73 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index b025e78797..2cd58a834c 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -620,11 +620,6 @@ parameters: count: 1 path: lib/CodeTransform/Tests/Adapter/TolerantParser/ClassToFile/Transformer/ClassNameFixerTransformerTest.php - - - message: "#^Method Phpactor\\\\CodeTransform\\\\Tests\\\\Adapter\\\\TolerantParser\\\\Refactor\\\\AbstractTolerantImportNameTest\\:\\:importNameFromTestFile\\(\\) has no return type specified\\.$#" - count: 1 - path: lib/CodeTransform/Tests/Adapter/TolerantParser/Refactor/AbstractTolerantImportNameTest.php - - message: "#^Method Phpactor\\\\CodeTransform\\\\Tests\\\\Adapter\\\\TolerantParser\\\\Refactor\\\\TolerantChangeVisiblityTest\\:\\:provideChangeVisibility\\(\\) has no return type specified\\.$#" count: 1 @@ -2975,11 +2970,6 @@ parameters: count: 1 path: lib/Extension/Core/Application/Status.php - - - message: "#^Method Phpactor\\\\Extension\\\\Core\\\\Application\\\\Status\\:\\:versionInfo\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: lib/Extension/Core/Application/Status.php - - message: "#^Cannot call method path\\(\\) on mixed\\.$#" count: 2 @@ -4055,11 +4045,6 @@ parameters: count: 1 path: lib/Extension/LanguageServerRename/Handler/FileRenameHandler.php - - - message: "#^Cannot call method getTraceAsString\\(\\) on Throwable\\|null\\.$#" - count: 1 - path: lib/Extension/LanguageServerRename/Handler/RenameHandler.php - - message: "#^Method Phpactor\\\\Extension\\\\LanguageServerRename\\\\Handler\\\\RenameHandler\\:\\:prepareRename\\(\\) should return Amp\\\\Promise\\ but returns Amp\\\\Promise\\\\.$#" count: 1 @@ -5555,51 +5540,16 @@ parameters: count: 1 path: lib/Indexer/Adapter/Php/InMemory/InMemoryIndex.php - - - message: "#^Cannot call method path\\(\\) on Phpactor\\\\TextDocument\\\\TextDocumentUri\\|null\\.$#" - count: 2 - path: lib/Indexer/Adapter/Tolerant/Indexer/AbstractClassLikeIndexer.php - - message: "#^Parameter \\#1 \\$name of static method Phpactor\\\\Indexer\\\\Model\\\\Record\\\\ClassRecord\\:\\:fromName\\(\\) expects string, Microsoft\\\\PhpParser\\\\ResolvedName\\|string\\|null given\\.$#" count: 1 path: lib/Indexer/Adapter/Tolerant/Indexer/ClassDeclarationIndexer.php - - - message: "#^Cannot call method path\\(\\) on Phpactor\\\\TextDocument\\\\TextDocumentUri\\|null\\.$#" - count: 3 - path: lib/Indexer/Adapter/Tolerant/Indexer/ClassLikeReferenceIndexer.php - - - - message: "#^Cannot call method path\\(\\) on Phpactor\\\\TextDocument\\\\TextDocumentUri\\|null\\.$#" - count: 2 - path: lib/Indexer/Adapter/Tolerant/Indexer/ConstantDeclarationIndexer.php - - - - message: "#^Cannot call method path\\(\\) on Phpactor\\\\TextDocument\\\\TextDocumentUri\\|null\\.$#" - count: 1 - path: lib/Indexer/Adapter/Tolerant/Indexer/FunctionDeclarationIndexer.php - - - - message: "#^Cannot call method path\\(\\) on Phpactor\\\\TextDocument\\\\TextDocumentUri\\|null\\.$#" - count: 3 - path: lib/Indexer/Adapter/Tolerant/Indexer/FunctionReferenceIndexer.php - - - - message: "#^Cannot call method path\\(\\) on Phpactor\\\\TextDocument\\\\TextDocumentUri\\|null\\.$#" - count: 3 - path: lib/Indexer/Adapter/Tolerant/Indexer/MemberIndexer.php - - message: "#^Cannot call method __toString\\(\\) on Phpactor\\\\TextDocument\\\\TextDocumentUri\\|null\\.$#" count: 1 path: lib/Indexer/Adapter/Tolerant/TolerantIndexBuilder.php - - - message: "#^Cannot call method path\\(\\) on Phpactor\\\\TextDocument\\\\TextDocumentUri\\|null\\.$#" - count: 1 - path: lib/Indexer/Adapter/Tolerant/TolerantIndexBuilder.php - - message: "#^Parameter \\#1 \\.\\.\\.\\$paths of static method Symfony\\\\Component\\\\Filesystem\\\\Path\\:\\:join\\(\\) expects string, string\\|null given\\.$#" count: 1 @@ -6000,16 +5950,6 @@ parameters: count: 1 path: lib/Rename/Tests/Adapter/ClassMover/FileRenamerTest.php - - - message: "#^Parameter \\#1 \\$from of method Phpactor\\\\Rename\\\\Adapter\\\\ClassMover\\\\FileRenamer\\:\\:renameFile\\(\\) expects Phpactor\\\\TextDocument\\\\TextDocumentUri, Phpactor\\\\TextDocument\\\\TextDocumentUri\\|null given\\.$#" - count: 1 - path: lib/Rename/Tests/Adapter/ClassMover/FileRenamerTest.php - - - - message: "#^Parameter \\#2 \\$to of method Phpactor\\\\Rename\\\\Adapter\\\\ClassMover\\\\FileRenamer\\:\\:renameFile\\(\\) expects Phpactor\\\\TextDocument\\\\TextDocumentUri, Phpactor\\\\TextDocument\\\\TextDocumentUri\\|null given\\.$#" - count: 1 - path: lib/Rename/Tests/Adapter/ClassMover/FileRenamerTest.php - - message: "#^Cannot call method newUri\\(\\) on mixed\\.$#" count: 1 @@ -6455,11 +6395,6 @@ parameters: count: 1 path: lib/WorseReflection/Core/Reflection/Collection/HomogeneousReflectionMemberCollection.php - - - message: "#^Parameter \\#3 \\$node of class Phpactor\\\\WorseReflection\\\\Bridge\\\\TolerantParser\\\\Reflection\\\\ReflectionArgument constructor expects Microsoft\\\\PhpParser\\\\Node\\\\Expression\\\\ArgumentExpression, mixed given\\.$#" - count: 1 - path: lib/WorseReflection/Core/Reflection/Collection/ReflectionArgumentCollection.php - - message: "#^Cannot call method getName\\(\\) on mixed\\.$#" count: 2 diff --git a/tests/Unit/Extension/ClassMover/Application/Finder/FileFinderTest.php b/tests/Unit/Extension/ClassMover/Application/Finder/FileFinderTest.php index 9d4162b7cc..8e54483050 100644 --- a/tests/Unit/Extension/ClassMover/Application/Finder/FileFinderTest.php +++ b/tests/Unit/Extension/ClassMover/Application/Finder/FileFinderTest.php @@ -14,13 +14,20 @@ use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use RuntimeException; +use Phpactor\TextDocument\TextDocument; class FileFinderTest extends TestCase { use ProphecyTrait; + /** + * @var ObjectProphecy + */ private ObjectProphecy $filesystem; + /** + * @var ObjectProphecy + */ private ObjectProphecy $fileList; public function setUp(): void @@ -84,7 +91,7 @@ public function testParentsTraitsAndInterfacesIfMemberIsProtected(): void $this->assertEquals(FileList::fromFilePaths(['barfoo', 'barfoo', 'barfoo', 'barfoo']), $files); } - private function filesFor(ReflectionClassLike $class = null, string $memberName = null) + private function filesFor(ReflectionClassLike $class = null, string $memberName = null): FileList { return (new FileFinder())->filesFor($this->filesystem->reveal(), $class, $memberName); } @@ -96,7 +103,7 @@ private function setupAllFiles(): void $this->fileList->phpFiles()->willReturn($this->fileList->reveal()); } - private function reflectClass($source, string $name) + private function reflectClass(string|TextDocument $source, string $name): ReflectionClassLike { if (is_string($source)) { $source = ' + */ private ObjectProphecy $classCopy; public function setUp(): void @@ -38,7 +41,7 @@ public function createHandler(): Handler */ public function testNoDestPath(): void { - /** @var $action InputCallbackAction */ + /** @var InputCallbackAction $action */ $action = $this->handle('copy_class', [ 'source_path' => self::SOURCE_PATH, 'dest_path' => null, diff --git a/tests/Unit/Extension/ClassMover/Rpc/ClassMoveHandlerTest.php b/tests/Unit/Extension/ClassMover/Rpc/ClassMoveHandlerTest.php index 17013fa15f..43c6389028 100644 --- a/tests/Unit/Extension/ClassMover/Rpc/ClassMoveHandlerTest.php +++ b/tests/Unit/Extension/ClassMover/Rpc/ClassMoveHandlerTest.php @@ -42,7 +42,7 @@ public function createHandler(): Handler public function testNotConfirmed(): void { - /** @var $action InputCallbackAction */ + /** @var InputCallbackAction $action */ $action = $this->handle('move_class', [ 'source_path' => self::SOURCE_PATH, 'dest_path' => null, diff --git a/tests/Unit/Extension/ClassMover/Rpc/ReferencesHandlerTest.php b/tests/Unit/Extension/ClassMover/Rpc/ReferencesHandlerTest.php index 1718bde068..3f365f694e 100644 --- a/tests/Unit/Extension/ClassMover/Rpc/ReferencesHandlerTest.php +++ b/tests/Unit/Extension/ClassMover/Rpc/ReferencesHandlerTest.php @@ -440,7 +440,7 @@ public function testReferencesAreSorted(): void ], $second->parameters()); } - private function exampleMemberRiskyResponse() + private function exampleMemberRiskyResponse(): array { return [ 'references' => [ @@ -467,7 +467,7 @@ private function exampleMemberRiskyResponse() ]; } - private function exampleClassResponse() + private function exampleClassResponse(): array { return [ 'references' => [ From 7e8b3b7e3ac30880f28e5c177bfc19bfb6e43d58 Mon Sep 17 00:00:00 2001 From: mamazu <14860264+mamazu@users.noreply.github.com> Date: Sun, 24 Mar 2024 14:56:11 +0100 Subject: [PATCH 2/5] More types --- lib/Filesystem/Domain/FilePath.php | 18 ++---- lib/Filesystem/Domain/FilesystemRegistry.php | 3 + .../Domain/MappedFilesystemRegistry.php | 1 - .../Tests/Adapter/AdapterTestCase.php | 2 - .../Tests/Adapter/IntegrationTestCase.php | 4 +- .../Tests/Unit/Domain/FilePathTest.php | 33 +++------- .../Domain/MappedFilesystemRegistryTest.php | 3 +- phpstan-baseline.neon | 60 ------------------- 8 files changed, 18 insertions(+), 106 deletions(-) diff --git a/lib/Filesystem/Domain/FilePath.php b/lib/Filesystem/Domain/FilePath.php index 933c0c9d2f..603884b653 100644 --- a/lib/Filesystem/Domain/FilePath.php +++ b/lib/Filesystem/Domain/FilePath.php @@ -29,6 +29,9 @@ public static function fromString(string $string): FilePath return new self($textDocumentUri); } + /** + * @param array $parts + */ public static function fromParts(array $parts): FilePath { $path = Path::join(...$parts); @@ -45,7 +48,7 @@ public static function fromSplFileInfo(SplFileInfo $fileInfo): FilePath return self::fromString((string) $fileInfo); } - public static function fromUnknown($path): FilePath + public static function fromUnknown(FilePath|string $path): FilePath { if ($path instanceof FilePath) { return $path; @@ -54,19 +57,6 @@ public static function fromUnknown($path): FilePath if (is_string($path)) { return self::fromString($path); } - - if (is_array($path)) { - return self::fromParts($path); - } - - if ($path instanceof SplFileInfo) { - return self::fromSplFileInfo($path); - } - - throw new RuntimeException(sprintf( - 'Do not know how to create FilePath from "%s"', - is_object($path) ? get_class($path) : gettype($path) - )); } public function isDirectory(): bool diff --git a/lib/Filesystem/Domain/FilesystemRegistry.php b/lib/Filesystem/Domain/FilesystemRegistry.php index ef10044d6b..06f4c9f1aa 100644 --- a/lib/Filesystem/Domain/FilesystemRegistry.php +++ b/lib/Filesystem/Domain/FilesystemRegistry.php @@ -8,5 +8,8 @@ public function get(string $name): Filesystem; public function has(string $name): bool; + /** + * @return array + */ public function names(): array; } diff --git a/lib/Filesystem/Domain/MappedFilesystemRegistry.php b/lib/Filesystem/Domain/MappedFilesystemRegistry.php index 4e9bbdb58e..91cd03cbb2 100644 --- a/lib/Filesystem/Domain/MappedFilesystemRegistry.php +++ b/lib/Filesystem/Domain/MappedFilesystemRegistry.php @@ -35,7 +35,6 @@ public function has(string $name): bool return isset($this->filesystems[$name]); } - /** @return array */ public function names(): array { return array_keys($this->filesystems); diff --git a/lib/Filesystem/Tests/Adapter/AdapterTestCase.php b/lib/Filesystem/Tests/Adapter/AdapterTestCase.php index 7a4c1f736c..40ecffc210 100644 --- a/lib/Filesystem/Tests/Adapter/AdapterTestCase.php +++ b/lib/Filesystem/Tests/Adapter/AdapterTestCase.php @@ -6,8 +6,6 @@ abstract class AdapterTestCase extends IntegrationTestCase { - private $filesystem; - public function setUp(): void { $this->initWorkspace(); diff --git a/lib/Filesystem/Tests/Adapter/IntegrationTestCase.php b/lib/Filesystem/Tests/Adapter/IntegrationTestCase.php index 9b661a0d96..16e00e8b66 100644 --- a/lib/Filesystem/Tests/Adapter/IntegrationTestCase.php +++ b/lib/Filesystem/Tests/Adapter/IntegrationTestCase.php @@ -17,7 +17,7 @@ protected function initWorkspace(): void $filesystem->mkdir($this->workspacePath()); } - protected function workspacePath() + protected function workspacePath(): string { return realpath(__DIR__.'/..') . '/Workspace'; } @@ -31,7 +31,7 @@ protected function loadProject(): void exec('composer dumpautoload 2> /dev/null'); } - protected function getProjectAutoloader() + protected function getProjectAutoloader(): string { return require __DIR__.'/project/vendor/autoload.php'; } diff --git a/lib/Filesystem/Tests/Unit/Domain/FilePathTest.php b/lib/Filesystem/Tests/Unit/Domain/FilePathTest.php index 5acf2615ad..03464274c8 100644 --- a/lib/Filesystem/Tests/Unit/Domain/FilePathTest.php +++ b/lib/Filesystem/Tests/Unit/Domain/FilePathTest.php @@ -2,6 +2,7 @@ namespace Phpactor\Filesystem\Tests\Unit\Domain; +use Generator; use PHPUnit\Framework\TestCase; use Phpactor\Filesystem\Domain\FilePath; use Phpactor\TextDocument\Exception\InvalidUriException; @@ -9,7 +10,6 @@ use RuntimeException; use SplFileInfo; use Symfony\Component\Filesystem\Path; -use stdClass; class FilePathTest extends TestCase { @@ -32,14 +32,14 @@ public function testFromParts(): void /** * @dataProvider provideUnknown */ - public function testFromUnknownWith($path, string $expectedPath): void + public function testFromUnknownWith(FilePath|string $path, string $expectedPath): void { $filePath = FilePath::fromUnknown($path); $this->assertInstanceOf(FilePath::class, $filePath); $this->assertEquals($expectedPath, (string) $filePath); } - public function provideUnknown() + public function provideUnknown(): Generator { yield 'FilePath instance' => [ FilePath::fromString('/foo.php'), @@ -65,39 +65,20 @@ public function provideUnknown() 'phar:///foo.php', '/foo.php', ]; - - yield 'array' => [ - [ 'one', 'two' ], - '/one/two' - ]; - - yield 'SplFileInfo' => [ - new SplFileInfo(__FILE__), - Path::canonicalize(__FILE__) - ]; - yield 'SplFileInfo with scheme' => [ - new SplFileInfo((string)TextDocumentUri::fromString(__FILE__)), - Path::canonicalize(__FILE__) - ]; } /** * @dataProvider provideUnsupportedInput */ - public function testThrowExceptionOnUnknowableType($input, string $expectedExceptionMessage): void + public function testThrowExceptionOnUnknowableType(string $input, string $expectedExceptionMessage): void { $this->expectException(RuntimeException::class); $this->expectExceptionMessage($expectedExceptionMessage); - FilePath::fromUnknown($input); + FilePath::fromString($input); } - public function provideUnsupportedInput() + public function provideUnsupportedInput(): Generator { - yield 'object' => [ - new stdClass(), - 'Do not know', - ]; - yield 'unsupported scheme' => [ 'ftp://host/foo.php', 'are supported', // only X schemes are supported @@ -186,7 +167,7 @@ public function testIsNamed(): void public function testAsSplFileInfo(): void { - $path1 = FilePath::fromUnknown(new SplFileInfo((string)TextDocumentUri::fromString(__FILE__))); + $path1 = FilePath::fromSplFileInfo(new SplFileInfo((string)TextDocumentUri::fromString(__FILE__))); self::assertEquals(Path::canonicalize(__FILE__), $path1->__toString()); self::assertEquals(Path::canonicalize(__FILE__), $path1->asSplFileInfo()->__toString()); } diff --git a/lib/Filesystem/Tests/Unit/Domain/MappedFilesystemRegistryTest.php b/lib/Filesystem/Tests/Unit/Domain/MappedFilesystemRegistryTest.php index 36028b26c0..7a383e6f4f 100644 --- a/lib/Filesystem/Tests/Unit/Domain/MappedFilesystemRegistryTest.php +++ b/lib/Filesystem/Tests/Unit/Domain/MappedFilesystemRegistryTest.php @@ -55,7 +55,8 @@ public function testExceptionOnNotFound(): void $registry->get('barfoo'); } - private function createRegistry(array $filesystems) + /** @param array $filesystems */ + private function createRegistry(array $filesystems): MappedFilesystemRegistry { return new MappedFilesystemRegistry($filesystems); } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 2cd58a834c..43ce970be3 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -5460,46 +5460,6 @@ parameters: count: 1 path: lib/Filesystem/Adapter/Simple/SimpleFilesystem.php - - - message: "#^Method Phpactor\\\\Filesystem\\\\Domain\\\\FallbackFilesystemRegistry\\:\\:names\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: lib/Filesystem/Domain/FallbackFilesystemRegistry.php - - - - message: "#^Method Phpactor\\\\Filesystem\\\\Domain\\\\FilePath\\:\\:fromParts\\(\\) has parameter \\$parts with no value type specified in iterable type array\\.$#" - count: 1 - path: lib/Filesystem/Domain/FilePath.php - - - - message: "#^Method Phpactor\\\\Filesystem\\\\Domain\\\\FilePath\\:\\:fromUnknown\\(\\) has parameter \\$path with no type specified\\.$#" - count: 1 - path: lib/Filesystem/Domain/FilePath.php - - - - message: "#^Method Phpactor\\\\Filesystem\\\\Domain\\\\FilesystemRegistry\\:\\:names\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: lib/Filesystem/Domain/FilesystemRegistry.php - - - - message: "#^Property Phpactor\\\\Filesystem\\\\Tests\\\\Adapter\\\\AdapterTestCase\\:\\:\\$filesystem has no type specified\\.$#" - count: 1 - path: lib/Filesystem/Tests/Adapter/AdapterTestCase.php - - - - message: "#^Property Phpactor\\\\Filesystem\\\\Tests\\\\Adapter\\\\AdapterTestCase\\:\\:\\$filesystem is unused\\.$#" - count: 1 - path: lib/Filesystem/Tests/Adapter/AdapterTestCase.php - - - - message: "#^Method Phpactor\\\\Filesystem\\\\Tests\\\\Adapter\\\\IntegrationTestCase\\:\\:getProjectAutoloader\\(\\) has no return type specified\\.$#" - count: 1 - path: lib/Filesystem/Tests/Adapter/IntegrationTestCase.php - - - - message: "#^Method Phpactor\\\\Filesystem\\\\Tests\\\\Adapter\\\\IntegrationTestCase\\:\\:workspacePath\\(\\) has no return type specified\\.$#" - count: 1 - path: lib/Filesystem/Tests/Adapter/IntegrationTestCase.php - - message: "#^Method Phpactor\\\\Filesystem\\\\Tests\\\\Unit\\\\Domain\\\\FilePathTest\\:\\:provideUnknown\\(\\) has no return type specified\\.$#" count: 1 @@ -5510,26 +5470,6 @@ parameters: count: 1 path: lib/Filesystem/Tests/Unit/Domain/FilePathTest.php - - - message: "#^Method Phpactor\\\\Filesystem\\\\Tests\\\\Unit\\\\Domain\\\\FilePathTest\\:\\:testFromUnknownWith\\(\\) has parameter \\$path with no type specified\\.$#" - count: 1 - path: lib/Filesystem/Tests/Unit/Domain/FilePathTest.php - - - - message: "#^Method Phpactor\\\\Filesystem\\\\Tests\\\\Unit\\\\Domain\\\\FilePathTest\\:\\:testThrowExceptionOnUnknowableType\\(\\) has parameter \\$input with no type specified\\.$#" - count: 1 - path: lib/Filesystem/Tests/Unit/Domain/FilePathTest.php - - - - message: "#^Method Phpactor\\\\Filesystem\\\\Tests\\\\Unit\\\\Domain\\\\MappedFilesystemRegistryTest\\:\\:createRegistry\\(\\) has no return type specified\\.$#" - count: 1 - path: lib/Filesystem/Tests/Unit/Domain/MappedFilesystemRegistryTest.php - - - - message: "#^Method Phpactor\\\\Filesystem\\\\Tests\\\\Unit\\\\Domain\\\\MappedFilesystemRegistryTest\\:\\:createRegistry\\(\\) has parameter \\$filesystems with no value type specified in iterable type array\\.$#" - count: 1 - path: lib/Filesystem/Tests/Unit/Domain/MappedFilesystemRegistryTest.php - - message: "#^Method Phpactor\\\\Indexer\\\\Adapter\\\\Php\\\\InMemory\\\\InMemoryIndex\\:\\:get\\(\\) should return TRecord of Phpactor\\\\Indexer\\\\Model\\\\Record but returns Phpactor\\\\Indexer\\\\Model\\\\Record\\.$#" count: 1 From 9e74bc48b0b50179d7a053bcd26b46eb3722e14a Mon Sep 17 00:00:00 2001 From: mamazu <14860264+mamazu@users.noreply.github.com> Date: Mon, 25 Mar 2024 21:29:50 +0100 Subject: [PATCH 3/5] Removing dead function --- lib/TextDocument/Location.php | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/TextDocument/Location.php b/lib/TextDocument/Location.php index 6c7afe2dbf..25abd71869 100644 --- a/lib/TextDocument/Location.php +++ b/lib/TextDocument/Location.php @@ -18,17 +18,6 @@ public static function fromPathAndOffsets(string $path, int $start, int $end): s ); } - /** - * @deprecated Use fromPathAndOffsets instead - */ - public static function fromPathAndOffset(string $path, int $offset): self - { - return new self( - TextDocumentUri::fromString($path), - ByteOffsetRange::fromInts($offset, $offset), - ); - } - public function uri(): TextDocumentUri { return $this->uri; From 5c9ba235c37ef13fa98544959bb88c9e52226fd4 Mon Sep 17 00:00:00 2001 From: mamazu <14860264+mamazu@users.noreply.github.com> Date: Mon, 25 Mar 2024 21:44:31 +0100 Subject: [PATCH 4/5] Cleaning up the ReflectionClass methods --- doc/reference/configuration.rst | 4 +- .../Reflection/AbstractReflectionClass.php | 16 -- .../Reflection/ReflectionClass.php | 6 +- .../Core/Reflection/ReflectionClassLike.php | 13 -- .../TypeResolver/MethodTypeResolver.php | 9 +- .../Reflection/ReflectionClassTest.php | 1 - .../Reflection/ReflectionEnumTest.php | 1 - .../Reflection/ReflectionInterfaceTest.php | 1 - .../Reflection/ReflectionTraitTest.php | 215 +++++++++--------- phpstan-baseline.neon | 10 - .../Core/Reflection/ReflectionMethod.twig | 2 +- 11 files changed, 118 insertions(+), 160 deletions(-) diff --git a/doc/reference/configuration.rst b/doc/reference/configuration.rst index 9d5317f155..5f144a5c94 100644 --- a/doc/reference/configuration.rst +++ b/doc/reference/configuration.rst @@ -894,7 +894,7 @@ FilePathResolverExtension """"""""""""""""""""""""""""""""""" -**Default**: ``"\/home\/daniel\/www\/phpactor\/phpactor"`` +**Default**: ``"\/home\/mamazu\/packages\/phpactor\/phpactor"`` .. _param_file_path_resolver.app_name: @@ -1348,7 +1348,7 @@ Amount of time (in milliseconds) to wait before responding to a shutdown notific Internal use only - name path to Phpactor binary -**Default**: ``"\/home\/daniel\/www\/phpactor\/phpactor\/lib\/Extension\/LanguageServer\/..\/..\/..\/bin\/phpactor"`` +**Default**: ``"\/home\/mamazu\/packages\/phpactor\/phpactor\/lib\/Extension\/LanguageServer\/..\/..\/..\/bin\/phpactor"`` .. _param_language_server.self_destruct_timeout: diff --git a/lib/WorseReflection/Bridge/TolerantParser/Reflection/AbstractReflectionClass.php b/lib/WorseReflection/Bridge/TolerantParser/Reflection/AbstractReflectionClass.php index a4ec4b554c..6d248345c1 100644 --- a/lib/WorseReflection/Bridge/TolerantParser/Reflection/AbstractReflectionClass.php +++ b/lib/WorseReflection/Bridge/TolerantParser/Reflection/AbstractReflectionClass.php @@ -11,7 +11,6 @@ use Phpactor\WorseReflection\Core\Reflection\Collection\ReflectionMethodCollection as PhpactorReflectionMethodCollection; use Phpactor\WorseReflection\Core\Reflection\Collection\ReflectionTraitCollection; use Phpactor\WorseReflection\Core\Reflection\ReflectionClassLike; -use Phpactor\WorseReflection\Core\Reflection\ReflectionEnum; use Phpactor\WorseReflection\Core\TemplateMap; use Phpactor\WorseReflection\Core\TypeFactory; use Phpactor\WorseReflection\Core\Type\ReflectedClassType; @@ -22,21 +21,6 @@ abstract class AbstractReflectionClass extends AbstractReflectedNode implements abstract public function name(): ClassName; abstract public function docblock(): DocBlock; - public function isInterface(): bool - { - return $this instanceof ReflectionInterface; - } - - public function isTrait(): bool - { - return $this instanceof ReflectionTrait; - } - - public function isEnum(): bool - { - return $this instanceof ReflectionEnum; - } - public function isClass(): bool { return $this instanceof ReflectionClass; diff --git a/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionClass.php b/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionClass.php index a6d96a9677..fde4ae772a 100644 --- a/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionClass.php +++ b/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionClass.php @@ -300,11 +300,7 @@ public function sourceCode(): TextDocument public function isConcrete(): bool { - if (false === $this->isClass()) { - return false; - } - - return false === $this->isAbstract(); + return !$this->isAbstract(); } public function docblock(): DocBlock diff --git a/lib/WorseReflection/Core/Reflection/ReflectionClassLike.php b/lib/WorseReflection/Core/Reflection/ReflectionClassLike.php index 3ac370a1cd..e509548394 100644 --- a/lib/WorseReflection/Core/Reflection/ReflectionClassLike.php +++ b/lib/WorseReflection/Core/Reflection/ReflectionClassLike.php @@ -32,21 +32,8 @@ public function ownMembers(): ReflectionMemberCollection; public function sourceCode(): TextDocument; - /** - * @deprecated Use instanceof instead - */ - public function isInterface(): bool; - public function isInstanceOf(ClassName $className): bool; - - /** - * @deprecated Use instanceof instead - */ - public function isClass(): bool; - - public function isEnum(): bool; - public function isConcrete(): bool; public function docblock(): DocBlock; diff --git a/lib/WorseReflection/Core/Reflection/TypeResolver/MethodTypeResolver.php b/lib/WorseReflection/Core/Reflection/TypeResolver/MethodTypeResolver.php index fc9fd34753..b0b00f977f 100644 --- a/lib/WorseReflection/Core/Reflection/TypeResolver/MethodTypeResolver.php +++ b/lib/WorseReflection/Core/Reflection/TypeResolver/MethodTypeResolver.php @@ -7,7 +7,6 @@ use Phpactor\WorseReflection\Core\Type; use Phpactor\WorseReflection\Core\Reflection\ReflectionClass; use Phpactor\WorseReflection\Core\Reflection\ReflectionClassLike; -use Phpactor\WorseReflection\Core\Reflection\ReflectionInterface; class MethodTypeResolver { @@ -73,15 +72,11 @@ private function getTypesFromParentClass(ReflectionClassLike $reflectionClassLik private function getTypesFromInterfaces(ReflectionClassLike $reflectionClassLike): Type { - if (false === $reflectionClassLike->isClass()) { + if (!$reflectionClassLike instanceof ReflectionClass) { return TypeFactory::undefined(); } - /** @var ReflectionClass $reflectionClass */ - $reflectionClass = $reflectionClassLike; - - /** @var ReflectionInterface $interface */ - foreach ($reflectionClass->interfaces() as $interface) { + foreach ($reflectionClassLike->interfaces() as $interface) { if ($interface->methods()->has($this->method->name())) { return $interface->methods()->get($this->method->name())->inferredType(); } diff --git a/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionClassTest.php b/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionClassTest.php index 92121c889f..53aa1457bf 100644 --- a/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionClassTest.php +++ b/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionClassTest.php @@ -48,7 +48,6 @@ class Foobar function ($class): void { $this->assertEquals('Foobar', (string) $class->name()->short()); $this->assertInstanceOf(ReflectionClass::class, $class); - $this->assertFalse($class->isInterface()); }, ]; diff --git a/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionEnumTest.php b/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionEnumTest.php index 1fdccccb6c..ee4b614e45 100644 --- a/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionEnumTest.php +++ b/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionEnumTest.php @@ -38,7 +38,6 @@ enum Barfoo function ($class): void { $this->assertEquals('Barfoo', (string) $class->name()->short()); $this->assertInstanceOf(ReflectionEnum::class, $class); - $this->assertTrue($class->isEnum()); }, ]; yield 'It reflect enum methods' => [ diff --git a/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionInterfaceTest.php b/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionInterfaceTest.php index db1839a345..aaafb542b9 100644 --- a/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionInterfaceTest.php +++ b/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionInterfaceTest.php @@ -37,7 +37,6 @@ interface Barfoo function ($class): void { $this->assertEquals('Barfoo', (string) $class->name()->short()); $this->assertInstanceOf(ReflectionInterface::class, $class); - $this->assertTrue($class->isInterface()); }, ]; diff --git a/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionTraitTest.php b/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionTraitTest.php index 13615c7e6a..42b83d4e1a 100644 --- a/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionTraitTest.php +++ b/lib/WorseReflection/Tests/Integration/Bridge/TolerantParser/Reflection/ReflectionTraitTest.php @@ -2,6 +2,8 @@ namespace Phpactor\WorseReflection\Tests\Integration\Bridge\TolerantParser\Reflection; +use Generator; +use Phpactor\WorseReflection\Core\Reflection\ReflectionClass; use Phpactor\WorseReflection\Core\TypeFactory; use Phpactor\WorseReflection\Tests\Integration\IntegrationTestCase; use Phpactor\WorseReflection\Core\ClassName; @@ -12,6 +14,7 @@ class ReflectionTraitTest extends IntegrationTestCase { /** * @dataProvider provideReflectionTrait + * @param Closure(ReflectionTrait): void $assertion */ public function testReflectTrait(string $source, string $class, Closure $assertion): void { @@ -19,117 +22,123 @@ public function testReflectTrait(string $source, string $class, Closure $asserti $assertion($class); } - public function provideReflectionTrait() + /** + * @return Generator + */ + public function provideReflectionTrait(): Generator { - return [ - 'It reflects a trait' => [ - <<<'EOT' - [ + <<<'EOT' + assertEquals('Barfoo', (string) $class->name()->short()); - $this->assertInstanceOf(ReflectionTrait::class, $class); - $this->assertTrue($class->isTrait()); - }, - ], - 'It reflects a classes traits' => [ - <<<'EOT' - assertEquals('Barfoo', (string) $class->name()->short()); + $this->assertInstanceOf(ReflectionTrait::class, $class); + }, + ]; - trait Bazbar - { - } + yield 'It reflects a classes traits' => [ + <<<'EOT' + traits(); - $this->assertCount(2, $traits); - $trait = $traits[0]; - $this->assertInstanceOf(ReflectionTrait::class, $trait); - }, - ], - 'It reflect trait methods' => [ - <<<'EOT' - assertEquals('Barfoo', (string) $class->name()->short()); - $this->assertEquals(['foobar'], $class->methods()->keys()); - }, - ], - 'Trait properties' => [ - <<<'EOT' - assertCount(2, $class->properties()); - $this->assertEquals('foobar', $class->properties()->first()->name()); - }, - ], - 'Ignores inherit docs on trait' => [ - <<<'EOT' - traits(); + $this->assertCount(2, $traits); + $trait = $traits->first(); + $this->assertInstanceOf(ReflectionTrait::class, $trait); + }, + ]; + + yield 'It reflect trait methods' => [ + <<<'EOT' + assertEquals(TypeFactory::unknown(), $class->methods()->first()->inferredReturnTypes()->best()); - }, - ], - 'instanceof' => [ - <<<'EOT' - assertEquals('Barfoo', (string) $class->name()->short()); + $this->assertEquals(['foobar'], $class->methods()->keys()); + }, + ]; + + yield 'Trait properties' => [ + <<<'EOT' + assertCount(2, $class->properties()); + $this->assertEquals('foobar', $class->properties()->first()->name()); + }, + ]; + + yield 'Ignores inherit docs on trait' => [ + <<<'EOT' + assertTrue($class->isInstanceOf(ClassName::fromString('Trait1'))); - $this->assertFalse($class->isInstanceOf(ClassName::fromString('Interface1'))); - }, - ], + } + EOT + , + 'Int1', + function (ReflectionTrait $class): void { + $method = $class->methods()->first(); + $this->assertEquals(TypeFactory::unknown(), $method->type()); + }, + ]; + + yield 'instanceof' => [ + <<<'EOT' + assertTrue($class->isInstanceOf(ClassName::fromString('Trait1'))); + $this->assertFalse($class->isInstanceOf(ClassName::fromString('Interface1'))); + }, ]; yield 'Returns all members' => [ diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 43ce970be3..845fe1da94 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -5460,16 +5460,6 @@ parameters: count: 1 path: lib/Filesystem/Adapter/Simple/SimpleFilesystem.php - - - message: "#^Method Phpactor\\\\Filesystem\\\\Tests\\\\Unit\\\\Domain\\\\FilePathTest\\:\\:provideUnknown\\(\\) has no return type specified\\.$#" - count: 1 - path: lib/Filesystem/Tests/Unit/Domain/FilePathTest.php - - - - message: "#^Method Phpactor\\\\Filesystem\\\\Tests\\\\Unit\\\\Domain\\\\FilePathTest\\:\\:provideUnsupportedInput\\(\\) has no return type specified\\.$#" - count: 1 - path: lib/Filesystem/Tests/Unit/Domain/FilePathTest.php - - message: "#^Method Phpactor\\\\Indexer\\\\Adapter\\\\Php\\\\InMemory\\\\InMemoryIndex\\:\\:get\\(\\) should return TRecord of Phpactor\\\\Indexer\\\\Model\\\\Record but returns Phpactor\\\\Indexer\\\\Model\\\\Record\\.$#" count: 1 diff --git a/templates/help/markdown/Phpactor/WorseReflection/Core/Reflection/ReflectionMethod.twig b/templates/help/markdown/Phpactor/WorseReflection/Core/Reflection/ReflectionMethod.twig index 0461e06817..9c31318e49 100644 --- a/templates/help/markdown/Phpactor/WorseReflection/Core/Reflection/ReflectionMethod.twig +++ b/templates/help/markdown/Phpactor/WorseReflection/Core/Reflection/ReflectionMethod.twig @@ -11,7 +11,7 @@ {%- endif -%} {% if object.isVirtual %}[virtual] {% endif -%} {% if object.isAbstract -%}abstract {% endif -%} -{{- object.visibility ~ ' ' -}} +{{- object.visibility ~ ' ' -}} {%- if object.isStatic %}static {% endif %} function {{ object.name }}({{ render(object.parameters) }}) {%- if typeDefined(object.inferredType) %}: {{ render(object.inferredType) }}{% endif -%} From 6639284db6762dbeb136dfa10bda074649790ef3 Mon Sep 17 00:00:00 2001 From: mamazu <14860264+mamazu@users.noreply.github.com> Date: Thu, 28 Mar 2024 14:17:05 +0100 Subject: [PATCH 5/5] Improved naming --- lib/Filesystem/Adapter/Git/GitFilesystem.php | 10 +++++----- .../Adapter/Simple/SimpleFilesystem.php | 16 ++++++++-------- lib/Filesystem/Domain/FileList.php | 1 - lib/Filesystem/Domain/FilePath.php | 2 +- .../Tests/Unit/Domain/FilePathTest.php | 14 ++++++++++---- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/lib/Filesystem/Adapter/Git/GitFilesystem.php b/lib/Filesystem/Adapter/Git/GitFilesystem.php index 2b75366064..ece05438b5 100644 --- a/lib/Filesystem/Adapter/Git/GitFilesystem.php +++ b/lib/Filesystem/Adapter/Git/GitFilesystem.php @@ -49,7 +49,7 @@ public function fileList(): FileList public function remove(FilePath|string $path): void { - $path = FilePath::fromUnknown($path); + $path = FilePath::fromFilePathOrString($path); if (false === $this->trackedByGit($path)) { parent::remove($path); return; @@ -65,8 +65,8 @@ public function remove(FilePath|string $path): void public function move(FilePath|string $srcPath, FilePath|string $destPath): void { - $srcPath = FilePath::fromUnknown($srcPath); - $destPath = FilePath::fromUnknown($destPath); + $srcPath = FilePath::fromFilePathOrString($srcPath); + $destPath = FilePath::fromFilePathOrString($destPath); if (false === $this->trackedByGit($srcPath)) { parent::move($srcPath, $destPath); @@ -82,8 +82,8 @@ public function move(FilePath|string $srcPath, FilePath|string $destPath): void public function copy(FilePath|string $srcPath, FilePath|string $destPath): CopyReport { - $srcPath = FilePath::fromUnknown($srcPath); - $destPath = FilePath::fromUnknown($destPath); + $srcPath = FilePath::fromFilePathOrString($srcPath); + $destPath = FilePath::fromFilePathOrString($destPath); $list = parent::copy($srcPath, $destPath); $this->exec(['add', $destPath->__toString()]); diff --git a/lib/Filesystem/Adapter/Simple/SimpleFilesystem.php b/lib/Filesystem/Adapter/Simple/SimpleFilesystem.php index 705ef21564..96796ea6ce 100644 --- a/lib/Filesystem/Adapter/Simple/SimpleFilesystem.php +++ b/lib/Filesystem/Adapter/Simple/SimpleFilesystem.php @@ -35,14 +35,14 @@ public function fileList(): FileList public function remove(FilePath|string $path): void { - $path = FilePath::fromUnknown($path); + $path = FilePath::fromFilePathOrString($path); $this->filesystem->remove($path); } public function move(FilePath|string $srcLocation, FilePath|string $destPath): void { - $srcLocation = FilePath::fromUnknown($srcLocation); - $destPath = FilePath::fromUnknown($destPath); + $srcLocation = FilePath::fromFilePathOrString($srcLocation); + $destPath = FilePath::fromFilePathOrString($destPath); $this->makeDirectoryIfNotExists((string) $destPath); $this->filesystem->rename($srcLocation->__toString(), $destPath->__toString()); @@ -50,8 +50,8 @@ public function move(FilePath|string $srcLocation, FilePath|string $destPath): v public function copy(FilePath|string $srcLocation, FilePath|string $destPath): CopyReport { - $srcLocation = FilePath::fromUnknown($srcLocation); - $destPath = FilePath::fromUnknown($destPath); + $srcLocation = FilePath::fromFilePathOrString($srcLocation); + $destPath = FilePath::fromFilePathOrString($destPath); if ($srcLocation->isDirectory()) { return $this->copyDirectory($srcLocation, $destPath); @@ -77,7 +77,7 @@ public function createPath(string $path): FilePath public function getContents(FilePath|string $path): string { - $path = FilePath::fromUnknown($path); + $path = FilePath::fromFilePathOrString($path); $contents = file_get_contents($path->path()); if (false === $contents) { @@ -89,13 +89,13 @@ public function getContents(FilePath|string $path): string public function writeContents(FilePath|string $path, string $contents): void { - $path = FilePath::fromUnknown($path); + $path = FilePath::fromFilePathOrString($path); file_put_contents($path->path(), $contents); } public function exists(FilePath|string $path): bool { - $path = FilePath::fromUnknown($path); + $path = FilePath::fromFilePathOrString($path); return file_exists($path); } diff --git a/lib/Filesystem/Domain/FileList.php b/lib/Filesystem/Domain/FileList.php index 5b2c792668..eb5590950e 100644 --- a/lib/Filesystem/Domain/FileList.php +++ b/lib/Filesystem/Domain/FileList.php @@ -26,7 +26,6 @@ private function __construct(private Iterator $iterator) { } - public static function fromIterator(Iterator $iterator): self { return new self($iterator); diff --git a/lib/Filesystem/Domain/FilePath.php b/lib/Filesystem/Domain/FilePath.php index 603884b653..d727a0c355 100644 --- a/lib/Filesystem/Domain/FilePath.php +++ b/lib/Filesystem/Domain/FilePath.php @@ -48,7 +48,7 @@ public static function fromSplFileInfo(SplFileInfo $fileInfo): FilePath return self::fromString((string) $fileInfo); } - public static function fromUnknown(FilePath|string $path): FilePath + public static function fromFilePathOrString(FilePath|string $path): FilePath { if ($path instanceof FilePath) { return $path; diff --git a/lib/Filesystem/Tests/Unit/Domain/FilePathTest.php b/lib/Filesystem/Tests/Unit/Domain/FilePathTest.php index 03464274c8..e0edd25969 100644 --- a/lib/Filesystem/Tests/Unit/Domain/FilePathTest.php +++ b/lib/Filesystem/Tests/Unit/Domain/FilePathTest.php @@ -30,16 +30,19 @@ public function testFromParts(): void } /** - * @dataProvider provideUnknown + * @dataProvider provideFilePathOrString */ - public function testFromUnknownWith(FilePath|string $path, string $expectedPath): void + public function testFromFilePathOrString(FilePath|string $path, string $expectedPath): void { - $filePath = FilePath::fromUnknown($path); + $filePath = FilePath::fromFilePathOrString($path); $this->assertInstanceOf(FilePath::class, $filePath); $this->assertEquals($expectedPath, (string) $filePath); } - public function provideUnknown(): Generator + /** + * @return Generator + */ + public function provideFilePathOrString(): Generator { yield 'FilePath instance' => [ FilePath::fromString('/foo.php'), @@ -77,6 +80,9 @@ public function testThrowExceptionOnUnknowableType(string $input, string $expect FilePath::fromString($input); } + /** + * @return Generator + */ public function provideUnsupportedInput(): Generator { yield 'unsupported scheme' => [