From ee7e823a216950b5fea7b3232007a8e9b302cf22 Mon Sep 17 00:00:00 2001 From: jon-ht Date: Fri, 15 May 2020 18:41:14 +0200 Subject: [PATCH 1/2] Add test case for multiline string --- src/Util/YamlSourceManipulator.php | 67 +++++++++++++++++-- tests/Util/YamlSourceManipulatorTest.php | 3 +- .../Util/yaml_fixtures/multiline_string.test | 16 +++++ .../multiline_string_before_property.test | 18 +++++ .../multiline_string_remove_line.test | 18 +++++ .../multiline_string_with_blank_line.test | 20 ++++++ .../multiline_string_without_update.test | 15 +++++ 7 files changed, 150 insertions(+), 7 deletions(-) create mode 100644 tests/Util/yaml_fixtures/multiline_string.test create mode 100644 tests/Util/yaml_fixtures/multiline_string_before_property.test create mode 100644 tests/Util/yaml_fixtures/multiline_string_remove_line.test create mode 100644 tests/Util/yaml_fixtures/multiline_string_with_blank_line.test create mode 100644 tests/Util/yaml_fixtures/multiline_string_without_update.test diff --git a/src/Util/YamlSourceManipulator.php b/src/Util/YamlSourceManipulator.php index 267111a51..8a40ef2e2 100644 --- a/src/Util/YamlSourceManipulator.php +++ b/src/Util/YamlSourceManipulator.php @@ -216,6 +216,14 @@ private function updateData(array $newData) continue; } + if ( + $this->isMultilineString($newVal) + && rtrim($currentVal, "\n") !== $currentVal + && rtrim($newVal, "\n") === $newVal + ) { + $newVal .= "\n"; + } + // 3b) value DID change $this->log(sprintf('updating value to {%s}', \is_array($newVal) ? '' : $newVal)); $this->changeValueInYaml($newVal); @@ -449,8 +457,15 @@ private function changeValueInYaml($value) $endValuePosition = $this->findEndPositionOfValue($originalVal); - $newYamlValue = $this->convertToYaml($value); - if (!\is_array($originalVal) && \is_array($value)) { + $isMultilineValue = null !== $this->findPositionOfMultilineCharInLine($this->currentPosition); + $originalValEndsWithNewLine = $isMultilineValue ? rtrim($originalVal, "\n") !== $originalVal : false; + + // In case of multiline, $value is converted as plain string like "Foo\nBar" + // We need to keep it "as is" + $newYamlValue = $isMultilineValue ? rtrim($value, "\n") : $this->convertToYaml($value); + if ((!\is_array($originalVal) && \is_array($value)) || + ($this->isMultilineString($originalVal) && $this->isMultilineString($value)) + ) { // we're converting from a scalar to a (multiline) array // this means we need to break onto the next line @@ -468,8 +483,14 @@ private function changeValueInYaml($value) ++$newPosition; } + if ($isMultilineValue) { + // strlen(" |") + $newPosition -= 2; + } + $newContents = substr($this->contents, 0, $this->currentPosition) - .$newYamlValue + .($isMultilineValue ? ' |' : '') + .($originalValEndsWithNewLine ? rtrim($newYamlValue, ' ') : $newYamlValue) /* * If the next line is a comment, this means we probably had * a structure that looks like this: @@ -771,16 +792,26 @@ private function findEndPositionOfValue($value, $offset = null) } if (is_scalar($value) || null === $value) { + $offset = null === $offset ? $this->currentPosition : $offset; + if (\is_bool($value)) { // (?i) & (?-i) opens/closes case insensitive match $pattern = sprintf('(?i)%s(?-i)', $value ? 'true' : 'false'); } elseif (null === $value) { $pattern = '(~|NULL|null|\n)'; } else { - $pattern = sprintf('\'?"?%s\'?"?', preg_quote($value, '#')); - } + // Multiline value ends with \n. + // If we remove this character, the next property will ne merged with this value + $quotedValue = preg_quote(rtrim($value, "\n"), '#'); + $patternValue = $quotedValue; + + // Iterates until we find a new line char or we reach end of file + if ($this->findPositionOfMultilineCharInLine($offset)) { + $patternValue = str_replace(["\r\n", "\n"], '\r?\n\s*', $quotedValue); + } - $offset = null === $offset ? $this->currentPosition : $offset; + $pattern = sprintf('\'?"?%s\'?"?', $patternValue); + } // a value like "foo:" can simply end a file // this means the value is null @@ -1164,7 +1195,31 @@ private function indentMultilineYamlArray(string $yaml): string // also need to be indented artificially by the same amount $yaml = str_replace("\n", "\n".$this->getCurrentIndentation(), $yaml); + if ($this->isMultilineString($yaml)) { + // Remove extra indentation in case of blank line in multiline string + $yaml = str_replace("\n".$this->getCurrentIndentation()."\n", "\n\n", $yaml); + } + // now indent this level return $this->getCurrentIndentation().$yaml; } + + private function findPositionOfMultilineCharInLine(int $position): ?int + { + $cursor = $position; + while (!$this->isCharLineBreak($currentChar = substr($this->contents, $cursor + 1, 1)) && !$this->isEOF($cursor)) { + if ('|' === $currentChar) { + return $cursor; + } + + ++$cursor; + } + + return null; + } + + private function isMultilineString($value): bool + { + return \is_string($value) && false !== strpos($value, "\n"); + } } diff --git a/tests/Util/YamlSourceManipulatorTest.php b/tests/Util/YamlSourceManipulatorTest.php index 5e01d6ce6..5e979ebec 100644 --- a/tests/Util/YamlSourceManipulatorTest.php +++ b/tests/Util/YamlSourceManipulatorTest.php @@ -58,7 +58,8 @@ private function getYamlDataTests() foreach ($finder as $file) { list($source, $changeCode, $expected) = explode('===', $file->getContents()); - $data = Yaml::parse($source); + // Multiline string ends with an \n + $data = Yaml::parse(trim($source, "\n")); eval($changeCode); yield $file->getFilename() => [ diff --git a/tests/Util/yaml_fixtures/multiline_string.test b/tests/Util/yaml_fixtures/multiline_string.test new file mode 100644 index 000000000..b070fe0c2 --- /dev/null +++ b/tests/Util/yaml_fixtures/multiline_string.test @@ -0,0 +1,16 @@ +nelmio_api_doc: + documentation: + info: + description: | + Multiline string + Second line +=== +$data['nelmio_api_doc']['documentation']['info']['description'] = "Multiline string\nSecond line\nThird line"; +=== +nelmio_api_doc: + documentation: + info: + description: | + Multiline string + Second line + Third line \ No newline at end of file diff --git a/tests/Util/yaml_fixtures/multiline_string_before_property.test b/tests/Util/yaml_fixtures/multiline_string_before_property.test new file mode 100644 index 000000000..d68ec6208 --- /dev/null +++ b/tests/Util/yaml_fixtures/multiline_string_before_property.test @@ -0,0 +1,18 @@ +nelmio_api_doc: + documentation: + info: + description: | + Multiline string + Second line + foo: bar +=== +$data['nelmio_api_doc']['documentation']['info']['description'] = "Multiline string\nSecond line\nThird line\n"; +=== +nelmio_api_doc: + documentation: + info: + description: | + Multiline string + Second line + Third line + foo: bar \ No newline at end of file diff --git a/tests/Util/yaml_fixtures/multiline_string_remove_line.test b/tests/Util/yaml_fixtures/multiline_string_remove_line.test new file mode 100644 index 000000000..72956ea7b --- /dev/null +++ b/tests/Util/yaml_fixtures/multiline_string_remove_line.test @@ -0,0 +1,18 @@ +nelmio_api_doc: + documentation: + info: + description: | + Multiline string + Second line + Third line + foo: bar +=== +$data['nelmio_api_doc']['documentation']['info']['description'] = "Multiline string\nSecond line\n"; +=== +nelmio_api_doc: + documentation: + info: + description: | + Multiline string + Second line + foo: bar \ No newline at end of file diff --git a/tests/Util/yaml_fixtures/multiline_string_with_blank_line.test b/tests/Util/yaml_fixtures/multiline_string_with_blank_line.test new file mode 100644 index 000000000..c92296a31 --- /dev/null +++ b/tests/Util/yaml_fixtures/multiline_string_with_blank_line.test @@ -0,0 +1,20 @@ +nelmio_api_doc: + documentation: + info: + description: | + Multiline string + + Second line + foo: bar +=== +$data['nelmio_api_doc']['documentation']['info']['description'] = "Multiline string\n\nSecond line\nThird line\n"; +=== +nelmio_api_doc: + documentation: + info: + description: | + Multiline string + + Second line + Third line + foo: bar \ No newline at end of file diff --git a/tests/Util/yaml_fixtures/multiline_string_without_update.test b/tests/Util/yaml_fixtures/multiline_string_without_update.test new file mode 100644 index 000000000..c004d12c2 --- /dev/null +++ b/tests/Util/yaml_fixtures/multiline_string_without_update.test @@ -0,0 +1,15 @@ +nelmio_api_doc: + documentation: + info: + description: | + Multiline string + Second line +=== + +=== +nelmio_api_doc: + documentation: + info: + description: | + Multiline string + Second line \ No newline at end of file From 661acb001436932d4f555c4bcd001a01f5e06b88 Mon Sep 17 00:00:00 2001 From: jon-ht Date: Sun, 24 May 2020 13:47:41 +0200 Subject: [PATCH 2/2] remove unnecessary `\n` adding/removing --- src/Util/YamlSourceManipulator.php | 13 ++----------- tests/Util/YamlSourceManipulatorTest.php | 9 ++++++--- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/Util/YamlSourceManipulator.php b/src/Util/YamlSourceManipulator.php index 8a40ef2e2..882ba504d 100644 --- a/src/Util/YamlSourceManipulator.php +++ b/src/Util/YamlSourceManipulator.php @@ -216,14 +216,6 @@ private function updateData(array $newData) continue; } - if ( - $this->isMultilineString($newVal) - && rtrim($currentVal, "\n") !== $currentVal - && rtrim($newVal, "\n") === $newVal - ) { - $newVal .= "\n"; - } - // 3b) value DID change $this->log(sprintf('updating value to {%s}', \is_array($newVal) ? '' : $newVal)); $this->changeValueInYaml($newVal); @@ -458,7 +450,6 @@ private function changeValueInYaml($value) $endValuePosition = $this->findEndPositionOfValue($originalVal); $isMultilineValue = null !== $this->findPositionOfMultilineCharInLine($this->currentPosition); - $originalValEndsWithNewLine = $isMultilineValue ? rtrim($originalVal, "\n") !== $originalVal : false; // In case of multiline, $value is converted as plain string like "Foo\nBar" // We need to keep it "as is" @@ -490,7 +481,7 @@ private function changeValueInYaml($value) $newContents = substr($this->contents, 0, $this->currentPosition) .($isMultilineValue ? ' |' : '') - .($originalValEndsWithNewLine ? rtrim($newYamlValue, ' ') : $newYamlValue) + .$newYamlValue /* * If the next line is a comment, this means we probably had * a structure that looks like this: @@ -806,7 +797,7 @@ private function findEndPositionOfValue($value, $offset = null) $patternValue = $quotedValue; // Iterates until we find a new line char or we reach end of file - if ($this->findPositionOfMultilineCharInLine($offset)) { + if (null !== $this->findPositionOfMultilineCharInLine($offset)) { $patternValue = str_replace(["\r\n", "\n"], '\r?\n\s*', $quotedValue); } diff --git a/tests/Util/YamlSourceManipulatorTest.php b/tests/Util/YamlSourceManipulatorTest.php index 5e979ebec..45c6065cf 100644 --- a/tests/Util/YamlSourceManipulatorTest.php +++ b/tests/Util/YamlSourceManipulatorTest.php @@ -59,13 +59,16 @@ private function getYamlDataTests() list($source, $changeCode, $expected) = explode('===', $file->getContents()); // Multiline string ends with an \n - $data = Yaml::parse(trim($source, "\n")); + $source = rtrim($source, "\n"); + $expected = ltrim($expected, "\n"); + + $data = Yaml::parse($source); eval($changeCode); yield $file->getFilename() => [ - 'source' => rtrim($source, "\n"), + 'source' => $source, 'newData' => $data, - 'expectedSource' => ltrim($expected, "\n"), + 'expectedSource' => $expected, ]; }