Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add return type hints to tests #2366

Merged
merged 1 commit into from
Sep 14, 2021
Merged
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
11 changes: 4 additions & 7 deletions benchmark/BaseBench.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,12 @@ abstract class BaseBench
/** @var DocumentManager */
protected static $documentManager;

/**
* @return DocumentManager
*/
protected function getDocumentManager()
protected function getDocumentManager(): DocumentManager
{
return self::$documentManager;
}

public function initDocumentManager()
public function initDocumentManager(): void
{
$config = new Configuration();

Expand All @@ -59,7 +56,7 @@ public function initDocumentManager()
self::$documentManager = DocumentManager::create($client, $config);
}

public function clearDatabase()
public function clearDatabase(): void
{
// Check if the database exists. Calling listCollections on a non-existing
// database in a sharded setup will cause an invalid command cursor to be
Expand All @@ -85,7 +82,7 @@ public function clearDatabase()
}
}

protected static function createMetadataDriverImpl()
protected static function createMetadataDriverImpl(): AnnotationDriver
{
return AnnotationDriver::create(__DIR__ . '/../tests/Documents');
}
Expand Down
12 changes: 6 additions & 6 deletions benchmark/Document/LoadDocumentBench.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ final class LoadDocumentBench extends BaseBench
/** @var ObjectId */
private static $userId;

