Detect a pattern of two consequtive lines, anywhere.

[amomchilov]

I have a cop that aims to detect 2 consecutive (ignoring whitespace) lines that have this pattern:

def foo
  @foo = T.let(@foo, T.nilable(Foo))
  @foo ||= Foo.new
end

The current implementation works, but it only works if these two lines are the first thing in the method body. E.g. this won't be caught:

def foo
  some
  other
  code
  @foo = T.let(@foo, T.nilable(Foo))
  @foo ||= Foo.new
end

What would be the right way to match these two lines, and replace them without effecting other lines before/after them?

🧵 Shopify/rubocop-sorbet#162 (comment)

[dvandersluis]

I briefly looked at your PR. You're looking for a begin node, which is going to capture the entire body of the method in your example. As you discussed in the PR, you would need use ... to make the matcher match when there is extra code (likely before and after the nodes you're looking for), but this will still be matching the entire begin node.

You can use capturing within your pattern to return the exact child nodes within the begin that you are concerned about, and pass those into your correction rather than the parent begin node.

[dvandersluis]

Here's a node pattern example. The ivasgn node is capture 1 and the or_asgn node is capture 4.

[amomchilov]

you would need use ... to make the matcher match when there is extra code ... but this will still be matching the entire begin node.

Indeed, that was exactly the issue I was having (then auto-correction would replace all the lines, and clobber that extra code)

You can use capturing within your pattern to return the exact child nodes within the begin that you are concerned about, and pass those into your correction rather than the parent begin node.

And that's the solution!

I forgot that you can capture an entire subtree... which is funny, because my current node-matcher already captures an entire subtree 😅

[amomchilov]

Hey Daniel, I got pretty close! Here's my attempt: Shopify/rubocop-sorbet@cb802c9

There's just one issue with this, which is the surrounding indentation around the nodes:

sig { returns(T::Hash[Symbol, T.untyped]) }
def client_info_hash
  @client_info_hash ||= T.let(client_info.to_hash, T.nilable(T::Hash[Symbol, T.untyped]))
+  
end

Previously, I was replacing the entire begin node as a whole. I would fish out the relevant parts, and build up the replacement without any excess whitespace.

I can't replace the entire begin node anymore (obviously, since that'll clobber those previous unrelated lines, that was the whole issue). Now that I'm capturing the two child nodes directly, I try to replace the first line with the correction, and remove the second line. Since these captures don't capture any trailing/leading white space, it survives the replacement+removal of the subnodes.

I can fidget around with manually stripping the whitespace, but it feels like I'm missing something there.

Got any suggestions?

[dvandersluis]

range_by_whole_lines from RangeHelp will probably help you here. You should be able to expand your range to the entire line including the final new line, and give that to the corrector to remove.

[amomchilov]

That got me pretty close! It got rid of the trailing line, but I still needed to do some manual work to remove the blank lines between my two lines of interest.

What do you think?

Shopify/rubocop-sorbet@4b51532 (#162)