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

Undef problematic Object methods #2093

Merged
merged 2 commits into from
Apr 30, 2017

Conversation

bf4
Copy link
Member

@bf4 bf4 commented Mar 31, 2017

Needs tests and references to issues

@mention-bot
Copy link

@bf4, thanks for your PR! By analyzing the history of the files in this pull request, we identified @beauby, @NullVoxPopuli and @bolshakov to be potential reviewers.

@bf4 bf4 added the V: 0.10.x label Apr 24, 2017
@bf4 bf4 force-pushed the undef_unneeded_kernel_methods branch from 574b9a0 to 4fb635b Compare April 30, 2017 22:56
@bf4 bf4 merged commit 81a13c4 into rails-api:master Apr 30, 2017
@bf4 bf4 deleted the undef_unneeded_kernel_methods branch April 30, 2017 23:51
@idoa01
Copy link

idoa01 commented Apr 11, 2018

resurfacing an old issue which I think can be solved here: #2002

@bf4
Copy link
Member Author

bf4 commented Apr 12, 2018

@idoa01 not sure I follow

@idoa01
Copy link

idoa01 commented Apr 12, 2018

Oh, sorry, I thought that I didn't comment in the end :(
(I didn't want to comment since I didn't check if the bug is still active in the latest release, but I see that the code is still there)

There is an old bug that I think can be addressed in the same manner - you can't have an attribute named hash since your code checks if the object respond_to? the attribute. with basic ruby objects method names, respond_to will always be true.

my bug originated with the display attribute that this pull request handles, but doesn't handle other methods like hash (see example in #2002)

you might be able to fix this with undef_method like in this pull request (but it's a more complicated issue, since undefing the .hash method might cause issues with equality)

a better solution would be to ignore all Object methods, something in the lines of:

if respond_to?(attr) && method(attr).owner != Kernel
  send(attr)
else
  object.read_attribute_for_serialization(attr)
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants