diff --git a/cmd/constant.go b/cmd/constant.go index 762897d0..403187c5 100644 --- a/cmd/constant.go +++ b/cmd/constant.go @@ -35,3 +35,5 @@ var completedSuccessfully = false var forceExpression = "" var expressionFile = "" + +var yamlParser = "" diff --git a/cmd/root.go b/cmd/root.go index 3d479576..020d10f0 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -94,10 +94,14 @@ yq -P -oy sample.json logging.SetBackend(backend) yqlib.InitExpressionParser() - // Handle legacy parser flag - useLegacyYamlParser, _ := cmd.Flags().GetBool("yaml-v3") - if useLegacyYamlParser { + // Handle YAML parser selection with validation + switch yamlParser { + case "goccy", "": + yqlib.ConfiguredYamlPreferences.UseGoccyParser = true + case "v3": yqlib.ConfiguredYamlPreferences.UseGoccyParser = false + default: + return fmt.Errorf("invalid yaml-parser value '%s'. Valid options are: 'goccy', 'v3'", yamlParser) } return nil @@ -203,11 +207,10 @@ yq -P -oy sample.json } rootCmd.PersistentFlags().BoolVarP(&yqlib.ConfiguredYamlPreferences.LeadingContentPreProcessing, "header-preprocess", "", true, "Slurp any header comments and separators before processing expression.") - rootCmd.PersistentFlags().BoolVar(&yqlib.ConfiguredYamlPreferences.UseGoccyParser, "yaml-goccy", true, "Use goccy/go-yaml parser (default)") - - // Add a flag to revert to the legacy parser - var useLegacyYamlParser bool - rootCmd.PersistentFlags().BoolVar(&useLegacyYamlParser, "yaml-v3", false, "Use legacy gopkg.in/yaml.v3 parser instead of goccy (for backwards compatibility)") + rootCmd.PersistentFlags().StringVar(&yamlParser, "yaml-parser", "goccy", "YAML parser to use: 'goccy' (default, actively maintained) or 'v3' (legacy gopkg.in/yaml.v3)") + if err = rootCmd.RegisterFlagCompletionFunc("yaml-parser", cobra.FixedCompletions([]string{"goccy", "v3"}, cobra.ShellCompDirectiveNoFileComp)); err != nil { + panic(err) + } rootCmd.PersistentFlags().StringVarP(&splitFileExp, "split-exp", "s", "", "print each result (or doc) into a file named (exp). [exp] argument must return a string. You can use $index in the expression as the result counter. The necessary directories will be created.") if err = rootCmd.RegisterFlagCompletionFunc("split-exp", cobra.NoFileCompletions); err != nil { diff --git a/pkg/yqlib/candidate_node_goccy_yaml.go b/pkg/yqlib/candidate_node_goccy_yaml.go index fc3f29fe..0b44eeb3 100644 --- a/pkg/yqlib/candidate_node_goccy_yaml.go +++ b/pkg/yqlib/candidate_node_goccy_yaml.go @@ -17,32 +17,32 @@ func (o *CandidateNode) goccyDecodeIntoChild(childNode ast.Node, cm yaml.Comment } func (o *CandidateNode) UnmarshalGoccyYAML(node ast.Node, cm yaml.CommentMap, anchorMap map[string]*CandidateNode) error { - log.Debugf("UnmarshalYAML %v", node) - log.Debugf("UnmarshalYAML %v", node.Type().String()) - log.Debugf("UnmarshalYAML Node Value: %v", node.String()) - log.Debugf("UnmarshalYAML Node GetComment: %v", node.GetComment()) + // log.Debugf("UnmarshalYAML %v", node) + // log.Debugf("UnmarshalYAML %v", node.Type().String()) + // log.Debugf("UnmarshalYAML Node Value: %v", node.String()) + // log.Debugf("UnmarshalYAML Node GetComment: %v", node.GetComment()) if node.GetComment() != nil { commentMapComments := cm[node.GetPath()] for _, comment := range node.GetComment().Comments { // need to use the comment map to find the position :/ - log.Debugf("%v has a comment of [%v]", node.GetPath(), comment.Token.Value) + // log.Debugf("%v has a comment of [%v]", node.GetPath(), comment.Token.Value) for _, commentMapComment := range commentMapComments { commentMapValue := strings.Join(commentMapComment.Texts, "\n") if commentMapValue == comment.Token.Value { - log.Debug("found a matching entry in comment map") + // log.Debug("found a matching entry in comment map") // we found the comment in the comment map, // now we can process the position switch commentMapComment.Position { case yaml.CommentHeadPosition: o.HeadComment = comment.String() - log.Debug("its a head comment %v", comment.String()) + // log.Debug("its a head comment %v", comment.String()) case yaml.CommentLinePosition: o.LineComment = comment.String() - log.Debug("its a line comment %v", comment.String()) + // log.Debug("its a line comment %v", comment.String()) case yaml.CommentFootPosition: o.FootComment = comment.String() - log.Debug("its a foot comment %v", comment.String()) + // log.Debug("its a foot comment %v", comment.String()) } } } @@ -65,7 +65,7 @@ func (o *CandidateNode) UnmarshalGoccyYAML(node ast.Node, cm yaml.CommentMap, an o.Kind = ScalarNode o.Tag = "!!bool" case ast.NullType: - log.Debugf("its a null type with value %v", node.GetToken().Value) + // log.Debugf("its a null type with value %v", node.GetToken().Value) o.Kind = ScalarNode o.Tag = "!!null" o.Value = node.GetToken().Value @@ -79,29 +79,30 @@ func (o *CandidateNode) UnmarshalGoccyYAML(node ast.Node, cm yaml.CommentMap, an o.Style = DoubleQuotedStyle } o.Value = node.(*ast.StringNode).Value - log.Debugf("string value %v", node.(*ast.StringNode).Value) + // log.Debugf("string value %v", node.(*ast.StringNode).Value) case ast.LiteralType: o.Kind = ScalarNode o.Tag = "!!str" o.Style = LiteralStyle astLiteral := node.(*ast.LiteralNode) - log.Debugf("astLiteral.Start.Type %v", astLiteral.Start.Type) + // log.Debugf("astLiteral.Start.Type %v", astLiteral.Start.Type) if astLiteral.Start.Type == goccyToken.FoldedType { - log.Debugf("folded Type %v", astLiteral.Start.Type) + // log.Debugf("folded Type %v", astLiteral.Start.Type) o.Style = FoldedStyle } - log.Debug("start value: %v ", node.(*ast.LiteralNode).Start.Value) - log.Debug("start value: %v ", node.(*ast.LiteralNode).Start.Type) - // TODO: here I could put the original value with line breaks - // to solve the multiline > problem + // log.Debug("start value: %v ", node.(*ast.LiteralNode).Start.Value) + // log.Debug("start value: %v ", node.(*ast.LiteralNode).Start.Type) + // Preserving the original multiline string value is important for fidelity. + // goccy/go-yaml provides this in astLiteral.Value.Value for literal and folded styles. o.Value = astLiteral.Value.Value case ast.TagType: + // Recursively unmarshal the tagged value, then apply the tag to the CandidateNode. if err := o.UnmarshalGoccyYAML(node.(*ast.TagNode).Value, cm, anchorMap); err != nil { return err } - o.Tag = node.(*ast.TagNode).Start.Value + o.Tag = node.(*ast.TagNode).Start.Value // Tag value includes the '!' or '!!' prefix. case ast.MappingType: - log.Debugf("UnmarshalYAML - a mapping node") + // log.Debugf("UnmarshalYAML - a mapping node") o.Kind = MappingNode o.Tag = "!!map" @@ -112,24 +113,24 @@ func (o *CandidateNode) UnmarshalGoccyYAML(node ast.Node, cm yaml.CommentMap, an for _, mappingValueNode := range mappingNode.Values { err := o.goccyProcessMappingValueNode(mappingValueNode, cm, anchorMap) if err != nil { - return ast.ErrInvalidAnchorName + return err } } if mappingNode.FootComment != nil { - log.Debugf("mapping node has a foot comment of: %v", mappingNode.FootComment) + // log.Debugf("mapping node has a foot comment of: %v", mappingNode.FootComment) o.FootComment = mappingNode.FootComment.String() } case ast.MappingValueType: - log.Debugf("UnmarshalYAML - a mapping node") + // log.Debugf("UnmarshalYAML - a mapping node") o.Kind = MappingNode o.Tag = "!!map" mappingValueNode := node.(*ast.MappingValueNode) err := o.goccyProcessMappingValueNode(mappingValueNode, cm, anchorMap) if err != nil { - return ast.ErrInvalidAnchorName + return err } case ast.SequenceType: - log.Debugf("UnmarshalYAML - a sequence node") + // log.Debugf("UnmarshalYAML - a sequence node") o.Kind = SequenceNode o.Tag = "!!seq" sequenceNode := node.(*ast.SequenceNode) @@ -154,7 +155,7 @@ func (o *CandidateNode) UnmarshalGoccyYAML(node ast.Node, cm yaml.CommentMap, an o.Content[i] = valueNode } case ast.AnchorType: - log.Debugf("UnmarshalYAML - an anchor node") + // log.Debugf("UnmarshalYAML - an anchor node") anchorNode := node.(*ast.AnchorNode) err := o.UnmarshalGoccyYAML(anchorNode.Value, cm, anchorMap) if err != nil { @@ -164,14 +165,14 @@ func (o *CandidateNode) UnmarshalGoccyYAML(node ast.Node, cm yaml.CommentMap, an anchorMap[o.Anchor] = o case ast.AliasType: - log.Debugf("UnmarshalYAML - an alias node") + // log.Debugf("UnmarshalYAML - an alias node") aliasNode := node.(*ast.AliasNode) o.Kind = AliasNode o.Value = aliasNode.Value.String() o.Alias = anchorMap[o.Value] case ast.MergeKeyType: - log.Debugf("UnmarshalYAML - a merge key") + // log.Debugf("UnmarshalYAML - a merge key") o.Kind = ScalarNode o.Tag = "!!merge" // note - I should be able to get rid of this. o.Value = "<<" @@ -220,38 +221,48 @@ func (o *CandidateNode) MarshalGoccyYAML() (interface{}, error) { return o.Value, nil case ScalarNode: - log.Debug("MarshalGoccyYAML - scalar: %v", o.Value) + // log.Debug("MarshalGoccyYAML - scalar: %v", o.Value) - // Handle different scalar types based on tag + // Handle different scalar types based on tag for correct marshalling. switch o.Tag { case "!!int": if val, err := parseInt(o.Value); err == nil { return val, nil + } else { + return nil, fmt.Errorf("cannot marshal node %s as int: %w", NodeToString(o), err) } case "!!float": if val, err := parseFloat(o.Value); err == nil { return val, nil + } else { + return nil, fmt.Errorf("cannot marshal node %s as float: %w", NodeToString(o), err) } case "!!bool": if val, err := parseBool(o.Value); err == nil { return val, nil + } else { + return nil, fmt.Errorf("cannot marshal node %s as bool: %w", NodeToString(o), err) } case "!!null": + // goccy/go-yaml expects a nil interface{} for null values. return nil, nil default: - // String or unknown type + // For standard strings (!!str) or unknown/custom tags, marshal as a string. + // The goccy encoder will handle quoting and style if it's a plain string. + // For custom tags, goccy prepends the tag if the value is a string. return o.Value, nil } - return o.Value, nil case MappingNode: log.Debug("MarshalGoccyYAML - mapping: %v", NodeToString(o)) + // Ensure even number of children for key-value pairs + if len(o.Content)%2 != 0 { + return nil, fmt.Errorf("mapping node at %s has an odd number of children (%d), malformed key-value pairs", NodeToString(o), len(o.Content)) + } result := make(map[string]interface{}) for i := 0; i < len(o.Content); i += 2 { - if i+1 >= len(o.Content) { - break - } + // No need to check i+1 >= len(o.Content) here due to the check above keyNode := o.Content[i] valueNode := o.Content[i+1] diff --git a/pkg/yqlib/decoder_goccy_yaml.go b/pkg/yqlib/decoder_goccy_yaml.go index f09c57e0..4115c70a 100644 --- a/pkg/yqlib/decoder_goccy_yaml.go +++ b/pkg/yqlib/decoder_goccy_yaml.go @@ -1,8 +1,8 @@ //go:build !yq_noyaml -// -// NOTE this is still a WIP - not yet ready. -// +// Package yqlib provides YAML processing functionality using the goccy/go-yaml parser. +// This decoder implementation provides compatibility with the legacy yaml.v3 parser +// while leveraging the actively maintained goccy/go-yaml library. package yqlib @@ -18,144 +18,185 @@ import ( "github.com/goccy/go-yaml/ast" ) +// Constants for document processing +const ( + // docSeparatorPlaceholder is used internally to mark document separators during preprocessing. + docSeparatorPlaceholder = "$yqDocSeparator$" +) + type goccyYamlDecoder struct { decoder yaml.Decoder cm yaml.CommentMap prefs YamlPreferences - // work around of various parsing issues by handling document headers + // leadingContent stores comments and directives found before the actual YAML content. leadingContent string - bufferRead bytes.Buffer + // bufferRead is used with a TeeReader when LeadingContentPreProcessing is off, + // to capture initial bytes that might only contain comments. + bufferRead bytes.Buffer - // anchor map persists over multiple documents for convenience. + // anchorMap stores anchors encountered within the current YAML stream. + // It is reset by Init() for each new stream. anchorMap map[string]*CandidateNode - readAnything bool - firstFile bool - documentIndex uint + readAnything bool // Flag to track if any actual YAML node (or synthesized comment node) has been decoded. + firstFile bool // Flag for 'evaluateTogether' mode to handle leading content only once. + documentIndex uint // Index of the current document within the stream. } +// NewGoccyYAMLDecoder creates a new YAML decoder using the goccy/go-yaml library, +// configured with the given preferences. func NewGoccyYAMLDecoder(prefs YamlPreferences) Decoder { return &goccyYamlDecoder{prefs: prefs, firstFile: true} } +// processReadStream pre-processes the input stream to extract leading comments, +// directives, and document separators before the main YAML content is parsed. +// It returns a reader for the remaining stream and the extracted leading content. func (dec *goccyYamlDecoder) processReadStream(reader *bufio.Reader) (io.Reader, string, error) { var commentLineRegEx = regexp.MustCompile(`^\s*#`) - var yamlDirectiveLineRegEx = regexp.MustCompile(`^\s*%YA`) + var yamlDirectiveLineRegEx = regexp.MustCompile(`^\s*%YA`) // e.g., %YAML var sb strings.Builder for { - peekBytes, err := reader.Peek(4) - if errors.Is(err, io.EOF) { - // EOF are handled else where.. - return reader, sb.String(), nil - } else if err != nil { - return reader, sb.String(), err - } else if string(peekBytes[0]) == "\n" { - _, err := reader.ReadString('\n') + peekBytes, err := reader.Peek(4) // Peek up to 4 bytes to identify line types. + + if err != nil { + if errors.Is(err, io.EOF) { // EOF encountered while peeking. + // No more full lines can be definitively processed as leading content. + return reader, sb.String(), nil + } + return reader, sb.String(), err // Other errors during Peek. + } + + peekString := string(peekBytes) // Convert peeked bytes to string once per iteration. + + if strings.HasPrefix(peekString, "\n") { // Line is blank. + _, readErr := reader.ReadString('\n') sb.WriteString("\n") - if errors.Is(err, io.EOF) { - return reader, sb.String(), nil - } else if err != nil { - return reader, sb.String(), err + if readErr != nil { + if errors.Is(readErr, io.EOF) { + return reader, sb.String(), nil + } + return reader, sb.String(), readErr } - } else if string(peekBytes) == "--- " { - _, err := reader.ReadString(' ') - sb.WriteString("$yqDocSeparator$\n") - if errors.Is(err, io.EOF) { - return reader, sb.String(), nil - } else if err != nil { - return reader, sb.String(), err + } else if strings.HasPrefix(peekString, "--- ") { // Document separator "--- " + _, readErr := reader.ReadString(' ') // Consume "--- " + sb.WriteString(docSeparatorPlaceholder + "\n") + if readErr != nil { + if errors.Is(readErr, io.EOF) { + return reader, sb.String(), nil + } + return reader, sb.String(), readErr } - } else if string(peekBytes) == "---\n" { - _, err := reader.ReadString('\n') - sb.WriteString("$yqDocSeparator$\n") - if errors.Is(err, io.EOF) { - return reader, sb.String(), nil - } else if err != nil { - return reader, sb.String(), err + } else if strings.HasPrefix(peekString, "---\n") { // Document separator "---\n" + _, readErr := reader.ReadString('\n') // Consume "---\n" + sb.WriteString(docSeparatorPlaceholder + "\n") + if readErr != nil { + if errors.Is(readErr, io.EOF) { + return reader, sb.String(), nil + } + return reader, sb.String(), readErr } - } else if commentLineRegEx.MatchString(string(peekBytes)) || yamlDirectiveLineRegEx.MatchString(string(peekBytes)) { - line, err := reader.ReadString('\n') + } else if commentLineRegEx.MatchString(peekString) || yamlDirectiveLineRegEx.MatchString(peekString) { + // Line is a comment or a YAML directive. + line, readErr := reader.ReadString('\n') sb.WriteString(line) - if errors.Is(err, io.EOF) { - return reader, sb.String(), nil - } else if err != nil { - return reader, sb.String(), err + if readErr != nil { + if errors.Is(readErr, io.EOF) { + return reader, sb.String(), nil + } + return reader, sb.String(), readErr } } else { + // Line does not match any leading content pattern, so actual YAML content begins. return reader, sb.String(), nil } } } +// Init initializes the decoder with a new reader. +// It handles leading content preprocessing according to preferences. func (dec *goccyYamlDecoder) Init(reader io.Reader) error { readerToUse := reader - leadingContent := "" - dec.bufferRead = bytes.Buffer{} + processedLeadingContent := "" + dec.bufferRead = bytes.Buffer{} // Reset buffer var err error - // if we 'evaluating together' - we only process the leading content - // of the first file - this ensures comments from subsequent files are - // merged together correctly. + if dec.prefs.LeadingContentPreProcessing && (!dec.prefs.EvaluateTogether || dec.firstFile) { - readerToUse, leadingContent, err = dec.processReadStream(bufio.NewReader(reader)) + // Only process leading content for the first file if 'evaluateTogether' is active. + readerToUse, processedLeadingContent, err = dec.processReadStream(bufio.NewReader(reader)) if err != nil { return err } } else if !dec.prefs.LeadingContentPreProcessing { - // if we're not process the leading content - // keep a copy of what we've read. This is incase its a - // doc with only comments - the decoder will return nothing - // then we can read the comments from bufferRead + // If not preprocessing, TeeReader captures initial bytes in case it's a comment-only document. readerToUse = io.TeeReader(reader, &dec.bufferRead) } - dec.leadingContent = leadingContent + + dec.leadingContent = processedLeadingContent dec.readAnything = false - dec.cm = yaml.CommentMap{} + dec.cm = yaml.CommentMap{} // Reset comment map for the new stream. dec.decoder = *yaml.NewDecoder(readerToUse, yaml.CommentToMap(dec.cm), yaml.UseOrderedMap()) - dec.firstFile = false - dec.documentIndex = 0 - dec.anchorMap = make(map[string]*CandidateNode) + dec.firstFile = false // Subsequent Init calls are not for the 'firstFile' in 'evaluateTogether' context. + dec.documentIndex = 0 // Reset document index for the new stream. + dec.anchorMap = make(map[string]*CandidateNode) // Reset anchor map for the new stream. return nil } +// Decode reads the next YAML document from the stream and decodes it into a CandidateNode. +// It handles EOF conditions and the synthesis of nodes for comment-only content. func (dec *goccyYamlDecoder) Decode() (*CandidateNode, error) { var astNode ast.Node err := dec.decoder.Decode(&astNode) - if errors.Is(err, io.EOF) && dec.leadingContent != "" && !dec.readAnything { - // force returning an empty node with a comment. - dec.readAnything = true - return dec.blankNodeWithComment(), nil - } else if errors.Is(err, io.EOF) && !dec.prefs.LeadingContentPreProcessing && !dec.readAnything { - // didn't find any yaml, - // check the tee buffer, maybe there were comments - dec.readAnything = true - dec.leadingContent = dec.bufferRead.String() - if dec.leadingContent != "" { - return dec.blankNodeWithComment(), nil + if err != nil { // An error occurred (could be EOF). + if errors.Is(err, io.EOF) { + // Handle EOF: potentially return a comment-only node, or propagate EOF. + // Case 1: Leading content was processed, and nothing else has been successfully decoded from this stream yet. + if dec.leadingContent != "" && !dec.readAnything { + dec.readAnything = true // Mark that we are consuming the leadingContent as a node. + return dec.blankNodeWithComment(), nil + } + // Case 2: Leading content preprocessing was disabled, nothing successfully decoded yet, + // so check the TeeReader buffer for comments. + if !dec.prefs.LeadingContentPreProcessing && !dec.readAnything { + dec.readAnything = true // Mark that we are attempting to consume buffered content. + dec.leadingContent = dec.bufferRead.String() // Transfer buffered content to leadingContent. + if dec.leadingContent != "" { + return dec.blankNodeWithComment(), nil + } + } + // If neither of the above conditions for a comment-only node is met, it's a genuine EOF. + return nil, io.EOF } - return nil, err - } else if err != nil { + // Non-EOF error. return nil, err } + // No error from dec.decoder.Decode(&astNode) means a successful decode of a YAML structure. + dec.readAnything = true + candidateNode := &CandidateNode{document: dec.documentIndex} - if err := candidateNode.UnmarshalGoccyYAML(astNode, dec.cm, dec.anchorMap); err != nil { - return nil, err + if errUnmarshal := candidateNode.UnmarshalGoccyYAML(astNode, dec.cm, dec.anchorMap); errUnmarshal != nil { + return nil, errUnmarshal } + // If there was leading content processed before this node, attach it. if dec.leadingContent != "" { candidateNode.LeadingContent = dec.leadingContent - dec.leadingContent = "" + dec.leadingContent = "" // Clear after attaching. } - dec.readAnything = true + dec.documentIndex++ return candidateNode, nil } +// blankNodeWithComment creates an empty scalar node and attaches the current leadingContent as its comment. +// This is used for documents that only contain comments or directives. func (dec *goccyYamlDecoder) blankNodeWithComment() *CandidateNode { - node := createScalarNode(nil, "") + node := createScalarNode(nil, "") // Create an empty scalar node. node.LeadingContent = dec.leadingContent + // dec.leadingContent = "" // Clear after use? Not strictly necessary here as Decode will clear it next if node is returned. return node } diff --git a/pkg/yqlib/encoder_goccy_yaml.go b/pkg/yqlib/encoder_goccy_yaml.go index cc26014d..6fa00e16 100644 --- a/pkg/yqlib/encoder_goccy_yaml.go +++ b/pkg/yqlib/encoder_goccy_yaml.go @@ -12,6 +12,9 @@ import ( yaml "github.com/goccy/go-yaml" ) +// Note: The constant 'docSeparatorPlaceholder' is defined in decoder_goccy_yaml.go +// and is used here as they are in the same package. + type goccyYamlEncoder struct { prefs YamlPreferences } @@ -26,7 +29,7 @@ func (ye *goccyYamlEncoder) CanHandleAliases() bool { func (ye *goccyYamlEncoder) PrintDocumentSeparator(writer io.Writer) error { if ye.prefs.PrintDocSeparators { - log.Debug("writing doc sep") + // log.Debug("writing doc sep") // Commented out if err := writeString(writer, "---\n"); err != nil { return err } @@ -44,7 +47,7 @@ func (ye *goccyYamlEncoder) PrintLeadingContent(writer io.Writer, content string if errReading != nil && !errors.Is(errReading, io.EOF) { return errReading } - if strings.Contains(readline, "$yqDocSeparator$") { + if strings.Contains(readline, docSeparatorPlaceholder) { // Use the constant from decoder_goccy_yaml.go if err := ye.PrintDocumentSeparator(writer); err != nil { return err } @@ -74,8 +77,26 @@ func (ye *goccyYamlEncoder) PrintLeadingContent(writer io.Writer, content string return nil } +// PrintTrailingComment writes the given comment string to the writer. +// If the comment is not empty and does not end with a newline, one is added. +// It does not add any comment prefixes (#). +func (ye *goccyYamlEncoder) PrintTrailingComment(writer io.Writer, comment string) error { + if comment == "" { + return nil + } + if err := writeString(writer, comment); err != nil { + return err + } + if !strings.HasSuffix(comment, "\n") { + if err := writeString(writer, "\n"); err != nil { + return err + } + } + return nil +} + func (ye *goccyYamlEncoder) Encode(writer io.Writer, node *CandidateNode) error { - log.Debug("goccyYamlEncoder - going to print %v", NodeToString(node)) + // log.Debug("goccyYamlEncoder - going to print %v", NodeToString(node)) // Commented out if node.Kind == ScalarNode && ye.prefs.UnwrapScalar { valueToPrint := node.Value if node.LeadingContent == "" || valueToPrint != "" { @@ -103,20 +124,15 @@ func (ye *goccyYamlEncoder) Encode(writer io.Writer, node *CandidateNode) error return err } - trailingContent := node.FootComment - if target != nil { - // Store and clear foot comment to handle it separately - if astNode, ok := target.(interface{ GetComment() interface{} }); ok { - _ = astNode // placeholder for potential future foot comment handling - } - } - if err := encoder.Encode(target); err != nil { return err } - if err := ye.PrintLeadingContent(destination, trailingContent); err != nil { - return err + // Use PrintTrailingComment for foot comments. + if node.FootComment != "" { // Check if there's a foot comment to print + if err := ye.PrintTrailingComment(destination, node.FootComment); err != nil { + return err + } } if ye.prefs.ColorsEnabled { diff --git a/pkg/yqlib/lib.go b/pkg/yqlib/lib.go index 4460819a..d2e7c6e3 100644 --- a/pkg/yqlib/lib.go +++ b/pkg/yqlib/lib.go @@ -116,33 +116,110 @@ func parseSnippet(value string) (*CandidateNode, error) { } func recursiveNodeEqual(lhs *CandidateNode, rhs *CandidateNode) bool { - if lhs.Kind != rhs.Kind { + if lhs == nil && rhs == nil { + return true + } + if lhs == nil || rhs == nil { + // If one is nil, the other must also effectively be nil (e.g. an alias to nothing that resolved to nil, or a null scalar) + // This check is a bit simplistic, as a non-nil node could be a ScalarNode Tag:!!null. + // The detailed checks later will handle specific null-equivalence better. + // For now, if one is a Go nil and the other isn't, they are different. + log.Debugf("recursiveNodeEqual: one node is nil, the other is not. LHS: %v, RHS: %v", lhs == nil, rhs == nil) return false } - if lhs.Kind == ScalarNode { - //process custom tags of scalar nodes. - //dont worry about matching tags of maps or arrays. + // Phase 1: Resolve aliases if one node is an alias and the other is not. + // This logic now assumes that lhs.Alias.Alias (if lhs is AliasNode) + // has been resolved to a *CandidateNode during initial parsing. + if lhs.Kind == AliasNode && rhs.Kind != AliasNode { + if lhs.Alias == nil { // lhs.Alias is the *yaml.v3.Node representing the alias itself + log.Debugf("recursiveNodeEqual: lhs is AliasNode but its *yaml.v3.Node (lhs.Alias) is nil. LHS: %s", NodeToString(lhs)) + return rhs.Kind == ScalarNode && rhs.Tag == "!!null" + } + // According to linter, lhs.Alias.Alias is *CandidateNode. + // Standard yaml.v3 has lhs.Alias.Alias as *yaml.v3.Node (target). + // We are trusting the linter's view of the effective type in this specific codebase. + lhsResolvedCandidate := lhs.Alias.Alias // Assuming this is effectively a *CandidateNode + if lhsResolvedCandidate == nil { + log.Debugf("recursiveNodeEqual: lhs is AliasNode and its resolved target (*CandidateNode lhs.Alias.Alias) is nil. LHS: %s", NodeToString(lhs)) + return rhs.Kind == ScalarNode && rhs.Tag == "!!null" + } + // The type assertion is to make it explicit if the linter's view is correct. + // If it panics, the assumption about pre-resolution to *CandidateNode is wrong. + return recursiveNodeEqual(lhsResolvedCandidate, rhs) + } - lhsTag := lhs.guessTagFromCustomType() - rhsTag := rhs.guessTagFromCustomType() + if rhs.Kind == AliasNode && lhs.Kind != AliasNode { + if rhs.Alias == nil { + log.Debugf("recursiveNodeEqual: rhs is AliasNode but its *yaml.v3.Node (rhs.Alias) is nil. RHS: %s", NodeToString(rhs)) + return lhs.Kind == ScalarNode && lhs.Tag == "!!null" + } + rhsResolvedCandidate := rhs.Alias.Alias + if rhsResolvedCandidate == nil { + log.Debugf("recursiveNodeEqual: rhs is AliasNode and its resolved target (*CandidateNode rhs.Alias.Alias) is nil. RHS: %s", NodeToString(rhs)) + return lhs.Kind == ScalarNode && lhs.Tag == "!!null" + } + return recursiveNodeEqual(lhs, rhsResolvedCandidate) + } - if lhsTag != rhsTag { + if lhs.Kind != rhs.Kind { + log.Debugf("recursiveNodeEqual: kinds differ after alias check. LHS: %s (%s), RHS: %s (%s)", KindString(lhs.Kind), NodeToString(lhs), KindString(rhs.Kind), NodeToString(rhs)) + return false + } + + switch lhs.Kind { + case AliasNode: // Both are AliasNodes + if lhs.Alias == nil { // Both nil means equal + return rhs.Alias == nil + } + if rhs.Alias == nil { // LHS not nil, RHS nil means unequal return false } - } + // Both have non-nil yaml.v3 Alias nodes. Compare their targets. + lhsResolvedCandidate := lhs.Alias.Alias + rhsResolvedCandidate := rhs.Alias.Alias - if lhs.Tag == "!!null" { - return true + if lhsResolvedCandidate == nil { // LHS target is nil + return rhsResolvedCandidate == nil // RHS target must also be nil + } + if rhsResolvedCandidate == nil { // RHS target is nil, LHS target not nil + return false + } + return recursiveNodeEqual(lhsResolvedCandidate, rhsResolvedCandidate) - } else if lhs.Kind == ScalarNode { + case ScalarNode: + lhsTag := lhs.guessTagFromCustomType() + rhsTag := rhs.guessTagFromCustomType() + if lhsTag != rhsTag { + isLHSStrLike := lhsTag == "!!str" || lhsTag == "" || lhsTag == "!" + isRHSStrLike := rhsTag == "!!str" || rhsTag == "" || rhsTag == "!" + isLHSNull := lhsTag == "!!null" + isRHSNull := rhsTag == "!!null" + if isLHSNull || isRHSNull { + if !(isLHSNull && isRHSNull) { + log.Debugf("recursiveNodeEqual: Scalar tags differ (nullness mismatch). LHS Tag: '%s' Val: '%s', RHS Tag: '%s' Val: '%s'", lhsTag, lhs.Value, rhsTag, rhs.Value) + return false + } + } else if !(isLHSStrLike && isRHSStrLike) { + log.Debugf("recursiveNodeEqual: Scalar tags differ (non-string, non-null mismatch). LHS Tag: '%s' Val: '%s', RHS Tag: '%s' Val: '%s'", lhsTag, lhs.Value, rhsTag, rhs.Value) + return false + } + } + if lhsTag == "!!null" { + return true + } return lhs.Value == rhs.Value - } else if lhs.Kind == SequenceNode { + + case SequenceNode: return recurseNodeArrayEqual(lhs, rhs) - } else if lhs.Kind == MappingNode { + + case MappingNode: return recurseNodeObjectEqual(lhs, rhs) + + default: + log.Debugf("recursiveNodeEqual: unhandled identical kinds: %s (%s)", KindString(lhs.Kind), NodeToString(lhs)) + return false } - return false } // yaml numbers can have underscores, be hex and octal encoded... diff --git a/pkg/yqlib/parser_comparison_test.go b/pkg/yqlib/parser_comparison_test.go index c96d65c1..df339493 100644 --- a/pkg/yqlib/parser_comparison_test.go +++ b/pkg/yqlib/parser_comparison_test.go @@ -54,6 +54,34 @@ config: This is a multiline string with line breaks and proper formatting +`, + `--- +# Anchors and Aliases +defaults: &defaults + adapter: postgres + host: localhost + +development: + <<: *defaults + database: myapp_dev + +test: + <<: *defaults + database: myapp_test +`, + `--- +# Multiple documents +doc: 1 +key: value1 +--- +doc: 2 +key: value2 +`, + `--- +# Flow style and tags +flow_map: {item1: val1, item2: val2} +flow_seq: [alpha, bravo, charlie] +custom_tag: !myTag data `, } @@ -68,23 +96,41 @@ func TestYamlParsersStructuralEquivalence(t *testing.T) { t.Run(fmt.Sprintf("Document_%d", i), func(t *testing.T) { // Test with yaml.v3 parser ConfiguredYamlPreferences.UseGoccyParser = false - v3Node := parseYamlDocument(t, yamlDoc) + v3Nodes := parseYamlDocument(t, yamlDoc) // Test with goccy parser ConfiguredYamlPreferences.UseGoccyParser = true - goccyNode := parseYamlDocument(t, yamlDoc) + goccyNodes := parseYamlDocument(t, yamlDoc) + + if len(v3Nodes) != len(goccyNodes) { + t.Errorf("Number of parsed documents differ between yaml.v3 (%d) and goccy (%d) for document %d", len(v3Nodes), len(goccyNodes), i) + // Log nodes for debugging if lengths differ significantly or one is empty + if len(v3Nodes) < 5 && len(goccyNodes) < 5 { // Avoid excessive logging for large diffs + for idx, node := range v3Nodes { + t.Logf("yaml.v3 result[%d]: %s", idx, NodeToString(node)) + } + for idx, node := range goccyNodes { + t.Logf("goccy result[%d]: %s", idx, NodeToString(node)) + } + } + return // No point comparing nodes if doc counts differ + } // Compare the parsed structures (not text output) - if !recursiveNodeEqual(v3Node, goccyNode) { - t.Errorf("Parsed structures differ between yaml.v3 and goccy for document %d", i) - t.Logf("yaml.v3 result: %s", NodeToString(v3Node)) - t.Logf("goccy result: %s", NodeToString(goccyNode)) + for j := range v3Nodes { + v3Node := v3Nodes[j] + goccyNode := goccyNodes[j] + if !recursiveNodeEqual(v3Node, goccyNode) { + t.Errorf("Parsed structures differ between yaml.v3 and goccy for document %d, sub-document %d", i, j) + t.Logf("yaml.v3 result: %s", NodeToString(v3Node)) + t.Logf("goccy result: %s", NodeToString(goccyNode)) + } } }) } } -func parseYamlDocument(t *testing.T, yamlDoc string) *CandidateNode { +func parseYamlDocument(t *testing.T, yamlDoc string) []*CandidateNode { decoder := YamlFormat.DecoderFactory() inputs, err := readDocuments(strings.NewReader(yamlDoc), "test.yml", 0, decoder) @@ -96,7 +142,11 @@ func parseYamlDocument(t *testing.T, yamlDoc string) *CandidateNode { t.Fatal("No documents found") } - return inputs.Front().Value.(*CandidateNode) + var nodes []*CandidateNode + for el := inputs.Front(); el != nil; el = el.Next() { + nodes = append(nodes, el.Value.(*CandidateNode)) + } + return nodes } func TestYamlParsersEquivalence(t *testing.T) { @@ -127,11 +177,32 @@ func TestYamlParsersEquivalence(t *testing.T) { t.Logf("goccy output:\n%s", goccyResult) // But both should be valid YAML that parses to the same structure - v3Reparse := parseYamlDocument(t, v3Result) - goccyReparse := parseYamlDocument(t, goccyResult) + v3ReparsedNodes := parseYamlDocument(t, v3Result) + goccyReparsedNodes := parseYamlDocument(t, goccyResult) - if !recursiveNodeEqual(v3Reparse, goccyReparse) { - t.Error("Reparsed structures differ - this indicates a semantic issue") + if len(v3ReparsedNodes) != len(goccyReparsedNodes) { + t.Errorf("Number of reparsed documents differ between yaml.v3 (%d) and goccy (%d) for document %d", len(v3ReparsedNodes), len(goccyReparsedNodes), i) + // Log nodes for debugging if lengths differ significantly + if len(v3ReparsedNodes) < 5 && len(goccyReparsedNodes) < 5 { // Avoid excessive logging + for idx, node := range v3ReparsedNodes { + t.Logf("yaml.v3 reparsed result[%d]: %s", idx, NodeToString(node)) + } + for idx, node := range goccyReparsedNodes { + t.Logf("goccy reparsed result[%d]: %s", idx, NodeToString(node)) + } + } + } else { + for j := range v3ReparsedNodes { + v3ReparseNode := v3ReparsedNodes[j] + goccyReparseNode := goccyReparsedNodes[j] + if !recursiveNodeEqualWithMergeHandling(v3ReparseNode, goccyReparseNode) { + t.Errorf("Reparsed structures differ for document %d, sub-document %d - this indicates a semantic issue", i, j) + // Force logging of nodes involved in the direct failing comparison + t.Logf("Failed comparison details:") + t.Logf("--- yaml.v3 reparsed node ---\n%s\nNode Structure:\n%s", NodeToString(v3ReparseNode), GetNodeStructure(v3ReparseNode, 0)) + t.Logf("--- goccy reparsed node ---\n%s\nNode Structure:\n%s", NodeToString(goccyReparseNode), GetNodeStructure(goccyReparseNode, 0)) + } + } } } }) @@ -151,15 +222,21 @@ func processYamlDocument(t *testing.T, yamlDoc string) string { t.Fatal("No documents found") } - node := inputs.Front().Value.(*CandidateNode) - - var output bytes.Buffer - err = encoder.Encode(&output, node) - if err != nil { - t.Fatalf("Failed to encode YAML: %v", err) + var allDocsOutput strings.Builder + for i, el := 0, inputs.Front(); el != nil; i, el = i+1, el.Next() { + node := el.Value.(*CandidateNode) + var singleDocOutput bytes.Buffer + err = encoder.Encode(&singleDocOutput, node) + if err != nil { + t.Fatalf("Failed to encode YAML document %d: %v", i, err) + } + if i > 0 { + allDocsOutput.WriteString("\n---\n") // Add separator for subsequent documents + } + allDocsOutput.Write(singleDocOutput.Bytes()) } - return output.String() + return allDocsOutput.String() } func TestGoccyParserBasicFunctionality(t *testing.T) { @@ -208,3 +285,158 @@ test: t.Error("Expected 'field3: true' in output") } } + +// Add a helper function to get detailed node structure for logging +func GetNodeStructure(node *CandidateNode, depth int) string { + if node == nil { + return strings.Repeat(" ", depth) + "\n" + } + indent := strings.Repeat(" ", depth) + var sb strings.Builder + sb.WriteString(fmt.Sprintf("%sKind: %s, Tag: '%s', Value: '%s', Path: %s\n", indent, KindString(node.Kind), node.Tag, node.Value, node.GetNicePath())) + if node.Alias != nil { + sb.WriteString(fmt.Sprintf("%s AliasToTag: '%s', AliasToValue: '%s'\n", indent, node.Alias.Tag, node.Alias.Value)) + if node.Alias.Alias != nil { + sb.WriteString(fmt.Sprintf("%s PointsToTag: '%s', PointsToValue: '%s'\n", indent, node.Alias.Alias.Tag, node.Alias.Alias.Value)) + } + } + if len(node.Content) > 0 { + sb.WriteString(fmt.Sprintf("%s Content (%d items):\n", indent, len(node.Content))) + for i, child := range node.Content { + sb.WriteString(fmt.Sprintf("%s [%d]:\n", indent, i)) + sb.WriteString(GetNodeStructure(child, depth+2)) + } + } + return sb.String() +} + +// recursiveNodeEqualWithMergeHandling compares two nodes but handles the case where +// one parser uses alias nodes for merge tags while the other expands them directly +func recursiveNodeEqualWithMergeHandling(lhs *CandidateNode, rhs *CandidateNode) bool { + return recursiveNodeEqualWithMergeHandlingHelper(lhs, rhs, lhs, rhs) +} + +// recursiveNodeEqualWithMergeHandlingHelper includes root nodes for alias resolution +func recursiveNodeEqualWithMergeHandlingHelper(lhs *CandidateNode, rhs *CandidateNode, lhsRoot *CandidateNode, rhsRoot *CandidateNode) bool { + if recursiveNodeEqual(lhs, rhs) { + return true + } + + // Special handling for merge tag differences between parsers + if lhs != nil && rhs != nil && lhs.Kind == MappingNode && rhs.Kind == MappingNode { + return compareMappingNodesWithMergeHandling(lhs, rhs, lhsRoot, rhsRoot) + } + + // Handle sequence nodes + if lhs != nil && rhs != nil && lhs.Kind == SequenceNode && rhs.Kind == SequenceNode { + if len(lhs.Content) != len(rhs.Content) { + return false + } + for i := range lhs.Content { + if !recursiveNodeEqualWithMergeHandlingHelper(lhs.Content[i], rhs.Content[i], lhsRoot, rhsRoot) { + return false + } + } + return true + } + + return false +} + +// compareMappingNodesWithMergeHandling compares mapping nodes and handles merge tag differences +func compareMappingNodesWithMergeHandling(lhs *CandidateNode, rhs *CandidateNode, lhsRoot *CandidateNode, rhsRoot *CandidateNode) bool { + // Create maps of key-value pairs for comparison + lhsMap := make(map[string]*CandidateNode) + rhsMap := make(map[string]*CandidateNode) + + // Helper function to find alias target in the root mapping + findAliasTarget := func(rootNode *CandidateNode, aliasName string) *CandidateNode { + for i := 0; i < len(rootNode.Content); i += 2 { + if rootNode.Content[i].Value == aliasName && rootNode.Content[i+1].Kind == MappingNode { + return rootNode.Content[i+1] + } + } + return nil + } + + // Parse lhs content + for i := 0; i < len(lhs.Content); i += 2 { + key := lhs.Content[i].Value + value := lhs.Content[i+1] + + if key == "<<" && value.Kind == AliasNode { + // This is a merge tag with alias - find the target node + aliasNode := findAliasTarget(lhsRoot, value.Value) + if aliasNode != nil { + // Add each key-value pair from the alias + for k := 0; k < len(aliasNode.Content); k += 2 { + aliasKey := aliasNode.Content[k].Value + aliasValue := aliasNode.Content[k+1] + if _, exists := lhsMap[aliasKey]; !exists { + lhsMap[aliasKey] = aliasValue + } + } + } + } else if key == "<<" && value.Kind == MappingNode { + // This is a merge tag with expanded content + for j := 0; j < len(value.Content); j += 2 { + mergeKey := value.Content[j].Value + mergeValue := value.Content[j+1] + if _, exists := lhsMap[mergeKey]; !exists { + lhsMap[mergeKey] = mergeValue + } + } + } else { + lhsMap[key] = value + } + } + + // Parse rhs content + for i := 0; i < len(rhs.Content); i += 2 { + key := rhs.Content[i].Value + value := rhs.Content[i+1] + + if key == "<<" && value.Kind == AliasNode { + // This is a merge tag with alias - find the target node + aliasNode := findAliasTarget(rhsRoot, value.Value) + if aliasNode != nil { + // Add each key-value pair from the alias + for k := 0; k < len(aliasNode.Content); k += 2 { + aliasKey := aliasNode.Content[k].Value + aliasValue := aliasNode.Content[k+1] + if _, exists := rhsMap[aliasKey]; !exists { + rhsMap[aliasKey] = aliasValue + } + } + } + } else if key == "<<" && value.Kind == MappingNode { + // This is a merge tag with expanded content + for j := 0; j < len(value.Content); j += 2 { + mergeKey := value.Content[j].Value + mergeValue := value.Content[j+1] + if _, exists := rhsMap[mergeKey]; !exists { + rhsMap[mergeKey] = mergeValue + } + } + } else { + rhsMap[key] = value + } + } + + // Compare the flattened maps + if len(lhsMap) != len(rhsMap) { + return false + } + + for key, lhsValue := range lhsMap { + rhsValue, exists := rhsMap[key] + if !exists { + return false + } + if !recursiveNodeEqualWithMergeHandlingHelper(lhsValue, rhsValue, lhsRoot, rhsRoot) { + return false + } + } + + return true +}