__feature__: provide useful error message when feature is active

Features seem to work quite good so far, but they clearly need
much more testing.

One of the problems is that error messages are produced,
but the function name is not found and we get an ugly default
error message.

This is an efficient implementation that does all name mangling
in the C code. A helper dict avoids linear search for properties.

Task-number: PYSIDE-1019
Change-Id: Ic87c4a6e7dc2b2a251e809d6df0eb7fb9ca8021c
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>

5 files changed, 153 insertions, 5 deletions

diff --git a/sources/pyside6/libpyside/feature_select.cpp b/sources/pyside6/libpyside/feature_select.cpp
index d6d77a530..23a39ed34 100644
--- a/sources/pyside6/libpyside/feature_select.cpp
+++ b/sources/pyside6/libpyside/feature_select.cpp
@@ -43,7 +43,6 @@
 #include "class_property.h"
 
 #include <shiboken.h>
-#include <sbkstaticstrings.h>
 
 //////////////////////////////////////////////////////////////////////////////
 //
@@ -142,7 +141,7 @@ static inline PyObject *getFeatureSelectId()
     static PyObject *feature_dict = GetFeatureDict();
     // these things are all borrowed
     PyObject *globals = PyEval_GetGlobals();
-    if (   globals == nullptr
+    if (globals == nullptr
         || globals == cached_globals)
         return last_select_id;
 
@@ -151,7 +150,7 @@ static inline PyObject *getFeatureSelectId()
         return last_select_id;
 
     PyObject *select_id = PyDict_GetItem(feature_dict, modname);
-    if (   select_id == nullptr
+    if (select_id == nullptr
         || !PyInt_Check(select_id)  // int/long cheating
         || select_id == undef)
         return last_select_id;
@@ -525,7 +524,7 @@ static bool feature_01_addLowerNames(PyTypeObject *type, PyObject *prev_dict, in
 
     // We first copy the things over which will not be changed:
     while (PyDict_Next(prev_dict, &pos, &key, &value)) {
-        if (   Py_TYPE(value) != PepMethodDescr_TypePtr
+        if (Py_TYPE(value) != PepMethodDescr_TypePtr
             && Py_TYPE(value) != PepStaticMethod_TypePtr) {
             if (PyDict_SetItem(lower_dict, key, value))
                 return false;
@@ -641,6 +640,15 @@ static bool feature_02_true_property(PyTypeObject *type, PyObject *prev_dict, in
     if (PyDict_Update(prop_dict, prev_dict) < 0)
         return false;
 
+    // For speed, we establish a helper dict that maps the removed property
+    // method names to property name.
+    PyObject *prop_methods = PyDict_GetItem(prop_dict, PyMagicName::property_methods());
+    if (prop_methods == nullptr) {
+        prop_methods = PyDict_New();
+        if (prop_methods == nullptr
+            || PyDict_SetItem(prop_dict, PyMagicName::property_methods(), prop_methods))
+            return false;
+    }
     // We then replace methods by properties.
     bool lower = (id & 0x01) != 0;
     auto props = SbkObjectType_GetPropertyStrings(type);
@@ -662,7 +670,9 @@ static bool feature_02_true_property(PyTypeObject *type, PyObject *prev_dict, in
         AutoDecRef PyProperty(createProperty(type, getter, setter));
         if (PyProperty.isNull())
             return false;
-        if (PyDict_SetItem(prop_dict, name, PyProperty) < 0)
+        if (PyDict_SetItem(prop_dict, name, PyProperty) < 0
+            || PyDict_SetItem(prop_methods, read, name) < 0
+            || (setter != nullptr && PyDict_SetItem(prop_methods, write, name) < 0))
             return false;
         if (fields[0] != fields[1] && PyDict_GetItem(prop_dict, read))
             if (PyDict_DelItem(prop_dict, read) < 0)
diff --git a/sources/pyside6/libpyside/pysidestaticstrings.cpp b/sources/pyside6/libpyside/pysidestaticstrings.cpp
index 760d77632..7705fd058 100644
--- a/sources/pyside6/libpyside/pysidestaticstrings.cpp
+++ b/sources/pyside6/libpyside/pysidestaticstrings.cpp
@@ -60,4 +60,11 @@ STATIC_STRING_IMPL(name, "name")
 STATIC_STRING_IMPL(property, "property")
 STATIC_STRING_IMPL(select_id, "select_id")
 } // namespace PyName
+namespace PyMagicName
+{
+STATIC_STRING_IMPL(doc, "__doc__")
+STATIC_STRING_IMPL(func, "__func__")
+STATIC_STRING_IMPL(name, "__name__")
+STATIC_STRING_IMPL(property_methods, "__property_methods__")
+} // namespace PyMagicName
 } // namespace PySide
diff --git a/sources/pyside6/libpyside/pysidestaticstrings.h b/sources/pyside6/libpyside/pysidestaticstrings.h
index 1222d8f47..6774e936a 100644
--- a/sources/pyside6/libpyside/pysidestaticstrings.h
+++ b/sources/pyside6/libpyside/pysidestaticstrings.h
@@ -55,6 +55,13 @@ PyObject *name();
 PyObject *property();
 PyObject *select_id();
 } // namespace PyName
+namespace PyMagicName
+{
+PyObject *doc();
+PyObject *func();
+PyObject *name();
+PyObject *property_methods();
+} // namespace PyMagicName
 } // namespace PySide
 
 #endif // PYSIDESTRINGS_H
diff --git a/sources/pyside6/tests/QtCore/CMakeLists.txt b/sources/pyside6/tests/QtCore/CMakeLists.txt
index ee87345db..f9044a1b6 100644
--- a/sources/pyside6/tests/QtCore/CMakeLists.txt
+++ b/sources/pyside6/tests/QtCore/CMakeLists.txt
@@ -36,6 +36,7 @@ PYSIDE_TEST(deletelater_test.py)
 PYSIDE_TEST(destroysignal_test.py)
 PYSIDE_TEST(duck_punching_test.py)
 PYSIDE_TEST(emoji_string_test.py)
+PYSIDE_TEST(errormessages_with_features_test.py)
 PYSIDE_TEST(hash_test.py)
 PYSIDE_TEST(inherits_test.py)
 PYSIDE_TEST(max_signals.py)
diff --git a/sources/pyside6/tests/QtCore/errormessages_with_features_test.py b/sources/pyside6/tests/QtCore/errormessages_with_features_test.py
new file mode 100644
index 000000000..0e55c2e49
--- /dev/null
+++ b/sources/pyside6/tests/QtCore/errormessages_with_features_test.py
@@ -0,0 +1,123 @@
+#############################################################################
+##
+## Copyright (C) 2020 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 os
+import sys
+import unittest
+
+sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+from init_paths import init_test_paths
+init_test_paths(False)
+
+from PySide6 import QtWidgets
+from PySide6.support import __feature__
+
+"""
+errormessages_with_features_test.py
+-----------------------------------
+
+When errors occur while features are switched, we must always produce a
+valid error message.
+
+This test is in its own file because combining it with
+"snake_prop_feature_test" gave strange interactions with the other tests.
+"""
+
+class ErrormessagesWithFeatures(unittest.TestCase):
+    probe = "called with wrong argument types"
+    probe_miss = "missing signature"
+
+    def setUp(self):
+        qApp or QtWidgets.QApplication()
+        __feature__.set_selection(0)
+
+    def tearDown(self):
+        __feature__.set_selection(0)
+        qApp.shutdown()
+
+    def testCorrectErrorMessagesPlain(self):
+        with self.assertRaises(TypeError) as cm:
+            QtWidgets.QLabel().setFont(42)
+        print("\n\n" + cm.exception.args[0])
+        self.assertTrue(self.probe in cm.exception.args[0])
+
+    def testCorrectErrorMessagesSnake(self):
+        from __feature__ import snake_case
+        with self.assertRaises(TypeError) as cm:
+            QtWidgets.QLabel().set_font(42)
+        print("\n\n" + cm.exception.args[0])
+        self.assertTrue(self.probe in cm.exception.args[0])
+
+    def testCorrectErrorMessagesProp(self):
+        from __feature__ import true_property
+        with self.assertRaises(TypeError) as cm:
+            QtWidgets.QLabel().font = 42
+        print("\n\n" + cm.exception.args[0])
+        self.assertTrue(self.probe in cm.exception.args[0])
+
+    def testCorrectErrorMessagesSnakeProp(self):
+        from __feature__ import snake_case, true_property
+        with self.assertRaises(TypeError) as cm:
+            QtWidgets.QLabel().font = 42
+        print("\n\n" + cm.exception.args[0])
+        self.assertTrue(self.probe in cm.exception.args[0])
+
+    def testCorrectErrorMessagesClassProp(self):
+        from __feature__ import true_property
+        with self.assertRaises(TypeError) as cm:
+            QtWidgets.QApplication.quitOnLastWindowClosed = object
+        print("\n\n" + cm.exception.args[0])
+        self.assertTrue(self.probe_miss in cm.exception.args[0])
+        with self.assertRaises(TypeError) as cm:
+            qApp.quitOnLastWindowClosed = object
+        self.assertTrue(self.probe_miss in cm.exception.args[0])
+
+    def testCorrectErrorMessagesClassSnakeProp(self):
+        from __feature__ import snake_case, true_property
+        with self.assertRaises(TypeError) as cm:
+            QtWidgets.QApplication.quit_on_last_window_closed = object
+        print("\n\n" + cm.exception.args[0])
+        self.assertTrue(self.probe_miss in cm.exception.args[0])
+        with self.assertRaises(TypeError) as cm:
+            qApp.quit_on_last_window_closed = object
+        self.assertTrue(self.probe_miss in cm.exception.args[0])
+
+
+if __name__ == '__main__':
+    unittest.main()