From fcab5ba68ffade43f2cf1538f391b63dc757baae Mon Sep 17 00:00:00 2001 From: Ignace Nyamagana Butera Date: Mon, 14 Aug 2017 13:58:38 +0200 Subject: [PATCH] remove ValidatorTrait --- CHANGELOG.md | 4 +-- src/AbstractCsv.php | 44 ++++++++++++++++++------- src/Reader.php | 18 +++++++--- src/ResultSet.php | 5 +-- src/Stream.php | 45 ++++++++++++++++++------- src/ValidatorTrait.php | 75 ------------------------------------------ src/Writer.php | 15 ++++++--- src/functions.php | 18 ++++++++-- tests/CsvTest.php | 4 ++- tests/ReaderTest.php | 7 ++-- tests/StreamTest.php | 18 ++++++++++ tests/WriterTest.php | 3 +- 12 files changed, 134 insertions(+), 122 deletions(-) delete mode 100644 src/ValidatorTrait.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 708b3ed0..34269921 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ All Notable changes to `Csv` will be documented in this file - `League\Csv\HTMLConverter` converts CSV records into HTML table. - Improved Exception handling - `League\Csv\Exception` the default exception - - `League\Csv\Exception\CannotInsertRecord` + - `League\Csv\CannotInsertRecord` - Improved CSV document output - `League\Csv\AbstractCsv::chunk` method to output the CSV document in chunk - `League\Csv\bom_match` function to detect BOM sequence in a given string @@ -32,7 +32,7 @@ All Notable changes to `Csv` will be documented in this file - `League\Csv\Writer::setFlushThreshold` - `League\Csv\Writer::getFlushThreshold` - Improve RFC4180 compliance - - `League\Csv\RFC4180FieldFormatter` to format field according to RFC4180 rules + - `League\Csv\RFC4180Field` to format field according to RFC4180 rules ### Deprecated diff --git a/src/AbstractCsv.php b/src/AbstractCsv.php index dbfb79b9..e8b0cc52 100644 --- a/src/AbstractCsv.php +++ b/src/AbstractCsv.php @@ -28,8 +28,6 @@ */ abstract class AbstractCsv implements ByteSequence { - use ValidatorTrait; - /** * The stream filter mode (read or write) * @@ -325,16 +323,24 @@ public function output(string $filename = null): int * * @param string $delimiter * + * @throws Exception If the Csv control character is not one character only. + * * @return static */ public function setDelimiter(string $delimiter): self { - if ($delimiter != $this->delimiter) { - $this->delimiter = $this->filterControl($delimiter, 'delimiter', __METHOD__); + if ($delimiter === $this->delimiter) { + return $this; + } + + if (1 === strlen($delimiter)) { + $this->delimiter = $delimiter; $this->resetProperties(); + + return $this; } - return $this; + throw new Exception(sprintf('%s() expects delimiter to be a single character %s given', __METHOD__, $delimiter)); } /** @@ -349,16 +355,24 @@ protected function resetProperties() * * @param string $enclosure * + * @throws Exception If the Csv control character is not one character only. + * * @return static */ public function setEnclosure(string $enclosure): self { - if ($enclosure != $this->enclosure) { - $this->enclosure = $this->filterControl($enclosure, 'enclosure', __METHOD__); + if ($enclosure === $this->enclosure) { + return $this; + } + + if (1 === strlen($enclosure)) { + $this->enclosure = $enclosure; $this->resetProperties(); + + return $this; } - return $this; + throw new Exception(sprintf('%s() expects enclosure to be a single character %s given', __METHOD__, $enclosure)); } /** @@ -366,16 +380,24 @@ public function setEnclosure(string $enclosure): self * * @param string $escape * + * @throws Exception If the Csv control character is not one character only. + * * @return static */ public function setEscape(string $escape): self { - if ($escape != $this->escape) { - $this->escape = $this->filterControl($escape, 'escape', __METHOD__); + if ($escape === $this->escape) { + return $this; + } + + if (1 === strlen($escape)) { + $this->escape = $escape; $this->resetProperties(); + + return $this; } - return $this; + throw new Exception(sprintf('%s() expects escape to be a single character %s given', __METHOD__, $escape)); } /** diff --git a/src/Reader.php b/src/Reader.php index 694ca084..8934ba5f 100644 --- a/src/Reader.php +++ b/src/Reader.php @@ -21,6 +21,7 @@ use IteratorAggregate; use JsonSerializable; use SplFileObject; +use TypeError; /** * A class to manage records selection from a CSV document @@ -308,12 +309,21 @@ protected function stripBOM(Iterator $iterator, string $bom): Iterator */ public function setHeaderOffset($offset): self { - $this->filterNullableInteger($offset, 0, __METHOD__.'() expects the header offset index to be a positive integer or 0'); - if ($offset !== $this->header_offset) { - $this->header_offset = $offset; - $this->resetProperties(); + if ($offset === $this->header_offset) { + return $this; } + if (!is_nullable_int($offset)) { + throw new TypeError(sprintf(__METHOD__.'() expects 1 Argument to be null or an integer %s given', gettype($offset))); + } + + if (null !== $offset && 0 > $offset) { + throw new Exception(__METHOD__.'() expects 1 Argument to be greater or equal to 0'); + } + + $this->header_offset = $offset; + $this->resetProperties(); + return $this; } diff --git a/src/ResultSet.php b/src/ResultSet.php index e3358bc9..5305ffef 100644 --- a/src/ResultSet.php +++ b/src/ResultSet.php @@ -169,10 +169,7 @@ public function fetchColumn($index = 0): Generator */ protected function getColumnIndex($field, string $error_message) { - $method = 'getColumnIndexByKey'; - if (is_string($field)) { - $method = 'getColumnIndexByValue'; - } + $method = is_string($field) ? 'getColumnIndexByValue' : 'getColumnIndexByKey'; return $this->$method($field, $error_message); } diff --git a/src/Stream.php b/src/Stream.php index 57eae3b8..4e6602d9 100644 --- a/src/Stream.php +++ b/src/Stream.php @@ -30,8 +30,6 @@ */ class Stream implements SeekableIterator { - use ValidatorTrait; - /** * Attached filters * @@ -238,9 +236,36 @@ public function appendFilter(string $filtername, int $read_write, $params = null */ public function setCsvControl(string $delimiter = ',', string $enclosure = '"', string $escape = '\\') { - $this->delimiter = $this->filterControl($delimiter, 'delimiter', __METHOD__); - $this->enclosure = $this->filterControl($enclosure, 'enclosure', __METHOD__); - $this->escape = $this->filterControl($escape, 'escape', __METHOD__); + list($this->delimiter, $this->enclosure, $this->escape) = $this->filterControl($delimiter, $enclosure, $escape, __METHOD__); + } + + /** + * Filter Csv control characters + * + * @param string $delimiter CSV delimiter character + * @param string $enclosure CSV enclosure character + * @param string $escape CSV escape character + * @param string $caller public API method calling the method + * + * @throws Exception If the Csv control character is not one character only. + * + * @return array + */ + protected function filterControl(string $delimiter, string $enclosure, string $escape, string $caller): array + { + if (1 !== strlen($delimiter)) { + throw new Exception(sprintf('%s() expects delimiter to be a single character', $caller)); + } + + if (1 !== strlen($enclosure)) { + throw new Exception(sprintf('%s() expects enclosure to be a single character', $caller)); + } + + if (1 !== strlen($escape)) { + throw new Exception(sprintf('%s() expects escape to be a single character', $caller)); + } + + return [$delimiter, $enclosure, $escape]; } /** @@ -281,13 +306,9 @@ public function setFlags(int $flags) */ public function fputcsv(array $fields, string $delimiter = ',', string $enclosure = '"', string $escape = '\\') { - return fputcsv( - $this->stream, - $fields, - $this->filterControl($delimiter, 'delimiter', __METHOD__), - $this->filterControl($enclosure, 'enclosure', __METHOD__), - $this->filterControl($escape, 'escape', __METHOD__) - ); + $controls = $this->filterControl($delimiter, $enclosure, $escape, __METHOD__); + + return fputcsv($this->stream, $fields, ...$controls); } /** diff --git a/src/ValidatorTrait.php b/src/ValidatorTrait.php deleted file mode 100644 index 2283015e..00000000 --- a/src/ValidatorTrait.php +++ /dev/null @@ -1,75 +0,0 @@ - - * @internal Use to validate incoming data - */ -trait ValidatorTrait -{ - /** - * Filter Csv control character - * - * @param string $char Csv control character - * @param string $type Csv control character type - * @param string $caller public API method calling the method - * - * @throws Exception If the Csv control character is not one character only. - * - * @return string - */ - protected function filterControl(string $char, string $type, string $caller): string - { - if (1 == strlen($char)) { - return $char; - } - - throw new Exception(sprintf('%s() expects %s to be a single character %s given', $caller, $type, $char)); - } - - /** - * Filter Nullable Integer with mininal range check - * - * @see https://wiki.php.net/rfc/nullable_types - * - * @param int|null $value - * @param int $min_range - * @param string $error_message - * - * @throws TypError if value is not a integer or null - * @throws Exception if value is not in a valid int range - */ - protected function filterNullableInteger($value, int $min_range, string $error_message) - { - if (null === $value) { - return; - } - - if (!is_int($value)) { - throw new TypeError(sprintf('Expected argument to be null or a integer %s given', gettype($value))); - } - - if ($value < $min_range) { - throw new Exception($error_message); - } - } -} diff --git a/src/Writer.php b/src/Writer.php index 58befd5a..ae5c5f0d 100644 --- a/src/Writer.php +++ b/src/Writer.php @@ -241,17 +241,22 @@ public function setNewline(string $newline): self */ public function setFlushThreshold($threshold): self { - $this->filterNullableInteger($threshold, 1, __METHOD__.'() expects 1 Argument to be null or a valid integer greater or equal to 1'); if ($threshold === $this->flush_threshold) { return $this; } - $this->flush_threshold = $threshold; - if (0 < $this->flush_counter) { - $this->flush_counter = 0; - $this->document->fflush(); + if (!is_nullable_int($threshold)) { + throw new TypeError(sprintf(__METHOD__.'() expects 1 Argument to be null or an integer %s given', gettype($threshold))); + } + + if (null !== $threshold && 1 >= $threshold) { + throw new Exception(__METHOD__.'() expects 1 Argument to be null or a valid integer greater or equal to 1'); } + $this->flush_threshold = $threshold; + $this->flush_counter = 0; + $this->document->fflush(); + return $this; } } diff --git a/src/functions.php b/src/functions.php index ab17bbfd..a50cb96e 100644 --- a/src/functions.php +++ b/src/functions.php @@ -78,11 +78,11 @@ function delimiter_detect(Reader $csv, array $delimiters, int $limit = 1): array if (!function_exists('\is_iterable')) { /** - * Tell whether the contents of the variable is iterable + * Tell whether the content of the variable is iterable * * @see http://php.net/manual/en/function.is-iterable.php * - * @param array|Traversable $iterable + * @param mixed $iterable * * @return bool */ @@ -91,3 +91,17 @@ function is_iterable($iterable): bool return is_array($iterable) || $iterable instanceof Traversable; } } + +/** + * Tell whether the content of the variable is an int or null + * + * @see https://wiki.php.net/rfc/nullable_types + * + * @param mixed $value + * + * @return bool + */ +function is_nullable_int($value): bool +{ + return null === $value || is_int($value); +} diff --git a/tests/CsvTest.php b/tests/CsvTest.php index 5e0c8660..8f144370 100644 --- a/tests/CsvTest.php +++ b/tests/CsvTest.php @@ -194,13 +194,13 @@ public function testStreamFilterMode() /** * @covers ::getDelimiter * @covers ::setDelimiter - * @covers ::filterControl */ public function testDelimeter() { $this->expectException(Exception::class); $this->csv->setDelimiter('o'); $this->assertSame('o', $this->csv->getDelimiter()); + $this->assertSame($this->csv, $this->csv->setDelimiter('o')); $this->csv->setDelimiter('foo'); } @@ -251,6 +251,7 @@ public function testEscape() $this->expectException(Exception::class); $this->csv->setEscape('o'); $this->assertSame('o', $this->csv->getEscape()); + $this->assertSame($this->csv, $this->csv->setEscape('o')); $this->csv->setEscape('foo'); } @@ -264,6 +265,7 @@ public function testEnclosure() $this->expectException(Exception::class); $this->csv->setEnclosure('o'); $this->assertSame('o', $this->csv->getEnclosure()); + $this->assertSame($this->csv, $this->csv->setEnclosure('o')); $this->csv->setEnclosure('foo'); } diff --git a/tests/ReaderTest.php b/tests/ReaderTest.php index dabac958..090650e8 100644 --- a/tests/ReaderTest.php +++ b/tests/ReaderTest.php @@ -298,8 +298,8 @@ public function testGetHeaderThrowsException() } /** - * @covers ::setHeader - * @covers ::filterNullableInteger + * @covers ::setHeaderOffset + * @covers \League\Csv\is_nullable_int */ public function testSetHeaderThrowsExceptionOnWrongInput() { @@ -308,8 +308,7 @@ public function testSetHeaderThrowsExceptionOnWrongInput() } /** - * @covers ::setHeader - * @covers ::filterNullableInteger + * @covers ::setHeaderOffset */ public function testSetHeaderThrowsExceptionOnWrongInputRange() { diff --git a/tests/StreamTest.php b/tests/StreamTest.php index ef75918c..41a51b39 100644 --- a/tests/StreamTest.php +++ b/tests/StreamTest.php @@ -88,6 +88,8 @@ public function testCreateStreamFromPathWithContext() /** * @covers ::fputcsv + * @covers ::filterControl + * * @dataProvider fputcsvProvider * * @param string $delimiter @@ -140,4 +142,20 @@ public function testSeek() $doc->seek(1); $this->assertSame(['Aaron', '55', 'M', '2004'], $doc->current()); } + + /** + * @covers ::setCsvControl + * @covers ::getCsvControl + * @covers ::filterControl + */ + public function testCsvControl() + { + $doc = Stream::createFromString('foo,bar'); + $this->assertSame([',', '"', '\\'], $doc->getCsvControl()); + $expected = [';', '|', '"']; + $doc->setCsvControl(...$expected); + $this->assertSame($expected, $doc->getCsvControl()); + $this->expectException(Exception::class); + $doc->setCsvControl(...['foo']); + } } diff --git a/tests/WriterTest.php b/tests/WriterTest.php index f07d526c..d46fa002 100644 --- a/tests/WriterTest.php +++ b/tests/WriterTest.php @@ -37,7 +37,6 @@ public function tearDown() /** * @covers ::getFlushThreshold * @covers ::setFlushThreshold - * @covers ::filterNullableInteger */ public function testflushThreshold() { @@ -50,7 +49,7 @@ public function testflushThreshold() /** * @covers ::setFlushThreshold - * @covers ::filterNullableInteger + * @covers \League\Csv\is_nullable_int */ public function testflushThresholdThrowsTypeError() {