diff --git a/src/Document/Emitter/Emitter.php b/src/Document/Emitter/Emitter.php index 76853db..3fd6981 100644 --- a/src/Document/Emitter/Emitter.php +++ b/src/Document/Emitter/Emitter.php @@ -175,7 +175,7 @@ private function emitScalar(ScalarNode $node): void { $properties = $this->renderValueProperties($node); if ($this->isBlockStyleScalar($node)) { - $raw = $node->getRawSource(); + $raw = $this->rawSource($node); if ($raw !== null) { $this->buffer .= $properties . $raw; } else { @@ -322,7 +322,7 @@ private function emitMapEntry(MapEntry $entry, int $indent): void // Block scalars span multiple lines and end with their own // newline. if ($this->isBlockStyleScalar($value)) { - $rawSource = $value->getRawSource(); + $rawSource = $this->rawSource($value); if ($rawSource !== null) { $this->buffer .= ' ' . $properties . $rawSource; } else { @@ -623,14 +623,42 @@ private function emitSequenceItem(SequenceItem $item, int $indent): void private function emitStandaloneComment(CommentNode $comment, int $indent): void { - // Use the comment's stored indent if it has one; otherwise - // the surrounding container's indent. - $useIndent = $comment->getIndent() > 0 ? $comment->getIndent() : $indent; + // Use the comment's own column when the parser recorded one, so a + // comment comes back where its author put it. null means no column was + // recorded - a comment built programmatically rather than read from a + // document - and those follow the surrounding container. + // + // Nullable rather than `> 0`: column 1 is a real position, and a + // flush-left comment inside an indented block is a deliberate style + // that was previously indistinguishable from "unset". + $useIndent = $comment->getIndent() ?? $indent; $this->buffer .= str_repeat(' ', $useIndent); $this->buffer .= $comment->getText(); $this->buffer .= $this->lineEnding; } + /** + * A node's raw source, in the line ending this document uses. + * + * Raw source is captured after the scanner has normalised CRLF to LF, so a + * multi-line scalar re-emitted verbatim carried LF into a CRLF document - + * a block scalar was the one construct that still broke a Windows file's + * round trip after the stream learned its line ending. + * + * A no-op for LF documents, which is every document until one says + * otherwise. + */ + private function rawSource(ScalarNode $node): ?string + { + $raw = $node->getRawSource(); + + if ($raw === null || $this->lineEnding === "\n") { + return $raw; + } + + return str_replace("\n", $this->lineEnding, $raw); + } + private function emitBlankLines(BlankLineNode $node): void { // count() blank lines = count() newline characters since the @@ -640,7 +668,7 @@ private function emitBlankLines(BlankLineNode $node): void private function scalarOrRaw(ScalarNode $node): string { - $rawSource = $node->getRawSource(); + $rawSource = $this->rawSource($node); if ($rawSource !== null) { return $rawSource; } @@ -817,7 +845,7 @@ private function emitBlockScalarSynthesized(ScalarNode $node, int $parentIndent) private function renderBlockScalar(ScalarNode $node, int $parentIndent): string { - $rawSource = $node->getRawSource(); + $rawSource = $this->rawSource($node); if ($rawSource !== null) { return $rawSource; } @@ -1003,10 +1031,23 @@ private function isPlainSafe(string $value, ?string $tag = null): bool return false; } $first = $value[0]; - $reservedLeaders = ['[', ']', '{', '}', '#', '&', '*', '!', '|', '>', "'", '"', '%', '@', '`', '?', ':', '-', ',', "\t", ' ']; + $reservedLeaders = ['[', ']', '{', '}', '#', '&', '*', '!', '|', '>', "'", '"', '%', '@', '`', ',', "\t", ' ']; if (in_array($first, $reservedLeaders, true)) { return false; } + // "?", ":" and "-" are indicators only when they stand alone or are + // followed by a space. YAML 1.2 ns-plain-first admits them otherwise, + // so "--cert-dir=/tmp" and "-single-dash" are valid plain scalars and + // quoting them rewrites a line the author did not write that way. + if (in_array($first, ['?', ':', '-'], true)) { + if (strlen($value) === 1) { + return false; + } + $second = $value[1]; + if ($second === ' ' || $second === "\t") { + return false; + } + } if (str_contains($value, ': ') || str_contains($value, " #")) { return false; } diff --git a/src/Document/Node/CommentNode.php b/src/Document/Node/CommentNode.php index 210e608..129fea7 100644 --- a/src/Document/Node/CommentNode.php +++ b/src/Document/Node/CommentNode.php @@ -34,7 +34,7 @@ final class CommentNode implements Node use NodeTrait; private string $text; - private int $indent; + private ?int $indent; private string $gap; /** @@ -50,7 +50,7 @@ final class CommentNode implements Node * Empty for synthesized nodes; the emitter falls * back to a sensible default when emitting. */ - public function __construct(string $text = '', int $indent = 0, string $gap = '') + public function __construct(string $text = '', ?int $indent = null, string $gap = '') { $this->text = $text; $this->indent = $indent; @@ -67,12 +67,12 @@ public function setText(string $text): void $this->text = $text; } - public function getIndent(): int + public function getIndent(): ?int { return $this->indent; } - public function setIndent(int $indent): void + public function setIndent(?int $indent): void { $this->indent = $indent; } diff --git a/src/Document/Parser/Parser.php b/src/Document/Parser/Parser.php index a481e2b..64b248b 100644 --- a/src/Document/Parser/Parser.php +++ b/src/Document/Parser/Parser.php @@ -589,7 +589,7 @@ private function migrateTrivia(array $trivia, MapNode|SequenceNode $container): { foreach ($trivia as $t) { if ($t->type === TriviaType::Comment) { - $node = new CommentNode($t->text, 0, $t->gap); + $node = new CommentNode($t->text, max(0, $t->column - 1), $t->gap); $node->setPosition($t->line, $t->column); $container->appendChildInternal($node); continue; @@ -652,7 +652,7 @@ private function drainTriviaIntoDoc(array $trivia, YamlDocument $doc): void private function triviaToNode(TriviaToken $t): CommentNode|BlankLineNode|null { if ($t->type === TriviaType::Comment) { - $node = new CommentNode($t->text, 0, $t->gap); + $node = new CommentNode($t->text, max(0, $t->column - 1), $t->gap); $node->setPosition($t->line, $t->column); return $node; } @@ -885,7 +885,7 @@ private function extractEolCommentFromTrailingTrivia(Token $token): ?CommentNode { foreach ($token->trailingTrivia as $t) { if ($t->type === TriviaType::Comment) { - $node = new CommentNode($t->text, 0, $t->gap); + $node = new CommentNode($t->text, max(0, $t->column - 1), $t->gap); $node->setPosition($t->line, $t->column); return $node; } @@ -955,7 +955,7 @@ private function parseBlockMapEntry(): MapEntry $first->type === TriviaType::Comment && $first->line === $valueToken->line ) { - $eolComment = new CommentNode($first->text, 0, $first->gap); + $eolComment = new CommentNode($first->text, max(0, $first->column - 1), $first->gap); $eolComment->setPosition($first->line, $first->column); // Strip this one trivia entry from the inner token so // it isn't re-emitted as a child of the nested value. diff --git a/src/Document/Parser/Pipeline.php b/src/Document/Parser/Pipeline.php index 168a833..a5155c4 100644 --- a/src/Document/Parser/Pipeline.php +++ b/src/Document/Parser/Pipeline.php @@ -61,6 +61,19 @@ public function parse(string $source): YamlStream if ($source !== '' && str_ends_with($source, "\n")) { $stream->setTrailingNewline(true); } + // Round-trip: the scanner normalises CRLF to LF so the rest of the + // parser sees one line ending, and the emitter already writes whatever + // the stream names - but nothing recorded what the source used, so a + // CRLF document was silently rewritten with LF. + // + // Set only when EVERY newline is CRLF. A file with mixed endings has no + // single ending to preserve, and quietly picking one would rewrite the + // other; LF leaves that document visibly changed rather than subtly, so + // a caller comparing input to output can still tell. + $lf = substr_count($source, "\n"); + if ($lf > 0 && substr_count($source, "\r\n") === $lf) { + $stream->setLineEnding("\r\n"); + } $this->resolver->resolve($stream); return $stream; diff --git a/src/Document/Parser/Scanner.php b/src/Document/Parser/Scanner.php index 6af6880..0b1e2f7 100644 --- a/src/Document/Parser/Scanner.php +++ b/src/Document/Parser/Scanner.php @@ -317,7 +317,7 @@ public function scan(string $source): array $commentColumn = $this->column; $text = ''; while ($this->pos < $this->length && $this->source[$this->pos] !== "\n") { - $text .= $this->source[$this->pos]; + $text .= $this->currentChar(); $this->advance(); } $this->triviaBuffer[] = TriviaToken::comment( @@ -810,7 +810,7 @@ private function scanBlockSequenceItem(array &$tokens): void $commentColumn = $this->column; $text = ''; while ($this->pos < $this->length && $this->source[$this->pos] !== "\n") { - $text .= $this->source[$this->pos]; + $text .= $this->currentChar(); $this->advance(); } $this->triviaBuffer[] = TriviaToken::comment( @@ -1746,7 +1746,7 @@ private function scanBlockMapEntry(array &$tokens): void $commentColumn = $this->column; $text = ''; while ($this->pos < $this->length && $this->source[$this->pos] !== "\n") { - $text .= $this->source[$this->pos]; + $text .= $this->currentChar(); $this->advance(); } $this->triviaBuffer[] = TriviaToken::comment( @@ -1821,7 +1821,7 @@ private function scanBlockMapEntry(array &$tokens): void $commentColumn = $this->column; $text = ''; while ($this->pos < $this->length && $this->source[$this->pos] !== "\n") { - $text .= $this->source[$this->pos]; + $text .= $this->currentChar(); $this->advance(); } $this->triviaBuffer[] = TriviaToken::comment( @@ -2079,7 +2079,7 @@ private function collectTrivia(): bool $startColumn = $this->column; $text = ''; while ($this->pos < $this->length && $this->source[$this->pos] !== "\n") { - $text .= $this->source[$this->pos]; + $text .= $this->currentChar(); $this->advance(); } $this->triviaBuffer[] = TriviaToken::comment( @@ -2193,7 +2193,7 @@ private function consumeDirective(): Token $this->advance(); $value = ''; while ($this->pos < $this->length && $this->source[$this->pos] !== "\n") { - $value .= $this->source[$this->pos]; + $value .= $this->currentChar(); $this->advance(); } if ($this->pos < $this->length && $this->source[$this->pos] === "\n") { @@ -3950,7 +3950,7 @@ private function collectTrailingEolTrivia(): array $startColumn = $this->column; $text = ''; while ($this->pos < $this->length && $this->source[$this->pos] !== "\n") { - $text .= $this->source[$this->pos]; + $text .= $this->currentChar(); $this->advance(); } $trailing[] = TriviaToken::comment($text, $startLine, $startColumn, $gap); diff --git a/test/unit/Document/CommentIndentTest.php b/test/unit/Document/CommentIndentTest.php new file mode 100644 index 0000000..3782228 --- /dev/null +++ b/test/unit/Document/CommentIndentTest.php @@ -0,0 +1,54 @@ + */ + public static function comments(): array + { + return [ + 'matching its sibling' => ["parent:\n # aligned\n key: 1\n"], + 'deeper than its sibling'=> ["parent:\n child:\n # over-indented\n key: 1\n"], + 'shallower' => ["parent:\n # four spaces\n key: 1\n"], + 'flush left in a block' => ["parent:\n# flush left\n key: 1\n"], + 'before a nested block' => ["a:\n b:\n # note\n c: 1\n"], + ]; + } + + #[DataProvider('comments')] + public function testCommentsKeepTheirColumn(string $source): void + { + self::assertSame($source, (new Emitter())->emit((new YamlStringLoader())->load($source))); + } + + public function testAProgrammaticCommentStillFollowsItsContainer(): void + { + // No column was ever recorded for it, so it indents with the block it + // is added to rather than landing at column 1. + $stream = (new YamlStringLoader())->load("parent:\n key: 1\n"); + $parent = $stream->getDocument(0)->root()->entry('parent')->getValue(); + $parent->appendChildInternal(new CommentNode('# added later')); + + self::assertStringContainsString(" # added later", (new Emitter())->emit($stream)); + } +} diff --git a/test/unit/Document/LineEndingTest.php b/test/unit/Document/LineEndingTest.php new file mode 100644 index 0000000..1ed38c7 --- /dev/null +++ b/test/unit/Document/LineEndingTest.php @@ -0,0 +1,53 @@ + */ + public static function documents(): array + { + return [ + 'crlf' => ["# note\r\na:\r\n b: 1\r\n"], + 'lf' => ["# note\na:\n b: 1\n"], + 'crlf with indentation' => ["a:\r\n b:\r\n c: 1\r\n"], + 'crlf with a comment column' => ["a:\r\n # deep\r\n b: 1\r\n"], + 'crlf with a block scalar' => ["a: |\r\n line one\r\n line two\r\n"], + 'crlf with a sequence' => ["a:\r\n - one\r\n - two\r\n"], + ]; + } + + #[DataProvider('documents')] + public function testLineEndingsRoundTrip(string $source): void + { + self::assertSame($source, (new Emitter())->emit((new YamlStringLoader())->load($source))); + } + + public function testTheStreamReportsTheEndingItRead(): void + { + self::assertSame("\r\n", (new YamlStringLoader())->load("a: 1\r\n")->getLineEnding()); + self::assertSame("\n", (new YamlStringLoader())->load("a: 1\n")->getLineEnding()); + } + + public function testAMixedDocumentIsNotGuessedAt(): void + { + // No single ending to preserve. LF leaves it visibly changed rather + // than subtly, so a caller comparing input to output can still tell. + self::assertSame("\n", (new YamlStringLoader())->load("a:\r\n b: 1\n")->getLineEnding()); + } +} diff --git a/test/unit/Document/MultibyteCommentTest.php b/test/unit/Document/MultibyteCommentTest.php new file mode 100644 index 0000000..80071d9 --- /dev/null +++ b/test/unit/Document/MultibyteCommentTest.php @@ -0,0 +1,59 @@ + */ + public static function nonAsciiComments(): array + { + return [ + 'latin-1 supplement' => ["# see §4.3\nkey: value\n"], + 'accented latin' => ["# café rollout\nkey: value\n"], + 'arabic' => ["# مملوك\nkey: value\n"], + 'cjk' => ["# 設定\nkey: value\n"], + 'emoji (4-byte)' => ["# ship it 🚀\nkey: value\n"], + 'eol comment' => ["key: value # ° degrees\n"], + 'indented comment' => ["parent:\n # é\n child: 1\n"], + ]; + } + + #[DataProvider('nonAsciiComments')] + public function testNonAsciiCommentsRoundTripByteIdentically(string $source): void + { + $stream = (new YamlStringLoader())->load($source); + + self::assertSame($source, (new Emitter())->emit($stream)); + } + + public function testTheCommentTextItselfIsIntact(): void + { + $stream = (new YamlStringLoader())->load("# see §4.3 café\nkey: value\n"); + + $texts = []; + foreach ($stream->getDocument(0)->root()->children() as $child) { + if (method_exists($child, 'getText')) { + $texts[] = $child->getText(); + } + } + + self::assertContains('# see §4.3 café', $texts); + } +} diff --git a/test/unit/Document/PlainScalarLeaderTest.php b/test/unit/Document/PlainScalarLeaderTest.php new file mode 100644 index 0000000..39f45c5 --- /dev/null +++ b/test/unit/Document/PlainScalarLeaderTest.php @@ -0,0 +1,62 @@ + */ + public static function plainScalars(): array + { + return [ + 'double dash flag' => ["key: --cert-dir=/tmp\n"], + 'single dash' => ["key: -single-dash\n"], + 'in a sequence' => ["args:\n - --kubelet-insecure-tls\n - --metric-resolution=15s\n"], + 'question leader' => ["key: ?query=1\n"], + 'colon leader' => ["key: ::1\n"], + ]; + } + + #[DataProvider('plainScalars')] + public function testPlainLeadersRoundTripUnquoted(string $source): void + { + self::assertSame($source, (new Emitter())->emit((new YamlStringLoader())->load($source))); + } + + /** @return array */ + public static function stillQuoted(): array + { + return [ + 'bare dash' => ["key: '-'\n"], + 'dash space' => ["key: '- not a sequence'\n"], + 'negative number'=> ["key: '-5'\n"], + 'bare colon' => ["key: ':'\n"], + 'bare question' => ["key: '?'\n"], + ]; + } + + /** + * The relaxation must not go too far: a leader that really is an indicator, + * or a value that would reparse as a non-string, still needs its quotes. + */ + #[DataProvider('stillQuoted')] + public function testGenuineIndicatorsKeepTheirQuotes(string $source): void + { + self::assertSame($source, (new Emitter())->emit((new YamlStringLoader())->load($source))); + } +}