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

Use psalm errorLevel 3 #169

Merged
merged 3 commits into from
Apr 22, 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
29 changes: 21 additions & 8 deletions lib/Doctrine/Persistence/AbstractManagerRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,9 @@ public function getManager($name = null)
*/
public function getManagerForClass($class)
{
// Check for namespace alias
if (strpos($class, ':') !== false) {
[$namespaceAlias, $simpleClassName] = explode(':', $class, 2);
$class = $this->getAliasNamespace($namespaceAlias) . '\\' . $simpleClassName;
}
$className = $this->getRealClassName($class);

$proxyClass = new ReflectionClass($class);
$proxyClass = new ReflectionClass($className);

if ($proxyClass->implementsInterface($this->proxyInterfaceName)) {
$parentClass = $proxyClass->getParentClass();
Expand All @@ -177,13 +173,13 @@ public function getManagerForClass($class)
return null;
}

$class = $parentClass->getName();
$className = $parentClass->getName();
}

foreach ($this->managers as $id) {
$manager = $this->getService($id);

if (! $manager->getMetadataFactory()->isTransient($class)) {
if (! $manager->getMetadataFactory()->isTransient($className)) {
return $manager;
}
}
Expand Down Expand Up @@ -250,4 +246,21 @@ private function selectManager(string $persistentObjectName, ?string $persistent

return $this->getManagerForClass($persistentObjectName) ?? $this->getManager();
}

/**
* @psalm-return class-string
*/
private function getRealClassName(string $classNameOrAlias): string
{
// Check for namespace alias
if (strpos($classNameOrAlias, ':') !== false) {
[$namespaceAlias, $simpleClassName] = explode(':', $classNameOrAlias, 2);

/** @psalm-var class-string */
return $this->getAliasNamespace($namespaceAlias) . '\\' . $simpleClassName;
}

/** @psalm-var class-string */
return $classNameOrAlias;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use function array_map;
use function array_reverse;
use function array_unshift;
use function assert;
use function explode;
use function sprintf;
use function str_replace;
Expand Down Expand Up @@ -225,6 +226,7 @@ public function getMetadataFor($className)

$realClassName = $this->getFqcnFromAlias($namespaceAlias, $simpleClassName);
} else {
/** @psalm-var class-string $className */
$realClassName = $this->getRealClass($className);
}

Expand Down Expand Up @@ -449,6 +451,7 @@ public function isTransient($class)
$class = $this->getFqcnFromAlias($namespaceAlias, $simpleClassName);
}

/** @psalm-var class-string $class */
return $this->getDriver()->isTransient($class);
}

Expand Down Expand Up @@ -496,6 +499,8 @@ private function getRealClass(string $class): string
$this->createDefaultProxyClassNameResolver();
}

assert($this->proxyClassNameResolver !== null);

return $this->proxyClassNameResolver->resolveClassName($class);
}

Expand Down
4 changes: 1 addition & 3 deletions lib/Doctrine/Persistence/Mapping/Driver/AnnotationDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,7 @@ public function setFileExtension($fileExtension)
* A class is non-transient if it is annotated with an annotation
* from the {@see AnnotationDriver::entityAnnotationClasses}.
*
* @param string $className
*
* @return bool
* {@inheritDoc}
*/
public function isTransient($className)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,6 @@ public function findMappingFile($className)
}
}

throw MappingException::mappingFileNotFound($className, substr($className, strrpos($className, '\\') + 1) . $this->fileExtension);
throw MappingException::mappingFileNotFound($className, substr($className, (int) strrpos($className, '\\') + 1) . $this->fileExtension);
}
}
2 changes: 1 addition & 1 deletion lib/Doctrine/Persistence/Mapping/MappingException.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static function fileMappingDriversRequireConfiguredDirectoryPath($path =
return new self(sprintf(
'File mapping drivers must have a valid directory path, ' .
'however the given path %s seems to be incorrect!',
$path
(string) $path
));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ public function getClassNamespace($class)
}

/**
* {@inheritDoc}
* @param string $class
* @psalm-param class-string $class
*
* @return ReflectionClass
*/
public function getClass($class)
{
Expand Down
8 changes: 5 additions & 3 deletions lib/Doctrine/Persistence/Mapping/StaticReflectionService.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ public function getParentClasses($class)
*/
public function getClassShortName($className)
{
if (strpos($className, '\\') !== false) {
$className = substr($className, strrpos($className, '\\') + 1);
$nsSeparatorLastPosition = strrpos($className, '\\');

if ($nsSeparatorLastPosition !== false) {
$className = substr($className, $nsSeparatorLastPosition + 1);
}

return $className;
Expand All @@ -39,7 +41,7 @@ public function getClassNamespace($className)
{
$namespace = '';
if (strpos($className, '\\') !== false) {
$namespace = strrev(substr(strrev($className), strpos(strrev($className), '\\') + 1));
$namespace = strrev(substr(strrev($className), (int) strpos(strrev($className), '\\') + 1));
}

return $namespace;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@

namespace Doctrine\Persistence\Reflection;

use Closure;
use ReflectionProperty;

use function assert;

/**
* PHP Typed No Default Reflection Property - special override for typed properties without a default value.
*/
Expand Down Expand Up @@ -38,6 +41,9 @@ public function setValue($object, $value = null)
unset($this->$propertyName);
};
$unsetter = $unsetter->bindTo($object, $this->getDeclaringClass()->getName());

assert($unsetter instanceof Closure);

$unsetter();

return;
Expand Down
17 changes: 14 additions & 3 deletions psalm.xml
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
<?xml version="1.0"?>
<psalm
totallyTyped="false"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

errorLevel="4"
resolveFromConfigFile="true"
errorLevel="3"
findUnusedPsalmSuppress="true"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
Expand Down Expand Up @@ -39,5 +38,17 @@
<file name="tests/Doctrine/Tests/Persistence/Mapping/SymfonyFileLocatorTest.php"/>
</errorLevel>
</NullArgument>
<ArgumentTypeCoercion>
<errorLevel type="suppress">
<!-- On purpose to use a non existing class for tests -->
<file name="tests/Doctrine/Tests/Persistence/Mapping/RuntimeReflectionServiceTest.php"/>
</errorLevel>
</ArgumentTypeCoercion>
<MoreSpecificReturnType>
<errorLevel type="suppress">
<!-- FileDriver::loadMappingFile() in tests could have a more specific return, but is not needed -->
<file name="tests/Doctrine/Tests/Persistence/Mapping/FileDriverTest.php"/>
</errorLevel>
</MoreSpecificReturnType>
</issueHandlers>
</psalm>
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public function __construct(ObjectManager $wrapped)

class ObjectManagerDecoratorTest extends TestCase
{
/** @var MockObject|ObjectManager */
/** @var MockObject&ObjectManager */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

About this, it was included in PHPStorm apparently in 2018:https://blog.jetbrains.com/phpstorm/2018/09/phpstorm-2018-3-eap-183-2635-12/

Let me now if we should use @psalm-var.

Copy link
Member

Choose a reason for hiding this comment

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

PHPStorm indeed added parsing for it in 2.018 (it was still treating it as a union type internally as it did not support intersection types, but that's not a reason to avoid it in favor of writing a union type directly).

So +1 from me

private $wrapped;

/** @var NullObjectManagerDecorator */
Expand Down