Skip to content

Commit d5aaf49

Browse files
Jenson3210Jenson3210
authored
Fix DeleteProperty losing newlines when deleting entries with block scalars (#6670)
* Fix DeleteProperty losing newlines when deleting entries with block scalars When deleting YAML entries that have block scalar values (like `>-` or `|`), the newlines that separate entries were being lost. This happened because block scalar content in YAML includes trailing newlines that serve as separators between entries. The fix adds: 1. Detection of when deleted entries had block scalar values 2. Proper newline restoration for sibling entries in the same mapping 3. Proper newline restoration for entries in parent mappings when child mappings are modified Added test for glob pattern matching with folded block scalar values. * Remove comments * Remove comment and add higher parent level test * Add test for deleting leading key before block scalar * Fix higher parent test to actually test parent level sibling * Add test for removing block scalar as first entry before non-scalar * Fix block scalar newline loss in sequences
1 parent 3d79451 commit d5aaf49

2 files changed

Lines changed: 210 additions & 1 deletion

File tree

‎rewrite-yaml/src/main/java/org/openrewrite/yaml/DeleteProperty.java‎

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,35 @@ public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionC
112112
@Override
113113
public Yaml.Sequence visitSequence(Yaml.Sequence sequence, ExecutionContext ctx) {
114114
Yaml.Sequence s = super.visitSequence(sequence, ctx);
115+
boolean childModified = (s != sequence);
115116
List<Yaml.Sequence.Entry> entries = s.getEntries();
116117
if (entries.isEmpty()) {
117118
return s;
118119
}
119120

121+
boolean changed = false;
120122
entries = ListUtils.map(entries, entry -> ToBeRemoved.hasMarker(entry) ? null : entry);
123+
if (entries.size() != s.getEntries().size()) {
124+
changed = true;
125+
}
126+
127+
if ((changed || childModified) && s.getOpeningBracketPrefix() == null) {
128+
List<Yaml.Sequence.Entry> fixedEntries = null;
129+
for (int i = 1; i < entries.size(); i++) {
130+
Yaml.Sequence.Entry entry = entries.get(i);
131+
Yaml.Sequence.Entry prevEntry = entries.get(i - 1);
132+
if (!startsWithNewline(entry.getPrefix()) && !endsWithBlockScalar(prevEntry.getBlock())) {
133+
if (fixedEntries == null) {
134+
fixedEntries = new ArrayList<>(entries);
135+
}
136+
fixedEntries.set(i, entry.withPrefix("\n" + entry.getPrefix()));
137+
}
138+
}
139+
if (fixedEntries != null) {
140+
entries = fixedEntries;
141+
}
142+
}
143+
121144
return entries.isEmpty() ? ToBeRemoved.withMarker(s) : s.withEntries(entries);
122145
}
123146

@@ -136,10 +159,12 @@ public Yaml.Sequence.Entry visitSequenceEntry(Yaml.Sequence.Entry entry, Executi
136159
@Override
137160
public Yaml.Mapping visitMapping(Yaml.Mapping mapping, ExecutionContext ctx) {
138161
Yaml.Mapping m = super.visitMapping(mapping, ctx);
162+
boolean childModified = (m != mapping);
139163

140164
boolean changed = false;
141165
List<Yaml.Mapping.Entry> entries = new ArrayList<>();
142166
String firstDeletedPrefix = null;
167+
boolean previousWasDeleted = false;
143168
for (Yaml.Mapping.Entry entry : m.getEntries()) {
144169
if (ToBeRemoved.hasMarker(entry.getValue()) ||
145170
ToBeRemoved.hasMarker(entry) ||
@@ -149,12 +174,15 @@ public Yaml.Mapping visitMapping(Yaml.Mapping mapping, ExecutionContext ctx) {
149174
firstDeletedPrefix = entry.getPrefix();
150175
}
151176
changed = true;
177+
previousWasDeleted = true;
152178
} else {
153179
if (entries.isEmpty() && firstDeletedPrefix != null && containsOnlyWhitespace(entry.getPrefix())) {
154-
// This is the first kept entry and there were deleted entries before it
155180
entry = entry.withPrefix(firstDeletedPrefix);
181+
} else if (previousWasDeleted && !entries.isEmpty() && !startsWithNewline(entry.getPrefix())) {
182+
entry = entry.withPrefix("\n" + entry.getPrefix());
156183
}
157184
entries.add(entry);
185+
previousWasDeleted = false;
158186
}
159187
}
160188

@@ -171,6 +199,25 @@ public Yaml.Mapping visitMapping(Yaml.Mapping mapping, ExecutionContext ctx) {
171199
}
172200
}
173201
}
202+
203+
if ((changed || childModified) && m.getOpeningBracePrefix() == null) {
204+
List<Yaml.Mapping.Entry> currentEntries = m.getEntries();
205+
List<Yaml.Mapping.Entry> fixedEntries = null;
206+
for (int i = 1; i < currentEntries.size(); i++) {
207+
Yaml.Mapping.Entry entry = currentEntries.get(i);
208+
Yaml.Mapping.Entry prevEntry = currentEntries.get(i - 1);
209+
if (!startsWithNewline(entry.getPrefix()) && !endsWithBlockScalar(prevEntry)) {
210+
if (fixedEntries == null) {
211+
fixedEntries = new ArrayList<>(currentEntries);
212+
}
213+
fixedEntries.set(i, entry.withPrefix("\n" + entry.getPrefix()));
214+
}
215+
}
216+
if (fixedEntries != null) {
217+
m = m.withEntries(fixedEntries);
218+
}
219+
}
220+
174221
return m;
175222
}
176223
});
@@ -191,6 +238,32 @@ private static boolean containsOnlyWhitespace(@Nullable String str) {
191238
return true;
192239
}
193240

