Complex predicates with arguments about rubocop-ast

[marcandre]

In the "What I would love" section, there's #{Expectations::ALL.node_pattern_union} which I assume you meant to write as :expect to correspond with the rest of the examples, right?

I think you simply have one too many () in your pattern...

def_node_matcher :expectation_with_return_block, <<~PATTERN
  #expectation(
    $(block #message_expectation? args _)
  )
PATTERN

Otherwise you are asking for a node with no children and a type that will match with expectaction...

That should work (can't test it now but could later if it doesn't work).

Just for completion's sake, another way to write it (I prefer yours though):

def_node_matcher :expectation, <<~PATTERN
  (send (send nil? :expect ...) :to $%with)
PATTERN

def_node_matcher :return_block, <<~PATTERN
  (block #message_expectation? args _)
PATTERN

def on_send
  if expectation(with: method(:return_block))
    # ...
end

[marcandre]

The expectation_with_configured_response do |a, b| actually only yields the capture that is declared in expectation_with_configured_response itself, not the one declared in expectation.
I don't really care about the first one, only about the one from expectation, but it doesn't yield it.

Yes, that's a limitation where patterns must know, at compile time, how many captures there are and where they come from. This is necessary for example so that the {} has the same number of captures no matter which branch was matched. So it is not possible to return captures from two node patterns at once. If you tried $#expectation, this would not capture the result of expectation, just the argument it was given.

That being said, we could consider a way to indicate that we want to capture the result of a method call, not it's argument, say #$expectation; that might be quite useful...

In this particular case, since the capture you really want is the expectation node pattern, then you need to call that directly. I think the other form I gave should work, and you can even capture both nodes:

        def_node_matcher :expectation, <<~PATTERN
          (send
            $(send nil? :expect ...) :to $% # expect(foo).to be_bop
          )
        PATTERN

        def_node_matcher :configured_response, <<~PATTERN
          (send #message_expectation? ... _)
        PATTERN

        def expectation_with_configured_response(node, &block)
          expectation(method(:configured_response), &block)
        end

I'm impressed by how the node pattern evolved for the last year, child matching, descendant matching, now this.

Thanks :-) I'm working on an actual lexer/parser/compiler for NodePattern that would allow a bit more flexibility and make it easier to debug

[pirj]

I think you simply have one too many () in your pattern...

You're right, thanks. I've never seen a node pattern without the outer braces of either kind, that confused me.

It kind of works, but the problem is different now (this code is slightly simplified):

        def_node_matcher :expectation, <<~PATTERN
          (send
            $(send nil? :expect ...) :to % # expect(foo).to be_bop
          )
        PATTERN

        def_node_matcher :expectation_with_configured_response, <<~PATTERN
          #expectation(
            $(send #message_expectation? ... _)
          )
        PATTERN

The expectation_with_configured_response do |a, b| actually only yields the capture that is declared in expectation_with_configured_response itself, not the one declared in expectation.
I don't really care about the first one, only about the one from expectation, but it doesn't yield it.

This is the commit that makes the change rubocop/rubocop-rspec@f7d813c
I've added captures to expectation_with_configured_response/expectation_with_return_block/expectation_with_blockpass_or_hash not because I need them, but to show that they are yielded, but not the one from expectation.

Again, if this is by design, I don't mind closing this.

Also, I'm impressed by how the node pattern evolved for the last year, child matching, descendant matching, now this. Thanks for moving it forward!

[pirj]

Thanks for the explanation, it all makes sense. Looking forward to the updates.

I'll close this for now, and will be looking at RuboCop core's cop improvements and will try to reflect those on RSpec-related cops.