Browse Source

VariantParser: Ensure all parse errors have an explanation

Likewise in ResourceFormatText and JSON.
Rémi Verschelde 1 year ago
parent
commit
c049d07121
3 changed files with 40 additions and 38 deletions
  1. 10 9
      core/io/json.cpp
  2. 16 14
      core/variant/variant_parser.cpp
  3. 14 15
      scene/resources/resource_format_text.cpp

+ 10 - 9
core/io/json.cpp

@@ -198,7 +198,7 @@ Error JSON::_get_token(const char32_t *p_str, int &index, int p_len, Token &r_to
 				String str;
 				while (true) {
 					if (p_str[index] == 0) {
-						r_err_str = "Unterminated String";
+						r_err_str = "Unterminated string";
 						return ERR_PARSE_ERROR;
 					} else if (p_str[index] == '"') {
 						index++;
@@ -208,7 +208,7 @@ Error JSON::_get_token(const char32_t *p_str, int &index, int p_len, Token &r_to
 						index++;
 						char32_t next = p_str[index];
 						if (next == 0) {
-							r_err_str = "Unterminated String";
+							r_err_str = "Unterminated string";
 							return ERR_PARSE_ERROR;
 						}
 						char32_t res = 0;
@@ -234,7 +234,7 @@ Error JSON::_get_token(const char32_t *p_str, int &index, int p_len, Token &r_to
 								for (int j = 0; j < 4; j++) {
 									char32_t c = p_str[index + j + 1];
 									if (c == 0) {
-										r_err_str = "Unterminated String";
+										r_err_str = "Unterminated string";
 										return ERR_PARSE_ERROR;
 									}
 									if (!is_hex_digit(c)) {
@@ -270,7 +270,7 @@ Error JSON::_get_token(const char32_t *p_str, int &index, int p_len, Token &r_to
 									for (int j = 0; j < 4; j++) {
 										char32_t c = p_str[index + j + 1];
 										if (c == 0) {
-											r_err_str = "Unterminated String";
+											r_err_str = "Unterminated string";
 											return ERR_PARSE_ERROR;
 										}
 										if (!is_hex_digit(c)) {
@@ -313,7 +313,7 @@ Error JSON::_get_token(const char32_t *p_str, int &index, int p_len, Token &r_to
 								res = next;
 							} break;
 							default: {
-								r_err_str = "Invalid escape sequence.";
+								r_err_str = "Invalid escape sequence";
 								return ERR_PARSE_ERROR;
 							}
 						}
@@ -361,19 +361,20 @@ Error JSON::_get_token(const char32_t *p_str, int &index, int p_len, Token &r_to
 					r_token.value = id;
 					return OK;
 				} else {
-					r_err_str = "Unexpected character.";
+					r_err_str = "Unexpected character";
 					return ERR_PARSE_ERROR;
 				}
 			}
 		}
 	}
 
+	r_err_str = "Unknown error getting token";
 	return ERR_PARSE_ERROR;
 }
 
 Error JSON::_parse_value(Variant &value, Token &token, const char32_t *p_str, int &index, int p_len, int &line, int p_depth, String &r_err_str) {
 	if (p_depth > Variant::MAX_RECURSION_DEPTH) {
-		r_err_str = "JSON structure is too deep. Bailing.";
+		r_err_str = "JSON structure is too deep";
 		return ERR_OUT_OF_MEMORY;
 	}
 
@@ -400,7 +401,7 @@ Error JSON::_parse_value(Variant &value, Token &token, const char32_t *p_str, in
 		} else if (id == "null") {
 			value = Variant();
 		} else {
-			r_err_str = "Expected 'true','false' or 'null', got '" + id + "'.";
+			r_err_str = vformat("Expected 'true', 'false', or 'null', got '%s'", id);
 			return ERR_PARSE_ERROR;
 		}
 	} else if (token.type == TK_NUMBER) {
@@ -408,7 +409,7 @@ Error JSON::_parse_value(Variant &value, Token &token, const char32_t *p_str, in
 	} else if (token.type == TK_STRING) {
 		value = token.value;
 	} else {
-		r_err_str = "Expected value, got " + String(tk_name[token.type]) + ".";
+		r_err_str = vformat("Expected value, got '%s'", String(tk_name[token.type]));
 		return ERR_PARSE_ERROR;
 	}
 

+ 16 - 14
core/variant/variant_parser.cpp

@@ -278,7 +278,7 @@ Error VariantParser::get_token(Stream *p_stream, Token &r_token, int &line, Stri
 					char32_t ch = p_stream->get_char();
 
 					if (ch == 0) {
-						r_err_str = "Unterminated String";
+						r_err_str = "Unterminated string";
 						r_token.type = TK_ERROR;
 						return ERR_PARSE_ERROR;
 					} else if (ch == '"') {
@@ -287,7 +287,7 @@ Error VariantParser::get_token(Stream *p_stream, Token &r_token, int &line, Stri
 						//escaped characters...
 						char32_t next = p_stream->get_char();
 						if (next == 0) {
-							r_err_str = "Unterminated String";
+							r_err_str = "Unterminated string";
 							r_token.type = TK_ERROR;
 							return ERR_PARSE_ERROR;
 						}
@@ -317,7 +317,7 @@ Error VariantParser::get_token(Stream *p_stream, Token &r_token, int &line, Stri
 									char32_t c = p_stream->get_char();
 
 									if (c == 0) {
-										r_err_str = "Unterminated String";
+										r_err_str = "Unterminated string";
 										r_token.type = TK_ERROR;
 										return ERR_PARSE_ERROR;
 									}
@@ -504,7 +504,7 @@ Error VariantParser::get_token(Stream *p_stream, Token &r_token, int &line, Stri
 					r_token.value = id.as_string();
 					return OK;
 				} else {
-					r_err_str = "Unexpected character.";
+					r_err_str = "Unexpected character";
 					r_token.type = TK_ERROR;
 					return ERR_PARSE_ERROR;
 				}
@@ -512,6 +512,7 @@ Error VariantParser::get_token(Stream *p_stream, Token &r_token, int &line, Stri
 		}
 	}
 
+	r_err_str = "Unknown error getting token";
 	r_token.type = TK_ERROR;
 	return ERR_PARSE_ERROR;
 }
@@ -1007,7 +1008,7 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream,
 			Object *obj = ClassDB::instantiate(type);
 
 			if (!obj) {
-				r_err_str = "Can't instantiate Object() of type: " + type;
+				r_err_str = vformat("Can't instantiate Object() of type '%s'", type);
 				return ERR_PARSE_ERROR;
 			}
 
@@ -1025,7 +1026,7 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream,
 
 			while (true) {
 				if (p_stream->is_eof()) {
-					r_err_str = "Unexpected End of File while parsing Object()";
+					r_err_str = "Unexpected EOF while parsing Object()";
 					return ERR_FILE_CORRUPT;
 				}
 
@@ -1123,7 +1124,7 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream,
 					String path = token.value;
 					Ref<Resource> res = ResourceLoader::load(path);
 					if (res.is_null()) {
-						r_err_str = "Can't load resource at path: '" + path + "'.";
+						r_err_str = "Can't load resource at path: " + path;
 						return ERR_PARSE_ERROR;
 					}
 
@@ -1135,7 +1136,7 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream,
 
 					value = res;
 				} else {
-					r_err_str = "Expected string as argument for Resource().";
+					r_err_str = "Expected string as argument for Resource()";
 					return ERR_PARSE_ERROR;
 				}
 			}
@@ -1571,7 +1572,7 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream,
 
 			value = arr;
 		} else {
-			r_err_str = "Unexpected identifier: '" + id + "'.";
+			r_err_str = vformat("Unexpected identifier '%s'", id);
 			return ERR_PARSE_ERROR;
 		}
 
@@ -1590,7 +1591,7 @@ Error VariantParser::parse_value(Token &token, Variant &value, Stream *p_stream,
 		value = token.value;
 		return OK;
 	} else {
-		r_err_str = "Expected value, got " + String(tk_name[token.type]) + ".";
+		r_err_str = vformat("Expected value, got '%s'", String(tk_name[token.type]));
 		return ERR_PARSE_ERROR;
 	}
 }
@@ -1601,7 +1602,7 @@ Error VariantParser::_parse_array(Array &array, Stream *p_stream, int &line, Str
 
 	while (true) {
 		if (p_stream->is_eof()) {
-			r_err_str = "Unexpected End of File while parsing array";
+			r_err_str = "Unexpected EOF while parsing array";
 			return ERR_FILE_CORRUPT;
 		}
 
@@ -1643,7 +1644,7 @@ Error VariantParser::_parse_dictionary(Dictionary &object, Stream *p_stream, int
 
 	while (true) {
 		if (p_stream->is_eof()) {
-			r_err_str = "Unexpected End of File while parsing dictionary";
+			r_err_str = "Unexpected EOF while parsing dictionary";
 			return ERR_FILE_CORRUPT;
 		}
 
@@ -1775,7 +1776,7 @@ Error VariantParser::_parse_tag(Token &token, Stream *p_stream, int &line, Strin
 
 	while (true) {
 		if (p_stream->is_eof()) {
-			r_err_str = "Unexpected End of File while parsing tag: " + r_tag.name;
+			r_err_str = vformat("Unexpected EOF while parsing tag '%s'", r_tag.name);
 			return ERR_FILE_CORRUPT;
 		}
 
@@ -1795,7 +1796,7 @@ Error VariantParser::_parse_tag(Token &token, Stream *p_stream, int &line, Strin
 		}
 
 		if (token.type != TK_IDENTIFIER) {
-			r_err_str = "Expected Identifier";
+			r_err_str = "Expected identifier";
 			return ERR_PARSE_ERROR;
 		}
 
@@ -1808,6 +1809,7 @@ Error VariantParser::_parse_tag(Token &token, Stream *p_stream, int &line, Strin
 
 		get_token(p_stream, token, line, r_err_str);
 		if (token.type != TK_EQUAL) {
+			r_err_str = "Expected '=' after identifier";
 			return ERR_PARSE_ERROR;
 		}
 

+ 14 - 15
scene/resources/resource_format_text.cpp

@@ -35,10 +35,8 @@
 #include "core/io/missing_resource.h"
 #include "core/object/script_language.h"
 
-///
-
 void ResourceLoaderText::_printerr() {
-	ERR_PRINT(String(res_path + ":" + itos(lines) + " - Parse Error: " + error_text).utf8().get_data());
+	ERR_PRINT(vformat("%s:%d - Parse Error: %s.", res_path, lines, error_text));
 }
 
 Ref<Resource> ResourceLoaderText::get_resource() {
@@ -242,7 +240,7 @@ Ref<PackedScene> ResourceLoaderText::_parse_node_tag(VariantParser::ResourcePars
 
 				if (packed_scene->get_state()->get_node_count() == 0) {
 					error = ERR_FILE_CORRUPT;
-					error_text = "Instance Placeholder can't be used for inheritance.";
+					error_text = "Instance Placeholder can't be used for inheritance";
 					_printerr();
 					return Ref<PackedScene>();
 				}
@@ -372,7 +370,7 @@ Ref<PackedScene> ResourceLoaderText::_parse_node_tag(VariantParser::ResourcePars
 		} else if (next_tag.name == "editable") {
 			if (!next_tag.fields.has("path")) {
 				error = ERR_FILE_CORRUPT;
-				error_text = "missing 'path' field from editable tag";
+				error_text = "Missing 'path' field from editable tag";
 				_printerr();
 				return Ref<PackedScene>();
 			}
@@ -394,6 +392,7 @@ Ref<PackedScene> ResourceLoaderText::_parse_node_tag(VariantParser::ResourcePars
 			}
 		} else {
 			error = ERR_FILE_CORRUPT;
+			error_text = vformat("Unknown tag '%s' in file", next_tag.name);
 			_printerr();
 			return Ref<PackedScene>();
 		}
@@ -547,7 +546,7 @@ Error ResourceLoaderText::load() {
 						missing_resource->set_recording_properties(true);
 						obj = missing_resource;
 					} else {
-						error_text += "Can't create sub resource of type: " + type;
+						error_text = vformat("Can't create sub resource of type '%s'", type);
 						_printerr();
 						error = ERR_FILE_CORRUPT;
 						return error;
@@ -556,7 +555,7 @@ Error ResourceLoaderText::load() {
 
 				Resource *r = Object::cast_to<Resource>(obj);
 				if (!r) {
-					error_text += "Can't create sub resource of type, because not a resource: " + type;
+					error_text = vformat("Can't create sub resource of type '%s' as it's not a resource type", type);
 					_printerr();
 					error = ERR_FILE_CORRUPT;
 					return error;
@@ -668,7 +667,7 @@ Error ResourceLoaderText::load() {
 		}
 
 		if (is_scene) {
-			error_text += "found the 'resource' tag on a scene file!";
+			error_text = "Unexpected 'resource' tag in a scene file";
 			_printerr();
 			error = ERR_FILE_CORRUPT;
 			return error;
@@ -693,7 +692,7 @@ Error ResourceLoaderText::load() {
 						missing_resource->set_recording_properties(true);
 						obj = missing_resource;
 					} else {
-						error_text += "Can't create sub resource of type: " + res_type;
+						error_text = vformat("Can't create sub resource of type '%s'", res_type);
 						_printerr();
 						error = ERR_FILE_CORRUPT;
 						return error;
@@ -702,7 +701,7 @@ Error ResourceLoaderText::load() {
 
 				Resource *r = Object::cast_to<Resource>(obj);
 				if (!r) {
-					error_text += "Can't create sub resource of type, because not a resource: " + res_type;
+					error_text = vformat("Can't create sub resource of type '%s' as it's not a resource type", res_type);
 					_printerr();
 					error = ERR_FILE_CORRUPT;
 					return error;
@@ -815,7 +814,7 @@ Error ResourceLoaderText::load() {
 
 	if (next_tag.name == "node") {
 		if (!is_scene) {
-			error_text += "found the 'node' tag on a resource file!";
+			error_text = "Unexpected 'node' tag in a resource file";
 			_printerr();
 			error = ERR_FILE_CORRUPT;
 			return error;
@@ -847,7 +846,7 @@ Error ResourceLoaderText::load() {
 
 		return error;
 	} else {
-		error_text += "Unknown tag in file: " + next_tag.name;
+		error_text = vformat("Unknown tag '%s' in file", next_tag.name);
 		_printerr();
 		error = ERR_FILE_CORRUPT;
 		return error;
@@ -1114,7 +1113,7 @@ void ResourceLoaderText::open(Ref<FileAccess> p_f, bool p_skip_first_tag) {
 		res_type = tag.fields["type"];
 
 	} else {
-		error_text = "Unrecognized file type: " + tag.name;
+		error_text = vformat("Unrecognized file type '%s'", tag.name);
 		_printerr();
 		error = ERR_PARSE_ERROR;
 		return;
@@ -1218,9 +1217,9 @@ Error ResourceLoaderText::get_classes_used(HashSet<StringName> *r_classes) {
 		// This is a node, must save one more!
 
 		if (!is_scene) {
-			error_text += "found the 'node' tag on a resource file!";
-			_printerr();
 			error = ERR_FILE_CORRUPT;
+			error_text = "Unexpected 'node' tag in a resource file";
+			_printerr();
 			return error;
 		}