241+
private static boolean startsWithNewline(@Nullable String str) {
242+
return str != null && !str.isEmpty() && str.charAt(0) == '\n';
243+
}
244+
245+
private static boolean endsWithBlockScalar(Yaml.Mapping.Entry entry) {
246+
return endsWithBlockScalar(entry.getValue());
247+
}
248+
249+
private static boolean endsWithBlockScalar(Yaml.Block block) {
250+
if (block instanceof Yaml.Scalar) {
251+
Yaml.Scalar.Style style = ((Yaml.Scalar) block).getStyle();
252+
return style == Yaml.Scalar.Style.FOLDED || style == Yaml.Scalar.Style.LITERAL;
253+
} else if (block instanceof Yaml.Mapping) {
254+
List<Yaml.Mapping.Entry> entries = ((Yaml.Mapping) block).getEntries();
255+
if (!entries.isEmpty()) {
256+
return endsWithBlockScalar(entries.get(entries.size() - 1));
257+
}
258+
} else if (block instanceof Yaml.Sequence) {
259+
List<Yaml.Sequence.Entry> entries = ((Yaml.Sequence) block).getEntries();
260+
if (!entries.isEmpty()) {
261+
return endsWithBlockScalar(entries.get(entries.size() - 1).getBlock());
262+
}
263+
}
264+
return false;
265+
}
266+
194267
@Value
195268
@With
196269
private static class ToBeRemoved implements Marker {

‎rewrite-yaml/src/test/java/org/openrewrite/yaml/DeletePropertyKeyTest.java‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,4 +408,140 @@ void globPatternWithExactMatch() {
408408
)
409409
);
410410
}
411+
412+
@Test
413+
void globPatternWithFoldedBlockScalarDeleteLeading() {
414+
rewriteRun(
415+
spec -> spec.recipe(new DeleteProperty("acme.my-project.*.first-name", null, null, null)),
416+
yaml(
417+
"""
418+
acme:
419+
my-project:
420+
person:
421+
first-name: John
422+
last-name: >-
423+
Doe
424+
employee:
425+
firstName: Jane
426+
lastName: Smith
427+
""",
428+
"""
429+
acme:
430+
my-project:
431+
person:
432+
last-name: >-
433+
Doe
434+
employee:
435+
lastName: Smith
436+
"""
437+
)
438+
);
439+
}
440+
441+
@Test
442+
void globPatternWithFoldedBlockScalarDeleteLeadingScalar() {
443+
rewriteRun(
444+
spec -> spec.recipe(new DeleteProperty("acme.my-project.*.first-name", null, null, null)),
445+
yaml(
446+
"""
447+
acme:
448+
my-project:
449+
person:
450+
first-name: >-
451+
John
452+
last-name: Doe
453+
employee:
454+
firstName: Jane
455+
lastName: Smith
456+
""",
457+
"""
458+
acme:
459+
my-project:
460+
person:
461+
last-name: Doe
462+
employee:
463+
lastName: Smith
464+
"""
465+
)
466+
);
467+
}
468+
469+
@Test
470+
void globPatternWithFoldedBlockScalarDeleteTrailing() {
471+
rewriteRun(
472+
spec -> spec.recipe(new DeleteProperty("acme.my-project.*.last-name", null, null, null)),
473+
yaml(
474+
"""
475+
acme:
476+
my-project:
477+
person:
478+
first-name: John
479+
last-name: >-
480+
Doe
481+
employee:
482+
firstName: Jane
483+
lastName: Smith
484+
""",
485+
"""
486+
acme:
487+
my-project:
488+
person:
489+
first-name: John
490+
employee:
491+
firstName: Jane
492+
"""
493+
)
494+
);
495+
}
496+
497+
@Test
498+
void globPatternWithFoldedBlockScalarInSequence() {
499+
rewriteRun(
500+
spec -> spec.recipe(new DeleteProperty("acme.items.last-name", null, null, null)),
501+
yaml(
502+
"""
503+
acme:
504+
items:
505+
- first-name: John
506+
last-name: >-
507+
Doe
508+
- first-name: Jane
509+
last-name: Smith
510+
""",
511+
"""
512+
acme:
513+
items:
514+
- first-name: John
515+
- first-name: Jane
516+
"""
517+
)
518+
);
519+
}
520+
521+
@Test
522+
void globPatternWithFoldedBlockScalarHigherParent() {
523+
rewriteRun(
524+
spec -> spec.recipe(new DeleteProperty("acme.my-project.*.last-name", null, null, null)),
525+
yaml(
526+
"""
527+
acme:
528+
my-project:
529+
person:
530+
first-name: John
531+
last-name: >-
532+
Doe
533+
other:
534+
key: value
535+
""",
536+
"""
537+
acme:
538+
my-project:
539+
person:
540+
first-name: John
541+
other:
542+
key: value
543+
"""
544+
)
545+
);
546+
}
411547
}

0 commit comments

Comments
 (0)
⚡