Update extension api validation

- Ensure that multiple changes to one method cannot hide each other in the CI.
- Check virtual methods for changes.
- Compare the detailed changes to a method.
- Compare enums.
- Fix comparing global enums.
- Use `vformat` to build error messages.

1 files changed, 104 insertions, 27 deletions

diff --git a/core/extension/extension_api_dump.cpp b/core/extension/extension_api_dump.cpp
index 0bcc8a44e8..c67867f65d 100644
--- a/core/extension/extension_api_dump.cpp
+++ b/core/extension/extension_api_dump.cpp
@@ -1065,7 +1065,57 @@ void GDExtensionAPIDump::generate_extension_json_file(const String &p_path) {
 	fa->store_string(text);
 }
 
-static bool compare_dict_array(const Dictionary &p_old_api, const Dictionary &p_new_api, const String &p_base_array, const String &p_name_field, const Vector<String> &p_fields_to_compare, bool p_compare_hashes, const String &p_outer_class = String(), bool p_compare_operators = false) {
+static bool compare_value(const String &p_path, const String &p_field, const Variant &p_old_value, const Variant &p_new_value, bool p_allow_name_change) {
+	bool failed = false;
+	String path = p_path + "/" + p_field;
+	if (p_old_value.get_type() == Variant::ARRAY && p_new_value.get_type() == Variant::ARRAY) {
+		Array old_array = p_old_value;
+		Array new_array = p_new_value;
+		if (!compare_value(path, "size", old_array.size(), new_array.size(), p_allow_name_change)) {
+			failed = true;
+		}
+		for (int i = 0; i < old_array.size() && i < new_array.size(); i++) {
+			if (!compare_value(path, itos(i), old_array[i], new_array[i], p_allow_name_change)) {
+				failed = true;
+			}
+		}
+	} else if (p_old_value.get_type() == Variant::DICTIONARY && p_new_value.get_type() == Variant::DICTIONARY) {
+		Dictionary old_dict = p_old_value;
+		Dictionary new_dict = p_new_value;
+		Array old_keys = old_dict.keys();
+		for (int i = 0; i < old_keys.size(); i++) {
+			Variant key = old_keys[i];
+			if (!new_dict.has(key)) {
+				failed = true;
+				print_error(vformat("Validate extension JSON: Error: Field '%s': %s was removed.", p_path, key));
+				continue;
+			}
+			if (p_allow_name_change && key == "name") {
+				continue;
+			}
+			if (!compare_value(path, key, old_dict[key], new_dict[key], p_allow_name_change)) {
+				failed = true;
+			}
+		}
+		Array new_keys = old_dict.keys();
+		for (int i = 0; i < new_keys.size(); i++) {
+			Variant key = new_keys[i];
+			if (!old_dict.has(key)) {
+				failed = true;
+				print_error(vformat("Validate extension JSON: Error: Field '%s': %s was added with value %s.", p_path, key, new_dict[key]));
+			}
+		}
+	} else {
+		bool equal = Variant::evaluate(Variant::OP_EQUAL, p_old_value, p_new_value);
+		if (!equal) {
+			print_error(vformat("Validate extension JSON: Error: Field '%s': %s changed value in new API, from %s to %s.", p_path, p_field, p_old_value.get_construct_string(), p_new_value.get_construct_string()));
+			return false;
+		}
+	}
+	return !failed;
+}
+
+static bool compare_dict_array(const Dictionary &p_old_api, const Dictionary &p_new_api, const String &p_base_array, const String &p_name_field, const Vector<String> &p_fields_to_compare, bool p_compare_hashes, const String &p_outer_class = String(), bool p_compare_operators = false, bool p_compare_enum_value = false) {
 	String base_array = p_outer_class + p_base_array;
 	if (!p_old_api.has(p_base_array)) {
 		return true; // May just not have this array and its still good. Probably added recently.
@@ -1077,7 +1127,7 @@ static bool compare_dict_array(const Dictionary &p_old_api, const Dictionary &p_
 
 	for (int i = 0; i < new_api.size(); i++) {
 		Dictionary elem = new_api[i];
-		ERR_FAIL_COND_V_MSG(!elem.has(p_name_field), false, "Validate extension JSON: Element of base_array '" + base_array + "' is missing field '" + p_name_field + "'. This is a bug.");
+		ERR_FAIL_COND_V_MSG(!elem.has(p_name_field), false, vformat("Validate extension JSON: Element of base_array '%s' is missing field '%s'. This is a bug.", base_array, p_name_field));
 		String name = elem[p_name_field];
 		if (p_compare_operators && elem.has("right_type")) {
 			name += " " + String(elem["right_type"]);
@@ -1090,7 +1140,7 @@ static bool compare_dict_array(const Dictionary &p_old_api, const Dictionary &p_
 		Dictionary old_elem = old_api[i];
 		if (!old_elem.has(p_name_field)) {
 			failed = true;
-			print_error("Validate extension JSON: JSON file: element of base array '" + base_array + "' is missing the field: '" + p_name_field + "'.");
+			print_error(vformat("Validate extension JSON: JSON file: element of base array '%s' is missing the field: '%s'.", base_array, p_name_field));
 			continue;
 		}
 		String name = old_elem[p_name_field];
@@ -1099,7 +1149,7 @@ static bool compare_dict_array(const Dictionary &p_old_api, const Dictionary &p_
 		}
 		if (!new_api_assoc.has(name)) {
 			failed = true;
-			print_error("Validate extension JSON: API was removed: " + base_array + "/" + name);
+			print_error(vformat("Validate extension JSON: API was removed: %s/%s", base_array, name));
 			continue;
 		}
 
@@ -1119,19 +1169,31 @@ static bool compare_dict_array(const Dictionary &p_old_api, const Dictionary &p_
 				field = field.substr(1, field.length());
 			}
 
+			bool enum_values = field.begins_with("$");
+			if (enum_values) {
+				// Meaning this field is a list of enum values.
+				field = field.substr(1, field.length());
+			}
+
+			bool allow_name_change = field.begins_with("@");
+			if (allow_name_change) {
+				// Meaning that when structurally comparing the old and new value, the dictionary entry 'name' may change.
+				field = field.substr(1, field.length());
+			}
+
 			Variant old_value;
 
 			if (!old_elem.has(field)) {
 				if (optional) {
 					if (new_elem.has(field)) {
 						failed = true;
-						print_error("Validate extension JSON: JSON file: Field was added in a way that breaks compatibility '" + base_array + "/" + name + "': " + field);
+						print_error(vformat("Validate extension JSON: JSON file: Field was added in a way that breaks compatibility '%s/%s': %s", base_array, name, field));
 					}
 				} else if (added && new_elem.has(field)) {
 					// Should be ok, field now exists, should not be verified in prior versions where it does not.
 				} else {
 					failed = true;
-					print_error("Validate extension JSON: JSON file: Missing filed in '" + base_array + "/" + name + "': " + field);
+					print_error(vformat("Validate extension JSON: JSON file: Missing field in '%s/%s': %s", base_array, name, field));
 				}
 				continue;
 			} else {
@@ -1140,17 +1202,24 @@ static bool compare_dict_array(const Dictionary &p_old_api, const Dictionary &p_
 
 			if (!new_elem.has(field)) {
 				failed = true;
-				ERR_PRINT("Validate extension JSON: Missing filed in current API '" + base_array + "/" + name + "': " + field + ". This is a bug.");
+				ERR_PRINT(vformat("Validate extension JSON: Missing field in current API '%s/%s': %s. This is a bug.", base_array, name, field));
 				continue;
 			}
 
 			Variant new_value = new_elem[field];
 
-			bool equal = Variant::evaluate(Variant::OP_EQUAL, old_value, new_value);
-			if (!equal) {
+			if (p_compare_enum_value && name.ends_with("_MAX")) {
+				if (static_cast<int64_t>(new_value) > static_cast<int64_t>(old_value)) {
+					// Ignore the _MAX value of an enum increasing.
+					continue;
+				}
+			}
+			if (enum_values) {
+				if (!compare_dict_array(old_elem, new_elem, field, "name", { "value" }, false, base_array + "/" + name + "/", false, true)) {
+					failed = true;
+				}
+			} else if (!compare_value(base_array + "/" + name, field, old_value, new_value, allow_name_change)) {
 				failed = true;
-				print_error("Validate extension JSON: Error: Field '" + base_array + "/" + name + "': " + field + " changed value in new API, from " + old_value.get_construct_string() + " to " + new_value.get_construct_string() + ".");
-				continue;
 			}
 		}
 
@@ -1161,7 +1230,7 @@ static bool compare_dict_array(const Dictionary &p_old_api, const Dictionary &p_
 				}
 
 				failed = true;
-				print_error("Validate extension JSON: JSON file: element of base array '" + base_array + "' is missing the field: 'hash'.");
+				print_error(vformat("Validate extension JSON: JSON file: element of base array '%s' is missing the field: 'hash'.", base_array));
 				continue;
 			}
 
@@ -1169,7 +1238,7 @@ static bool compare_dict_array(const Dictionary &p_old_api, const Dictionary &p_
 
 			if (!new_elem.has("hash")) {
 				failed = true;
-				print_error("Validate extension JSON: Error: Field '" + base_array + "' is missing the field: 'hash'.");
+				print_error(vformat("Validate extension JSON: Error: Field '%s' is missing the field: 'hash'.", base_array));
 				continue;
 			}
 
@@ -1190,7 +1259,7 @@ static bool compare_dict_array(const Dictionary &p_old_api, const Dictionary &p_
 
 			if (!hash_found) {
 				failed = true;
-				print_error("Validate extension JSON: Error: Hash mismatch for '" + base_array + "/" + name + "'. This means that the function has changed and no compatibility function was provided.");
+				print_error(vformat("Validate extension JSON: Error: Hash changed for '%s/%s', from %08X to %08X. This means that the function has changed and no compatibility function was provided.", base_array, name, old_hash, new_hash));
 				continue;
 			}
 		}
@@ -1210,7 +1279,7 @@ static bool compare_sub_dict_array(HashSet<String> &r_removed_classes_registered
 
 	for (int i = 0; i < new_api.size(); i++) {
 		Dictionary elem = new_api[i];
-		ERR_FAIL_COND_V_MSG(!elem.has(p_outer_name), false, "Validate extension JSON: Element of base_array '" + p_outer + "' is missing field '" + p_outer_name + "'. This is a bug.");
+		ERR_FAIL_COND_V_MSG(!elem.has(p_outer_name), false, vformat("Validate extension JSON: Element of base_array '%s' is missing field '%s'. This is a bug.", p_outer, p_outer_name));
 		new_api_assoc.insert(elem[p_outer_name], elem);
 	}
 
@@ -1220,14 +1289,14 @@ static bool compare_sub_dict_array(HashSet<String> &r_removed_classes_registered
 		Dictionary old_elem = old_api[i];
 		if (!old_elem.has(p_outer_name)) {
 			failed = true;
-			print_error("Validate extension JSON: JSON file: element of base array '" + p_outer + "' is missing the field: '" + p_outer_name + "'.");
+			print_error(vformat("Validate extension JSON: JSON file: element of base array '%s' is missing the field: '%s'.", p_outer, p_outer_name));
 			continue;
 		}
 		String name = old_elem[p_outer_name];
 		if (!new_api_assoc.has(name)) {
 			failed = true;
 			if (!r_removed_classes_registered.has(name)) {
-				print_error("Validate extension JSON: API was removed: " + p_outer + "/" + name);
+				print_error(vformat("Validate extension JSON: API was removed: %s/%s", p_outer, name));
 				r_removed_classes_registered.insert(name);
 			}
 			continue;
@@ -1247,7 +1316,7 @@ Error GDExtensionAPIDump::validate_extension_json_file(const String &p_path) {
 	Error error;
 	String text = FileAccess::get_file_as_string(p_path, &error);
 	if (error != OK) {
-		ERR_PRINT("Validate extension JSON: Could not open file '" + p_path + "'.");
+		ERR_PRINT(vformat("Validate extension JSON: Could not open file '%s'.", p_path));
 		return error;
 	}
 
@@ -1255,7 +1324,7 @@ Error GDExtensionAPIDump::validate_extension_json_file(const String &p_path) {
 	json.instantiate();
 	error = json->parse(text);
 	if (error != OK) {
-		ERR_PRINT("Validate extension JSON: Error parsing '" + p_path + "' at line " + itos(json->get_error_line()) + ": " + json->get_error_message());
+		ERR_PRINT(vformat("Validate extension JSON: Error parsing '%s' at line %d: %s", p_path, json->get_error_line(), json->get_error_message()));
 		return error;
 	}
 
@@ -1269,19 +1338,23 @@ Error GDExtensionAPIDump::validate_extension_json_file(const String &p_path) {
 		int major = header["version_major"];
 		int minor = header["version_minor"];
 
-		ERR_FAIL_COND_V_MSG(major != VERSION_MAJOR, ERR_INVALID_DATA, "JSON API dump is for a different engine version (" + itos(major) + ") than this one (" + itos(major) + ")");
-		ERR_FAIL_COND_V_MSG(minor > VERSION_MINOR, ERR_INVALID_DATA, "JSON API dump is for a newer version of the engine: " + itos(major) + "." + itos(minor));
+		ERR_FAIL_COND_V_MSG(major != VERSION_MAJOR, ERR_INVALID_DATA, vformat("JSON API dump is for a different engine version (%d) than this one (%d)", major, VERSION_MAJOR));
+		ERR_FAIL_COND_V_MSG(minor > VERSION_MINOR, ERR_INVALID_DATA, vformat("JSON API dump is for a newer version of the engine: %d.%d", major, minor));
 	}
 
 	bool failed = false;
 
 	HashSet<String> removed_classes_registered;
 
-	if (!compare_dict_array(old_api, new_api, "constants", "name", Vector<String>({ "value", "is_bitfield" }), false)) {
+	if (!compare_dict_array(old_api, new_api, "global_constants", "name", Vector<String>({ "value", "is_bitfield" }), false)) {
+		failed = true;
+	}
+
+	if (!compare_dict_array(old_api, new_api, "global_enums", "name", Vector<String>({ "$values", "is_bitfield" }), false)) {
 		failed = true;
 	}
 
-	if (!compare_dict_array(old_api, new_api, "utility_functions", "name", Vector<String>({ "category" }), true)) {
+	if (!compare_dict_array(old_api, new_api, "utility_functions", "name", Vector<String>({ "category", "is_vararg", "*return_type", "*@arguments" }), true)) {
 		failed = true;
 	}
 
@@ -1297,11 +1370,11 @@ Error GDExtensionAPIDump::validate_extension_json_file(const String &p_path) {
 		failed = true;
 	}
 
-	if (!compare_sub_dict_array(removed_classes_registered, "builtin_classes", "name", old_api, new_api, "methods", "name", {}, true)) {
+	if (!compare_sub_dict_array(removed_classes_registered, "builtin_classes", "name", old_api, new_api, "methods", "name", { "is_vararg", "is_static", "is_const", "*return_type", "*@arguments" }, true)) {
 		failed = true;
 	}
 
-	if (!compare_sub_dict_array(removed_classes_registered, "builtin_classes", "name", old_api, new_api, "constructors", "index", { "*arguments" }, false)) {
+	if (!compare_sub_dict_array(removed_classes_registered, "builtin_classes", "name", old_api, new_api, "constructors", "index", { "*@arguments" }, false)) {
 		failed = true;
 	}
 
@@ -1309,11 +1382,15 @@ Error GDExtensionAPIDump::validate_extension_json_file(const String &p_path) {
 		failed = true;
 	}
 
-	if (!compare_sub_dict_array(removed_classes_registered, "classes", "name", old_api, new_api, "methods", "name", { "is_virtual", "is_vararg", "is_static" }, true)) { // is_const sometimes can change because they are added if someone forgot, but should not be a problem for the extensions.
+	if (!compare_sub_dict_array(removed_classes_registered, "classes", "name", old_api, new_api, "enums", "name", { "is_bitfield", "$values" }, false)) {
+		failed = true;
+	}
+
+	if (!compare_sub_dict_array(removed_classes_registered, "classes", "name", old_api, new_api, "methods", "name", { "is_virtual", "is_vararg", "is_static", "is_const", "*return_value", "*@arguments" }, true)) {
 		failed = true;
 	}
 
-	if (!compare_sub_dict_array(removed_classes_registered, "classes", "name", old_api, new_api, "signals", "name", { "*arguments" }, false)) {
+	if (!compare_sub_dict_array(removed_classes_registered, "classes", "name", old_api, new_api, "signals", "name", { "*@arguments" }, false)) {
 		failed = true;
 	}