rails/rails — pull request #10278 by codeodor (2013-04-20 18:46:09+00:00)
Title: Fix #to_json for BasicObject Enumerables

Objects which `include Enumerable` already get the `as_json` method, but they do not get `to_json`.

That means it would get `to_json` from `Object` in most cases, however, not if they inherit from `BasicObject`.

You might think, "who does that?!" :confused: (and rightfully so).

That is just the simplest case where we'd have an issue. For a real-life use case, consider someone who uses `SimpleDelegator` and includes `Enumerable`. In that case, `#to_json` will forward on to the wrapped object, which means it will end up calling `#as_json` on each element in the wrapped collection, instead of each element in the collection we actually want to call it on.

Delegation is a much more common pattern than inheriting directly from `BasicObject` and [we ran into this issue in DisplayCase](https://github.com/objects-on-rails/display-case/issues/27).

Now, it could be the case that we should just say, "if you don't want to forward the `#to_json` message to the other object, then write a method to define it."

I thought about that a lot, but since `as_json` is already on `Enumerable` I thought `#to_json` probably should be as well. (Especially since there was already a test case for it, but it was not testing against a custom `Enumerable`, it was testing against a `Hash`).

(And we certainly don't want to pollute `BasicObject` with the method, even though it is the simplest test case I could come up with!)

In addition to testing the case I mentioned above, this pull request also adds tests to demonstrate:

1. The classes to which Rails adds `#to_json` get the method. (On the face of it, I thought that's really not testing much, but it could be useful to catch regressions for objects that expect it to be defined).
2. That we actually test (to|as)_json against an enumerable instead of a hash.

:thumbsup: ?