public function init()
public function init(): void
{
self::$userId = new ObjectId();

Expand Down Expand Up @@ -58,23 +58,23 @@ public function init()
/**
* @Warmup(2)
*/
public function benchLoadDocument()
public function benchLoadDocument(): void
{
$this->loadDocument();
}

/**
* @Warmup(2)
*/
public function benchLoadEmbedOne()
public function benchLoadEmbedOne(): void
{
$this->loadDocument()->getAddress()->getCity();
}

/**
* @Warmup(2)
*/
public function benchLoadEmbedMany()
public function benchLoadEmbedMany(): void
{
$this->loadDocument()->getPhonenumbers()->forAll(static function (int $key, Phonenumber $element) {
return $element->getPhoneNumber() !== null;
Expand All @@ -94,8 +94,8 @@ public function benchLoadReferenceOne()
*/
public function benchLoadReferenceMany()
{
$this->loadDocument()->getGroups()->forAll(static function ($key, Group $group) {
return $group->getName();
$this->loadDocument()->getGroups()->forAll(static function (int $key, Group $group) {
return $group->getName() !== null;
});
}

Expand Down
2 changes: 2 additions & 0 deletions lib/Doctrine/ODM/MongoDB/DocumentManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,8 @@ public function getClassMetadata($className): ClassMetadata

/**
* Returns the MongoDB instance for a class.
*
* @psalm-param class-string $className
*/
public function getDocumentDatabase(string $className): Database
{
Expand Down
5 changes: 4 additions & 1 deletion phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
<!-- Will cause BC breaks to method signatures - disabled for now -->
<exclude name="SlevomatCodingStandard.TypeHints.ParameterTypeHint.MissingAnyTypeHint" />
<exclude name="SlevomatCodingStandard.TypeHints.ParameterTypeHint.MissingNativeTypeHint" />
<exclude name="SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingAnyTypeHint" />
<exclude name="SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingNativeTypeHint" />

<!-- Disabled to avoid class renaming - to be handled in a separate PR -->
Expand Down Expand Up @@ -67,6 +66,10 @@
<exclude-pattern>*/tests/*</exclude-pattern>
</rule>

<rule ref="SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingAnyTypeHint">
<exclude-pattern>*/lib/*</exclude-pattern>
</rule>

<rule ref="SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingNativeTypeHint">
<!-- We do want to test generating collections without return types -->
<exclude-pattern>*/tests/Doctrine/ODM/MongoDB/Tests/PersistentCollection/Coll*</exclude-pattern>
Expand Down
11 changes: 6 additions & 5 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,6 @@ parameters:
count: 1
path: tests/Doctrine/ODM/MongoDB/Tests/Functional/CustomFieldNameTest.php

-
message: "#^Offset 'userId' does not exist on array\\(\\)\\|array\\('ids' \\=\\> array\\<int, mixed\\>&nonEmpty, \\?'userId' \\=\\> true\\)\\.$#"
count: 1
path: tests/Doctrine/ODM/MongoDB/Tests/Functional/CustomIdTest.php

-
message: "#^Cannot call method current\\(\\) on array\\|int\\|object\\.$#"
count: 1
Expand Down Expand Up @@ -229,3 +224,9 @@ parameters:
message: "#^Unreachable statement \\- code above always terminates\\.$#"
count: 1
path: lib/Doctrine/ODM/MongoDB/Types/DateImmutableType.php

# See https://github.com/phpstan/phpstan/issues/5512
-
message: "#^Method Doctrine\\\\ODM\\\\MongoDB\\\\Tests\\\\Functional\\\\My.*Collection\\:\\:.*\\(\\) should return Doctrine\\\\ODM\\\\MongoDB\\\\Tests\\\\Functional\\\\My.*Collection but returns Doctrine\\\\Common\\\\Collections\\\\ArrayCollection\\<\\(int\\|string\\), mixed\\>\\.$#"
count: 3
path: tests/Doctrine/ODM/MongoDB/Tests/Functional/CustomCollectionsTest.php
23 changes: 13 additions & 10 deletions tests/Doctrine/ODM/MongoDB/Tests/BaseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,7 @@ public static function assertArraySubset($subset, $array, bool $checkForObjectId
}
}

/**
* @return MappingDriver
*/
protected function createMetadataDriverImpl()
protected function createMetadataDriverImpl(): MappingDriver
{
return AnnotationDriver::create(__DIR__ . '/../../../../Documents');
}
Expand All @@ -120,14 +117,17 @@ protected function createTestDocumentManager(): DocumentManager
return DocumentManager::create($client, $config);
}

protected function getServerVersion()
protected function getServerVersion(): string
{
$result = $this->dm->getClient()->selectDatabase(DOCTRINE_MONGODB_DATABASE)->command(['buildInfo' => 1])->toArray()[0];

return $result['version'];
}

protected function skipTestIfNotSharded($className): void
/**
* @psalm-param class-string $className
*/
protected function skipTestIfNotSharded(string $className): void
{
$result = $this->dm->getDocumentDatabase($className)->command(['listCommands' => true])->toArray()[0];

Expand All @@ -138,7 +138,10 @@ protected function skipTestIfNotSharded($className): void
$this->markTestSkipped('Test skipped because server does not support sharding');
}

protected function skipTestIfSharded($className): void
/**
* @psalm-param class-string $className
*/
protected function skipTestIfSharded(string $className): void
{
$result = $this->dm->getDocumentDatabase($className)->command(['listCommands' => true])->toArray()[0];

Expand All @@ -149,7 +152,7 @@ protected function skipTestIfSharded($className): void
$this->markTestSkipped('Test does not apply on sharded clusters');
}

protected function requireVersion($installedVersion, $requiredVersion, $operator, $message): void
protected function requireVersion(string $installedVersion, string $requiredVersion, ?string $operator, string $message): void
{
if (! version_compare($installedVersion, $requiredVersion, $operator)) {
return;
Expand All @@ -158,12 +161,12 @@ protected function requireVersion($installedVersion, $requiredVersion, $operator
$this->markTestSkipped($message);
}

protected function skipOnMongoDB42($message): void
protected function skipOnMongoDB42(string $message): void
{
$this->requireVersion($this->getServerVersion(), '4.2.0', '>=', $message);
}

protected function requireMongoDB42($message): void
protected function requireMongoDB42(string $message): void
{
$this->requireVersion($this->getServerVersion(), '4.2.0', '<', $message);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,14 +277,14 @@ public function __construct(string $name, bool $enabled)

class MyEmbedsCollection extends ArrayCollection
{
public function getByName($name)
public function getByName(string $name): MyEmbedsCollection
{
return $this->filter(static function ($item) use ($name) {
return $item->name === $name;
});
}

public function getEnabled()
public function getEnabled(): MyEmbedsCollection
{
return $this->filter(static function ($item) {
return $item->enabled;
Expand All @@ -305,7 +305,7 @@ public function nothingReally(): void

class MyDocumentsCollection extends ArrayCollection
{
public function havingEmbeds()
public function havingEmbeds(): MyDocumentsCollection
{
return $this->filter(static function ($item) {
return $item->coll->count();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public function testBatchInsertCustomId(): void

$this->assertCount(2, $users);

$results = [];
$results = ['userId' => false];
foreach ($users as $user) {
if ($user->getId() === 'userId') {
$results['userId'] = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ class DateCollectionType extends Type

/**
* Method called by PersistenceBuilder
*
* {@inheritDoc}
*/
public function convertToDatabaseValue($value)
{
Expand All @@ -74,6 +76,9 @@ public function convertToDatabaseValue($value)
return $value;
}

/**
* {@inheritDoc}
*/
public function convertToPHPValue($value)
{
if ($value === null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,21 @@ abstract class ParentDocument
*/
protected $id;

/** @var ChildDocument|null */
/** @var ChildDocument */
protected $referencedChild;

/** @var ChildDocument[] */
/** @var Collection<int, ChildDocument>|array<ChildDocument> */
protected $referencedChildren;

/** @var ChildDocument|null */
/** @var ChildDocument */
protected $embeddedChild;

/** @var ChildDocument[] */
/** @var Collection<int, ChildDocument>|array<ChildDocument> */
protected $embeddedChildren;

/**
* @param array{0: ChildDocument, 1: ChildDocument} $children
*/
public function __construct(array $children)
{
$this->referencedChild = $children[0];
Expand All @@ -115,21 +118,27 @@ public function getId(): ?string
return $this->id;
}

public function getReferencedChild()
public function getReferencedChild(): ChildDocument
{
return $this->referencedChild;
}

/**
* @return Collection<int, ChildDocument>|array<ChildDocument>
*/
public function getReferencedChildren()
{
return $this->referencedChildren;
}

public function getEmbeddedChild()
public function getEmbeddedChild(): ChildDocument
{
return $this->embeddedChild;
}

/**
* @return Collection<int, ChildDocument>|array<ChildDocument>
*/
public function getEmbeddedChildren()
{
return $this->embeddedChildren;
Expand Down Expand Up @@ -158,12 +167,12 @@ public function __construct(string $type)
$this->type = $type;
}

public function getId()
public function getId(): ?string
{
return $this->id;
}

public function getType()
public function getType(): string
{
return $this->type;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,9 @@ final class DocumentPersisterCustomIdType extends Type
{
use ClosureToPHP;

/**
* {@inheritdoc}
*/
public function convertToDatabaseValue($value)
{
if ($value instanceof ObjectId) {
Expand All @@ -1048,6 +1051,9 @@ public function convertToDatabaseValue($value)
throw self::createException($value);
}

/**
* {@inheritdoc}
*/
public function convertToPHPValue($value)
{
if ($value instanceof DocumentPersisterCustomTypedId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public function testRemoveOption(): void
$this->assertCount(2, $product->getOptions());
}

protected function getProduct()
protected function getProduct(): ConfigurableProduct
{
$products = $this->dm->getRepository(ConfigurableProduct::class)
->createQueryBuilder()
Expand Down
4 changes: 2 additions & 2 deletions tests/Doctrine/ODM/MongoDB/Tests/Functional/FilterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public function testRepositoryFindOneBy(): void
$this->assertEquals('John', $this->getJohnsUsernameWithFindOneBy());
}

protected function getJohnsUsernameWithFindOneBy()
protected function getJohnsUsernameWithFindOneBy(): ?string
{
$john = $this->dm->getRepository(User::class)->findOneBy(['id' => $this->ids['john']]);

Expand Down Expand Up @@ -235,7 +235,7 @@ public function testReferenceOne(): void
$this->assertEquals('Timothy', $this->getProfileByReference());
}

protected function getProfileByReference()
protected function getProfileByReference(): ?string
{
$tim = $this->dm->getRepository(User::class)->find($this->ids['tim']);

Expand Down
Loading