Skip to content

Commit

Permalink
fix: module names can no longer collide with keywords or builtins (#595)
Browse files Browse the repository at this point in the history
E.g. protobuf/any.proto will be imported as

from google.protobuf import any_pb2 as gp_any # Was previously 'as any'
  • Loading branch information
software-dov authored Sep 15, 2020
1 parent e3faa38 commit 960d550
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 4 deletions.
3 changes: 2 additions & 1 deletion gapic/schema/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import collections
import dataclasses
import keyword
import os
import sys
from typing import Callable, Container, Dict, FrozenSet, Mapping, Optional, Sequence, Set, Tuple
Expand Down Expand Up @@ -229,7 +230,7 @@ def disambiguate_keyword_fname(
visited_names: Container[str]) -> str:
path, fname = os.path.split(full_path)
name, ext = os.path.splitext(fname)
if name in RESERVED_NAMES or full_path in visited_names:
if name in keyword.kwlist or full_path in visited_names:
name += "_"
full_path = os.path.join(path, name + ext)
if full_path in visited_names:
Expand Down
4 changes: 4 additions & 0 deletions gapic/schema/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
from gapic.schema import imp
from gapic.schema import naming
from gapic.utils import cached_property
from gapic.utils import RESERVED_NAMES


@dataclasses.dataclass(frozen=True)
Expand All @@ -48,6 +49,9 @@ class Address:
)
collisions: FrozenSet[str] = dataclasses.field(default_factory=frozenset)

def __post_init__(self):
super().__setattr__("collisions", self.collisions | RESERVED_NAMES)

def __eq__(self, other) -> bool:
return all([getattr(self, i) == getattr(other, i) for i
in ('name', 'module', 'module_path', 'package', 'parent')])
Expand Down
13 changes: 12 additions & 1 deletion gapic/utils/reserved_names.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,18 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import builtins
import itertools
import keyword


RESERVED_NAMES = frozenset(keyword.kwlist)
# The filter and map builtins are a historical artifact;
# they are not used in modern, idiomatic python,
# nor are they used in the gapic surface.
# They are too useful to reserve.
RESERVED_NAMES = frozenset(
itertools.chain(
keyword.kwlist,
set(dir(builtins)) - {"filter", "map"},
)
)
24 changes: 22 additions & 2 deletions tests/unit/schema/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

from gapic.schema import metadata
from gapic.schema import naming
from gapic.utils import RESERVED_NAMES


def test_address_str():
Expand Down Expand Up @@ -160,9 +161,28 @@ def test_address_subpackage_empty():

def test_metadata_with_context():
meta = metadata.Metadata()
assert meta.with_context(
collisions = meta.with_context(
collisions={'foo', 'bar'},
).address.collisions == {'foo', 'bar'}
).address.collisions - RESERVED_NAMES
assert collisions == {'foo', 'bar'}


def test_address_name_builtin_keyword():
addr_builtin = metadata.Address(
name="Any",
module="any",
package=("google", "protobuf"),
api_naming=naming.NewNaming(proto_package="foo.bar.baz.v1"),
)
assert addr_builtin.module_alias == "gp_any"

addr_kword = metadata.Address(
name="Class",
module="class",
package=("google", "protobuf"),
api_naming=naming.NewNaming(proto_package="foo.bar.baz.v1"),
)
assert addr_kword.module_alias == "gp_class"


def test_doc_nothing():
Expand Down

0 comments on commit 960d550

Please sign in to comment.