nelfin / pylint-protobuf

A plugin for making Pylint aware of the fields of protobuf-generated classes
MIT License
29 stars 12 forks source link

Incorrect indention in inner type generator template #33

Closed mabrowning closed 3 years ago

mabrowning commented 3 years ago

This one took awhile to track down, and it looks stupid but it is a legal and realistic case that happens in our codebase that I've distilled down to the minimal example.

Consider:

$ cat nest_test.proto
syntax = "proto3";

message A {
    message B {
        message C {
            message D {
            }
        }
    }
}

message Test {
    string name = 1;
}

$ protoc nest_test.proto --python_out=.
$ cat test.py
"""
Nested Message text
"""

from nest_test_pb2 import Test
a = Test(name="Hello")
print(a.name)

$ python test.py
Hello

But:

$ pylint test.py
************* Module test
test.py:7:6: E1101: Instance of 'Test' has no 'name' member (no-member)

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

Removing D and the later Test message is parsed correctly; pylint passes. Moving Test before ABCD also allows it to be parsed correctly and pylint passes.

Instrumenting transform::transform_module a bit shows we're not catching:

Traceback (most recent call last):
  File "/usr/lib/python3.7/site-packages/astroid/builder.py", line 166, in _data_build
    node = _parse(data + "\n")
  File "/usr/lib/python3.7/site-packages/astroid/_ast.py", line 40, in _parse
    return parse_func(string)
  File "/usr/lib/python3.7/site-packages/typed_ast/ast3.py", line 62, in parse
    return _ast3._parse(source, filename, mode, feature_version)
  File "<unknown>", line 13
    class D(object):
    ^
IndentationError: unexpected indent

and it breaks out of the loop over the wildcard_import_names() prematurely.

It it appears the generated template for this is

class A(object):
    __slots__ = ('B',)
    def __getitem__(self, idx):
        pass
    class B(object):
        __slots__ = ('C',)
        def __getitem__(self, idx):
            pass
            class C(object):
                __slots__ = ('D',)
                def __getitem__(self, idx):
                    pass
                        class D(object):
                            __slots__ = ()
                            def __getitem__(self, idx):
                                pass
                            def __init__(self, *args, **kwargs):
                                pass
                def __init__(self, *args, **kwargs):
                    pass
        def __init__(self, *args, **kwargs):
            pass
    def __init__(self, *args, **kwargs):
        pass

So something in the template is indenting it too much. In fact, all the inner classes seems indented twice as much as they should be, but it's only at the 4th level that it becomes a syntax error. However, at depth 3 it's putting the inner class (e.g. C) at too local of a scope.

Changing https://github.com/nelfin/pylint-protobuf/blob/1c4caaa926ced28a9419bc2d06e1e93386653eb7/pylint_protobuf/transform.py#L60 to

        fragments.append(_template_message(inner_desc, depth=1))

Fixes the issue, but doesn't seem like the cleanest approach.