Misc cleanup, see commit body (#227)

- Enable oneof_enum test case that passes now (removed the xfail)
- Switch from toml to tomlkit as a dev dep for better toml support
- upgrade poethepoet to latest stable release
- use full table format for poe tasks to avoid long lines in pyproject.toml
- remove redundant _WrappedMessage class
- fix various Mypy warnings
- reformat some comments for consistent line length

Author: nat-n

poetry.lock

@@ -20,6 +20,7 @@ jinja2 = { version = "^2.11.2", optional = true }
 python-dateutil = "^2.8"
 
 [tool.poetry.dev-dependencies]
+asv = "^0.4.2"
 black = "^20.8b1"
 bpython = "^0.19"
 grpcio-tools = "^1.30.0"
@@ -31,11 +32,10 @@ pytest = "^5.4.2"
 pytest-asyncio = "^0.12.0"
 pytest-cov = "^2.9.0"
 pytest-mock = "^3.1.1"
-toml = "^0.10.1"
-tox = "^3.15.1"
 sphinx = "3.1.2"
 sphinx-rtd-theme = "0.5.0"
-asv = "^0.4.2"
+tomlkit = "^0.7.0"
+tox = "^3.15.1"
 
 
 [tool.poetry.scripts]
@@ -44,29 +44,62 @@ protoc-gen-python_betterproto = "betterproto.plugin:main"
 [tool.poetry.extras]
 compiler = ["black", "jinja2"]
 
-[tool.poe.tasks]
+
 # Dev workflow tasks
-generate    = { script = "tests.generate:main", help = "Generate test cases (do this once before running test)" }
-test        = { cmd = "pytest --cov src", help = "Run tests" }
-types       = { cmd = "mypy src --ignore-missing-imports", help = "Check types with mypy" }
-format      = { cmd = "black . --exclude tests/output_",   help = "Apply black formatting to source code" }
-docs        = { cmd = "sphinx-build docs docs/build", help = "Build the sphinx docs"}
-bench       = { shell = "asv run master^! && asv run HEAD^! && asv compare master HEAD", help = "Benchmark current commit vs. master branch"}
-clean       = { cmd = "rm -rf .asv .coverage .mypy_cache .pytest_cache dist betterproto.egg-info **/__pycache__ tests/output_*", help  = "Clean out generated files from the workspace" }
-
-generate_lib.cmd = """
+
+[tool.poe.tasks.generate]
+script = "tests.generate:main"
+help = "Generate test cases (do this once before running test)"
+
+[tool.poe.tasks.test]
+cmd = "pytest"
+help = "Run tests"
+
+[tool.poe.tasks.types]
+cmd  = "mypy src --ignore-missing-imports"
+help = "Check types with mypy"
+
+[tool.poe.tasks.format]
+cmd  = "black . --exclude tests/output_"
+help = "Apply black formatting to source code"
+
+[tool.poe.tasks.docs]
+cmd  = "sphinx-build docs docs/build"
+help = "Build the sphinx docs"
+
+[tool.poe.tasks.bench]
+shell = "asv run master^! && asv run HEAD^! && asv compare master HEAD"
+help  = "Benchmark current commit vs. master branch"
+
+[tool.poe.tasks.clean]
+cmd  = """
+rm -rf .asv .coverage .mypy_cache .pytest_cache
+       dist betterproto.egg-info **/__pycache__
+       testsoutput_*
+"""
+help = "Clean out generated files from the workspace"
+
+[tool.poe.tasks.generate_lib]
+cmd = """
 protoc
     --plugin=protoc-gen-custom=src/betterproto/plugin/main.py
     --custom_opt=INCLUDE_GOOGLE
     --custom_out=src/betterproto/lib
     -I /usr/local/include/
     /usr/local/include/google/protobuf/**/*.proto
 """
-generate_lib.help = "Regenerate the types in betterproto.lib.google"
+help = "Regenerate the types in betterproto.lib.google"
 
 # CI tasks
-full-test   = { shell = "poe generate && tox", help = "Run tests with multiple pythons" }
-check-style = { cmd = "black . --check --diff --exclude tests/output_", help = "Check if code style is correct"}
+
+[tool.poe.tasks.full-test]
+shell = "poe generate && tox"
+help = "Run tests with multiple pythons"
+
+[tool.poe.tasks.check-style]
+cmd = "black . --check --diff --exclude tests/output_"
+help = "Check if code style is correct"
+
 
 [tool.black]
 target-version = ['py36']

pyproject.toml

@@ -15,6 +15,7 @@
     Callable,
     Dict,
     Generator,
+    Iterable,
     List,
     Optional,
     Set,
@@ -272,7 +273,7 @@ def from_string(cls, name: str) -> "Enum":
             The member was not found in the Enum.
         """
         try:
-            return cls._member_map_[name]
+            return cls._member_map_[name]  # type: ignore
         except KeyError as e:
             raise ValueError(f"Unknown value {name} for enum {cls.__name__}") from e
 
@@ -522,13 +523,13 @@ def __init__(self, cls: Type["Message"]):
 
     @staticmethod
     def _get_default_gen(
-        cls: Type["Message"], fields: List[dataclasses.Field]
+        cls: Type["Message"], fields: Iterable[dataclasses.Field]
     ) -> Dict[str, Callable[[], Any]]:
         return {field.name: cls._get_field_default_gen(field) for field in fields}
 
     @staticmethod
     def _get_cls_by_field(
-        cls: Type["Message"], fields: List[dataclasses.Field]
+        cls: Type["Message"], fields: Iterable[dataclasses.Field]
     ) -> Dict[str, Type]:
         field_cls = {}
 
@@ -687,7 +688,7 @@ def _betterproto(self) -> ProtoClassMetadata:
         meta = getattr(self.__class__, "_betterproto_meta", None)
         if not meta:
             meta = ProtoClassMetadata(self.__class__)
-            self.__class__._betterproto_meta = meta
+            self.__class__._betterproto_meta = meta  # type: ignore
         return meta
 
     def __bytes__(self) -> bytes:
@@ -763,7 +764,7 @@ def __bytes__(self) -> bytes:
                     meta.number,
                     meta.proto_type,
                     value,
-                    serialize_empty=serialize_empty or selected_in_group,
+                    serialize_empty=serialize_empty or bool(selected_in_group),
                     wraps=meta.wraps or "",
                 )
 
