protocolbuffers / protobuf

Protocol Buffers - Google's data interchange format
http://protobuf.dev
Other
65.92k stars 15.52k forks source link

python: pylint no-name-in-module #10372

Open huwcbjones opened 2 years ago

huwcbjones commented 2 years ago

Since v3.20.0 generated protobuf files trigger pylint no-name-in-module when importing the message types. Compiling foo.proto (see below) on two different protobuf versions obviously different results, however the old (3.19.x) can be pylinted successfully, but the new (3.20.x) result cannot be pylinted.

Compilation command: python3 -m grpc_tools.protoc -I . --python_out=. foo.proto

syntax = "proto3";

package foo;

// Foo service
service Foo {
  // Say Foo
  rpc SayFoo(SayFooRequest) returns (SayFooResponse) {}
}

// Say Foo Request
message SayFooRequest {}

// Say Foo Response
message SayFooResponse {}

libprotoc 3.19.4 output
$ python3 -m grpc_tools.protoc --version
libprotoc 3.19.4
```py # -*- coding: utf-8 -*- # Generated by the protocol buffer compiler. DO NOT EDIT! # source: foo.proto """Generated protocol buffer code.""" from google.protobuf import descriptor as _descriptor from google.protobuf import descriptor_pool as _descriptor_pool from google.protobuf import message as _message from google.protobuf import reflection as _reflection from google.protobuf import symbol_database as _symbol_database # @@protoc_insertion_point(imports) _sym_db = _symbol_database.Default() DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n\tfoo.proto\x12\x03\x66oo\"\x0f\n\rSayFooRequest\"\x10\n\x0eSayFooResponse2:\n\x03\x46oo\x12\x33\n\x06SayFoo\x12\x12.foo.SayFooRequest\x1a\x13.foo.SayFooResponse\"\x00\x62\x06proto3') _SAYFOOREQUEST = DESCRIPTOR.message_types_by_name['SayFooRequest'] _SAYFOORESPONSE = DESCRIPTOR.message_types_by_name['SayFooResponse'] SayFooRequest = _reflection.GeneratedProtocolMessageType('SayFooRequest', (_message.Message,), { 'DESCRIPTOR' : _SAYFOOREQUEST, '__module__' : 'foo_pb2' # @@protoc_insertion_point(class_scope:foo.SayFooRequest) }) _sym_db.RegisterMessage(SayFooRequest) SayFooResponse = _reflection.GeneratedProtocolMessageType('SayFooResponse', (_message.Message,), { 'DESCRIPTOR' : _SAYFOORESPONSE, '__module__' : 'foo_pb2' # @@protoc_insertion_point(class_scope:foo.SayFooResponse) }) _sym_db.RegisterMessage(SayFooResponse) _FOO = DESCRIPTOR.services_by_name['Foo'] if _descriptor._USE_C_DESCRIPTORS == False: DESCRIPTOR._options = None _SAYFOOREQUEST._serialized_start=18 _SAYFOOREQUEST._serialized_end=33 _SAYFOORESPONSE._serialized_start=35 _SAYFOORESPONSE._serialized_end=51 _FOO._serialized_start=53 _FOO._serialized_end=111 # @@protoc_insertion_point(module_scope) ```
libprotoc 3.20.1 output
$ python3 -m grpc_tools.protoc --version
libprotoc 3.20.1
```py # -*- coding: utf-8 -*- # Generated by the protocol buffer compiler. DO NOT EDIT! # source: foo.proto """Generated protocol buffer code.""" from google.protobuf.internal import builder as _builder from google.protobuf import descriptor as _descriptor from google.protobuf import descriptor_pool as _descriptor_pool from google.protobuf import symbol_database as _symbol_database # @@protoc_insertion_point(imports) _sym_db = _symbol_database.Default() DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n\tfoo.proto\x12\x03\x66oo\"\x0f\n\rSayFooRequest\"\x10\n\x0eSayFooResponse2:\n\x03\x46oo\x12\x33\n\x06SayFoo\x12\x12.foo.SayFooRequest\x1a\x13.foo.SayFooResponse\"\x00\x62\x06proto3') _builder.BuildMessageAndEnumDescriptors(DESCRIPTOR, globals()) _builder.BuildTopDescriptorsAndMessages(DESCRIPTOR, 'foo_pb2', globals()) if _descriptor._USE_C_DESCRIPTORS == False: DESCRIPTOR._options = None _SAYFOOREQUEST._serialized_start=18 _SAYFOOREQUEST._serialized_end=33 _SAYFOORESPONSE._serialized_start=35 _SAYFOORESPONSE._serialized_end=51 _FOO._serialized_start=53 _FOO._serialized_end=111 # @@protoc_insertion_point(module_scope) ```

