Improve FragmentFinder public API

thephpleague · Aug 13, 2024 · da072eb · da072eb
1 parent e4d983e
commit da072eb
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 38 deletions.
diff --git 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
@@ -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
@@ -232,44 +232,51 @@ 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);
 
         $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]