refactor: Remove unnecessary blocks when cleaning up unused unsafe

[randomPoison]

• Fixes Remove unnecessary blocks when cleaning up unused unsafe #1596.

The deleted parse_extras function had logic for mapping the file-local comment positions to their global positions, which I also needed to do here. I merged that logic into collect_comments since in both places where we're calling it we want to do the same local-to-global remapping.

[kkysen]

Deleting comments doesn't seem good.

[randomPoison]

Agreed, but I there are some deeper issues in how the refactorer handles comments that make me hesitant to try to address it in this PR.

The easiest way to handle comments is to track them based on the AST nodes that they're attached to, and with a trailing comment in a block like this there's no AST node for it to attach to. When we have a trailing comment like this, any rewriting of the block causes us to lose the comment, e.g. just removing the unsafe keyword but otherwise leaving the block the same will also delete any trailing comment. There's also a bug in how we associate comments with AST nodes which causes us to associate a trailing comment with a node that comes AFTER the block, which causes other bad interactions with the logic in this PR.

There's also a broader problem with this approach of associating comments with AST nodes, in that comments that appear in weird positions don't always have a corresponding node. For example, unsafe /* comment */ {}, where the comment appears between the unsafe keyword and the block, can't easily be handled this way. Cases like those are generally not idiomatic anyway, so I don't think it's worth putting effort into handing this better, but it highlights that we can't really handle comments in all cases (at least not without deeper fixes in the refactorer).

I'm writing up a ticket about comment handling in general, but for this PR I'd prefer to keep the behavior as-is, and leave it documented in the test that we have this unfortunate edge case.

[kkysen]

Are we able to get the span of the source and just manually search/re-lex for a comment?

[randomPoison]

Yeah, we can check the spans to at least detect the presence of a comment that's not attached to anything. I slapped together a prototype of it on another branch: legare/remove-blocks...legare/remove-blocks-but-handle-comments

But that feels very hacky to me, and this isn't the only place where this problem comes up. For example, the convert math/exits transforms can also eat comments in a similar way:

Examples of convert_math_funcs eating comments

let _ = sin(/* comment */ 1.0);

Becomes

let _ = 1.0.sin();

And

let _ = sin(
    // comment
    1.0
    // trailing comment
);

becomes

let _ = // comment
    1.0.sin();

It's not ideal, but for now I think the practical thing to do is accept that we can't always preserve comments in all cases. In general any attempt to rewrite a chunk of the AST is going to potentially eat comments. I think the best way to handle this (at least for now) is to leave it to a post-transform review pass (by a human or an LLM) to identify lost comments and restore them to an appropriate place.

[randomPoison]

Issue for tracking weird comments like this: #1641