Test file (test.py)

from foo_pb2 import SayFooRequest, SayFooResponse

Running pylint --disable=all --enable=no-name-in-module test.py With 3.19.x generated code

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 0.00/10, +10.00)

With 3.20.x generated code

************* Module test
test.py:1:0: E0611: No name 'SayFooRequest' in module 'foo_pb2' (no-name-in-module)
test.py:1:0: E0611: No name 'SayFooResponse' in module 'foo_pb2' (no-name-in-module)

----------------------------------------------------------------------
Your code has been rated at -90.00/10 (previous run: -90.00/10, +0.00)

Clearly this is due to the abuse of globals() when using the builder to inject the descriptors into the module context. However, this is dynamically generated code, why does it have to be so clever (c.f.: obfuscated) that industry standard tooling can't ensure that your code is integrating with the library code properly?

huwcbjones commented 2 years ago

From a quick git history browse, the change was introduced in this commit of doom ab4585a6956675ce14a1cba5d321fde980bbf12b

haberman commented 2 years ago

I was hoping you would be able to use our .pyi files for this, since protoc is capable of producing full and complete pyi files for the generated interface. Unfortunately it looks like pylint doesn't support this yet: https://github.com/PyCQA/pylint/issues/4987

Perhaps we could change the generated code to explicitly assign the top-level globals. The main thing we are trying to avoid is having the generated code be tightly coupled to the core runtime.

huwcbjones commented 2 years ago

We're making great use of the .pyi's for mypy and we're running both mypy and pylint because they both catch things that the other doesn't. But as you say, pylint doesn't have support for .pyis 😢

we could change the generated code to explicitly assign the top-level globals.

If that's possible, that'd be awesome!

haberman commented 1 year ago

I think this should have been fixed by: https://github.com/protocolbuffers/protobuf/pull/11011

Please let me know if this did not fix it.

haberman commented 1 year ago

Oh wait, I take it back. That PR did not explicitly assign globals, it just made references happen through _globals.

peterdemin commented 1 year ago

What is the current recommendation for projects using pylint and protobuf?

AnthonyDiGirolamo commented 1 year ago

Seems like there is some progress for pyi files in pylint:

Ruoyu-y commented 1 year ago

Any more update on this one?

jenstroeger commented 1 year ago

Six months on, any updates on this issue? It looks like a resolution is blocked by the pylint issues linked above?

haberman commented 1 year ago

Using the proto file given in the original bug report:

syntax = "proto3";

package foo;

// Foo service
service Foo {
  // Say Foo
  rpc SayFoo(SayFooRequest) returns (SayFooResponse) {}
}

// Say Foo Request
message SayFooRequest {}

// Say Foo Response
message SayFooResponse {}

The current codegen output is:

# -*- coding: utf-8 -*-
# Generated by the protocol buffer compiler.  DO NOT EDIT!
# source: protoc_explorer/other2.proto
"""Generated protocol buffer code."""
import google3
from google.protobuf import descriptor as _descriptor
from google.protobuf import descriptor_pool as _descriptor_pool
from google.protobuf import symbol_database as _symbol_database
from google.protobuf.internal import builder as _builder
# @@protoc_insertion_point(imports)

_sym_db = _symbol_database.Default()

DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n\x1cprotoc_explorer/other2.proto\x12\x03\x66oo\"\x0f\n\rSayFooRequest\"\x10\n\x0eSayFooResponse2:\n\x03\x46oo\x12\x33\n\x06SayFoo\x12\x12.foo.SayFooRequest\x1a\x13.foo.SayFooResponse\"\x00\x62\x06proto3')

