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

Generics for cmf #167

Merged
merged 4 commits into from
Apr 20, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
},
"require-dev": {
"composer/package-versions-deprecated": "^1.11",
"phpstan/phpstan": "^0.12",
"doctrine/coding-standard": "^6.0 || ^8.0",
"phpstan/phpstan": "^0.12.84",
Copy link

Choose a reason for hiding this comment

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

I'd advise pinning the version of static tools, especially now that we started deploying baseline. It will make sure the version used to generate the baseline is the same for everyone, and make sure not to break builds the day before the release :)

"doctrine/coding-standard": "^6.0 || ^9.0",
"doctrine/common": "^3.0",
"phpunit/phpunit": "^7.5.20 || ^8.0 || ^9.0",
"vimeo/psalm": "^4.3.1"
Expand Down
4 changes: 2 additions & 2 deletions lib/Doctrine/Persistence/ManagerRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,12 @@ public function getManagerNames();
*
* @param string $persistentObject The name of the persistent object.
* @param string $persistentManagerName The object manager name (null for the default one).
* @psalm-param class-string<T> $persistentObject
*
* @return ObjectRepository
* @psalm-return ObjectRepository<T>
*
* @template T
* @psalm-param class-string<T> $persistentObject
* @psalm-return ObjectRepository<T>
*/
public function getRepository($persistentObject, $persistentManagerName = null);

Expand Down
48 changes: 29 additions & 19 deletions lib/Doctrine/Persistence/Mapping/AbstractClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
* to a relational database.
*
* This class was abstracted from the ORM ClassMetadataFactory.
*
* @template CMTemplate of ClassMetadata
* @template-implements ClassMetadataFactory<CMTemplate>
*/
abstract class AbstractClassMetadataFactory implements ClassMetadataFactory
{
Expand All @@ -49,7 +52,10 @@ abstract class AbstractClassMetadataFactory implements ClassMetadataFactory
/** @var CacheItemPoolInterface|null */
private $cache;

/** @var ClassMetadata[] */
/**
* @var ClassMetadata[]
* @psalm-var CMTemplate[]
*/
private $loadedMetadata = [];

/** @var bool */
Expand Down Expand Up @@ -116,17 +122,15 @@ final protected function getCache(): ?CacheItemPoolInterface
* Returns an array of all the loaded metadata currently in memory.
*
* @return ClassMetadata[]
* @psalm-return CMTemplate[]
*/
public function getLoadedMetadata()
{
return $this->loadedMetadata;
}

/**
* Forces the factory to load the metadata of all classes known to the underlying
* mapping driver.
*
* @return ClassMetadata[] The ClassMetadata instances of all mapped classes.
* {@inheritDoc}
*/
public function getAllMetadata()
{
Expand Down Expand Up @@ -176,13 +180,17 @@ abstract protected function getDriver();
/**
* Wakes up reflection after ClassMetadata gets unserialized from cache.
*
* @psalm-param CMTemplate $class
*
* @return void
*/
abstract protected function wakeupReflection(ClassMetadata $class, ReflectionService $reflService);

/**
* Initializes Reflection after ClassMetadata was constructed.
*
* @psalm-param CMTemplate $class
*
* @return void
*/
abstract protected function initializeReflection(ClassMetadata $class, ReflectionService $reflService);
Expand All @@ -192,16 +200,14 @@ abstract protected function initializeReflection(ClassMetadata $class, Reflectio
*
* This method should return false for mapped superclasses or embedded classes.
*
* @psalm-param CMTemplate $class
*
* @return bool
*/
abstract protected function isEntity(ClassMetadata $class);

/**
* Gets the class metadata descriptor for a class.
*
* @param string $className The name of the class.
*
* @return ClassMetadata
* {@inheritDoc}
*
* @throws ReflectionException
* @throws MappingException
Expand Down Expand Up @@ -232,6 +238,7 @@ public function getMetadataFor($className)
if ($this->cache) {
$cached = $this->cache->getItem($this->getCacheKey($realClassName))->get();
if ($cached instanceof ClassMetadata) {
/** @psalm-var CMTemplate $cached */
Copy link

Choose a reason for hiding this comment

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

This feels like cheating, but without trying to wrap the cache interface, I'm not sure we can do better

$this->loadedMetadata[$realClassName] = $cached;

$this->wakeupReflection($cached, $this->getReflectionService());
Expand Down Expand Up @@ -275,11 +282,7 @@ public function getMetadataFor($className)
}

/**
* Checks whether the factory has the metadata for a class loaded already.
*
* @param string $className
*
* @return bool TRUE if the metadata of the class in question is already loaded, FALSE otherwise.
* {@inheritDoc}
*/
public function hasMetadataFor($className)
{
Expand All @@ -291,8 +294,7 @@ public function hasMetadataFor($className)
*
* NOTE: This is only useful in very special cases, like when generating proxy classes.
*
* @param string $className
* @param ClassMetadata $class
* {@inheritDoc}
*
* @return void
*/
Expand Down Expand Up @@ -395,6 +397,7 @@ protected function loadMetadata($name)
* @param string $className
*
* @return ClassMetadata|null
* @psalm-return CMTemplate|null
*/
protected function onNotFoundMetadata($className)
{
Expand All @@ -409,6 +412,8 @@ protected function onNotFoundMetadata($className)
* @param bool $rootEntityFound
* @param string[] $nonSuperclassParents All parent class names
* that are not marked as mapped superclasses.
* @psalm-param CMTemplate $class
* @psalm-param CMTemplate|null $parent
*
* @return void
*/
Expand All @@ -420,6 +425,7 @@ abstract protected function doLoadMetadata($class, $parent, $rootEntityFound, ar
* @param string $className
*
* @return ClassMetadata
* @psalm-return CMTemplate
*/
abstract protected function newClassMetadataInstance($className);

Expand Down Expand Up @@ -473,9 +479,11 @@ protected function getCacheKey(string $realClassName): string
/**
* Gets the real class name of a class name that could be a proxy.
*
* @template T of object
* @psalm-param class-string<Proxy<T>>|class-string<T> $class
*
* @psalm-return class-string<T>
*
* @template T of object
*/
private function getRealClass(string $class): string
{
Expand All @@ -490,9 +498,11 @@ private function createDefaultProxyClassNameResolver(): void
{
$this->proxyClassNameResolver = new class implements ProxyClassNameResolver {
/**
* @template T of object
* @psalm-param class-string<Proxy<T>>|class-string<T> $className
*
* @psalm-return class-string<T>
*
* @template T of object
*/
public function resolveClassName(string $className): string
{
Expand Down
1 change: 0 additions & 1 deletion lib/Doctrine/Persistence/Mapping/ClassMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ public function getTypeOfField($fieldName);
* @param string $assocName
*
* @return string
*
* @psalm-return class-string
*/
public function getAssociationTargetClass($assocName);
Expand Down
5 changes: 5 additions & 0 deletions lib/Doctrine/Persistence/Mapping/ClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

/**
* Contract for a Doctrine persistence layer ClassMetadata class to implement.
*
* @template T of ClassMetadata
*/
interface ClassMetadataFactory
{
Expand All @@ -12,6 +14,7 @@ interface ClassMetadataFactory
* mapping driver.
*
* @return ClassMetadata[] The ClassMetadata instances of all mapped classes.
* @psalm-return T[]
*/
public function getAllMetadata();

Expand All @@ -21,6 +24,7 @@ public function getAllMetadata();
* @param string $className The name of the class.
*
* @return ClassMetadata
* @psalm-return T
*/
public function getMetadataFor($className);

Expand All @@ -38,6 +42,7 @@ public function hasMetadataFor($className);
*
* @param string $className
* @param ClassMetadata $class
* @psalm-param T $class
*/
public function setMetadataFor($className, $class);

Expand Down
1 change: 0 additions & 1 deletion lib/Doctrine/Persistence/Mapping/Driver/FileDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ public function getAllClassNames()
* @param string $file The mapping file to load.
*
* @return ClassMetadata[]
*
* @psalm-return array<class-string, ClassMetadata>
*/
abstract protected function loadMappingFile($file);
Expand Down
4 changes: 3 additions & 1 deletion lib/Doctrine/Persistence/Mapping/ProxyClassNameResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
interface ProxyClassNameResolver
{
/**
* @template T of object
* @psalm-param class-string<Proxy<T>>|class-string<T> $className
*
* @psalm-return class-string<T>
*
* @template T of object
*/
public function resolveClassName(string $className): string;
}
8 changes: 4 additions & 4 deletions lib/Doctrine/Persistence/ObjectManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ interface ObjectManager
*
* @param string $className The class name of the object to find.
* @param mixed $id The identity of the object to find.
* @psalm-param class-string<T> $className
*
* @return object|null The found object.
* @psalm-return T|null
*
* @template T
* @psalm-param class-string<T> $className
* @psalm-return T|null
*/
public function find($className, $id);

Expand Down Expand Up @@ -115,12 +115,12 @@ public function flush();
* Gets the repository for a class.
*
* @param string $className
* @psalm-param class-string<T> $className
*
* @return ObjectRepository
* @psalm-return ObjectRepository<T>
*
* @template T
* @psalm-param class-string<T> $className
* @psalm-return ObjectRepository<T>
*/
public function getRepository($className);

Expand Down
8 changes: 2 additions & 6 deletions lib/Doctrine/Persistence/ObjectRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ interface ObjectRepository
* @param mixed $id The identifier.
*
* @return object|null The object.
*
* @psalm-return T|null
*/
public function find($id);
Expand All @@ -26,7 +25,6 @@ public function find($id);
* Finds all objects in the repository.
*
* @return array<int, object> The objects.
*
* @psalm-return T[]
*/
public function findAll();
Expand All @@ -42,13 +40,12 @@ public function findAll();
* @param string[]|null $orderBy
* @param int|null $limit
* @param int|null $offset
* @psalm-param array<string, 'asc'|'desc'|'ASC'|'DESC'> $orderBy
*
* @return object[] The objects.
* @psalm-return T[]
*
* @throws UnexpectedValueException
*
* @psalm-param array<string, 'asc'|'desc'|'ASC'|'DESC'> $orderBy
* @psalm-return T[]
*/
public function findBy(array $criteria, ?array $orderBy = null, $limit = null, $offset = null);

Expand All @@ -58,7 +55,6 @@ public function findBy(array $criteria, ?array $orderBy = null, $limit = null, $
* @param mixed[] $criteria The criteria.
*
* @return object|null The object.
*
* @psalm-return T|null
*/
public function findOneBy(array $criteria);
Expand Down
8 changes: 0 additions & 8 deletions phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,4 @@
<rule ref="SlevomatCodingStandard.Classes.SuperfluousExceptionNaming.SuperfluousSuffix">
<exclude-pattern>lib/Doctrine/Persistence/Mapping/MappingException.php</exclude-pattern>
</rule>

<rule ref="SlevomatCodingStandard.Classes.UnusedPrivateElements.UnusedProperty">
<exclude-pattern>RuntimeReflectionServiceTest</exclude-pattern>
</rule>

<rule ref="SlevomatCodingStandard.Classes.UnusedPrivateElements.WriteOnlyProperty">
<exclude-pattern>tests/Doctrine/Tests_PHP74/Persistence/Reflection/TypedNoDefaultReflectionPropertyTest.php</exclude-pattern>
</rule>
</ruleset>
6 changes: 0 additions & 6 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,3 @@ parameters:
message: "#^Parameter \\#3 \\$nsSeparator of class Doctrine\\\\Persistence\\\\Mapping\\\\Driver\\\\SymfonyFileLocator constructor expects string, null given\\.$#"
count: 1
path: tests/Doctrine/Tests/Persistence/Mapping/SymfonyFileLocatorTest.php

-
# Remove it when https://github.com/phpstan/phpstan/issues/4803 is solved
message: "#^Method Doctrine\\\\Persistence\\\\Mapping\\\\AbstractClassMetadataFactory\\:\\:getRealClass\\(\\) should return class\\-string\\<T of object\\> but returns class\\-string\\<Doctrine\\\\Persistence\\\\Proxy\\<T of object\\>\\|T of object\\>\\.$#"
count: 1
path: lib/Doctrine/Persistence/Mapping/AbstractClassMetadataFactory.php