Skip to content

Commit

Permalink
fix(python): classproperty not working with type checkers (#3694)
Browse files Browse the repository at this point in the history
The `jsii.python` submodule was not re-exported in `__init__.py`, which
made type resolves not manage to access the. `@classproperty` decorator
and default to treating it as `Any`.

This adds some missing type annotations, runs `pyright` on the runtime
library and tests thereof (as `mypy` did not identify this problem),
and adds the missing export declaration.

Fixes #3633



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
RomainMuller authored Aug 11, 2022
1 parent f584a17 commit 5413720
Show file tree
Hide file tree
Showing 19 changed files with 655 additions and 436 deletions.
6 changes: 4 additions & 2 deletions packages/@jsii/python-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@
"build": "cp ../../../README.md . && rm -f jsii-*.whl && npm run generate && npm run deps && npm run lint",
"lint": "ts-node build-tools/venv.ts black .",
"package": "package-python && package-private",
"test": "npm run test:gen && npm run test:run",
"test": "npm run test:gen && npm run test:run && npm run test:types",
"test:gen": "npm run deps && ts-node build-tools/gen-calc.ts",
"test:run": "ts-node build-tools/venv.ts py.test -v --mypy",
"test:types": "pyright -p .",
"test:update": "UPDATE_DIFF=1 npm run test"
},
"dependencies": {
Expand All @@ -40,6 +41,7 @@
"fs-extra": "^10.1.0",
"jsii-build-tools": "^0.0.0",
"jsii-calc": "^3.20.120",
"jsii-pacmak": "^0.0.0"
"jsii-pacmak": "^0.0.0",
"pyright": "^1.1.266"
}
}
5 changes: 5 additions & 0 deletions packages/@jsii/python-runtime/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,8 @@ exclude = '\.(git|mypy_cache|env)'

[tool.mypy]
ignore_missing_imports = true

[tool.pyright]
pythonVersion = "3.7"
venv = ".env"
venvPath = "."
2 changes: 2 additions & 0 deletions packages/@jsii/python-runtime/src/jsii/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
kernel,
proxy_for,
)
from . import python


# JS doesn't have distinct float or integer types, but we do. So we'll define our own
Expand Down Expand Up @@ -70,4 +71,5 @@
"ainvoke",
"sinvoke",
"stats",
"python",
]
4 changes: 2 additions & 2 deletions packages/@jsii/python-runtime/src/jsii/_kernel/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import itertools
from types import FunctionType, MethodType, BuiltinFunctionType, LambdaType

from typing import cast, Any, List, Optional, Type
from typing import cast, Any, List, Optional, Sequence, Type

import functools

Expand Down Expand Up @@ -300,7 +300,7 @@ def load(self, name: str, version: str, tarball: str) -> None:
self.provider.load(LoadRequest(name=name, version=version, tarball=tarball))

def invokeBinScript(
self, pkgname: str, script: str, args: Optional[List[Any]] = None
self, pkgname: str, script: str, args: Optional[Sequence[str]] = None
) -> InvokeScriptResponse:
if args is None:
args = []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ def _jsii_runtime(self) -> str:
return resources[jsii._embedded.jsii.ENTRYPOINT]

def _next_message(self) -> Mapping[Any, Any]:
assert self._process.stdout is not None
return json.loads(self._process.stdout.readline(), object_hook=ohook)

def start(self):
Expand Down Expand Up @@ -278,6 +279,7 @@ def stop(self):
# This process is closing already, un-registering the hook to not fire twice
atexit.unregister(self.stop)

