Subtle breaking change with custom `object_class` and duplicate keys

[dgollahon]

I have a codebase in which we reject duplicate JSON keys with a friendly error message. We achieved this by parsing with something like:

JSON.parse(json_string, object_class: TrackingHash, array_class: TrackingArray)

Then, everywhere []= is called on TrackingHash we track if the key has been added before. If it is duplicated, we add it to a list of duplicated keys. We then can reconstruct (in a single parse) errors like:

[
  "a.b.c is a duplicate key",
  "a.d.1.dup is a duplicate key",
  "a is a duplicate key",
]

As of sometime recently (~2.13 ? I haven't bisected the exact version but it doesn't work on latest and it did work on 2.9.1) the second []= is never invoked on the custom class. I did discover that you can set allow_duplicate_key: false and have it raise, but I can't reconstruct the keypath--so there's no way to tell if "c" is duplicated at the root or somewhere nested and I would ideally like to know where in the JSON this occurs.

I imagine this is fairly niche but it would be nice if either the old behavior was restored so I could track this myself or if the error message had the whole keypath in it so I could at least parse it out. In that world I'd still only be able to detect one duplicate at a time but that would be more or less fine. I'd like to upgrade to the latest version (lots of nice improvements in perf, etc.!) but I have to either change this behavior in my app or stick on the version I'm on.

Maybe this isn't a big enough issue to fix but it is a somewhat subtle and confusing change. I would expect allow_duplicate_key: true at least to still provide it to my custom class otherwise there's no way to access it (that I am aware of).

[byroot]

It's hard to tell without a reproduction script, but I highly suspect that it is because when encountering a duplicated key, the parser raise immediately (there is no point inserting a key if we're raising).

That being said. I suspect duplicate keys are rare even in your case, and using object_class: and array_class: cause a pretty big deoptimization, so have you considered doing something like:

begin
   JSON.parse(doc, allow_duplicated_key: false)
rescue JSON::ParserError => error
   if error.include?("duplicated")
     JSON.parse(json_string, object_class: TrackingHash, array_class: TrackingArray, allow_duplicated_key: true)
   else
     raise
  end
end