From 22361cfb8737f8369c969df19994ccef769b15f4 Mon Sep 17 00:00:00 2001 From: itchyny Date: Wed, 25 Feb 2026 08:14:23 +0900 Subject: [PATCH] Improve error message truncation with closing delimiters (#3478) Truncated strings now show closing delimiters: `..."` for strings, `...]` for arrays, and `...}` for objects, making error messages clearer. --- .github/workflows/decnum.yml | 11 +++++++- src/builtin.c | 21 ++++++++------- src/execute.c | 24 ++++++++--------- src/jv_aux.c | 20 +++++---------- src/jv_print.c | 28 +++++++++++++++----- src/parser.c | 2 +- src/parser.y | 2 +- tests/base64.test | 2 +- tests/jq.test | 50 +++++++++++++++++++----------------- 9 files changed, 88 insertions(+), 72 deletions(-) diff --git a/.github/workflows/decnum.yml b/.github/workflows/decnum.yml index bbf9389d..aed67436 100644 --- a/.github/workflows/decnum.yml +++ b/.github/workflows/decnum.yml @@ -25,9 +25,18 @@ jobs: --disable-maintainer-mode \ --disable-decnum make -j"$(nproc)" - make check file ./jq - name: Test run: | diff <(echo 100000000000000000000 | ./jq) <(echo 1e+20) + make check VERBOSE=yes git diff --exit-code + - name: Upload Test Logs + if: ${{ failure() }} + uses: actions/upload-artifact@v6 + with: + name: test-logs-decnum-disabled + retention-days: 7 + path: | + test-suite.log + tests/*.log diff --git a/src/builtin.c b/src/builtin.c index 5a5d1e0c..b6169cd7 100644 --- a/src/builtin.c +++ b/src/builtin.c @@ -50,24 +50,23 @@ BINOPS static jv type_error(jv bad, const char* msg) { - char errbuf[15]; + char errbuf[30]; const char *badkind = jv_kind_name(jv_get_kind(bad)); - jv err = jv_invalid_with_msg(jv_string_fmt("%s (%s) %s", badkind, - jv_dump_string_trunc(bad, errbuf, sizeof(errbuf)), - msg)); + jv err = jv_invalid_with_msg(jv_string_fmt( + "%s (%s) %s", badkind, + jv_dump_string_trunc(bad, errbuf, sizeof(errbuf)), msg)); return err; } static jv type_error2(jv bad1, jv bad2, const char* msg) { - char errbuf1[15],errbuf2[15]; + char errbuf1[30], errbuf2[30]; const char *badkind1 = jv_kind_name(jv_get_kind(bad1)); const char *badkind2 = jv_kind_name(jv_get_kind(bad2)); - jv err = jv_invalid_with_msg(jv_string_fmt("%s (%s) and %s (%s) %s", - badkind1, - jv_dump_string_trunc(bad1, errbuf1, sizeof(errbuf1)), - badkind2, - jv_dump_string_trunc(bad2, errbuf2, sizeof(errbuf2)), - msg)); + jv err = jv_invalid_with_msg(jv_string_fmt( + "%s (%s) and %s (%s) %s", badkind1, + jv_dump_string_trunc(bad1, errbuf1, sizeof(errbuf1)), + badkind2, + jv_dump_string_trunc(bad2, errbuf2, sizeof(errbuf2)), msg)); return err; } diff --git a/src/execute.c b/src/execute.c index 356e6dfd..a40d3436 100644 --- a/src/execute.c +++ b/src/execute.c @@ -10,13 +10,10 @@ #include "bytecode.h" #include "jv_alloc.h" -#include "jq_parser.h" #include "locfile.h" #include "jv.h" #include "jq.h" -#include "parser.h" #include "builtin.h" -#include "util.h" #include "linker.h" struct jq_state { @@ -499,10 +496,11 @@ jv jq_next(jq_state *jq) { stack_push(jq, jv_object_set(objv, k, v)); stack_push(jq, stktop); } else { - char errbuf[15]; - set_error(jq, jv_invalid_with_msg(jv_string_fmt("Cannot use %s (%s) as object key", - jv_kind_name(jv_get_kind(k)), - jv_dump_string_trunc(jv_copy(k), errbuf, sizeof(errbuf))))); + char errbuf[30]; + set_error(jq, jv_invalid_with_msg(jv_string_fmt( + "Cannot use %s (%s) as object key", + jv_kind_name(jv_get_kind(k)), + jv_dump_string_trunc(jv_copy(k), errbuf, sizeof(errbuf))))); jv_free(stktop); jv_free(v); jv_free(k); @@ -685,7 +683,7 @@ jv jq_next(jq_state *jq) { jv k = stack_pop(jq); // detect invalid path expression like path(reverse | .a) if (!path_intact(jq, jv_copy(t))) { - char keybuf[15]; + char keybuf[30]; char objbuf[30]; jv msg = jv_string_fmt( "Invalid path expression near attempt to access element %s of %s", @@ -771,11 +769,11 @@ jv jq_next(jq_state *jq) { } else { assert(opcode == EACH || opcode == EACH_OPT); if (opcode == EACH) { - char errbuf[15]; - set_error(jq, - jv_invalid_with_msg(jv_string_fmt("Cannot iterate over %s (%s)", - jv_kind_name(jv_get_kind(container)), - jv_dump_string_trunc(jv_copy(container), errbuf, sizeof(errbuf))))); + char errbuf[30]; + set_error(jq, jv_invalid_with_msg(jv_string_fmt( + "Cannot iterate over %s (%s)", + jv_kind_name(jv_get_kind(container)), + jv_dump_string_trunc(jv_copy(container), errbuf, sizeof(errbuf))))); } keep_going = 0; } diff --git a/src/jv_aux.c b/src/jv_aux.c index bc1405f0..84546cd3 100644 --- a/src/jv_aux.c +++ b/src/jv_aux.c @@ -135,20 +135,12 @@ jv jv_get(jv t, jv k) { jv_free(k); v = jv_null(); } else { - /* - * If k is a short string it's probably from a jq .foo expression or - * similar, in which case putting it in the invalid msg may help the - * user. The length 30 is arbitrary. - */ - if (jv_get_kind(k) == JV_KIND_STRING && jv_string_length_bytes(jv_copy(k)) < 30) { - v = jv_invalid_with_msg(jv_string_fmt("Cannot index %s with string \"%s\"", - jv_kind_name(jv_get_kind(t)), - jv_string_value(k))); - } else { - v = jv_invalid_with_msg(jv_string_fmt("Cannot index %s with %s", - jv_kind_name(jv_get_kind(t)), - jv_kind_name(jv_get_kind(k)))); - } + char errbuf[30]; + v = jv_invalid_with_msg(jv_string_fmt( + "Cannot index %s with %s (%s)", + jv_kind_name(jv_get_kind(t)), + jv_kind_name(jv_get_kind(k)), + jv_dump_string_trunc(jv_copy(k), errbuf, sizeof(errbuf)))); jv_free(t); jv_free(k); } diff --git a/src/jv_print.c b/src/jv_print.c index f717d079..5c86c5d9 100644 --- a/src/jv_print.c +++ b/src/jv_print.c @@ -15,6 +15,7 @@ #include "jv_unicode.h" #include "jv_alloc.h" #include "jv_private.h" +#include "util.h" #ifndef MAX_PRINT_DEPTH #define MAX_PRINT_DEPTH (10000) @@ -408,17 +409,30 @@ jv jv_dump_string(jv x, int flags) { } char *jv_dump_string_trunc(jv x, char *outbuf, size_t bufsize) { + assert(bufsize > 0); x = jv_dump_string(x, 0); const char *str = jv_string_value(x); const size_t len = strlen(str); - strncpy(outbuf, str, bufsize); - if (len > bufsize - 1 && bufsize >= 4) { - // Indicate truncation with '...' without breaking UTF-8. - const char *s = jvp_utf8_backtrack(outbuf + bufsize - 4, outbuf, NULL); - if (s) bufsize = s + 4 - outbuf; - strcpy(outbuf + bufsize - 4, "..."); + if (len > bufsize - 1 && bufsize >= 8) { + char delim = 0; + switch (str[0]) { + case '"': delim = '"'; break; + case '[': delim = ']'; break; + case '{': delim = '}'; break; + } + size_t l = bufsize - (delim ? 5 : 4); // "...", delim (if any), '\0' + const char *s = jvp_utf8_backtrack(str + l, str, NULL); + if (s) l = s - str; + memcpy(outbuf, str, l); + outbuf[l++] = '.'; + outbuf[l++] = '.'; + outbuf[l++] = '.'; + if (delim) outbuf[l++] = delim; + outbuf[l] = '\0'; } else { - outbuf[bufsize - 1] = '\0'; + size_t l = MIN(len, bufsize - 1); + memcpy(outbuf, str, l); + outbuf[l] = '\0'; } jv_free(x); return outbuf; diff --git a/src/parser.c b/src/parser.c index 8bba4684..c90e3134 100644 --- a/src/parser.c +++ b/src/parser.c @@ -431,7 +431,7 @@ int yylex(YYSTYPE* yylval, YYLTYPE* yylloc, block* answer, int* errors, * object key. */ static jv check_object_key(block k) { if (block_is_const(k) && block_const_kind(k) != JV_KIND_STRING) { - char errbuf[15]; + char errbuf[30]; return jv_string_fmt("Cannot use %s (%s) as object key", jv_kind_name(block_const_kind(k)), jv_dump_string_trunc(block_const(k), errbuf, sizeof(errbuf))); diff --git a/src/parser.y b/src/parser.y index bc9803f2..987a4eca 100644 --- a/src/parser.y +++ b/src/parser.y @@ -164,7 +164,7 @@ int yylex(YYSTYPE* yylval, YYLTYPE* yylloc, block* answer, int* errors, * object key. */ static jv check_object_key(block k) { if (block_is_const(k) && block_const_kind(k) != JV_KIND_STRING) { - char errbuf[15]; + char errbuf[30]; return jv_string_fmt("Cannot use %s (%s) as object key", jv_kind_name(block_const_kind(k)), jv_dump_string_trunc(block_const(k), errbuf, sizeof(errbuf))); diff --git a/tests/base64.test b/tests/base64.test index 52874a2d..991f1330 100644 --- a/tests/base64.test +++ b/tests/base64.test @@ -39,7 +39,7 @@ # invalid base64 characters (whitespace) . | try @base64d catch . "Not base64 data" -"string (\"Not base64...) is not valid base64 data" +"string (\"Not base64 data\") is not valid base64 data" # invalid base64 (too many bytes, QUJD = "ABCD" . | try @base64d catch . diff --git a/tests/jq.test b/tests/jq.test index 20ae3be5..59a7013a 100644 --- a/tests/jq.test +++ b/tests/jq.test @@ -935,19 +935,19 @@ null # Destructuring DUP/POP issues .[] | . as {a:$a} ?// {a:$a} ?// {a:$a} | $a [[3],[4],[5],6] -# Runtime error: "jq: Cannot index array with string \"c\"" +# Runtime error: "jq: Cannot index array with string (\"c\")" .[] as {a:$a} ?// {a:$a} ?// {a:$a} | $a [[3],[4],[5],6] -# Runtime error: "jq: Cannot index array with string \"c\"" +# Runtime error: "jq: Cannot index array with string (\"c\")" [[3],[4],[5],6][] | . as {a:$a} ?// {a:$a} ?// {a:$a} | $a null -# Runtime error: "jq: Cannot index array with string \"c\"" +# Runtime error: "jq: Cannot index array with string (\"c\")" [[3],[4],[5],6] | .[] as {a:$a} ?// {a:$a} ?// {a:$a} | $a null -# Runtime error: "jq: Cannot index array with string \"c\"" +# Runtime error: "jq: Cannot index array with string (\"c\")" .[] | . as {a:$a} ?// {a:$a} ?// $a | $a [[3],[4],[5],6] @@ -1242,10 +1242,10 @@ def inc(x): x |= .+1; inc(.[].a) [null,{"b":0},{"a":0},{"a":null},{"a":[0,1]},{"a":{"b":1}},{"a":[{}]},{"a":[{"c":3}]}] {"a":[{"b":5}]} {"b":0,"a":[{"b":5}]} -"Cannot index number with number" +"Cannot index number with number (0)" {"a":[{"b":5}]} -"Cannot index number with string \"b\"" -"Cannot index object with number" +"Cannot index number with string (\"b\")" +"Cannot index object with number (0)" {"a":[{"b":5}]} {"a":[{"c":3,"b":5}]} @@ -1430,7 +1430,7 @@ null # Try/catch and general `?` operator [.[]|try if . == 0 then error("foo") elif . == 1 then .a elif . == 2 then empty else . end catch .] [0,1,2,3] -["foo","Cannot index number with string \"a\"",3] +["foo","Cannot index number with string (\"a\")",3] [.[]|(.a, .a)?] [null,true,{"a":1}] @@ -1965,21 +1965,25 @@ null true try -. catch . -"very-long-string" -"string (\"very-long-...) cannot be negated" +"very-long-long-long-long-string" +"string (\"very-long-long-long-long...\") cannot be negated" try (.-.) catch . -"very-long-string" -"string (\"very-long-...) and string (\"very-long-...) cannot be subtracted" +"very-long-long-long-long-string" +"string (\"very-long-long-long-long...\") and string (\"very-long-long-long-long...\") cannot be subtracted" -"x" * range(0; 12; 2) + "☆" * 5 | try -. catch . +"x" * range(0; 12; 2) + "☆" * 8 | try -. catch . null -"string (\"☆☆☆...) cannot be negated" -"string (\"xx☆☆...) cannot be negated" -"string (\"xxxx☆☆...) cannot be negated" -"string (\"xxxxxx☆...) cannot be negated" -"string (\"xxxxxxxx...) cannot be negated" -"string (\"xxxxxxxxxx...) cannot be negated" +"string (\"☆☆☆☆☆☆☆☆\") cannot be negated" +"string (\"xx☆☆☆☆☆☆☆☆\") cannot be negated" +"string (\"xxxx☆☆☆☆☆☆...\") cannot be negated" +"string (\"xxxxxx☆☆☆☆☆☆...\") cannot be negated" +"string (\"xxxxxxxx☆☆☆☆☆...\") cannot be negated" +"string (\"xxxxxxxxxx☆☆☆☆...\") cannot be negated" + +try (. + "x") catch . == if have_decnum then "number (12345678901234567890123456...) and string (\"x\") cannot be added" else "number (12345678901234568000000000...) and string (\"x\") cannot be added" end +123456789012345678901234567890 +true join(",") ["1",2,true,false,3.4] @@ -1999,7 +2003,7 @@ join(",") try join(",") catch . ["1","2",{"a":{"b":{"c":33}}}] -"string (\"1,2,\") and object ({\"a\":{\"b\":{...) cannot be added" +"string (\"1,2,\") and object ({\"a\":{\"b\":{\"c\":33}}}) cannot be added" try join(",") catch . ["1","2",[3,4,5]] @@ -2376,7 +2380,7 @@ map(try implode catch .) try 0[implode] catch . [] -"Cannot index number with string \"\"" +"Cannot index number with string (\"\")" # walk walk(.) @@ -2452,14 +2456,14 @@ null try ("foobar" | .[1.5]) catch . null -"Cannot index string with number" +"Cannot index string with number (1.5)" # setpath/2 does not leak the input after an invalid get #2970 try ["ok", setpath([1]; 1)] catch ["ko", .] {"hi":"hello"} -["ko","Cannot index object with number"] +["ko","Cannot index object with number (1)"] try fromjson catch . "{'a': 123}"