Skip to content

Commit 2dcc229

Browse files
mikefarahmikefarah
committed
Merge branch 'tomers-fix/toml-comments-table-scope-2588'
2 parents 7cf88a0 + eb4fde4 commit 2dcc229

2 files changed

Lines changed: 109 additions & 21 deletions

File tree

‎pkg/yqlib/decoder_toml.go‎

Lines changed: 85 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,18 @@ func (dec *tomlDecoder) Init(reader io.Reader) error {
4747
return nil
4848
}
4949

50+
func (dec *tomlDecoder) attachOrphanedCommentsToNode(tableNodeValue *CandidateNode) {
51+
if len(dec.pendingComments) > 0 {
52+
comments := strings.Join(dec.pendingComments, "\n")
53+
if tableNodeValue.HeadComment == "" {
54+
tableNodeValue.HeadComment = comments
55+
} else {
56+
tableNodeValue.HeadComment = tableNodeValue.HeadComment + "\n" + comments
57+
}
58+
dec.pendingComments = make([]string, 0)
59+
}
60+
}
61+
5062
func (dec *tomlDecoder) getFullPath(tomlNode *toml.Node) []interface{} {
5163
path := make([]interface{}, 0)
5264
for {
@@ -329,20 +341,39 @@ func (dec *tomlDecoder) processTable(currentNode *toml.Node) (bool, error) {
329341

330342
var tableValue *toml.Node
331343
runAgainstCurrentExp := false
332-
hasValue := dec.parser.NextExpression()
333-
// check to see if there is any table data
334-
if hasValue {
344+
sawKeyValue := false
345+
for dec.parser.NextExpression() {
335346
tableValue = dec.parser.Expression()
336-
// next expression is not table data, so we are done
347+
// Allow standalone comments inside the table before the first key-value.
348+
// These should be associated with the next element in the table (usually the first key-value),
349+
// not treated as "end of table" (which would cause subsequent key-values to be parsed at root).
350+
if tableValue.Kind == toml.Comment {
351+
dec.pendingComments = append(dec.pendingComments, string(tableValue.Data))
352+
continue
353+
}
354+
355+
// next expression is not table data, so we are done (but we need to re-process it at top-level)
337356
if tableValue.Kind != toml.KeyValue {
338-
log.Debug("got an empty table")
339-
runAgainstCurrentExp = true
340-
} else {
341-
runAgainstCurrentExp, err = dec.decodeKeyValuesIntoMap(tableNodeValue, tableValue)
342-
if err != nil && !errors.Is(err, io.EOF) {
343-
return false, err
357+
log.Debug("got an empty table (or reached next section)")
358+
// If the table had only comments, attach them to the table itself so they don't leak to the next node.
359+
if !sawKeyValue {
360+
dec.attachOrphanedCommentsToNode(tableNodeValue)
344361
}
362+
runAgainstCurrentExp = true
363+
break
364+
}
365+
366+
sawKeyValue = true
367+
runAgainstCurrentExp, err = dec.decodeKeyValuesIntoMap(tableNodeValue, tableValue)
368+
if err != nil && !errors.Is(err, io.EOF) {
369+
return false, err
345370
}
371+
break
372+
}
373+
// If we hit EOF after only seeing comments inside this table, attach them to the table itself
374+
// so they don't leak to whatever comes next.
375+
if !sawKeyValue {
376+
dec.attachOrphanedCommentsToNode(tableNodeValue)
346377
}
347378

348379
err = dec.d.DeeplyAssign(c, fullPath, tableNodeValue)
@@ -405,19 +436,52 @@ func (dec *tomlDecoder) processArrayTable(currentNode *toml.Node) (bool, error)
405436
}
406437

407438
runAgainstCurrentExp := false
408-
// if the next value is a ArrayTable or Table, then its not part of this declaration (not a key value pair)
409-
// so lets leave that expression for the next round of parsing
410-
if hasValue && (dec.parser.Expression().Kind == toml.ArrayTable || dec.parser.Expression().Kind == toml.Table) {
411-
runAgainstCurrentExp = true
412-
} else if hasValue {
413-
// otherwise, if there is a value, it must be some key value pairs of the
414-
// first object in the array!
415-
tableValue := dec.parser.Expression()
416-
runAgainstCurrentExp, err = dec.decodeKeyValuesIntoMap(tableNodeValue, tableValue)
417-
if err != nil && !errors.Is(err, io.EOF) {
418-
return false, err
439+
sawKeyValue := false
440+
if hasValue {
441+
for {
442+
exp := dec.parser.Expression()
443+
// Allow standalone comments inside array tables before the first key-value.
444+
if exp.Kind == toml.Comment {
445+
dec.pendingComments = append(dec.pendingComments, string(exp.Data))
446+
hasValue = dec.parser.NextExpression()
447+
if !hasValue {
448+
break
449+
}
450+
continue
451+
}
452+
453+
// if the next value is a ArrayTable or Table, then its not part of this declaration (not a key value pair)
454+
// so lets leave that expression for the next round of parsing
455+
if exp.Kind == toml.ArrayTable || exp.Kind == toml.Table {
456+
// If this array-table entry had only comments, attach them to the entry so they don't leak.
457+
if !sawKeyValue {
458+
dec.attachOrphanedCommentsToNode(tableNodeValue)
459+
}
460+
runAgainstCurrentExp = true
461+
break
462+
}
463+
464+
sawKeyValue = true
465+
// otherwise, if there is a value, it must be some key value pairs of the
466+
// first object in the array!
467+
runAgainstCurrentExp, err = dec.decodeKeyValuesIntoMap(tableNodeValue, exp)
468+
if err != nil && !errors.Is(err, io.EOF) {
469+
return false, err
470+
}
471+
break
419472
}
420473
}
474+
// If we hit EOF after only seeing comments inside this array-table entry, attach them to the entry
475+
// so they don't leak to whatever comes next.
476+
if !sawKeyValue && len(dec.pendingComments) > 0 {
477+
comments := strings.Join(dec.pendingComments, "\n")
478+
if tableNodeValue.HeadComment == "" {
479+
tableNodeValue.HeadComment = comments
480+
} else {
481+
tableNodeValue.HeadComment = tableNodeValue.HeadComment + "\n" + comments
482+
}
483+
dec.pendingComments = make([]string, 0)
484+
}
421485

422486
// += function
423487
err = dec.arrayAppend(c, fullPath, tableNodeValue)

‎pkg/yqlib/toml_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,14 @@ B = 12
228228
name = "Tom" # name comment
229229
`
230230

231+
// Reproduce bug for https://github.com/mikefarah/yq/issues/2588
232+
// Bug: standalone comments inside a table cause subsequent key-values to be assigned at root.
233+
var issue2588RustToolchainWithComments = `
234+
[owner]
235+
# comment
236+
name = "Tomer"
237+
`
238+
231239
var sampleFromWeb = `# This is a TOML document
232240
title = "TOML Example"
233241
@@ -550,6 +558,22 @@ var tomlScenarios = []formatScenario{
550558
expected: rtComments,
551559
scenarioType: "roundtrip",
552560
},
561+
{
562+
skipDoc: true,
563+
description: "Issue #2588: comments inside table must not flatten (.owner.name)",
564+
input: issue2588RustToolchainWithComments,
565+
expression: ".owner.name",
566+
expected: "Tomer\n",
567+
scenarioType: "decode",
568+
},
569+
{
570+
skipDoc: true,
571+
description: "Issue #2588: comments inside table must not flatten (.name)",
572+
input: issue2588RustToolchainWithComments,
573+
expression: ".name",
574+
expected: "null\n",
575+
scenarioType: "decode",
576+
},
553577
{
554578
description: "Roundtrip: sample from web",
555579
input: sampleFromWeb,

0 commit comments

Comments
 (0)