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 parameter types #455

Merged
merged 2 commits into from
Dec 14, 2023
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
14 changes: 5 additions & 9 deletions src/AbstractFixture.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,11 @@
*
* @see Doctrine\Common\DataFixtures\ReferenceRepository::setReference
*
* @param string $name
* @param object $object - managed object
*
* @return void
*/
public function setReference($name, $object)
public function setReference(string $name, object $object)

Check warning on line 52 in src/AbstractFixture.php

View check run for this annotation

Codecov / codecov/patch

src/AbstractFixture.php#L52

Added line #L52 was not covered by tests
{
$this->getReferenceRepository()->setReference($name, $object);
}
Expand All @@ -63,14 +62,13 @@
*
* @see Doctrine\Common\DataFixtures\ReferenceRepository::addReference
*
* @param string $name
* @param object $object - managed object
*
* @return void
*
* @throws BadMethodCallException - if repository already has a reference by $name.
*/
public function addReference($name, $object)
public function addReference(string $name, object $object)
{
$this->getReferenceRepository()->addReference($name, $object);
}
Expand All @@ -81,15 +79,14 @@
*
* @see Doctrine\Common\DataFixtures\ReferenceRepository::getReference
*
* @param string $name
* @psalm-param class-string<T>|null $class
*
* @return object
* @psalm-return ($class is null ? object : T)
*
* @template T of object
*/
public function getReference($name, ?string $class = null)
public function getReference(string $name, ?string $class = null)
{
if ($class === null) {
Deprecation::trigger(
Expand All @@ -109,12 +106,11 @@
*
* @see Doctrine\Common\DataFixtures\ReferenceRepository::hasReference
*
* @param string $name
* @psalm-param class-string $class
* @psalm-param class-string|null $class
*
* @return bool
*/
public function hasReference($name, ?string $class = null)
public function hasReference(string $name, ?string $class = null)

Check warning on line 113 in src/AbstractFixture.php

View check run for this annotation

Codecov / codecov/patch

src/AbstractFixture.php#L113

Added line #L113 was not covered by tests
{
if ($class === null) {
Deprecation::trigger(
Expand Down
8 changes: 2 additions & 6 deletions src/Executor/AbstractExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,23 +76,19 @@
/**
* Set the logger callable to execute with the log() method.
*
* @param callable $logger
*
* @return void
*/
public function setLogger($logger)
public function setLogger(callable $logger)

Check warning on line 81 in src/Executor/AbstractExecutor.php

View check run for this annotation

Codecov / codecov/patch

src/Executor/AbstractExecutor.php#L81

Added line #L81 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

Why is the logger a callable anyway? 🙈

Copy link
Member Author

Choose a reason for hiding this comment

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

Line 94.

        $logger = $this->logger;
        $logger($message);

You think we should change it to accept a Psr\Log\LoggerInterface?

Copy link
Member

Choose a reason for hiding this comment

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

We should look into that, yes. But probably in a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

it's used for console output directly.

We could try to leverage Symfony\Component\Console\Logger\ConsoleLogger. My problem is that a closure is a bad logging abstraction. While it works for the way the bundle uses it, it sucks if you want the logs to go anywhere else, e.g. you want to use a logging framework instead.

Deprecation: #462

{
$this->logger = $logger;
}

/**
* Logs a message using the logger.
*
* @param string $message
*
* @return void
*/
public function log($message)
public function log(string $message)

Check warning on line 91 in src/Executor/AbstractExecutor.php

View check run for this annotation

Codecov / codecov/patch

src/Executor/AbstractExecutor.php#L91

Added line #L91 was not covered by tests
{
$logger = $this->logger;
$logger($message);
Expand Down
2 changes: 1 addition & 1 deletion src/Executor/MongoDBExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
}

/** @inheritDoc */
public function execute(array $fixtures, $append = false)
public function execute(array $fixtures, bool $append = false)

Check warning on line 63 in src/Executor/MongoDBExecutor.php

View check run for this annotation

Codecov / codecov/patch

src/Executor/MongoDBExecutor.php#L63

Added line #L63 was not covered by tests
{
if ($append === false) {
$this->purge();
Expand Down
2 changes: 1 addition & 1 deletion src/Executor/MultipleTransactionORMExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ final class MultipleTransactionORMExecutor extends AbstractExecutor
use ORMExecutorCommon;

/** @inheritDoc */
public function execute(array $fixtures, $append = false): void
public function execute(array $fixtures, bool $append = false): void
{
$executor = $this;
if ($append === false) {
Expand Down
2 changes: 1 addition & 1 deletion src/Executor/ORMExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class ORMExecutor extends AbstractExecutor
use ORMExecutorCommon;

/** @inheritDoc */
public function execute(array $fixtures, $append = false)
public function execute(array $fixtures, bool $append = false)
{
$executor = $this;
$this->em->wrapInTransaction(static function (EntityManagerInterface $em) use ($executor, $fixtures, $append) {
Expand Down
2 changes: 1 addition & 1 deletion src/Executor/PHPCRExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
}

/** @inheritDoc */
public function execute(array $fixtures, $append = false)
public function execute(array $fixtures, bool $append = false)

Check warning on line 43 in src/Executor/PHPCRExecutor.php

View check run for this annotation

Codecov / codecov/patch

src/Executor/PHPCRExecutor.php#L43

Added line #L43 was not covered by tests
{
$that = $this;

Expand Down
18 changes: 6 additions & 12 deletions src/Loader.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
*
* @return array $fixtures Array of loaded fixture object instances.
*/
public function loadFromDirectory($dir)
public function loadFromDirectory(string $dir)
{
if (! is_dir($dir)) {
throw new InvalidArgumentException(sprintf('"%s" does not exist', $dir));
Expand All @@ -94,7 +94,7 @@
*
* @return array $fixtures Array of loaded fixture object instances.
*/
public function loadFromFile($fileName)
public function loadFromFile(string $fileName)
{
if (! is_readable($fileName)) {
throw new InvalidArgumentException(sprintf('"%s" does not exist or is not readable', $fileName));
Expand All @@ -108,23 +108,19 @@
/**
* Has fixture?
*
* @param FixtureInterface $fixture
*
* @return bool
*/
public function hasFixture($fixture)
public function hasFixture(FixtureInterface $fixture)

Check warning on line 113 in src/Loader.php

View check run for this annotation

Codecov / codecov/patch

src/Loader.php#L113

Added line #L113 was not covered by tests
{
return isset($this->fixtures[get_class($fixture)]);
}

/**
* Get a specific fixture instance
*
* @param string $className
*
* @return FixtureInterface
*/
public function getFixture($className)
public function getFixture(string $className)
{
if (! isset($this->fixtures[$className])) {
throw new InvalidArgumentException(sprintf(
Expand Down Expand Up @@ -204,7 +200,7 @@
*
* @return bool
*/
public function isTransient($className)
public function isTransient(string $className)
{
$rc = new ReflectionClass($className);
if ($rc->isAbstract()) {
Expand All @@ -219,11 +215,9 @@
/**
* Creates the fixture object from the class.
*
* @param string $class
*
* @return FixtureInterface
*/
protected function createFixture($class)
protected function createFixture(string $class)
{
return new $class();
}
Expand Down
6 changes: 3 additions & 3 deletions src/ProxyReferenceRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
*
* @return void
*/
public function unserialize($serializedData)
public function unserialize(string $serializedData)
{
$repositoryData = unserialize($serializedData);

Expand Down Expand Up @@ -98,7 +98,7 @@
*
* @return bool
*/
public function load($baseCacheName)
public function load(string $baseCacheName)

Check warning on line 101 in src/ProxyReferenceRepository.php

View check run for this annotation

Codecov / codecov/patch

src/ProxyReferenceRepository.php#L101

Added line #L101 was not covered by tests
{
$filename = $baseCacheName . '.ser';

Expand All @@ -124,7 +124,7 @@
*
* @return void
*/
public function save($baseCacheName)
public function save(string $baseCacheName)

Check warning on line 127 in src/ProxyReferenceRepository.php

View check run for this annotation

Codecov / codecov/patch

src/ProxyReferenceRepository.php#L127

Added line #L127 was not covered by tests
{
$serializedData = $this->serialize();

Expand Down
4 changes: 1 addition & 3 deletions src/Purger/ORMPurger.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,9 @@
/**
* Set the purge mode
*
* @param int $mode
*
* @return void
*/
public function setPurgeMode($mode)
public function setPurgeMode(int $mode)

Check warning on line 64 in src/Purger/ORMPurger.php

View check run for this annotation

Codecov / codecov/patch

src/Purger/ORMPurger.php#L64

Added line #L64 was not covered by tests
{
$this->purgeMode = $mode;
$this->cachedSqlStatements = null;
Expand Down
32 changes: 10 additions & 22 deletions src/ReferenceRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function __construct(ObjectManager $manager)
*
* @return array
*/
protected function getIdentifier($reference, $uow)
protected function getIdentifier(object $reference, object $uow)
{
// In case Reference is not yet managed in UnitOfWork
if (! $this->hasIdentifier($reference)) {
Expand Down Expand Up @@ -103,12 +103,9 @@ protected function getIdentifier($reference, $uow)
* and referenced to $reference. If $name
* already is set, it overrides it
*
* @param string $name
* @param object $reference
*
* @return void
*/
public function setReference($name, $reference)
public function setReference(string $name, object $reference)
{
$class = $this->getRealClass(get_class($reference));

Expand All @@ -134,13 +131,12 @@ public function setReference($name, $reference)
/**
* Store the identifier of a reference
*
* @param string $name
* @param mixed $identity
* @param class-string|null $class
*
* @return void
*/
public function setReferenceIdentity($name, $identity, ?string $class = null)
public function setReferenceIdentity(string $name, $identity, ?string $class = null)
{
if ($class === null) {
Deprecation::trigger(
Expand All @@ -166,14 +162,13 @@ public function setReferenceIdentity($name, $identity, ?string $class = null)
* the record is inserted, be sure tu use this method
* after $object is flushed
*
* @param string $name
* @param object $object - managed object
*
* @return void
*
* @throws BadMethodCallException - if repository already has a reference by $name.
*/
public function addReference($name, $object)
public function addReference(string $name, object $object)
{
// For BC, to be removed in next major.
if (isset($this->references[$name])) {
Expand All @@ -199,7 +194,6 @@ public function addReference($name, $object)
* Loads an object using stored reference
* named by $name
*
* @param string $name
* @psalm-param class-string<T>|null $class
*
* @return object
Expand All @@ -209,7 +203,7 @@ public function addReference($name, $object)
*
* @template T of object
*/
public function getReference($name, ?string $class = null)
public function getReference(string $name, ?string $class = null)
{
if ($class === null) {
Deprecation::trigger(
Expand Down Expand Up @@ -256,12 +250,11 @@ public function getReference($name, ?string $class = null)
* Check if an object is stored using reference
* named by $name
*
* @param string $name
* @psalm-param class-string $class
*
* @return bool
*/
public function hasReference($name, ?string $class = null)
public function hasReference(string $name, ?string $class = null)
{
if ($class === null) {
Deprecation::trigger(
Expand All @@ -281,11 +274,9 @@ public function hasReference($name, ?string $class = null)
* Searches for reference names in the
* list of stored references
*
* @param object $reference
*
* @return array<string>
*/
public function getReferenceNames($reference)
public function getReferenceNames(object $reference)
{
$class = $this->getRealClass(get_class($reference));
if (! isset($this->referencesByClass[$class])) {
Expand All @@ -298,12 +289,11 @@ public function getReferenceNames($reference)
/**
* Checks if reference has identity stored
*
* @param string $name
* @param class-string|null $class
*
* @return bool
*/
public function hasIdentity($name, ?string $class = null)
public function hasIdentity(string $name, ?string $class = null)
{
if ($class === null) {
Deprecation::trigger(
Expand Down Expand Up @@ -380,19 +370,17 @@ public function getManager()
*
* @return string
*/
protected function getRealClass($className)
protected function getRealClass(string $className)
{
return $this->manager->getClassMetadata($className)->getName();
}

/**
* Checks if object has identifier already in unit of work.
*
* @param object $reference
*
* @return bool
*/
private function hasIdentifier($reference)
private function hasIdentifier(object $reference)
{
// in case if reference is set after flush, store its identity
$uow = $this->manager->getUnitOfWork();
Expand Down
Loading