From cbe895c121445de1a8be42f4af5b53e78f1b4756 Mon Sep 17 00:00:00 2001 From: Hugo Locurcio Date: Tue, 13 Aug 2024 21:07:16 +0200 Subject: [PATCH] Improve string formatting error messages - Print additional context to make troubleshooting easier. - Add the placeholder where the error occurred as a prefix to the error message. - Use sentence case for all error messages. --- core/string/ustring.cpp | 40 ++++++++++++++++----------------- tests/core/string/test_string.h | 16 ++++++------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/core/string/ustring.cpp b/core/string/ustring.cpp index 9e99fc3b2f57..e28ed0c663c9 100644 --- a/core/string/ustring.cpp +++ b/core/string/ustring.cpp @@ -5618,11 +5618,11 @@ String String::sprintf(const Array &values, bool *error) const { case 'x': // Hexadecimal (lowercase) case 'X': { // Hexadecimal (uppercase) if (value_index >= values.size()) { - return "not enough arguments for format string"; + return vformat("[%%%c] Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", c, value_index); } if (!values[value_index].is_num()) { - return "a number is required"; + return vformat("[%%%c] A number (int/float) is required, but a %s was provided.", c, Variant().get_type_name(values[value_index].get_type())); } int64_t value = values[value_index]; @@ -5684,11 +5684,11 @@ String String::sprintf(const Array &values, bool *error) const { } case 'f': { // Float if (value_index >= values.size()) { - return "not enough arguments for format string"; + return vformat("[%%f] Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", value_index); } if (!values[value_index].is_num()) { - return "a number is required"; + return vformat("[%%f] A number (int/float) is required, but a %s was provided.", Variant().get_type_name(values[value_index].get_type())); } double value = values[value_index]; @@ -5729,7 +5729,7 @@ String String::sprintf(const Array &values, bool *error) const { } case 'v': { // Vector2/3/4/2i/3i/4i if (value_index >= values.size()) { - return "not enough arguments for format string"; + return vformat("[%%v] Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", value_index); } int count; @@ -5747,7 +5747,7 @@ String String::sprintf(const Array &values, bool *error) const { count = 4; } break; default: { - return "%v requires a vector type (Vector2/3/4/2i/3i/4i)"; + return vformat("[%%v]: A vector is required (Vector2/3/4/2i/3i/4i), but a %s was provided.", Variant().get_type_name(values[value_index].get_type())); } } @@ -5799,7 +5799,7 @@ String String::sprintf(const Array &values, bool *error) const { } case 's': { // String if (value_index >= values.size()) { - return "not enough arguments for format string"; + return vformat("[%%s] Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", value_index); } String str = values[value_index]; @@ -5817,7 +5817,7 @@ String String::sprintf(const Array &values, bool *error) const { } case 'c': { if (value_index >= values.size()) { - return "not enough arguments for format string"; + return vformat("[%%c] Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", value_index); } // Convert to character. @@ -5825,20 +5825,20 @@ String String::sprintf(const Array &values, bool *error) const { if (values[value_index].is_num()) { int value = values[value_index]; if (value < 0) { - return "unsigned integer is lower than minimum"; + return vformat("[%%c]: Unsigned integer (-0x%x) is lower than the minimum allowed value (0).", Math::abs(value)); } else if (value >= 0xd800 && value <= 0xdfff) { - return "unsigned integer is invalid Unicode character"; + return vformat("[%%c]: Unsigned integer (0x%x) is an invalid Unicode character.", value); } else if (value > 0x10ffff) { - return "unsigned integer is greater than maximum"; + return vformat("[%%c]: Unsigned integer (0x%x) is greater than the maximum allowed value (0x10ffff).", value); } str = chr(values[value_index]); } else if (values[value_index].get_type() == Variant::STRING) { str = values[value_index]; if (str.length() != 1) { - return "%c requires number or single-character string"; + return vformat("[%%c]: A number or a single-character string is required, but a string of %d characters was provided.", str.length()); } } else { - return "%c requires number or single-character string"; + return vformat("[%%c]: A number or a single-character string is required, but a %s was provided.", Variant().get_type_name(values[value_index].get_type())); } // Padding. @@ -5882,7 +5882,7 @@ String String::sprintf(const Array &values, bool *error) const { } else { if (c == '0' && min_chars == 0) { if (left_justified) { - WARN_PRINT("'0' flag ignored with '-' flag in string format"); + WARN_PRINT("[%0]: The \"0\" flag is ignored with \"-\" flag in string formatting."); } else { pad_with_zeros = true; } @@ -5895,7 +5895,7 @@ String String::sprintf(const Array &values, bool *error) const { } case '.': { // Float/Vector separator. if (in_decimals) { - return "too many decimal points in format"; + return "[%.]: Too many decimal points in format (only 1 point is allowed)."; } in_decimals = true; min_decimals = 0; // We want to add the value manually. @@ -5904,7 +5904,7 @@ String String::sprintf(const Array &values, bool *error) const { case '*': { // Dynamic width, based on value. if (value_index >= values.size()) { - return "not enough arguments for format string"; + return vformat("[%*]: Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", value_index); } Variant::Type value_type = values[value_index].get_type(); @@ -5912,7 +5912,7 @@ String String::sprintf(const Array &values, bool *error) const { value_type != Variant::VECTOR2 && value_type != Variant::VECTOR2I && value_type != Variant::VECTOR3 && value_type != Variant::VECTOR3I && value_type != Variant::VECTOR4 && value_type != Variant::VECTOR4I) { - return "* wants number or vector"; + return vformat("[%*]: A number (int/float) or a vector type (Vector2/3/4/2i/3i/4i) is required, but a %s was provided.", Variant().get_type_name(values[value_index].get_type())); } int size = values[value_index]; @@ -5928,7 +5928,7 @@ String String::sprintf(const Array &values, bool *error) const { } default: { - return "unsupported format character"; + return vformat("Unsupported format character \"%%%c\".", c); } } } else { // Not in format string. @@ -5950,11 +5950,11 @@ String String::sprintf(const Array &values, bool *error) const { } if (in_format) { - return "incomplete format"; + return "Incomplete format string. Check that all % placeholders use the correct syntax and are terminated correctly."; } if (value_index != values.size()) { - return "not all arguments converted during string formatting"; + return vformat("Not all arguments were converted during string formatting (%d provided, but %d were expected). Check for missing %% placeholders or extraneous arguments.", values.size(), value_index); } if (error) { diff --git a/tests/core/string/test_string.h b/tests/core/string/test_string.h index 9adc97e84509..d3ef61436a5e 100644 --- a/tests/core/string/test_string.h +++ b/tests/core/string/test_string.h @@ -1179,7 +1179,7 @@ TEST_CASE("[String] sprintf") { args.push_back("cheese"); output = format.sprintf(args, &error); REQUIRE(error); - CHECK(output == "not enough arguments for format string"); + CHECK(output == "[%s] Not enough arguments for string formatting (1 provided, but more were expected). Check for extraneous % placeholders or missing arguments."); // More arguments than formats. format = "fish %s frog"; @@ -1188,7 +1188,7 @@ TEST_CASE("[String] sprintf") { args.push_back("cheese"); output = format.sprintf(args, &error); REQUIRE(error); - CHECK(output == "not all arguments converted during string formatting"); + CHECK(output == "Not all arguments were converted during string formatting (2 provided, but 1 were expected). Check for missing % placeholders or extraneous arguments."); // Incomplete format. format = "fish %10"; @@ -1196,7 +1196,7 @@ TEST_CASE("[String] sprintf") { args.push_back("cheese"); output = format.sprintf(args, &error); REQUIRE(error); - CHECK(output == "incomplete format"); + CHECK(output == "Incomplete format string. Check that all % placeholders use the correct syntax and are terminated correctly."); // Bad character in format string. format = "fish %&f frog"; @@ -1204,7 +1204,7 @@ TEST_CASE("[String] sprintf") { args.push_back("cheese"); output = format.sprintf(args, &error); REQUIRE(error); - CHECK(output == "unsupported format character"); + CHECK(output == "Unsupported format character \"%&\"."); // Too many decimals. format = "fish %2.2.2f frog"; @@ -1212,7 +1212,7 @@ TEST_CASE("[String] sprintf") { args.push_back(99.99); output = format.sprintf(args, &error); REQUIRE(error); - CHECK(output == "too many decimal points in format"); + CHECK(output == "[%.]: Too many decimal points in format (only 1 point is allowed)."); // * not a number or vector. format = "fish %*f frog"; @@ -1221,7 +1221,7 @@ TEST_CASE("[String] sprintf") { args.push_back(99.99); output = format.sprintf(args, &error); REQUIRE(error); - CHECK(output == "* wants number or vector"); + CHECK(output == "[%*]: A number (int/float) or a vector type (Vector2/3/4/2i/3i/4i) is required, but a String was provided."); // Character too long. format = "fish %c frog"; @@ -1229,7 +1229,7 @@ TEST_CASE("[String] sprintf") { args.push_back("sc"); output = format.sprintf(args, &error); REQUIRE(error); - CHECK(output == "%c requires number or single-character string"); + CHECK(output == "[%c]: Requires a number or a single-character string, but a string of 2 characters was provided."); // Character bad type. format = "fish %c frog"; @@ -1237,7 +1237,7 @@ TEST_CASE("[String] sprintf") { args.push_back(Array()); output = format.sprintf(args, &error); REQUIRE(error); - CHECK(output == "%c requires number or single-character string"); + CHECK(output == "[%c]: A number or a single-character string is required, but a Array was provided."); } TEST_CASE("[String] is_numeric") {