Skip to content

feat(ast): support moving and copying nodes by path - #971

Open
SC123667 wants to merge 1 commit into
bytedance:mainfrom
SC123667:feature/ast-move-copy-path-732
Open

feat(ast): support moving and copying nodes by path#971
SC123667 wants to merge 1 commit into
bytedance:mainfrom
SC123667:feature/ast-move-copy-path-732

Conversation

@SC123667

Copy link
Copy Markdown

What type of PR is this?

feat

Check the PR title.

  • This PR title matches the format: (optional scope): .
  • The title is user-oriented and clear.
  • User-facing AST documentation is updated in this PR.

(Optional) Translate the PR title into Chinese.

feat(ast): 支持按路径移动和复制节点

(Optional) More detailed description for this PR(en: English/zh: Chinese).

en:

  • Add ast.Path, Node.MovePath, and Node.CopyPath for reshaping JSON trees at arbitrary depth.
  • Treat an empty path as the root and create missing object/array destination parents from path component types.
  • Fill missing array slots with null, preserve existing in-array move/reorder semantics, and reject moves into a source descendant.
  • Serialize a copied subtree into a detached lazy node so later source and destination mutations do not alias.
  • Keep empty-source-ancestor pruning explicit; this API does not delete unrelated empty parents automatically.
  • Document the API and cover the practical reshape/rename scenarios from ast.Node should support move and/or copy at any depth of the tree; not just within arrays #732.

Validation (Go 1.25.12, macOS arm64):

  • go test -race across the package set used by the upstream ARM CI workflow.
  • (cd issue_test && go test -race ./...).
  • SONIC_USE_OPTDEC=1 SONIC_USE_FASTMAP=1 SONIC_ENCODER_USE_VM=1 go test ./ast -count=1.

zh:

  • 新增任意深度节点移动/复制 API;支持根路径、自动创建目标父路径、数组补 null、同数组重排与复制隔离。

(Optional) Which issue(s) this PR fixes:

Fixes #732

(optional) The PR that updates user documentation:

The in-repository AST README section is updated in this PR.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a206fdd47b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ast/node.go
Comment on lines +1065 to +1068
moved := *source
if err := self.setByPath(dst, moved); err != nil {
return err
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve nested destinations when removing an earlier array item

When the source is an array element and the destination is nested beneath a later element of that same array, writing the destination before deleting the source shifts the destination to a different index. For example, moving path {0} to {1, "x"} in ["a", {}] returns success but produces [{"x":"a"}], where the requested destination path no longer exists. The move must account for the index shift or resolve/create the destination after removing the earlier source element.

Useful? React with 👍 / 👎.

@codecov-commenter

codecov-commenter commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 64.31373% with 91 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.53%. Comparing base (59be92f) to head (a206fdd).
⚠️ Report is 86 commits behind head on main.

Files with missing lines Patch % Lines
ast/node.go 64.31% 90 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #971      +/-   ##
==========================================
+ Coverage   51.86%   53.53%   +1.66%     
==========================================
  Files         127      171      +44     
  Lines       10893    14401    +3508     
==========================================
+ Hits         5650     7710    +2060     
- Misses       4920     6595    +1675     
+ Partials      323       96     -227     
Flag Coverage Δ
arm 45.83% <64.31%> (?)
macos-15 47.62% <74.12%> (?)
ubuntu-24.04-arm 45.83% <64.31%> (?)
ubuntu-latest 52.42% <74.12%> (?)
x86 52.42% <74.12%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ast.Node should support move and/or copy at any depth of the tree; not just within arrays

3 participants