Warn when submitting hashes with same key as string and symbol to AR methods?

[Andrew Selder]

Hi all,

After we got bit by it a couple times recently, I was thinking of
submitting a PR for ActiveRecord that would issue a warning if you
submit a hash containing the same key as both a symbol and a string to
any of the AR construction methods (create, new, update_attributes,
etc…) . As far as I can tell, the behavior when this happens in
undefined (and change between Rails 3 and 4 for example).

For example:

attr = { ‘a’ => 123, :a => 456 }
ARModel.create!(attr)

In Rails 3, you’ll get an object with the ‘a’ attributes set to
456, in Rails 4, you’ll get 123.

I think this kind of things is almost always unintentional and worth a
warning.

I’ll be happy to code this up and submit it if people think it would
be useful and accepted.

Please let me know your thoughts and feedback.

Thanks,

Andrew

[Andrew Kaspick]

I'd label this purely as undefined behavior and such cases shouldn't occur in the first place.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

[Andrew Selder]

Andrew,

I agree they shouldn’t happen in the first place, but unfortunately
they crop up in the real world, you get a hash from a JSON object and
mix that with some params, and you might do this without even realizing
it.

This is why I propose that AR spit out a warning message (perhaps
upgraded to an error in future versions) if this happens.

Andrew

[richard schneeman]

It's going to be really expensive to check that on every call. Maybe it could be a good feature for a gem you only use in development.

---
Richard Schneeman
http://www.schneems.com

On Thu, Mar 24, 2016 at 4:22 PM -0700, "Andrew Selder" <andrew...@gmail.com> wrote:

Andrew,

I agree they shouldn’t happen in the first place, but unfortunately 
they crop up in the real world, you get a hash from a JSON object and 
mix that with some params, and you might do this without even realizing 
it.

This is why I propose that AR spit out a warning message (perhaps 
upgraded to an error in future versions) if this happens.

Andrew

On 24 Mar 2016, at 16:09, Andrew Kaspick wrote:

> I'd label this purely as undefined behavior and such cases shouldn't 
> occur
> in the first place.
>
> On Thu, Mar 24, 2016 at 6:59 PM, Andrew Selder 
> 

> wrote:
>
>> Hi all,
>>
>> After we got bit by it a couple times recently, I was thinking of
>> submitting a PR for ActiveRecord that would issue a warning if you 
>> submit a
>> hash containing the same key as both a symbol and a string to any of 
>> the AR
>> construction methods (create, new, update_attributes, etc…) . As 
>> far as I
>> can tell, the behavior when this happens in undefined (and change 
>> between
>> Rails 3 and 4 for example).
>>
>> For example:
>>
>> attr = { ‘a’ => 123, :a => 456 }
>> ARModel.create!(attr)
>>
>> In Rails 3, you’ll get an object with the ‘a’ attributes set to 
>> 456, in
>> Rails 4, you’ll get 123.
>>
>> I think this kind of things is almost always unintentional and worth 
>> a
>> warning.
>>
>> I’ll be happy to code this up and submit it if people think it 
>> would be
>> useful and accepted.
>>
>> Please let me know your thoughts and feedback.
>>
>> Thanks,
>>
>> Andrew
>>
>> --
>> You received this message because you are subscribed to the Google 
>> Groups
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, 
>> send an
>> email to rubyonrails-co...@googlegroups.com.
>> To post to this group, send email to 
>> rubyonra...@googlegroups.com.
>> Visit this group at https://groups.google.com/group/rubyonrails-core.
>> For more options, visit https://groups.google.com/d/optout.
>>
>
> -- 
> You received this message because you are subscribed to the Google 
> Groups "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send 
> an email to rubyonrails-co...@googlegroups.com.
> To post to this group, send email to 
> rubyonra...@googlegroups.com.
> Visit this group at https://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

[Andrew Selder]

Richard,

Thanks for your input. Perhaps controlled by a config variable (a la the
old whiny_nils)? Can be off by default and turned on if desired what
ever environments you want (probably dev and test)

Thanks,

Andrew

[richard schneeman]

> Perhaps controlled by a config variable (a la the old whiny_nils)? 

I would prefer not. We are way overloaded with config options. Those options aren't free, adding each feature behind a config flag requires maintenance and support. Instead have bundler be your config flag. If it's important enough to add to Rails Core, it should be important enough to make its own gem. If that gem is so insanely wildly downloaded that everyone uses it then maybe we can consider adding it as a configuration option. 

-- 
Richard Schneeman

http://schneems.com