Use psalm errorLevel 3 #169
Conversation
Some methods are able to handle classname and classname alias, so they could receive a class-string or a string. Adding phpdoc to the variables makes psalm able to understand them.
| @@ -22,7 +22,7 @@ public function __construct(ObjectManager $wrapped) | |||
|
|
|||
| class ObjectManagerDecoratorTest extends TestCase | |||
| { | |||
| /** @var MockObject|ObjectManager */ | |||
| /** @var MockObject&ObjectManager */ | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
psalm.xml
Outdated
| <PossiblyNullReference> | ||
| <errorLevel type="suppress"> | ||
| <!-- ReflectionProperty::getType() can return null, but there is ReflectionProperty::hasType() | ||
| check before --> | ||
| <file name="lib/Doctrine/Persistence/Reflection/TypedNoDefaultReflectionProperty.php"/> | ||
| </errorLevel> | ||
| </PossiblyNullReference> |
There was a problem hiding this comment.
For things like this that can be fixed in next psalm releases, can we maybe use annotations (@psalm-suppress) so we can enable findUnusedPsalmSuppress and if this is fixed, psalm would warn us.
There was a problem hiding this comment.
Hmm I added this PossiblyNullReference in the first place because locally I used vendor/bin/psalm --no-cache --php-version=7.4 and running that command it throws the PossiblyNullReference.
But since we are not specifying the php version here, I think it's using the lowest one from composer.json, 7.1 and using that one there is no error.
| @@ -1,8 +1,7 @@ | |||
| <?xml version="1.0"?> | |||
| <psalm | |||
| totallyTyped="false" | |||
There was a problem hiding this comment.
totallyTyped is deprecated and resolveFromConfigFile by default is true.
|
@orklah please review 🙂 |
| @@ -62,6 +62,8 @@ public function getClassNamespace($class) | |||
|
|
|||
| /** | |||
| * {@inheritDoc} | |||
There was a problem hiding this comment.
Are you sure this works properly? I have a vague memory if reading that inheritDoc was ignored by Psalm when there was other tags in the block
There was a problem hiding this comment.
You're right, I thought it read both since I got some other error about instantiating ReflectionClass, I've added the other tags.
This includes: - adding assertions to avoid nullable values - Override phpdoc to be more specific Apart from ignore some issues.
|
Thanks @franmomu ! |
Another part from #159