_globals = globals()
_builder.BuildMessageAndEnumDescriptors(DESCRIPTOR, _globals)
_builder.BuildTopDescriptorsAndMessages(DESCRIPTOR, 'google3.protoc_explorer.other2_pb2', _globals)
if _descriptor._USE_C_DESCRIPTORS == False:
  DESCRIPTOR._loaded_options = None
  _globals['_SAYFOOREQUEST']._serialized_start=37
  _globals['_SAYFOOREQUEST']._serialized_end=52
  _globals['_SAYFOORESPONSE']._serialized_start=54
  _globals['_SAYFOORESPONSE']._serialized_end=70
  _globals['_FOO']._serialized_start=72
  _globals['_FOO']._serialized_end=130
# @@protoc_insertion_point(module_scope)

It seems like it should address the issue if we just change this to:

# -*- coding: utf-8 -*-
# Generated by the protocol buffer compiler.  DO NOT EDIT!
# source: protoc_explorer/other2.proto
"""Generated protocol buffer code."""
import google3
from google.protobuf import descriptor as _descriptor
from google.protobuf import descriptor_pool as _descriptor_pool
from google.protobuf import symbol_database as _symbol_database
from google.protobuf.internal import builder as _builder
# @@protoc_insertion_point(imports)

_sym_db = _symbol_database.Default()

DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n\x1cprotoc_explorer/other2.proto\x12\x03\x66oo\"\x0f\n\rSayFooRequest\"\x10\n\x0eSayFooResponse2:\n\x03\x46oo\x12\x33\n\x06SayFoo\x12\x12.foo.SayFooRequest\x1a\x13.foo.SayFooResponse\"\x00\x62\x06proto3')

_globals = {}
_builder.BuildMessageAndEnumDescriptors(DESCRIPTOR, _globals)
_builder.BuildTopDescriptorsAndMessages(DESCRIPTOR, 'google3.protoc_explorer.other2_pb2', _globals)
if _descriptor._USE_C_DESCRIPTORS == False:
  DESCRIPTOR._loaded_options = None
  _globals['_SAYFOOREQUEST']._serialized_start=37
  _globals['_SAYFOOREQUEST']._serialized_end=52
  _globals['_SAYFOORESPONSE']._serialized_start=54
  _globals['_SAYFOORESPONSE']._serialized_end=70
  _globals['_FOO']._serialized_start=72
  _globals['_FOO']._serialized_end=130
# @@protoc_insertion_point(module_scope)

# Assign all top-level globals explicitly.
SayFooRequest = _globals['SayFooRequest']
SayFooResponse = _globals['SayFooResponse']

Does that sound accurate?

@anandolee could you take a look?

charlesbvll commented 10 months ago

Any update on this?

github-actions[bot] commented 7 months ago

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

HoangNguyen689 commented 7 months ago

The problem stayed the same until now.

protobuf v1.33.0 
E0611: No name 'XXXRequest' in module 'xxxgrpc.user_svc_pb2' (no-name-in-module)

@anandolee Do you have any updates on this problem?

github-actions[bot] commented 4 months ago

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.

huwcbjones commented 4 months ago

Still an issue with

astroid==3.2.4
grpcio==1.65.1
grpcio-tools==1.65.1
protobuf==5.27.2
pylint==3.2.6
kristofersomlin commented 3 months ago

Still an issue with

astroid==3.2.4
grpcio==1.65.1
grpcio-tools==1.65.1
protobuf==5.27.2
pylint==3.2.6

have you tried using --prefer-stubs when using pylint? Helped me.

mforkel commented 3 months ago

have you tried using --prefer-stubs when using pylint? Helped me.

Helped me, too. Option added in pylint 3.2.1.

hvent90 commented 3 months ago

Still an issue with

astroid==3.2.4
grpcio==1.65.1
grpcio-tools==1.65.1
protobuf==5.27.2
pylint==3.2.6

have you tried using --prefer-stubs when using pylint? Helped me.

Oh my god this fixed my hours of debugging. THANK YOU!

danslinger commented 3 months ago

Adding --prefer-stubs made the no-name-in-module go away. However now I get no-member E1101 when trying to access enum properties. Has anyone come across this?

furgerf commented 3 months ago

Adding --prefer-stubs made the no-name-in-module go away. However now I get no-member E1101 when trying to access enum properties. Has anyone come across this?

Same for me - it also created some issues with numpy, which led me to open this issue - sounds like --prefer-stubs may not be a great solution, a change to the code/stubs generated from protobuf might be more useful.

github-actions[bot] commented 1 day ago

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.

mforkel commented 1 day ago

This issue should remain active