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

Fix enum hash equality #390

Merged
merged 1 commit into from
Nov 13, 2018
Merged
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
19 changes: 19 additions & 0 deletions lib/protobuf/enum.rb
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,25 @@ def class
tag.class
end

# Protobuf::Enum delegates methods to Fixnum, which has a custom hash equality method (`eql?`)
# This causes enum values to be equivalent when using `==`, `===`, `equals?`, but not `eql?`**:
#
# 2.3.7 :002 > ::Test::EnumTestType::ZERO.eql?(::Test::EnumTestType::ZERO)
# => false
#
# However, they are equilvalent to their tag value:
#
# 2.3.7 :002 > ::Test::EnumTestType::ZERO.eql?(::Test::EnumTestType::ZERO.tag)
# => true
#
# **The implementation changed in Ruby 2.5, so this only affects Ruby versions less than v2.4.
#
# Use the hash equality implementation from Object#eql?, which is equivalent to == instead.
#
def eql?(other)
self == other
end

def inspect
"\#<Protobuf::Enum(#{parent_class})::#{name}=#{tag}>"
end
Expand Down
14 changes: 14 additions & 0 deletions spec/lib/protobuf/enum_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,20 @@
specify { subject.try { |yielded| expect(yielded).to eq(subject) } }
end

describe '#eql?' do
it "is equal to itself" do
expect(::Test::EnumTestType::ZERO.eql?(::Test::EnumTestType::ZERO)).to be(true)
end

it "is equal to it's tag" do
expect(::Test::EnumTestType::ZERO.eql?(::Test::EnumTestType::ZERO.tag)).to be(true)
end

it "is not equal to it's name" do
expect(::Test::EnumTestType::ZERO.eql?(::Test::EnumTestType::ZERO.name)).to be(false)
end
end

context 'when coercing from enum' do
subject { Test::StatusType::PENDING }
it { is_expected.to eq(0) }
Expand Down