Make State#configure and #merge only write the options they are given

[youdie006]

Problem

On the pure-Ruby generator (the one TruffleRuby loads), JSON::State#configure -- and therefore its alias #merge -- resets every generation option it was not given back to that option's default.

state = JSON.state.new(indent: '  ', object_nl: "\n", array_nl: "\n")
state.generate({ 'foo' => [1] })
# => "{\n  \"foo\":[\n    1\n  ]\n}"

state.configure(depth: 0)          # or state.merge(depth: 0)
state.generate({ 'foo' => [1] })
# => "{\"foo\":[1]}"     <-- the layout is gone

The consumer is the public State#configure / State#merge pair itself. Anything that hands a configured state around and lets a later stage adjust one knob loses the rest: to_h's own rdoc (lib/json/ext/generator/state.rb:38-41) advertises the round trip -- "Returns the configuration instance variables as a hash, that can be passed to the configure method" -- and on the pure generator state.configure(state.to_h.slice(:depth)) throws away everything else. The same holds for max_nesting, allow_nan, ascii_only, script_safe, strict, buffer_initial_length, sort_keys and allow_duplicate_key.

The existing coverage cannot see this. test_configure_using_configure_and_merge (test/json/json_generator_test.rb:495) calls merge on a fresh JSON.state.new and passes all five options at once, so "merge into defaults" and "replace with defaults plus these five" produce the same state.

Why this is a bug and not a design choice

Your own C extension already merges. configure_state_i (ext/json/ext/generator/generator.c:1860) is an if/else if chain that writes only sym_indent, sym_space, sym_space_before, ... as they appear, and it is driven by rb_hash_foreach(config, configure_state_i, (VALUE)&data) at generator.c:1911, right under the comment at generator.c:1909:

We assume in most cases few keys are set so it's faster to go over the provided keys than to check all possible keys

So on CRuby, State#merge really merges. The pure _configure at lib/json/truffle_ruby/generator.rb:305 gives every keyword a literal default and therefore replaces. The alias is literally named merge (lib/json/truffle_ruby/generator.rb:303).

Where the three implementations stand today