@@ -1067,7 +1068,7 @@ def to_dict(
                         output[cased_name] = b64encode(value).decode("utf8")
                 elif meta.proto_type == TYPE_ENUM:
                     if field_is_repeated:
-                        enum_class: Type[Enum] = field_types[field_name].__args__[0]
+                        enum_class = field_types[field_name].__args__[0]
                         if isinstance(value, typing.Iterable) and not isinstance(
                             value, str
                         ):
@@ -1076,7 +1077,7 @@ def to_dict(
                             # transparently upgrade single value to repeated
                             output[cased_name] = [enum_class(value).name]
                     else:
-                        enum_class: Type[Enum] = field_types[field_name]  # noqa
+                        enum_class = field_types[field_name]  # noqa
                         output[cased_name] = enum_class(value).name
                 elif meta.proto_type in (TYPE_FLOAT, TYPE_DOUBLE):
                     if field_is_repeated:
@@ -1293,23 +1294,6 @@ def timestamp_to_json(dt: datetime) -> str:
         return f"{result}.{nanos:09d}"
 
 
-class _WrappedMessage(Message):
-    """
-    Google protobuf wrapper types base class. JSON representation is just the
-    value itself.
-    """
-
-    value: Any
-
-    def to_dict(self, casing: Casing = Casing.CAMEL) -> Any:
-        return self.value
-
-    def from_dict(self: T, value: Any) -> T:
-        if value is not None:
-            self.value = value
-        return self
-
-
 def _get_wrapper(proto_type: str) -> Type:
     """Get the wrapper message class for a wrapped type."""
 

src/betterproto/__init__.py

@@ -100,8 +100,9 @@ def reference_descendent(
     current_package: List[str], imports: Set[str], py_package: List[str], py_type: str
 ) -> str:
     """
-    Returns a reference to a python type in a package that is a descendent of the current package,
-    and adds the required import that is aliased to avoid name conflicts.
+    Returns a reference to a python type in a package that is a descendent of the
+    current package, and adds the required import that is aliased to avoid name
+    conflicts.
     """
     importing_descendent = py_package[len(current_package) :]
     string_from = ".".join(importing_descendent[:-1])
@@ -119,8 +120,9 @@ def reference_ancestor(
     current_package: List[str], imports: Set[str], py_package: List[str], py_type: str
 ) -> str:
     """
-    Returns a reference to a python type in a package which is an ancestor to the current package,
-    and adds the required import that is aliased (if possible) to avoid name conflicts.
+    Returns a reference to a python type in a package which is an ancestor to the
+    current package, and adds the required import that is aliased (if possible) to avoid
+    name conflicts.
 
     Adds trailing __ to avoid name mangling (python.org/dev/peps/pep-0008/#id34).
     """
@@ -141,10 +143,10 @@ def reference_cousin(
     current_package: List[str], imports: Set[str], py_package: List[str], py_type: str
 ) -> str:
     """
-    Returns a reference to a python type in a package that is not descendent, ancestor or sibling,
-    and adds the required import that is aliased to avoid name conflicts.
+    Returns a reference to a python type in a package that is not descendent, ancestor
+    or sibling, and adds the required import that is aliased to avoid name conflicts.
     """
-    shared_ancestry = os.path.commonprefix([current_package, py_package])
+    shared_ancestry = os.path.commonprefix([current_package, py_package])  # type: ignore
     distance_up = len(current_package) - len(shared_ancestry)
     string_from = f".{'.' * distance_up}" + ".".join(
         py_package[len(shared_ancestry) : -1]

src/betterproto/compile/importing.py

@@ -70,7 +70,7 @@ class AsyncChannel(AsyncIterable[T]):
     """
 
     def __init__(self, *, buffer_limit: int = 0, close: bool = False):
-        self._queue: asyncio.Queue[Union[T, object]] = asyncio.Queue(buffer_limit)
+        self._queue: asyncio.Queue[T] = asyncio.Queue(buffer_limit)
         self._closed = False
         self._waiting_receivers: int = 0
         # Track whether flush has been invoked so it can only happen once

src/betterproto/grpc/util/async_channel.py

@@ -170,6 +170,8 @@ class ProtoContentBase:
     comment_indent: int = 4
     parent: Union["betterproto.Message", "OutputTemplate"]
 
+    __dataclass_fields__: Dict[str, object]
+
     def __post_init__(self) -> None:
         """Checks that no fake default fields were left as placeholders."""
         for field_name, field_val in self.__dataclass_fields__.items():

src/betterproto/plugin/models.py

@@ -13,7 +13,7 @@
 import itertools
 import pathlib
 import sys
-from typing import Iterator, List, Tuple, TYPE_CHECKING, Union
+from typing import Iterator, List, Set, Tuple, TYPE_CHECKING, Union
 from .compiler import outputfile_compiler
 from .models import (
     EnumDefinitionCompiler,
@@ -38,7 +38,7 @@ def traverse(
 ) -> "itertools.chain[Tuple[Union[str, EnumDescriptorProto], List[int]]]":
     # Todo: Keep information about nested hierarchy
     def _traverse(
-        path: List[int], items: List["Descriptor"], prefix=""
+        path: List[int], items: List["EnumDescriptorProto"], prefix=""
     ) -> Iterator[Tuple[Union[str, EnumDescriptorProto], List[int]]]:
         for i, item in enumerate(items):
             # Adjust the name since we flatten the hierarchy.

src/betterproto/plugin/parser.py

@@ -1,7 +1,6 @@
 # Test cases that are expected to fail, e.g. unimplemented features or bug-fixes.
 # Remove from list when fixed.
 xfail = {
-    "oneof_enum",  # 63
     "namespace_keywords",  # 70
     "namespace_builtin_types",  # 53
     "googletypes_struct",  # 9

tests/inputs/config.py

@@ -1,13 +1,13 @@
 from betterproto import __version__
 from pathlib import Path
-import toml
+import tomlkit
 
 PROJECT_TOML = Path(__file__).joinpath("..", "..", "pyproject.toml").resolve()
 
 
 def test_version():
     with PROJECT_TOML.open() as toml_file:
-        project_config = toml.load(toml_file)
+        project_config = tomlkit.loads(toml_file.read())
     assert (
         __version__ == project_config["tool"]["poetry"]["version"]
     ), "Project version should match in package and package config"