Skip to content

Migrate remove_dbgs from ted to SyntaxEditor #19267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ThouCheese
Copy link
Contributor

@ThouCheese ThouCheese commented Mar 2, 2025

Hi! I am trying to get started on migrating a couple of assists to the SyntaxEditor machinery. I am struggling ever so slightly with converting things however. One of the problems is that because I prepare all the changes into the SyntaxEditor, and only apply it later, I don't really know where to look when the apply-step results in an error.

Specifically, the tests are currently failing with assertion failed: is_ancestor_or_self_of_element(&old, &self.root), and with that I am leaving trailing semicolons in place when I am removing dbgs. Any help on this would me greatly appreciated :)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 2, 2025
@ThouCheese ThouCheese force-pushed the migrate-remove-dbgs branch 2 times, most recently from c7b3432 to bd9083a Compare March 2, 2025 18:37
@ThouCheese ThouCheese force-pushed the migrate-remove-dbgs branch from bd9083a to 2ba82fd Compare March 2, 2025 18:39
@DropDemBits
Copy link
Contributor

assertion failed: is_ancestor_or_self_of_element is raised whenever you're trying to replace/delete a node from a different syntax tree. I assume this is happening inside of replace_nested_dbgs because expanded eventually comes from input_expressions, which are newly created syntax trees that aren't part of the original source syntax tree.
https://github.com/ThouCheese/rust-analyzer/blob/2ba82fdd2401d5dc32b90af703394c663525fc42/crates/ide-assists/src/handlers/remove_dbg.rs#L78-L82
Instead, you'll likely need to create a separate SyntaxEditor inside of replace_nested_dbgs, and then finish() it to get the new_root() which contains the applied changes.

Regarding the semicolon, it seems that this is supposed to be deleting the entire ast::ExprStmt node instead of just the macro expr:
https://github.com/ThouCheese/rust-analyzer/blob/2ba82fdd2401d5dc32b90af703394c663525fc42/crates/ide-assists/src/handlers/remove_dbg.rs#L99-L107

@ThouCheese ThouCheese force-pushed the migrate-remove-dbgs branch from 9bb47c7 to bb78a84 Compare April 13, 2025 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants