Naming collision resolution. (#51)

This adds a system for resolving naming conflicts between imported modules and field or method names.

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
+  E128, E131

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

@@ -20,7 +20,8 @@
 import collections
 import dataclasses
 import sys
-from typing import Callable, List, Mapping, Sequence, Tuple
+from itertools import chain
+from typing import Callable, List, Mapping, Sequence, Set, Tuple
 
 from google.api import annotations_pb2
 from google.protobuf import descriptor_pb2
@@ -81,6 +82,32 @@ def module_name(self) -> str:
         """
         return to_snake_case(self.name.split('/')[-1][:-len('.proto')])
 
+    @cached_property
+    def names(self) -> Set[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.enums.values()}
+        for message in self.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 = {}
+        for t in chain(*[m.field_types for m in self.messages.values()]):
+            modules.setdefault(t.ident.module, set())
+            modules[t.ident.module].add(t.ident.package)
+        for module_name, packages in modules.items():
+            if len(packages) > 1:
+                answer.add(module_name)
+
+        # Return the set of collision names.
+        return frozenset(answer)
+
     @cached_property
     def python_modules(self) -> Sequence[Tuple[str, str]]:
         """Return a sequence of Python modules, for import.
@@ -94,19 +121,14 @@ def python_modules(self) -> Sequence[Tuple[str, str]]:
             of statement.
         """
         answer = set()
-        for message in self.messages.values():
-            for field in message.fields.values():
-                # We only need to add imports for fields that
-                # are messages or enums.
-                if not field.message and not field.enum:
-                    continue
-
-                # Add the appropriate Python import for the field.
-                answer.add(field.type.ident.python_import)
-
-        # We may have gotten an import for this proto.
-        # Obviously no Python module may import itself; get rid of that.
-        answer = answer.difference({self.meta.address.python_import})
+        self_reference = self.meta.address.context(self).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)
 
         # Done; return the sorted sequence.
         return tuple(sorted(list(answer)))
@@ -130,6 +152,17 @@ def top(self) -> 'Proto':
             meta=self.meta,
         )
 
+    def disambiguate(self, string: str) -> str:
+        """Return a disambiguated string for the context of this proto.
+
+        This is used for avoiding naming collisions. Generally, this method
+        returns the same string, but it returns a modified version if
+        it will cause a naming collision with messages or fields in this proto.
+        """
+        if string in self.names:
+            return self.disambiguate(f'_{string}')
+        return string
+
 
 @dataclasses.dataclass(frozen=True)
 class API:

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

@@ -0,0 +1,34 @@
+# Copyright 2018 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import dataclasses
+from typing import Tuple
+
+
+@dataclasses.dataclass(frozen=True, order=True)
+class Import:
+    package: Tuple[str]
+    module: str
+    alias: str = ''
+
+    def __eq__(self, other) -> bool:
+        return self.package == other.package and self.module == other.module
+
+    def __str__(self) -> str:
+        answer = f'import {self.module}'
+        if self.package:
+            answer = f"from {'.'.join(self.package)} {answer}"
+        if self.alias:
+            answer += f' as {self.alias}'
+        return answer

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

@@ -27,11 +27,13 @@
 """
 
 import dataclasses
-from typing import Sequence, Tuple
+from typing import Tuple, Set
 
 from google.protobuf import descriptor_pb2
 
+from gapic.schema import imp
 from gapic.schema import naming
+from gapic.utils import cached_property
 
 
 @dataclasses.dataclass(frozen=True)
@@ -44,6 +46,11 @@ class Address:
     api_naming: naming.Naming = dataclasses.field(
         default_factory=naming.Naming,
     )
+    collisions: Set[str] = dataclasses.field(default_factory=frozenset)
+
+    def __eq__(self, other) -> bool:
+        return all([getattr(self, i) == getattr(other, i) for i
+                    in ('name', 'module', 'module_path', 'package', 'parent')])
 
     def __str__(self) -> str:
         """Return the Python identifier for this type.
@@ -52,9 +59,36 @@ def __str__(self) -> str:
         members from modules, this is consistently `module.Name`.
         """
         if self.module:
-            return '.'.join((self.module,) + self.parent + (self.name,))
+            # If collisions are registered and conflict with our module,
+            # use the module alias instead.
+            module_name = self.module
+            if self.module_alias:
+                module_name = self.module_alias
+
+            # Return the dot-separated Python identifier.
+            return '.'.join((module_name,) + self.parent + (self.name,))
+
+        # Return the Python identifier for this module-less identifier.
         return '.'.join(self.parent + (self.name,))
 
+    @property
+    def module_alias(self) -> str:
+        """Return an appropriate module alias if necessary.
+
+        If the module name is not a collision, return empty string.
+
+        This method provides a mechanism for resolving naming conflicts,
+        while still providing names that are fundamentally readable
+        to users (albeit looking auto-generated).
+        """
+        if self.module in self.collisions:
+            return '_'.join((
+                ''.join([i[0] for i in self.package
+                         if i != self.api_naming.version]),
+                self.module,
+            ))
+        return ''
+
     @property
     def proto(self) -> str:
         """Return the proto selector for this type."""
@@ -65,27 +99,36 @@ def proto_package(self) -> str:
         """Return the proto package for this type."""
         return '.'.join(self.package)
 
-    @property
-    def python_import(self) -> Tuple[Sequence[str], str]:
+    @cached_property
+    def python_import(self) -> imp.Import:
         """Return the Python import for this type."""
         # If there is no naming object, this is a special case for operation.
         # FIXME(#34): OperationType does not work well. Fix or expunge it.
         if not self.api_naming:
-            return ('.'.join(self.package), self.module)
+            return imp.Import(
+                package=self.package,
+                module=self.module,
+                alias=self.module_alias,
+            )
 
         # If this is part of the proto package that we are generating,
         # rewrite the package to our structure.
         if self.proto_package.startswith(self.api_naming.proto_package):
-            return (
-                '.'.join(self.api_naming.module_namespace + (
+            return imp.Import(
+                package=self.api_naming.module_namespace + (
                     self.api_naming.versioned_module_name,
                     'types',
-                )),
-                self.module,
+                ),
+                module=self.module,
+                alias=self.module_alias,
             )
 
         # Return the standard import.
-        return ('.'.join(self.package), f'{self.module}_pb2')
+        return imp.Import(
+            package=self.package,
+            module=f'{self.module}_pb2',
+            alias=self.module_alias if self.module_alias else self.module,
+        )
 
     @property
     def sphinx(self) -> str:
@@ -104,15 +147,21 @@ def child(self, child_name: str, path: Tuple[int]) -> 'Address':
         Returns:
             ~.Address: The new address object.
         """
-        return type(self)(
-            api_naming=self.api_naming,
-            module=self.module,
+        return dataclasses.replace(self,
             module_path=self.module_path + path,
             name=child_name,
-            package=self.package,
             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.
 
@@ -161,7 +210,7 @@ def rel(self, address: 'Address') -> str:
             return '.'.join(self.parent + (self.name,))
 
         # Return the usual `module.Name`.
-        return f'_.{str(self)}'
+        return str(self)
 
     def resolve(self, selector: str) -> str:
         """Resolve a potentially-relative protobuf selector.
@@ -226,3 +275,7 @@ 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