Skip to content

Import bug - Circular Dependencies #100

New issue

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

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

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions betterproto/compile/importing.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def reference_absolute(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}"
return f'"{string_alias}.{py_type}"'


def reference_sibling(py_type: str) -> str:
Expand All @@ -109,10 +109,10 @@ def reference_descendent(
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}"
return f'"{string_alias}.{py_type}"'
else:
imports.add(f"from . import {string_import}")
return f"{string_import}.{py_type}"
return f'"{string_import}.{py_type}"'


def reference_ancestor(
Expand All @@ -130,11 +130,11 @@ def reference_ancestor(
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}"
return f'"{string_alias}.{py_type}"'
else:
string_alias = f"{'_' * distance_up}{py_type}__"
imports.add(f"from .{'.' * distance_up} import {py_type} as {string_alias}")
return string_alias
return f'"{string_alias}"'


def reference_cousin(
Expand All @@ -157,4 +157,4 @@ def reference_cousin(
+ "__"
)
imports.add(f"from {string_from} import {string_import} as {string_alias}")
return f"{string_alias}.{py_type}"
return f'"{string_alias}.{py_type}"'
2 changes: 1 addition & 1 deletion betterproto/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ def generate_code(request, response):
output["imports"],
method.output_type,
unwrap=False,
).strip('"'),
),
"client_streaming": method.client_streaming,
"server_streaming": method.server_streaming,
}
Expand Down
16 changes: 8 additions & 8 deletions betterproto/templates/template.py.j2
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ import betterproto
import grpclib
{% endif %}

{% for i in description.imports %}
{{ i }}
{% endfor %}


{% if description.enums %}{% for enum in description.enums %}
class {{ enum.py_name }}(betterproto.Enum):
Expand Down Expand Up @@ -102,14 +98,14 @@ class {{ service.py_name }}Stub(betterproto.ServiceStub):
"{{ method.route }}",
request_iterator,
{{ method.input }},
{{ method.output }},
{{ method.output.strip('"') }},
):
yield response
{% else %}{# i.e. not client streaming #}
async for response in self._unary_stream(
"{{ method.route }}",
request,
{{ method.output }},
{{ method.output.strip('"') }},
):
yield response

Expand All @@ -120,16 +116,20 @@ class {{ service.py_name }}Stub(betterproto.ServiceStub):
"{{ method.route }}",
request_iterator,
{{ method.input }},
{{ method.output }}
{{ method.output.strip('"') }}
)
{% else %}{# i.e. not client streaming #}
return await self._unary_unary(
"{{ method.route }}",
request,
{{ method.output }}
{{ method.output.strip('"') }}
)
{% endif %}{# client streaming #}
{% endif %}

{% endfor %}
{% endfor %}

{% for i in description.imports %}
{{ i }}
{% endfor %}
1 change: 0 additions & 1 deletion betterproto/tests/inputs/config.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Test cases that are expected to fail, e.g. unimplemented features or bug-fixes.
# Remove from list when fixed.
xfail = {
"import_circular_dependency",
"oneof_enum", # 63
"namespace_keywords", # 70
"namespace_builtin_types", # 53
Expand Down
76 changes: 44 additions & 32 deletions betterproto/tests/test_get_ref_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,22 @@
[
(
".google.protobuf.Empty",
"betterproto_lib_google_protobuf.Empty",
'"betterproto_lib_google_protobuf.Empty"',
"import betterproto.lib.google.protobuf as betterproto_lib_google_protobuf",
),
(
".google.protobuf.Struct",
"betterproto_lib_google_protobuf.Struct",
'"betterproto_lib_google_protobuf.Struct"',
"import betterproto.lib.google.protobuf as betterproto_lib_google_protobuf",
),
(
".google.protobuf.ListValue",
"betterproto_lib_google_protobuf.ListValue",
'"betterproto_lib_google_protobuf.ListValue"',
"import betterproto.lib.google.protobuf as betterproto_lib_google_protobuf",
),
(
".google.protobuf.Value",
"betterproto_lib_google_protobuf.Value",
'"betterproto_lib_google_protobuf.Value"',
"import betterproto.lib.google.protobuf as betterproto_lib_google_protobuf",
),
],
Expand Down Expand Up @@ -67,15 +67,27 @@ def test_referenceing_google_wrappers_unwraps_them(
@pytest.mark.parametrize(
["google_type", "expected_name"],
[
(".google.protobuf.DoubleValue", "betterproto_lib_google_protobuf.DoubleValue"),
(".google.protobuf.FloatValue", "betterproto_lib_google_protobuf.FloatValue"),
(".google.protobuf.Int32Value", "betterproto_lib_google_protobuf.Int32Value"),
(".google.protobuf.Int64Value", "betterproto_lib_google_protobuf.Int64Value"),
(".google.protobuf.UInt32Value", "betterproto_lib_google_protobuf.UInt32Value"),
(".google.protobuf.UInt64Value", "betterproto_lib_google_protobuf.UInt64Value"),
(".google.protobuf.BoolValue", "betterproto_lib_google_protobuf.BoolValue"),
(".google.protobuf.StringValue", "betterproto_lib_google_protobuf.StringValue"),
(".google.protobuf.BytesValue", "betterproto_lib_google_protobuf.BytesValue"),
(
".google.protobuf.DoubleValue",
'"betterproto_lib_google_protobuf.DoubleValue"',
),
(".google.protobuf.FloatValue", '"betterproto_lib_google_protobuf.FloatValue"'),
(".google.protobuf.Int32Value", '"betterproto_lib_google_protobuf.Int32Value"'),
(".google.protobuf.Int64Value", '"betterproto_lib_google_protobuf.Int64Value"'),
(
".google.protobuf.UInt32Value",
'"betterproto_lib_google_protobuf.UInt32Value"',
),
(
".google.protobuf.UInt64Value",
'"betterproto_lib_google_protobuf.UInt64Value"',
),
(".google.protobuf.BoolValue", '"betterproto_lib_google_protobuf.BoolValue"'),
(
".google.protobuf.StringValue",
'"betterproto_lib_google_protobuf.StringValue"',
),
(".google.protobuf.BytesValue", '"betterproto_lib_google_protobuf.BytesValue"'),
],
)
def test_referenceing_google_wrappers_without_unwrapping(
Expand All @@ -95,15 +107,15 @@ def test_reference_child_package_from_package():
)

assert imports == {"from . import child"}
assert name == "child.Message"
assert name == '"child.Message"'


def test_reference_child_package_from_root():
imports = set()
name = get_type_reference(package="", imports=imports, source_type="child.Message")

assert imports == {"from . import child"}
assert name == "child.Message"
assert name == '"child.Message"'


def test_reference_camel_cased():
Expand All @@ -113,7 +125,7 @@ def test_reference_camel_cased():
)

assert imports == {"from . import child_package"}
assert name == "child_package.ExampleMessage"
assert name == '"child_package.ExampleMessage"'


def test_reference_nested_child_from_root():
Expand All @@ -123,7 +135,7 @@ def test_reference_nested_child_from_root():
)

assert imports == {"from .nested import child as nested_child"}
assert name == "nested_child.Message"
assert name == '"nested_child.Message"'


def test_reference_deeply_nested_child_from_root():
Expand All @@ -133,7 +145,7 @@ def test_reference_deeply_nested_child_from_root():
)

assert imports == {"from .deeply.nested import child as deeply_nested_child"}
assert name == "deeply_nested_child.Message"
assert name == '"deeply_nested_child.Message"'


def test_reference_deeply_nested_child_from_package():
Expand All @@ -145,7 +157,7 @@ def test_reference_deeply_nested_child_from_package():
)

