Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Yaml] Emit {} instead of { } when dumping an empty map#64151

Open
ousamabenyounes wants to merge 1 commit intosymfony:8.1symfony/symfony:8.1from
ousamabenyounes:fix/issue-64137ousamabenyounes/symfony:fix/issue-64137Copy head branch name to clipboard
Open

[Yaml] Emit {} instead of { } when dumping an empty map#64151
ousamabenyounes wants to merge 1 commit intosymfony:8.1symfony/symfony:8.1from
ousamabenyounes:fix/issue-64137ousamabenyounes/symfony:fix/issue-64137Copy head branch name to clipboard

Conversation

@ousamabenyounes
Copy link
Copy Markdown
Contributor

@ousamabenyounes ousamabenyounes commented May 6, 2026

Q A
Branch? 8.1
Bug fix? no
New feature? no
Deprecations? no
Issues Fix #64137
License MIT

Inline::dumpHashArray() always wrapped its body with sprintf('{ %s }', ...), so an empty hash dumped as { } (two spaces). Empty arrays go through dumpArray(), which falls through to dumpHashArray() when neither sequence nor populated, so they suffered from the same artefact.

Mirror the empty-sequence form ([]) and emit {} for empty maps, which is also what Inline::parse('{}') already produces. This removes pointless whitespace from dumped configs — tooling around YAML coding standards (e.g. Drupal config) re-formats every diff otherwise.

Targeting 8.1 rather than 6.4 per @stof: { } is technically a valid YAML representation, this is a formatting cleanup, not a behavioural bug — patch versions on the LTS shouldn't move dumped output for projects asserting on it.

Coverage: existing DumperTest empty-map assertions are tightened from '{ }' to '{}', and the inline-level fixtures used in testInlineLevel/testIndentationInConstructor (which all contain a 'foo\'bar' => [] slot) are updated accordingly. A new ['{}', []] and ['{ foo: {} }', ['foo' => []]] are added to InlineTest::getTestsForDump so both the bare and nested empty-map paths are covered for dump+parse round-trip.

@stof
Copy link
Copy Markdown
Member

stof commented May 6, 2026

In the past, we shipped formatting changes in minor versions, not a bug fixes (this is not a bug after all. { } is also a valid Yaml representation).
I think it makes sense to also do it in this case (if some projects have tests doing assertions on the dumped output, it avoids making them fail when upgrading patch versions on the LTS branch).

Empty hashes/objects fall through to dumpHashArray() with an empty
$output, so sprintf('{ %s }', '') produced two spaces between the
braces. Mirror the empty-sequence form ([]) and emit '{}' instead,
which removes the pointless whitespace from dumped configs (Drupal-style
tooling re-formats every diff otherwise) and round-trips cleanly:
parse('{}') already returns [].
@ousamabenyounes ousamabenyounes changed the base branch from 6.4 to 8.1 May 6, 2026 15:54
@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 8.1 May 6, 2026
@nicolas-grekas nicolas-grekas removed the Bug label May 6, 2026
@ousamabenyounes
Copy link
Copy Markdown
Contributor Author

ousamabenyounes commented May 6, 2026

The red Unit Tests you can still see on 0a8e381 was a legitimate failure: DumperTest::testIndentationInConstructor and testInlineLevel embed "foo'bar": { } directly inside heredocs, and I had only updated the direct assertSame('{ }', …) calls in the first round.

2411aef (current head, rebased onto 8.1) updates those heredoc fixtures along with the eight remaining occurrences in getDumpCompactNestedMapping. The 474 tests in Yaml/Tests/ pass locally on PHPUnit 11 / PHP 8.4. CI on 2411aef is currently running.

The "Docker registry 500" only applied to the very first run (080e9aa, against 6.4). Apologies for the conflation.

@nicolas-grekas
Copy link
Copy Markdown
Member

Please don't let your LLM reply instead of you, that's noisy.

@ousamabenyounes
Copy link
Copy Markdown
Contributor Author

I've removed comments after noticing there were errors, and will keep future interactions direct and human-reviewed.

I'm putting the necessary checks in place to avoid having to correct this kind of issue going forward. Apologies again, and thanks for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Yaml dumper writes 2 spaces on empty array's

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.