Traverse enter scope after enter_*, exit before exit_*

[overlookmotel]

Currently Traverse's timing of entering and exiting scopes is different for different node types. It depends on whether the scope covers all fields, or only a subset.

• If scope starts after some fields, scope is entered after enter_* and exited before exit_*.
• If scope covers whole node, scope is entered before enter_* and exited after exit_*.

This means enter_* and exit_* for any node are always called with same scope at top of the scope stack. Good.

But the fact that current scope in enter_* and exit_* is different depending on the node makes it confusing.

Change this to be consistent for all node types:

• Always enter the scope after enter_*.
• Always exit the scope before exit_*.

If they need it, enter_* / exit_* can get the node's ScopeId from the node's scope_id field.

[Dunqing]

Currently, I want to use current_node_id to do something in leave_scope. But leave_scope after leave_node. This means that I can't get the node id corresponding to the scope

ast-codegen also needs to follow this. @rzvxa cc

[overlookmotel]

I want to use current_node_id to do something in leave_scope. But leave_scope after leave_node.

That's a different issue from this. Traverse doesn't have enter_scope, leave_scope, enter_node or leave_node.

[rzvxa]

Currently, I want to use current_node_id to do something in leave_scope. But leave_scope after leave_node. This means that I can't get the node id corresponding to the scope

ast-codegen also needs to follow this. @rzvxa cc

Previously we were mostly visiting scope before nodes that's why I made that the default of our generated Rust code. But I have to say it makes more sense to fire off the scope events inside of node events.

Does this order work for you: enter_node -> enter_scope -> walk -> leave_scope -> leave_node If it does let me know so I can submit a PR for it.

[overlookmotel]

Note to self: When making this change to Traverse, will need to alter some of the transforms which rely on ctx.current_scope_id() being the scope of the current node - whereas after this change it'll be the parent.

Take care - no tests for scopes in transformer at present, so missing something could silently break things.

Probably should rename it parent_scope_id to reflect this.

[overlookmotel]

Closed in #4684.