Fix string lookups asserting or failing with unsupported collections

A recent change from [0] added an assert when a string lookup was
performed on a collection item with no name-property,
but silently failed with release builds.

This isn't correct in a couple of ways.
The assert makes it seem like the RNA API needs to be updated when
it's in fact valid to have collection items without a name-property.
It also misleads script authors to silently fail since it implies a
key that exists would return a result.

Raise a TypeError exception when string lookups are attempted on
collections that don't support them.

This change also applies to get() & __contains__().

[0]: 5bb3a3f157
This commit is contained in:
2023-04-26 15:58:20 +10:00
parent c81e6d3ff3
commit 6a49041be3
3 changed files with 57 additions and 8 deletions

View File

@@ -433,6 +433,12 @@ int RNA_property_collection_lookup_string_index(
bool RNA_property_collection_lookup_int_has_fn(PropertyRNA *prop);
bool RNA_property_collection_lookup_string_has_fn(PropertyRNA *prop);
bool RNA_property_collection_lookup_string_has_nameprop(PropertyRNA *prop);
/**
* Return true when this type supports string lookups,
* it has a lookup function or it's type has a name property.
*/
bool RNA_property_collection_lookup_string_supported(PropertyRNA *prop);
/**
* Zero return is an assignment error.

View File

@@ -4237,6 +4237,19 @@ bool RNA_property_collection_lookup_string_has_fn(PropertyRNA *prop)
return cprop->lookupstring != nullptr;
}
bool RNA_property_collection_lookup_string_has_nameprop(PropertyRNA *prop)
{
BLI_assert(RNA_property_type(prop) == PROP_COLLECTION);
CollectionPropertyRNA *cprop = (CollectionPropertyRNA *)rna_ensure_property(prop);
return (cprop->item_type && cprop->item_type->nameproperty);
}
bool RNA_property_collection_lookup_string_supported(PropertyRNA *prop)
{
return (RNA_property_collection_lookup_string_has_fn(prop) ||
RNA_property_collection_lookup_string_has_nameprop(prop));
}
int RNA_property_collection_lookup_int(PointerRNA *ptr,
PropertyRNA *prop,
int key,

View File

@@ -2235,6 +2235,27 @@ static int pyrna_prop_collection_subscript_is_valid_or_error(const PyObject *val
return 0;
}
static void pyrna_prop_collection_string_subscript_unsupported_error(BPy_PropertyRNA *self,
const char *error_prefix)
{
PyErr_Format(PyExc_TypeError,
"%.200s: %.200s.%.200s does not support string lookups",
error_prefix,
RNA_struct_identifier(self->ptr.type),
RNA_property_identifier(self->prop));
}
static int pyrna_prop_collection_string_subscript_supported_or_error(BPy_PropertyRNA *self,
const char *error_prefix)
{
BLI_assert(BPy_PropertyRNA_Check(self));
if (RNA_property_collection_lookup_string_supported(self->prop)) {
return 0;
}
pyrna_prop_collection_string_subscript_unsupported_error(self, error_prefix);
return -1;
}
/* Internal use only. */
static PyObject *pyrna_prop_collection_subscript_int(BPy_PropertyRNA *self, Py_ssize_t keynum)
{
@@ -2360,7 +2381,7 @@ static PyObject *pyrna_prop_collection_subscript_str(BPy_PropertyRNA *self, cons
return pyrna_struct_CreatePyObject(&newptr);
}
}
else {
else if (RNA_property_collection_lookup_string_has_nameprop(self->prop)) {
/* No callback defined, just iterate and find the nth item. */
const int keylen = strlen(keyname);
char name[256];
@@ -2371,13 +2392,10 @@ static PyObject *pyrna_prop_collection_subscript_str(BPy_PropertyRNA *self, cons
RNA_property_collection_begin(&self->ptr, self->prop, &iter);
for (int i = 0; iter.valid; RNA_property_collection_next(&iter), i++) {
PropertyRNA *nameprop = RNA_struct_name_property(iter.ptr.type);
BLI_assert_msg(
nameprop,
"Attempted to use a string to index into a collection of items with no 'nameproperty'.");
if (nameprop == NULL) {
/* For non-debug builds, bail if there's no 'nameproperty' to check. */
break;
}
/* The #RNA_property_collection_lookup_string_has_nameprop check should account for this.
* Although it's technically possible a sub-type clears the name property,
* this seems unlikely. */
BLI_assert(nameprop != NULL);
char *nameptr = RNA_property_string_get_alloc(
&iter.ptr, nameprop, name, sizeof(name), &namelen);
if ((keylen == namelen) && STREQ(nameptr, keyname)) {
@@ -2402,6 +2420,10 @@ static PyObject *pyrna_prop_collection_subscript_str(BPy_PropertyRNA *self, cons
return result;
}
}
else {
pyrna_prop_collection_string_subscript_unsupported_error(self, "bpy_prop_collection[key]");
return NULL;
}
PyErr_Format(PyExc_KeyError, "bpy_prop_collection[key]: key \"%.200s\" not found", keyname);
return NULL;
@@ -3349,6 +3371,10 @@ static int pyrna_prop_collection_contains(BPy_PropertyRNA *self, PyObject *key)
if (RNA_property_collection_lookup_string(&self->ptr, self->prop, keyname, &newptr)) {
return 1;
}
if (pyrna_prop_collection_string_subscript_supported_or_error(
self, "bpy_prop_collection.__contains__") == -1) {
return -1;
}
return 0;
}
@@ -5109,6 +5135,10 @@ static PyObject *pyrna_prop_collection_get(BPy_PropertyRNA *self, PyObject *args
if (RNA_property_collection_lookup_string(&self->ptr, self->prop, key, &newptr)) {
return pyrna_struct_CreatePyObject(&newptr);
}
if (pyrna_prop_collection_string_subscript_supported_or_error(
self, "bpy_prop_collection.get") == -1) {
return NULL;
}
}
else if (PyTuple_Check(key_ob)) {
PyObject *ret = pyrna_prop_collection_subscript_str_lib_pair(