diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3df55f7..962943a 100644 --- a/.github/workflows/test.yml +++ b/.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 diff --git a/bin/lint_bindings.py b/bin/lint_bindings.py new file mode 100755 index 0000000..156012a --- /dev/null +++ b/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 +(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()) diff --git a/src/cpp/utils/py_monero_utils_bindings.cpp b/src/cpp/utils/py_monero_utils_bindings.cpp index 0249594..c740880 100644 --- a/src/cpp/utils/py_monero_utils_bindings.cpp +++ b/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")) diff --git a/src/cpp/wallet/py_monero_wallet_bindings.cpp b/src/cpp/wallet/py_monero_wallet_bindings.cpp index c737074..75bf629 100644 --- a/src/cpp/wallet/py_monero_wallet_bindings.cpp +++ b/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()); })