Determine naming collisions up front. (#58)

This commit determines and resolves naming collisions logically
prior to templates (in other words, so templates do not have to worry
about it and they just get the "right thing").

Author: lukesneeringer

packages/gapic-generator/.flake8

@@ -5,4 +5,4 @@ ignore =
   E123, E124
   # Line over-indented for visual indent.
   # This works poorly with type annotations in method declarations.
-  E128, E131
+  E126, E128, E131

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

@@ -121,14 +121,13 @@ def python_modules(self) -> Sequence[Tuple[str, str]]:
             of statement.
         """
         answer = set()
-        self_reference = self.meta.address.context(self).python_import
+        self_reference = self.meta.address.python_import
         for t in chain(*[m.field_types for m in self.messages.values()]):
             # Add the appropriate Python import for the field.
             # Sanity check: We do make sure that we are not trying to have
             # a module import itself.
-            imp = t.ident.context(self).python_import
-            if imp != self_reference:
-                answer.add(imp)
+            if t.ident.python_import != self_reference:
+                answer.add(t.ident.python_import)
 
         # Done; return the sorted sequence.
         return tuple(sorted(list(answer)))
@@ -144,10 +143,14 @@ def top(self) -> 'Proto':
         return type(self)(
             file_pb2=self.file_pb2,
             services=self.services,
-            messages={k: v for k, v in self.messages.items()
-                      if not v.meta.address.parent},
-            enums={k: v for k, v in self.enums.items()
-                   if not v.meta.address.parent},
+            messages=collections.OrderedDict([
+                (k, v) for k, v in self.messages.items()
+                if not v.meta.address.parent
+            ]),
+            enums=collections.OrderedDict([
+                (k, v) for k, v in self.enums.items()
+                if not v.meta.address.parent
+            ]),
             file_to_generate=False,
             meta=self.meta,
         )
@@ -326,7 +329,10 @@ def __init__(self, file_descriptor: descriptor_pb2.FileDescriptorProto,
     @property
     def proto(self) -> Proto:
         """Return a Proto dataclass object."""
-        return Proto(
+        # Create a "context-naïve" proto.
+        # This has everything but is ignorant of naming collisions in the
+        # ultimate file that will be written.
+        naive = Proto(
             enums=self.enums,
             file_pb2=self.file_descriptor,
             file_to_generate=self.file_to_generate,
@@ -337,6 +343,30 @@ def proto(self) -> Proto:
             ),
         )
 
+        # If this is not a file being generated, we do not need to
+        # do anything else.
+        if not self.file_to_generate:
+            return naive
+
+        # Return a context-aware proto object.
+        # Note: The services bind to themselves, because services get their
+        # own output files.
+        return dataclasses.replace(naive,
+            enums=collections.OrderedDict([
+                (k, v.with_context(collisions=naive.names))
+                for k, v in naive.enums.items()
+            ]),
+            messages=collections.OrderedDict([
+                (k, v.with_context(collisions=naive.names))
+                for k, v in naive.messages.items()
+            ]),
+            services=collections.OrderedDict([
+                (k, v.with_context(collisions=v.names))
+                for k, v in naive.services.items()
+            ]),
+            meta=naive.meta.with_context(collisions=naive.names),
+        )
+
     @cached_property
     def all_enums(self) -> Mapping[str, wrappers.EnumType]:
         return collections.ChainMap({}, self.enums,

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

@@ -153,15 +153,6 @@ def child(self, child_name: str, path: Tuple[int]) -> 'Address':
             parent=self.parent + (self.name,) if self.name else self.parent,
         )
 
-    def context(self, context) -> 'Address':
-        """Return a derivative of this address with the provided context.
-
-        This method is used to address naming collisions. The returned
-        ``Address`` object aliases module names to avoid naming collisions in
-        the file being written.
-        """
-        return dataclasses.replace(self, collisions=frozenset(context.names))
-
     def rel(self, address: 'Address') -> str:
         """Return an identifier for this type, relative to the given address.
 
@@ -233,6 +224,15 @@ def resolve(self, selector: str) -> str:
             return f'{".".join(self.package)}.{selector}'
         return selector
 
+    def with_context(self, *, collisions: Set[str]) -> 'Address':
+        """Return a derivative of this address with the provided context.
+
+        This method is used to address naming collisions. The returned
+        ``Address`` object aliases module names to avoid naming collisions in
+        the file being written.
+        """
+        return dataclasses.replace(self, collisions=frozenset(collisions))
+
 
 @dataclasses.dataclass(frozen=True)
 class Metadata:
@@ -259,6 +259,17 @@ def doc(self):
             return '\n\n'.join(self.documentation.leading_detached_comments)
         return ''
 
+    def with_context(self, *, collisions: Set[str]) -> 'Metadata':
+        """Return a derivative of this metadata with the provided context.
+
+        This method is used to address naming collisions. The returned
+        ``Address`` object aliases module names to avoid naming collisions in
+        the file being written.
+        """
+        return dataclasses.replace(self,
+            address=self.address.with_context(collisions=collisions),
+        )
+
 
 @dataclasses.dataclass(frozen=True)
 class FieldIdentifier:
@@ -275,7 +286,3 @@ def sphinx(self) -> str:
         if self.repeated:
             return f'Sequence[{self.ident.sphinx}]'
         return self.ident.sphinx
-
-    def context(self, arg) -> 'FieldIdentifier':
-        """Return self. Provided for compatibility with Address."""
-        return self

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

@@ -128,6 +128,23 @@ def type(self) -> Union['MessageType', 'EnumType', 'PythonType']:
         raise TypeError('Unrecognized protobuf type. This code should '
                         'not be reachable; please file a bug.')
 
+    def with_context(self, *, collisions: Set[str]) -> 'Field':
+        """Return a derivative of this field with the provided context.
+
+        This method is used to address naming collisions. The returned
+        ``Field`` object aliases module names to avoid naming collisions
+        in the file being written.
+        """
+        return dataclasses.replace(self,
+            message=self.message.with_context(
+                collisions=collisions,
+                skip_fields=True,
+            ) if self.message else None,
+            enum=self.enum.with_context(collisions=collisions)
+                if self.enum else None,
+            meta=self.meta.with_context(collisions=collisions),
+        )
+
 
 @dataclasses.dataclass(frozen=True)
 class MessageType:
@@ -152,7 +169,13 @@ def field_types(self) -> Sequence[Union['MessageType', 'EnumType']]:
                 answer.append(field.type)
         return tuple(answer)
 
-    def get_field(self, *field_path: Sequence[str]) -> Field:
+    @property
+    def ident(self) -> metadata.Address:
+        """Return the identifier data to be used in templates."""
+        return self.meta.address
+
+    def get_field(self, *field_path: Sequence[str],
+            collisions: Set[str] = frozenset()) -> Field:
         """Return a field arbitrarily deep in this message's structure.
 
         This method recursively traverses the message tree to return the
@@ -171,12 +194,21 @@ def get_field(self, *field_path: Sequence[str]) -> Field:
             KeyError: If a repeated field is used in the non-terminal position
                 in the path.
         """
+        # If collisions are not explicitly specified, retrieve them
+        # from this message's address.
+        # This ensures that calls to `get_field` will return a field with
+        # the same context, regardless of the number of levels through the
+        # chain (in order to avoid infinite recursion on circular references,
+        # we only shallowly bind message references held by fields; this
+        # binds deeply in the one spot where that might be a problem).
+        collisions = collisions or self.meta.address.collisions
+
         # Get the first field in the path.
         cursor = self.fields[field_path[0]]
 
         # Base case: If this is the last field in the path, return it outright.
         if len(field_path) == 1:
-            return cursor
+            return cursor.with_context(collisions=collisions)
 
         # Sanity check: If cursor is a repeated field, then raise an exception.
         # Repeated fields are only permitted in the terminal position.
@@ -191,12 +223,37 @@ def get_field(self, *field_path: Sequence[str]) -> Field:
 
         # Recursion case: Pass the remainder of the path to the sub-field's
         # message.
-        return cursor.message.get_field(*field_path[1:])
+        return cursor.message.get_field(*field_path[1:], collisions=collisions)
 
-    @property
-    def ident(self) -> metadata.Address:
-        """Return the identifier data to be used in templates."""
-        return self.meta.address
+    def with_context(self, *,
+            collisions: Set[str],
+            skip_fields: bool = False,
+            ) -> 'MessageType':
+        """Return a derivative of this message with the provided context.
+
+        This method is used to address naming collisions. The returned
+        ``MessageType`` object aliases module names to avoid naming collisions
+        in the file being written.
+
+        The ``skip_fields`` argument will omit applying the context to the
+        underlying fields. This provides for an "exit" in the case of circular
+        references.
+        """
+        return dataclasses.replace(self,
+            fields=collections.OrderedDict([
+                (k, v.with_context(collisions=collisions))
+                for k, v in self.fields.items()
+            ]) if not skip_fields else self.fields,
+            nested_enums=collections.OrderedDict([
+                (k, v.with_context(collisions=collisions))
+                for k, v in self.nested_enums.items()
+            ]),
+            nested_messages=collections.OrderedDict([(k, v.with_context(
+                collisions=collisions,
+                skip_fields=skip_fields,
+            )) for k, v in self.nested_messages.items()]),
+            meta=self.meta.with_context(collisions=collisions),
+        )
 
 
 @dataclasses.dataclass(frozen=True)
@@ -228,6 +285,17 @@ def ident(self) -> metadata.Address:
         """Return the identifier data to be used in templates."""
         return self.meta.address
 
+    def with_context(self, *, collisions: Set[str]) -> 'EnumType':
+        """Return a derivative of this enum with the provided context.
+
+        This method is used to address naming collisions. The returned
+        ``EnumType`` object aliases module names to avoid naming collisions in
+        the file being written.
+        """
+        return dataclasses.replace(self,
+            meta=self.meta.with_context(collisions=collisions),
+        )
+
 
 @dataclasses.dataclass(frozen=True)
 class PythonType:
@@ -275,6 +343,7 @@ def meta(self) -> metadata.Metadata:
                 name='Operation',
                 module='operation',
                 package=('google', 'api_core'),
+                collisions=self.lro_response.meta.address.collisions,
             ),
             documentation=descriptor_pb2.SourceCodeInfo.Location(
                 leading_comments='An object representing a long-running '
@@ -298,6 +367,18 @@ def name(self) -> str:
         # on google.api_core just to get these strings.
         return 'Operation'
 
+    def with_context(self, *, collisions: Set[str]) -> 'OperationType':
+        """Return a derivative of this operation with the provided context.
+
+        This method is used to address naming collisions. The returned
+        ``OperationType`` object aliases module names to avoid naming
+        collisions in the file being written.
+        """
+        return dataclasses.replace(self,
+            lro_response=self.lro_response.with_context(collisions=collisions),
+            lro_metadata=self.lro_metadata.with_context(collisions=collisions),
+        )
+
 
 @dataclasses.dataclass(frozen=True)
 class Method:
@@ -381,6 +462,19 @@ def signatures(self) -> Tuple[signature_pb2.MethodSignature]:
         # Done; return a tuple of signatures.
         return MethodSignatures(all=tuple(answer))
 
+    def with_context(self, *, collisions: Set[str]) -> 'Method':
+        """Return a derivative of this method with the provided context.
+
+        This method is used to address naming collisions. The returned
+        ``Method`` object aliases module names to avoid naming collisions
+        in the file being written.
+        """
+        return dataclasses.replace(self,
+            input=self.input.with_context(collisions=collisions),
+            output=self.output.with_context(collisions=collisions),
+            meta=self.meta.with_context(collisions=collisions),
+        )
+
 
 @dataclasses.dataclass(frozen=True)
 class MethodSignature:
@@ -519,11 +613,26 @@ def python_modules(self) -> Sequence[imp.Import]:
         answer = set()
         for method in self.methods.values():
             for t in method.ref_types:
-                answer.add(t.ident.context(self).python_import)
+                answer.add(t.ident.python_import)
         return tuple(sorted(list(answer)))
 
     @property
     def has_lro(self) -> bool:
         """Return whether the service has a long-running method."""
         return any([getattr(m.output, 'lro_response', None)
                     for m in self.methods.values()])
+
+    def with_context(self, *, collisions: Set[str]) -> 'Service':
+        """Return a derivative of this service with the provided context.
+
+        This method is used to address naming collisions. The returned
+        ``Service`` object aliases module names to avoid naming collisions
+        in the file being written.
+        """
+        return dataclasses.replace(self,
+            methods=collections.OrderedDict([
+                (k, v.with_context(collisions=collisions))
+                for k, v in self.methods.items()
+            ]),
+            meta=self.meta.with_context(collisions=collisions),
+        )