ci: lint bindings

.github/workflows/test.yml

@@ -51,7 +51,7 @@ jobs:
         run: |
           sudo apt update
           sudo apt install -y build-essential cmake pkg-config libssl-dev libzmq3-dev libunbound-dev libsodium-dev libunwind8-dev liblzma-dev libreadline6-dev libexpat1-dev libpgm-dev qttools5-dev-tools libhidapi-dev libusb-1.0-0-dev libprotobuf-dev protobuf-compiler libudev-dev libboost-chrono-dev libboost-date-time-dev libboost-filesystem-dev libboost-locale-dev libboost-program-options-dev libboost-regex-dev libboost-serialization-dev libboost-system-dev libboost-thread-dev python3 ccache doxygen graphviz git curl autoconf libtool gperf nettle-dev libevent-dev debhelper python3-all python3-pip python3-pybind11 lcov python3-scikit-build-core
-          pip3 install pybind11-stubgen pytest pytest-rerunfailures pytest-cov pytest-timeout pyproject-metadata --break-system-packages
+          pip3 install pybind11-stubgen==2.5.5 pytest pytest-rerunfailures pytest-cov pytest-timeout pyproject-metadata --break-system-packages
 
       - name: Install expat
         run: |
@@ -116,6 +116,12 @@ jobs:
           cmake --build build
           cp build/monero.cpython-312-x86_64-linux-gnu.so ~/.local/lib/python3.12/site-packages/
 
+      # stubgen, pinned because a new version can report more, reads the signatures that pybind11 writes in
+      # the docstrings and fails on what is wrong with the bindings: a C++ type that leaked into one, a default
+      # that is only a repr, a parameter with no name. The bindings are the same on every platform.
+      - name: Lint the bindings with pybind11-stubgen
+        run: python3 bin/lint_bindings.py
+
       - name: Show ccache statistics
         if: ${{ !cancelled() }}
         run: ccache --show-stats

bin/lint_bindings.py

@@ -0,0 +1,80 @@
+#!/usr/bin/env python3
+"""Lints the bindings of the installed module with pybind11-stubgen.
+
+pybind11 writes the signature of every function it binds in the docstring of the function, and stubgen turns
+those into stubs. What it can not make sense of is what is wrong with the bindings: a C++ type that leaked into
+a docstring because it is not registered, a default value that is only a repr like <MoneroNetworkType.MAINNET: 0>
+(py::arg_v gives it a text), a name that does not resolve.
+
+This fails on an error or a warning of stubgen, and on a public function with parameters that have no name (arg0,
+arg1, ...): a py::arg is missing in the binding, so the function can not be called with keywords and its docs say
+nothing about the parameter. The special methods are left out, pybind11 names those of the containers it binds
+itself, and so are the setters of the properties.
+
+    pip install pybind11-stubgen
+    bin/lint_bindings.py [module]      # monero, which has to be installed or on the path
+"""
+import ast
+import re
+import subprocess
+import sys
+import tempfile
+from pathlib import Path
+
+
+def unnamed_parameters(tree):
+    """The public functions of a stub that have parameters named by pybind11: ['Class.function(arg0)', ...]"""
+    found = []
+
+    def visit(node, owner):
+        for child in ast.iter_child_nodes(node):
+            if isinstance(child, ast.ClassDef):
+                visit(child, child.name)
+            elif isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef)):
+                if child.name.startswith('_'):
+                    continue
+                if any(isinstance(d, ast.Attribute) and d.attr == 'setter' for d in child.decorator_list):
+                    continue
+                args = child.args
+                names = [a.arg for a in args.posonlyargs + args.args + args.kwonlyargs]
+                unnamed = [n for n in names if re.fullmatch(r'arg\d+', n)]
+                if unnamed:
+                    found.append('%s.%s(%s)' % (owner, child.name, ', '.join(unnamed)))
+
+    visit(tree, 'module')
+    return sorted(set(found))
+
+
+def main():
+    module = sys.argv[1] if len(sys.argv) > 1 else 'monero'
+    ok = True
+    with tempfile.TemporaryDirectory() as out:
+        run = subprocess.run([sys.executable, '-m', 'pybind11_stubgen', module, '-o', out, '--exit-code'],
+                             stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True)
+        if run.stdout.strip():
+            print(run.stdout.strip())
+        if run.returncode != 0:
+            print('::error::pybind11-stubgen found errors in the bindings of %s' % module)
+            ok = False
+        elif '[WARNING]' in run.stdout:
+            print('::error::pybind11-stubgen warns about the bindings of %s' % module)
+            ok = False
+
+        # no stubs are written when stubgen finds errors
+        stubs = sorted(Path(out).rglob('*.pyi'))
+        offenders = []
+        for stub in stubs:
+            offenders += unnamed_parameters(ast.parse(stub.read_text()))
+        if offenders:
+            print('Public functions with parameters that have no name, add a py::arg to their binding:')
+            for o in offenders:
+                print('  ' + o)
+            print('::error::%d public functions of %s have unnamed parameters' % (len(offenders), module))
+            ok = False
+        elif stubs:
+            print('%d stub files generated, no unnamed parameters' % len(stubs))
+    return 0 if ok else 1
+
+
+if __name__ == '__main__':
+    sys.exit(main())

src/cpp/utils/py_monero_utils_bindings.cpp

@@ -160,10 +160,10 @@ void py_monero_bind_utils(py::module_& m, PyMoneroTypes& t) {
     }, py::arg("outputs"))
     .def_static("get_payment_uri", [](const monero_tx_config &config, monero_network_type network_type) {
       MONERO_CATCH_AND_RETHROW(monero_utils::get_payment_uri(config, network_type));
-    }, py::arg("config"), py::arg("network_type") = monero_network_type::MAINNET)
+    }, py::arg("config"), py::arg_v("network_type", monero_network_type::MAINNET, "MoneroNetworkType.MAINNET"))
     .def_static("parse_payment_uri", [](const std::string& uri, monero_network_type network_type) {
       MONERO_CATCH_AND_RETHROW(monero_utils::parse_payment_uri(uri, network_type));
-    }, py::arg("uri"), py::arg("network_type") = monero_network_type::MAINNET)
+    }, py::arg("uri"), py::arg_v("network_type", monero_network_type::MAINNET, "MoneroNetworkType.MAINNET"))
     .def_static("xmr_to_atomic_units", [](double amount_xmr) {
       MONERO_CATCH_AND_RETHROW(monero_utils::xmr_to_atomic_units(amount_xmr));
     }, py::arg("amount_xmr"))

src/cpp/wallet/py_monero_wallet_bindings.cpp

@@ -479,7 +479,7 @@ void py_monero_bind_wallet(py::module_& m, PyMoneroTypes& t) {
         self.m_destinations.push_back(dest);
       }
       else self.m_destinations[0]->m_address = address;
-    })
+    }, py::arg("address"))
     .def("copy", [](monero_tx_config& self) {
       MONERO_CATCH_AND_RETHROW(self.copy());
     })