Produce TypeError Messages Using the Signature Module

The TypeError messages can now be produced, based upon the
signature module.

As a feature under test, we produce ValueErrors instead in
certain cases. This will probably improve, later.
We are currently investigating how much can be determined,
automatically.

Task-number: PYSIDE-795
Change-Id: Ie8a648beaf8a3bed388e3c01ba501bb36859722e
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>

12 files changed, 174 insertions, 128 deletions

diff --git a/sources/pyside2/PySide2/support/signature/mapping.py b/sources/pyside2/PySide2/support/signature/mapping.py
index 61fa2d41f..96afd3c7a 100644
--- a/sources/pyside2/PySide2/support/signature/mapping.py
+++ b/sources/pyside2/PySide2/support/signature/mapping.py
@@ -63,6 +63,7 @@ class Reloader(Sbk_Reloader):
         Sbk_Reloader.update(self, globals())
 
 update_mapping = Reloader().update
+namespace = globals()  # our module's __dict__, updated
 
 
 def init_QtCore():
diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
index 99bfae9f0..a62a0ac27 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
@@ -2083,93 +2083,8 @@ void CppGenerator::writeErrorSection(QTextStream& s, OverloadData& overloadData)
 
     QString argsVar = pythonFunctionWrapperUsesListOfArguments(overloadData)
         ? QLatin1String("args") : QLatin1String(PYTHON_ARG);
-    if (verboseErrorMessagesDisabled()) {
-        s << INDENT << "Shiboken::setErrorAboutWrongArguments(" << argsVar << ", \"" << funcName << "\", 0);" << endl;
-    } else {
-        QStringList overloadSignatures;
-        const OverloadData::MetaFunctionList &overloads = overloadData.overloads();
-        for (const AbstractMetaFunction *f : overloads) {
-            QStringList args;
-            const AbstractMetaArgumentList &arguments = f->arguments();
-            for (AbstractMetaArgument *arg : arguments)  {
-                QString strArg;
-                AbstractMetaType* argType = arg->type();
-                if (isCString(argType)) {
-                    strArg = QLatin1String("\" SBK_BYTES_NAME \"");
-                } else if (argType->isPrimitive()) {
-                    const PrimitiveTypeEntry* ptp = reinterpret_cast<const PrimitiveTypeEntry*>(argType->typeEntry());
-                    while (ptp->referencedTypeEntry())
-                        ptp = ptp->referencedTypeEntry();
-                    strArg = ptp->name();
-                    if (strArg == QLatin1String("QString")) {
-                        strArg = QLatin1String("unicode");
-                    } else if (strArg == QLatin1String("QChar")) {
-                        strArg = QLatin1String("1-unicode");
-                    } else {
-                        strArg = ptp->name();
-                        static const QRegularExpression regex(QStringLiteral("^signed\\s+"));
-                        Q_ASSERT(regex.isValid());
-                        strArg.remove(regex);
-                        if (strArg == QLatin1String("double"))
-                            strArg = QLatin1String("float");
-                    }
-                } else if (argType->typeEntry()->isContainer()) {
-                    strArg = argType->fullName();
-                    if (strArg == QLatin1String("QList") || strArg == QLatin1String("QVector")
-                        || strArg == QLatin1String("QLinkedList") || strArg == QLatin1String("QStack")
-                        || strArg == QLatin1String("QQueue")) {
-                        strArg = QLatin1String("list");
-                    } else if (strArg == QLatin1String("QMap") || strArg == QLatin1String("QHash")
-                               || strArg == QLatin1String("QMultiMap") || strArg == QLatin1String("QMultiHash")) {
-                        strArg = QLatin1String("dict");
-                    } else if (strArg == QLatin1String("QPair")) {
-                        strArg = QLatin1String("2-tuple");
-                    }
-                } else {
-                    strArg = argType->fullName();
-                    if (strArg == QLatin1String("PyUnicode"))
-                        strArg = QLatin1String("unicode");
-                    else if (strArg == QLatin1String("PyString"))
-                        strArg = QLatin1String("str");
-                    else if (strArg == QLatin1String("PyBytes"))
-                        strArg = QLatin1String("\" SBK_BYTES_NAME \"");
-                    else if (strArg == QLatin1String("PyByteArray"))
-                        strArg = QLatin1String("bytearray");
-                    else if (strArg == QLatin1String("PySequence"))
-                        strArg = QLatin1String("list");
-                    else if (strArg == QLatin1String("PyTuple"))
-                        strArg = QLatin1String("tuple");
-                    else if (strArg == QLatin1String("PyDict"))
-                        strArg = QLatin1String("dict");
-                    else if (strArg == QLatin1String("PyObject"))
-                        strArg = QLatin1String("object");
-                    else if (strArg == QLatin1String("PyCallable"))
-                        strArg = QLatin1String("callable");
-                    else if (strArg == QLatin1String("uchar"))
-                        strArg = QLatin1String("buffer"); // This depends on an inject code to be true, but if it's not true
-                                           // the function wont work at all, so it must be true.
-                }
-                if (!arg->defaultValueExpression().isEmpty()) {
-                    strArg += QLatin1String(" = ");
-                    if ((isCString(argType) || isPointerToWrapperType(argType))
-                        && arg->defaultValueExpression() == QLatin1String("0")) {
-                        strArg += QLatin1String("None");
-                    } else {
-                        QString e = arg->defaultValueExpression();
-                        e.replace(QLatin1String("::"), QLatin1String("."));
-                        e.replace(QLatin1String("\""), QLatin1String("\\\""));
-                        strArg += e;
-                    }
-                }
-                args << strArg;
-            }
-            overloadSignatures << QLatin1Char('"') + args.join(QLatin1String(", ")) + QLatin1Char('"');
-        }
-        s << INDENT << "const char* overloads[] = {" << overloadSignatures.join(QLatin1String(", "))
-              << ", 0};" << endl;
-        s << INDENT << "Shiboken::setErrorAboutWrongArguments(" << argsVar << ", \"" << funcName << "\", overloads);" << endl;
-    }
-    s << INDENT << returnStatement(m_currentErrorCode) << endl;
+    s << INDENT << "Shiboken::setErrorAboutWrongArguments(" << argsVar << ", \"" << funcName << "\");" << endl;
+    s << INDENT << "return " << m_currentErrorCode << ';' << endl;
 }
 
 void CppGenerator::writeFunctionReturnErrorCheckSection(QTextStream& s, bool hasReturnValue)
