Update dependencies and tooling #173
Conversation
phil-davis
commented
Jan 29, 2020
•
phil-davis
commented
- drop PHP 7.0
- move to phpunit 7
- use later phpstan
- run php-cs-fixer in CI
|
The newer version of phpstan reports: I am looking at those. |
Codecov Report
@@ Coverage Diff @@
## master #173 +/- ##
=========================================
Coverage 97.61% 97.61%
+ Complexity 114 112 -2
=========================================
Files 13 13
Lines 461 461
=========================================
Hits 450 450
Misses 11 11
Continue to review full report at Codecov.
|
| @@ -169,7 +171,8 @@ public function parseInnerTree(array $elementMap = null) | |||
| case self::END_ELEMENT: | |||
| // Ensuring we are moving the cursor after the end element. | |||
| $this->read(); | |||
| break 2; | |||
There was a problem hiding this comment.
This break 2 was causing phpstan to report:
Line 187 Unreachable statement - code above always terminates.
phpstan seems to not realise that break 2 is a way out of both the switch and while statements.
It is easy enough to refactor the code to have a boolean to control the while loop. And maybe that makes it easier to read?
There was a problem hiding this comment.
we should report a upstream issue.. in case you dont want/don't have time to do it, i will do it.
just let me know.
There was a problem hiding this comment.
@staabm you know better exactly about upstream. Please report.
There was a problem hiding this comment.
will do. thx.
IMO the newly introduced logic is better to read, because its now obvious what this single spot is trying to achieve. the break 2 could also be read as a mistake
| @@ -165,8 +164,6 @@ function standardSerializer(Writer $writer, $value) | |||
| } elseif (is_callable($value)) { | |||
| // A callback | |||
| $value($writer); | |||
| } elseif (is_null($value)) { | |||
There was a problem hiding this comment.
phpstan complains:
167 Call to function is_null() with array|object will always evaluate to false
It knows that $value at this point might be an array or object.
After looking at the code, there are elseif cases below that explicitly handle the array and object cases. So I refactored so that is_null() is not needed here.
| bootstrap: %currentWorkingDirectory%/vendor/autoload.php | ||
| ignoreErrors: | ||
| - | ||
| message: '!Parameter #3 \$uri of method XMLWriter::startElementNs\(\) expects string, null given.!' |
There was a problem hiding this comment.
phpstan complains:
130 Parameter #3 $uri of method XMLWriter::startElementNs() expects string, null given.
The code is:
$result = $this->startElementNS(
'' === $this->namespaceMap[$namespace] ? null : $this->namespaceMap[$namespace],
$localName,
null
);
Actually phpstan is correct - startElementNS is supposed to take a string and not allow null to be passed.
But also I found https://www.php.net/manual/en/function.xmlwriter-start-element-ns.php#76920
If you don't want any namespace prefix at all but still want the xmlns attribute, set $prefix to null.
So I ignored this phpstan error, rather than "fixing" the code.
…array|object will always evaluate to false'
…:startElementNs() expects string, null given'
