From f99abefc204c2ecd2908b6a359923e692d1d2fd3 Mon Sep 17 00:00:00 2001 From: Sukhwinder Dhillon Date: Thu, 25 Jan 2024 01:50:37 +0100 Subject: [PATCH 1/5] phpstan: Streamline vendor file location with local dev-env phpstan is not run with an action anymore, as the action runs it its own docker container and hence has no access to files outside the repository root. A side-effect of this is, that phpstan now **really** runs with the php version set up by the matrix. --- .github/workflows/php.yml | 4 ++-- phpstan.neon | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 772519d..4965b68 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -31,7 +31,7 @@ jobs: tools: phpcs - name: Setup dependencies - run: composer require -n --no-progress overtrue/phplint + run: composer require -n --no-progress overtrue/phplint phpstan/phpstan - name: PHP Lint if: ${{ ! cancelled() }} @@ -43,7 +43,7 @@ jobs: - name: PHPStan if: ${{ ! cancelled() }} - uses: php-actions/phpstan@v3 + run: ./vendor/bin/phpstan analyse test: name: Unit tests with php ${{ matrix.php }} on ${{ matrix.os }} diff --git a/phpstan.neon b/phpstan.neon index b0ed57d..ef6cdd2 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -11,9 +11,6 @@ parameters: paths: - src - scanDirectories: - - vendor - ignoreErrors: - messages: From 35518beb20a18981e5cf27ac843eb2edb3bf596c Mon Sep 17 00:00:00 2001 From: Sukhwinder Dhillon Date: Thu, 25 Jan 2024 02:25:04 +0100 Subject: [PATCH 2/5] GithubActions: Add php 8.3 --- .github/workflows/php.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 4965b68..c2e0753 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -17,7 +17,7 @@ jobs: strategy: fail-fast: false matrix: - php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2'] + php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3'] os: ['ubuntu-latest'] steps: @@ -55,7 +55,7 @@ jobs: strategy: fail-fast: false matrix: - php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2'] + php: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3'] os: ['ubuntu-latest'] steps: From 08d047a2b0d59d1342ca04dade29f6810d6471e5 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 12 Feb 2024 13:03:04 +0100 Subject: [PATCH 3/5] Filter: Fix matching of null values --- src/Filter.php | 48 ++++++++++++------------- tests/FilterTest.php | 86 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 106 insertions(+), 28 deletions(-) diff --git a/src/Filter.php b/src/Filter.php index 9523f1f..3bbdd36 100644 --- a/src/Filter.php +++ b/src/Filter.php @@ -2,7 +2,6 @@ namespace ipl\Stdlib; -use Exception; use InvalidArgumentException; use ipl\Stdlib\Filter\All; use ipl\Stdlib\Filter\Any; @@ -18,6 +17,7 @@ use ipl\Stdlib\Filter\Rule; use ipl\Stdlib\Filter\Unequal; use ipl\Stdlib\Filter\Unlike; +use Throwable; class Filter { @@ -258,19 +258,14 @@ protected function performEqualityMatch($value, $rowValue, $ignoreCase = false) { if ($ignoreCase && is_string($rowValue)) { $rowValue = strtolower($rowValue); + /** @var string|string[] $value {@see self::normalizeTypes} ensures this is the case */ $value = is_array($value) - ? array_map(function ($val) { - return strtolower((string) $val); - }, $value) - : strtolower((string) $value); + ? array_map('strtolower', $value) + : ($value === null ? null : strtolower($value)); // phpstan is wrong here } if (is_array($value)) { return in_array($rowValue, $value, true); - } elseif (! is_string($value)) { - if (is_string($rowValue)) { - $value = (string) $value; - } } return $rowValue === $value; @@ -279,23 +274,26 @@ protected function performEqualityMatch($value, $rowValue, $ignoreCase = false) /** * Apply similarity matching rules on the given row value * - * @param string|string[] $value - * @param string $rowValue + * @param mixed $value + * @param mixed $rowValue * @param bool $ignoreCase * * @return bool */ protected function performSimilarityMatch($value, $rowValue, $ignoreCase = false) { - if ($ignoreCase) { + if ($ignoreCase && is_string($rowValue)) { $rowValue = strtolower($rowValue); + /** @var string|string[] $value {@see self::normalizeTypes} ensures this is the case */ $value = is_array($value) ? array_map('strtolower', $value) - : strtolower($value); + : ($value === null ? null : strtolower($value)); // phpstan is wrong here } if (is_array($value)) { return in_array($rowValue, $value, true); + } elseif (! is_string($value) || ! is_string($rowValue)) { + return $this->performEqualityMatch($value, $rowValue); } $wildcardSubSegments = preg_split('~\*~', $value); @@ -394,7 +392,10 @@ public static function greaterThan($column, $value) */ protected function matchGreaterThan(GreaterThan $rule, $row) { - return $this->extractValue($rule->getColumn(), $row) > $rule->getValue(); + $rowValue = $this->extractValue($rule->getColumn(), $row); + $value = $rule->getValue(); + + return $rowValue !== null && $value !== null && $rowValue > $value; } /** @@ -421,11 +422,9 @@ public static function lessThan($column, $value) protected function matchLessThan(LessThan $rule, $row) { $rowValue = $this->extractValue($rule->getColumn(), $row); - if ($rowValue === null) { - return false; - } + $value = $rule->getValue(); - return $rowValue < $rule->getValue(); + return $rowValue !== null && $value !== null && $rowValue < $value; } /** @@ -451,7 +450,10 @@ public static function greaterThanOrEqual($column, $value) */ protected function matchGreaterThanOrEqual(GreaterThanOrEqual $rule, $row) { - return $this->extractValue($rule->getColumn(), $row) >= $rule->getValue(); + $rowValue = $this->extractValue($rule->getColumn(), $row); + $value = $rule->getValue(); + + return $rowValue !== null && $value !== null && $rowValue >= $value; } /** @@ -478,11 +480,9 @@ public static function lessThanOrEqual($column, $value) protected function matchLessThanOrEqual(LessThanOrEqual $rule, $row) { $rowValue = $this->extractValue($rule->getColumn(), $row); - if ($rowValue === null) { - return false; - } + $value = $rule->getValue(); - return $rowValue <= $rule->getValue(); + return $rowValue !== null && $value !== null && $rowValue <= $value; } /** @@ -538,7 +538,7 @@ protected function extractValue($column, $row) { try { return $row->{$column}; - } catch (Exception $_) { + } catch (Throwable $_) { return null; } } diff --git a/tests/FilterTest.php b/tests/FilterTest.php index f7fc277..45fb76e 100644 --- a/tests/FilterTest.php +++ b/tests/FilterTest.php @@ -457,10 +457,6 @@ public function testConditionsAreValueTypeAgnostic() Filter::match(Filter::equal('some_id', null), ['some_id' => null]), "Filter\Equal fails to match NULL" ); - $this->assertFalse( - Filter::match(Filter::equal('some_id', 0), ['some_id' => null]), - "Filter\Equal doesn't compare NULL strictly" - ); $this->assertTrue( Filter::match(Filter::greaterThan('length', '21'), ['length' => 22]), "Filter\GreaterThan fails to match strings with integers" @@ -471,6 +467,88 @@ public function testConditionsAreValueTypeAgnostic() ); } + public function testConditionsNeverEvaluateToTrueForNullValues() + { + $this->assertFalse( + Filter::match(Filter::equal('some_id', 0), ['some_id' => null]), + "Filter\Equal doesn't compare NULL always to FALSE" + ); + $this->assertFalse( + Filter::match(Filter::equal('some_id', null), ['some_id' => 0]), + "Filter\Equal doesn't compare NULL always to FALSE" + ); + + $this->assertFalse( + Filter::match(Filter::equal('some_id', null), ['some_id' => '']), + "Filter\Equal doesn't compare NULL always to FALSE" + ); + $this->assertFalse( + Filter::match(Filter::equal('some_id', ''), ['some_id' => null]), + "Filter\Equal doesn't compare NULL always to FALSE" + ); + $this->assertFalse( + Filter::match(Filter::equal('some_col', null)->ignoreCase(), ['some_col' => '']), + "Filter\Equal doesn't compare NULL always to FALSE" + ); + $this->assertFalse( + Filter::match(Filter::equal('some_col', '')->ignoreCase(), ['some_col' => null]), + "Filter\Equal doesn't compare NULL always to FALSE" + ); + + $this->assertFalse( + Filter::match(Filter::like('some_id', null), ['some_id' => '']), + "Filter\Like doesn't compare NULL always to FALSE" + ); + $this->assertFalse( + Filter::match(Filter::like('some_id', ''), ['some_id' => null]), + "Filter\Like doesn't compare NULL always to FALSE" + ); + $this->assertFalse( + Filter::match(Filter::like('some_col', null)->ignoreCase(), ['some_col' => '']), + "Filter\Like doesn't compare NULL always to FALSE" + ); + $this->assertFalse( + Filter::match(Filter::like('some_col', '')->ignoreCase(), ['some_col' => null]), + "Filter\Like doesn't compare NULL always to FALSE" + ); + + $this->assertFalse( + Filter::match(Filter::greaterThan('some_id', 1), ['some_id' => null]), + "Filter\GreaterThan doesn't compare NULL always to FALSE" + ); + $this->assertFalse( + Filter::match(Filter::greaterThan('some_id', null), ['some_id' => -1]), + "Filter\GreaterThan doesn't compare NULL always to FALSE" + ); + + $this->assertFalse( + Filter::match(Filter::greaterThanOrEqual('some_id', 1), ['some_id' => null]), + "Filter\GreaterThanOrEqual doesn't compare NULL always to FALSE" + ); + $this->assertFalse( + Filter::match(Filter::greaterThanOrEqual('some_id', null), ['some_id' => -1]), + "Filter\GreaterThanOrEqual doesn't compare NULL always to FALSE" + ); + + $this->assertFalse( + Filter::match(Filter::lessThan('some_id', -1), ['some_id' => null]), + "Filter\GreaterThan doesn't compare NULL always to FALSE" + ); + $this->assertFalse( + Filter::match(Filter::lessThan('some_id', null), ['some_id' => 1]), + "Filter\GreaterThan doesn't compare NULL always to FALSE" + ); + + $this->assertFalse( + Filter::match(Filter::lessThanOrEqual('some_id', -1), ['some_id' => null]), + "Filter\GreaterThanOrEqual doesn't compare NULL always to FALSE" + ); + $this->assertFalse( + Filter::match(Filter::lessThanOrEqual('some_id', null), ['some_id' => 1]), + "Filter\GreaterThanOrEqual doesn't compare NULL always to FALSE" + ); + } + public function testConditionsCanBeCloned() { $condition1 = Filter::equal('host', 'localhost'); From 3500c52072007fabc24666cef3105a1102e2a4af Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 12 Feb 2024 13:03:44 +0100 Subject: [PATCH 4/5] Improve several type hints --- src/PriorityQueue.php | 1 + src/Seq.php | 14 ++++++++++---- src/Str.php | 4 ++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/PriorityQueue.php b/src/PriorityQueue.php index 9047af4..c3576e8 100644 --- a/src/PriorityQueue.php +++ b/src/PriorityQueue.php @@ -36,6 +36,7 @@ public function yieldAll() $queue->setExtractFlags(static::EXTR_BOTH); foreach ($queue as $item) { + /** @var array{priority: array{0: mixed, 1: int}, data: mixed} $item */ yield $item['priority'][0] => $item['data']; } } diff --git a/src/Seq.php b/src/Seq.php index 02a3bd0..dbae277 100644 --- a/src/Seq.php +++ b/src/Seq.php @@ -50,8 +50,11 @@ public static function find($traversable, $needle, $caseSensitive = true) $item = strtolower($item); } - if ($usesCallback && $needle($item)) { - return [$key, $originalItem]; + if ($usesCallback) { + /** @var Closure $needle */ + if ($needle($item)) { + return [$key, $originalItem]; + } } elseif ($item === $needle) { return [$key, $originalItem]; } @@ -99,8 +102,11 @@ public static function findValue($traversable, $needle, $caseSensitive = true) $key = strtolower($key); } - if ($usesCallback && $needle($key)) { - return $item; + if ($usesCallback) { + /** @var Closure $needle */ + if ($needle($key)) { + return $item; + } } elseif ($key === $needle) { return $item; } diff --git a/src/Str.php b/src/Str.php index 9cf1cae..4fb07ad 100644 --- a/src/Str.php +++ b/src/Str.php @@ -60,7 +60,7 @@ public static function startsWith(?string $subject, string $start, bool $caseSen */ public static function symmetricSplit(?string $subject, string $delimiter, int $limit, $default = null) { - if ($subject === null) { + if ($subject === null || empty($delimiter)) { return array_pad([], $limit, $default); } @@ -78,7 +78,7 @@ public static function symmetricSplit(?string $subject, string $delimiter, int $ */ public static function trimSplit(?string $subject, string $delimiter = ',', int $limit = null) { - if ($subject === null) { + if ($subject === null || empty($delimiter)) { return []; } From 03965671a1ff1e738737eec2130831179092f2cd Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 12 Feb 2024 13:06:10 +0100 Subject: [PATCH 5/5] phpstan: Update configuration --- phpstan-baseline.neon | 41 +++-------------------------------------- phpstan.neon | 1 + 2 files changed, 4 insertions(+), 38 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index eaed12a..2ebfbe8 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,46 +1,11 @@ parameters: ignoreErrors: - - message: "#^Cannot cast mixed to string\\.$#" - count: 2 - path: src/Filter.php - - - - message: "#^Dead catch \\- Exception is never thrown in the try block\\.$#" + message: "#^Dead catch \\- Throwable is never thrown in the try block\\.$#" count: 1 path: src/Filter.php - - message: "#^Class ipl\\\\Stdlib\\\\Filter\\\\Chain implements generic interface IteratorAggregate but does not specify its types\\: TKey, TValue$#" - count: 1 - path: src/Filter/Chain.php - - - - message: "#^Cannot access offset 'data' on mixed\\.$#" - count: 1 - path: src/PriorityQueue.php - - - - message: "#^Cannot access offset 'priority' on mixed\\.$#" - count: 1 - path: src/PriorityQueue.php - - - - message: "#^Cannot access offset 0 on mixed\\.$#" - count: 1 - path: src/PriorityQueue.php - - - - message: "#^Class ipl\\\\Stdlib\\\\PriorityQueue extends generic class SplPriorityQueue but does not specify its types\\: TPriority, TValue$#" - count: 1 - path: src/PriorityQueue.php - - - - message: "#^Trying to invoke mixed but it's not a callable\\.$#" + message: "#^Parameter \\#1 \\$(str|string) of function strtolower expects string, mixed given\\.$#" count: 2 - path: src/Seq.php - - - - message: "#^Parameter \\#1 \\$separator of function explode expects non\\-empty\\-string, string given\\.$#" - count: 3 - path: src/Str.php + path: src/Filter.php diff --git a/phpstan.neon b/phpstan.neon index ef6cdd2..cc1f8de 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -7,6 +7,7 @@ parameters: checkFunctionNameCase: true checkInternalClassCaseSensitivity: true treatPhpDocTypesAsCertain: false + checkGenericClassInNonGenericObjectType: false paths: - src