protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

python: pylint no-name-in-module

Open huwcbjones opened this issue 3 years ago • 3 comments

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
# -*- 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
# -*- 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 avatar Aug 08 '22 17:08 huwcbjones

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

huwcbjones avatar Aug 08 '22 17:08 huwcbjones

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.

haberman avatar Oct 24 '22 17:10 haberman

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!

huwcbjones avatar Oct 24 '22 17:10 huwcbjones

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 avatar Feb 27 '23 23:02 haberman

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

haberman avatar Feb 27 '23 23:02 haberman

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

peterdemin avatar May 05 '23 14:05 peterdemin

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

  • https://github.com/pylint-dev/pylint/issues/4987
  • https://github.com/pylint-dev/astroid/pull/2182
  • https://github.com/pylint-dev/pylint/issues/6281#issuecomment-1571705142

AnthonyDiGirolamo avatar Jun 16 '23 00:06 AnthonyDiGirolamo

Any more update on this one?

Ruoyu-y avatar Jul 27 '23 01:07 Ruoyu-y

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

jenstroeger avatar Dec 01 '23 06:12 jenstroeger

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?

haberman avatar Dec 01 '23 16:12 haberman

Any update on this?

charlesbvll avatar Jan 18 '24 09:01 charlesbvll