Conversation
|
Please note that we have to support PHP 7.2, so mixed and Union types are not supported there... You should keep a look at your GitHub Action here: https://github.com/grimmdude/simplepie/actions/runs/7536994157/job/20515243462 |
tests/Unit/IRITest.php
Outdated
| $base = new IRI($base); | ||
| $this->assertSame($expected, IRI::absolutize($base, $relative)->get_iri()); | ||
| $absolutized = IRI::absolutize($base, $relative); | ||
| $this->assertSame($expected, ($absolutized ? $absolutized->get_iri() : null)); |
There was a problem hiding this comment.
These tests all assume $absolutized !== null so it might be more explicit to turn the assumption to a separate asserting. But maybe it does not make much difference to bother.
This will be redundant with the PHPStan extension.
jtojnar
left a comment
There was a problem hiding this comment.
Still need to review SimplePie and Parser.
; Conflicts: ; src/HTTP/FileClient.php
We do not use the $contextNode argument so that is really the only failure mode according to docs: https://www.php.net/manual/en/domxpath.query.php And these methods are internal but it is probably safer to throw an exception since the values might come from user config.
Since we switched from `File::$success` property to `File::$error !== null` to have `FileClient` detect failure, we need to inject the value to tests. Otherwise, PHPStan would not be able to detect if the `File::$error` property is set and thus will complain about it potentially being none when passing it to `ClientException` constructor.
| @@ -166,9 +166,9 @@ public function pass_cache_data(bool $enable_cache = true, string $cache_locatio | |||
| } | |||
|
|
|||
| // BC: $cache_name_function could be a callable as string | |||
There was a problem hiding this comment.
$cache_name_function was only allowed as string since 1.3 or longer, see https://github.com/simplepie/simplepie/blob/1.3/library/SimplePie/Sanitize.php
We switched to NameFilter in 1.8.0, but allowing string for BC.
Allowingcallable|NameFilter isntead of string|NameFilter would not be a BC break, but we should not widen the potential deprecated string type without a good reason.
; Conflicts: ; src/File.php ; src/Locator.php ; src/Misc.php ; src/Sanitize.php ; src/SimplePie.php
Co-authored-by: Artur Weigandt <[email protected]>
* FreshRSS/simplepie#45 SimplePie increased to PHPStan Level 8: * simplepie/simplepie#857
* Bump SimplePie with PHPStan Level 8 * FreshRSS/simplepie#45 SimplePie increased to PHPStan Level 8: * simplepie/simplepie#857 * Merge upstream Including my two PRs: * simplepie/simplepie#932 * simplepie/simplepie#933 * Resolve upstream sync of Expose HTTP status * FreshRSS/simplepie#47 Finalise merge, following: * simplepie/simplepie#905 (comment) * simplepie/simplepie#909 * #7038
Depends on #892, #890 , #894, #869