Psalm level 6 3 #1566
Psalm level 6 3 #1566
Conversation
| { | ||
| /** | ||
| * @var string | ||
| * @var Suite |
There was a problem hiding this comment.
Comment was wrong
|
|
||
| $this->example = $event->getScenario(); | ||
| assert($this->example instanceof ExampleNode); | ||
| $scenario = $event->getScenario(); |
There was a problem hiding this comment.
We need to assert that the scenario is an ExampleNode before assigning to the property as the property has the ExampleNode type
| $call = $hookCallResult->getCall(); | ||
| $callee = $call->getCallee(); | ||
| $hook = (string) $callee; | ||
| $hook = $callee->__toString(); |
There was a problem hiding this comment.
PSalm is not happy with the cast and requires a explicit call to __toString()
There was a problem hiding this comment.
I would ignore such rule for now, especially if we migrate to phpstan (as discussed in the Gherking repo), as phpstan does not have such rule in its default config (at least not for stringable types)
There was a problem hiding this comment.
Looking into this, it seems like the types don't guarantee that the callee received here is a child of RuntimeHook, which is where __toString is defined (and not in the Callee interface). So Psalm is right about complaining here, but it would also complain about the explicit call to __toString.
Maybe the rule complaining about it is not enabled yet. This is the risk of enabling only some rules instead of a full level, as it could lead to switching to a different error (not reported yet) instead of fixing the root cause.
There was a problem hiding this comment.
I ran this with the full level 6 enabled and did not get any complains, so maybe it is detected at another level. Let me try to see which is the best way to handle this
There was a problem hiding this comment.
I have narrowed the types before applying the conversion to string
| private $key; | ||
| /** | ||
| * @var string[] | ||
| * @var array<string, string[]> |
There was a problem hiding this comment.
doc-block was wrong
|
|
||
| /** | ||
| * @return null|float | ||
| * @return float |
There was a problem hiding this comment.
This function never returns null
| * @author Konstantin Kudryashov <ever.zet@gmail.com> | ||
| */ | ||
| interface TestworkException | ||
| interface TestworkException extends Throwable |
There was a problem hiding this comment.
This exception interface needs to extend the Throwable interface so that Psalm accepts that any exception which implements it can be thrown
src/Behat/Behat/Output/Node/Printer/Pretty/PrettySkippedStepPrinter.php
Outdated
Show resolved
Hide resolved
src/Behat/Behat/Output/Node/Printer/Pretty/PrettyStepPrinter.php
Outdated
Show resolved
Hide resolved
a3a2659 to
ee31647
Compare
Apply several new rules. Most don't need any changes in the code