assert imports == {"from .deeply.nested import child as deeply_nested_child"}
assert name == "deeply_nested_child.Message"
assert name == '"deeply_nested_child.Message"'


def test_reference_root_sibling():
Expand Down Expand Up @@ -181,7 +193,7 @@ def test_reference_parent_package_from_child():
)

assert imports == {"from ... import package as __package__"}
assert name == "__package__.Message"
assert name == '"__package__.Message"'


def test_reference_parent_package_from_deeply_nested_child():
Expand All @@ -193,7 +205,7 @@ def test_reference_parent_package_from_deeply_nested_child():
)

assert imports == {"from ... import nested as __nested__"}
assert name == "__nested__.Message"
assert name == '"__nested__.Message"'


def test_reference_ancestor_package_from_nested_child():
Expand All @@ -205,7 +217,7 @@ def test_reference_ancestor_package_from_nested_child():
)

assert imports == {"from .... import ancestor as ___ancestor__"}
assert name == "___ancestor__.Message"
assert name == '"___ancestor__.Message"'


def test_reference_root_package_from_child():
Expand All @@ -215,7 +227,7 @@ def test_reference_root_package_from_child():
)

assert imports == {"from ... import Message as __Message__"}
assert name == "__Message__"
assert name == '"__Message__"'


def test_reference_root_package_from_deeply_nested_child():
Expand All @@ -225,23 +237,23 @@ def test_reference_root_package_from_deeply_nested_child():
)

assert imports == {"from ..... import Message as ____Message__"}
assert name == "____Message__"
assert name == '"____Message__"'


def test_reference_unrelated_package():
imports = set()
name = get_type_reference(package="a", imports=imports, source_type="p.Message")

assert imports == {"from .. import p as _p__"}
assert name == "_p__.Message"
assert name == '"_p__.Message"'


def test_reference_unrelated_nested_package():
imports = set()
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"
assert name == '"__p_q__.Message"'


def test_reference_unrelated_deeply_nested_package():
Expand All @@ -251,15 +263,15 @@ def test_reference_unrelated_deeply_nested_package():
)

assert imports == {"from .....p.q.r import s as ____p_q_r_s__"}
assert name == "____p_q_r_s__.Message"
assert name == '"____p_q_r_s__.Message"'


def test_reference_cousin_package():
imports = set()
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"
assert name == '"_y__.Message"'


def test_reference_cousin_package_different_name():
Expand All @@ -269,7 +281,7 @@ def test_reference_cousin_package_different_name():
)

assert imports == {"from ...cousin import package2 as __cousin_package2__"}
assert name == "__cousin_package2__.Message"
assert name == '"__cousin_package2__.Message"'


def test_reference_cousin_package_same_name():
Expand All @@ -279,7 +291,7 @@ def test_reference_cousin_package_same_name():
)

assert imports == {"from ...cousin import package as __cousin_package__"}
assert name == "__cousin_package__.Message"
assert name == '"__cousin_package__.Message"'


def test_reference_far_cousin_package():
Expand All @@ -289,7 +301,7 @@ def test_reference_far_cousin_package():
)

assert imports == {"from ...b import c as __b_c__"}
assert name == "__b_c__.Message"
assert name == '"__b_c__.Message"'


def test_reference_far_far_cousin_package():
Expand All @@ -299,7 +311,7 @@ def test_reference_far_far_cousin_package():
)

assert imports == {"from ....b.c import d as ___b_c_d__"}
assert name == "___b_c_d__.Message"
assert name == '"___b_c_d__.Message"'


@pytest.mark.parametrize(
Expand Down