__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>

9 files changed, 250 insertions, 16 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()
diff --git a/sources/shiboken6/libshiboken/sbkstaticstrings.cpp b/sources/shiboken6/libshiboken/sbkstaticstrings.cpp
index 5559d58d6..cd42643f8 100644
--- a/sources/shiboken6/libshiboken/sbkstaticstrings.cpp
+++ b/sources/shiboken6/libshiboken/sbkstaticstrings.cpp
@@ -59,6 +59,7 @@ STATIC_STRING_IMPL(loads, "loads")
 STATIC_STRING_IMPL(multi, "multi")
 STATIC_STRING_IMPL(name, "name")
 STATIC_STRING_IMPL(result, "result")
+STATIC_STRING_IMPL(select_id, "select_id")
 STATIC_STRING_IMPL(value, "value")
 STATIC_STRING_IMPL(values, "values")
 
@@ -87,8 +88,10 @@ STATIC_STRING_IMPL(get, "__get__")
 STATIC_STRING_IMPL(members, "__members__")
 STATIC_STRING_IMPL(module, "__module__")
 STATIC_STRING_IMPL(name, "__name__")
+STATIC_STRING_IMPL(property_methods, "__property_methods__")
 STATIC_STRING_IMPL(qualname, "__qualname__")
 STATIC_STRING_IMPL(self, "__self__")
+STATIC_STRING_IMPL(select_i, "__self__")
 
 // Internal:
 STATIC_STRING_IMPL(base, "__base__")
diff --git a/sources/shiboken6/libshiboken/sbkstaticstrings.h b/sources/shiboken6/libshiboken/sbkstaticstrings.h
index 16d36041e..d835d286c 100644
--- a/sources/shiboken6/libshiboken/sbkstaticstrings.h
+++ b/sources/shiboken6/libshiboken/sbkstaticstrings.h
@@ -58,6 +58,7 @@ LIBSHIBOKEN_API PyObject *loads();
 LIBSHIBOKEN_API PyObject *multi();
 LIBSHIBOKEN_API PyObject *name();
 LIBSHIBOKEN_API PyObject *result();
+LIBSHIBOKEN_API PyObject *select_id();
 LIBSHIBOKEN_API PyObject *value();
 LIBSHIBOKEN_API PyObject *values();
 } // namespace PyName
@@ -74,6 +75,7 @@ LIBSHIBOKEN_API PyObject *get();
 LIBSHIBOKEN_API PyObject *members();
 LIBSHIBOKEN_API PyObject *module();
 LIBSHIBOKEN_API PyObject *name();
+LIBSHIBOKEN_API PyObject *property_methods();
 LIBSHIBOKEN_API PyObject *qualname();
 LIBSHIBOKEN_API PyObject *self();
 } // namespace PyMagicName
diff --git a/sources/shiboken6/libshiboken/signature/signature.cpp b/sources/shiboken6/libshiboken/signature/signature.cpp
index 5ebb83fbb..f69c00b24 100644
--- a/sources/shiboken6/libshiboken/signature/signature.cpp
+++ b/sources/shiboken6/libshiboken/signature/signature.cpp
@@ -427,6 +427,85 @@ void FinishSignatureInitialization(PyObject *module, const char *signatures[])
     }
 }
 