option	C ext (configure_state_i)	Java (GeneratorState#_configure)	pure, before	pure, after
indent, space, space_before, object_nl, array_nl	merge	merge (if (x != null) this.x = x;, GeneratorState.java:548-563)	reset	merge
max_nesting, allow_nan, ascii_only, script_safe, strict, buffer_initial_length, depth, allow_duplicate_key, sort_keys, as_json	merge	reset (opts.getInt(..., DEFAULT_...) / opts.getBool(..., DEFAULT_...), GeneratorState.java:565-576)	reset	merge
explicit indent: nil	writes ""	keeps the previous value (OptionsReader#getString returns null for a falsy value)	writes ""	writes ""

This patch only moves the pure generator onto the C extension's behaviour. The Java extension is a third case in the middle row; I did not touch it, and I have a question about it at the bottom.

Fix

+        # Defaults are the current values so that #configure only writes what it was given.
         private def _configure(
-          indent: '', space: '', space_before: '', object_nl: '', array_nl: '', allow_nan: false,
-          as_json: false, ascii_only: false, sort_keys: false, depth: 0, buffer_initial_length: 1024,
-          allow_duplicate_key: false, script_safe: false, strict: false, max_nesting: 100
+          indent: @indent, space: @space, space_before: @space_before, object_nl: @object_nl,
+          array_nl: @array_nl, allow_nan: @allow_nan, as_json: @as_json, ascii_only: @ascii_only,
+          sort_keys: @sort_keys, depth: @depth, buffer_initial_length: @buffer_initial_length,
+          allow_duplicate_key: @allow_duplicate_key, script_safe: @script_safe, strict: @strict,
+          max_nesting: @max_nesting
         )

Construction is byte-identical: initialize (lib/json/truffle_ruby/generator.rb:155-171) assigns every ivar to its literal default before calling _configure(**opts) if opts, so a fresh State.new still starts from the documented defaults. sort_keys= (:212) is idempotent for Proc / false / true, so self.sort_keys = @sort_keys is a no-op.

The literal defaults in the old signature were the only place the pure generator disagreed with the C extension here; #664 ("Skip calling configure if there are no options") is what made initialize seed the ivars, and it left these literals in place.

Tests

Four tests, added next to test_configure_using_configure_and_merge.

The first two are unguarded on purpose: C and Java both merge the string options, so these pass on every CI row and go red only on the pure generator.

def test_configure_keeps_the_layout_of_a_pretty_state
  state = JSON.state.new(indent: '  ', object_nl: "\n", array_nl: "\n")
  state.configure(depth: 0)
  assert_equal %({\n  "foo":[\n    1\n  ]\n}), state.generate({ 'foo' => [1] })
end

The other two touch the non-string options and an explicit nil, which is exactly where the Java extension is a third case, so they carry the repo's own engine guard (omit ... if RUBY_ENGINE == 'jruby', as used at test/json/json_parser_test.rb:830 and test/json/resumable_parser_test.rb:9):

def test_configure_only_writes_the_other_options_it_is_given
  omit 'JRuby resets the non-string options' if RUBY_ENGINE == 'jruby'
  ...
def test_configure_writes_a_string_option_given_as_nil
  omit 'JRuby keeps the previous value for an explicit nil' if RUBY_ENGINE == 'jruby'
  ...

I read java/src/json/ext/GeneratorState.java:544-580 and java/src/json/ext/OptionsReader.java to write those two guards; without them the jruby-9.4 rows in .github/workflows/ci.yml would go red on a patch that does not touch the Java extension.

Verification

TruffleRuby is not installed on this machine, so I could not run the real engine. Instead I force-loaded the clone's pure generator on CRuby exactly the way lib/json/ext.rb:34-36 does it for TruffleRuby, and printed the loaded method's source_location on every run so the swap is provable:

LOADED-PATH PROOF: JSON.state              = JSON::TruffleRuby::Generator::State
LOADED-PATH PROOF: State#configure  source = [".../lib/json/truffle_ruby/generator.rb", 293]
LOADED-PATH PROOF: State#_configure source = [".../lib/json/truffle_ruby/generator.rb", 305]

Red / green under that harness, -n "/test_configure_/":

generator	result
before the patch	exit 1, 6 tests, 19 assertions, 4 failures -- all four new tests fail
after the patch	exit 0, 6 tests, 31 assertions, 0 failures

The failure that matters:

Failure: test_configure_keeps_the_layout_of_a_pretty_state
<"{\n" + "  \"foo\":[\n" + "    1\n" + "  ]\n" + "}"> expected but was
<"{\"foo\":[1]}">

No regression on the engine that actually loads this file: I ran the full json_generator_test.rb plus json_coder_test.rb, json_common_interface_test.rb and json_encoding_test.rb under the pure generator, before and after, and got no delta across the four suites. (Three failures appear in both runs; they are artifacts of the hybrid C-parser + pure-generator harness, identical before and after.)

Repo gates, taken verbatim from .github/workflows/ci.yml:55-59. bundle is broken on this machine so I ran bare rake:

gate	before	after
rake compile	0	0
rake test JSON_COMPACT=1	0, 589 tests, 3401 assertions, 0 failures	0, 593 tests, 3419 assertions, 0 failures
rake build	0	0, json 3.0.2 built to pkg/json-3.0.2.gem

That is the normal CRuby run, so it exercises the C extension: the four new tests pass there both before and after the patch, with 0 omissions. That is the point -- they encode the C extension's behaviour, and only the pure generator was failing to match it.

Mutation-checked in both directions, under the pure generator:

• revert the signature -> all four new tests fail.
• default the five string options to nil and write @indent = indent || @indent ("an explicit nil means keep") -> dies on test_configure_writes_a_string_option_given_as_nil (<""> expected but was <"1">), because the C extension writes "" there.
• merge only the string options, keep resetting the rest -> dies on test_configure_only_writes_the_other_options_it_is_given (<3> expected but was <100>).

One question

Should GeneratorState#_configure (java/src/json/ext/GeneratorState.java:565-576) get the same treatment? It resets the non-string options for the same reason -- opts.getInt("max_nesting", DEFAULT_MAX_NESTING) supplies a default rather than the current field. I left it alone because I have no JRuby build here to red/green it, and I did not want to ship an untested change to a second extension. Happy to open a follow-up if you want it aligned.

---

I used AI assistance for this change: Claude Code, model Claude Opus 5. I reproduced the failure, wrote and ran the tests and the gates myself, and I can explain and defend every line of the diff.