Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update extension API validation #77445

Merged
merged 1 commit into from
May 27, 2023
+259 −92
Merged

Update extension API validation #77445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 104 additions & 27 deletions core/extension/extension_api_dump.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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"]);
Expand All @@ -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];
Expand All @@ -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;
}

Expand All @@ -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 {
Expand All @@ -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;
}
}

Expand All @@ -1161,15 +1230,15 @@ 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;
}

uint64_t old_hash = old_elem["hash"];

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;
}

Expand All @@ -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;
}
}
Expand All @@ -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);
}

Expand All @@ -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;
Expand All @@ -1247,15 +1316,15 @@ 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;
}

Ref<JSON> json;
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;
}

Expand All @@ -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;
}

Expand All @@ -1297,23 +1370,27 @@ 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;
}

if (!compare_sub_dict_array(removed_classes_registered, "classes", "name", old_api, new_api, "constants", "name", { "value" }, 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" }, 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;
}

Expand Down
Loading