Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Invalid code generated when package name contains uppercase letter #562

Open
3 tasks done
paskozdilar opened this issue Mar 12, 2024 · 3 comments · Fixed by betterproto/python-betterproto2#4 · May be fixed by #635
Open
3 tasks done

Invalid code generated when package name contains uppercase letter #562

paskozdilar opened this issue Mar 12, 2024 · 3 comments · Fixed by betterproto/python-betterproto2#4 · May be fixed by #635
Labels
bug Something isn't working investigation needed

Comments

@paskozdilar
Copy link

Summary

Title

Reproduction Steps

This protofile works:

// proto/test.proto
syntax = "proto3";

package p;

service Service {
  rpc Method(Empty) returns (Empty) {}
}

message Empty {}

This doesn't:

// proto/test.proto
syntax = "proto3";

package P;

service Service {
  rpc Method(Empty) returns (Empty) {}
}

message Empty {}

With the latter, betterproto compiles to this:

# Generated by the protocol buffer compiler.  DO NOT EDIT!
# sources: test.proto
# plugin: python-betterproto
# This file has been @generated

from dataclasses import dataclass
from typing import (
    TYPE_CHECKING,
    Dict,
    Optional,
)

import betterproto
import grpclib
from betterproto.grpc.grpclib_server import ServiceBase

from .. import PEmpty as _PEmpty__

# [...]

The from .. import PEmpty is invalid import as nothing exists in the parent package.

Command used:

python3 -m grpc_tools.protoc --proto_path=./proto/ --python_betterproto_out=./proto/ proto/*.proto

Python environment (output of pip freeze):

betterproto @ git+https://github.com/danielgtaylor/python-betterproto@5666393f9d10e13609d8eeac8d1ab3815dce5fd6
black==24.2.0
click==8.1.7
grpcio==1.62.1
grpcio-tools==1.62.1
grpclib==0.4.7
h2==4.1.0
hpack==4.0.0
hyperframe==6.0.1
isort==5.13.2
Jinja2==3.1.3
MarkupSafe==2.1.5
multidict==6.0.5
mypy-extensions==1.0.0
nodeenv==1.8.0
packaging==24.0
pathspec==0.12.1
platformdirs==4.2.0
protobuf==4.25.3
pyright==1.1.353
python-dateutil==2.9.0.post0
six==1.16.0
typing_extensions==4.10.0

Expected Results

Correct code generated

Actual Results

Incorrect code generated

System Information

libprotoc 3.12.4
Python 3.11.0rc1
Name: betterproto
Version: 2.0.0b6
Summary: A better Protobuf / gRPC generator & library
Home-page: https://github.com/danielgtaylor/python-betterproto
Author: Daniel G. Taylor
Author-email: [email protected]
License: MIT
Location:
Requires: grpclib, python-dateutil, typing-extensions
Required-by:

Checklist

  • I have searched the issues for duplicates.
  • I have shown the entire traceback, if possible.
  • I have verified this issue occurs on the latest prelease of betterproto which can be installed using pip install -U --pre betterproto, if possible.
@paskozdilar paskozdilar added bug Something isn't working investigation needed labels Mar 12, 2024
@gbmhunter
Copy link

I have got the same issue. I'm running the generation on Linux with betterproto v2.0.0b6. Adding any uppercase letter to any part of the package name in the .proto causes the same import error, where there are imports that don't exist in the parent __init__.py. Problem goes away if I use all lowercase letters.

@vkorotkovs
Copy link

The problem lies in the regex expression for splitting the source type into a tuple:

def parse_source_type_name(field_type_name: str) -> Tuple[str, str]:
"""
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

However, your package naming should probably follow the style guide, which suggests using all lowercase letters. Adhering to this guideline would prevent this issue from occurring in the first place.

@hf-kklein
Copy link
Contributor

See #437

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working investigation needed
Projects
None yet
4 participants