diff --git a/sources/shiboken2/libshiboken/basewrapper.cpp b/sources/shiboken2/libshiboken/basewrapper.cpp
index e820e749b..3e1a7e8e5 100644
--- a/sources/shiboken2/libshiboken/basewrapper.cpp
+++ b/sources/shiboken2/libshiboken/basewrapper.cpp
@@ -596,42 +596,10 @@ void init()
     shibokenAlreadInitialised = true;
 }
 
-void setErrorAboutWrongArguments(PyObject* args, const char* funcName, const char** cppOverloads)
-{
-    std::string msg;
-    std::string params;
-    if (args) {
-        if (PyTuple_Check(args)) {
-            for (Py_ssize_t i = 0, max = PyTuple_GET_SIZE(args); i < max; ++i) {
-                if (i)
-                    params += ", ";
-                PyObject* arg = PyTuple_GET_ITEM(args, i);
-                params += Py_TYPE(arg)->tp_name;
-            }
-        } else {
-            params = Py_TYPE(args)->tp_name;
-        }
-    }
-
-    if (!cppOverloads) {
-        msg = "'" + std::string(funcName) + "' called with wrong argument types: " + params;
-    } else {
-        msg = "'" + std::string(funcName) + "' called with wrong argument types:\n  ";
-        msg += funcName;
-        msg += '(';
-        msg += params;
-        msg += ")\n";
-        msg += "Supported signatures:";
-        for (int i = 0; cppOverloads[i]; ++i) {
-            msg += "\n  ";
-            msg += funcName;
-            msg += '(';
-            msg += cppOverloads[i];
-            msg += ')';
-        }
-    }
-    PyErr_SetString(PyExc_TypeError, msg.c_str());
-
+// setErrorAboutWrongArguments now gets overload info from the signature module.
+void setErrorAboutWrongArguments(PyObject *args, const char *funcName)
+{
+    SetError_Argument(args, funcName);
 }
 
 class FindBaseTypeVisitor : public HierarchyVisitor
diff --git a/sources/shiboken2/libshiboken/basewrapper.h b/sources/shiboken2/libshiboken/basewrapper.h
index 15682c600..e1cc64ba9 100644
--- a/sources/shiboken2/libshiboken/basewrapper.h
+++ b/sources/shiboken2/libshiboken/basewrapper.h
@@ -141,7 +141,9 @@ void callCppDestructor(void* cptr)
  *  Shiboken::importModule is DEPRECATED. Use Shiboken::Module::import() instead.
  */
 SBK_DEPRECATED(LIBSHIBOKEN_API bool importModule(const char* moduleName, PyTypeObject*** cppApiPtr));
-LIBSHIBOKEN_API void        setErrorAboutWrongArguments(PyObject* args, const char* funcName, const char** cppOverloads);
+
+// setErrorAboutWrongArguments now gets overload info from the signature module.
+LIBSHIBOKEN_API void        setErrorAboutWrongArguments(PyObject* args, const char* funcName);
 
 namespace ObjectType {
 
diff --git a/sources/shiboken2/libshiboken/signature.cpp b/sources/shiboken2/libshiboken/signature.cpp
index 564e5fcef..a8874e2e0 100644
--- a/sources/shiboken2/libshiboken/signature.cpp
+++ b/sources/shiboken2/libshiboken/signature.cpp
@@ -74,6 +74,7 @@ typedef struct safe_globals_struc {
     // init part 2: run module
     PyObject *sigparse_func;
     PyObject *createsig_func;
+    PyObject *seterror_argument_func;
 } safe_globals_struc, *safe_globals;
 
 static safe_globals pyside_globals = 0;
@@ -510,6 +511,9 @@ init_phase_2(safe_globals_struc *p, PyMethodDef *methods)
     p->createsig_func = PyObject_GetAttrString(p->helper_module, "create_signature");
     if (p->createsig_func == NULL)
         goto error;
+    p->seterror_argument_func = PyObject_GetAttrString(p->helper_module, "seterror_argument");
+    if (p->seterror_argument_func == NULL)
+        goto error;
     return 0;
 
 error:
@@ -950,4 +954,29 @@ FinishSignatureInitialization(PyObject *module, const char *signatures)
     }
 }
 
