Fix some issues identified by phpstan #123
Conversation
| @@ -5,6 +5,7 @@ | |||
| namespace Aranyasen\HL7\Segments; | |||
|
|
|||
| use Aranyasen\HL7\Segment; | |||
| use InvalidArgumentException; | |||
There was a problem hiding this comment.
This exception is thrown in this class, but was not used.
| public function setField(int $index, $value = ''): bool | ||
| public function setField(int $index, string|int|array|null $value = ''): bool | ||
| { | ||
| if (($index === 1) && strlen($value) !== 1) { | ||
| if (($index === 1) && (!is_string($value) || strlen($value) !== 1)) { | ||
| return false; | ||
| } | ||
|
|
||
| if (($index === 2) && strlen($value) !== 4) { | ||
| if (($index === 2) && (!is_string($value) || strlen($value) !== 4)) { |
There was a problem hiding this comment.
MSH::setTriggerEvent() calls $this->setField($position, $value), and $value is an array. This changes this function to take the same types as Segment::setField(), and ensures that index 1 or 2 must be a string. It could be better to throw an exception instead of returning false if a non-string value is given for MSH.1 or MSH.2.
| @@ -8,6 +8,7 @@ | |||
| use Aranyasen\HL7; | |||
| use Aranyasen\HL7\Segments\PID; | |||
| use DMS\PHPUnitExtensions\ArraySubset\Assert; | |||
| use Exception; | |||
There was a problem hiding this comment.
The docblock for empty_subfields_can_be_retained_if_needed() says it throws Exception. This uses that class to avoid ambiguity.
tests/Hl7ListenerTrait.php
Outdated
| throw new RuntimeException('socket_bind() failed: reason: ' . socket_strerror($ret) . "\n"); | ||
| throw new RuntimeException('socket_bind() failed: reason: ' . socket_strerror(socket_last_error($socket)) . "\n"); | ||
| } | ||
| if (($ret = socket_listen($socket, 5)) === false) { | ||
| throw new RuntimeException('socket_listen() failed: reason: ' . socket_strerror($ret) . "\n"); | ||
| throw new RuntimeException('socket_listen() failed: reason: ' . socket_strerror(socket_last_error($socket)) . "\n"); |
There was a problem hiding this comment.
In both of these $ret is a bool, and socket_strerror() expects an int. So, this calls socket_last_error($socket) instead.
tests/Hl7ListenerTrait.php
Outdated
| throw new RuntimeException('socket_bind() failed: reason: ' . socket_strerror($ret) . "\n"); | ||
| throw new RuntimeException('socket_bind() failed: reason: ' . socket_strerror(socket_last_error($socket)) . "\n"); | ||
| } | ||
| if (($ret = socket_listen($socket, 5)) === false) { | ||
| throw new RuntimeException('socket_listen() failed: reason: ' . socket_strerror($ret) . "\n"); | ||
| throw new RuntimeException('socket_listen() failed: reason: ' . socket_strerror(socket_last_error($socket)) . "\n"); | ||
| } | ||
|
|
||
| $clientCount = 0; | ||
| while (true) { // Loop over each client | ||
| if (($clientSocket = socket_accept($socket)) === false) { | ||
| echo 'socket_accept() failed: reason: ' . socket_strerror(socket_last_error()) . "\n"; | ||
| socket_close($clientSocket); | ||
| echo 'socket_accept() failed: reason: ' . socket_strerror(socket_last_error($socket)) . "\n"; | ||
| socket_close($socket); |
There was a problem hiding this comment.
This block runs if $clientSocket is false, so this is calling socket_close(false). I changed it to close the "parent" socket, since the code exits here.
| if ($clientSocket === false) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
The above block exits if $clientSocket is false, so at this point $clientSocket will always be a Socket object, and so this block will never run.
|
Thanks for these changes. However, some of these are part of 4.x roadmap, so I'll need to check if there could be any conflicts. Could be a while as I'll be traveling, but will try as soon as I can |
jay-knight
force-pushed
the
fix-phpstan-issues
branch
from
7aacc7b to
217834b
Compare
Is that published/available somewhere? I'd be interested to see what's in store. |
This fixes most issues identified by PHPstan up to level 5. I'll comment on each change to describe the issue.