From da072ebda66ba5ccb6708663ed4ec5f8b6caf15b Mon Sep 17 00:00:00 2001 From: ignace nyamagana butera Date: Tue, 13 Aug 2024 22:29:15 +0200 Subject: [PATCH] Improve FragmentFinder public API --- src/Fragment/ExpressionTest.php | 52 ++++++++++++++++++++----------- src/Fragment/Selection.php | 5 +-- src/TabularDataReaderTestCase.php | 37 ++++++++++++---------- 3 files changed, 56 insertions(+), 38 deletions(-) diff --git a/src/Fragment/ExpressionTest.php b/src/Fragment/ExpressionTest.php index 83985737..db311b5b 100644 --- a/src/Fragment/ExpressionTest.php +++ b/src/Fragment/ExpressionTest.php @@ -92,30 +92,44 @@ public static function validExpressionProvider(): iterable #[Test] #[DataProvider('invalidExpressionProvider')] - public function it_will_fail_parsing_incorrect_expression(string $expression, string $expected): void + public function it_will_throw_parsing_incorrect_expression(string $expression): void { - self::assertSame($expected, Expression::from($expression)->toString()); + $this->expectException(FragmentNotFound::class); + + Expression::from($expression)->toString(); } public static function invalidExpressionProvider(): iterable { - yield 'invalid row index' => ['expression' => 'row=-1', 'expected' => 'row=']; - yield 'invalid row end' => ['expression' => 'row=1--1', 'expected' => 'row=']; - yield 'invalid row number' => ['expression' => 'row=1-four', 'expected' => 'row=']; - yield 'invalid row range infinite' => ['expression' => 'row=*-1', 'expected' => 'row=']; - yield 'invalid multiple row range' => ['expression' => 'row=1-4,2-5', 'expected' => 'row=']; - - yield 'invalid column index' => ['expression' => 'col=-1', 'expected' => 'col=']; - yield 'invalid column end' => ['expression' => 'col=1--1', 'expected' => 'col=']; - yield 'invalid column number' => ['expression' => 'col=1-four', 'expected' => 'col=']; - yield 'invalid column range infinite' => ['expression' => 'col=*-1', 'expected' => 'col=']; - yield 'invalid multiple column range' => ['expression' => 'col=1-4,2-5', 'expected' => 'col=']; - - yield 'invalid cell' => ['expression' => 'cell=1,*', 'expected' => 'cell=']; - yield 'invalid cell index' => ['expression' => 'cell=1,-3', 'expected' => 'cell=']; - yield 'invalid cell number' => ['expression' => 'cell=1,three', 'expected' => 'cell=']; - yield 'invalid cell location' => ['expression' => 'cell=2,3-1,4', 'expected' => 'cell=']; - yield 'invalid multiple cell selection' => ['expression' => 'cell=2,3-14,16;22-23', 'expected' => 'cell=2,3-14,16']; + yield 'invalid row index' => ['expression' => 'row=-1']; + yield 'invalid row end' => ['expression' => 'row=1--1']; + yield 'invalid row number' => ['expression' => 'row=1-four']; + yield 'invalid row range infinite' => ['expression' => 'row=*-1']; + yield 'invalid multiple row range' => ['expression' => 'row=1-4,2-5']; + + yield 'invalid column index' => ['expression' => 'col=-1']; + yield 'invalid column end' => ['expression' => 'col=1--1']; + yield 'invalid column number' => ['expression' => 'col=1-four']; + yield 'invalid column range infinite' => ['expression' => 'col=*-1']; + yield 'invalid multiple column range' => ['expression' => 'col=1-4,2-5']; + + yield 'invalid cell' => ['expression' => 'cell=1,*']; + yield 'invalid cell index' => ['expression' => 'cell=1,-3']; + yield 'invalid cell number' => ['expression' => 'cell=1,three']; + } + + #[Test] + #[DataProvider('ignoreExpressionProvider')] + public function it_will_fail_parsing_incorrect_expression(string $expression): void + { + $this->expectException(FragmentNotFound::class); + + Expression::from($expression); + } + + public static function ignoreExpressionProvider(): iterable + { + yield 'invalid multiple cell selection' => ['expression' => 'cell=2,3-14,16;22-23']; } #[Test] diff --git a/src/Fragment/Selection.php b/src/Fragment/Selection.php index 20f623fe..0ed71d0d 100644 --- a/src/Fragment/Selection.php +++ b/src/Fragment/Selection.php @@ -13,6 +13,7 @@ namespace League\Csv\Fragment; +use League\Csv\FragmentNotFound; use const FILTER_VALIDATE_INT; /** @@ -65,7 +66,7 @@ public static function fromColumn(string $selection): ?self public static function fromCell(string $selection): ?self { if (1 !== preg_match(self::REGEXP_CELLS_SELECTION, $selection, $found)) { - return null; + return throw new FragmentNotFound('The fragment selection "'.$selection.'" is invalid.'); } $cellStartRow = filter_var($found['csr'], FILTER_VALIDATE_INT, ['options' => ['min_range' => 1]]); @@ -108,7 +109,7 @@ public static function fromCell(string $selection): ?self private static function parseRowColumnSelection(string $selection): array { if (1 !== preg_match(self::REGEXP_ROWS_COLUMNS_SELECTION, $selection, $found)) { - return [-1, 0]; + throw new FragmentNotFound('The fragment selection "'.$selection.'" is invalid.'); } $start = $found['start']; diff --git a/src/TabularDataReaderTestCase.php b/src/TabularDataReaderTestCase.php index db26bf7a..260f7b6a 100644 --- a/src/TabularDataReaderTestCase.php +++ b/src/TabularDataReaderTestCase.php @@ -232,22 +232,35 @@ public static function provideValidExpressions(): iterable } #[Test] - public function it_will_fail_to_parse_invalid_expression(): void + #[DataProvider('provideInvalidExpressions')] + public function it_will_fail_to_parse_invalid_expression(string $expression): void { $this->expectException(Throwable::class); - iterator_to_array($this->tabularData()->matching('2-4')); + iterator_to_array($this->tabularData()->matching($expression)); + } + + public static function provideInvalidExpressions(): iterable + { + return [ + 'missing expression selection row' => ['expression' => 'row='], + 'missing expression selection cell' => ['expression' => 'cell='], + 'missing expression selection coll' => ['expression' => 'col='], + 'expression selection is invalid for cell 1' => ['expression' => 'cell=5'], + 'expression selection is invalid for row or column 1' => ['expression' => 'row=4,3'], + 'expression selection is invalid for row or column 2' => ['expression' => 'row=four-five'], + ]; } #[Test] - #[DataProvider('provideInvalidExpressions')] + #[DataProvider('provideExpressionWithIgnoredSelections')] public function it_will_return_null_on_invalid_expression(string $expression): void { self::assertNull($this->tabularData()->matchingFirst($expression)); } #[Test] - #[DataProvider('provideInvalidExpressions')] + #[DataProvider('provideExpressionWithIgnoredSelections')] public function it_will_fail_to_parse_the_expression(string $expression): void { $this->expectException(FragmentNotFound::class); @@ -255,21 +268,15 @@ public function it_will_fail_to_parse_the_expression(string $expression): void $this->tabularData()->matchingFirstOrFail($expression); } - public static function provideInvalidExpressions(): iterable + public static function provideExpressionWithIgnoredSelections(): iterable { return [ - 'missing expression selection row' => ['row='], - 'missing expression selection cell' => ['cell='], - 'missing expression selection coll' => ['col='], - 'expression selection is invalid for cell 1' => ['cell=5'], 'expression selection is invalid for cell 2' => ['cell=0,3'], 'expression selection is invalid for cell 3' => ['cell=3,0'], 'expression selection is invalid for cell 4' => ['cell=1,3-0,4'], 'expression selection is invalid for cell 5' => ['cell=1,3-4,0'], 'expression selection is invalid for cell 6' => ['cell=0,3-1,4'], 'expression selection is invalid for cell 7' => ['cell=1,0-2,3'], - 'expression selection is invalid for row or column 1' => ['row=4,3'], - 'expression selection is invalid for row or column 2' => ['row=four-five'], 'expression selection is invalid for row or column 3' => ['row=0-3'], 'expression selection is invalid for row or column 4' => ['row=3-0'], ]; @@ -278,17 +285,13 @@ public static function provideInvalidExpressions(): iterable #[Test] public function it_returns_multiple_selections_in_one_tabular_data_instance(): void { - $count = iterator_count($this->tabularData()->matching('row=1-2;5-4;2-4')); - - self::assertSame(1, $count); + self::assertSame(1, iterator_count($this->tabularData()->matching('row=1-2;5-4;2-4'))); } #[Test] public function it_returns_no_selection(): void { - $count = iterator_count($this->tabularData()->matching('row=5-4')); - - self::assertSame(0, $count); + self::assertSame(0, iterator_count($this->tabularData()->matching('row=5-4'))); } #[Test]