assert self._process.stdin is not None
if not self._process.stdin.closed:
self._process.stdin.write(b'{"exit":0}\n')
# Close the process' STDIN, singaling we are done with it
Expand Down Expand Up @@ -311,6 +313,7 @@ def send(
data = json.dumps(req_dict, default=jdefault).encode("utf8")

# Send our data, ensure that it is framed with a trailing \n
assert self._process.stdin is not None
self._process.stdin.write(b"%b\n" % (data,))
self._process.stdin.flush()

Expand Down
12 changes: 7 additions & 5 deletions packages/@jsii/python-runtime/src/jsii/_reference_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ def resolve(self, kernel, ref):

python_props = {
python_name: kernel.get(remote_struct, jsii_name)
for python_name, jsii_name in python_jsii_mapping(data_type).items()
for python_name, jsii_name in (
python_jsii_mapping(data_type) or {}
).items()
}

return data_type(**python_props)
Expand All @@ -117,8 +119,8 @@ def resolve(self, kernel, ref):
return struct(
**{
python_name: kernel.get(remote_struct, jsii_name)
for python_name, jsii_name in python_jsii_mapping(
struct
for python_name, jsii_name in (
python_jsii_mapping(struct) or {}
).items()
}
)
Expand Down Expand Up @@ -152,7 +154,7 @@ def __getattr__(self, name):

def __setattr__(self, name, value):
if name == "_delegates":
return super.__setattr__(self, name, value)
return super().__setattr__(name, value)
for delegate in self._delegates:
if hasattr(delegate, name):
return setattr(delegate, name, value)
Expand All @@ -164,7 +166,7 @@ def new_combined_struct(structs: Iterable[Type]) -> Type:
label = " + ".join(struct.__name__ for struct in structs)

def __init__(self, **kwargs):
self._values: Mapping[str, Any] = kwargs
self._values = kwargs

def __eq__(self, rhs: Any) -> bool:
return isinstance(rhs, self.__class__) and rhs._values == self._values
Expand Down
6 changes: 3 additions & 3 deletions packages/@jsii/python-runtime/src/jsii/_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import attr

from typing import cast, Any, Callable, ClassVar, List, Optional, Mapping, Type, TypeVar
from typing import cast, Any, Callable, List, Optional, Mapping, Type, TypeVar

from . import _reference_map
from ._compat import importlib_resources
Expand Down Expand Up @@ -49,7 +49,7 @@ def load(cls, *args, _kernel=kernel, **kwargs) -> "JSIIAssembly":
def invokeBinScript(
cls, pkgname: str, script: str, *args: str, _kernel=kernel
) -> None:
response = _kernel.invokeBinScript(pkgname, script, *args)
response = _kernel.invokeBinScript(pkgname, script, args)
print(response.stdout)


Expand Down Expand Up @@ -154,7 +154,7 @@ def proxy_for(abstract_class: Type[Any]) -> Type[Any]:
if not hasattr(abstract_class, "__jsii_proxy_class__"):
raise TypeError(f"{abstract_class} is not a JSII Abstract class.")

return abstract_class.__jsii_proxy_class__()
return cast(Any, abstract_class).__jsii_proxy_class__()


def python_jsii_mapping(cls: Type[Any]) -> Optional[Mapping[str, str]]:
Expand Down
4 changes: 2 additions & 2 deletions packages/@jsii/python-runtime/src/jsii/_utils.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import functools

from typing import Any, Mapping, Type
from typing import Any, MutableMapping, Type


class Singleton(type):

_instances: Mapping[Type[Any], Any] = {}
_instances: MutableMapping[Type[Any], Any] = {}

def __call__(cls, *args, **kwargs):
if cls not in cls._instances:
Expand Down
46 changes: 30 additions & 16 deletions packages/@jsii/python-runtime/src/jsii/python.py
Original file line number Diff line number Diff line change
@@ -1,35 +1,49 @@
from typing import Any, Callable, Optional, Type, Union


class _ClassProperty:
def __init__(self, fget, fset=None):
def __init__(
self,
fget: classmethod,
fset: Optional[classmethod] = None,
):
self.fget = fget
self.fset = fset

def __get__(self, obj, klass=None):
def __get__(self, obj: Any, klass: Optional[Type] = None) -> Any:
if klass is None:
klass = type(obj)
return self.fget.__get__(obj, klass)(klass)

def __set__(self, obj, value):
if not self.fset:
raise AttributeError("Can't set attribute.")
return self.fget.__get__(obj, klass)()

def __set__(self, obj: Any, value: Any) -> None:
if self.fset is None:
raise AttributeError("Can't set class property (no setter)")
klass = type(obj)
return self.fset.__get__(obj, klass)(value)

def setter(self, func):
if not isinstance(func, (classmethod, staticmethod)):
func = classmethod(func)

self.fset = func

def setter(
self, fset: Union[Callable[[Any, Any], None], classmethod]
) -> "_ClassProperty":
"""
Defines the setter for a class property
"""
if not isinstance(fset, classmethod):
fset = classmethod(fset)
self.fset = fset
return self


def classproperty(func):
return _ClassProperty(func)
def classproperty(fget: Union[Callable[[Any], Any], classmethod]) -> _ClassProperty:
"""
Declares a new class property with the decorated getter.
"""
if not isinstance(fget, classmethod):
fget = classmethod(fget)
return _ClassProperty(fget)


class _ClassPropertyMeta(type):
def __setattr__(self, key, value):
def __setattr__(self, key: str, value: Any) -> None:
obj = getattr(self, key, None)
if isinstance(obj, _ClassProperty):
return obj.__set__(self, value)
Expand Down
30 changes: 19 additions & 11 deletions packages/@jsii/python-runtime/tests/test_compliance.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import platform

from datetime import datetime, timezone
from typing import cast, List

import pytest

Expand Down Expand Up @@ -272,7 +273,9 @@ def test_dynamicTypes():

# json (notice that when deserialized, it is deserialized as a map).
types.any_property = {"Goo": ["Hello", {"World": 123}]}
assert types.any_property.get("Goo")[1].get("World") == 123
got = types.any_property.get("Goo")
assert got is not None
assert got[1].get("World") == 123

# array
types.any_property = ["Hello", "World"]
Expand All @@ -292,7 +295,7 @@ def test_dynamicTypes():
# map of any
map_["Goo"] = 19_289_812
types.any_map_property = map_
types.any_map_property.get("Goo") == 19_289_812
assert types.any_map_property.get("Goo") == 19_289_812

# classes
mult = Multiply(Number(10), Number(20))
Expand Down Expand Up @@ -323,7 +326,7 @@ def test_unionTypes():

# array
types.union_array_property = [123, Number(33)]
assert types.union_array_property[1].value == 33
assert cast(Number, types.union_array_property[1]).value == 33


def test_createObjectAndCtorOverloads():
Expand Down Expand Up @@ -421,9 +424,11 @@ def test_maps():
calc2.add(20)
calc2.mul(2)

assert len(calc2.operations_map.get("add")) == 2
assert len(calc2.operations_map.get("mul")) == 1
assert calc2.operations_map.get("add")[1].value == 30
assert len(cast(List, calc2.operations_map.get("add"))) == 2
assert len(cast(List, calc2.operations_map.get("mul"))) == 1
got = calc2.operations_map.get("add")
assert got is not None
assert got[1].value == 30


def test_exceptions():
Expand Down Expand Up @@ -649,11 +654,11 @@ def read_only_string(self):

@property
def read_write_string(self):
return self.x + "?"
return f"{self.x}?"

@read_write_string.setter
def read_write_string(self, value):
self.x = value + "!"
self.x = f"{value}!"

obj = TInterfaceWithProperties()
interact = UsesInterfaceWithProperties(obj)
Expand Down Expand Up @@ -969,7 +974,7 @@ def test_passNestedStruct():

assert output.required == "hello"
assert output.optional is None
assert output.second_level.deeper_required_prop == "exists"
assert cast(SecondLevelStruct, output.second_level).deeper_required_prop == "exists"

# Test stringification
# Dicts are ordered in Python 3.7+, so this is fine: https://mail.python.org/pipermail/python-dev/2017-December/151283.html
Expand Down Expand Up @@ -1086,7 +1091,7 @@ def test_return_subclass_that_implements_interface_976():
def test_return_subclass_that_implements_interface_976_raises_attributeerror_when_using_non_existent_method():
obj = SomeTypeJsii976.return_return()
try:
print(obj.not_a_real_method_I_swear)
print(obj.not_a_real_method_I_swear) # type:ignore
failed = False
except AttributeError as err:
failed = True
Expand Down Expand Up @@ -1136,7 +1141,10 @@ def test_structs_are_undecorated_on_the_way_to_kernel():
json = JsonFormatter.stringify(
StructB(required_string="Bazinga!", optional_boolean=False)
)
assert loads(json) == {"requiredString": "Bazinga!", "optionalBoolean": False}
assert loads(cast(str, json)) == {
"requiredString": "Bazinga!",
"optionalBoolean": False,
}


def test_can_obtain_reference_with_overloaded_setter():
Expand Down
8 changes: 5 additions & 3 deletions packages/@jsii/python-runtime/tests/test_python.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from typing import cast, Any, Optional
import jsii
import pytest
import re
Expand Down Expand Up @@ -35,8 +36,7 @@ def test_descriptive_error_when_passing_function(self):
"type of argument value must be one of (int, float); got method instead"
),
):
# types: ignore
obj.add(self.test_descriptive_error_when_passing_function)
obj.add(cast(Any, self.test_descriptive_error_when_passing_function))

def test_implements_interface(self) -> None:
"""Checks that jsii-generated classes correctly implement the relevant jsii-generated interfaces."""
Expand All @@ -56,7 +56,9 @@ def baz_interface_func(b: IBaz) -> None:

def test_overrides_method_with_kwargs() -> None:
class Overridden(OverrideMe):
def implement_me(self, *, name: str, count: jsii.Number = None) -> bool:
def implement_me(
self, *, name: str, count: Optional[jsii.Number] = None
) -> bool:
return name == "John Doe" and count is None

assert OverrideMe.call_abstract(Overridden())
Expand Down
Loading

0 comments on commit 5413720

Please sign in to comment.