Issue when referencing nested enums

[NickeZ]

given

syntax = "proto3";

message Request {
  enum Type {
    IN = 0;
    OUT = 1;
  }
  Type type = 1;
}

and

import messages_pb2
def fun(_typ: messages_pb2.Request.Type):
    pass

I get

example.py:3:14: E5901: Field 'Type' does not appear in the declared fields of protobuf-generated class 'messages_pb2.Request' and will raise AttributeError on access (protobuf-undefined-attribute)

Is this expected behavior?

[nelfin]

Hi @NickeZ, seems like Type isn't showing up in Request, but type is. I have some testing around nested enum definitions, but not enough to catch this apparently! Thanks for the report

[fivepapertigers]

Just chiming in here to say that the same problem exists for nested message definitions:

syntax = "proto3";

message Foo {
    message Bar {
        string baz = 1;
    }
    Bar bar = 1;
}

bar = Foo.Bar(baz='baz') #  Field 'Bar' does not appear in the declared fields of protobuf-generated class

EDIT

Fixed on current master. I just did:

$ pip install git+git://github.com/nelfin/pylint-protobuf.git@457dc4b5380aa01a7eb0d617c844f2660a426005

[nelfin]

Hi @fivepapertigers, thanks for the update. Yeah, your specific case was fixed in 197349b. I can't remember why I didn't cut a new release for that, maybe I was focused on resolving the nested enum definitions. I'll release it now as v0.12.

[NickeZ]

Hey @nelfin, I just wanna say thanks for your hard work so far!

[diana-infinitus-ai]

Hi @nelfin !

I recently updated the grpcio & grpcio-tools modules which caused nested enums and messages not being recognized anymore. I was able to trace down where it is failing (and a possible fix)

The new generated _pb2.py files have replaced:

_reflection.GeneratedProtocolMessageType('ProtoMessage', (_message.Message,), dict(
  DESCRIPTOR = _PROTOMESSAGE,
  __module__ = 'proto_message_pb2'
))

with

_reflection.GeneratedProtocolMessageType('ProtoMessage', (_message.Message,), {
  'DESCRIPTOR': _PROTOMESSAGE,
  '__module__': 'proto_message_pb2'
})

which seems innocuous, but because of how inner types are detected here https://github.com/nelfin/pylint-protobuf/blob/master/pylint_protobuf/parse_pb2.py#L193 (where it skips over all arguments that is not a Call type, but {} way of declaring dicts doesn't fall inside this) inner message types are no longer detected which causes incorrect protobuf-undefined-attribute errors.

This can be fixed by updating add_inner_types function to something like:

    for arg in rhs.args:
        if type(arg) is not astroid.Call and type(arg) is not astroid.Dict:
            continue

        keywords = arg.keywords if type(arg) is astroid.Call else arg.items
        for kw in keywords:
            kwarg = kw.arg if hasattr(kw, 'arg') else kw[0].value
            if kwarg == 'DESCRIPTOR':
                try:
                    name = kw.value.name if hasattr(
                        kw, 'value') else kw[1].name
    ...

(but maybe cleaner than what I hacked together during debugging?)

Let me know if you'd like me to open a PR with this change, or if you could make a similar fix?

Thanks again for all the work on this package!

[nelfin]

Hi @diana-infinitus-ai,

I thought those guards might come back to bite me some day, too brittle! I even specifically remember dict() and {}-literals.

You should send through a PR, you've already gone through the hard work of diagnosing the issue and coming up with a fix. If you'd rather not or if you don't have time then no problem, don't feel obligated, I'll sort something out tonight (UTC+10:00).

There are a few tests mocked XFAIL in test_enum.py that you may want to test your fix against.

On Wed., 17 Jun. 2020, 07:23 diana-infinitus-ai, notifications@github.com wrote:

Hi @nelfin https://github.com/nelfin !

I recently updated the grpcio & grpcio-tools modules which caused nested enums and messages not being recognized anymore. I was able to trace down where it is failing (and a possible fix)

The new generated _pb2.py files have replaced:

_reflection.GeneratedProtocolMessageType('ProtoMessage', (_message.Message,), dict( DESCRIPTOR = _PROTOMESSAGE, module = 'proto_message_pb2' ))

with

_reflection.GeneratedProtocolMessageType('ProtoMessage', (_message.Message,), { 'DESCRIPTOR': _PROTOMESSAGE, 'module': 'proto_message_pb2' })

which seems innocuous, but because of how inner types are detected here https://github.com/nelfin/pylint-protobuf/blob/master/pylint_protobuf/parse_pb2.py#L193 (where it skips over all arguments that is not a Call type, but {} way of declaring dicts doesn't fall inside this) inner message types are no longer detected which causes incorrect protobuf-undefined-attribute errors.

