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
[Ruby] Remove to_hash methods #6166
Conversation
The PR (#338) and commit (d1b52a0) that added the implicit coercion methods only mentions the explicit use-case, and this comment in particular makes me think the consequences of including the implicit methods was probably not fully understood and unintentional. |
Yes this appears to have been unintentional and not fully understood. I wasn't aware of this corner of Ruby and it's surprising to me that you can't pass two actual hashes to a function that takes optional named arguments. I agree that it does seem better overall to not implement |
Yes, users can add methods to the Protobuf resource classes to restore |
Matz has stated that the way named arguments were implemented was a mistake and this will be changed in Ruby 3. I assume he means this issue of unrolling the last positional Hash object and matching the named arguments. |
/cc @dazuma |
Ping. |
I was surprised to come across this because the documentation still states that |
The Ruby implementation creates the explicit coercion
#to_h
method and the implicit coercion#to_hash
method on Message and Map. Ruby treats objects that respond to implicit coercion methods differently. I do not think Protobuf intends that Message and Map are to be treated implicitly as hashes. In fact, I think this should actually be considered a bug.For example, here are how the explicit and implicit methods such as
#to_h
and#to_hash
are used:Similarily, Protobuf Messages and Maps are not Ruby Hashes. They can generate hash representations, but they are not themselves hashes. They should not be used in place of hashes.
What does the presence of the
#to_hash
method mean? It means Ruby will treat these objects as if they were hashes. Consider the following method:You would expect you could pass two Protobuf objects to the method, but you cannot.
This is because Ruby sees that the
blue
object has defined#to_hash
and treats it as a hash, and tries to match the optional named argument instead of the positional object.Because of this, and because there is no documentation that states that this method is intended to be part of the public API, I think these methods can and should be removed.