Skip to content

Commit

Permalink
Adding better types (#2614)
Browse files Browse the repository at this point in the history 
* Adding better types

* More types

* Removing dead function

* Cleaning up the ReflectionClass methods

* Improved naming
  • Loading branch information
@mamazu
mamazu committed Mar 29, 2024
1 parent 6f4c98e commit 7f00bf8
Show file tree
Hide file tree
Showing 27 changed files with 175 additions and 366 deletions.
4 changes: 2 additions & 2 deletions doc/reference/configuration.rst
View file
Expand Up @@ -894,7 +894,7 @@ FilePathResolverExtension
"""""""""""""""""""""""""""""""""""


**Default**: ``"\/home\/daniel\/www\/phpactor\/phpactor"``
**Default**: ``"\/home\/mamazu\/packages\/phpactor\/phpactor"``


.. _param_file_path_resolver.app_name:
Expand Down Expand Up @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions lib/Filesystem/Adapter/Git/GitFilesystem.php
View file
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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()]);

Expand Down
16 changes: 8 additions & 8 deletions lib/Filesystem/Adapter/Simple/SimpleFilesystem.php
View file
Expand Up @@ -35,23 +35,23 @@ 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());
}

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);
Expand All @@ -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) {
Expand All @@ -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);
}

Expand Down
1 change: 0 additions & 1 deletion lib/Filesystem/Domain/FileList.php
View file
Expand Up @@ -26,7 +26,6 @@ private function __construct(private Iterator $iterator)
{
}


public static function fromIterator(Iterator $iterator): self
{
return new self($iterator);
Expand Down
18 changes: 4 additions & 14 deletions lib/Filesystem/Domain/FilePath.php
View file
Expand Up @@ -29,6 +29,9 @@ public static function fromString(string $string): FilePath
return new self($textDocumentUri);
}

/**
* @param array<string> $parts
*/
public static function fromParts(array $parts): FilePath
{
$path = Path::join(...$parts);
Expand All @@ -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 fromFilePathOrString(FilePath|string $path): FilePath
{
if ($path instanceof FilePath) {
return $path;
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions lib/Filesystem/Domain/FilesystemRegistry.php
View file
Expand Up @@ -8,5 +8,8 @@ public function get(string $name): Filesystem;

public function has(string $name): bool;

/**
* @return array<string>
*/
public function names(): array;
}
1 change: 0 additions & 1 deletion lib/Filesystem/Domain/MappedFilesystemRegistry.php
View file
Expand Up @@ -35,7 +35,6 @@ public function has(string $name): bool
return isset($this->filesystems[$name]);
}

/** @return array<string> */
public function names(): array
{
return array_keys($this->filesystems);
Expand Down
2 changes: 0 additions & 2 deletions lib/Filesystem/Tests/Adapter/AdapterTestCase.php
View file
Expand Up @@ -6,8 +6,6 @@

abstract class AdapterTestCase extends IntegrationTestCase
{
private $filesystem;

public function setUp(): void
{
$this->initWorkspace();
Expand Down
4 changes: 2 additions & 2 deletions lib/Filesystem/Tests/Adapter/IntegrationTestCase.php
View file
Expand Up @@ -17,7 +17,7 @@ protected function initWorkspace(): void
$filesystem->mkdir($this->workspacePath());
}

protected function workspacePath()
protected function workspacePath(): string
{
return realpath(__DIR__.'/..') . '/Workspace';
}
Expand All @@ -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';
}
Expand Down
43 changes: 15 additions & 28 deletions lib/Filesystem/Tests/Unit/Domain/FilePathTest.php
View file
Expand Up @@ -2,14 +2,14 @@

namespace Phpactor\Filesystem\Tests\Unit\Domain;

use Generator;
use PHPUnit\Framework\TestCase;
use Phpactor\Filesystem\Domain\FilePath;
use Phpactor\TextDocument\Exception\InvalidUriException;
use Phpactor\TextDocument\TextDocumentUri;
use RuntimeException;
use SplFileInfo;
use Symfony\Component\Filesystem\Path;
use stdClass;

class FilePathTest extends TestCase
{
Expand All @@ -30,16 +30,19 @@ public function testFromParts(): void
}

/**
* @dataProvider provideUnknown
* @dataProvider provideFilePathOrString
*/
public function testFromUnknownWith($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()
/**
* @return Generator<string,array{FilePath|string,string}>
*/
public function provideFilePathOrString(): Generator
{
yield 'FilePath instance' => [
FilePath::fromString('/foo.php'),
Expand All @@ -65,39 +68,23 @@ 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()
/**
* @return Generator<string,array{string,string}>
*/
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
Expand Down Expand Up @@ -186,7 +173,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());
}
Expand Down
3 changes: 2 additions & 1 deletion lib/Filesystem/Tests/Unit/Domain/MappedFilesystemRegistryTest.php
View file
Expand Up @@ -55,7 +55,8 @@ public function testExceptionOnNotFound(): void
$registry->get('barfoo');
}

private function createRegistry(array $filesystems)
/** @param array<string, Filesystem> $filesystems */
private function createRegistry(array $filesystems): MappedFilesystemRegistry
{
return new MappedFilesystemRegistry($filesystems);
}
Expand Down
11 changes: 0 additions & 11 deletions lib/TextDocument/Location.php
View file
Expand Up @@ -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;
Expand Down
16 changes: 0 additions & 16 deletions lib/WorseReflection/Bridge/TolerantParser/Reflection/AbstractReflectionClass.php
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
6 changes: 1 addition & 5 deletions lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionClass.php
View file
Expand Up @@ -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
Expand Down
13 changes: 0 additions & 13 deletions lib/WorseReflection/Core/Reflection/ReflectionClassLike.php
View file
Expand Up @@ -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;
Expand Down

0 comments on commit 7f00bf8

Please sign in to comment.