From da7fc36a434e3e23ed28c6692cfbc6bbe2c42f1b Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Fri, 10 Jun 2016 23:57:14 +0200 Subject: [PATCH] [Yaml] properly count skipped comment lines --- src/Symfony/Component/Yaml/Parser.php | 107 ++++++++---------- .../Component/Yaml/Tests/ParserTest.php | 19 ++++ 2 files changed, 66 insertions(+), 60 deletions(-) diff --git a/src/Symfony/Component/Yaml/Parser.php b/src/Symfony/Component/Yaml/Parser.php index e134a4be99..dcc9a0150e 100644 --- a/src/Symfony/Component/Yaml/Parser.php +++ b/src/Symfony/Component/Yaml/Parser.php @@ -30,17 +30,21 @@ class Parser private $currentLineNb = -1; private $currentLine = ''; private $refs = array(); + private $skippedLineNumbers = array(); + private $locallySkippedLineNumbers = array(); /** * Constructor. * * @param int $offset The offset of YAML document (used for line numbers in error messages) * @param int|null $totalNumberOfLines The overall number of lines being parsed + * @param int[] $skippedLineNumbers Number of comment lines that have been skipped by the parser */ - public function __construct($offset = 0, $totalNumberOfLines = null) + public function __construct($offset = 0, $totalNumberOfLines = null, array $skippedLineNumbers = array()) { $this->offset = $offset; $this->totalNumberOfLines = $totalNumberOfLines; + $this->skippedLineNumbers = $skippedLineNumbers; } /** @@ -101,25 +105,18 @@ class Parser // array if (!isset($values['value']) || '' == trim($values['value'], ' ') || 0 === strpos(ltrim($values['value'], ' '), '#')) { - $c = $this->getRealCurrentLineNb() + 1; - $parser = new self($c, $this->totalNumberOfLines); - $parser->refs = &$this->refs; - $data[] = $parser->parse($this->getNextEmbedBlock(null, true), $exceptionOnInvalidType, $objectSupport, $objectForMap); + $data[] = $this->parseBlock($this->getRealCurrentLineNb() + 1, $this->getNextEmbedBlock(null, true), $exceptionOnInvalidType, $objectSupport, $objectForMap); } else { if (isset($values['leadspaces']) && preg_match('#^(?P'.Inline::REGEX_QUOTED_STRING.'|[^ \'"\{\[].*?) *\:(\s+(?P.+?))?\s*$#u', $values['value'], $matches) ) { // this is a compact notation element, add to next block and parse - $c = $this->getRealCurrentLineNb(); - $parser = new self($c, $this->totalNumberOfLines); - $parser->refs = &$this->refs; - $block = $values['value']; if ($this->isNextLineIndented()) { $block .= "\n".$this->getNextEmbedBlock($this->getCurrentLineIndentation() + strlen($values['leadspaces']) + 1); } - $data[] = $parser->parse($block, $exceptionOnInvalidType, $objectSupport, $objectForMap); + $data[] = $this->parseBlock($this->getRealCurrentLineNb(), $block, $exceptionOnInvalidType, $objectSupport, $objectForMap); } else { $data[] = $this->parseValue($values['value'], $exceptionOnInvalidType, $objectSupport, $objectForMap); } @@ -175,10 +172,7 @@ class Parser } else { $value = $this->getNextEmbedBlock(); } - $c = $this->getRealCurrentLineNb() + 1; - $parser = new self($c, $this->totalNumberOfLines); - $parser->refs = &$this->refs; - $parsed = $parser->parse($value, $exceptionOnInvalidType, $objectSupport, $objectForMap); + $parsed = $this->parseBlock($this->getRealCurrentLineNb() + 1, $value, $exceptionOnInvalidType, $objectSupport, $objectForMap); if (!is_array($parsed)) { throw new ParseException('YAML merge keys used with a scalar value instead of an array.', $this->getRealCurrentLineNb() + 1, $this->currentLine); @@ -226,10 +220,7 @@ class Parser $data[$key] = null; } } else { - $c = $this->getRealCurrentLineNb() + 1; - $parser = new self($c, $this->totalNumberOfLines); - $parser->refs = &$this->refs; - $value = $parser->parse($this->getNextEmbedBlock(), $exceptionOnInvalidType, $objectSupport, $objectForMap); + $value = $this->parseBlock($this->getRealCurrentLineNb() + 1, $this->getNextEmbedBlock(), $exceptionOnInvalidType, $objectSupport, $objectForMap); // Spec: Keys MUST be unique; first one wins. // But overwriting is allowed when a merge node is used in current block. if ($allowOverwrite || !isset($data[$key])) { @@ -317,6 +308,24 @@ class Parser return empty($data) ? null : $data; } + private function parseBlock($offset, $yaml, $exceptionOnInvalidType, $objectSupport, $objectForMap) + { + $skippedLineNumbers = $this->skippedLineNumbers; + + foreach ($this->locallySkippedLineNumbers as $lineNumber) { + if ($lineNumber < $offset) { + continue; + } + + $skippedLineNumbers[] = $lineNumber; + } + + $parser = new self($offset, $this->totalNumberOfLines, $skippedLineNumbers); + $parser->refs = &$this->refs; + + return $parser->parse($yaml, $exceptionOnInvalidType, $objectSupport, $objectForMap); + } + /** * Returns the current line number (takes the offset into account). * @@ -324,7 +333,17 @@ class Parser */ private function getRealCurrentLineNb() { - return $this->currentLineNb + $this->offset; + $realCurrentLineNumber = $this->currentLineNb + $this->offset; + + foreach ($this->skippedLineNumbers as $skippedLineNumber) { + if ($skippedLineNumber > $realCurrentLineNumber) { + break; + } + + ++$realCurrentLineNumber; + } + + return $realCurrentLineNumber; } /** @@ -426,7 +445,15 @@ class Parser } // we ignore "comment" lines only when we are not inside a scalar block - if (empty($blockScalarIndentations) && $this->isCurrentLineComment() && false === $this->checkIfPreviousNonCommentLineIsCollectionItem()) { + if (empty($blockScalarIndentations) && $this->isCurrentLineComment()) { + // remember ignored comment lines (they are used later in nested + // parser calls to determine real line numbers) + // + // CAUTION: beware to not populate the global property here as it + // will otherwise influence the getRealCurrentLineNb() call here + // for consecutive comment lines and subsequent embedded blocks + $this->locallySkippedLineNumbers[] = $this->getRealCurrentLineNb(); + continue; } @@ -786,44 +813,4 @@ class Parser { return (bool) preg_match('~'.self::BLOCK_SCALAR_HEADER_PATTERN.'$~', $this->currentLine); } - - /** - * Returns true if the current line is a collection item. - * - * @return bool - */ - private function isCurrentLineCollectionItem() - { - $ltrimmedLine = ltrim($this->currentLine, ' '); - - return '' !== $ltrimmedLine && '-' === $ltrimmedLine[0]; - } - - /** - * Tests whether the current comment line is in a collection. - * - * @return bool - */ - private function checkIfPreviousNonCommentLineIsCollectionItem() - { - $isCollectionItem = false; - $moves = 0; - while ($this->moveToPreviousLine()) { - ++$moves; - // If previous line is a comment, move back again. - if ($this->isCurrentLineComment()) { - continue; - } - $isCollectionItem = $this->isCurrentLineCollectionItem(); - break; - } - - // Move parser back to previous line. - while ($moves > 0) { - $this->moveToNextLine(); - --$moves; - } - - return $isCollectionItem; - } } diff --git a/src/Symfony/Component/Yaml/Tests/ParserTest.php b/src/Symfony/Component/Yaml/Tests/ParserTest.php index 149dd2b073..1b6193a692 100644 --- a/src/Symfony/Component/Yaml/Tests/ParserTest.php +++ b/src/Symfony/Component/Yaml/Tests/ParserTest.php @@ -607,6 +607,25 @@ EOT; $this->assertSame($expected, $this->parser->parse($yaml)); } + public function testSequenceFollowedByCommentEmbeddedInMapping() + { + $yaml = << array( + 'b' => array('c'), + 'd' => 'e', + ), + ); + + $this->assertSame($expected, $this->parser->parse($yaml)); + } + /** * @expectedException \Symfony\Component\Yaml\Exception\ParseException */