Prevent custom matcher is called twice without compound

[AlecksJohannes]

Hey lovely folks 👋, I hope you all are doing well

This PR here is to address #1426 which is to prevent a custom matcher from calling twice without compound like .and or .or

[JonRowe]

Thanks for starting to tackle this, the issue as I see it is somehow this method is being responded to by the matcher, this is what needs to be fixed .matcher().matcher should cause a natural NoMethodError e.g.

     NoMethodError:
       undefined method `eq' for #<RSpec::Matchers::BuiltIn::Eq:0x00000001111948f0 @expected="ok">

               expect(ok).to eq(ok).eq

[AlecksJohannes]

Thanks for starting to tackle this, the issue as I see it is somehow this method is being responded to by the matcher, this is what needs to be fixed .matcher().matcher should cause a natural NoMethodError e.g.

     NoMethodError:
       undefined method `eq' for #<RSpec::Matchers::BuiltIn::Eq:0x00000001111948f0 @expected="ok">

               expect(ok).to eq(ok).eq

Hello @JonRowe thank you, sure I can raise NoMethodError rather than using RSpec.warning. WDYT ?

P/S: #1428 (comment)

[JonRowe]

Hello @JonRowe thank you, sure I can raise NoMethodError rather than using RSpec.warning. WDYT ?

The fix should be to find where the method is being responded to and prevent it, I suspect its probably including a module it shouldn't do.

[AlecksJohannes]

Hello @JonRowe thank you, sure I can raise NoMethodError rather than using RSpec.warning. WDYT ?

The fix should be to find where the method is being responded to and prevent it, I suspect its probably including a module it shouldn't do.

Hey there JonRowe, sorry I have updated my comment above.

[AlecksJohannes]

I'll paste it here just to be sure

I believe I understand what you said and I agree with you too. I tried to do that but I am not sure about removing or adjusting this piece of code, I played around with our code before (the method was coming from include RSpec::Matchers in DSL::Matcher, I was able to extract it to different module) and make it to work exactly like you said but we need to deal with 👇

def respond_to_missing?(method, include_private=false)
   super || @matcher_execution_context.respond_to?(method, include_private)
end

def method_missing(method, *args, &block)
   if @matcher_execution_context.respond_to?(method)
      @matcher_execution_context.__send__ method, *args, &block
   else
       super(method, *args, &block)
   end
end

Otherwise, even calling matcher().matcher it will still work even though the method does not exist. Because of this blocker, I had to provide another solution

P/S: If we were somehow to adjust that method_missing I think you won't be able to call that custom matcher recursively which I think resulting in a breaking changes.

[pirj]

What concerns me the most, why are matchers available as methods on an i stance of a matcher in the first place?

[AlecksJohannes]

Hello there @pirj 👋 I am not really sure I am not sure (maybe it could be recursive?) if I should do like what Jon said because it can result in breaking changes

This is what I found:

I checked and the matcher available because of this

https://github.com/rspec/rspec-expectations/blob/main/lib/rspec/matchers/dsl.rb#L75 here we call define_method name in Matchers::DSL

and later in RSpec::Matchers we https://github.com/rspec/rspec-expectations/blob/main/lib/rspec/matchers.rb#L242 extend Matchers::DSL

And then in class RSpec::Matchers::DSL::Matcher class we include RSpec::Matchers which has that defined method again https://github.com/rspec/rspec-expectations/blob/main/lib/rspec/matchers/dsl.rb#L438. Because of this circular mixins causes that custom matcher method available like everywhere, I tried to extract the code into different module and include it but still it doesn't work due to 👇

def respond_to_missing?(method, include_private=false)
   super || @matcher_execution_context.respond_to?(method, include_private)
end

def method_missing(method, *args, &block)
   if @matcher_execution_context.respond_to?(method)
      @matcher_execution_context.__send__ method, *args, &block
   else
       super(method, *args, &block)
   end
end

Even though RSpec::Matchers::DSL::Matcher does not have such method but it will call the method from @matcher_execution_context because of method_missing logic