Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: module names can no longer collide with keywords or builtins #595

Merged
merged 4 commits into from
Sep 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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