Prevent Aliased Matchers from Overriding Expected Data

[lnestor]

Previous Behavior

The aliased matcher implementation of a description currently replaces all instance of the old matcher's name in the new matcher's name. If it happens that the expected values include the old matcher's name, these also get overwritten. In the examples below, the string "include" is replaced in both the outputs by the new matcher's name.

it 'overwrites data for a_string_including' do
  my_string = "a string with 'include'"
  expect("some random string").to match(a_string_including(my_string))
end

# => expected "some random string" to match 
#       (a string including "a string with 'a string including'")

it 'overwrites data for a_hash_including' do
  my_hash = { my_string: "a string with 'include'" }
  expect({}).to match(a_hash_including(my_hash))
end

# => expected {} to match 
#       (a hash including {:my_string => "a string with 'a hash including'"})

New Behavior

This change makes it so it only replaces the first instance of the original name so none of the expected data is also overwritten. The two above error outputs would be the following:

expected {} to match (a hash including {:my_string => "a string with 'include'"})
expected "some random string" to match (a string including "a string with 'include'")

I saw this in rspec/rspec-rails#1835.

[JonRowe]

Thanks for tackling this, a bit of feedback above!

[JonRowe]

I think this needs to only match the start of the string, this might still accidentally override terms if the original description is not generated.

[lnestor]

Thanks for the feedback! Can you clarify this a bit? #sub matches the first occurrence, so are you saying something like matching the first n characters of the string?

Also, what do you mean by if the original description is not generated? Since BaseMatcher implements description, I would think that they all would have a description. Unless a class implements its own description which does not contain the name of the matcher.

[JonRowe]

Can you clarify this a bit? #sub matches the first occurrence, so are you saying something like matching the first n characters of the string?

"hello_old_name".sub("old_name","new_name") would produce "hello_new_name" but we don't want that. We only want to replace old_name as it was generated initially.

Also, what do you mean by if the original description is not generated? Since BaseMatcher implements description, I would think that they all would have a description.

Similarly it is possible for matchers to implement description themselves and it might mean they don't contain the old_name at all, in this case we shouldn't be replacing the first instance blindly as it might not be in the description but in the expected.

[JonRowe]

It'd prefer a practical example showing the problem using fail_with, even the one from rspec/rspec-rails#1835 would suffice.

[JonRowe]

This is a lot of added complication, and still doesn't remove the global sub in favour of a "start of string only" replace.

[pirj]

Do you have any questions on how to proceed with this @lnestor ?

[pirj]

@lnestor Are you interested in wrapping this up?