+void
+SetError_Argument(PyObject *args, const char *func_name)
+{
+    /*
+     * This function replaces the type error construction with extra
+     * overloads parameter in favor of using the signature module.
+     * Error messages are rare, so we do it completely in Python.
+     */
+    init_module_1();
+    init_module_2();
+    Shiboken::AutoDecRef res(PyObject_CallFunction(
+                                 pyside_globals->seterror_argument_func,
+                                 const_cast<char *>("(Os)"), args, func_name));
+    if (res.isNull()) {
+        PyErr_Print();
+        Py_FatalError("seterror_argument did not receive a result");
+    }
+    PyObject *err, *msg;
+    if (!PyArg_UnpackTuple(res, func_name, 2, 2, &err, &msg)) {
+        PyErr_Print();
+        Py_FatalError("unexpected failure in seterror_argument");
+    }
+    PyErr_SetObject(err, msg);
+}
+
 } //extern "C"
diff --git a/sources/shiboken2/libshiboken/signature.h b/sources/shiboken2/libshiboken/signature.h
index b65317662..c850698a6 100644
--- a/sources/shiboken2/libshiboken/signature.h
+++ b/sources/shiboken2/libshiboken/signature.h
@@ -47,6 +47,7 @@ extern "C"
 
 LIBSHIBOKEN_API int SbkSpecial_Type_Ready(PyObject *, PyTypeObject *, const char *); //WS
 LIBSHIBOKEN_API void FinishSignatureInitialization(PyObject *, const char *);
