[feat] Type-check the generator. (#130)

This adds type-checking to the generator, and fixes a lot of
erroneous or incomplete type annotations.

This does _not_ add type-checking to the generated code (that
will come next).

Author: Luke Sneeringer

packages/gapic-generator/.circleci/config.yml

@@ -28,6 +28,8 @@ workflows:
               only: /^\d+\.\d+\.\d+$/
       - showcase:
           requires:
+            - docs
+            - mypy
             - showcase-unit-3.6
             - showcase-unit-3.7
           filters:
@@ -37,10 +39,13 @@ workflows:
           filters:
             tags:
               only: /^\d+\.\d+\.\d+$/
+      - mypy:
+          filters:
+            tags:
+              only: /^\d+\.\d+\.\d+$/
       - publish_package:
           requires:
             - showcase
-            - docs
           filters:
             branches:
               ignore: /.*/
@@ -49,7 +54,6 @@ workflows:
       - publish_image:
           requires:
             - showcase
-            - docs
           filters:
             branches:
               ignore: /.*/
@@ -67,6 +71,17 @@ jobs:
       - run:
           name: Build the documentation.
           command: nox -s docs
+  mypy:
+    docker:
+      - image: python:3.7-slim
+    steps:
+      - checkout
+      - run:
+          name: Install nox.
+          command: pip install nox
+      - run:
+          name: Check type annotations.
+          command: nox -s mypy
   publish_image:
     docker:
       - image: docker

packages/gapic-generator/.gitignore

@@ -56,3 +56,6 @@ coverage.xml
 # Make sure a generated file isn't accidentally committed.
 pylintrc
 pylintrc.test
+
+# Mypy
+.mypy_cache

packages/gapic-generator/gapic/cli/dump.py

@@ -18,8 +18,6 @@
 
 import click
 
-from google.protobuf.compiler import plugin_pb2
-
 
 @click.command()
 @click.option('--request', type=click.File('rb'), default=sys.stdin.buffer,
@@ -41,6 +39,6 @@ def dump(request: typing.BinaryIO) -> None:
     click.secho(
         'Request dumped to `request.desc`. '
         'This script will now exit 1 to satisfy protoc.',
-        file=sys.stderr, color='green',
+        file=sys.stderr, fg='green',
     )
     sys.exit(1)

packages/gapic-generator/gapic/generator/generator.py

@@ -12,10 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import collections
+from collections import OrderedDict
+from typing import Dict, Mapping
 import os
 import re
-from typing import Mapping, Sequence
 
 import jinja2
 
@@ -66,7 +66,7 @@ def get_response(self, api_schema: api.API) -> CodeGeneratorResponse:
             ~.CodeGeneratorResponse: A response describing appropriate
             files and contents. See ``plugin.proto``.
         """
-        output_files = collections.OrderedDict()
+        output_files: Dict[str, CodeGeneratorResponse.File] = OrderedDict()
 
         # Iterate over each template and add the appropriate output files
         # based on that template.
@@ -88,7 +88,7 @@ def _render_template(
             self,
             template_name: str, *,
             api_schema: api.API,
-            ) -> Sequence[CodeGeneratorResponse.File]:
+            ) -> Dict[str, CodeGeneratorResponse.File]:
         """Render the requested templates.
 
         Args:
@@ -103,7 +103,7 @@ def _render_template(
             Sequence[~.CodeGeneratorResponse.File]: A sequence of File
                 objects for inclusion in the final response.
         """
-        answer = collections.OrderedDict()
+        answer: Dict[str, CodeGeneratorResponse.File] = OrderedDict()
         skip_subpackages = False
 
         # Sanity check: Rendering per service and per proto would be a

packages/gapic-generator/gapic/generator/options.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from typing import Tuple
+from typing import Dict, List, Tuple
 import dataclasses
 import os
 import warnings
@@ -26,8 +26,8 @@ class Options:
     on unrecognized arguments (essentially, we throw them away, but we do
     warn if it looks like it was meant for us).
     """
-    templates: Tuple[str] = dataclasses.field(default=('DEFAULT',))
-    namespace: Tuple[str] = dataclasses.field(default=())
+    templates: Tuple[str, ...] = dataclasses.field(default=('DEFAULT',))
+    namespace: Tuple[str, ...] = dataclasses.field(default=())
     name: str = ''
 
     @classmethod
@@ -45,10 +45,10 @@ def build(cls, opt_string: str) -> 'Options':
             ~.Options: The Options instance.
         """
         # Parse out every option beginning with `python-gapic`
-        opts = {}
+        opts: Dict[str, List[str]] = {}
         for opt in opt_string.split(','):
             # Parse out the key and value.
-            value = True
+            value = 'true'
             if '=' in opt:
                 opt, value = opt.split('=')
 

packages/gapic-generator/gapic/schema/api.py

@@ -21,9 +21,9 @@
 import dataclasses
 import sys
 from itertools import chain
-from typing import Callable, List, Mapping, Sequence, Set, Tuple
+from typing import Callable, Dict, FrozenSet, Mapping, Sequence, Set, Tuple
 
-from google.longrunning import operations_pb2
+from google.longrunning import operations_pb2  # type: ignore
 from google.protobuf import descriptor_pb2
 
 from gapic.generator import options
@@ -100,21 +100,21 @@ def module_name(self) -> str:
         return to_snake_case(self.name.split('/')[-1][:-len('.proto')])
 
     @cached_property
-    def names(self) -> Set[str]:
+    def names(self) -> FrozenSet[str]:
         """Return a set of names used by this proto.
 
         This is used for detecting naming collisions in the module names
         used for imports.
         """
         # Add names of all enums, messages, and fields.
-        answer = {e.name for e in self.all_enums.values()}
+        answer: Set[str] = {e.name for e in self.all_enums.values()}
         for message in self.all_messages.values():
             answer = answer.union({f.name for f in message.fields.values()})
             answer.add(message.name)
 
         # Identify any import module names where the same module name is used
         # from distinct packages.
-        modules = {}
+        modules: Dict[str, Set[str]] = {}
         for t in chain(*[m.field_types for m in self.all_messages.values()]):
             modules.setdefault(t.ident.module, set())
             modules[t.ident.module].add(t.ident.package)
@@ -175,7 +175,7 @@ class API:
     """
     naming: api_naming.Naming
     all_protos: Mapping[str, Proto]
-    subpackage_view: Tuple[str] = dataclasses.field(default_factory=tuple)
+    subpackage_view: Tuple[str, ...] = dataclasses.field(default_factory=tuple)
 
     @classmethod
     def build(cls,
@@ -202,7 +202,7 @@ def build(cls,
 
         # Iterate over each FileDescriptorProto and fill out a Proto
         # object describing it, and save these to the instance.
-        protos = {}
+        protos: Dict[str, Proto] = {}
         for fd in file_descriptors:
             protos[fd.name] = _ProtoBuilder(
                 file_descriptor=fd,
@@ -256,7 +256,7 @@ def subpackages(self) -> Mapping[str, 'API']:
         Each value in the mapping is another API object, but the ``protos``
         property only shows protos belonging to the subpackage.
         """
-        answer = collections.OrderedDict()
+        answer: Dict[str, API] = collections.OrderedDict()
 
         # Get the actual subpackages we have.
         #
@@ -294,9 +294,9 @@ def __init__(self, file_descriptor: descriptor_pb2.FileDescriptorProto,
                  file_to_generate: bool,
                  naming: api_naming.Naming,
                  prior_protos: Mapping[str, Proto] = None):
-        self.proto_messages = {}
-        self.proto_enums = {}
-        self.proto_services = {}
+        self.proto_messages: Dict[str, wrappers.MessageType] = {}
+        self.proto_enums: Dict[str, wrappers.EnumType] = {}
+        self.proto_services: Dict[str, wrappers.Service] = {}
         self.file_descriptor = file_descriptor
         self.file_to_generate = file_to_generate
         self.prior_protos = prior_protos or {}
@@ -307,7 +307,8 @@ def __init__(self, file_descriptor: descriptor_pb2.FileDescriptorProto,
         # the "path", which is a sequence of integers described in more
         # detail below; this code simply shifts from a list to a dict,
         # with tuples of paths as the dictionary keys.
-        self.docs = {}
+        self.docs: Dict[Tuple[int, ...],
+                descriptor_pb2.SourceCodeInfo.Location] = {}
         for location in file_descriptor.source_code_info.location:
             self.docs[tuple(location.path)] = location
 
@@ -414,8 +415,9 @@ def api_messages(self) -> Mapping[str, wrappers.MessageType]:
             *[p.all_messages for p in self.prior_protos.values()],
         )
 
-    def _load_children(self, children: Sequence, loader: Callable, *,
-                       address: metadata.Address, path: Tuple[int]) -> Mapping:
+    def _load_children(self,
+                children: Sequence, loader: Callable, *,
+                address: metadata.Address, path: Tuple[int, ...]) -> Mapping:
         """Return wrapped versions of arbitrary children from a Descriptor.
 
         Args:
@@ -445,9 +447,10 @@ def _load_children(self, children: Sequence, loader: Callable, *,
             answer[wrapped.name] = wrapped
         return answer
 
-    def _get_fields(self, field_pbs: List[descriptor_pb2.FieldDescriptorProto],
-                    address: metadata.Address, path: Tuple[int],
-                    ) -> Mapping[str, wrappers.Field]:
+    def _get_fields(self,
+                field_pbs: Sequence[descriptor_pb2.FieldDescriptorProto],
+                address: metadata.Address, path: Tuple[int, ...],
+                ) -> Dict[str, wrappers.Field]:
         """Return a dictionary of wrapped fields for the given message.
 
         Args:
@@ -472,7 +475,7 @@ def _get_fields(self, field_pbs: List[descriptor_pb2.FieldDescriptorProto],
         # message wrapper is not yet created, because it needs this object
         # first) and this will be None. This case is addressed in the
         # `_load_message` method.
-        answer = collections.OrderedDict()
+        answer: Dict[str, wrappers.Field] = collections.OrderedDict()
         for field_pb, i in zip(field_pbs, range(0, sys.maxsize)):
             answer[field_pb.name] = wrappers.Field(
                 field_pb=field_pb,
@@ -487,9 +490,10 @@ def _get_fields(self, field_pbs: List[descriptor_pb2.FieldDescriptorProto],
         # Done; return the answer.
         return answer
 
-    def _get_methods(self, methods: List[descriptor_pb2.MethodDescriptorProto],
-                     address: metadata.Address, path: Tuple[int],
-                     ) -> Mapping[str, wrappers.Method]:
+    def _get_methods(self,
+            methods: Sequence[descriptor_pb2.MethodDescriptorProto],
+            address: metadata.Address, path: Tuple[int, ...],
+            ) -> Mapping[str, wrappers.Method]:
         """Return a dictionary of wrapped methods for the given service.
 
         Args:
@@ -505,7 +509,7 @@ def _get_methods(self, methods: List[descriptor_pb2.MethodDescriptorProto],
                 :class:`~.wrappers.Method` objects.
         """
         # Iterate over the methods and collect them into a dictionary.
-        answer = collections.OrderedDict()
+        answer: Dict[str, wrappers.Method] = collections.OrderedDict()
         for meth_pb, i in zip(methods, range(0, sys.maxsize)):
             lro = None
 

packages/gapic-generator/gapic/schema/imp.py

@@ -18,7 +18,7 @@
 
 @dataclasses.dataclass(frozen=True, order=True)
 class Import:
-    package: Tuple[str]
+    package: Tuple[str, ...]
     module: str
     alias: str = ''
 

packages/gapic-generator/gapic/schema/metadata.py

@@ -27,7 +27,7 @@
 """
 
 import dataclasses
-from typing import Tuple, Set
+from typing import FrozenSet, Tuple
 
 from google.protobuf import descriptor_pb2
 
@@ -40,13 +40,13 @@
 class Address:
     name: str = ''
     module: str = ''
-    module_path: Tuple[int] = dataclasses.field(default_factory=tuple)
-    package: Tuple[str] = dataclasses.field(default_factory=tuple)
-    parent: Tuple[str] = dataclasses.field(default_factory=tuple)
+    module_path: Tuple[int, ...] = dataclasses.field(default_factory=tuple)
+    package: Tuple[str, ...] = dataclasses.field(default_factory=tuple)
+    parent: Tuple[str, ...] = dataclasses.field(default_factory=tuple)
     api_naming: naming.Naming = dataclasses.field(
         default_factory=naming.Naming,
     )
-    collisions: Set[str] = dataclasses.field(default_factory=frozenset)
+    collisions: FrozenSet[str] = dataclasses.field(default_factory=frozenset)
 
     def __eq__(self, other) -> bool:
         return all([getattr(self, i) == getattr(other, i) for i
@@ -145,13 +145,13 @@ def sphinx(self) -> str:
         return self.name
 
     @property
-    def subpackage(self) -> Tuple[str]:
+    def subpackage(self) -> Tuple[str, ...]:
         """Return the subpackage below the versioned module name, if any."""
         return tuple(
             self.package[len(self.api_naming.proto_package.split('.')):]
         )
 
-    def child(self, child_name: str, path: Tuple[int]) -> 'Address':
+    def child(self, child_name: str, path: Tuple[int, ...]) -> 'Address':
         """Return a new child of the current Address.
 
         Args:
@@ -236,7 +236,7 @@ def resolve(self, selector: str) -> str:
             return f'{".".join(self.package)}.{selector}'
         return selector
 
-    def with_context(self, *, collisions: Set[str]) -> 'Address':
+    def with_context(self, *, collisions: FrozenSet[str]) -> 'Address':
         """Return a derivative of this address with the provided context.
 
         This method is used to address naming collisions. The returned
@@ -271,7 +271,7 @@ def doc(self):
             return '\n\n'.join(self.documentation.leading_detached_comments)
         return ''
 
-    def with_context(self, *, collisions: Set[str]) -> 'Metadata':
+    def with_context(self, *, collisions: FrozenSet[str]) -> 'Metadata':
         """Return a derivative of this metadata with the provided context.
 
         This method is used to address naming collisions. The returned