From df40f463dace76a456e8111df41748dde256b52a Mon Sep 17 00:00:00 2001 From: Gary Gitton Date: Wed, 15 Dec 2021 00:12:43 +0100 Subject: [PATCH 1/6] add test on glob and GLOB_BRACE flag Signed-off-by: Gary Gitton --- src/Glob.php | 16 ++++++++-- test/GlobTest.php | 71 ++++++++++++++++++++++++++++++++++++++--- test/_files/{alph,bet}a | 0 3 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 test/_files/{alph,bet}a diff --git a/src/Glob.php b/src/Glob.php index dd187758..60b8c166 100644 --- a/src/Glob.php +++ b/src/Glob.php @@ -109,7 +109,7 @@ protected static function systemGlob($pattern, $flags) */ protected static function fallbackGlob($pattern, $flags) { - if (! $flags & self::GLOB_BRACE) { + if (self::flagsIsEqualTo($flags, self::GLOB_BRACE)) { return static::systemGlob($pattern, $flags); } @@ -195,14 +195,19 @@ protected static function nextBraceSub($pattern, $begin, $flags) $current = $begin; while ($current < $length) { - if (! $flags & self::GLOB_NOESCAPE && $pattern[$current] === '\\') { + $flagsEqualsNoEscape = self::flagsIsEqualTo($flags, self::GLOB_NOESCAPE); + + if ($flagsEqualsNoEscape && $pattern[$current] === '\\') { if (++$current === $length) { break; } $current++; } else { - if (($pattern[$current] === '}' && $depth-- === 0) || ($pattern[$current] === ',' && $depth === 0)) { + if ( + ($pattern[$current] === '}' && $depth-- === 0) + || ($pattern[$current] === ',' && $depth === 0) + ) { break; } elseif ($pattern[$current++] === '{') { $depth++; @@ -212,4 +217,9 @@ protected static function nextBraceSub($pattern, $begin, $flags) return $current < $length ? $current : null; } + + public static function flagsIsEqualTo(int $flags, int $otherFlags): bool + { + return (bool) ($flags & $otherFlags); + } } diff --git a/test/GlobTest.php b/test/GlobTest.php index 7e74754d..a1d08909 100644 --- a/test/GlobTest.php +++ b/test/GlobTest.php @@ -11,6 +11,7 @@ use function count; use function defined; use function glob; +use function realpath; use function str_repeat; use const GLOB_BRACE; @@ -23,15 +24,29 @@ public function testFallback(): void $this->markTestSkipped('GLOB_BRACE not available'); } - self::assertEquals( - glob(__DIR__ . '/_files/{alph,bet}a', GLOB_BRACE), - Glob::glob(__DIR__ . '/_files/{alph,bet}a', Glob::GLOB_BRACE, true) + $expected = glob(__DIR__ . '/_files/{alph,bet}a', GLOB_BRACE); + $actual = Glob::glob( + __DIR__ . '/_files/{alph,bet}a', + Glob::GLOB_BRACE, + true + ); + + self::assertEquals($actual, $expected); + + $notExpectedPath = realpath(__DIR__ . '/_files/{alph,bet}a'); + + self::assertNotContains( + $notExpectedPath, + $actual ); } public function testNonMatchingGlobReturnsArray(): void { - $result = Glob::glob('/some/path/{,*.}{this,orthis}.php', Glob::GLOB_BRACE); + $result = Glob::glob( + '/some/path/{,*.}{this,orthis}.php', + Glob::GLOB_BRACE + ); self::assertIsArray($result); } @@ -80,4 +95,52 @@ public function patternsProvider(): array ], ]; } + + public function testGlobWithoutGlobBraceFlag(): void + { + $expected = [ + realpath(__DIR__ . '/_files/{alph,bet}a'), + ]; + + self::assertEquals( + glob(__DIR__ . '/_files/{alph,bet}a', 0), + $expected + ); + } + + /** + * @psalm-return array + */ + public function flagsIsEqualsToMethodDataProvider(): array + { + return [ + [ + Glob::GLOB_BRACE, + Glob::GLOB_BRACE, + true, + ], + [ + Glob::GLOB_BRACE, + Glob::GLOB_NOSORT, + false, + ], + ]; + } + + /** + * @dataProvider flagsIsEqualsToMethodDataProvider + */ + public function testFlagsIsEqualsToMethod( + int $flags, + int $otherFlags, + bool $expected + ): void { + $actual = Glob::flagsIsEqualTo($flags, $otherFlags); + + $this->assertEquals($expected, $actual); + } } diff --git a/test/_files/{alph,bet}a b/test/_files/{alph,bet}a new file mode 100644 index 00000000..e69de29b From 1a588a5f21dca2d9ac06580b23093216f946a8d1 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 24 Dec 2021 18:22:14 +0100 Subject: [PATCH 2/6] #43 marking `Glob::flagsIsEqualTo()` as `@internal` to prevent API surface proliferation --- src/Glob.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Glob.php b/src/Glob.php index 60b8c166..5f7fc094 100644 --- a/src/Glob.php +++ b/src/Glob.php @@ -218,6 +218,7 @@ protected static function nextBraceSub($pattern, $begin, $flags) return $current < $length ? $current : null; } + /** @internal */ public static function flagsIsEqualTo(int $flags, int $otherFlags): bool { return (bool) ($flags & $otherFlags); From 20a147676170ac50e7062a3effd793f1a63a3055 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 24 Dec 2021 18:26:08 +0100 Subject: [PATCH 3/6] #43 `GlobTest` is allowed to call internal method `Glob::flagsIsEqualTo()` --- test/GlobTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/GlobTest.php b/test/GlobTest.php index a1d08909..6a291524 100644 --- a/test/GlobTest.php +++ b/test/GlobTest.php @@ -139,6 +139,10 @@ public function testFlagsIsEqualsToMethod( int $otherFlags, bool $expected ): void { + /** + * @psalm-suppress InternalMethod this test is specifically testing the behavior of this method, + * to prevent regressions + */ $actual = Glob::flagsIsEqualTo($flags, $otherFlags); $this->assertEquals($expected, $actual); From 2d3358fd3f4f6e390b24256bd6cc7096334e944b Mon Sep 17 00:00:00 2001 From: Gary Gitton Date: Tue, 14 Dec 2021 22:46:24 +0100 Subject: [PATCH 4/6] return toArray value only for traversable which are not iterator Signed-off-by: Gary Gitton --- src/ArrayUtils.php | 7 +- test/ArrayUtilsTest.php | 27 ++++++++ test/TestAsset/IteratorWithToArrayMethod.php | 69 ++++++++++++++++++++ 3 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 test/TestAsset/IteratorWithToArrayMethod.php diff --git a/src/ArrayUtils.php b/src/ArrayUtils.php index 454a7a5b..2cf61e08 100644 --- a/src/ArrayUtils.php +++ b/src/ArrayUtils.php @@ -5,6 +5,7 @@ namespace Laminas\Stdlib; +use Iterator; use Laminas\Stdlib\ArrayUtils\MergeRemoveKey; use Laminas\Stdlib\ArrayUtils\MergeReplaceKeyInterface; use Traversable; @@ -240,7 +241,11 @@ public static function iteratorToArray($iterator, $recursive = true) return iterator_to_array($iterator); } - if (is_object($iterator) && method_exists($iterator, 'toArray')) { + if ( + is_object($iterator) + && ! $iterator instanceof Iterator + && method_exists($iterator, 'toArray') + ) { return $iterator->toArray(); } diff --git a/test/ArrayUtilsTest.php b/test/ArrayUtilsTest.php index 9982d904..89acdb98 100644 --- a/test/ArrayUtilsTest.php +++ b/test/ArrayUtilsTest.php @@ -11,6 +11,7 @@ use Laminas\Stdlib\ArrayUtils\MergeReplaceKeyInterface; use Laminas\Stdlib\Exception\InvalidArgumentException; use Laminas\Stdlib\Parameters; +use LaminasTest\Stdlib\TestAsset\IteratorWithToArrayMethod; use PHPUnit\Framework\TestCase; use stdClass; use Traversable; @@ -579,4 +580,30 @@ public function testInvalidCallableRaiseInvalidArgumentException(): void $this->expectException(InvalidArgumentException::class); ArrayUtils::filter([], "INVALID"); } + + /** + * @link https://github.com/laminas/laminas-stdlib/issues/18 + */ + public function testIteratorToArrayWithIteratorHavingMethodToArrayAndRecursiveIsFalse() + { + $arrayB = [ + 'foo' => 'bar', + ]; + $iteratorB = new IteratorWithToArrayMethod($arrayB); + + $arrayA = [ + 'iteratorB' => $iteratorB, + ]; + $iterator = new IteratorWithToArrayMethod($arrayA); + + $result = ArrayUtils::iteratorToArray($iterator, true); + + $expectedResult = [ + 'iteratorB' => [ + 'foo' => 'bar', + ], + ]; + + $this->assertEquals($expectedResult, $result); + } } diff --git a/test/TestAsset/IteratorWithToArrayMethod.php b/test/TestAsset/IteratorWithToArrayMethod.php new file mode 100644 index 00000000..0fd4ef02 --- /dev/null +++ b/test/TestAsset/IteratorWithToArrayMethod.php @@ -0,0 +1,69 @@ +elements = $elements; + } + } + + /** @return void */ + #[ReturnTypeWillChange] + public function rewind() + { + reset($this->elements); + } + + /** @return mixed */ + #[ReturnTypeWillChange] + public function current() + { + return current($this->elements); + } + + /** @return int|string */ + #[ReturnTypeWillChange] + public function key() + { + return key($this->elements); + } + + /** @return mixed */ + #[ReturnTypeWillChange] + public function next() + { + return next($this->elements); + } + + /** @return bool */ + #[ReturnTypeWillChange] + public function valid() + { + $key = key($this->elements); + return $key !== null && $key !== false; + } + + public function toArray(): array + { + return [ + 'data from to array' => 'not good', + ]; + } +} From 69cb131f6a1aca385d117e29e8ef6c05c6b474d1 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 24 Dec 2021 18:38:24 +0100 Subject: [PATCH 5/6] #42 added return type declaration to newly added test method --- test/ArrayUtilsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ArrayUtilsTest.php b/test/ArrayUtilsTest.php index 89acdb98..cac6748e 100644 --- a/test/ArrayUtilsTest.php +++ b/test/ArrayUtilsTest.php @@ -584,7 +584,7 @@ public function testInvalidCallableRaiseInvalidArgumentException(): void /** * @link https://github.com/laminas/laminas-stdlib/issues/18 */ - public function testIteratorToArrayWithIteratorHavingMethodToArrayAndRecursiveIsFalse() + public function testIteratorToArrayWithIteratorHavingMethodToArrayAndRecursiveIsFalse(): void { $arrayB = [ 'foo' => 'bar', From 11c31f85ed0ff232e369f7955da88ba08e3372b9 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 24 Dec 2021 18:40:22 +0100 Subject: [PATCH 6/6] #42 corrected `IteratorWithToArrayMethod` to be compatible with PHP 7.3+ This effectively removes a property type definition (only for PHP 7.4+) --- test/TestAsset/IteratorWithToArrayMethod.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/TestAsset/IteratorWithToArrayMethod.php b/test/TestAsset/IteratorWithToArrayMethod.php index 0fd4ef02..8857771b 100644 --- a/test/TestAsset/IteratorWithToArrayMethod.php +++ b/test/TestAsset/IteratorWithToArrayMethod.php @@ -8,20 +8,18 @@ use ReturnTypeWillChange; use function current; -use function is_array; use function key; use function next; use function reset; class IteratorWithToArrayMethod implements Iterator { - private array $elements = []; + /** @var array */ + private $elements = []; public function __construct(array $elements) { - if (is_array($elements)) { - $this->elements = $elements; - } + $this->elements = $elements; } /** @return void */