+LIBSHIBOKEN_API void SetError_Argument(PyObject *, const char *);
 
 } // extern "C"
 
diff --git a/sources/shiboken2/shibokenmodule/CMakeLists.txt b/sources/shiboken2/shibokenmodule/CMakeLists.txt
index 0eba3eaff..373ce102f 100644
--- a/sources/shiboken2/shibokenmodule/CMakeLists.txt
+++ b/sources/shiboken2/shibokenmodule/CMakeLists.txt
@@ -57,6 +57,8 @@ configure_file("${CMAKE_CURRENT_SOURCE_DIR}/support/__init__.py"
                "${CMAKE_CURRENT_BINARY_DIR}/support/__init__.py" COPYONLY)
 configure_file("${CMAKE_CURRENT_SOURCE_DIR}/support/signature/__init__.py"
                "${CMAKE_CURRENT_BINARY_DIR}/support/signature/__init__.py" COPYONLY)
+configure_file("${CMAKE_CURRENT_SOURCE_DIR}/support/signature/errorhandler.py"
+               "${CMAKE_CURRENT_BINARY_DIR}/support/signature/errorhandler.py" COPYONLY)
 configure_file("${CMAKE_CURRENT_SOURCE_DIR}/support/signature/layout.py"
                "${CMAKE_CURRENT_BINARY_DIR}/support/signature/layout.py" COPYONLY)
 configure_file("${CMAKE_CURRENT_SOURCE_DIR}/support/signature/loader.py"
diff --git a/sources/shiboken2/shibokenmodule/support/signature/errorhandler.py b/sources/shiboken2/shibokenmodule/support/signature/errorhandler.py
new file mode 100644
index 000000000..902bb05af
--- /dev/null
+++ b/sources/shiboken2/shibokenmodule/support/signature/errorhandler.py
@@ -0,0 +1,124 @@
+#############################################################################
+##
+## 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$
+##
+#############################################################################
+
+from __future__ import print_function, absolute_import
+
+"""
+errorhandler.py
+
+This module handles the TypeError messages which were previously
+produced by the generated C code.
+
+This version is at least consistent with the signatures, which
+are created by the same module.
+
+Experimentally, we are trying to guess those errors which are
+just the wrong number of elements in an iterator.
+At the moment, it is unclear whether the information given is
+enough to produce a useful ValueError.
+
+This matter will be improved in a later version.
+"""
+
+from signature_loader import get_signature, inspect
+from signature_loader.mapping import update_mapping, namespace
+from textwrap import dedent
+
+
+def qt_isinstance(inst, the_type):
+    if the_type == float:
+        return isinstance(inst, int) or isinstance(int, float)
+    try:
+        return isinstance(inst, the_type)
+    except TypeError as e:
+        print("FIXME", e)
+        return False
+
+
+def matched_type(args, sigs):
+    for sig in sigs:
+        params = list(sig.parameters.values())
+        if len(args) > len(params):
+            continue
+        if len(args) < len(params):
+            k = len(args)
+            if params[k].default is params[k].empty:
+                # this is a necessary parameter, so it fails.
+                continue
+        ok = True
+        for arg, param in zip(args, params):
+            ann = param.annotation
+            if qt_isinstance(arg, ann):
+                continue
+            ok = False
+        if ok:
+            return sig
+    return None
+
+
+def seterror_argument(args, func_name):
+    update_mapping()
+    func = eval(func_name, namespace)
+    sigs = get_signature(func, "typeerror")
+    if type(sigs) != list:
+        sigs = [sigs]
+    if type(args) != tuple:
+        args = (args,)
+    # temp!
+    found = matched_type(args, sigs)
+    if found:
+        msg = dedent("""
+            '{func_name}' called with wrong argument values:
+              {func_name}{args}
+            Found signature:
+              {func_name}{found}
+            """.format(**locals())).strip()
+        return ValueError, msg
+    type_str = ", ".join(type(arg).__name__ for arg in args)
+    msg = dedent("""
+        '{func_name}' called with wrong argument types:
+          {func_name}({type_str})
+        Supported signatures:
+        """.format(**locals())).strip()
+    for sig in sigs:
+        msg += "\n  {func_name}{sig}".format(**locals())
+    # We don't raise the error here, to avoid the loader in the traceback.
+    return TypeError, msg
+
+# end of file
diff --git a/sources/shiboken2/shibokenmodule/support/signature/loader.py b/sources/shiboken2/shibokenmodule/support/signature/loader.py
index de27d441c..be30483fe 100644
--- a/sources/shiboken2/shibokenmodule/support/signature/loader.py
+++ b/sources/shiboken2/shibokenmodule/support/signature/loader.py
@@ -181,6 +181,8 @@ with ensure_import_support():
         mapping = sbk_mapping
         mapping.__name__ = "mapping"
     put_into_loader_package(mapping)
+    from support.signature import errorhandler
+    put_into_loader_package(errorhandler)
     from support.signature import layout
     put_into_loader_package(layout)
     from support.signature.lib import enum_sig
diff --git a/sources/shiboken2/shibokenmodule/support/signature/mapping.py b/sources/shiboken2/shibokenmodule/support/signature/mapping.py
index 0195f0280..40db43729 100644
--- a/sources/shiboken2/shibokenmodule/support/signature/mapping.py
+++ b/sources/shiboken2/shibokenmodule/support/signature/mapping.py
@@ -203,6 +203,7 @@ def check_module(mod):
 
 update_mapping = Reloader().update
 type_map = {}
+namespace = globals()  # our module's __dict__
 
 
 def init_Shiboken():
diff --git a/sources/shiboken2/shibokenmodule/support/signature/parser.py b/sources/shiboken2/shibokenmodule/support/signature/parser.py
index 5178d9ef9..2f6e6e3f5 100644
--- a/sources/shiboken2/shibokenmodule/support/signature/parser.py
+++ b/sources/shiboken2/shibokenmodule/support/signature/parser.py
@@ -45,8 +45,7 @@ import warnings
 import types
 import keyword
 import functools
-from signature_loader.mapping import (
-    type_map, update_mapping, __dict__ as namespace)
+from signature_loader.mapping import type_map, update_mapping, namespace
 
 _DEBUG = False
 LIST_KEYWORDS = False
diff --git a/sources/shiboken2/tests/samplebinding/decisor_test.py b/sources/shiboken2/tests/samplebinding/decisor_test.py
index 9102e2d4e..734c43760 100644
--- a/sources/shiboken2/tests/samplebinding/decisor_test.py
+++ b/sources/shiboken2/tests/samplebinding/decisor_test.py
@@ -43,13 +43,15 @@ class DecisorTest(unittest.TestCase):
         This can trigger the bug #262, which means using an argument
         not provided by the user.'''
         pt = Point()
-        self.assertRaises(TypeError, SampleNamespace.forceDecisorSideA, pt)
+        # This exception may move from a TypeError to a ValueError.
+        self.assertRaises((TypeError, ValueError), SampleNamespace.forceDecisorSideA, pt)
 
     def testCallWithInvalidParametersSideB(self):
         '''Same as the previous test, but with an integer as first argument,
         just to complicate things for the overload method decisor.'''
         pt = Point()
-        self.assertRaises(TypeError, SampleNamespace.forceDecisorSideB, 1, pt)
+        # This exception may move from a TypeError to a ValueError.
+        self.assertRaises((TypeError, ValueError), SampleNamespace.forceDecisorSideB, 1, pt)
 
     def testDecideCallWithInheritance(self):
         '''Call methods overloads that receive parent and inheritor classes' instances.'''