DeleteProperty should not remove pre-existing empty mappings#7145
Merged
steve-aom-elliott merged 3 commits intomainfrom Mar 25, 2026
Merged
DeleteProperty should not remove pre-existing empty mappings#7145steve-aom-elliott merged 3 commits intomainfrom
steve-aom-elliott merged 3 commits intomainfrom
Conversation
The visitMapping method in DeleteProperty unconditionally removed any
mapping entry whose value was an empty Yaml.Mapping. This caused
unrelated entries like `workflow_dispatch: {}` in GitHub Actions
workflow files to be deleted when DeleteProperty visited those files
looking for a completely different property key.
The empty-mapping check was intended to cascade deletions upward (when
a mapping becomes empty because all its children were deleted). But
this cascading is already handled correctly by the ToBeRemoved marker
propagation: when visitMapping filters out all entries, it marks the
now-empty mapping with ToBeRemoved, which the parent visitMapping
picks up via the existing `ToBeRemoved.hasMarker(entry.getValue())`
check.
Removing the redundant empty-mapping condition fixes the regression
without breaking the cascading delete behavior.
Verifies that deleting `on.push` preserves the sibling
`workflow_dispatch: {}` entry under the same parent mapping.
Verifies that deleting a 4-level deep property correctly cascades upward, and that pre-existing empty mapping siblings at intermediate levels are preserved during the cascade.
steve-aom-elliott
added
bug
3 tasks
sambsnyd
approved these changes
Mar 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DeleteProperty.visitMapping()unconditionally removed mapping entries whose value was an emptyYaml.Mapping(e.g.workflow_dispatch: {}), even when those entries had nothing to do with the property being deletedDeleteSpringProperty→DeletePropertyon all YAML files) to stripworkflow_dispatch: {}from GitHub Actions workflow filesToBeRemovedmarker propagationTest plan
doesNotDeletePreExistingEmptyMappings— verifies thatworkflow_dispatch: {}in a workflow file is left untouched when deleting an unrelated propertydeletesEmptyMappingThatBecameEmptyFromDeletion— verifies that cascading deletion still works (a parent mapping that becomes empty after its children are deleted is still removed)DeletePropertyKeyTesttests passDeleteKeyTesttests pass