nelfin / pylint-protobuf

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

handle ambiguous/multiple result from astroid.inferred() #36

Closed mabrowning closed 3 years ago

mabrowning commented 3 years ago

node.expr.inferred() can return multiple values, e.g. when a class member is initialized, then redefined:

"""
ambiguous type inference
"""

from value_pb2 import Value
class MyClass:
    """
    Class that initializes its members to None
    """
    def __init__(self):
        self.value = None
    def load(self, filename):
        """
        Loads value from a binary protobuf encoded at filename
        """
        self.value = Value()
        with open(filename, "r") as f:
            self.value.ParseFromString(f.read())

Before this PR, pylint produces:

pylint test.py
************* Module test
test.py:18:12: E1101: Instance of 'Value' has no 'ParseFromString' member (no-member)

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

Because node.expr.inferred()[0] reflects the initial setting of self.value = None and returns (basically) NoneType.

I'm not sure the new logic is correct, but it visits all possible inferred types of the expression, and if any are a Protobuf class, enforce the protobuf specific constraints.

nelfin commented 3 years ago

Yeah, I never really knew what the appropriate approach was here. If we switch your example around just a bit:

class MyClass:
    def __init__(self):
        self.value = None
    def set(self):
        self.value = Test()
    def parse(self):
        self.value.ParseFromString("blahblahblah")

Then node.expr.inferred() also reports [<Const.NoneType l.7 at 0x...>, <Instance of .Test at 0x...>] for the line self.value.ParseFromString(...). I think it's fairly reasonable to run the checker over types which have at one point been protobuf messages, short of simulating the whole universe.