Icinga · nilmerg · Feb 12, 2024 · Jan 25, 2024 · Jan 25, 2024 · Feb 12, 2024
diff --git 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:
@@ -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 }}
@@ -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:

diff --git 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
@@ -7,13 +7,11 @@ parameters:
     checkFunctionNameCase: true
     checkInternalClassCaseSensitivity: true
     treatPhpDocTypesAsCertain: false
+    checkGenericClassInNonGenericObjectType: false
 
     paths:
         - src
 
-    scanDirectories:
-        - vendor
-
     ignoreErrors:
         -
             messages:

diff --git 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/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
@@ -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
@@ -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 [];
         }
 

diff --git 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');