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>
 

3 files changed, 104 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;
             }
         }
     }
diff --git a/sources/pyside2/tests/pysidetest/CMakeLists.txt b/sources/pyside2/tests/pysidetest/CMakeLists.txt
index cb8ba04cf..3c993cf4e 100644
--- a/sources/pyside2/tests/pysidetest/CMakeLists.txt
+++ b/sources/pyside2/tests/pysidetest/CMakeLists.txt
@@ -118,6 +118,7 @@ target_link_libraries(testbinding
 add_dependencies(testbinding pyside2 QtCore QtGui QtWidgets pysidetest)
 create_generator_target(testbinding)
 
+PYSIDE_TEST(constructor_properties_test.py)
 PYSIDE_TEST(decoratedslot_test.py)
 # Will always crash when built against Qt 5.6, no point in running it.
 if (Qt5Core_VERSION VERSION_GREATER 5.7.0)
diff --git a/sources/pyside2/tests/pysidetest/constructor_properties_test.py b/sources/pyside2/tests/pysidetest/constructor_properties_test.py
new file mode 100644
index 000000000..48e2a7aae
--- /dev/null
+++ b/sources/pyside2/tests/pysidetest/constructor_properties_test.py
@@ -0,0 +1,64 @@
+#############################################################################
+##
+## Copyright (C) 2019 The Qt Company Ltd.
+## Contact: https://www.qt.io/licensing/
+##
+## This file is part of Qt for Python.
+##
+## $QT_BEGIN_LICENSE:LGPL$
+## Commercial License Usage
+## Licensees holding valid commercial Qt licenses may use this file in
+## accordance with the commercial license agreement provided with the
+## Software or, alternatively, in accordance with the terms contained in
+## a written agreement between you and The Qt Company. For licensing terms
+## and conditions see https://www.qt.io/terms-conditions. For further
+## information use the contact form at https://www.qt.io/contact-us.
+##
+## GNU Lesser General Public License Usage
+## Alternatively, this file may be used under the terms of the GNU Lesser
+## General Public License version 3 as published by the Free Software
+## Foundation and appearing in the file LICENSE.LGPL3 included in the
+## packaging of this file. Please review the following information to
+## ensure the GNU Lesser General Public License version 3 requirements
+## will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
+##
+## GNU General Public License Usage
+## Alternatively, this file may be used under the terms of the GNU
+## General Public License version 2.0 or (at your option) the GNU General
+## Public license version 3 or any later version approved by the KDE Free
+## Qt Foundation. The licenses are as published by the Free Software
+## Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
+## included in the packaging of this file. Please review the following
+## information to ensure the GNU General Public License requirements will
+## be met: https://www.gnu.org/licenses/gpl-2.0.html and
+## https://www.gnu.org/licenses/gpl-3.0.html.
+##
+## $QT_END_LICENSE$
+##
+#############################################################################
+
+import unittest
+
+from helper import UsesQApplication
+from PySide2.QtCore import Qt
+from PySide2.QtWidgets import QApplication, QLabel, QFrame
+
+
+class ConstructorPropertiesTest(UsesQApplication):
+
+    def testCallConstructor(self):
+        label = QLabel(
+            frameStyle=QFrame.Panel | QFrame.Sunken,
+            text="first line\nsecond line",
+            alignment=Qt.AlignBottom | Qt.AlignRight
+        )
+        self.assertRaises(AttributeError, lambda: QLabel(
+            somethingelse=42,
+            text="first line\nsecond line",
+            alignment=Qt.AlignBottom | Qt.AlignRight
+        ))
+
+
+if __name__ == '__main__':
+    unittest.main()
+