Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Yaml] properly count skipped comment lines #19028

Merged
merged 1 commit into from
Jun 12, 2016
Merged

[Yaml] properly count skipped comment lines #19028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
107 changes: 47 additions & 60 deletions src/Symfony/Component/Yaml/Parser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -101,25 +105,18 @@ public function parse($value, $exceptionOnInvalidType = false, $objectSupport =

// 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<key>'.Inline::REGEX_QUOTED_STRING.'|[^ \'"\{\[].*?) *\:(\s+(?P<value>.+?))?\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);
}
Expand Down Expand Up @@ -175,10 +172,7 @@ public function parse($value, $exceptionOnInvalidType = false, $objectSupport =
} 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);
Expand Down Expand Up @@ -226,10 +220,7 @@ public function parse($value, $exceptionOnInvalidType = false, $objectSupport =
$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])) {
Expand Down Expand Up @@ -317,14 +308,42 @@ public function parse($value, $exceptionOnInvalidType = false, $objectSupport =
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).
*
* @return int The current line number
*/
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;
}

/**
Expand Down Expand Up @@ -426,7 +445,15 @@ private function getNextEmbedBlock($indentation = null, $inSequence = false)
}

// 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;
}

Expand Down Expand Up @@ -786,44 +813,4 @@ private function isBlockScalarHeader()
{
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;
}
}
19 changes: 19 additions & 0 deletions src/Symfony/Component/Yaml/Tests/ParserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,25 @@ public function testSequenceInMappingStartedBySingleDashLine()
$this->assertSame($expected, $this->parser->parse($yaml));
}

public function testSequenceFollowedByCommentEmbeddedInMapping()
{
$yaml = <<<EOT
a:
b:
- c
# comment
d: e
EOT;
$expected = array(
'a' => array(
'b' => array('c'),
'd' => 'e',
),
);

$this->assertSame($expected, $this->parser->parse($yaml));
}

/**
* @expectedException \Symfony\Component\Yaml\Exception\ParseException
*/
Expand Down