CM-873 May detect linebreak incorrectly in edge case

.gitattributes

@@ -0,0 +1 @@
+*.clrf.csv eol=crlf

composer.json

@@ -29,11 +29,11 @@
     },
     "require-dev": {
         "ext-json": "*",
-        "keboola/coding-standard": "^13.0",
+        "keboola/coding-standard": "^15.0",
         "php-parallel-lint/php-parallel-lint": "^1.3",
-        "phpstan/phpstan": "^1.4",
+        "phpstan/phpstan": "^1.10",
         "phpunit/phpunit": ">=7.5 <=9.6",
-        "phpstan/phpdoc-parser": "1.5.*"
+        "phpstan/phpdoc-parser": "^1.25"
     },
     "scripts": {
         "phpstan": "phpstan analyse ./src ./tests --level=max --no-progress -c phpstan.neon",

docker-compose.yml

@@ -2,9 +2,13 @@ version: "3"
 
 services:
 
-  dev:
+  dev: &dev
     build: .
     tty: true
     volumes:
       - ./:/code
-    working_dir: /code
+    working_dir: /code
+  dev-xdebug:
+    <<: *dev
+    build:
+      context: docker/xdebug

docker/xdebug/Dockerfile

@@ -0,0 +1,4 @@
+FROM php-csv-dev
+
+RUN pecl install xdebug-2.9.8 \
+  && docker-php-ext-enable xdebug

phpcs.xml

@@ -21,4 +21,7 @@
     <rule ref="SlevomatCodingStandard.TypeHints.DeclareStrictTypes">
         <exclude name="SlevomatCodingStandard.TypeHints.DeclareStrictTypes.DeclareStrictTypesMissing"/>
     </rule>
-</ruleset>
+    <rule ref="SlevomatCodingStandard.Functions.RequireTrailingCommaInDeclaration">
+        <exclude name="SlevomatCodingStandard.Functions.RequireTrailingCommaInDeclaration.MissingTrailingComma"/>
+    </rule>
+</ruleset>

phpstan-baseline-8+.neon

@@ -275,6 +275,11 @@ parameters:
 			count: 1
 			path: tests/CsvReadTest.php
 
+		-
+			message: "#^Method Keboola\\\\Csv\\\\Tests\\\\CsvReadTest\\:\\:testNewlineDetectionEdgecaseWithCrLf\\(\\) has no return type specified\\.$#"
+			count: 1
+			path: tests/CsvReadTest.php
+
 		-
 			message: "#^Method Keboola\\\\Csv\\\\Tests\\\\CsvReadTest\\:\\:testParse\\(\\) has no return type specified\\.$#"
 			count: 1

phpstan-baseline.neon

@@ -280,6 +280,11 @@ parameters:
 			count: 1
 			path: tests/CsvReadTest.php
 
+		-
+			message: "#^Method Keboola\\\\Csv\\\\Tests\\\\CsvReadTest\\:\\:testNewlineDetectionEdgecaseWithCrLf\\(\\) has no return type specified\\.$#"
+			count: 1
+			path: tests/CsvReadTest.php
+
 		-
 			message: "#^Method Keboola\\\\Csv\\\\Tests\\\\CsvReadTest\\:\\:testParse\\(\\) has no return type specified\\.$#"
 			count: 1

src/CsvOptions.php

@@ -50,7 +50,7 @@ protected function validateEnclosure($enclosure)
         if (strlen($enclosure) > 1) {
             throw new InvalidArgumentException(
                 'Enclosure must be a single character. ' . json_encode($enclosure) . ' received',
-                Exception::INVALID_PARAM
+                Exception::INVALID_PARAM,
             );
         }
     }
@@ -64,14 +64,14 @@ protected function validateDelimiter($delimiter)
         if (strlen($delimiter) > 1) {
             throw new InvalidArgumentException(
                 'Delimiter must be a single character. ' . json_encode($delimiter) . ' received',
-                Exception::INVALID_PARAM
+                Exception::INVALID_PARAM,
             );
         }
 
         if (strlen($delimiter) === 0) {
             throw new InvalidArgumentException(
                 'Delimiter cannot be empty.',
-                Exception::INVALID_PARAM
+                Exception::INVALID_PARAM,
             );
         }
     }

src/CsvReader.php

@@ -13,6 +13,7 @@ class CsvReader extends AbstractCsvFile implements Iterator
      * @deprecated use Keboola\Csv\CsvOptions::DEFAULT_ENCLOSURE
      */
     const DEFAULT_ESCAPED_BY = CsvOptions::DEFAULT_ESCAPED_BY;
