Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Aliased Matchers from Overriding Expected Data #1116

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 44 additions & 3 deletions lib/rspec/matchers/aliased_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def description
#
# @api private
def failure_message
@description_block.call(super)
@description_block.call(super, true)
end

# Provides the failure_message_when_negated of the aliased matcher. Aliased matchers
Expand All @@ -54,7 +54,7 @@ def failure_message
#
# @api private
def failure_message_when_negated
@description_block.call(super)
@description_block.call(super, true)
end
end

Expand Down Expand Up @@ -105,12 +105,53 @@ def failure_message_when_negated
def optimal_failure_message(same, inverted)
if DefaultFailureMessages.has_default_failure_messages?(@base_matcher)
base_message = @base_matcher.__send__(same)
overriden = @description_block.call(base_message)
overriden = @description_block.call(base_message, true)
return overriden if overriden != base_message
end

@base_matcher.__send__(inverted)
end
end

# Wrapper around an overriden description for an aliased
# matcher.
#
# @api private
class AliasDescription
def initialize(new_name, old_name, override_block)
@new_name = new_name
@old_name = old_name
@override_block = override_block
end

# Creates the overriden description.
#
# @api private
def call(old_desc, replace_after_start=false)
if @override_block
@override_block.call(old_desc)
else
default_description(old_desc, replace_after_start)
end
end

private

# Generates a default message when no block is provided
# when aliasing the matcher. Replaces instances of the old
# matcher's name with the new matcher's name where
# applicable.
#
# #api private
def default_description(old_desc, replace_after_start)
old_name_split = EnglishPhrasing.split_words(@old_name)

if old_desc.start_with?(old_name_split) || replace_after_start
old_desc.sub(old_name_split, EnglishPhrasing.split_words(@new_name))
else
old_desc
end
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

end
end
6 changes: 2 additions & 4 deletions lib/rspec/matchers/dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,8 @@ module DSL
# logic. The yielded arg is the original description or failure message. If no
# block is provided, a default override is used based on the old and new names.
# @see RSpec::Matchers
def alias_matcher(new_name, old_name, options={}, &description_override)
description_override ||= lambda do |old_desc|
old_desc.gsub(EnglishPhrasing.split_words(old_name), EnglishPhrasing.split_words(new_name))
end
def alias_matcher(new_name, old_name, options={}, &description_block)
description_override = AliasDescription.new(new_name, old_name, description_block)
klass = options.fetch(:klass) { AliasedMatcher }

define_method(new_name) do |*args, &block|
Expand Down
12 changes: 11 additions & 1 deletion spec/rspec/matchers/aliased_matcher_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module RSpec
module Matchers
RSpec.describe AliasedMatcher do
let(:base_matcher_desc) { "my base matcher description" }
RSpec::Matchers.define :my_base_matcher do
match { |actual| actual == foo }

Expand All @@ -9,7 +10,7 @@ def foo
end

def description
"my base matcher description"
base_matcher_desc
end
end
RSpec::Matchers.alias_matcher :alias_of_my_base_matcher, :my_base_matcher
Expand Down Expand Up @@ -79,6 +80,15 @@ def description
expect(matcher.description).to eq("my blockless override description")
end

context "when the matcher's description starts with the matcher's name" do
let(:base_matcher_desc) { "my base matcher my base matcher" }

it 'only overrides the first instance of the name in the description' do
matcher = alias_of_my_base_matcher
expect(matcher.description).to eq("alias of my base matcher my base matcher")
end
end

it 'works properly with a chained method off a negated matcher' do
expect { }.to avoid_outputting.to_stdout

Expand Down
16 changes: 16 additions & 0 deletions spec/rspec/matchers/aliases_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,14 @@ module RSpec
).with_description('a string including "a"')
end

specify do
expect {
expect("some random string").to match(a_string_including("include"))
}.to fail_including(
'expected "some random string" to match (a string including "include")'
)
end

specify do
expect(
a_collection_including("a")
Expand All @@ -252,6 +260,14 @@ module RSpec
).with_description('a hash including {:a => 5}')
end

specify do
expect {
expect({}).to match(a_hash_including({:a => 'include'}))
}.to fail_including(
'expected {} to match (a hash including {:a => "include"}'
)
end

specify do
expect(
including(3)
Expand Down