+static PyObject *adjustFuncName(const char *func_name)
+{
+    /*
+     * PYSIDE-1019: Modify the function name expression according to feature.
+     *
+     * - snake_case
+     *      The function name must be converted.
+     * - full_property
+     *      The property name must be used and "fset" appended.
+     *
+     *          modname.subname.classsname.propname.fset
+     *
+     *      Class properties must use the expression
+     *
+     *          modname.subname.classsname.__dict__['propname'].fset
+     *
+     * Note that fget is impossible because there are no parameters.
+     */
+    static const char mapping_name[] = "shibokensupport.signature.mapping";
+    static PyObject *sys_modules = PySys_GetObject("modules");
+    static PyObject *mapping = PyDict_GetItemString(sys_modules, mapping_name);
+    static PyObject *ns = PyModule_GetDict(mapping);
+
+    char _path[200 + 1] = {};
+    const char *_name = strrchr(func_name, '.');
+    strncat(_path, func_name, _name - func_name);
+    ++_name;
+
+    // This is a very cheap call into `mapping.py`.
+    PyObject *update_mapping = PyDict_GetItemString(ns, "update_mapping");
+    AutoDecRef res(PyObject_CallFunctionObjArgs(update_mapping, nullptr));
+    if (res == nullptr)
+        return nullptr;
+
+    // Run `eval` on the type string to get the object.
+    AutoDecRef obtype(PyRun_String(_path, Py_eval_input, ns, ns));
+    if (PyModule_Check(obtype.object())) {
+        // This is a plain function. Return the unmangled name.
+        return String::fromCString(func_name);
+    }
+    assert(PyType_Check(obtype));   // This was not true for __init__!
+
+    // Find the feature flags
+    auto type = reinterpret_cast<PyTypeObject *>(obtype.object());
+    auto dict = type->tp_dict;
+    int id = SbkObjectType_GetReserved(type);
+    id = id < 0 ? 0 : id;   // if undefined, set to zero
+    auto lower = id & 0x01;
+    auto is_prop = id & 0x02;
+    bool is_class_prop = false;
+
+    // Compute all needed info.
+    PyObject *name = String::getSnakeCaseName(_name, lower);
+    PyObject *prop_name;
+    if (is_prop) {
+        PyObject *prop_methods = PyDict_GetItem(dict, PyMagicName::property_methods());
+        prop_name = PyDict_GetItem(prop_methods, name);
+        if (prop_name != nullptr) {
+            PyObject *prop = PyDict_GetItem(dict, prop_name);
+            is_class_prop = Py_TYPE(prop) != &PyProperty_Type;
+        }
+    }
+
+    // Finally, generate the correct path expression.
+    char _buf[200 + 1] = {};
+    if (is_prop) {
+        auto _prop_name = String::toCString(prop_name);
+        if (is_class_prop)
+            sprintf(_buf, "%s.__dict__['%s'].fset", _path, _prop_name);
+        else
+            sprintf(_buf, "%s.%s.fset", _path, _prop_name);
+    }
+    else {
+        auto _name = String::toCString(name);
+        sprintf(_buf, "%s.%s", _path, _name);
+    }
+    return String::fromCString(_buf);
+}
+
 void SetError_Argument(PyObject *args, const char *func_name)
 {
     /*
@@ -436,8 +515,16 @@ void SetError_Argument(PyObject *args, const char *func_name)
      */
     init_module_1();
     init_module_2();
+
+    // PYSIDE-1019: Modify the function name expression according to feature.
+    AutoDecRef new_func_name(adjustFuncName(func_name));
+    if (new_func_name.isNull()) {
+        PyErr_Print();
+        Py_FatalError("seterror_argument failed to call update_mapping");
+    }
     AutoDecRef res(PyObject_CallFunction(pyside_globals->seterror_argument_func,
-                                         const_cast<char *>("(Os)"), args, func_name));
+                                         const_cast<char *>("(OO)"),
+                                         args, new_func_name.object()));
     if (res.isNull()) {
         PyErr_Print();
         Py_FatalError("seterror_argument did not receive a result");
diff --git a/sources/shiboken6/shibokenmodule/files.dir/shibokensupport/signature/errorhandler.py b/sources/shiboken6/shibokenmodule/files.dir/shibokensupport/signature/errorhandler.py
index 264700ffa..7083fd1e6 100644
--- a/sources/shiboken6/shibokenmodule/files.dir/shibokensupport/signature/errorhandler.py
+++ b/sources/shiboken6/shibokenmodule/files.dir/shibokensupport/signature/errorhandler.py
@@ -61,11 +61,6 @@ from shibokensupport.signature import get_signature
 from shibokensupport.signature.mapping import update_mapping, namespace
 from textwrap import dedent
 
-try:
-    import PySide6
-except ImportError:
-    pass
-
 
 def qt_isinstance(inst, the_type):
     if the_type == float:
@@ -99,16 +94,15 @@ def matched_type(args, sigs):
 
 
 def seterror_argument(args, func_name):
-    update_mapping()
     func = None
     try:
         func = eval(func_name, namespace)
     except Exception as e:
-        msg = f"Internal error evaluating {func_name}: {e}"
+        msg = f"Internal error evaluating {func_name}: " + str(e)
         return TypeError, msg
     sigs = get_signature(func, "typeerror")
     if not sigs:
-        msg = f"Missing signature: {func_name}({args})"
+        msg = f"{func_name}({args} is wrong (missing signature)"
         return TypeError, msg
     if type(sigs) != list:
         sigs = [sigs]
@@ -118,7 +112,7 @@ def seterror_argument(args, func_name):
     found = matched_type(args, sigs)
     if found:
         msg = dedent(f"""
-            '{func_name}' called with wrong argument values:
+            {func_name!r} called with wrong argument values:
               {func_name}{args}
             Found signature:
               {func_name}{found}
@@ -126,7 +120,7 @@ def seterror_argument(args, func_name):
         return ValueError, msg
     type_str = ", ".join(type(arg).__name__ for arg in args)
     msg = dedent(f"""
-        '{func_name}' called with wrong argument types:
+        {func_name!r} called with wrong argument types:
           {func_name}({type_str})
         Supported signatures:
         """).strip()