Skip to content

Fix comment parsing on anchors and aliases#815

Open
joshuapare wants to merge 3 commits intogoccy:masterfrom
joshuapare:anchor-alias-comment-fix
Open

Fix comment parsing on anchors and aliases#815
joshuapare wants to merge 3 commits intogoccy:masterfrom
joshuapare:anchor-alias-comment-fix

Conversation

@joshuapare
Copy link
Copy Markdown

Fixes two bugs around alias parsing (discovered while fixing #608) that occurred when CommentToMap was enabled with comments present on alias lines:

  • alias lookup failed with "could not find alias" error because the lookup used the string representation (which included the comment) instead of the token value
  • comments on alias lines were attached to the alias name StringNode but weren't propagated to the AliasNode, so CommentToMap never extracted them

Example:

anchor: &anc value
alias: *anc # comment <--- this would throw an error of "could not find alias 'anc'" when CommentToMap was enabled

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 10, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 60.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.35%. Comparing base (b0ab069) to head (e790356).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #815      +/-   ##
==========================================
- Coverage   80.48%   80.35%   -0.13%     
==========================================
  Files          22       22              
  Lines        6845     6863      +18     
==========================================
+ Hits         5509     5515       +6     
- Misses        914      920       +6     
- Partials      422      428       +6     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joshuapare joshuapare force-pushed the anchor-alias-comment-fix branch from 60f75bd to 0faf12c Compare November 10, 2025 18:49
Copy link
Copy Markdown
Owner

@goccy goccy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution !!! I've commented

}{
{"$.root.anchor_value", []*yaml.Comment{yaml.LineComment(" anchor comment")}},
{"$.root.alias_value", []*yaml.Comment{yaml.LineComment(" alias comment")}},
{"$.root.alias_flow", []*yaml.Comment{yaml.LineComment(" alias flow comment")}},
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the test cases for anchor_flow and anchor_map

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the wait! This actually found a bug, sequence anchor ending comments weren't getting picked up. it's fixed now 👍

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes two bugs that occurred when CommentToMap was enabled with comments present on alias lines. The first bug caused alias lookup to fail because it used the string representation (which included comments) instead of the token value. The second bug prevented comments on alias lines from being propagated to the AliasNode, causing CommentToMap to miss them.

  • Fixed alias lookup to use token value instead of string representation
  • Added comment propagation from alias name to alias node
  • Added comprehensive test coverage for anchors and aliases with comments

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
decode.go Fixed alias lookup to use GetToken().Value instead of String() to avoid including comments in the lookup key
parser/parser.go Added comment propagation from alias name StringNode to AliasNode to ensure CommentToMap can extract alias comments
testdata/yaml_test.go Added test case covering anchors and aliases with comments for scalar values, flow sequences, and flow maps

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants