diff --git a/CHANGES.md b/CHANGES.md index 58e3f60d..13b7e279 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,10 @@ ### Unreleased +### 2025-07-28 (2.13.2) + +* Improve duplicate key warning and errors to include the key name and point to the right caller. + ### 2025-07-24 (2.13.1) * Fix support for older compilers without `__builtin_cpu_supports`. diff --git a/ext/json/ext/parser/parser.c b/ext/json/ext/parser/parser.c index 01b6e629..ab9d6c20 100644 --- a/ext/json/ext/parser/parser.c +++ b/ext/json/ext/parser/parser.c @@ -422,10 +422,12 @@ static void emit_parse_warning(const char *message, JSON_ParserState *state) long line, column; cursor_position(state, &line, &column); - rb_warn("%s at line %ld column %ld", message, line, column); + VALUE warning = rb_sprintf("%s at line %ld column %ld", message, line, column); + rb_funcall(mJSON, rb_intern("deprecation_warning"), 1, warning); } #define PARSE_ERROR_FRAGMENT_LEN 32 + #ifdef RBIMPL_ATTR_NORETURN RBIMPL_ATTR_NORETURN() #endif @@ -830,21 +832,64 @@ static inline VALUE json_decode_array(JSON_ParserState *state, JSON_ParserConfig return array; } +static VALUE json_find_duplicated_key(size_t count, const VALUE *pairs) +{ + VALUE set = rb_hash_new_capa(count / 2); + for (size_t index = 0; index < count; index += 2) { + size_t before = RHASH_SIZE(set); + VALUE key = pairs[index]; + rb_hash_aset(set, key, Qtrue); + if (RHASH_SIZE(set) == before) { + if (RB_SYMBOL_P(key)) { + return rb_sym2str(key); + } + return key; + } + } + return Qfalse; +} + +static void emit_duplicate_key_warning(JSON_ParserState *state, VALUE duplicate_key) +{ + VALUE message = rb_sprintf( + "detected duplicate key %"PRIsVALUE" in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`", + rb_inspect(duplicate_key) + ); + + emit_parse_warning(RSTRING_PTR(message), state); + RB_GC_GUARD(message); +} + +#ifdef RBIMPL_ATTR_NORETURN +RBIMPL_ATTR_NORETURN() +#endif +static void raise_duplicate_key_error(JSON_ParserState *state, VALUE duplicate_key) +{ + VALUE message = rb_sprintf( + "duplicate key %"PRIsVALUE, + rb_inspect(duplicate_key) + ); + + raise_parse_error(RSTRING_PTR(message), state); + RB_GC_GUARD(message); +} + static inline VALUE json_decode_object(JSON_ParserState *state, JSON_ParserConfig *config, size_t count) { size_t entries_count = count / 2; VALUE object = rb_hash_new_capa(entries_count); - rb_hash_bulk_insert(count, rvalue_stack_peek(state->stack, count), object); + const VALUE *pairs = rvalue_stack_peek(state->stack, count); + rb_hash_bulk_insert(count, pairs, object); if (RB_UNLIKELY(RHASH_SIZE(object) < entries_count)) { switch (config->on_duplicate_key) { case JSON_IGNORE: break; case JSON_DEPRECATED: - emit_parse_warning("detected duplicate keys in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`", state); + emit_duplicate_key_warning(state, json_find_duplicated_key(count, pairs)); break; case JSON_RAISE: - raise_parse_error("duplicate key", state); + raise_duplicate_key_error(state, json_find_duplicated_key(count, pairs)); break; } } diff --git a/ext/json/ext/simd/simd.h b/ext/json/ext/simd/simd.h index f8503d13..3abbdb02 100644 --- a/ext/json/ext/simd/simd.h +++ b/ext/json/ext/simd/simd.h @@ -7,45 +7,45 @@ typedef enum { #ifdef JSON_ENABLE_SIMD #ifdef __clang__ - #if __has_builtin(__builtin_ctzll) - #define HAVE_BUILTIN_CTZLL 1 - #else - #define HAVE_BUILTIN_CTZLL 0 - #endif +# if __has_builtin(__builtin_ctzll) +# define HAVE_BUILTIN_CTZLL 1 +# else +# define HAVE_BUILTIN_CTZLL 0 +# endif #elif defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)) - #define HAVE_BUILTIN_CTZLL 1 +# define HAVE_BUILTIN_CTZLL 1 #else - #define HAVE_BUILTIN_CTZLL 0 +# define HAVE_BUILTIN_CTZLL 0 #endif static inline uint32_t trailing_zeros64(uint64_t input) { #if HAVE_BUILTIN_CTZLL - return __builtin_ctzll(input); + return __builtin_ctzll(input); #else - uint32_t trailing_zeros = 0; - uint64_t temp = input; - while ((temp & 1) == 0 && temp > 0) { - trailing_zeros++; - temp >>= 1; - } - return trailing_zeros; + uint32_t trailing_zeros = 0; + uint64_t temp = input; + while ((temp & 1) == 0 && temp > 0) { + trailing_zeros++; + temp >>= 1; + } + return trailing_zeros; #endif } static inline int trailing_zeros(int input) { - #if HAVE_BUILTIN_CTZLL +#if HAVE_BUILTIN_CTZLL return __builtin_ctz(input); - #else +#else int trailing_zeros = 0; int temp = input; while ((temp & 1) == 0 && temp > 0) { - trailing_zeros++; - temp >>= 1; + trailing_zeros++; + temp >>= 1; } return trailing_zeros; - #endif +#endif } #if (defined(__GNUC__ ) || defined(__clang__)) @@ -79,37 +79,38 @@ static inline FORCE_INLINE uint64_t neon_match_mask(uint8x16_t matches) static inline FORCE_INLINE uint64_t compute_chunk_mask_neon(const char *ptr) { - uint8x16_t chunk = vld1q_u8((const unsigned char *)ptr); + uint8x16_t chunk = vld1q_u8((const unsigned char *)ptr); - // Trick: c < 32 || c == 34 can be factored as c ^ 2 < 33 - // https://lemire.me/blog/2025/04/13/detect-control-characters-quotes-and-backslashes-efficiently-using-swar/ - const uint8x16_t too_low_or_dbl_quote = vcltq_u8(veorq_u8(chunk, vdupq_n_u8(2)), vdupq_n_u8(33)); + // Trick: c < 32 || c == 34 can be factored as c ^ 2 < 33 + // https://lemire.me/blog/2025/04/13/detect-control-characters-quotes-and-backslashes-efficiently-using-swar/ + const uint8x16_t too_low_or_dbl_quote = vcltq_u8(veorq_u8(chunk, vdupq_n_u8(2)), vdupq_n_u8(33)); - uint8x16_t has_backslash = vceqq_u8(chunk, vdupq_n_u8('\\')); - uint8x16_t needs_escape = vorrq_u8(too_low_or_dbl_quote, has_backslash); - return neon_match_mask(needs_escape); + uint8x16_t has_backslash = vceqq_u8(chunk, vdupq_n_u8('\\')); + uint8x16_t needs_escape = vorrq_u8(too_low_or_dbl_quote, has_backslash); + return neon_match_mask(needs_escape); } static inline FORCE_INLINE int string_scan_simd_neon(const char **ptr, const char *end, uint64_t *mask) { while (*ptr + sizeof(uint8x16_t) <= end) { - uint64_t chunk_mask = compute_chunk_mask_neon(*ptr); - if (chunk_mask) { - *mask = chunk_mask; - return 1; - } - *ptr += sizeof(uint8x16_t); + uint64_t chunk_mask = compute_chunk_mask_neon(*ptr); + if (chunk_mask) { + *mask = chunk_mask; + return 1; + } + *ptr += sizeof(uint8x16_t); } return 0; } -uint8x16x4_t load_uint8x16_4(const unsigned char *table) { - uint8x16x4_t tab; - tab.val[0] = vld1q_u8(table); - tab.val[1] = vld1q_u8(table+16); - tab.val[2] = vld1q_u8(table+32); - tab.val[3] = vld1q_u8(table+48); - return tab; +static inline uint8x16x4_t load_uint8x16_4(const unsigned char *table) +{ + uint8x16x4_t tab; + tab.val[0] = vld1q_u8(table); + tab.val[1] = vld1q_u8(table+16); + tab.val[2] = vld1q_u8(table+32); + tab.val[3] = vld1q_u8(table+48); + return tab; } #endif /* ARM Neon Support.*/ @@ -150,12 +151,12 @@ static inline TARGET_SSE2 FORCE_INLINE int compute_chunk_mask_sse2(const char *p static inline TARGET_SSE2 FORCE_INLINE int string_scan_simd_sse2(const char **ptr, const char *end, int *mask) { while (*ptr + sizeof(__m128i) <= end) { - int chunk_mask = compute_chunk_mask_sse2(*ptr); - if (chunk_mask) { - *mask = chunk_mask; - return 1; - } - *ptr += sizeof(__m128i); + int chunk_mask = compute_chunk_mask_sse2(*ptr); + if (chunk_mask) { + *mask = chunk_mask; + return 1; + } + *ptr += sizeof(__m128i); } return 0; diff --git a/java/src/json/ext/ParserConfig.java b/java/src/json/ext/ParserConfig.java index ccfc558e..2bd03bab 100644 --- a/java/src/json/ext/ParserConfig.java +++ b/java/src/json/ext/ParserConfig.java @@ -2125,10 +2125,10 @@ else if ( _widec > _JSON_object_trans_keys[_mid+1] ) if (((RubyHash)result).hasKey(lastName)) { if (config.deprecateDuplicateKey) { context.runtime.getWarnings().warning( - "detected duplicate keys in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`" + "detected duplicate key " + name.inspect() + " in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`" ); } else { - throw parsingError(context, "duplicate key", p, pe); + throw parsingError(context, "duplicate key" + name.inspect(), p, pe); } } } diff --git a/java/src/json/ext/ParserConfig.rl b/java/src/json/ext/ParserConfig.rl index 4bc5d93b..533b1322 100644 --- a/java/src/json/ext/ParserConfig.rl +++ b/java/src/json/ext/ParserConfig.rl @@ -692,10 +692,10 @@ public class ParserConfig extends RubyObject { if (((RubyHash)result).hasKey(lastName)) { if (config.deprecateDuplicateKey) { context.runtime.getWarnings().warning( - "detected duplicate keys in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`" + "detected duplicate key " + name.inspect() + " in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`" ); } else { - throw parsingError(context, "duplicate key", p, pe); + throw parsingError(context, "duplicate key" + name.inspect(), p, pe); } } } diff --git a/lib/json/common.rb b/lib/json/common.rb index 486ec62a..9a878cea 100644 --- a/lib/json/common.rb +++ b/lib/json/common.rb @@ -48,7 +48,7 @@ def array_class_proc(array_class, on_load) end end - # TODO: exctract :create_additions support to another gem for version 3.0 + # TODO: extract :create_additions support to another gem for version 3.0 def create_additions_proc(opts) if opts[:symbolize_names] raise ArgumentError, "options :symbolize_names and :create_additions cannot be used in conjunction" @@ -87,31 +87,32 @@ def create_additions_proc(opts) opts end - GEM_ROOT = File.expand_path("../../../", __FILE__) + "/" def create_additions_warning - message = "JSON.load implicit support for `create_additions: true` is deprecated " \ + JSON.deprecation_warning "JSON.load implicit support for `create_additions: true` is deprecated " \ "and will be removed in 3.0, use JSON.unsafe_load or explicitly " \ "pass `create_additions: true`" + end + end + end - uplevel = 4 - caller_locations(uplevel, 10).each do |frame| - if frame.path.nil? || frame.path.start_with?(GEM_ROOT) || frame.path.end_with?("/truffle/cext_ruby.rb", ".c") - uplevel += 1 - else - break - end - end - - if RUBY_VERSION >= "3.0" - warn(message, uplevel: uplevel - 1, category: :deprecated) + class << self + def deprecation_warning(message, uplevel = 3) # :nodoc: + gem_root = File.expand_path("../../../", __FILE__) + "/" + caller_locations(uplevel, 10).each do |frame| + if frame.path.nil? || frame.path.start_with?(gem_root) || frame.path.end_with?("/truffle/cext_ruby.rb", ".c") + uplevel += 1 else - warn(message, uplevel: uplevel - 1) + break end end + + if RUBY_VERSION >= "3.0" + warn(message, uplevel: uplevel, category: :deprecated) + else + warn(message, uplevel: uplevel) + end end - end - class << self # :call-seq: # JSON[object] -> new_array or new_string # diff --git a/lib/json/version.rb b/lib/json/version.rb index 2aef3d7f..f9ac3e17 100644 --- a/lib/json/version.rb +++ b/lib/json/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module JSON - VERSION = '2.13.1' + VERSION = '2.13.2' end diff --git a/test/json/json_parser_test.rb b/test/json/json_parser_test.rb index 106492e1..6455d297 100644 --- a/test/json/json_parser_test.rb +++ b/test/json/json_parser_test.rb @@ -333,11 +333,36 @@ def test_parse_big_integers def test_parse_duplicate_key expected = {"a" => 2} + expected_sym = {a: 2} + assert_equal expected, parse('{"a": 1, "a": 2}', allow_duplicate_key: true) assert_raise(ParserError) { parse('{"a": 1, "a": 2}', allow_duplicate_key: false) } - assert_deprecated_warning(/duplicate keys/) do + assert_raise(ParserError) { parse('{"a": 1, "a": 2}', allow_duplicate_key: false, symbolize_names: true) } + + assert_deprecated_warning(/duplicate key "a"/) do assert_equal expected, parse('{"a": 1, "a": 2}') end + assert_deprecated_warning(/duplicate key "a"/) do + assert_equal expected_sym, parse('{"a": 1, "a": 2}', symbolize_names: true) + end + + if RUBY_ENGINE == 'RUBY_ENGINE' + assert_deprecated_warning(/#{File.basename(__FILE__)}\:#{__LINE__ + 1}/) do + assert_equal expected, parse('{"a": 1, "a": 2}') + end + end + + unless RUBY_ENGINE == 'jruby' + assert_raise(ParserError) do + fake_key = Object.new + JSON.load('{"a": 1, "a": 2}', -> (obj) { obj == "a" ? fake_key : obj }, allow_duplicate_key: false) + end + + assert_deprecated_warning(/duplicate key # (obj) { obj == "a" ? fake_key : obj }) + end + end end def test_some_wrong_inputs