Enable "toggle block style" refactor when there's no selection

[rolandcrosby-columntax]

Motivation

The "toggle block style" refactor action currently only works if there is a selection in the editor which covers a call node and its associated block. When there's no selection but the cursor is inside a block, it would be nice to be able to toggle the style of the block that the cursor is inside.

Implementation

• In the CodeActions request: add a quick check using @document.locate_node to conditionally offer the "toggle block style" action if the cursor is inside a BlockNode
• In the CodeActionResolve request: add a similar call to find the current block node when there's no selection. The actual rewrite also needs the CallNode associated with the block, and I couldn't figure out a good way to do this without adding a second RubyDocument.locate call. (When you call locate_node to get the BlockNode, you don't get any reference to the CallNode that the BlockNode is attached to. You also can't just look for a CallNode covering the cursor - that could return a CallNode that's actually inside the block that you want to toggle.)

Automated Tests

Added some expectation tests:

• verify that the code action is offered when the cursor is inside a block and isn't offered when the cursor is outside a block
• verify that the rewrite action toggles the expected block(s) depending on the cursor position

Manual Tests

• Place your cursor outside a block and trigger code actions => none should be available.
• Place your cursor inside a block and trigger code actions => "Refactor: Toggle block style" should be available, and should toggle the style of the block that the cursor is inside (and any nested blocks)

[graphite-app]

How to use the Graphite Merge Queue

Add the label graphite-merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

[rolandcrosby-columntax]

I have signed the CLA!

[vinistock]

Thanks for the PR, this is great. Let's just ensure we're not traversing the AST multiple times for discovering code actions and we should be good to ship

[vinistock]

Locating nodes is expensive because it needs to traverse the AST. Let's start locating the node under the cursor with no node_types filtering and then we can pass the node to this method and attribute_actions.

That way, both can check what type of node is under the cursor without traversing twice.

[rolandcrosby-columntax]

Hmm, that could be slightly complicated, because the two calls to locate have different node_types filters. attribute_actions is trying to find INSTANCE_VARIABLE_NODES covering the cursor, while toggle_block_style_action is looking for BlockNodes. Additionally, in attribute_actions, there are two different cases where we call locate with the range start position: (a) the case where there's no selection, and (b) the case where there's nothing fully contained within the selection that matches INSTANCE_VARIABLE_NODES.

Since locate happens to build an array of ClassNode, DefNode, BlockNode, etc. in the NodeContext's @nesting_nodes, we could potentially reuse the attribute_actions locate result for toggle_block_style_action as follows:

• Add attr_reader :nesting_nodes to NodeContext
• In CodeActions#perform, if there's no selection, call @document.locate_node(@range[:start], node_types: INSTANCE_VARIABLE_NODES) (the filter that we need in attribute_actions)
• Pass the returned NodeContext to attribute_actions and toggle_block_style_action as a nilable param
• In toggle_block_style_action, if that param is provided, only return the toggle action if the context's nesting_nodes contains a BlockNode
• In the if node.nil? branch of attribute_actions, use the NodeContext passed by the caller if it's present; if not, we're in case (b) above and need to invoke locate

This optimization would only work because the conditional actions only care about these two kinds of nodes; if additional conditional code actions were added which needed to match other node types, we'd be back where we started and would need multiple locate calls. What do you think we should do?