+    const SAMPLE_SIZE = 10000;
 
     /**
      * @var int
@@ -92,7 +93,7 @@ protected function validateSkipLines($skipLines)
         if (!is_int($skipLines) || $skipLines < 0) {
             throw new InvalidArgumentException(
                 "Number of lines to skip must be a positive integer. \"$skipLines\" received.",
-                Exception::INVALID_PARAM
+                Exception::INVALID_PARAM,
             );
         }
     }
@@ -106,14 +107,14 @@ protected function openCsvFile($fileName)
         if (!is_file($fileName)) {
             throw new Exception(
                 'Cannot open file ' . $fileName,
-                Exception::FILE_NOT_EXISTS
+                Exception::FILE_NOT_EXISTS,
             );
         }
         $this->filePointer = @fopen($fileName, 'r');
         if (!$this->filePointer) {
             throw new Exception(
                 "Cannot open file {$fileName} " . error_get_last()['message'],
-                Exception::FILE_NOT_EXISTS
+                Exception::FILE_NOT_EXISTS,
             );
         }
     }
@@ -124,7 +125,12 @@ protected function openCsvFile($fileName)
     protected function detectLineBreak()
     {
         @rewind($this->getFilePointer());
-        $sample = @fread($this->getFilePointer(), 10000);
+        $sample = @fread($this->getFilePointer(), self::SAMPLE_SIZE);
+        if (substr((string) $sample, -1) === "\r") {
+            // we might have hit the file in the middle of CR+LF, only getting CR
+            @rewind($this->getFilePointer());
+            $sample = @fread($this->getFilePointer(), self::SAMPLE_SIZE+1);
+        }
 
         return LineBreaksHelper::detectLineBreaks($sample, $this->getEnclosure(), $this->getEscapedBy());
     }
@@ -155,7 +161,7 @@ protected function validateLineBreak()
 
         throw new InvalidArgumentException(
             "Invalid line break. Please use unix \\n or win \\r\\n line breaks.",
-            Exception::INVALID_PARAM
+            Exception::INVALID_PARAM,
         );
     }
 

src/CsvWriter.php

@@ -55,7 +55,7 @@ private function validateLineBreak($lineBreak)
             throw new Exception(
                 'Invalid line break: ' . json_encode($lineBreak) .
                 ' allowed line breaks: ' . json_encode($allowedLineBreaks),
-                Exception::INVALID_PARAM
+                Exception::INVALID_PARAM,
             );
         }
     }
@@ -72,14 +72,14 @@ protected function openCsvFile($fileName)
             throw new Exception(
                 "Cannot open file {$fileName} " . $e->getMessage(),
                 Exception::FILE_NOT_EXISTS,
-                $e
+                $e,
             );
         }
 
         if (!$this->filePointer) {
             throw new Exception(
                 "Cannot open file {$fileName} " . error_get_last()['message'],
-                Exception::FILE_NOT_EXISTS
+                Exception::FILE_NOT_EXISTS,
             );
         }
     }
@@ -100,7 +100,7 @@ public function writeRow(array $row)
                 ' Return: false' .
                 ' To write: ' . strlen($str) . ' Written: 0',
                 Exception::WRITE_ERROR,
-                $e
+                $e,
             );
         }
 
@@ -114,7 +114,7 @@ public function writeRow(array $row)
                 ($ret === false && error_get_last() ? 'Error: ' . error_get_last()['message'] : '') .
                 ' Return: ' . json_encode($ret) .
                 ' To write: ' . strlen($str) . ' Written: ' . (int) $ret,
-                Exception::WRITE_ERROR
+                Exception::WRITE_ERROR,
             );
         }
     }
@@ -138,7 +138,7 @@ public function rowToStr(array $row)
             )) {
                 throw new Exception(
                     'Cannot write data into column: ' . var_export($column, true),
-                    Exception::WRITE_ERROR
+                    Exception::WRITE_ERROR,
                 );
             }
 

tests/CsvReadTest.php

@@ -31,6 +31,13 @@ public function testColumnsCount()
         self::assertEquals(9, $csv->getColumnsCount());
     }
 
+    public function testNewlineDetectionEdgecaseWithCrLf()
+    {
+        $this->expectNotToPerformAssertions();
+        // this used to throw "Invalid line break. Please use unix \n or win \r\n line breaks." before the fix
+        new CsvReader(__DIR__ . '/data/test-input-edgecase.crlf.csv');
+    }
+
     /**
      * @dataProvider validCsvFiles
      * @param string $fileName
@@ -266,7 +273,7 @@ public function testSkipsHeaders()
             CsvOptions::DEFAULT_DELIMITER,
             CsvOptions::DEFAULT_ENCLOSURE,
             CsvOptions::DEFAULT_ESCAPED_BY,
-            1
+            1,
         );
         self::assertEquals(['id', 'isImported'], $csvFile->getHeader());
         self::assertEquals([
@@ -285,7 +292,7 @@ public function testSkipNoLines()
             CsvOptions::DEFAULT_DELIMITER,
             CsvOptions::DEFAULT_ENCLOSURE,
             CsvOptions::DEFAULT_ESCAPED_BY,
-            0
+            0,
         );
         self::assertEquals(['id', 'isImported'], $csvFile->getHeader());
         self::assertEquals([
@@ -305,7 +312,7 @@ public function testSkipsMultipleLines()
             CsvOptions::DEFAULT_DELIMITER,
             CsvOptions::DEFAULT_ENCLOSURE,
             CsvOptions::DEFAULT_ESCAPED_BY,
-            3
+            3,
         );
         self::assertEquals(['id', 'isImported'], $csvFile->getHeader());
         self::assertEquals([
@@ -322,7 +329,7 @@ public function testSkipsOverflow()
             CsvOptions::DEFAULT_DELIMITER,
             CsvOptions::DEFAULT_ENCLOSURE,
             CsvOptions::DEFAULT_ESCAPED_BY,
-            100
+            100,
         );
         self::assertEquals(['id', 'isImported'], $csvFile->getHeader());
         self::assertEquals([], iterator_to_array($csvFile));
@@ -396,7 +403,7 @@ public function testInvalidSkipLines($skipLines, $message)
             CsvOptions::DEFAULT_DELIMITER,
             CsvOptions::DEFAULT_ENCLOSURE,
             CsvOptions::DEFAULT_ENCLOSURE,
-            $skipLines
+            $skipLines,
         );
     }
 
@@ -475,9 +482,9 @@ public function testWriteReadInTheMiddle()
                     '"1","first"',
                     '"2","second"',
                     '',
-                ]
+                ],
             ),
-            $data
+            $data,
         );
     }
 
@@ -525,7 +532,7 @@ public function testInvalidFile()
     public function testPerformance($fileContent, $expectedRows, $maxDuration)
     {
         self::markTestSkipped(
-            'Run this test only manually. Because the duration is very different in local CI environment.'
+            'Run this test only manually. Because the duration is very different in local CI environment.',
         );
 
         try {

tests/CsvWriteTest.php

@@ -91,9 +91,9 @@ public function testWrite()
                     '"true","1.123"',
                     '"1","null"',
                     '',
-                ]
+                ],
             ),
-            $data
+            $data,
         );
     }
 
@@ -150,9 +150,9 @@ public function testWriteValidObject()
                     '"col1","col2"' ,
                     '"1","me string"',
                     '',
-                ]
+                ],
             ),
-            $data
+            $data,
         );
     }
 
@@ -211,9 +211,9 @@ public function testWritePointer()
                 [
                     '"col1","col2"' ,
                     'foo,bar',
-                ]
+                ],
             ),
-            $data
+            $data,
         );
     }
 
@@ -233,15 +233,15 @@ public function testInvalidPointer()
             $or = new LogicalOr();
             $or->setConstraints([
                 new StringContains(
-                    'Cannot write to CSV file  Return: 0 To write: 14 Written: 0'
+                    'Cannot write to CSV file  Return: 0 To write: 14 Written: 0',
                 ),
                 new StringContains(
                     'Cannot write to CSV file Error: fwrite(): ' .
-                    'write of 14 bytes failed with errno=9 Bad file descriptor Return: false To write: 14 Written: 0'
+                    'write of 14 bytes failed with errno=9 Bad file descriptor Return: false To write: 14 Written: 0',
                 ),
                 new StringContains(
                     'Cannot write to CSV file Error: fwrite(): ' .
-                    'Write of 14 bytes failed with errno=9 Bad file descriptor Return: false To write: 14 Written: 0'
+                    'Write of 14 bytes failed with errno=9 Bad file descriptor Return: false To write: 14 Written: 0',
                 ),
             ]);
             self::assertThat($e->getMessage(), $or);
@@ -258,7 +258,7 @@ public function testInvalidPointer2()
         $rows = [['col1', 'col2']];
         self::expectException(Exception::class);
         self::expectExceptionMessage(
-            'a valid stream resource Return: false To write: 14 Written: '
+            'a valid stream resource Return: false To write: 14 Written: ',
         );
         $csvFile->writeRow($rows[0]);
     }
@@ -278,7 +278,7 @@ public function testWriteLineBreak()
             $fileName,
             CsvOptions::DEFAULT_DELIMITER,
             CsvOptions::DEFAULT_ENCLOSURE,
-            "\r\n"
+            "\r\n",
         );
         $rows = [
             [
@@ -300,9 +300,9 @@ public function testWriteLineBreak()
                     '"col1","col2"',
                     '"val1","val2"',
                     '',
-                ]
+                ],
             ),
-            $data
+            $data,
         );
     }
 }

tests/LineBreaksHelperTest.php

@@ -30,7 +30,7 @@ public function testLineBreaksDetection($enclosure, $escapedBy, $input, $expecte
         // Test line breaks detection
         self::assertSame(
             json_encode($expectedLineBreak),
-            json_encode(LineBreaksHelper::detectLineBreaks($input, $enclosure, $escapedBy))
+            json_encode(LineBreaksHelper::detectLineBreaks($input, $enclosure, $escapedBy)),
         );
     }