diff --git a/docs/project/changelog.md b/docs/project/changelog.md index a3f224ac6..966baec80 100644 --- a/docs/project/changelog.md +++ b/docs/project/changelog.md @@ -53,6 +53,12 @@ myst: `{env: {HOME: whatever_directory}}`. {pr}`3870` +- {{ Fix }} A `PyProxy` of a callable is now an `instanceof Function`. (If you + are trying to feature detect whether something is callable or not in + JavaScript, the correct way is to use `typeof o === "function"`. But you may + have dependencies that don't do this correctly.) + {pr}`3925` + ### Packages - OpenBLAS has been added and scipy now uses OpenBLAS rather than CLAPACK diff --git a/src/core/pyproxy.ts b/src/core/pyproxy.ts index 00e53eb81..85ed0db7e 100644 --- a/src/core/pyproxy.ts +++ b/src/core/pyproxy.ts @@ -14,6 +14,7 @@ * See Makefile recipe for src/js/pyproxy.gen.ts */ +declare var Tests: any; declare var Module: any; declare var Hiwire: any; declare var API: any; @@ -322,9 +323,10 @@ Module.getPyProxyClass = function (flags: number) { descriptors, Object.getOwnPropertyDescriptors({ $$flags: flags }), ); - let new_proto = Object.create(PyProxy.prototype, descriptors); + const super_proto = flags & IS_CALLABLE ? PyProxyFunctionProto : PyProxyProto; + const sub_proto = Object.create(super_proto, descriptors); function NewPyProxyClass() {} - NewPyProxyClass.prototype = new_proto; + NewPyProxyClass.prototype = sub_proto; pyproxyClassMap.set(flags, NewPyProxyClass); return NewPyProxyClass; }; @@ -472,6 +474,12 @@ export class PyProxy { /** @private */ $$flags: number; + static [Symbol.hasInstance](obj: any): obj is PyProxy { + return [PyProxy, PyProxyFunction].some((cls) => + Function.prototype[Symbol.hasInstance].call(cls, obj), + ); + } + /** * @private * @hideconstructor @@ -734,6 +742,20 @@ export class PyProxy { } } +const PyProxyProto = PyProxy.prototype; +// For some weird reason in the node and firefox tests, the identity of +// `Function` changes between now and the test suite. Can't reproduce this +// outside the test suite though... +// See test_pyproxy_instanceof_function. +Tests.Function = Function; +const PyProxyFunctionProto = Object.create( + Function.prototype, + Object.getOwnPropertyDescriptors(PyProxyProto), +); +function PyProxyFunction() {} +PyProxyFunction.prototype = PyProxyFunctionProto; +globalThis.PyProxyFunction = PyProxyFunction; + /** * A :js:class:`~pyodide.ffi.PyProxy` whose proxied Python object has a :meth:`~object.__len__` * method. @@ -2024,6 +2046,36 @@ function python_pop(jsobj: any, pop_start: boolean): void { return Hiwire.pop_value(res); } +function filteredHasKey( + jsobj: PyProxy, + jskey: string | symbol, + filterProto: boolean, +) { + if (jsobj instanceof Function) { + // If we are a PyProxy of a callable we have to subclass function so that if + // someone feature detects callables with `instanceof Function` it works + // correctly. But the callable might have attributes `name` and `length` and + // we don't want to shadow them with the values from `Function.prototype`. + return ( + jskey in jsobj && + !( + [ + "name", + "length", + "caller", + "arguments", + // we are required by JS law to return `true` for `"prototype" in pycallable` + // but we are allowed to return the value of `getattr(pycallable, "prototype")`. + // So we filter prototype out of the "get" trap but not out of the "has" trap + filterProto ? "prototype" : undefined, + ] as (string | symbol)[] + ).includes(jskey) + ); + } else { + return jskey in jsobj; + } +} + // See explanation of which methods should be defined here and what they do // here: // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy @@ -2031,11 +2083,10 @@ const PyProxyHandlers = { isExtensible(): boolean { return true; }, - has(jsobj: PyProxy, jskey: any): boolean { + has(jsobj: PyProxy, jskey: string | symbol): boolean { // Note: must report "prototype" in proxy when we are callable. // (We can return the wrong value from "get" handler though.) - let objHasKey = Reflect.has(jsobj, jskey); - if (objHasKey) { + if (filteredHasKey(jsobj, jskey, false)) { return true; } // python_hasattr will crash if given a Symbol. @@ -2047,13 +2098,13 @@ const PyProxyHandlers = { } return python_hasattr(jsobj, jskey); }, - get(jsobj: PyProxy, jskey: any): any { + get(jsobj: PyProxy, jskey: string | symbol): any { // Preference order: // 1. stuff from JavaScript // 2. the result of Python getattr // python_getattr will crash if given a Symbol. - if (jskey in jsobj || typeof jskey === "symbol") { + if (typeof jskey === "symbol" || filteredHasKey(jsobj, jskey, true)) { return Reflect.get(jsobj, jskey); } // If keys start with $ remove the $. User can use initial $ to @@ -2067,10 +2118,10 @@ const PyProxyHandlers = { return Hiwire.pop_value(idresult); } }, - set(jsobj: PyProxy, jskey: any, jsval: any): boolean { + set(jsobj: PyProxy, jskey: string | symbol, jsval: any): boolean { let descr = Object.getOwnPropertyDescriptor(jsobj, jskey); if (descr && !descr.writable) { - throw new TypeError(`Cannot set read only field '${jskey}'`); + throw new TypeError(`Cannot set read only field '${String(jskey)}'`); } // python_setattr will crash if given a Symbol. if (typeof jskey === "symbol") { @@ -2082,10 +2133,10 @@ const PyProxyHandlers = { python_setattr(jsobj, jskey, jsval); return true; }, - deleteProperty(jsobj: PyProxy, jskey: any): boolean { + deleteProperty(jsobj: PyProxy, jskey: string | symbol): boolean { let descr = Object.getOwnPropertyDescriptor(jsobj, jskey); if (descr && !descr.writable) { - throw new TypeError(`Cannot delete read only field '${jskey}'`); + throw new TypeError(`Cannot delete read only field '${String(jskey)}'`); } if (typeof jskey === "symbol") { return Reflect.deleteProperty(jsobj, jskey); diff --git a/src/tests/test_pyproxy.py b/src/tests/test_pyproxy.py index 1b021c050..27cb462f5 100644 --- a/src/tests/test_pyproxy.py +++ b/src/tests/test_pyproxy.py @@ -527,7 +527,7 @@ def test_pyproxy_mixins3(selenium): ) -def test_pyproxy_mixins4(selenium): +def test_pyproxy_mixins41(selenium): selenium.run_js( """ [Test, t] = pyodide.runPython(` @@ -536,24 +536,47 @@ def test_pyproxy_mixins4(selenium): prototype="prototype" name="me" length=7 + def __call__(self, x): + return x + 1 + from pyodide.ffi import to_js to_js([Test, Test()]) `); assert(() => Test.$prototype === "prototype"); - assert(() => Test.prototype === undefined); + assert(() => Test.prototype === "prototype"); assert(() => Test.name==="me"); assert(() => Test.length === 7); assert(() => t.caller === "fifty"); + assert(() => "prototype" in t); assert(() => t.prototype === "prototype"); assert(() => t.name==="me"); assert(() => t.length === 7); + assert(() => t(7) === 8); Test.destroy(); t.destroy(); """ ) +def test_pyproxy_mixins42(selenium): + selenium.run_js( + """ + let t = pyodide.runPython(` + class Test: + def __call__(self, x): + return x + 1 + + from pyodide.ffi import to_js + Test() + `); + assert(() => "prototype" in t); + assert(() => t.prototype === undefined); + t.destroy(); + """ + ) + + def test_pyproxy_mixins5(selenium): selenium.run_js( """ @@ -2096,3 +2119,101 @@ def test_pyproxy_of_list_fill(selenium, func): assert func(a) is a func(ajs) assert a == ajs.to_py() + + +def test_pyproxy_instanceof_function(selenium): + weird_function_shim = "" + if selenium.browser in ["firefox", "node"]: + # A hack to make the test work: In node and firefox this test fails. But + # I can't reproduce the failure in a normal browser / outside of the + # test suite. The trouble seems to be that the value of + # `globalThis.Function` changes its identity from when we define + # `PyProxyFunction` to when we execute this test. So we store `Function` + # on `pyodide._api.tests` so we can retrieve the original value of it + # for the test. This is nonsense but because the failure only occurs in + # the test suite and not in real life I guess it's okay???? + # Also, no clue how node and firefox are affected but not Chrome. + weird_function_shim = "let Function = pyodide._api.tests.Function;" + + selenium.run_js( + f""" + {weird_function_shim} + """ + """ + const pyFunc_0 = pyodide.runPython(` + lambda: print("zero") + `); + + const pyFunc_1 = pyodide.runPython(` + def foo(): + print("two") + foo + `); + + const pyFunc_2 = pyodide.runPython(` + class A(): + def a(self): + print("three") # method from class + A.a + `); + + const pyFunc_3 = pyodide.runPython(` + class B(): + def __call__(self): + print("five (B as a callable instance)") + + b = B() + b + `); + + assert(() => pyFunc_0 instanceof Function); + assert(() => pyFunc_0 instanceof pyodide.ffi.PyProxy); + assert(() => pyFunc_0 instanceof pyodide.ffi.PyCallable); + + assert(() => pyFunc_1 instanceof Function); + assert(() => pyFunc_1 instanceof pyodide.ffi.PyProxy); + assert(() => pyFunc_1 instanceof pyodide.ffi.PyCallable); + + assert(() => pyFunc_2 instanceof Function); + assert(() => pyFunc_2 instanceof pyodide.ffi.PyProxy); + assert(() => pyFunc_2 instanceof pyodide.ffi.PyCallable); + + assert(() => pyFunc_3 instanceof Function); + assert(() => pyFunc_3 instanceof pyodide.ffi.PyProxy); + assert(() => pyFunc_3 instanceof pyodide.ffi.PyCallable); + + d = pyodide.runPython("{}"); + assert(() => !(d instanceof Function)); + assert(() => !(d instanceof pyodide.ffi.PyCallable)); + assert(() => d instanceof pyodide.ffi.PyProxy); + assert(() => d instanceof pyFunc_0.constructor); + assert(() => pyFunc_0 instanceof d.constructor); + + for(const p of [pyFunc_0, pyFunc_1, pyFunc_2, pyFunc_3, d]) { + p.destroy(); + } + """ + ) + + +def test_pyproxy_callable_prototype(selenium): + r = selenium.run_js( + """ + const o = pyodide.runPython("lambda:None"); + const res = Object.fromEntries(Reflect.ownKeys(Function.prototype).map(k => [k.toString(), k in o])); + o.destroy(); + return res; + """ + ) + assert r == { + "length": False, + "name": False, + "arguments": False, + "caller": False, + "constructor": True, + "apply": True, + "bind": True, + "call": True, + "toString": True, + "Symbol(Symbol.hasInstance)": True, + }