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

PHPStan level 6 with AuditReader not generic #488

Merged
merged 2 commits into from
May 21, 2022
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
4 changes: 3 additions & 1 deletion phpstan.neon.dist
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
parameters:
level: 5
level: 6
paths:
- src
- tests
Expand All @@ -12,3 +12,5 @@ parameters:
checkMissingIterableValueType: true
checkMissingVarTagTypehint: true
checkMissingTypehints: true
ignoreErrors:
- '#^Method SimpleThings\\EntityAudit\\Tests\\[a-zA-Z0-9_\\]+Test::(data|provide)[a-zA-Z0-9_]+\(\) return type has no value type specified in iterable type [a-zA-Z0-9_]+\.$#'
3 changes: 3 additions & 0 deletions src/Action/CompareAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ public function __construct(Environment $twig, AuditReader $auditReader)
$this->auditReader = $auditReader;
}

/**
* @phpstan-param class-string $className
*/
public function __invoke(Request $request, string $className, string $id, ?int $oldRev = null, ?int $newRev = null): Response
{
if (null === $oldRev) {
Expand Down
3 changes: 3 additions & 0 deletions src/Action/ViewDetailAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ public function __construct(Environment $twig, AuditReader $auditReader)
$this->auditReader = $auditReader;
}

/**
* @phpstan-param class-string $className
*/
public function __invoke(string $className, string $id, int $rev): Response
{
$entity = $this->auditReader->find($className, $id, $rev);
Expand Down
3 changes: 3 additions & 0 deletions src/Action/ViewEntityAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ public function __construct(Environment $twig, AuditReader $auditReader)
$this->auditReader = $auditReader;
}

/**
* @phpstan-param class-string $className
*/
public function __invoke(string $className, string $id): Response
{
$revisions = $this->auditReader->findRevisions($className, $id);
Expand Down
62 changes: 36 additions & 26 deletions src/AuditReader.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@
use SimpleThings\EntityAudit\Metadata\MetadataFactory;
use SimpleThings\EntityAudit\Utils\ArrayDiff;

/**
* @phpstan-template T of object
*/
class AuditReader
{
use SQLResultCasing;
Expand Down Expand Up @@ -69,7 +66,7 @@ class AuditReader
*
* @var array<string, array<string, array<int|string, object>>>
*
* @phpstan-var array<class-string, array<string, array<int|string, T>>>
* @phpstan-var array<class-string, array<string, array<int|string, object>>>
*/
private $entityCache;

Expand Down Expand Up @@ -206,9 +203,12 @@ public function clearEntityCache(): void
* returns last revision INCLUDING "DEL" revision. If you want to throw exception instead, set
* $threatDeletionAsException to true.
*
* @param string $className
* @param int|string|array $id
* @param int|string $revision
* @template T of object
*
* @param string $className
Copy link
Member

Choose a reason for hiding this comment

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

isnt this a class-string?

* @param int|string|array<string, int|string> $id
* @param int|string $revision
* @param array{threatDeletionsAsExceptions?: bool} $options
*
* @throws DeletedException
* @throws NoRevisionFoundException
Expand Down Expand Up @@ -377,7 +377,7 @@ public function findRevisionHistory($limit = 20, $offset = 0)
*
* @param string|int $revision
*
* @return ChangedEntity[]
* @return ChangedEntity<object>[]
*/
public function findEntitesChangedAtRevision($revision)
{
Expand All @@ -396,7 +396,7 @@ public function findEntitesChangedAtRevision($revision)
* @throws ORMException
* @throws \RuntimeException
*
* @return ChangedEntity[]
* @return ChangedEntity<object>[]
*/
public function findEntitiesChangedAtRevision($revision)
{
Expand Down Expand Up @@ -508,15 +508,15 @@ public function findRevision($revision)
/**
* Find all revisions that were made of entity class with given id.
*
* @param string $className
* @param int|string|array $id
* @param string $className
Copy link
Member

Choose a reason for hiding this comment

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

And another one

* @param int|string|array<string, int|string> $id
*
* @throws NotAuditedException
* @throws Exception
*
* @return Revision[]
*
* @phpstan-param class-string<T> $className
* @phpstan-param class-string $className
*/
public function findRevisions($className, $id)
{
Expand Down Expand Up @@ -566,15 +566,15 @@ public function findRevisions($className, $id)
* NEXT_MAJOR: Add NoRevisionFoundException as possible exception.
* Gets the current revision of the entity with given ID.
*
* @param string $className
* @param int|string|array $id
* @param string $className
Copy link
Member

Choose a reason for hiding this comment

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

Another class-string?

* @param int|string|array<string, int|string> $id
*
* @throws NotAuditedException
* @throws Exception
*
* @return int|string|null
*
* @phpstan-param class-string<T> $className
* @phpstan-param class-string $className
*/
public function getCurrentRevision($className, $id)
{
Expand Down Expand Up @@ -636,7 +636,7 @@ public function getCurrentRevision($className, $id)
*
* @return array<string, array<string, mixed>>
*
* @phpstan-param class-string<T> $className
* @phpstan-param class-string $className
* @phpstan-return array<string, array{old: mixed, new: mixed, same: mixed}>
*/
public function diff($className, $id, $oldRevision, $newRevision)
Expand All @@ -660,8 +660,7 @@ public function diff($className, $id, $oldRevision, $newRevision)
*
* @return array<string, mixed>
*
* @phpstan-param class-string<T> $className
* @phpstan-param T $entity
* @phpstan-param class-string $className
*/
public function getEntityValues($className, $entity)
{
Expand All @@ -677,8 +676,10 @@ public function getEntityValues($className, $entity)
}

/**
* @param string $className
* @param int|string|array $id
* @template T of object
*
* @param string $className
* @param int|string|array<string, int|string> $id
*
* @throws DeletedException
* @throws NoRevisionFoundException
Expand Down Expand Up @@ -774,8 +775,12 @@ protected function getEntityPersister($className)
/**
* Simplified and stolen code from UnitOfWork::createEntity.
*
* @param string $className
* @param int|string $revision
* @template T of object
*
* @param string $className
Copy link
Member

Choose a reason for hiding this comment

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

isnt this a class-string?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, all these parameters are defined below as @phpstan-param

* @param array<string, string> $columnMap
* @param array<string, mixed> $data
* @param int|string $revision
*
* @throws DeletedException
* @throws NoRevisionFoundException
Expand All @@ -802,9 +807,11 @@ private function createEntity($className, array $columnMap, array $data, $revisi

$key = implode(':', $keyParts);

if (isset($this->entityCache[$className], $this->entityCache[$className][$key], $this->entityCache[$className][$key][$revision])
) {
return $this->entityCache[$className][$key][$revision];
if (isset($this->entityCache[$className][$key][$revision])) {
/** @phpstan-var T $cachedEntity */
$cachedEntity = $this->entityCache[$className][$key][$revision];

return $cachedEntity;
}

if (!$classMetadata->isInheritanceTypeNone()) {
Expand All @@ -827,7 +834,10 @@ private function createEntity($className, array $columnMap, array $data, $revisi
$pk[$classMetadata->getColumnName($field)] = $data[$field];
}

return $this->find($classMetadata->discriminatorMap[$discriminator], $pk, $revision);
/** @phpstan-var class-string<T> $classNameDiscriminator */
$classNameDiscriminator = $classMetadata->discriminatorMap[$discriminator];

return $this->find($classNameDiscriminator, $pk, $revision);
}
} else {
/** @phpstan-var T $entity */
Expand Down
6 changes: 4 additions & 2 deletions src/ChangedEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ChangedEntity
private $className;

/**
* @var array
* @var array<string, int|string>
*/
private $id;

Expand All @@ -43,6 +43,8 @@ class ChangedEntity
private $entity;

/**
* @param array<string, int|string> $id
*
* @phpstan-param class-string<T> $className
* @phpstan-param T $entity
*/
Expand All @@ -65,7 +67,7 @@ public function getClassName()
}

/**
* @return array
* @return array<string, int|string>
*/
public function getId()
{
Expand Down
9 changes: 5 additions & 4 deletions src/Collection/AuditedCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class AuditedCollection implements Collection
/**
* Related audit reader instance.
*
* @var AuditReader<T>
* @var AuditReader
*/
protected $auditReader;

Expand All @@ -53,7 +53,7 @@ class AuditedCollection implements Collection
/**
* Maximum revision to fetch.
*
* @var string
* @var string|int
*/
protected $revision;

Expand All @@ -74,7 +74,7 @@ class AuditedCollection implements Collection
* initialized yet or contain identifiers to load the entities.
*
* @var Collection<int|string, array>
* @phpstan-var Collection<TKey, array{keys: array, rev: string|int}>
* @phpstan-var Collection<TKey, array{keys: array<string, int|string>, rev: string|int}>
*/
protected $entities;

Expand Down Expand Up @@ -103,8 +103,9 @@ class AuditedCollection implements Collection
* @param string $class
* @param array<string, mixed> $associationDefinition
* @param array<string, mixed> $foreignKeys
* @param string|int $revision
*
* @phpstan-param AuditReader<T> $auditReader
* @phpstan-param AuditReader $auditReader
* @phpstan-param class-string<T> $class
* @phpstan-param ClassMetadataInfo<T> $classMeta
*/
Expand Down
3 changes: 3 additions & 0 deletions src/DependencyInjection/SimpleThingsEntityAuditExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ public function load(array $configs, ContainerBuilder $container): void
]);
}

/**
* @param string[] $definitionNames
*/
private function fixParametersFromDoctrineEventSubscriberTag(ContainerBuilder $container, array $definitionNames): void
{
foreach ($definitionNames as $definitionName) {
Expand Down
4 changes: 3 additions & 1 deletion tests/Issue/Issue308Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ public function testIssue308(): void
$revisions = $auditReader->findRevisions($userClass, $user->getId());
static::assertCount(1, $revisions);
$revision = reset($revisions);
$auditedUser = $auditReader->find($userClass, ['id' => $user->getId()], $revision->getRev());
$userId = $user->getId();
static::assertNotNull($userId);
$auditedUser = $auditReader->find($userClass, ['id' => $userId], $revision->getRev());

static::assertInstanceOf(Collection::class, $auditedUser->getChildren());
}
Expand Down
6 changes: 4 additions & 2 deletions tests/RelationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -790,8 +790,10 @@ public function testOneToOneEdgeCase(): void

$auditedBase = $reader->find(\get_class($base), $base->getId(), 1);

static::assertSame('foobar', $auditedBase->getReferencedEntity()->getFoobarField());
static::assertSame('referenced', $auditedBase->getReferencedEntity()->getReferencedField());
$referencedEntity = $auditedBase->getReferencedEntity();
static::assertInstanceOf(RelationFoobarEntity::class, $referencedEntity);
static::assertSame('foobar', $referencedEntity->getFoobarField());
static::assertSame('referenced', $referencedEntity->getReferencedField());
}

/**
Expand Down