From 9fd1c058e67f20ca0942b9a764f6ddad1da80bb7 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Thu, 4 Jun 2020 22:47:30 +0200 Subject: [PATCH 01/33] Create unit tests for importing --- betterproto/tests/test_get_ref_type.py | 94 ++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/betterproto/tests/test_get_ref_type.py b/betterproto/tests/test_get_ref_type.py index 9635356..25b48bc 100644 --- a/betterproto/tests/test_get_ref_type.py +++ b/betterproto/tests/test_get_ref_type.py @@ -80,3 +80,97 @@ def test_importing_google_wrappers_without_unwrapping( name = get_ref_type(package="", imports=set(), type_name=google_type, unwrap=False) assert name == expected_name + + +def test_import_child_package_from_package(): + imports = set() + name = get_ref_type( + package="package", imports=imports, type_name="package.child.Message" + ) + + assert imports == {"from . import child"} + assert name == "child.Message" + + +def test_import_child_package_from_root(): + imports = set() + name = get_ref_type(package="", imports=imports, type_name="child.Message") + + assert imports == {"from . import child"} + assert name == "child.Message" + + +def test_import_camel_cased(): + imports = set() + name = get_ref_type( + package="", imports=imports, type_name="child_package.example_message" + ) + + assert imports == {"from . import child_package"} + assert name == "child_package.ExampleMessage" + + +def test_import_nested_child_from_root(): + imports = set() + name = get_ref_type(package="", imports=imports, type_name="nested.child.Message") + + assert imports == {"from .nested import child as nested_child"} + assert name == "nested_child.Message" + + +def test_import_deeply_nested_child_from_root(): + imports = set() + name = get_ref_type( + package="", imports=imports, type_name="deeply.nested.child.Message" + ) + + assert imports == {"from .deeply.nested import child as deeply_nested_child"} + assert name == "deeply_nested_child.Message" + + +def test_import_parent_package_from_child(): + imports = set() + name = get_ref_type( + package="package.child", imports=imports, type_name="package.Message" + ) + + assert imports == {"from .. import Message"} + assert name == "Message" + + +def test_import_parent_package_from_deeply_nested_child(): + imports = set() + name = get_ref_type( + package="package.deeply.nested.child", + imports=imports, + type_name="package.deeply.nested.Message", + ) + + assert imports == {"from .. import Message"} + assert name == "Message" + + +def test_import_root_package_from_child(): + imports = set() + name = get_ref_type(package="package.child", imports=imports, type_name="Message") + + assert imports == {"from ... import Message"} + assert name == "Message" + + +def test_import_root_package_from_deeply_nested_child(): + imports = set() + name = get_ref_type( + package="package.deeply.nested.child", imports=imports, type_name="Message" + ) + + assert imports == {"from ..... import Message"} + assert name == "Message" + + +def test_import_root_sibling(): + imports = set() + name = get_ref_type(package="", imports=imports, type_name="Message") + + assert imports == set() + assert name == "Message" From e5e61c873cc5fe64c1a5ce34510b9c02c0144e13 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Thu, 4 Jun 2020 23:57:12 +0200 Subject: [PATCH 02/33] Implement some import scenarios --- betterproto/compile/importing.py | 71 ++++++++++++++++++++------ betterproto/tests/test_get_ref_type.py | 31 ++++++++++- 2 files changed, 85 insertions(+), 17 deletions(-) diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index 0c53e0b..a8039e9 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -1,4 +1,4 @@ -from typing import Dict, Type +from typing import Dict, List, Type import stringcase @@ -43,13 +43,6 @@ def get_ref_type( if type_name == "google.protobuf.Timestamp": return "datetime" - if type_name.startswith(package): - parts = type_name.lstrip(package).lstrip(".").split(".") - if len(parts) == 1 or (len(parts) > 1 and parts[0][0] == parts[0][0].upper()): - # This is the current package, which has nested types flattened. - # foo.bar_thing => FooBarThing - cased = [stringcase.pascalcase(part) for part in parts] - type_name = f'"{"".join(cased)}"' # Use precompiled classes for google.protobuf.* objects if type_name.startswith("google.protobuf.") and type_name.count(".") == 2: @@ -59,12 +52,58 @@ def get_ref_type( imports.add(f"import {import_package} as {import_alias}") return f"{import_alias}.{type_name}" - if "." in type_name: - # This is imported from another package. No need - # to use a forward ref and we need to add the import. - parts = type_name.split(".") - parts[-1] = stringcase.pascalcase(parts[-1]) - imports.add(f"from .{'.'.join(parts[:-2])} import {parts[-2]}") - type_name = f"{parts[-2]}.{parts[-1]}" + importing_package: List[str] = type_name.split('.') + importing_type: str = stringcase.pascalcase(importing_package.pop()) + current_package: List[str] = package.split('.') if package else [] - return type_name + # importing sibling + ''' + package = + name = Foo + + package = foo + name = foo.Bar + + package = foo.bar + name = foo.bar.Baz + ''' + if importing_package == current_package: + imports.add(f"from . import {importing_type}") + return importing_type + + # importing child & descendent: + ''' + package = + name = foo.Bar + + package = + name = foo.bar.Baz + ''' + if importing_package[0:len(current_package)] == current_package: + relative_importing_package = '.'.join(importing_package[len(current_package):]) + imports.add(f"from . import {relative_importing_package}") + return f"{relative_importing_package}.{importing_type}" + + # importing parent & ancestor + ''' + package = foo.bar + name = foo.Foo + + package = foo + name = Bar + + package = foo.bar.baz + name = Bar + ''' + + # importing unrelated or cousin + ''' + package = foo.bar + name = baz.Foo + + package = foo.bar.baz + name = foo.example.Bar + ''' + + return None + # return type_name diff --git a/betterproto/tests/test_get_ref_type.py b/betterproto/tests/test_get_ref_type.py index 25b48bc..d0f91b6 100644 --- a/betterproto/tests/test_get_ref_type.py +++ b/betterproto/tests/test_get_ref_type.py @@ -3,6 +3,7 @@ import pytest from ..compile.importing import get_ref_type +@pytest.mark.skip @pytest.mark.parametrize( ["google_type", "expected_name", "expected_import"], [ @@ -38,6 +39,7 @@ def test_import_google_wellknown_types_non_wrappers( assert imports.__contains__(expected_import) +@pytest.mark.skip @pytest.mark.parametrize( ["google_type", "expected_name"], [ @@ -60,6 +62,7 @@ def test_importing_google_wrappers_unwraps_them(google_type: str, expected_name: assert imports == set() +@pytest.mark.skip @pytest.mark.parametrize( ["google_type", "expected_name"], [ @@ -128,6 +131,16 @@ def test_import_deeply_nested_child_from_root(): assert name == "deeply_nested_child.Message" +def test_import_deeply_nested_child_from_package(): + imports = set() + name = get_ref_type( + package="package", imports=imports, type_name="package.deeply.nested.child.Message" + ) + + assert imports == {"from .deeply.nested import child as deeply_nested_child"} + assert name == "deeply_nested_child.Message" + + def test_import_parent_package_from_child(): imports = set() name = get_ref_type( @@ -172,5 +185,21 @@ def test_import_root_sibling(): imports = set() name = get_ref_type(package="", imports=imports, type_name="Message") - assert imports == set() + assert imports == {"from . import Message"} + assert name == "Message" + + +def test_import_nested_siblings(): + imports = set() + name = get_ref_type(package="foo", imports=imports, type_name="foo.Message") + + assert imports == {"from . import Message"} + assert name == "Message" + + +def test_import_deeply_nested_siblings(): + imports = set() + name = get_ref_type(package="foo.bar", imports=imports, type_name="foo.bar.Message") + + assert imports == {"from . import Message"} assert name == "Message" From 57523a9e7f91eb0ed6241098cd9dd16aa1bf8209 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Fri, 5 Jun 2020 00:33:02 +0200 Subject: [PATCH 03/33] Implement importing unrelated package --- betterproto/compile/importing.py | 28 ++++++++++++--- betterproto/tests/test_get_ref_type.py | 48 +++++++++++++++++++------- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index a8039e9..824e57e 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -1,3 +1,4 @@ +from functools import reduce from typing import Dict, List, Type import stringcase @@ -80,9 +81,17 @@ def get_ref_type( name = foo.bar.Baz ''' if importing_package[0:len(current_package)] == current_package: - relative_importing_package = '.'.join(importing_package[len(current_package):]) - imports.add(f"from . import {relative_importing_package}") - return f"{relative_importing_package}.{importing_type}" + importing_descendent = importing_package[len(current_package):] + string_from = '.'.join(importing_descendent[0:-1]) + string_import = importing_descendent[-1] + + if string_from: + string_alias = '_'.join(importing_descendent) + imports.add(f"from .{string_from} import {string_import} as {string_alias}") + return f"{string_alias}.{importing_type}" + else: + imports.add(f"from . import {string_import}") + return f"{string_import}.{importing_type}" # importing parent & ancestor ''' @@ -95,6 +104,10 @@ def get_ref_type( package = foo.bar.baz name = Bar ''' + if current_package[0:len(importing_package)] == importing_package: + distance = len(current_package) - len(importing_package) + imports.add(f"from .{'.' * distance} import {importing_type}") + return importing_type # importing unrelated or cousin ''' @@ -104,6 +117,13 @@ def get_ref_type( package = foo.bar.baz name = foo.example.Bar ''' + root_distance = len(current_package) + shared_ancestory_length = reduce(lambda l, pair: l + (pair[0] == pair[1]), zip(current_package, importing_package), 0) - return None + string_from = f"{'.' * (shared_ancestory_length+1)}.{'.'.join(importing_package[0:-1])}" + string_import = importing_package[-1] + string_alias = '_' * root_distance + safe_snake_case('.'.join(importing_package)) + imports.add(f"from {string_from} import {string_import} as {string_alias}") + + return f"{string_alias}.{importing_type}" # return type_name diff --git a/betterproto/tests/test_get_ref_type.py b/betterproto/tests/test_get_ref_type.py index d0f91b6..aa27352 100644 --- a/betterproto/tests/test_get_ref_type.py +++ b/betterproto/tests/test_get_ref_type.py @@ -141,6 +141,30 @@ def test_import_deeply_nested_child_from_package(): assert name == "deeply_nested_child.Message" +def test_import_root_sibling(): + imports = set() + name = get_ref_type(package="", imports=imports, type_name="Message") + + assert imports == {"from . import Message"} + assert name == "Message" + + +def test_import_nested_siblings(): + imports = set() + name = get_ref_type(package="foo", imports=imports, type_name="foo.Message") + + assert imports == {"from . import Message"} + assert name == "Message" + + +def test_import_deeply_nested_siblings(): + imports = set() + name = get_ref_type(package="foo.bar", imports=imports, type_name="foo.bar.Message") + + assert imports == {"from . import Message"} + assert name == "Message" + + def test_import_parent_package_from_child(): imports = set() name = get_ref_type( @@ -181,25 +205,25 @@ def test_import_root_package_from_deeply_nested_child(): assert name == "Message" -def test_import_root_sibling(): +def test_import_unrelated_package(): imports = set() - name = get_ref_type(package="", imports=imports, type_name="Message") + name = get_ref_type(package="a", imports=imports, type_name="b.Message") - assert imports == {"from . import Message"} - assert name == "Message" + assert imports == {"from .. import b as _b"} + assert name == "_b.Message" -def test_import_nested_siblings(): +def test_import_cousin_package(): imports = set() - name = get_ref_type(package="foo", imports=imports, type_name="foo.Message") + name = get_ref_type(package="a.a", imports=imports, type_name="a.b.Message") - assert imports == {"from . import Message"} - assert name == "Message" + assert imports == {"from .. import b as __b"} + assert name == "__b.Message" -def test_import_deeply_nested_siblings(): +def test_import_far_cousin_package(): imports = set() - name = get_ref_type(package="foo.bar", imports=imports, type_name="foo.bar.Message") + name = get_ref_type(package="a.a.a", imports=imports, type_name="a.b.c.Message") - assert imports == {"from . import Message"} - assert name == "Message" + assert imports == {"from ... import c as ___c"} + assert name == "___c.Message" From d7ba27de2b32d1b408b75f0f0424a82d6198dcf1 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Fri, 5 Jun 2020 12:33:51 +0200 Subject: [PATCH 04/33] fix all broken imports --- betterproto/compile/importing.py | 54 ++++++++++++++------------ betterproto/tests/test_get_ref_type.py | 46 +++++++++++++++++----- 2 files changed, 66 insertions(+), 34 deletions(-) diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index 824e57e..8b9ee3f 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -44,7 +44,6 @@ def get_ref_type( if type_name == "google.protobuf.Timestamp": return "datetime" - # Use precompiled classes for google.protobuf.* objects if type_name.startswith("google.protobuf.") and type_name.count(".") == 2: type_name = type_name.rsplit(".", maxsplit=1)[1] @@ -53,12 +52,12 @@ def get_ref_type( imports.add(f"import {import_package} as {import_alias}") return f"{import_alias}.{type_name}" - importing_package: List[str] = type_name.split('.') + importing_package: List[str] = type_name.split(".") importing_type: str = stringcase.pascalcase(importing_package.pop()) - current_package: List[str] = package.split('.') if package else [] + current_package: List[str] = package.split(".") if package else [] # importing sibling - ''' + """ package = name = Foo @@ -67,26 +66,26 @@ def get_ref_type( package = foo.bar name = foo.bar.Baz - ''' + """ if importing_package == current_package: imports.add(f"from . import {importing_type}") return importing_type # importing child & descendent: - ''' + """ package = name = foo.Bar package = name = foo.bar.Baz - ''' - if importing_package[0:len(current_package)] == current_package: - importing_descendent = importing_package[len(current_package):] - string_from = '.'.join(importing_descendent[0:-1]) + """ + if importing_package[0 : len(current_package)] == current_package: + importing_descendent = importing_package[len(current_package) :] + string_from = ".".join(importing_descendent[0:-1]) string_import = importing_descendent[-1] if string_from: - string_alias = '_'.join(importing_descendent) + string_alias = "_".join(importing_descendent) imports.add(f"from .{string_from} import {string_import} as {string_alias}") return f"{string_alias}.{importing_type}" else: @@ -94,7 +93,7 @@ def get_ref_type( return f"{string_import}.{importing_type}" # importing parent & ancestor - ''' + """ package = foo.bar name = foo.Foo @@ -103,27 +102,34 @@ def get_ref_type( package = foo.bar.baz name = Bar - ''' - if current_package[0:len(importing_package)] == importing_package: - distance = len(current_package) - len(importing_package) - imports.add(f"from .{'.' * distance} import {importing_type}") + """ + if current_package[0 : len(importing_package)] == importing_package: + distance_up = len(current_package) - len(importing_package) + imports.add(f"from .{'.' * distance_up} import {importing_type}") return importing_type # importing unrelated or cousin - ''' + """ package = foo.bar name = baz.Foo package = foo.bar.baz name = foo.example.Bar - ''' - root_distance = len(current_package) - shared_ancestory_length = reduce(lambda l, pair: l + (pair[0] == pair[1]), zip(current_package, importing_package), 0) + """ - string_from = f"{'.' * (shared_ancestory_length+1)}.{'.'.join(importing_package[0:-1])}" + shared_ancestory = [ + pair[0] + for pair in zip(current_package, importing_package) + if pair[0] == pair[1] + ] + distance_up = len(current_package) - len(shared_ancestory) + + string_from = f".{'.' * distance_up}" + ".".join( + importing_package[len(shared_ancestory) : -1] + ) string_import = importing_package[-1] - string_alias = '_' * root_distance + safe_snake_case('.'.join(importing_package)) + string_alias = f"{'_' * distance_up}" + safe_snake_case( + ".".join(importing_package[len(shared_ancestory) :]) + ) imports.add(f"from {string_from} import {string_import} as {string_alias}") - return f"{string_alias}.{importing_type}" - # return type_name diff --git a/betterproto/tests/test_get_ref_type.py b/betterproto/tests/test_get_ref_type.py index aa27352..8d72406 100644 --- a/betterproto/tests/test_get_ref_type.py +++ b/betterproto/tests/test_get_ref_type.py @@ -134,7 +134,9 @@ def test_import_deeply_nested_child_from_root(): def test_import_deeply_nested_child_from_package(): imports = set() name = get_ref_type( - package="package", imports=imports, type_name="package.deeply.nested.child.Message" + package="package", + imports=imports, + type_name="package.deeply.nested.child.Message", ) assert imports == {"from .deeply.nested import child as deeply_nested_child"} @@ -207,23 +209,47 @@ def test_import_root_package_from_deeply_nested_child(): def test_import_unrelated_package(): imports = set() - name = get_ref_type(package="a", imports=imports, type_name="b.Message") + name = get_ref_type(package="a", imports=imports, type_name="p.Message") - assert imports == {"from .. import b as _b"} - assert name == "_b.Message" + assert imports == {"from .. import p as _p"} + assert name == "_p.Message" + + +def test_import_unrelated_nested_package(): + imports = set() + name = get_ref_type(package="a.b", imports=imports, type_name="p.q.Message") + + assert imports == {"from ...p import q as __p_q"} + assert name == "__p_q.Message" + + +def test_import_unrelated_deeply_nested_package(): + imports = set() + name = get_ref_type(package="a.b.c.d", imports=imports, type_name="p.q.r.s.Message") + + assert imports == {"from .....p.q.r import s as ____p_q_r_s"} + assert name == "____p_q_r_s.Message" def test_import_cousin_package(): imports = set() - name = get_ref_type(package="a.a", imports=imports, type_name="a.b.Message") + name = get_ref_type(package="a.x", imports=imports, type_name="a.y.Message") - assert imports == {"from .. import b as __b"} - assert name == "__b.Message" + assert imports == {"from .. import y as _y"} + assert name == "_y.Message" def test_import_far_cousin_package(): imports = set() - name = get_ref_type(package="a.a.a", imports=imports, type_name="a.b.c.Message") + name = get_ref_type(package="a.x.y", imports=imports, type_name="a.b.c.Message") - assert imports == {"from ... import c as ___c"} - assert name == "___c.Message" + assert imports == {"from ...b import c as __b_c"} + assert name == "__b_c.Message" + + +def test_import_far_far_cousin_package(): + imports = set() + name = get_ref_type(package="a.x.y.z", imports=imports, type_name="a.b.c.d.Message") + + assert imports == {"from ....b.c import d as ___b_c_d"} + assert name == "___b_c_d.Message" From d8abb850f8662bd14fb729827ce1a803fae0fc44 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Sun, 7 Jun 2020 14:33:12 +0200 Subject: [PATCH 05/33] Update tests to reflect new generated package structure --- betterproto/tests/inputs/bool/test_bool.py | 2 +- betterproto/tests/inputs/casing/test_casing.py | 4 ++-- .../inputs/googletypes_response/test_googletypes_response.py | 4 +--- .../test_googletypes_response_embedded.py | 2 +- .../import_service_input_message/test_import_service.py | 2 +- betterproto/tests/inputs/oneof/test_oneof.py | 2 +- betterproto/tests/inputs/oneof_enum/test_oneof_enum.py | 2 +- 7 files changed, 8 insertions(+), 10 deletions(-) diff --git a/betterproto/tests/inputs/bool/test_bool.py b/betterproto/tests/inputs/bool/test_bool.py index 0d4daa6..3131236 100644 --- a/betterproto/tests/inputs/bool/test_bool.py +++ b/betterproto/tests/inputs/bool/test_bool.py @@ -1,4 +1,4 @@ -from betterproto.tests.output_betterproto.bool.bool import Test +from betterproto.tests.output_betterproto.bool import Test def test_value(): diff --git a/betterproto/tests/inputs/casing/test_casing.py b/betterproto/tests/inputs/casing/test_casing.py index 3255c4e..17f01bd 100644 --- a/betterproto/tests/inputs/casing/test_casing.py +++ b/betterproto/tests/inputs/casing/test_casing.py @@ -1,5 +1,5 @@ -import betterproto.tests.output_betterproto.casing.casing as casing -from betterproto.tests.output_betterproto.casing.casing import Test +import betterproto.tests.output_betterproto.casing as casing +from betterproto.tests.output_betterproto.casing import Test def test_message_attributes(): diff --git a/betterproto/tests/inputs/googletypes_response/test_googletypes_response.py b/betterproto/tests/inputs/googletypes_response/test_googletypes_response.py index 02fa193..a7e4938 100644 --- a/betterproto/tests/inputs/googletypes_response/test_googletypes_response.py +++ b/betterproto/tests/inputs/googletypes_response/test_googletypes_response.py @@ -4,9 +4,7 @@ import betterproto.lib.google.protobuf as protobuf import pytest from betterproto.tests.mocks import MockChannel -from betterproto.tests.output_betterproto.googletypes_response.googletypes_response import ( - TestStub, -) +from betterproto.tests.output_betterproto.googletypes_response import TestStub test_cases = [ (TestStub.get_double, protobuf.DoubleValue, 2.5), diff --git a/betterproto/tests/inputs/googletypes_response_embedded/test_googletypes_response_embedded.py b/betterproto/tests/inputs/googletypes_response_embedded/test_googletypes_response_embedded.py index 00b980a..4ef8c22 100644 --- a/betterproto/tests/inputs/googletypes_response_embedded/test_googletypes_response_embedded.py +++ b/betterproto/tests/inputs/googletypes_response_embedded/test_googletypes_response_embedded.py @@ -1,7 +1,7 @@ import pytest from betterproto.tests.mocks import MockChannel -from betterproto.tests.output_betterproto.googletypes_response_embedded.googletypes_response_embedded import ( +from betterproto.tests.output_betterproto.googletypes_response_embedded import ( Output, TestStub, ) diff --git a/betterproto/tests/inputs/import_service_input_message/test_import_service.py b/betterproto/tests/inputs/import_service_input_message/test_import_service.py index a8395fc..891b77a 100644 --- a/betterproto/tests/inputs/import_service_input_message/test_import_service.py +++ b/betterproto/tests/inputs/import_service_input_message/test_import_service.py @@ -1,7 +1,7 @@ import pytest from betterproto.tests.mocks import MockChannel -from betterproto.tests.output_betterproto.import_service_input_message.import_service_input_message import ( +from betterproto.tests.output_betterproto.import_service_input_message import ( RequestResponse, TestStub, ) diff --git a/betterproto/tests/inputs/oneof/test_oneof.py b/betterproto/tests/inputs/oneof/test_oneof.py index 400e4fd..058563e 100644 --- a/betterproto/tests/inputs/oneof/test_oneof.py +++ b/betterproto/tests/inputs/oneof/test_oneof.py @@ -1,5 +1,5 @@ import betterproto -from betterproto.tests.output_betterproto.oneof.oneof import Test +from betterproto.tests.output_betterproto.oneof import Test from betterproto.tests.util import get_test_case_json_data diff --git a/betterproto/tests/inputs/oneof_enum/test_oneof_enum.py b/betterproto/tests/inputs/oneof_enum/test_oneof_enum.py index 1d6ea98..ae9d40d 100644 --- a/betterproto/tests/inputs/oneof_enum/test_oneof_enum.py +++ b/betterproto/tests/inputs/oneof_enum/test_oneof_enum.py @@ -1,7 +1,7 @@ import pytest import betterproto -from betterproto.tests.output_betterproto.oneof_enum.oneof_enum import ( +from betterproto.tests.output_betterproto.oneof_enum import ( Move, Signal, Test, From f7c2fd1194e8e6419fe36928c3898a7411d3810d Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Sun, 7 Jun 2020 16:57:57 +0200 Subject: [PATCH 06/33] Support nested messages, fix casing. Support test-cases in packages. --- Pipfile | 1 - betterproto/__init__.py | 4 +- betterproto/casing.py | 46 ++++++- betterproto/compile/importing.py | 114 ++++++++++-------- betterproto/compile/naming.py | 13 ++ betterproto/plugin.py | 49 +++----- betterproto/tests/inputs/casing/casing.proto | 1 + .../tests/inputs/casing/test_casing.py | 1 + betterproto/tests/inputs/config.py | 17 ++- .../import_circular_dependency.proto | 6 +- .../child.proto | 11 ++ .../import_root_package_from_child.proto | 11 -- betterproto/tests/inputs/nested/nested.proto | 2 +- .../tests/inputs/nestedtwice/nestedtwice.json | 10 +- .../inputs/nestedtwice/nestedtwice.proto | 20 +-- betterproto/tests/inputs/ref/ref.proto | 2 - betterproto/tests/test_casing.py | 89 ++++++++++++++ betterproto/tests/test_get_ref_type.py | 84 ++++++++----- betterproto/tests/test_inputs.py | 15 ++- 19 files changed, 333 insertions(+), 163 deletions(-) create mode 100644 betterproto/compile/naming.py create mode 100644 betterproto/tests/inputs/import_root_package_from_child/child.proto delete mode 100644 betterproto/tests/inputs/import_root_package_from_child/import_root_package_from_child.proto create mode 100644 betterproto/tests/test_casing.py diff --git a/Pipfile b/Pipfile index 0e9397c..455081e 100644 --- a/Pipfile +++ b/Pipfile @@ -15,7 +15,6 @@ rope = "*" protobuf = "*" jinja2 = "*" grpclib = "*" -stringcase = "*" black = "*" backports-datetime-fromisoformat = "*" dataclasses = "*" diff --git a/betterproto/__init__.py b/betterproto/__init__.py index 5d901be..9ed73f6 100644 --- a/betterproto/__init__.py +++ b/betterproto/__init__.py @@ -30,7 +30,7 @@ from typing import ( import grpclib.const import stringcase -from .casing import safe_snake_case +from .casing import safe_snake_case, snake_case if TYPE_CHECKING: from grpclib._protocols import IProtoMessage @@ -132,7 +132,7 @@ class Casing(enum.Enum): """Casing constants for serialization.""" CAMEL = stringcase.camelcase - SNAKE = stringcase.snakecase + SNAKE = snake_case class _PLACEHOLDER: diff --git a/betterproto/casing.py b/betterproto/casing.py index 67ca9a2..919f02e 100644 --- a/betterproto/casing.py +++ b/betterproto/casing.py @@ -1,9 +1,21 @@ -import stringcase +import re + +# Word delimiters and symbols that will not be preserved when re-casing. +# language=PythonRegExp +SYMBOLS = "[^a-zA-Z0-9]*" + +# Optionally capitalized word. +# language=PythonRegExp +WORD = "[A-Z]*[a-z]*[0-9]*" + +# Uppercase word, not followed by lowercase letters. +# language=PythonRegExp +WORD_UPPER = "[A-Z]+(?![a-z])[0-9]*" def safe_snake_case(value: str) -> str: """Snake case a value taking into account Python keywords.""" - value = stringcase.snakecase(value) + value = snake_case(value) if value in [ "and", "as", @@ -39,3 +51,33 @@ def safe_snake_case(value: str) -> str: # https://www.python.org/dev/peps/pep-0008/#descriptive-naming-styles value += "_" return value + + +def snake_case(value: str): + """ + Join words with an underscore into lowercase and remove symbols. + """ + snake = re.sub( + f"{SYMBOLS}({WORD_UPPER}|{WORD})", lambda groups: "_" + groups[1].lower(), value + ) + return snake.strip("_") + + +def pascal_case(value: str): + """ + Capitalize each word and remove symbols. + """ + return re.sub( + f"{SYMBOLS}({WORD_UPPER}|{WORD})", lambda groups: groups[1].capitalize(), value + ) + + +def camel_case(value: str): + """ + Capitalize all words except first and remove symbols. + """ + return capitalize_first(pascal_case(value)) + + +def capitalize_first(value: str): + return value[0:1].lower() + value[1:] diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index 8b9ee3f..3988718 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -1,59 +1,72 @@ -from functools import reduce +import re from typing import Dict, List, Type -import stringcase - from betterproto import safe_snake_case +from betterproto.compile.naming import pythonize_class_name from betterproto.lib.google import protobuf as google_protobuf WRAPPER_TYPES: Dict[str, Type] = { - "google.protobuf.DoubleValue": google_protobuf.DoubleValue, - "google.protobuf.FloatValue": google_protobuf.FloatValue, - "google.protobuf.Int32Value": google_protobuf.Int32Value, - "google.protobuf.Int64Value": google_protobuf.Int64Value, - "google.protobuf.UInt32Value": google_protobuf.UInt32Value, - "google.protobuf.UInt64Value": google_protobuf.UInt64Value, - "google.protobuf.BoolValue": google_protobuf.BoolValue, - "google.protobuf.StringValue": google_protobuf.StringValue, - "google.protobuf.BytesValue": google_protobuf.BytesValue, + ".google.protobuf.DoubleValue": google_protobuf.DoubleValue, + ".google.protobuf.FloatValue": google_protobuf.FloatValue, + ".google.protobuf.Int32Value": google_protobuf.Int32Value, + ".google.protobuf.Int64Value": google_protobuf.Int64Value, + ".google.protobuf.UInt32Value": google_protobuf.UInt32Value, + ".google.protobuf.UInt64Value": google_protobuf.UInt64Value, + ".google.protobuf.BoolValue": google_protobuf.BoolValue, + ".google.protobuf.StringValue": google_protobuf.StringValue, + ".google.protobuf.BytesValue": google_protobuf.BytesValue, } +def parse_source_type_name(field_type_name): + """ + Split full source type name into package and type name. + E.g. 'root.package.Message' -> ('root.package', 'Message') + 'root.Message.SomeEnum' -> ('root', 'Message.SomeEnum') + """ + package_match = re.match(r"^\.?([^A-Z]+)\.(.+)", field_type_name) + if package_match: + package = package_match.group(1) + name = package_match.group(2) + else: + package = "" + name = field_type_name.lstrip(".") + return package, name + + def get_ref_type( - package: str, imports: set, type_name: str, unwrap: bool = True + package: str, imports: set, source_type: str, unwrap: bool = True, ) -> str: """ Return a Python type name for a proto type reference. Adds the import if necessary. Unwraps well known type if required. """ - # If the package name is a blank string, then this should still work - # because by convention packages are lowercase and message/enum types are - # pascal-cased. May require refactoring in the future. - type_name = type_name.lstrip(".") - - is_wrapper = type_name in WRAPPER_TYPES + is_wrapper = source_type in WRAPPER_TYPES if unwrap: if is_wrapper: - wrapped_type = type(WRAPPER_TYPES[type_name]().value) + wrapped_type = type(WRAPPER_TYPES[source_type]().value) return f"Optional[{wrapped_type.__name__}]" - if type_name == "google.protobuf.Duration": + if source_type == ".google.protobuf.Duration": return "timedelta" - if type_name == "google.protobuf.Timestamp": + if source_type == ".google.protobuf.Timestamp": return "datetime" - # Use precompiled classes for google.protobuf.* objects - if type_name.startswith("google.protobuf.") and type_name.count(".") == 2: - type_name = type_name.rsplit(".", maxsplit=1)[1] - import_package = "betterproto.lib.google.protobuf" - import_alias = safe_snake_case(import_package) - imports.add(f"import {import_package} as {import_alias}") - return f"{import_alias}.{type_name}" + source_package, source_type = parse_source_type_name(source_type) + + # Use precompiled classes for google.protobuf.* objects + if source_package == "google.protobuf": + string_import = f"betterproto.lib.{source_package}" + py_type = source_type + string_alias = safe_snake_case(string_import) + imports.add(f"import {string_import} as {string_alias}") + return f"{string_alias}.{py_type}" + + py_package: List[str] = source_package.split(".") if source_package else [] + py_type: str = pythonize_class_name(source_type) - importing_package: List[str] = type_name.split(".") - importing_type: str = stringcase.pascalcase(importing_package.pop()) current_package: List[str] = package.split(".") if package else [] # importing sibling @@ -67,9 +80,8 @@ def get_ref_type( package = foo.bar name = foo.bar.Baz """ - if importing_package == current_package: - imports.add(f"from . import {importing_type}") - return importing_type + if py_package == current_package: + return f'"{py_type}"' # importing child & descendent: """ @@ -79,18 +91,18 @@ def get_ref_type( package = name = foo.bar.Baz """ - if importing_package[0 : len(current_package)] == current_package: - importing_descendent = importing_package[len(current_package) :] + if py_package[0 : len(current_package)] == current_package: + importing_descendent = py_package[len(current_package) :] string_from = ".".join(importing_descendent[0:-1]) string_import = importing_descendent[-1] if string_from: string_alias = "_".join(importing_descendent) imports.add(f"from .{string_from} import {string_import} as {string_alias}") - return f"{string_alias}.{importing_type}" + return f"{string_alias}.{py_type}" else: imports.add(f"from . import {string_import}") - return f"{string_import}.{importing_type}" + return f"{string_import}.{py_type}" # importing parent & ancestor """ @@ -103,10 +115,10 @@ def get_ref_type( package = foo.bar.baz name = Bar """ - if current_package[0 : len(importing_package)] == importing_package: - distance_up = len(current_package) - len(importing_package) - imports.add(f"from .{'.' * distance_up} import {importing_type}") - return importing_type + if current_package[0 : len(py_package)] == py_package: + distance_up = len(current_package) - len(py_package) + imports.add(f"from .{'.' * distance_up} import {py_type}") + return py_type # importing unrelated or cousin """ @@ -116,20 +128,16 @@ def get_ref_type( package = foo.bar.baz name = foo.example.Bar """ - shared_ancestory = [ - pair[0] - for pair in zip(current_package, importing_package) - if pair[0] == pair[1] + pair[0] for pair in zip(current_package, py_package) if pair[0] == pair[1] ] distance_up = len(current_package) - len(shared_ancestory) - string_from = f".{'.' * distance_up}" + ".".join( - importing_package[len(shared_ancestory) : -1] + py_package[len(shared_ancestory) : -1] ) - string_import = importing_package[-1] - string_alias = f"{'_' * distance_up}" + safe_snake_case( - ".".join(importing_package[len(shared_ancestory) :]) + string_import = py_package[-1] + alias = f"{'_' * distance_up}" + safe_snake_case( + ".".join(py_package[len(shared_ancestory) :]) ) - imports.add(f"from {string_from} import {string_import} as {string_alias}") - return f"{string_alias}.{importing_type}" + imports.add(f"from {string_from} import {string_import} as {alias}") + return f"{alias}.{py_type}" diff --git a/betterproto/compile/naming.py b/betterproto/compile/naming.py new file mode 100644 index 0000000..3d56852 --- /dev/null +++ b/betterproto/compile/naming.py @@ -0,0 +1,13 @@ +from betterproto import casing + + +def pythonize_class_name(name): + return casing.pascal_case(name) + + +def pythonize_field_name(name: str): + return casing.safe_snake_case(name) + + +def pythonize_method_name(name: str): + return casing.safe_snake_case(name) diff --git a/betterproto/plugin.py b/betterproto/plugin.py index e300318..e30571f 100755 --- a/betterproto/plugin.py +++ b/betterproto/plugin.py @@ -10,6 +10,11 @@ from typing import List from betterproto.casing import safe_snake_case from betterproto.compile.importing import get_ref_type import betterproto +from betterproto.compile.naming import ( + pythonize_class_name, + pythonize_field_name, + pythonize_method_name, +) try: # betterproto[compiler] specific dependencies @@ -35,27 +40,22 @@ except ImportError as err: raise SystemExit(1) -def py_type( - package: str, - imports: set, - message: DescriptorProto, - descriptor: FieldDescriptorProto, -) -> str: - if descriptor.type in [1, 2, 6, 7, 15, 16]: +def py_type(package: str, imports: set, field: FieldDescriptorProto) -> str: + if field.type in [1, 2, 6, 7, 15, 16]: return "float" - elif descriptor.type in [3, 4, 5, 13, 17, 18]: + elif field.type in [3, 4, 5, 13, 17, 18]: return "int" - elif descriptor.type == 8: + elif field.type == 8: return "bool" - elif descriptor.type == 9: + elif field.type == 9: return "str" - elif descriptor.type in [11, 14]: + elif field.type in [11, 14]: # Type referencing another defined Message or a named enum - return get_ref_type(package, imports, descriptor.type_name) - elif descriptor.type == 12: + return get_ref_type(package, imports, field.type_name) + elif field.type == 12: return "bytes" else: - raise NotImplementedError(f"Unknown type {descriptor.type}") + raise NotImplementedError(f"Unknown type {field.type}") def get_py_zero(type_num: int) -> str: @@ -160,17 +160,10 @@ def generate_code(request, response): "services": [], } - type_mapping = {} - for proto_file in options["files"]: - # print(proto_file.message_type, file=sys.stderr) - # print(proto_file.service, file=sys.stderr) - # print(proto_file.source_code_info, file=sys.stderr) - + item: DescriptorProto for item, path in traverse(proto_file): - # print(item, file=sys.stderr) - # print(path, file=sys.stderr) - data = {"name": item.name, "py_name": stringcase.pascalcase(item.name)} + data = {"name": item.name, "py_name": pythonize_class_name(item.name)} if isinstance(item, DescriptorProto): # print(item, file=sys.stderr) @@ -187,7 +180,7 @@ def generate_code(request, response): ) for i, f in enumerate(item.field): - t = py_type(package, output["imports"], item, f) + t = py_type(package, output["imports"], f) zero = get_py_zero(f.type) repeated = False @@ -222,13 +215,11 @@ def generate_code(request, response): k = py_type( package, output["imports"], - item, nested.field[0], ) v = py_type( package, output["imports"], - item, nested.field[1], ) t = f"Dict[{k}, {v}]" @@ -264,7 +255,7 @@ def generate_code(request, response): data["properties"].append( { "name": f.name, - "py_name": safe_snake_case(f.name), + "py_name": pythonize_field_name(f.name), "number": f.number, "comment": get_comment(proto_file, path + [2, i]), "proto_type": int(f.type), @@ -305,7 +296,7 @@ def generate_code(request, response): data = { "name": service.name, - "py_name": stringcase.pascalcase(service.name), + "py_name": pythonize_class_name(service.name), "comment": get_comment(proto_file, [6, i]), "methods": [], } @@ -329,7 +320,7 @@ def generate_code(request, response): data["methods"].append( { "name": method.name, - "py_name": stringcase.snakecase(method.name), + "py_name": pythonize_method_name(method.name), "comment": get_comment(proto_file, [6, i, 2, j], indent=8), "route": f"/{package}.{service.name}/{method.name}", "input": get_ref_type( diff --git a/betterproto/tests/inputs/casing/casing.proto b/betterproto/tests/inputs/casing/casing.proto index ad0c427..ca458b5 100644 --- a/betterproto/tests/inputs/casing/casing.proto +++ b/betterproto/tests/inputs/casing/casing.proto @@ -10,6 +10,7 @@ message Test { int32 camelCase = 1; my_enum snake_case = 2; snake_case_message snake_case_message = 3; + int32 UPPERCASE = 4; } message snake_case_message { diff --git a/betterproto/tests/inputs/casing/test_casing.py b/betterproto/tests/inputs/casing/test_casing.py index 17f01bd..1c0dc80 100644 --- a/betterproto/tests/inputs/casing/test_casing.py +++ b/betterproto/tests/inputs/casing/test_casing.py @@ -8,6 +8,7 @@ def test_message_attributes(): message, "snake_case_message" ), "snake_case field name is same in python" assert hasattr(message, "camel_case"), "CamelCase field is snake_case in python" + assert hasattr(message, "uppercase"), "UPPERCASE field is lowercase in python" def test_message_casing(): diff --git a/betterproto/tests/inputs/config.py b/betterproto/tests/inputs/config.py index 2525d8f..48e4b09 100644 --- a/betterproto/tests/inputs/config.py +++ b/betterproto/tests/inputs/config.py @@ -1,12 +1,7 @@ # Test cases that are expected to fail, e.g. unimplemented features or bug-fixes. # Remove from list when fixed. tests = { - "import_root_sibling", # 61 - "import_child_package_from_package", # 58 - "import_root_package_from_child", # 60 - "import_parent_package_from_child", # 59 - "import_circular_dependency", # failing because of other bugs now - "import_packages_same_name", # 25 + "import_circular_dependency", "oneof_enum", # 63 "casing_message_field_uppercase", # 11 "namespace_keywords", # 70 @@ -15,6 +10,16 @@ tests = { "googletypes_value", # 9 } + +# Defines where the main package for this test resides. +# Needed to test relative package imports. +packages = { + "import_root_package_from_child": ".child", + "import_parent_package_from_child": ".parent.child", + "repeatedmessage": ".repeatedmessage", + "service": ".service", +} + services = { "googletypes_response", "googletypes_response_embedded", diff --git a/betterproto/tests/inputs/import_circular_dependency/import_circular_dependency.proto b/betterproto/tests/inputs/import_circular_dependency/import_circular_dependency.proto index 589d14f..7d02aad 100644 --- a/betterproto/tests/inputs/import_circular_dependency/import_circular_dependency.proto +++ b/betterproto/tests/inputs/import_circular_dependency/import_circular_dependency.proto @@ -3,9 +3,9 @@ syntax = "proto3"; import "root.proto"; import "other.proto"; -// This test-case verifies that future implementations will support circular dependencies in the generated python files. +// This test-case verifies support for circular dependencies in the generated python files. // -// This becomes important when generating 1 python file/module per package, rather than 1 file per proto file. +// This is important because we generate 1 python file/module per package, rather than 1 file per proto file. // // Scenario: // @@ -24,5 +24,5 @@ import "other.proto"; // (root: Test & RootPackageMessage) <-------> (other: OtherPackageMessage) message Test { RootPackageMessage message = 1; - other.OtherPackageMessage other =2; + other.OtherPackageMessage other = 2; } diff --git a/betterproto/tests/inputs/import_root_package_from_child/child.proto b/betterproto/tests/inputs/import_root_package_from_child/child.proto new file mode 100644 index 0000000..d2b29cc --- /dev/null +++ b/betterproto/tests/inputs/import_root_package_from_child/child.proto @@ -0,0 +1,11 @@ +syntax = "proto3"; + +package child; + +import "root.proto"; + +// Verify that we can import root message from child package + +message Test { + RootMessage message = 1; +} diff --git a/betterproto/tests/inputs/import_root_package_from_child/import_root_package_from_child.proto b/betterproto/tests/inputs/import_root_package_from_child/import_root_package_from_child.proto deleted file mode 100644 index 9e7dbcd..0000000 --- a/betterproto/tests/inputs/import_root_package_from_child/import_root_package_from_child.proto +++ /dev/null @@ -1,11 +0,0 @@ -syntax = "proto3"; - -import "root.proto"; - -package child; - -// Tests generated imports when a message inside a child-package refers to a message defined in the root. - -message Test { - RootMessage message = 1; -} diff --git a/betterproto/tests/inputs/nested/nested.proto b/betterproto/tests/inputs/nested/nested.proto index 974bf86..2eaef59 100644 --- a/betterproto/tests/inputs/nested/nested.proto +++ b/betterproto/tests/inputs/nested/nested.proto @@ -10,7 +10,7 @@ message Test { Nested nested = 1; Sibling sibling = 2; - Sibling sibling2 = 3; +// Sibling sibling2 = 3; } message Sibling { diff --git a/betterproto/tests/inputs/nestedtwice/nestedtwice.json b/betterproto/tests/inputs/nestedtwice/nestedtwice.json index 203a660..c953132 100644 --- a/betterproto/tests/inputs/nestedtwice/nestedtwice.json +++ b/betterproto/tests/inputs/nestedtwice/nestedtwice.json @@ -1,10 +1,10 @@ { - "root": { + "top": { "name": "double-nested", - "parent": { - "child": [{"foo": "hello"}], - "enumChild": ["A"], - "rootParentChild": [{"a": "hello"}], + "middle": { + "bottom": [{"foo": "hello"}], + "enumBottom": ["A"], + "topMiddleBottom": [{"a": "hello"}], "bar": true } } diff --git a/betterproto/tests/inputs/nestedtwice/nestedtwice.proto b/betterproto/tests/inputs/nestedtwice/nestedtwice.proto index 91c8050..7e9c206 100644 --- a/betterproto/tests/inputs/nestedtwice/nestedtwice.proto +++ b/betterproto/tests/inputs/nestedtwice/nestedtwice.proto @@ -1,26 +1,26 @@ syntax = "proto3"; message Test { - message Root { - message Parent { - message RootParentChild { + message Top { + message Middle { + message TopMiddleBottom { string a = 1; } - enum EnumChild{ + enum EnumBottom{ A = 0; B = 1; } - message Child { + message Bottom { string foo = 1; } reserved 1; - repeated Child child = 2; - repeated EnumChild enumChild=3; - repeated RootParentChild rootParentChild=4; + repeated Bottom bottom = 2; + repeated EnumBottom enumBottom=3; + repeated TopMiddleBottom topMiddleBottom=4; bool bar = 5; } string name = 1; - Parent parent = 2; + Middle middle = 2; } - Root root = 1; + Top top = 1; } diff --git a/betterproto/tests/inputs/ref/ref.proto b/betterproto/tests/inputs/ref/ref.proto index 6945590..e09fb15 100644 --- a/betterproto/tests/inputs/ref/ref.proto +++ b/betterproto/tests/inputs/ref/ref.proto @@ -1,7 +1,5 @@ syntax = "proto3"; -package ref; - import "repeatedmessage.proto"; message Test { diff --git a/betterproto/tests/test_casing.py b/betterproto/tests/test_casing.py new file mode 100644 index 0000000..f777e2c --- /dev/null +++ b/betterproto/tests/test_casing.py @@ -0,0 +1,89 @@ +import pytest + +from betterproto.casing import camel_case, pascal_case, snake_case + + +@pytest.mark.parametrize( + ["value", "expected"], + [ + ("", ""), + ("a", "A"), + ("foobar", "Foobar"), + ("FooBar", "FooBar"), + ("foo.bar", "FooBar"), + ("foo_bar", "FooBar"), + ("FOOBAR", "Foobar"), + ("FOOBar", "FooBar"), + ("UInt32", "UInt32"), + ("FOO_BAR", "FooBar"), + ("FOOBAR1", "Foobar1"), + ("FOOBAR_1", "Foobar1"), + ("FOO1BAR2", "Foo1Bar2"), + ("foo__bar", "FooBar"), + ("_foobar", "Foobar"), + ("foobaR", "FoobaR"), + ("foo~bar", "FooBar"), + ("foo:bar", "FooBar"), + ("1foobar", "1Foobar"), + ], +) +def test_pascal_case(value, expected): + actual = pascal_case(value) + assert actual == expected, f"{value} => {expected} (actual: {actual})" + + +@pytest.mark.parametrize( + ["value", "expected"], + [ + ("", ""), + ("a", "a"), + ("foobar", "foobar"), + ("FooBar", "fooBar"), + ("foo.bar", "fooBar"), + ("foo_bar", "fooBar"), + ("FOOBAR", "foobar"), + ("FOO_BAR", "fooBar"), + ("FOOBAR1", "foobar1"), + ("FOOBAR_1", "foobar1"), + ("FOO1BAR2", "foo1Bar2"), + ("foo__bar", "fooBar"), + ("_foobar", "foobar"), + ("foobaR", "foobaR"), + ("foo~bar", "fooBar"), + ("foo:bar", "fooBar"), + ("1foobar", "1Foobar"), + ], +) +def test_camel_case(value, expected): + actual = camel_case(value) + assert actual == expected, f"{value} => {expected} (actual: {actual})" + + +@pytest.mark.parametrize( + ["value", "expected"], + [ + ("", ""), + ("a", "a"), + ("foobar", "foobar"), + ("FooBar", "foo_bar"), + ("foo.bar", "foo_bar"), + ("foo_bar", "foo_bar"), + ("FOOBAR", "foobar"), + ("FOOBar", "foo_bar"), + ("UInt32", "u_int32"), + ("FOO_BAR", "foo_bar"), + ("FOOBAR1", "foobar1"), + ("FOOBAR_1", "foobar_1"), + ("FOOBAR_123", "foobar_123"), + ("FOO1BAR2", "foo1_bar2"), + ("foo__bar", "foo_bar"), + ("_foobar", "foobar"), + ("foobaR", "fooba_r"), + ("foo~bar", "foo_bar"), + ("foo:bar", "foo_bar"), + ("1foobar", "1_foobar"), + ], +) +def test_snake_case(value, expected): + actual = snake_case(value) + assert actual == expected, f"{value} => {expected} (actual: {actual})" diff --git a/betterproto/tests/test_get_ref_type.py b/betterproto/tests/test_get_ref_type.py index 8d72406..16a5466 100644 --- a/betterproto/tests/test_get_ref_type.py +++ b/betterproto/tests/test_get_ref_type.py @@ -1,9 +1,8 @@ import pytest -from ..compile.importing import get_ref_type +from ..compile.importing import get_ref_type, parse_source_type_name -@pytest.mark.skip @pytest.mark.parametrize( ["google_type", "expected_name", "expected_import"], [ @@ -33,13 +32,14 @@ def test_import_google_wellknown_types_non_wrappers( google_type: str, expected_name: str, expected_import: str ): imports = set() - name = get_ref_type(package="", imports=imports, type_name=google_type) + name = get_ref_type(package="", imports=imports, source_type=google_type) assert name == expected_name - assert imports.__contains__(expected_import) + assert imports.__contains__( + expected_import + ), f"{expected_import} not found in {imports}" -@pytest.mark.skip @pytest.mark.parametrize( ["google_type", "expected_name"], [ @@ -56,13 +56,12 @@ def test_import_google_wellknown_types_non_wrappers( ) def test_importing_google_wrappers_unwraps_them(google_type: str, expected_name: str): imports = set() - name = get_ref_type(package="", imports=imports, type_name=google_type) + name = get_ref_type(package="", imports=imports, source_type=google_type) assert name == expected_name assert imports == set() -@pytest.mark.skip @pytest.mark.parametrize( ["google_type", "expected_name"], [ @@ -80,7 +79,9 @@ def test_importing_google_wrappers_unwraps_them(google_type: str, expected_name: def test_importing_google_wrappers_without_unwrapping( google_type: str, expected_name: str ): - name = get_ref_type(package="", imports=set(), type_name=google_type, unwrap=False) + name = get_ref_type( + package="", imports=set(), source_type=google_type, unwrap=False + ) assert name == expected_name @@ -88,7 +89,7 @@ def test_importing_google_wrappers_without_unwrapping( def test_import_child_package_from_package(): imports = set() name = get_ref_type( - package="package", imports=imports, type_name="package.child.Message" + package="package", imports=imports, source_type="package.child.Message" ) assert imports == {"from . import child"} @@ -97,7 +98,7 @@ def test_import_child_package_from_package(): def test_import_child_package_from_root(): imports = set() - name = get_ref_type(package="", imports=imports, type_name="child.Message") + name = get_ref_type(package="", imports=imports, source_type="child.Message") assert imports == {"from . import child"} assert name == "child.Message" @@ -106,7 +107,7 @@ def test_import_child_package_from_root(): def test_import_camel_cased(): imports = set() name = get_ref_type( - package="", imports=imports, type_name="child_package.example_message" + package="", imports=imports, source_type="child_package.example_message" ) assert imports == {"from . import child_package"} @@ -115,7 +116,7 @@ def test_import_camel_cased(): def test_import_nested_child_from_root(): imports = set() - name = get_ref_type(package="", imports=imports, type_name="nested.child.Message") + name = get_ref_type(package="", imports=imports, source_type="nested.child.Message") assert imports == {"from .nested import child as nested_child"} assert name == "nested_child.Message" @@ -124,7 +125,7 @@ def test_import_nested_child_from_root(): def test_import_deeply_nested_child_from_root(): imports = set() name = get_ref_type( - package="", imports=imports, type_name="deeply.nested.child.Message" + package="", imports=imports, source_type="deeply.nested.child.Message" ) assert imports == {"from .deeply.nested import child as deeply_nested_child"} @@ -136,7 +137,7 @@ def test_import_deeply_nested_child_from_package(): name = get_ref_type( package="package", imports=imports, - type_name="package.deeply.nested.child.Message", + source_type="package.deeply.nested.child.Message", ) assert imports == {"from .deeply.nested import child as deeply_nested_child"} @@ -145,32 +146,32 @@ def test_import_deeply_nested_child_from_package(): def test_import_root_sibling(): imports = set() - name = get_ref_type(package="", imports=imports, type_name="Message") + name = get_ref_type(package="", imports=imports, source_type="Message") - assert imports == {"from . import Message"} - assert name == "Message" + assert imports == set() + assert name == '"Message"' def test_import_nested_siblings(): imports = set() - name = get_ref_type(package="foo", imports=imports, type_name="foo.Message") + name = get_ref_type(package="foo", imports=imports, source_type="foo.Message") - assert imports == {"from . import Message"} - assert name == "Message" + assert name == '"Message"' def test_import_deeply_nested_siblings(): imports = set() - name = get_ref_type(package="foo.bar", imports=imports, type_name="foo.bar.Message") + name = get_ref_type( + package="foo.bar", imports=imports, source_type="foo.bar.Message" + ) - assert imports == {"from . import Message"} - assert name == "Message" + assert name == '"Message"' def test_import_parent_package_from_child(): imports = set() name = get_ref_type( - package="package.child", imports=imports, type_name="package.Message" + package="package.child", imports=imports, source_type="package.Message" ) assert imports == {"from .. import Message"} @@ -182,7 +183,7 @@ def test_import_parent_package_from_deeply_nested_child(): name = get_ref_type( package="package.deeply.nested.child", imports=imports, - type_name="package.deeply.nested.Message", + source_type="package.deeply.nested.Message", ) assert imports == {"from .. import Message"} @@ -191,7 +192,7 @@ def test_import_parent_package_from_deeply_nested_child(): def test_import_root_package_from_child(): imports = set() - name = get_ref_type(package="package.child", imports=imports, type_name="Message") + name = get_ref_type(package="package.child", imports=imports, source_type="Message") assert imports == {"from ... import Message"} assert name == "Message" @@ -200,7 +201,7 @@ def test_import_root_package_from_child(): def test_import_root_package_from_deeply_nested_child(): imports = set() name = get_ref_type( - package="package.deeply.nested.child", imports=imports, type_name="Message" + package="package.deeply.nested.child", imports=imports, source_type="Message" ) assert imports == {"from ..... import Message"} @@ -209,7 +210,7 @@ def test_import_root_package_from_deeply_nested_child(): def test_import_unrelated_package(): imports = set() - name = get_ref_type(package="a", imports=imports, type_name="p.Message") + name = get_ref_type(package="a", imports=imports, source_type="p.Message") assert imports == {"from .. import p as _p"} assert name == "_p.Message" @@ -217,7 +218,7 @@ def test_import_unrelated_package(): def test_import_unrelated_nested_package(): imports = set() - name = get_ref_type(package="a.b", imports=imports, type_name="p.q.Message") + name = get_ref_type(package="a.b", imports=imports, source_type="p.q.Message") assert imports == {"from ...p import q as __p_q"} assert name == "__p_q.Message" @@ -225,7 +226,9 @@ def test_import_unrelated_nested_package(): def test_import_unrelated_deeply_nested_package(): imports = set() - name = get_ref_type(package="a.b.c.d", imports=imports, type_name="p.q.r.s.Message") + name = get_ref_type( + package="a.b.c.d", imports=imports, source_type="p.q.r.s.Message" + ) assert imports == {"from .....p.q.r import s as ____p_q_r_s"} assert name == "____p_q_r_s.Message" @@ -233,7 +236,7 @@ def test_import_unrelated_deeply_nested_package(): def test_import_cousin_package(): imports = set() - name = get_ref_type(package="a.x", imports=imports, type_name="a.y.Message") + name = get_ref_type(package="a.x", imports=imports, source_type="a.y.Message") assert imports == {"from .. import y as _y"} assert name == "_y.Message" @@ -241,7 +244,7 @@ def test_import_cousin_package(): def test_import_far_cousin_package(): imports = set() - name = get_ref_type(package="a.x.y", imports=imports, type_name="a.b.c.Message") + name = get_ref_type(package="a.x.y", imports=imports, source_type="a.b.c.Message") assert imports == {"from ...b import c as __b_c"} assert name == "__b_c.Message" @@ -249,7 +252,22 @@ def test_import_far_cousin_package(): def test_import_far_far_cousin_package(): imports = set() - name = get_ref_type(package="a.x.y.z", imports=imports, type_name="a.b.c.d.Message") + name = get_ref_type( + package="a.x.y.z", imports=imports, source_type="a.b.c.d.Message" + ) assert imports == {"from ....b.c import d as ___b_c_d"} assert name == "___b_c_d.Message" + + +@pytest.mark.parametrize( + ["full_name", "expected_output"], + [ + ("package.SomeMessage.NestedType", ("package", "SomeMessage.NestedType")), + (".package.SomeMessage.NestedType", ("package", "SomeMessage.NestedType")), + (".service.ExampleRequest", ("service", "ExampleRequest")), + (".package.lower_case_message", ("package", "lower_case_message")), + ], +) +def test_parse_field_type_name(full_name, expected_output): + assert parse_source_type_name(full_name) == expected_output diff --git a/betterproto/tests/test_inputs.py b/betterproto/tests/test_inputs.py index cac8327..6778425 100644 --- a/betterproto/tests/test_inputs.py +++ b/betterproto/tests/test_inputs.py @@ -57,7 +57,9 @@ plugin_output_package = "betterproto.tests.output_betterproto" reference_output_package = "betterproto.tests.output_reference" -TestData = namedtuple("TestData", "plugin_module, reference_module, json_data") +TestData = namedtuple( + "TestData", ["plugin_module", "reference_module", "json_data", "entry_point"] +) @pytest.fixture @@ -75,15 +77,18 @@ def test_data(request): sys.path.append(reference_module_root) + test_package = test_case_name + test_input_config.packages.get(test_case_name, "") + yield ( TestData( plugin_module=importlib.import_module( - f"{plugin_output_package}.{test_case_name}.{test_case_name}" + f"{plugin_output_package}.{test_package}" ), reference_module=lambda: importlib.import_module( f"{reference_output_package}.{test_case_name}.{test_case_name}_pb2" ), json_data=get_test_case_json_data(test_case_name), + entry_point=test_package, ) ) @@ -106,7 +111,7 @@ def test_message_equality(test_data: TestData) -> None: @pytest.mark.parametrize("test_data", test_cases.messages_with_json, indirect=True) def test_message_json(repeat, test_data: TestData) -> None: - plugin_module, _, json_data = test_data + plugin_module, _, json_data, entry_point = test_data for _ in range(repeat): message: betterproto.Message = plugin_module.Test() @@ -119,13 +124,13 @@ def test_message_json(repeat, test_data: TestData) -> None: @pytest.mark.parametrize("test_data", test_cases.services, indirect=True) def test_service_can_be_instantiated(test_data: TestData) -> None: - plugin_module, _, json_data = test_data + plugin_module, _, json_data, entry_point = test_data plugin_module.TestStub(MockChannel()) @pytest.mark.parametrize("test_data", test_cases.messages_with_json, indirect=True) def test_binary_compatibility(repeat, test_data: TestData) -> None: - plugin_module, reference_module, json_data = test_data + plugin_module, reference_module, json_data, entry_point = test_data reference_instance = Parse(json_data, reference_module().Test()) reference_binary_output = reference_instance.SerializeToString() From fdf3b2e764c34c4656a6db8bfbcc336f10c57d95 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Sun, 7 Jun 2020 17:00:02 +0200 Subject: [PATCH 07/33] Compile proto files based on package structure --- betterproto/plugin.py | 57 +++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/betterproto/plugin.py b/betterproto/plugin.py index e30571f..f64a563 100755 --- a/betterproto/plugin.py +++ b/betterproto/plugin.py @@ -3,11 +3,9 @@ import itertools import os.path import re -import stringcase import sys import textwrap from typing import List -from betterproto.casing import safe_snake_case from betterproto.compile.importing import get_ref_type import betterproto from betterproto.compile.naming import ( @@ -131,17 +129,21 @@ def generate_code(request, response): output_map = {} for proto_file in request.proto_file: - out = proto_file.package - - if out == "google.protobuf" and "INCLUDE_GOOGLE" not in plugin_options: + if ( + proto_file.package == "google.protobuf" + and "INCLUDE_GOOGLE" not in plugin_options + ): continue - if not out: - out = os.path.splitext(proto_file.name)[0].replace(os.path.sep, ".") + # if proto_file.package: + output_file = ( + os.path.join(proto_file.package.replace(".", os.path.sep), "__init__") + + ".py" + ) - if out not in output_map: - output_map[out] = {"package": proto_file.package, "files": []} - output_map[out]["files"].append(proto_file) + if output_file not in output_map: + output_map[output_file] = {"package": proto_file.package, "files": []} + output_map[output_file]["files"].append(proto_file) # TODO: Figure out how to handle gRPC request/response messages and add # processing below for Service. @@ -349,8 +351,7 @@ def generate_code(request, response): # Fill response f = response.file.add() - # print(filename, file=sys.stderr) - f.name = filename.replace(".", os.path.sep) + ".py" + f.name = filename # Render and then format the output file. f.content = black.format_str( @@ -358,32 +359,24 @@ def generate_code(request, response): mode=black.FileMode(target_versions=set([black.TargetVersion.PY37])), ) - inits = set([""]) - for f in response.file: - # Ensure output paths exist - # print(f.name, file=sys.stderr) - dirnames = os.path.dirname(f.name) - if dirnames: - os.makedirs(dirnames, exist_ok=True) - base = "" - for part in dirnames.split(os.path.sep): - base = os.path.join(base, part) - inits.add(base) + output_files = list(output_map.keys()) + for output_file in output_files: + path = os.path.dirname(output_file) - for base in inits: - name = os.path.join(base, "__init__.py") + for sub_path in path.split(os.path.sep): + init_file = os.path.join(sub_path, "__init__.py") - if os.path.exists(name): - # Never overwrite inits as they may have custom stuff in them. - continue + if init_file in output_files: + continue - init = response.file.add() - init.name = name - init.content = b"" + output_files.append(init_file) + init = response.file.add() + init.name = init_file + init.content = b"" filenames = sorted([f.name for f in response.file]) for fname in filenames: - print(f"Writing {fname}", file=sys.stderr) + print(f"Writing {repr(fname)}", file=sys.stderr) def main(): From c00e2aef19f56aa9b23901e22a6ce8d8ee885f52 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Mon, 8 Jun 2020 08:26:25 +0200 Subject: [PATCH 08/33] Break up importing logic in methods --- betterproto/compile/importing.py | 66 ++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index 3988718..faab18f 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -64,14 +64,25 @@ def get_ref_type( imports.add(f"import {string_import} as {string_alias}") return f"{string_alias}.{py_type}" + current_package: List[str] = package.split(".") if package else [] py_package: List[str] = source_package.split(".") if source_package else [] py_type: str = pythonize_class_name(source_type) - current_package: List[str] = package.split(".") if package else [] + if py_package == current_package: + return import_sibling(py_type) - # importing sibling + if py_package[0 : len(current_package)] == current_package: + return import_descendent(current_package, imports, py_package, py_type) + + if current_package[0 : len(py_package)] == py_package: + return import_ancestor(current_package, imports, py_package, py_type) + + return import_cousin(current_package, imports, py_package, py_type) + + +def import_sibling(py_type): """ - package = + package = name = Foo package = foo @@ -80,47 +91,46 @@ def get_ref_type( package = foo.bar name = foo.bar.Baz """ - if py_package == current_package: - return f'"{py_type}"' + return f'"{py_type}"' - # importing child & descendent: + +def import_descendent(current_package, imports, py_package, py_type): """ - package = + package = name = foo.Bar - - package = + + package = name = foo.bar.Baz """ - if py_package[0 : len(current_package)] == current_package: - importing_descendent = py_package[len(current_package) :] - string_from = ".".join(importing_descendent[0:-1]) - string_import = importing_descendent[-1] + importing_descendent = py_package[len(current_package) :] + string_from = ".".join(importing_descendent[0:-1]) + string_import = importing_descendent[-1] + if string_from: + string_alias = "_".join(importing_descendent) + imports.add(f"from .{string_from} import {string_import} as {string_alias}") + return f"{string_alias}.{py_type}" + else: + imports.add(f"from . import {string_import}") + return f"{string_import}.{py_type}" - if string_from: - string_alias = "_".join(importing_descendent) - imports.add(f"from .{string_from} import {string_import} as {string_alias}") - return f"{string_alias}.{py_type}" - else: - imports.add(f"from . import {string_import}") - return f"{string_import}.{py_type}" - # importing parent & ancestor +def import_ancestor(current_package, imports, py_package, py_type): """ package = foo.bar name = foo.Foo - + package = foo name = Bar - + package = foo.bar.baz name = Bar """ - if current_package[0 : len(py_package)] == py_package: - distance_up = len(current_package) - len(py_package) - imports.add(f"from .{'.' * distance_up} import {py_type}") - return py_type + distance_up = len(current_package) - len(py_package) + imports.add(f"from .{'.' * distance_up} import {py_type}") + return py_type - # importing unrelated or cousin + +def import_cousin(current_package, imports, py_package, py_type): """ package = foo.bar name = baz.Foo From 7c8d47de6d9434114d827566e2bc3347ee10e87f Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Mon, 8 Jun 2020 17:28:58 +0200 Subject: [PATCH 09/33] Add test cases for cousin imports that break due to aliases starting with two underscores --- betterproto/tests/inputs/config.py | 3 +++ .../tests/inputs/import_cousin_package/cousin.proto | 6 ++++++ .../tests/inputs/import_cousin_package/test.proto | 11 +++++++++++ .../import_cousin_package_same_name/cousin.proto | 6 ++++++ .../inputs/import_cousin_package_same_name/test.proto | 11 +++++++++++ 5 files changed, 37 insertions(+) create mode 100644 betterproto/tests/inputs/import_cousin_package/cousin.proto create mode 100644 betterproto/tests/inputs/import_cousin_package/test.proto create mode 100644 betterproto/tests/inputs/import_cousin_package_same_name/cousin.proto create mode 100644 betterproto/tests/inputs/import_cousin_package_same_name/test.proto diff --git a/betterproto/tests/inputs/config.py b/betterproto/tests/inputs/config.py index 48e4b09..7b52cd5 100644 --- a/betterproto/tests/inputs/config.py +++ b/betterproto/tests/inputs/config.py @@ -16,6 +16,9 @@ tests = { packages = { "import_root_package_from_child": ".child", "import_parent_package_from_child": ".parent.child", + "import_root_package_from_nested_child": ".nested.child", + "import_cousin_package": ".test.subpackage", + "import_cousin_package_same_name": ".test.subpackage", "repeatedmessage": ".repeatedmessage", "service": ".service", } diff --git a/betterproto/tests/inputs/import_cousin_package/cousin.proto b/betterproto/tests/inputs/import_cousin_package/cousin.proto new file mode 100644 index 0000000..4361545 --- /dev/null +++ b/betterproto/tests/inputs/import_cousin_package/cousin.proto @@ -0,0 +1,6 @@ +syntax = "proto3"; + +package cousin.cousin_subpackage; + +message CousinMessage { +} diff --git a/betterproto/tests/inputs/import_cousin_package/test.proto b/betterproto/tests/inputs/import_cousin_package/test.proto new file mode 100644 index 0000000..53f3b7f --- /dev/null +++ b/betterproto/tests/inputs/import_cousin_package/test.proto @@ -0,0 +1,11 @@ +syntax = "proto3"; + +package test.subpackage; + +import "cousin.proto"; + +// Verify that we can import message unrelated to us + +message Test { + cousin.cousin_subpackage.CousinMessage message = 1; +} diff --git a/betterproto/tests/inputs/import_cousin_package_same_name/cousin.proto b/betterproto/tests/inputs/import_cousin_package_same_name/cousin.proto new file mode 100644 index 0000000..9253b95 --- /dev/null +++ b/betterproto/tests/inputs/import_cousin_package_same_name/cousin.proto @@ -0,0 +1,6 @@ +syntax = "proto3"; + +package cousin.subpackage; + +message CousinMessage { +} diff --git a/betterproto/tests/inputs/import_cousin_package_same_name/test.proto b/betterproto/tests/inputs/import_cousin_package_same_name/test.proto new file mode 100644 index 0000000..fe31b5f --- /dev/null +++ b/betterproto/tests/inputs/import_cousin_package_same_name/test.proto @@ -0,0 +1,11 @@ +syntax = "proto3"; + +package test.subpackage; + +import "cousin.proto"; + +// Verify that we can import a message unrelated to us, in a subpackage with the same name as us. + +message Test { + cousin.subpackage.CousinMessage message = 1; +} From 3105e952ea6683ed1d16938f8a2e15cae4dea95f Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Tue, 9 Jun 2020 21:07:20 +0200 Subject: [PATCH 10/33] Fixes issue where importing cousin where path has a package with the same name broke import --- betterproto/compile/importing.py | 11 +++++------ betterproto/tests/test_get_ref_type.py | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index faab18f..aaf4a16 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -1,3 +1,4 @@ +import os import re from typing import Dict, List, Type @@ -138,16 +139,14 @@ def import_cousin(current_package, imports, py_package, py_type): package = foo.bar.baz name = foo.example.Bar """ - shared_ancestory = [ - pair[0] for pair in zip(current_package, py_package) if pair[0] == pair[1] - ] - distance_up = len(current_package) - len(shared_ancestory) + shared_ancestry = os.path.commonprefix([current_package, py_package]) + distance_up = len(current_package) - len(shared_ancestry) string_from = f".{'.' * distance_up}" + ".".join( - py_package[len(shared_ancestory) : -1] + py_package[len(shared_ancestry) : -1] ) string_import = py_package[-1] alias = f"{'_' * distance_up}" + safe_snake_case( - ".".join(py_package[len(shared_ancestory) :]) + ".".join(py_package[len(shared_ancestry) :]) ) imports.add(f"from {string_from} import {string_import} as {alias}") return f"{alias}.{py_type}" diff --git a/betterproto/tests/test_get_ref_type.py b/betterproto/tests/test_get_ref_type.py index 16a5466..b02dfea 100644 --- a/betterproto/tests/test_get_ref_type.py +++ b/betterproto/tests/test_get_ref_type.py @@ -242,6 +242,26 @@ def test_import_cousin_package(): assert name == "_y.Message" +def test_import_cousin_package_different_name(): + imports = set() + name = get_ref_type( + package="test.package1", imports=imports, source_type="cousin.package2.Message" + ) + + assert imports == {"from ...cousin import package2 as __cousin_package2"} + assert name == "__cousin_package2.Message" + + +def test_import_cousin_package_same_name(): + imports = set() + name = get_ref_type( + package="test.package", imports=imports, source_type="cousin.package.Message" + ) + + assert imports == {"from ...cousin import package as __cousin_package"} + assert name == "__cousin_package.Message" + + def test_import_far_cousin_package(): imports = set() name = get_ref_type(package="a.x.y", imports=imports, source_type="a.b.c.Message") From 8567892352cb90a8fc8a8c894ef79db95001ab0c Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Tue, 9 Jun 2020 21:10:30 +0200 Subject: [PATCH 11/33] Simplify logic for generating package init files --- betterproto/plugin.py | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/betterproto/plugin.py b/betterproto/plugin.py index f64a563..3d44a2b 100755 --- a/betterproto/plugin.py +++ b/betterproto/plugin.py @@ -2,12 +2,14 @@ import itertools import os.path +import pathlib import re import sys import textwrap from typing import List -from betterproto.compile.importing import get_ref_type + import betterproto +from betterproto.compile.importing import get_ref_type from betterproto.compile.naming import ( pythonize_class_name, pythonize_field_name, @@ -135,11 +137,7 @@ def generate_code(request, response): ): continue - # if proto_file.package: - output_file = ( - os.path.join(proto_file.package.replace(".", os.path.sep), "__init__") - + ".py" - ) + output_file = str(pathlib.Path(*proto_file.package.split("."), "__init__.py")) if output_file not in output_map: output_map[output_file] = {"package": proto_file.package, "files": []} @@ -359,24 +357,17 @@ def generate_code(request, response): mode=black.FileMode(target_versions=set([black.TargetVersion.PY37])), ) - output_files = list(output_map.keys()) - for output_file in output_files: - path = os.path.dirname(output_file) + # Make each output directory a package with __init__ file + output_paths = set(pathlib.Path(path) for path in output_map.keys()) + output_dirs = [directory for path in output_paths for directory in path.parents] + init_files = set(d.joinpath("__init__.py") for d in output_dirs) - output_paths - for sub_path in path.split(os.path.sep): - init_file = os.path.join(sub_path, "__init__.py") + for init_file in init_files: + init = response.file.add() + init.name = str(init_file) - if init_file in output_files: - continue - - output_files.append(init_file) - init = response.file.add() - init.name = init_file - init.content = b"" - - filenames = sorted([f.name for f in response.file]) - for fname in filenames: - print(f"Writing {repr(fname)}", file=sys.stderr) + for filename in sorted(output_paths.union(init_files)): + print(f"Writing {filename}", file=sys.stderr) def main(): From 76db2f153e2ae34da2cf49fd580ef8522f384d00 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Tue, 9 Jun 2020 22:27:26 +0200 Subject: [PATCH 12/33] Add import aliases to ancestor imports --- betterproto/compile/importing.py | 11 +++++++++-- betterproto/tests/test_get_ref_type.py | 20 ++++++++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index aaf4a16..cdef91f 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -127,8 +127,15 @@ def import_ancestor(current_package, imports, py_package, py_type): name = Bar """ distance_up = len(current_package) - len(py_package) - imports.add(f"from .{'.' * distance_up} import {py_type}") - return py_type + if py_package: + string_import = py_package[-1] + string_alias = f"__{'_' * distance_up}{string_import}" + string_from = f"..{'.' * distance_up}" + imports.add(f"from {string_from} import {string_import} as {string_alias}") + return f"{string_alias}.{py_type}" + else: + imports.add(f"from .{'.' * distance_up} import {py_type}") + return py_type def import_cousin(current_package, imports, py_package, py_type): diff --git a/betterproto/tests/test_get_ref_type.py b/betterproto/tests/test_get_ref_type.py index b02dfea..a2d089d 100644 --- a/betterproto/tests/test_get_ref_type.py +++ b/betterproto/tests/test_get_ref_type.py @@ -174,8 +174,8 @@ def test_import_parent_package_from_child(): package="package.child", imports=imports, source_type="package.Message" ) - assert imports == {"from .. import Message"} - assert name == "Message" + assert imports == {"from ... import package as ___package"} + assert name == "___package.Message" def test_import_parent_package_from_deeply_nested_child(): @@ -186,8 +186,20 @@ def test_import_parent_package_from_deeply_nested_child(): source_type="package.deeply.nested.Message", ) - assert imports == {"from .. import Message"} - assert name == "Message" + assert imports == {"from ... import nested as ___nested"} + assert name == "___nested.Message" + + +def test_import_ancestor_package_from_nested_child(): + imports = set() + name = get_ref_type( + package="package.ancestor.nested.child", + imports=imports, + source_type="package.ancestor.Message", + ) + + assert imports == {"from .... import ancestor as ____ancestor"} + assert name == "____ancestor.Message" def test_import_root_package_from_child(): From 1a95a7988e523ef4498d028f1cd206cae6920f34 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Tue, 9 Jun 2020 22:54:52 +0200 Subject: [PATCH 13/33] Ensure uniquely generated import aliases are not name mangled (python.org/dev/peps/pep-0008/#id34) --- betterproto/compile/importing.py | 12 ++--- .../casing_message_field_uppercase.py | 4 +- betterproto/tests/inputs/nested/nested.proto | 4 +- .../tests/inputs/nested2/nested2.proto | 19 ++++++++ .../tests/inputs/nested2/package.proto | 7 +++ betterproto/tests/test_get_ref_type.py | 44 +++++++++---------- 6 files changed, 58 insertions(+), 32 deletions(-) create mode 100644 betterproto/tests/inputs/nested2/nested2.proto create mode 100644 betterproto/tests/inputs/nested2/package.proto diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index cdef91f..091812b 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -129,7 +129,8 @@ def import_ancestor(current_package, imports, py_package, py_type): distance_up = len(current_package) - len(py_package) if py_package: string_import = py_package[-1] - string_alias = f"__{'_' * distance_up}{string_import}" + # Add trailing __ to avoid name mangling (python.org/dev/peps/pep-0008/#id34) + string_alias = f"_{'_' * distance_up}{string_import}__" string_from = f"..{'.' * distance_up}" imports.add(f"from {string_from} import {string_import} as {string_alias}") return f"{string_alias}.{py_type}" @@ -152,8 +153,9 @@ def import_cousin(current_package, imports, py_package, py_type): py_package[len(shared_ancestry) : -1] ) string_import = py_package[-1] - alias = f"{'_' * distance_up}" + safe_snake_case( + # Add trailing __ to avoid name mangling (python.org/dev/peps/pep-0008/#id34) + string_alias = f"{'_' * distance_up}" + safe_snake_case( ".".join(py_package[len(shared_ancestry) :]) - ) - imports.add(f"from {string_from} import {string_import} as {alias}") - return f"{alias}.{py_type}" + ) + "__" + imports.add(f"from {string_from} import {string_import} as {string_alias}") + return f"{string_alias}.{py_type}" diff --git a/betterproto/tests/inputs/casing_message_field_uppercase/casing_message_field_uppercase.py b/betterproto/tests/inputs/casing_message_field_uppercase/casing_message_field_uppercase.py index d77119e..e0dee0c 100644 --- a/betterproto/tests/inputs/casing_message_field_uppercase/casing_message_field_uppercase.py +++ b/betterproto/tests/inputs/casing_message_field_uppercase/casing_message_field_uppercase.py @@ -1,6 +1,4 @@ -from betterproto.tests.output_betterproto.casing_message_field_uppercase.casing_message_field_uppercase import ( - Test, -) +from betterproto.tests.output_betterproto.casing_message_field_uppercase import Test def test_message_casing(): diff --git a/betterproto/tests/inputs/nested/nested.proto b/betterproto/tests/inputs/nested/nested.proto index 2eaef59..98bafd9 100644 --- a/betterproto/tests/inputs/nested/nested.proto +++ b/betterproto/tests/inputs/nested/nested.proto @@ -10,9 +10,9 @@ message Test { Nested nested = 1; Sibling sibling = 2; -// Sibling sibling2 = 3; + Sibling sibling2 = 3; } message Sibling { int32 foo = 1; -} +} \ No newline at end of file diff --git a/betterproto/tests/inputs/nested2/nested2.proto b/betterproto/tests/inputs/nested2/nested2.proto new file mode 100644 index 0000000..3e39918 --- /dev/null +++ b/betterproto/tests/inputs/nested2/nested2.proto @@ -0,0 +1,19 @@ +syntax = "proto3"; + +import "package.proto"; + +message Game { + message Player { + enum Race { + human = 0; + orc = 1; + } + } +} + +message Test { + Game game = 1; + Game.Player GamePlayer = 2; + Game.Player.Race GamePlayerRace = 3; + equipment.Weapon Weapon = 4; +} \ No newline at end of file diff --git a/betterproto/tests/inputs/nested2/package.proto b/betterproto/tests/inputs/nested2/package.proto new file mode 100644 index 0000000..4466256 --- /dev/null +++ b/betterproto/tests/inputs/nested2/package.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package equipment; + +message Weapon { + +} \ No newline at end of file diff --git a/betterproto/tests/test_get_ref_type.py b/betterproto/tests/test_get_ref_type.py index a2d089d..412b4ed 100644 --- a/betterproto/tests/test_get_ref_type.py +++ b/betterproto/tests/test_get_ref_type.py @@ -174,8 +174,8 @@ def test_import_parent_package_from_child(): package="package.child", imports=imports, source_type="package.Message" ) - assert imports == {"from ... import package as ___package"} - assert name == "___package.Message" + assert imports == {"from ... import package as __package__"} + assert name == "__package__.Message" def test_import_parent_package_from_deeply_nested_child(): @@ -186,8 +186,8 @@ def test_import_parent_package_from_deeply_nested_child(): source_type="package.deeply.nested.Message", ) - assert imports == {"from ... import nested as ___nested"} - assert name == "___nested.Message" + assert imports == {"from ... import nested as __nested__"} + assert name == "__nested__.Message" def test_import_ancestor_package_from_nested_child(): @@ -198,8 +198,8 @@ def test_import_ancestor_package_from_nested_child(): source_type="package.ancestor.Message", ) - assert imports == {"from .... import ancestor as ____ancestor"} - assert name == "____ancestor.Message" + assert imports == {"from .... import ancestor as ___ancestor__"} + assert name == "___ancestor__.Message" def test_import_root_package_from_child(): @@ -224,16 +224,16 @@ def test_import_unrelated_package(): imports = set() name = get_ref_type(package="a", imports=imports, source_type="p.Message") - assert imports == {"from .. import p as _p"} - assert name == "_p.Message" + assert imports == {"from .. import p as _p__"} + assert name == "_p__.Message" def test_import_unrelated_nested_package(): imports = set() name = get_ref_type(package="a.b", imports=imports, source_type="p.q.Message") - assert imports == {"from ...p import q as __p_q"} - assert name == "__p_q.Message" + assert imports == {"from ...p import q as __p_q__"} + assert name == "__p_q__.Message" def test_import_unrelated_deeply_nested_package(): @@ -242,16 +242,16 @@ def test_import_unrelated_deeply_nested_package(): package="a.b.c.d", imports=imports, source_type="p.q.r.s.Message" ) - assert imports == {"from .....p.q.r import s as ____p_q_r_s"} - assert name == "____p_q_r_s.Message" + assert imports == {"from .....p.q.r import s as ____p_q_r_s__"} + assert name == "____p_q_r_s__.Message" def test_import_cousin_package(): imports = set() name = get_ref_type(package="a.x", imports=imports, source_type="a.y.Message") - assert imports == {"from .. import y as _y"} - assert name == "_y.Message" + assert imports == {"from .. import y as _y__"} + assert name == "_y__.Message" def test_import_cousin_package_different_name(): @@ -260,8 +260,8 @@ def test_import_cousin_package_different_name(): package="test.package1", imports=imports, source_type="cousin.package2.Message" ) - assert imports == {"from ...cousin import package2 as __cousin_package2"} - assert name == "__cousin_package2.Message" + assert imports == {"from ...cousin import package2 as __cousin_package2__"} + assert name == "__cousin_package2__.Message" def test_import_cousin_package_same_name(): @@ -270,16 +270,16 @@ def test_import_cousin_package_same_name(): package="test.package", imports=imports, source_type="cousin.package.Message" ) - assert imports == {"from ...cousin import package as __cousin_package"} - assert name == "__cousin_package.Message" + assert imports == {"from ...cousin import package as __cousin_package__"} + assert name == "__cousin_package__.Message" def test_import_far_cousin_package(): imports = set() name = get_ref_type(package="a.x.y", imports=imports, source_type="a.b.c.Message") - assert imports == {"from ...b import c as __b_c"} - assert name == "__b_c.Message" + assert imports == {"from ...b import c as __b_c__"} + assert name == "__b_c__.Message" def test_import_far_far_cousin_package(): @@ -288,8 +288,8 @@ def test_import_far_far_cousin_package(): package="a.x.y.z", imports=imports, source_type="a.b.c.d.Message" ) - assert imports == {"from ....b.c import d as ___b_c_d"} - assert name == "___b_c_d.Message" + assert imports == {"from ....b.c import d as ___b_c_d__"} + assert name == "___b_c_d__.Message" @pytest.mark.parametrize( From fb54917f2c86be46a263e7375014d92dc283ff3d Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Wed, 10 Jun 2020 22:42:38 +0200 Subject: [PATCH 14/33] Detect entry-point of tests automatically --- README.md | 4 ++-- betterproto/tests/README.md | 13 +++++----- betterproto/tests/inputs/config.py | 15 +----------- betterproto/tests/test_inputs.py | 38 ++++++++++++++++++++---------- betterproto/tests/util.py | 27 ++++++++++++++++++++- 5 files changed, 61 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 385d1ea..8e9f4cd 100644 --- a/README.md +++ b/README.md @@ -312,7 +312,7 @@ $ pip install -e . This project enforces [black](https://github.com/psf/black) python code formatting. -Before commiting changes run: +Before committing changes run: ```bash pipenv run black . @@ -333,7 +333,7 @@ Adding a standard test case is easy. - Create a new directory `betterproto/tests/inputs/` - add `.proto` with a message called `Test` - - add `.json` with some test data + - add `.json` with some test data (optional) It will be picked up automatically when you run the tests. diff --git a/betterproto/tests/README.md b/betterproto/tests/README.md index 1892cea..51cd8ec 100644 --- a/betterproto/tests/README.md +++ b/betterproto/tests/README.md @@ -12,12 +12,12 @@ inputs/ ## Test case directory structure -Each testcase has a `.proto` file with a message called `Test`, a matching `.json` file and optionally a custom test file called `test_*.py`. +Each testcase has a `.proto` file with a message called `Test`, and optionally a matching `.json` file and a custom test called `test_*.py`. ```bash bool/ bool.proto - bool.json + bool.json # optional test_bool.py # optional ``` @@ -61,21 +61,22 @@ def test_value(): The following tests are automatically executed for all cases: -- [x] Can the generated python code imported? +- [x] Can the generated python code be imported? - [x] Can the generated message class be instantiated? - [x] Is the generated code compatible with the Google's `grpc_tools.protoc` implementation? + - _when `.json` is present_ ## Running the tests -- `pipenv run generate` - This generates +- `pipenv run generate` + This generates: - `betterproto/tests/output_betterproto` — *the plugin generated python classes* - `betterproto/tests/output_reference` — *reference implementation classes* - `pipenv run test` ## Intentionally Failing tests -The standard test suite includes tests that fail by intention. These tests document known bugs and missing features that are intended to be corrented in the future. +The standard test suite includes tests that fail by intention. These tests document known bugs and missing features that are intended to be corrected in the future. When running `pytest`, they show up as `x` or `X` in the test results. diff --git a/betterproto/tests/inputs/config.py b/betterproto/tests/inputs/config.py index 7b52cd5..4e416e0 100644 --- a/betterproto/tests/inputs/config.py +++ b/betterproto/tests/inputs/config.py @@ -1,6 +1,6 @@ # Test cases that are expected to fail, e.g. unimplemented features or bug-fixes. # Remove from list when fixed. -tests = { +xfail = { "import_circular_dependency", "oneof_enum", # 63 "casing_message_field_uppercase", # 11 @@ -10,19 +10,6 @@ tests = { "googletypes_value", # 9 } - -# Defines where the main package for this test resides. -# Needed to test relative package imports. -packages = { - "import_root_package_from_child": ".child", - "import_parent_package_from_child": ".parent.child", - "import_root_package_from_nested_child": ".nested.child", - "import_cousin_package": ".test.subpackage", - "import_cousin_package_same_name": ".test.subpackage", - "repeatedmessage": ".repeatedmessage", - "service": ".service", -} - services = { "googletypes_response", "googletypes_response_embedded", diff --git a/betterproto/tests/test_inputs.py b/betterproto/tests/test_inputs.py index 6778425..ede31d6 100644 --- a/betterproto/tests/test_inputs.py +++ b/betterproto/tests/test_inputs.py @@ -3,6 +3,7 @@ import json import os import sys from collections import namedtuple +from types import ModuleType from typing import Set import pytest @@ -10,7 +11,12 @@ import pytest import betterproto from betterproto.tests.inputs import config as test_input_config from betterproto.tests.mocks import MockChannel -from betterproto.tests.util import get_directories, get_test_case_json_data, inputs_path +from betterproto.tests.util import ( + find_module, + get_directories, + get_test_case_json_data, + inputs_path, +) # Force pure-python implementation instead of C++, otherwise imports # break things because we can't properly reset the symbol database. @@ -50,16 +56,17 @@ class TestCases: test_cases = TestCases( path=inputs_path, services=test_input_config.services, - xfail=test_input_config.tests, + xfail=test_input_config.xfail, ) plugin_output_package = "betterproto.tests.output_betterproto" reference_output_package = "betterproto.tests.output_reference" +TestData = namedtuple("TestData", ["plugin_module", "reference_module", "json_data"]) -TestData = namedtuple( - "TestData", ["plugin_module", "reference_module", "json_data", "entry_point"] -) + +def module_has_entry_point(module: ModuleType): + return any(hasattr(module, attr) for attr in ["Test", "TestStub"]) @pytest.fixture @@ -77,18 +84,23 @@ def test_data(request): sys.path.append(reference_module_root) - test_package = test_case_name + test_input_config.packages.get(test_case_name, "") + plugin_module = importlib.import_module(f"{plugin_output_package}.{test_case_name}") + + plugin_module_entry_point = find_module(plugin_module, module_has_entry_point) + + if not plugin_module_entry_point: + raise Exception( + f"Test case {repr(test_case_name)} has no entry point. " + + "Please add a proto message or service called Test and recompile." + ) yield ( TestData( - plugin_module=importlib.import_module( - f"{plugin_output_package}.{test_package}" - ), + plugin_module=plugin_module_entry_point, reference_module=lambda: importlib.import_module( f"{reference_output_package}.{test_case_name}.{test_case_name}_pb2" ), json_data=get_test_case_json_data(test_case_name), - entry_point=test_package, ) ) @@ -111,7 +123,7 @@ def test_message_equality(test_data: TestData) -> None: @pytest.mark.parametrize("test_data", test_cases.messages_with_json, indirect=True) def test_message_json(repeat, test_data: TestData) -> None: - plugin_module, _, json_data, entry_point = test_data + plugin_module, _, json_data = test_data for _ in range(repeat): message: betterproto.Message = plugin_module.Test() @@ -124,13 +136,13 @@ def test_message_json(repeat, test_data: TestData) -> None: @pytest.mark.parametrize("test_data", test_cases.services, indirect=True) def test_service_can_be_instantiated(test_data: TestData) -> None: - plugin_module, _, json_data, entry_point = test_data + plugin_module, _, json_data = test_data plugin_module.TestStub(MockChannel()) @pytest.mark.parametrize("test_data", test_cases.messages_with_json, indirect=True) def test_binary_compatibility(repeat, test_data: TestData) -> None: - plugin_module, reference_module, json_data, entry_point = test_data + plugin_module, reference_module, json_data = test_data reference_instance = Parse(json_data, reference_module().Test()) reference_binary_output = reference_instance.SerializeToString() diff --git a/betterproto/tests/util.py b/betterproto/tests/util.py index a7cff7a..40b10c6 100644 --- a/betterproto/tests/util.py +++ b/betterproto/tests/util.py @@ -1,6 +1,9 @@ +import importlib import os +import pathlib import subprocess -from typing import Generator +from types import ModuleType +from typing import Callable, Generator, Optional os.environ["PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION"] = "python" @@ -60,3 +63,25 @@ def get_test_case_json_data(test_case_name, json_file_name=None): with open(test_data_file_path) as fh: return fh.read() + + +def find_module( + module: ModuleType, predicate: Callable[[ModuleType], bool] +) -> Optional[ModuleType]: + if predicate(module): + return module + + module_path = pathlib.Path(*module.__path__) + + for sub in list(module_path.glob("**/")): + if not sub.is_dir() or sub == module_path: + continue + sub_module_path = sub.relative_to(module_path) + sub_module_name = ".".join(sub_module_path.parts) + + sub_module = importlib.import_module(f".{sub_module_name}", module.__name__) + + if predicate(sub_module): + return sub_module + + return None From 34c34bd15a70c0812f51ab2d80e8527be0067def Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Wed, 10 Jun 2020 23:29:01 +0200 Subject: [PATCH 15/33] Add failing test for importing a message from package that looks like a nested type #87 --- betterproto/tests/inputs/config.py | 3 ++- .../inputs/import_capitalized_package/capitalized.proto | 8 ++++++++ .../tests/inputs/import_capitalized_package/test.proto | 9 +++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 betterproto/tests/inputs/import_capitalized_package/capitalized.proto create mode 100644 betterproto/tests/inputs/import_capitalized_package/test.proto diff --git a/betterproto/tests/inputs/config.py b/betterproto/tests/inputs/config.py index 4e416e0..0191227 100644 --- a/betterproto/tests/inputs/config.py +++ b/betterproto/tests/inputs/config.py @@ -7,7 +7,8 @@ xfail = { "namespace_keywords", # 70 "namespace_builtin_types", # 53 "googletypes_struct", # 9 - "googletypes_value", # 9 + "googletypes_value", # 9, + "import_capitalized_package", } services = { diff --git a/betterproto/tests/inputs/import_capitalized_package/capitalized.proto b/betterproto/tests/inputs/import_capitalized_package/capitalized.proto new file mode 100644 index 0000000..0b73bab --- /dev/null +++ b/betterproto/tests/inputs/import_capitalized_package/capitalized.proto @@ -0,0 +1,8 @@ +syntax = "proto3"; + + +package Capitalized; + +message Message { + +} diff --git a/betterproto/tests/inputs/import_capitalized_package/test.proto b/betterproto/tests/inputs/import_capitalized_package/test.proto new file mode 100644 index 0000000..f94bbc9 --- /dev/null +++ b/betterproto/tests/inputs/import_capitalized_package/test.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +import "capitalized.proto"; + +// Tests that we can import from a package with a capital name, that looks like a nested type, but isn't. + +message Test { + Capitalized.Message message = 1; +} From 65c1f366ef0252d9ce57cb37aae30a085b43bbe9 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Wed, 10 Jun 2020 23:47:07 +0200 Subject: [PATCH 16/33] Update readme with new output structure and fix example inconsistencies --- README.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 8e9f4cd..6c8fe24 100644 --- a/README.md +++ b/README.md @@ -68,14 +68,15 @@ message Greeting { You can run the following: ```sh -$ protoc -I . --python_betterproto_out=. example.proto +$ mkdir lib +$ protoc -I . --python_betterproto_out=lib example.proto ``` -This will generate `hello.py` which looks like: +This will generate `lib/hello/__init__.py` which looks like: -```py +```python # Generated by the protocol buffer compiler. DO NOT EDIT! -# sources: hello.proto +# sources: example.proto # plugin: python-betterproto from dataclasses import dataclass @@ -83,7 +84,7 @@ import betterproto @dataclass -class Hello(betterproto.Message): +class Greeting(betterproto.Message): """Greeting represents a message you can tell a user.""" message: str = betterproto.string_field(1) @@ -91,23 +92,23 @@ class Hello(betterproto.Message): Now you can use it! -```py ->>> from hello import Hello ->>> test = Hello() +```python +>>> from lib.hello import Greeting +>>> test = Greeting() >>> test -Hello(message='') +Greeting(message='') >>> test.message = "Hey!" >>> test -Hello(message="Hey!") +Greeting(message="Hey!") >>> serialized = bytes(test) >>> serialized b'\n\x04Hey!' ->>> another = Hello().parse(serialized) +>>> another = Greeting().parse(serialized) >>> another -Hello(message="Hey!") +Greeting(message="Hey!") >>> another.to_dict() {"message": "Hey!"} From 5d2f3a2cd9238cafaec30f2f030cac9419949f40 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Thu, 11 Jun 2020 00:06:50 +0200 Subject: [PATCH 17/33] Remove fixed test from xfail list #11 --- .../casing_message_field_uppercase.json | 5 ----- betterproto/tests/inputs/config.py | 2 +- betterproto/tests/inputs/example/example.proto | 8 ++++++++ 3 files changed, 9 insertions(+), 6 deletions(-) delete mode 100644 betterproto/tests/inputs/casing_message_field_uppercase/casing_message_field_uppercase.json create mode 100644 betterproto/tests/inputs/example/example.proto diff --git a/betterproto/tests/inputs/casing_message_field_uppercase/casing_message_field_uppercase.json b/betterproto/tests/inputs/casing_message_field_uppercase/casing_message_field_uppercase.json deleted file mode 100644 index 83bd111..0000000 --- a/betterproto/tests/inputs/casing_message_field_uppercase/casing_message_field_uppercase.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "UPPERCASE": 10, - "UPPERCASE_V2": 10, - "UPPER_CAMEL_CASE": 10 -} diff --git a/betterproto/tests/inputs/config.py b/betterproto/tests/inputs/config.py index 0191227..eab5ea4 100644 --- a/betterproto/tests/inputs/config.py +++ b/betterproto/tests/inputs/config.py @@ -3,12 +3,12 @@ xfail = { "import_circular_dependency", "oneof_enum", # 63 - "casing_message_field_uppercase", # 11 "namespace_keywords", # 70 "namespace_builtin_types", # 53 "googletypes_struct", # 9 "googletypes_value", # 9, "import_capitalized_package", + "example", # This is the example in the readme. Not a test. } services = { diff --git a/betterproto/tests/inputs/example/example.proto b/betterproto/tests/inputs/example/example.proto new file mode 100644 index 0000000..edc4d87 --- /dev/null +++ b/betterproto/tests/inputs/example/example.proto @@ -0,0 +1,8 @@ +syntax = "proto3"; + +package hello; + +// Greeting represents a message you can tell a user. +message Greeting { + string message = 1; +} From 3ca75dadd7679ecb8abbf2fdb82852b085b07be4 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Thu, 11 Jun 2020 00:22:23 +0200 Subject: [PATCH 18/33] Remove dependency on stringcase, apply black --- betterproto/__init__.py | 5 ++--- betterproto/compile/importing.py | 8 +++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/betterproto/__init__.py b/betterproto/__init__.py index 9ed73f6..bcd9292 100644 --- a/betterproto/__init__.py +++ b/betterproto/__init__.py @@ -28,9 +28,8 @@ from typing import ( import grpclib.const -import stringcase -from .casing import safe_snake_case, snake_case +from .casing import camel_case, safe_snake_case, snake_case if TYPE_CHECKING: from grpclib._protocols import IProtoMessage @@ -131,7 +130,7 @@ DATETIME_ZERO = datetime_default_gen() class Casing(enum.Enum): """Casing constants for serialization.""" - CAMEL = stringcase.camelcase + CAMEL = camel_case SNAKE = snake_case diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index 091812b..115ba78 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -154,8 +154,10 @@ def import_cousin(current_package, imports, py_package, py_type): ) string_import = py_package[-1] # Add trailing __ to avoid name mangling (python.org/dev/peps/pep-0008/#id34) - string_alias = f"{'_' * distance_up}" + safe_snake_case( - ".".join(py_package[len(shared_ancestry) :]) - ) + "__" + string_alias = ( + f"{'_' * distance_up}" + + safe_snake_case(".".join(py_package[len(shared_ancestry) :])) + + "__" + ) imports.add(f"from {string_from} import {string_import} as {string_alias}") return f"{string_alias}.{py_type}" From 83e13aa606a64bd7ca1b91bc1de39ce1242912ff Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Thu, 11 Jun 2020 00:43:58 +0200 Subject: [PATCH 19/33] Fix method name --- betterproto/casing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/betterproto/casing.py b/betterproto/casing.py index 919f02e..01e10bb 100644 --- a/betterproto/casing.py +++ b/betterproto/casing.py @@ -76,8 +76,8 @@ def camel_case(value: str): """ Capitalize all words except first and remove symbols. """ - return capitalize_first(pascal_case(value)) + return lowercase_first(pascal_case(value)) -def capitalize_first(value: str): +def lowercase_first(value: str): return value[0:1].lower() + value[1:] From c88edfd09370c473f98ef3507d67dfb0fe985849 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Fri, 12 Jun 2020 13:54:14 +0200 Subject: [PATCH 20/33] Support running plugin without installing betterproto --- betterproto/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/betterproto/plugin.py b/betterproto/plugin.py index 3d44a2b..33890af 100755 --- a/betterproto/plugin.py +++ b/betterproto/plugin.py @@ -1,5 +1,4 @@ #!/usr/bin/env python - import itertools import os.path import pathlib @@ -8,6 +7,7 @@ import sys import textwrap from typing import List +sys.path.append(str(pathlib.Path(__file__).parent.parent)) # add betterproto to path import betterproto from betterproto.compile.importing import get_ref_type from betterproto.compile.naming import ( From d9fa6d2dd35c377bc8f049abcaefb8cafabd818c Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Fri, 12 Jun 2020 13:55:55 +0200 Subject: [PATCH 21/33] Fixes issue where generated Google Protobuf messages imported from betterproto.lib instead of using local forward references --- betterproto/compile/importing.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index 115ba78..69cca91 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -42,10 +42,8 @@ def get_ref_type( Return a Python type name for a proto type reference. Adds the import if necessary. Unwraps well known type if required. """ - is_wrapper = source_type in WRAPPER_TYPES - if unwrap: - if is_wrapper: + if source_type in WRAPPER_TYPES: wrapped_type = type(WRAPPER_TYPES[source_type]().value) return f"Optional[{wrapped_type.__name__}]" @@ -57,18 +55,18 @@ def get_ref_type( source_package, source_type = parse_source_type_name(source_type) - # Use precompiled classes for google.protobuf.* objects - if source_package == "google.protobuf": - string_import = f"betterproto.lib.{source_package}" - py_type = source_type - string_alias = safe_snake_case(string_import) - imports.add(f"import {string_import} as {string_alias}") - return f"{string_alias}.{py_type}" - current_package: List[str] = package.split(".") if package else [] py_package: List[str] = source_package.split(".") if source_package else [] py_type: str = pythonize_class_name(source_type) + compiling_google_protobuf = current_package == ["google", "protobuf"] + importing_google_protobuf = py_package == ["google", "protobuf"] + if importing_google_protobuf and not compiling_google_protobuf: + py_package = ["betterproto", "lib"] + py_package + + if py_package[:1] == ["betterproto"]: + return import_root(imports, py_package, py_type) + if py_package == current_package: return import_sibling(py_type) @@ -81,6 +79,13 @@ def get_ref_type( return import_cousin(current_package, imports, py_package, py_type) +def import_root(imports, py_package, py_type): + string_import = ".".join(py_package) + string_alias = safe_snake_case(string_import) + imports.add(f"import {string_import} as {string_alias}") + return f"{string_alias}.{py_type}" + + def import_sibling(py_type): """ package = From 32c8e77274761e677a4f9432dbc3b3450739bf73 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Fri, 12 Jun 2020 13:56:32 +0200 Subject: [PATCH 22/33] Recompile Google Protobuf files --- betterproto/lib/google/{protobuf.py => protobuf/__init__.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename betterproto/lib/google/{protobuf.py => protobuf/__init__.py} (99%) diff --git a/betterproto/lib/google/protobuf.py b/betterproto/lib/google/protobuf/__init__.py similarity index 99% rename from betterproto/lib/google/protobuf.py rename to betterproto/lib/google/protobuf/__init__.py index fd379d5..936d175 100644 --- a/betterproto/lib/google/protobuf.py +++ b/betterproto/lib/google/protobuf/__init__.py @@ -84,7 +84,7 @@ class FieldOptionsCType(betterproto.Enum): STRING_PIECE = 2 -class FieldOptionsJSType(betterproto.Enum): +class FieldOptionsJsType(betterproto.Enum): JS_NORMAL = 0 JS_STRING = 1 JS_NUMBER = 2 @@ -717,7 +717,7 @@ class FieldOptions(betterproto.Message): # use the JavaScript "number" type. The behavior of the default option # JS_NORMAL is implementation dependent. This option is an enum to permit # additional types to be added, e.g. goog.math.Integer. - jstype: "FieldOptionsJSType" = betterproto.enum_field(6) + jstype: "FieldOptionsJsType" = betterproto.enum_field(6) # Should this field be parsed lazily? Lazy applies only to message-type # fields. It means that when the outer message is initially parsed, the # inner message's contents will not be parsed but instead stored in encoded From 2c360a55f2878b09a3e0bed190a4fa4ca82324ff Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Sun, 14 Jun 2020 16:51:52 +0200 Subject: [PATCH 23/33] Readability for generating init_files --- betterproto/plugin.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/betterproto/plugin.py b/betterproto/plugin.py index 33890af..aeef919 100755 --- a/betterproto/plugin.py +++ b/betterproto/plugin.py @@ -359,8 +359,14 @@ def generate_code(request, response): # Make each output directory a package with __init__ file output_paths = set(pathlib.Path(path) for path in output_map.keys()) - output_dirs = [directory for path in output_paths for directory in path.parents] - init_files = set(d.joinpath("__init__.py") for d in output_dirs) - output_paths + init_files = ( + set( + directory.joinpath("__init__.py") + for path in output_paths + for directory in path.parents + ) + - output_paths + ) for init_file in init_files: init = response.file.add() From 87f4b34930f7f1d2396913ab3762153eaf8bfe6f Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Sun, 14 Jun 2020 16:52:33 +0200 Subject: [PATCH 24/33] Revert "Support running plugin without installing betterproto" This reverts commit c88edfd0 --- betterproto/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/betterproto/plugin.py b/betterproto/plugin.py index aeef919..8065c13 100755 --- a/betterproto/plugin.py +++ b/betterproto/plugin.py @@ -1,4 +1,5 @@ #!/usr/bin/env python + import itertools import os.path import pathlib @@ -7,7 +8,6 @@ import sys import textwrap from typing import List -sys.path.append(str(pathlib.Path(__file__).parent.parent)) # add betterproto to path import betterproto from betterproto.compile.importing import get_ref_type from betterproto.compile.naming import ( From 63f5191f023ec26d4abc5575e11dcc54064acc6c Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Sun, 14 Jun 2020 16:54:34 +0200 Subject: [PATCH 25/33] Shorten list selectors --- betterproto/compile/importing.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index 69cca91..6890b35 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -70,10 +70,10 @@ def get_ref_type( if py_package == current_package: return import_sibling(py_type) - if py_package[0 : len(current_package)] == current_package: + if py_package[: len(current_package)] == current_package: return import_descendent(current_package, imports, py_package, py_type) - if current_package[0 : len(py_package)] == py_package: + if current_package[: len(py_package)] == py_package: return import_ancestor(current_package, imports, py_package, py_type) return import_cousin(current_package, imports, py_package, py_type) @@ -109,7 +109,7 @@ def import_descendent(current_package, imports, py_package, py_type): name = foo.bar.Baz """ importing_descendent = py_package[len(current_package) :] - string_from = ".".join(importing_descendent[0:-1]) + string_from = ".".join(importing_descendent[:-1]) string_import = importing_descendent[-1] if string_from: string_alias = "_".join(importing_descendent) From e2d672a422682cb631bfe772f5c51d0d98b02126 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Sun, 14 Jun 2020 21:40:12 +0200 Subject: [PATCH 26/33] Fix terminology, improve docstrings and add missing asserts to tests --- betterproto/compile/importing.py | 66 +++++++-------- betterproto/plugin.py | 10 +-- betterproto/tests/test_get_ref_type.py | 112 ++++++++++++++----------- 3 files changed, 94 insertions(+), 94 deletions(-) diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index 6890b35..7fbf317 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -1,6 +1,6 @@ import os import re -from typing import Dict, List, Type +from typing import Dict, List, Set, Type from betterproto import safe_snake_case from betterproto.compile.naming import pythonize_class_name @@ -35,7 +35,7 @@ def parse_source_type_name(field_type_name): return package, name -def get_ref_type( +def get_type_reference( package: str, imports: set, source_type: str, unwrap: bool = True, ) -> str: """ @@ -65,48 +65,43 @@ def get_ref_type( py_package = ["betterproto", "lib"] + py_package if py_package[:1] == ["betterproto"]: - return import_root(imports, py_package, py_type) + return reference_absolute(imports, py_package, py_type) if py_package == current_package: - return import_sibling(py_type) + return reference_sibling(py_type) if py_package[: len(current_package)] == current_package: - return import_descendent(current_package, imports, py_package, py_type) + return reference_descendent(current_package, imports, py_package, py_type) if current_package[: len(py_package)] == py_package: - return import_ancestor(current_package, imports, py_package, py_type) + return reference_ancestor(current_package, imports, py_package, py_type) - return import_cousin(current_package, imports, py_package, py_type) + return reference_cousin(current_package, imports, py_package, py_type) -def import_root(imports, py_package, py_type): +def reference_absolute(imports, py_package, py_type): + """ + Returns a reference to a python type located in the root, i.e. sys.path. + """ string_import = ".".join(py_package) string_alias = safe_snake_case(string_import) imports.add(f"import {string_import} as {string_alias}") return f"{string_alias}.{py_type}" -def import_sibling(py_type): +def reference_sibling(py_type: str) -> str: """ - package = - name = Foo - - package = foo - name = foo.Bar - - package = foo.bar - name = foo.bar.Baz + Returns a reference to a python type within the same package as the current package. """ return f'"{py_type}"' -def import_descendent(current_package, imports, py_package, py_type): +def reference_descendent( + current_package: List[str], imports: Set[str], py_package: List[str], py_type: str +) -> str: """ - package = - name = foo.Bar - - package = - name = foo.bar.Baz + Returns a reference to a python type in a package that is a descendent of the current package, + and adds the required import that is aliased to avoid name conflicts. """ importing_descendent = py_package[len(current_package) :] string_from = ".".join(importing_descendent[:-1]) @@ -120,16 +115,12 @@ def import_descendent(current_package, imports, py_package, py_type): return f"{string_import}.{py_type}" -def import_ancestor(current_package, imports, py_package, py_type): +def reference_ancestor( + current_package: List[str], imports: Set[str], py_package: List[str], py_type: str +) -> str: """ - package = foo.bar - name = foo.Foo - - package = foo - name = Bar - - package = foo.bar.baz - name = Bar + Returns a reference to a python type in a package which is an ancestor to the current package, + and adds the required import that is aliased (if possible) to avoid name conflicts. """ distance_up = len(current_package) - len(py_package) if py_package: @@ -144,13 +135,12 @@ def import_ancestor(current_package, imports, py_package, py_type): return py_type -def import_cousin(current_package, imports, py_package, py_type): +def reference_cousin( + current_package: List[str], imports: Set[str], py_package: List[str], py_type: str +) -> str: """ - package = foo.bar - name = baz.Foo - - package = foo.bar.baz - name = foo.example.Bar + Returns a reference to a python type in a package that is not descendent, ancestor or sibling, + and adds the required import that is aliased to avoid name conflicts. """ shared_ancestry = os.path.commonprefix([current_package, py_package]) distance_up = len(current_package) - len(shared_ancestry) diff --git a/betterproto/plugin.py b/betterproto/plugin.py index 8065c13..384e120 100755 --- a/betterproto/plugin.py +++ b/betterproto/plugin.py @@ -9,7 +9,7 @@ import textwrap from typing import List import betterproto -from betterproto.compile.importing import get_ref_type +from betterproto.compile.importing import get_type_reference from betterproto.compile.naming import ( pythonize_class_name, pythonize_field_name, @@ -51,7 +51,7 @@ def py_type(package: str, imports: set, field: FieldDescriptorProto) -> str: return "str" elif field.type in [11, 14]: # Type referencing another defined Message or a named enum - return get_ref_type(package, imports, field.type_name) + return get_type_reference(package, imports, field.type_name) elif field.type == 12: return "bytes" else: @@ -306,7 +306,7 @@ def generate_code(request, response): raise NotImplementedError("Client streaming not yet supported") input_message = None - input_type = get_ref_type( + input_type = get_type_reference( package, output["imports"], method.input_type ).strip('"') for msg in output["messages"]: @@ -323,11 +323,11 @@ def generate_code(request, response): "py_name": pythonize_method_name(method.name), "comment": get_comment(proto_file, [6, i, 2, j], indent=8), "route": f"/{package}.{service.name}/{method.name}", - "input": get_ref_type( + "input": get_type_reference( package, output["imports"], method.input_type ).strip('"'), "input_message": input_message, - "output": get_ref_type( + "output": get_type_reference( package, output["imports"], method.output_type, diff --git a/betterproto/tests/test_get_ref_type.py b/betterproto/tests/test_get_ref_type.py index 412b4ed..5cb5f74 100644 --- a/betterproto/tests/test_get_ref_type.py +++ b/betterproto/tests/test_get_ref_type.py @@ -1,6 +1,6 @@ import pytest -from ..compile.importing import get_ref_type, parse_source_type_name +from ..compile.importing import get_type_reference, parse_source_type_name @pytest.mark.parametrize( @@ -28,11 +28,11 @@ from ..compile.importing import get_ref_type, parse_source_type_name ), ], ) -def test_import_google_wellknown_types_non_wrappers( +def test_reference_google_wellknown_types_non_wrappers( google_type: str, expected_name: str, expected_import: str ): imports = set() - name = get_ref_type(package="", imports=imports, source_type=google_type) + name = get_type_reference(package="", imports=imports, source_type=google_type) assert name == expected_name assert imports.__contains__( @@ -54,9 +54,11 @@ def test_import_google_wellknown_types_non_wrappers( (".google.protobuf.BytesValue", "Optional[bytes]"), ], ) -def test_importing_google_wrappers_unwraps_them(google_type: str, expected_name: str): +def test_referenceing_google_wrappers_unwraps_them( + google_type: str, expected_name: str +): imports = set() - name = get_ref_type(package="", imports=imports, source_type=google_type) + name = get_type_reference(package="", imports=imports, source_type=google_type) assert name == expected_name assert imports == set() @@ -76,19 +78,19 @@ def test_importing_google_wrappers_unwraps_them(google_type: str, expected_name: (".google.protobuf.BytesValue", "betterproto_lib_google_protobuf.BytesValue"), ], ) -def test_importing_google_wrappers_without_unwrapping( +def test_referenceing_google_wrappers_without_unwrapping( google_type: str, expected_name: str ): - name = get_ref_type( + name = get_type_reference( package="", imports=set(), source_type=google_type, unwrap=False ) assert name == expected_name -def test_import_child_package_from_package(): +def test_reference_child_package_from_package(): imports = set() - name = get_ref_type( + name = get_type_reference( package="package", imports=imports, source_type="package.child.Message" ) @@ -96,17 +98,17 @@ def test_import_child_package_from_package(): assert name == "child.Message" -def test_import_child_package_from_root(): +def test_reference_child_package_from_root(): imports = set() - name = get_ref_type(package="", imports=imports, source_type="child.Message") + name = get_type_reference(package="", imports=imports, source_type="child.Message") assert imports == {"from . import child"} assert name == "child.Message" -def test_import_camel_cased(): +def test_reference_camel_cased(): imports = set() - name = get_ref_type( + name = get_type_reference( package="", imports=imports, source_type="child_package.example_message" ) @@ -114,17 +116,19 @@ def test_import_camel_cased(): assert name == "child_package.ExampleMessage" -def test_import_nested_child_from_root(): +def test_reference_nested_child_from_root(): imports = set() - name = get_ref_type(package="", imports=imports, source_type="nested.child.Message") + name = get_type_reference( + package="", imports=imports, source_type="nested.child.Message" + ) assert imports == {"from .nested import child as nested_child"} assert name == "nested_child.Message" -def test_import_deeply_nested_child_from_root(): +def test_reference_deeply_nested_child_from_root(): imports = set() - name = get_ref_type( + name = get_type_reference( package="", imports=imports, source_type="deeply.nested.child.Message" ) @@ -132,9 +136,9 @@ def test_import_deeply_nested_child_from_root(): assert name == "deeply_nested_child.Message" -def test_import_deeply_nested_child_from_package(): +def test_reference_deeply_nested_child_from_package(): imports = set() - name = get_ref_type( + name = get_type_reference( package="package", imports=imports, source_type="package.deeply.nested.child.Message", @@ -144,33 +148,35 @@ def test_import_deeply_nested_child_from_package(): assert name == "deeply_nested_child.Message" -def test_import_root_sibling(): +def test_reference_root_sibling(): imports = set() - name = get_ref_type(package="", imports=imports, source_type="Message") + name = get_type_reference(package="", imports=imports, source_type="Message") assert imports == set() assert name == '"Message"' -def test_import_nested_siblings(): +def test_reference_nested_siblings(): imports = set() - name = get_ref_type(package="foo", imports=imports, source_type="foo.Message") + name = get_type_reference(package="foo", imports=imports, source_type="foo.Message") + assert imports == set() assert name == '"Message"' -def test_import_deeply_nested_siblings(): +def test_reference_deeply_nested_siblings(): imports = set() - name = get_ref_type( + name = get_type_reference( package="foo.bar", imports=imports, source_type="foo.bar.Message" ) + assert imports == set() assert name == '"Message"' -def test_import_parent_package_from_child(): +def test_reference_parent_package_from_child(): imports = set() - name = get_ref_type( + name = get_type_reference( package="package.child", imports=imports, source_type="package.Message" ) @@ -178,9 +184,9 @@ def test_import_parent_package_from_child(): assert name == "__package__.Message" -def test_import_parent_package_from_deeply_nested_child(): +def test_reference_parent_package_from_deeply_nested_child(): imports = set() - name = get_ref_type( + name = get_type_reference( package="package.deeply.nested.child", imports=imports, source_type="package.deeply.nested.Message", @@ -190,9 +196,9 @@ def test_import_parent_package_from_deeply_nested_child(): assert name == "__nested__.Message" -def test_import_ancestor_package_from_nested_child(): +def test_reference_ancestor_package_from_nested_child(): imports = set() - name = get_ref_type( + name = get_type_reference( package="package.ancestor.nested.child", imports=imports, source_type="package.ancestor.Message", @@ -202,17 +208,19 @@ def test_import_ancestor_package_from_nested_child(): assert name == "___ancestor__.Message" -def test_import_root_package_from_child(): +def test_reference_root_package_from_child(): imports = set() - name = get_ref_type(package="package.child", imports=imports, source_type="Message") + name = get_type_reference( + package="package.child", imports=imports, source_type="Message" + ) assert imports == {"from ... import Message"} assert name == "Message" -def test_import_root_package_from_deeply_nested_child(): +def test_reference_root_package_from_deeply_nested_child(): imports = set() - name = get_ref_type( + name = get_type_reference( package="package.deeply.nested.child", imports=imports, source_type="Message" ) @@ -220,25 +228,25 @@ def test_import_root_package_from_deeply_nested_child(): assert name == "Message" -def test_import_unrelated_package(): +def test_reference_unrelated_package(): imports = set() - name = get_ref_type(package="a", imports=imports, source_type="p.Message") + name = get_type_reference(package="a", imports=imports, source_type="p.Message") assert imports == {"from .. import p as _p__"} assert name == "_p__.Message" -def test_import_unrelated_nested_package(): +def test_reference_unrelated_nested_package(): imports = set() - name = get_ref_type(package="a.b", imports=imports, source_type="p.q.Message") + name = get_type_reference(package="a.b", imports=imports, source_type="p.q.Message") assert imports == {"from ...p import q as __p_q__"} assert name == "__p_q__.Message" -def test_import_unrelated_deeply_nested_package(): +def test_reference_unrelated_deeply_nested_package(): imports = set() - name = get_ref_type( + name = get_type_reference( package="a.b.c.d", imports=imports, source_type="p.q.r.s.Message" ) @@ -246,17 +254,17 @@ def test_import_unrelated_deeply_nested_package(): assert name == "____p_q_r_s__.Message" -def test_import_cousin_package(): +def test_reference_cousin_package(): imports = set() - name = get_ref_type(package="a.x", imports=imports, source_type="a.y.Message") + name = get_type_reference(package="a.x", imports=imports, source_type="a.y.Message") assert imports == {"from .. import y as _y__"} assert name == "_y__.Message" -def test_import_cousin_package_different_name(): +def test_reference_cousin_package_different_name(): imports = set() - name = get_ref_type( + name = get_type_reference( package="test.package1", imports=imports, source_type="cousin.package2.Message" ) @@ -264,9 +272,9 @@ def test_import_cousin_package_different_name(): assert name == "__cousin_package2__.Message" -def test_import_cousin_package_same_name(): +def test_reference_cousin_package_same_name(): imports = set() - name = get_ref_type( + name = get_type_reference( package="test.package", imports=imports, source_type="cousin.package.Message" ) @@ -274,17 +282,19 @@ def test_import_cousin_package_same_name(): assert name == "__cousin_package__.Message" -def test_import_far_cousin_package(): +def test_reference_far_cousin_package(): imports = set() - name = get_ref_type(package="a.x.y", imports=imports, source_type="a.b.c.Message") + name = get_type_reference( + package="a.x.y", imports=imports, source_type="a.b.c.Message" + ) assert imports == {"from ...b import c as __b_c__"} assert name == "__b_c__.Message" -def test_import_far_far_cousin_package(): +def test_reference_far_far_cousin_package(): imports = set() - name = get_ref_type( + name = get_type_reference( package="a.x.y.z", imports=imports, source_type="a.b.c.d.Message" ) From fdbe0205f19dac00ebac04ec95958446ce107f4d Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Sun, 14 Jun 2020 22:54:03 +0200 Subject: [PATCH 27/33] find_module docstring and search for init files instead of directories --- betterproto/tests/test_inputs.py | 2 +- betterproto/tests/util.py | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/betterproto/tests/test_inputs.py b/betterproto/tests/test_inputs.py index ede31d6..13f20d1 100644 --- a/betterproto/tests/test_inputs.py +++ b/betterproto/tests/test_inputs.py @@ -91,7 +91,7 @@ def test_data(request): if not plugin_module_entry_point: raise Exception( f"Test case {repr(test_case_name)} has no entry point. " - + "Please add a proto message or service called Test and recompile." + "Please add a proto message or service called Test and recompile." ) yield ( diff --git a/betterproto/tests/util.py b/betterproto/tests/util.py index 40b10c6..4c5889d 100644 --- a/betterproto/tests/util.py +++ b/betterproto/tests/util.py @@ -68,13 +68,23 @@ def get_test_case_json_data(test_case_name, json_file_name=None): def find_module( module: ModuleType, predicate: Callable[[ModuleType], bool] ) -> Optional[ModuleType]: + """ + Recursively search module tree for a module that matches the search predicate. + Assumes that the submodules are directories containing __init__.py. + + Example: + + # find module inside foo that contains Test + import foo + test_module = find_module(foo, lambda m: hasattr(m, 'Test')) + """ if predicate(module): return module module_path = pathlib.Path(*module.__path__) - for sub in list(module_path.glob("**/")): - if not sub.is_dir() or sub == module_path: + for sub in list(sub.parent for sub in module_path.glob("**/__init__.py")): + if sub == module_path: continue sub_module_path = sub.relative_to(module_path) sub_module_name = ".".join(sub_module_path.parts) From 52eea5ce4c8c8fe5e4917aa0f65af07bf4f89ef8 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Sun, 14 Jun 2020 23:15:56 +0200 Subject: [PATCH 28/33] Added missing tests for casing --- betterproto/tests/test_casing.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/betterproto/tests/test_casing.py b/betterproto/tests/test_casing.py index f777e2c..ac18309 100644 --- a/betterproto/tests/test_casing.py +++ b/betterproto/tests/test_casing.py @@ -9,6 +9,7 @@ from betterproto.casing import camel_case, pascal_case, snake_case ("", ""), ("a", "A"), ("foobar", "Foobar"), + ("fooBar", "FooBar"), ("FooBar", "FooBar"), ("foo.bar", "FooBar"), ("foo_bar", "FooBar"), @@ -38,6 +39,7 @@ def test_pascal_case(value, expected): ("", ""), ("a", "a"), ("foobar", "foobar"), + ("fooBar", "fooBar"), ("FooBar", "fooBar"), ("foo.bar", "fooBar"), ("foo_bar", "fooBar"), @@ -65,6 +67,7 @@ def test_camel_case(value, expected): ("", ""), ("a", "a"), ("foobar", "foobar"), + ("fooBar", "foo_bar"), ("FooBar", "foo_bar"), ("foo.bar", "foo_bar"), ("foo_bar", "foo_bar"), From e3135ce766e84db90d015ebcf32ebb4ad166f8c4 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Wed, 1 Jul 2020 09:39:37 +0200 Subject: [PATCH 29/33] Add parameter for non-strict cased output that preserves delimiter count --- betterproto/casing.py | 49 +++++++++++++++++++++++++++----- betterproto/tests/test_casing.py | 41 +++++++++++++++++++++++--- 2 files changed, 79 insertions(+), 11 deletions(-) diff --git a/betterproto/casing.py b/betterproto/casing.py index 01e10bb..60ece6a 100644 --- a/betterproto/casing.py +++ b/betterproto/casing.py @@ -53,30 +53,65 @@ def safe_snake_case(value: str) -> str: return value -def snake_case(value: str): +def snake_case(value: str, strict: bool = True): """ Join words with an underscore into lowercase and remove symbols. + @param value: value to convert + @param strict: force single underscores """ + + def substitute_word(symbols, word, is_start): + if not word: + return "" + if strict: + delimiter_count = 0 if is_start else 1 # Single underscore if strict. + elif is_start: + delimiter_count = len(symbols) + elif word.isupper() or word.islower(): + delimiter_count = max(1, len(symbols)) # Preserve all delimiters if not strict. + else: + delimiter_count = len(symbols) + 1 # Extra underscore for leading capital. + + return ("_" * delimiter_count) + word.lower() + snake = re.sub( - f"{SYMBOLS}({WORD_UPPER}|{WORD})", lambda groups: "_" + groups[1].lower(), value + f"(^)?({SYMBOLS})({WORD_UPPER}|{WORD})", + lambda groups: substitute_word(groups[2], groups[3], groups[1] is not None), + value, ) - return snake.strip("_") + return snake -def pascal_case(value: str): +def pascal_case(value: str, strict: bool = True): """ Capitalize each word and remove symbols. + @param value: value to convert + @param strict: output only alphanumeric characters """ + + def substitute_word(symbols, word): + if strict: + return word.capitalize() # Remove all delimiters + + if word.islower(): + delimiter_length = len(symbols[:-1]) # Lose one delimiter + else: + delimiter_length = len(symbols) # Preserve all delimiters + + return ("_" * delimiter_length) + word.capitalize() + return re.sub( - f"{SYMBOLS}({WORD_UPPER}|{WORD})", lambda groups: groups[1].capitalize(), value + f"({SYMBOLS})({WORD_UPPER}|{WORD})", + lambda groups: substitute_word(groups[1], groups[2]), + value, ) -def camel_case(value: str): +def camel_case(value: str, strict: bool = True): """ Capitalize all words except first and remove symbols. """ - return lowercase_first(pascal_case(value)) + return lowercase_first(pascal_case(value, strict=strict)) def lowercase_first(value: str): diff --git a/betterproto/tests/test_casing.py b/betterproto/tests/test_casing.py index ac18309..ec60483 100644 --- a/betterproto/tests/test_casing.py +++ b/betterproto/tests/test_casing.py @@ -29,7 +29,7 @@ from betterproto.casing import camel_case, pascal_case, snake_case ], ) def test_pascal_case(value, expected): - actual = pascal_case(value) + actual = pascal_case(value, strict=True) assert actual == expected, f"{value} => {expected} (actual: {actual})" @@ -56,8 +56,22 @@ def test_pascal_case(value, expected): ("1foobar", "1Foobar"), ], ) -def test_camel_case(value, expected): - actual = camel_case(value) +def test_camel_case_strict(value, expected): + actual = camel_case(value, strict=True) + assert actual == expected, f"{value} => {expected} (actual: {actual})" + + +@pytest.mark.parametrize( + ["value", "expected"], + [ + ("foo_bar", "fooBar"), + ("FooBar", "fooBar"), + ("foo__bar", "foo_Bar"), + ("foo__Bar", "foo__Bar"), + ], +) +def test_camel_case_not_strict(value, expected): + actual = camel_case(value, strict=False) assert actual == expected, f"{value} => {expected} (actual: {actual})" @@ -71,6 +85,7 @@ def test_camel_case(value, expected): ("FooBar", "foo_bar"), ("foo.bar", "foo_bar"), ("foo_bar", "foo_bar"), + ("foo_Bar", "foo_bar"), ("FOOBAR", "foobar"), ("FOOBar", "foo_bar"), ("UInt32", "u_int32"), @@ -85,8 +100,26 @@ def test_camel_case(value, expected): ("foo~bar", "foo_bar"), ("foo:bar", "foo_bar"), ("1foobar", "1_foobar"), + ("GetUInt64", "get_u_int64"), ], ) -def test_snake_case(value, expected): +def test_snake_case_strict(value, expected): actual = snake_case(value) assert actual == expected, f"{value} => {expected} (actual: {actual})" + + +@pytest.mark.parametrize( + ["value", "expected"], + [ + ("fooBar", "foo_bar"), + ("FooBar", "foo_bar"), + ("foo_Bar", "foo__bar"), + ("foo__bar", "foo__bar"), + ("FOOBar", "foo_bar"), + ("__foo", "__foo"), + ("GetUInt64", "get_u_int64"), + ], +) +def test_snake_case_not_strict(value, expected): + actual = snake_case(value, strict=False) + assert actual == expected, f"{value} => {expected} (actual: {actual})" From 81711d2427eb100ab34e4c413ef6ee6eefa1bf43 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Wed, 1 Jul 2020 12:07:59 +0200 Subject: [PATCH 30/33] Avoid naming conflicts when importing multiple types with the same name from an ancestor package --- betterproto/compile/importing.py | 8 +++++--- betterproto/tests/test_get_ref_type.py | 8 ++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/betterproto/compile/importing.py b/betterproto/compile/importing.py index 7fbf317..40441f8 100644 --- a/betterproto/compile/importing.py +++ b/betterproto/compile/importing.py @@ -121,18 +121,20 @@ def reference_ancestor( """ Returns a reference to a python type in a package which is an ancestor to the current package, and adds the required import that is aliased (if possible) to avoid name conflicts. + + Adds trailing __ to avoid name mangling (python.org/dev/peps/pep-0008/#id34). """ distance_up = len(current_package) - len(py_package) if py_package: string_import = py_package[-1] - # Add trailing __ to avoid name mangling (python.org/dev/peps/pep-0008/#id34) string_alias = f"_{'_' * distance_up}{string_import}__" string_from = f"..{'.' * distance_up}" imports.add(f"from {string_from} import {string_import} as {string_alias}") return f"{string_alias}.{py_type}" else: - imports.add(f"from .{'.' * distance_up} import {py_type}") - return py_type + string_alias = f"{'_' * distance_up}{py_type}__" + imports.add(f"from .{'.' * distance_up} import {py_type} as {string_alias}") + return string_alias def reference_cousin( diff --git a/betterproto/tests/test_get_ref_type.py b/betterproto/tests/test_get_ref_type.py index 5cb5f74..2bedf76 100644 --- a/betterproto/tests/test_get_ref_type.py +++ b/betterproto/tests/test_get_ref_type.py @@ -214,8 +214,8 @@ def test_reference_root_package_from_child(): package="package.child", imports=imports, source_type="Message" ) - assert imports == {"from ... import Message"} - assert name == "Message" + assert imports == {"from ... import Message as __Message__"} + assert name == "__Message__" def test_reference_root_package_from_deeply_nested_child(): @@ -224,8 +224,8 @@ def test_reference_root_package_from_deeply_nested_child(): package="package.deeply.nested.child", imports=imports, source_type="Message" ) - assert imports == {"from ..... import Message"} - assert name == "Message" + assert imports == {"from ..... import Message as ____Message__"} + assert name == "____Message__" def test_reference_unrelated_package(): From 0d9387abecba88f4d4131addb23a23112049a7ff Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Wed, 1 Jul 2020 12:43:12 +0200 Subject: [PATCH 31/33] Remove stringcase dependency --- poetry.lock | 21 ++++++--------------- pyproject.toml | 1 - 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/poetry.lock b/poetry.lock index 6afb23f..3434f55 100644 --- a/poetry.lock +++ b/poetry.lock @@ -271,7 +271,7 @@ docs = ["sphinx", "rst.linker", "jaraco.packaging"] category = "main" description = "A very fast and expressive template engine." name = "jinja2" -optional = true +optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*" version = "2.11.2" @@ -285,7 +285,7 @@ i18n = ["Babel (>=0.8)"] category = "main" description = "Safely add untrusted strings to HTML/XML markup." name = "markupsafe" -optional = true +optional = false python-versions = ">=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*" version = "1.1.1" @@ -369,7 +369,7 @@ dev = ["pre-commit", "tox"] category = "main" description = "Protocol Buffers" name = "protobuf" -optional = true +optional = false python-versions = "*" version = "3.12.2" @@ -490,14 +490,6 @@ optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*" version = "1.15.0" -[[package]] -category = "main" -description = "String case converter." -name = "stringcase" -optional = false -python-versions = "*" -version = "1.2.0" - [[package]] category = "main" description = "Python Library for Tom's Obvious, Minimal Language" @@ -612,7 +604,7 @@ testing = ["jaraco.itertools", "func-timeout"] compiler = ["black", "jinja2", "protobuf"] [metadata] -content-hash = "ecafcaed2d4a25c2829e6dc3ef3c56cd72a8bc28c25c7aeae3484c978c816722" +content-hash = "8a4fa01ede86e1b5ba35b9dab8b6eacee766a9b5666f48ab41445c01882ab003" python-versions = "^3.6" [metadata.files] @@ -865,6 +857,8 @@ protobuf = [ {file = "protobuf-3.12.2-cp37-cp37m-win_amd64.whl", hash = "sha256:e72736dd822748b0721f41f9aaaf6a5b6d5cfc78f6c8690263aef8bba4457f0e"}, {file = "protobuf-3.12.2-cp38-cp38-macosx_10_9_x86_64.whl", hash = "sha256:87535dc2d2ef007b9d44e309d2b8ea27a03d2fa09556a72364d706fcb7090828"}, {file = "protobuf-3.12.2-cp38-cp38-manylinux1_x86_64.whl", hash = "sha256:50b5fee674878b14baea73b4568dc478c46a31dd50157a5b5d2f71138243b1a9"}, + {file = "protobuf-3.12.2-py2.py3-none-any.whl", hash = "sha256:a96f8fc625e9ff568838e556f6f6ae8eca8b4837cdfb3f90efcb7c00e342a2eb"}, + {file = "protobuf-3.12.2.tar.gz", hash = "sha256:49ef8ab4c27812a89a76fa894fe7a08f42f2147078392c0dee51d4a444ef6df5"}, ] py = [ {file = "py-1.8.2-py2.py3-none-any.whl", hash = "sha256:a673fa23d7000440cc885c17dbd34fafcb7d7a6e230b29f6766400de36a33c44"}, @@ -920,9 +914,6 @@ six = [ {file = "six-1.15.0-py2.py3-none-any.whl", hash = "sha256:8b74bedcbbbaca38ff6d7491d76f2b06b3592611af620f8426e82dddb04a5ced"}, {file = "six-1.15.0.tar.gz", hash = "sha256:30639c035cdb23534cd4aa2dd52c3bf48f06e5f4a941509c8bafd8ce11080259"}, ] -stringcase = [ - {file = "stringcase-1.2.0.tar.gz", hash = "sha256:48a06980661908efe8d9d34eab2b6c13aefa2163b3ced26972902e3bdfd87008"}, -] toml = [ {file = "toml-0.10.1-py2.py3-none-any.whl", hash = "sha256:bda89d5935c2eac546d648028b9901107a595863cb36bae0c73ac804a9b4ce88"}, {file = "toml-0.10.1.tar.gz", hash = "sha256:926b612be1e5ce0634a2ca03470f95169cf16f939018233a670519cb4ac58b0f"}, diff --git a/pyproject.toml b/pyproject.toml index f4466d9..f1e966b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,6 @@ dataclasses = { version = "^0.7", python = ">=3.6, <3.7" } grpclib = "^0.3.1" jinja2 = { version = "^2.11.2", optional = true } protobuf = { version = "^3.12.2", optional = true } -stringcase = "^1.2.0" [tool.poetry.dev-dependencies] black = "^19.10b0" From af7115429a3e29b54ddf3f5c1df22e41c5fd5ef0 Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Wed, 1 Jul 2020 12:43:28 +0200 Subject: [PATCH 32/33] Expose betterproto.ServiceStub --- betterproto/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/betterproto/__init__.py b/betterproto/__init__.py index fe60bd1..6c07feb 100644 --- a/betterproto/__init__.py +++ b/betterproto/__init__.py @@ -23,6 +23,7 @@ from typing import ( from ._types import T from .casing import camel_case, safe_snake_case, safe_snake_case, snake_case +from .grpc.grpclib_client import ServiceStub if not (sys.version_info.major == 3 and sys.version_info.minor >= 7): # Apply backport of datetime.fromisoformat from 3.7 From d21cd6e391f38a365181d958f9aa74d3ae422e3b Mon Sep 17 00:00:00 2001 From: boukeversteegh Date: Wed, 1 Jul 2020 13:15:03 +0200 Subject: [PATCH 33/33] black --- betterproto/casing.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/betterproto/casing.py b/betterproto/casing.py index 60ece6a..543df68 100644 --- a/betterproto/casing.py +++ b/betterproto/casing.py @@ -68,7 +68,9 @@ def snake_case(value: str, strict: bool = True): elif is_start: delimiter_count = len(symbols) elif word.isupper() or word.islower(): - delimiter_count = max(1, len(symbols)) # Preserve all delimiters if not strict. + delimiter_count = max( + 1, len(symbols) + ) # Preserve all delimiters if not strict. else: delimiter_count = len(symbols) + 1 # Extra underscore for leading capital.