This can be fixed by updating add_inner_types function to something like:

for arg in rhs.args:
    if type(arg) is not astroid.Call and type(arg) is not astroid.Dict:
        continue

    keywords = arg.keywords if type(arg) is astroid.Call else arg.items
    for kw in keywords:
        kwarg = kw.arg if hasattr(kw, 'arg') else kw[0].value
        if kwarg == 'DESCRIPTOR':
            try:
                name = kw.value.name if hasattr(
                    kw, 'value') else kw[1].name
...

(but maybe cleaner than what I hacked together during debugging?)

Let me know if you'd like me to open a PR with this change, or if you could make a similar fix?

Thanks again for all the work on this package!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nelfin/pylint-protobuf/issues/21#issuecomment-645019189, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABSH3PY72UBER5RPAWQACDRW7PERANCNFSM4JAFFFFA .

[nelfin]

*marked XFAIL

On Wed., 17 Jun. 2020, 08:30 Andrew, nelfin@gmail.com wrote:

Hi @diana-infinitus-ai,

I thought those guards might come back to bite me some day, too brittle! I even specifically remember dict() and {}-literals.

You should send through a PR, you've already gone through the hard work of diagnosing the issue and coming up with a fix. If you'd rather not or if you don't have time then no problem, don't feel obligated, I'll sort something out tonight (UTC+10:00).

There are a few tests mocked XFAIL in test_enum.py that you may want to test your fix against.

On Wed., 17 Jun. 2020, 07:23 diana-infinitus-ai, notifications@github.com wrote:

Hi @nelfin https://github.com/nelfin !

I recently updated the grpcio & grpcio-tools modules which caused nested enums and messages not being recognized anymore. I was able to trace down where it is failing (and a possible fix)

The new generated _pb2.py files have replaced:

_reflection.GeneratedProtocolMessageType('ProtoMessage', (_message.Message,), dict( DESCRIPTOR = _PROTOMESSAGE, module = 'proto_message_pb2' ))

with

_reflection.GeneratedProtocolMessageType('ProtoMessage', (_message.Message,), { 'DESCRIPTOR': _PROTOMESSAGE, 'module': 'proto_message_pb2' })

which seems innocuous, but because of how inner types are detected here https://github.com/nelfin/pylint-protobuf/blob/master/pylint_protobuf/parse_pb2.py#L193 (where it skips over all arguments that is not a Call type, but {} way of declaring dicts doesn't fall inside this) inner message types are no longer detected which causes incorrect protobuf-undefined-attribute errors.

This can be fixed by updating add_inner_types function to something like:

for arg in rhs.args:
    if type(arg) is not astroid.Call and type(arg) is not astroid.Dict:
        continue

    keywords = arg.keywords if type(arg) is astroid.Call else arg.items
    for kw in keywords:
        kwarg = kw.arg if hasattr(kw, 'arg') else kw[0].value
        if kwarg == 'DESCRIPTOR':
            try:
                name = kw.value.name if hasattr(
                    kw, 'value') else kw[1].name
...

(but maybe cleaner than what I hacked together during debugging?)

Let me know if you'd like me to open a PR with this change, or if you could make a similar fix?

Thanks again for all the work on this package!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nelfin/pylint-protobuf/issues/21#issuecomment-645019189, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABSH3PY72UBER5RPAWQACDRW7PERANCNFSM4JAFFFFA .

[diana-infinitus-ai]

Thanks for the reply!! I tried to push a branch but didn't have permissions - here's the commit in a fork https://github.com/diana-infinitus-ai/pylint-protobuf/commit/79fb1f88e0630ed24b24d9acdaf0da11d6585e14

I couldn't get any of the nested tests to pass, but when I tried out the change, it worked. Maybe you can shed some light?

[nelfin]

I tried to push a branch but didn't have permissions - here's the commit in a fork diana-infinitus-ai@79fb1f8

That's the usual approach to collaboration on GitHub. Instead of git pushing to repositories that other people manage, you push changes to your own and they pull from there. Hence the name Pull Request. I've created one here #28 to discuss your changes.

[nelfin]

@NickeZ, this should be fixed in the release of 0.14 (see tests here: https://github.com/nelfin/pylint-protobuf/blob/6593ec1f39bd607d68e30f43b25804ca1e8d71c4/tests/test_enum.py#L177). Can you confirm that this works for your use case?

[nelfin]

I've confirmed on my end that this is fixed, I'll close this issue in a couple of days if there's no update.

[NickeZ]

Sorry, I don't have the context anymore for this issue, so I cannot confirm that it is solved. But I'm sure you are right 🎉