PySide: Allow any existing attribute in the constructor

There are signals and properties which are correctly supported
in PySide by the function "fillQtProperties". The structures are
introspected by calling "QMetaObject::indexOfSignal" and
"QMetaObject::indexOfProperty".

By allowing any property, extending the above restriction,
we break the Qt API slightly, but have the tremendous
advantage of being able to write all needed properties into
the constructor call.

This approach is a compromize that keeps the attribute calls
as they currently are. Supporting real properties where we
actually have getter and setter functions would be possible
as well, but that would break compatibility very much!

It has to be discussed if we want to support a configuration
switch that enables this incompatible change.
If we would go this far, then I would do this together with
changing mixedCase to lower_case.

A simple test case has been added.

Task-number: PYSIDE-1019
Change-Id: I8094df51d63aa767a5a9ec1c83bcf7db7b157a01
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>
 

1 files changed, 39 insertions, 16 deletions

diff --git a/sources/pyside2/libpyside/pyside.cpp b/sources/pyside2/libpyside/pyside.cpp
index 6e4a3efd4..170a3587f 100644
--- a/sources/pyside2/libpyside/pyside.cpp
+++ b/sources/pyside2/libpyside/pyside.cpp
@@ -94,6 +94,30 @@ void init(PyObject *module)
     SignalManager::instance();
 }
 
+static bool _setProperty(PyObject* qObj, PyObject *name, PyObject *value, bool *accept)
+{
+    QByteArray propName(Shiboken::String::toCString(name));
+    propName[0] = std::toupper(propName[0]);
+    propName.prepend("set");
+
+    Shiboken::AutoDecRef propSetter(PyObject_GetAttrString(qObj, propName.constData()));
+    if (!propSetter.isNull()) {
+        *accept = true;
+        Shiboken::AutoDecRef args(PyTuple_Pack(1, value));
+        Shiboken::AutoDecRef retval(PyObject_CallObject(propSetter, args));
+        if (retval.isNull())
+            return false;
+    } else {
+        Shiboken::AutoDecRef attr(PyObject_GenericGetAttr(qObj, name));
+        if (PySide::Property::checkType(attr)) {
+            *accept = true;
+            if (PySide::Property::setValue(reinterpret_cast<PySideProperty*>(attr.object()), qObj, value) < 0)
+                return false;
+        }
+    }
+    return true;
+}
+
 bool fillQtProperties(PyObject* qObj, const QMetaObject* metaObj, PyObject* kwds, const char** blackList, unsigned int blackListSize)
 {
 
@@ -103,28 +127,27 @@ bool fillQtProperties(PyObject* qObj, const QMetaObject* metaObj, PyObject* kwds
     while (PyDict_Next(kwds, &pos, &key, &value)) {
         if (!blackListSize || !std::binary_search(blackList, blackList + blackListSize, std::string(Shiboken::String::toCString(key)))) {
             QByteArray propName(Shiboken::String::toCString(key));
+            bool accept = false;
             if (metaObj->indexOfProperty(propName) != -1) {
-                propName[0] = std::toupper(propName[0]);
-                propName.prepend("set");
-
-                Shiboken::AutoDecRef propSetter(PyObject_GetAttrString(qObj, propName.constData()));
-                if (!propSetter.isNull()) {
-                    Shiboken::AutoDecRef args(PyTuple_Pack(1, value));
-                    Shiboken::AutoDecRef retval(PyObject_CallObject(propSetter, args));
-                } else {
-                    PyObject* attr = PyObject_GenericGetAttr(qObj, key);
-                    if (PySide::Property::checkType(attr))
-                        PySide::Property::setValue(reinterpret_cast<PySideProperty*>(attr), qObj, value);
-                }
+                if (!_setProperty(qObj, key, value, &accept))
+                    return false;
             } else {
                 propName.append("()");
                 if (metaObj->indexOfSignal(propName) != -1) {
+                    accept = true;
                     propName.prepend('2');
-                    PySide::Signal::connect(qObj, propName, value);
-                } else {
-                    PyErr_Format(PyExc_AttributeError, "'%s' is not a Qt property or a signal", propName.constData());
+                    if (!PySide::Signal::connect(qObj, propName, value))
+                        return false;
+                }
+            }
+            if (!accept) {
+                // PYSIDE-1019: Allow any existing attribute in the constructor.
+                if (!_setProperty(qObj, key, value, &accept))
                     return false;
-                };
+            }
+            if (!accept) {
+                PyErr_Format(PyExc_AttributeError, "'%S' is not a Qt property or a signal", key);
+                return false;
             }
         }
     }