pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.3k stars 1.14k forks source link

false positive E1137 with optional list #3832

Open ZeeD opened 4 years ago

ZeeD commented 4 years ago

I have this little module:

'xxx'

import typing

def main() -> None:
    'collapse rows if the first column is empty'

    rows: typing.Iterable[typing.List[str]] = [['foo', 'bar'], ['', 'baz']]

    itor = iter(rows)
    prev: typing.Optional[typing.List[str]] = None
    while True:
        try:
            row = next(itor)
        except StopIteration:
            break

        if row[0]:
            if prev is not None:
                print(prev)
            prev = row
        else:
            assert prev is not None

            for i, cell in enumerate(row):
                if i == 0:
                    continue

                prev[i] += ' ' + cell

    if prev is not None:
        print(prev)

if __name__ == '__main__':
    main()

while it does what I expect (it prints ['foo', 'bar baz']), pylint says that there is an error:

>pylint falsepositive.py
************* Module falsepositive
falsepositive.py:29:16: E1137: 'prev' does not support item assignment (unsupported-assignment-operation)

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

pylint --version output

>pylint --version
pylint 2.6.0
astroid 2.4.2
Python 3.7.2 (tags/v3.7.2:9a3ffc0492, Dec 23 2018, 23:09:28) [MSC v.1916 64 bit (AMD64)]
clavedeluna commented 1 year ago

I looked into this and the reason is decidedly because prev, assigned as prev: typing.Optional[typing.List[str]] = None is initialized as None. While unsupported-assignment-operation is a red herring, it's telling the author to not initialize a value intended to be a list to a None type, instead initialize to [].

Would be nice if pylint had (does it have??) instead a warning to let us know if we've initialized the incorrect empty value for the type. But I'm not sure the particular issue as posed here is worth fixing?

DanielNoord commented 1 year ago

I mean, the typing says it can be None so it being None seems fine? The main issue is that we don't understand the assertion, for which an astroid PR has been in the making for quite some time now.