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

[unsubscriptable-object] False Positive on Optional list #9515

Open embonhomme opened 7 months ago

embonhomme commented 7 months ago

Bug description

# Optional method arguments or local scope variables are not properly inferred to be list[str] when in the if body.
# example.py
from typing import Optional
from pydantic import BaseModel

class Parents(BaseModel):
    name: str
    age: int
    children_name: Optional[list[str]] = None

    def get_first_children_name(self) -> Optional[str]:
        if self.children_name:
            return self.children_name[0]
        return None

Configuration

[FORMAT]

# Maximum number of characters on a single line.
max-line-length = 120

[MESSAGE CONTROL]

disable =
    missing-module-docstring,
    missing-function-docstring,
    missing-class-docstring,
    fixme,
    unused-argument,
    broad-exception-caught,
    too-few-public-methods,
    import-error,
    no-name-in-module

[BASIC]
good-names=
    i,
    e,
    logger

[MASTER]

# disable false positives for Pydantic, which uses C extensions
# see https://github.com/pydantic/pydantic/issues/1961#issuecomment-759522422
extension-pkg-whitelist = pydantic

Command used

pylint --reports=n example.py

Pylint output

E1136: Value 'self.children_name' is unsubscriptable (unsubscriptable-object)

Expected behavior

No error at all

Pylint version

pylint 2.17.5
astroid 2.15.6
Python 3.11.6 (main, Nov  6 2023, 08:36:52) [GCC 9.4.0]

OS / Environment

No response

Additional dependencies

pydantic-core 2.6.3
typing-extensions 4.7.1
jacobtylerwalls commented 7 months ago

Thanks for the report. pylint-dev/astroid#1189 added an inference constraint for is None, but we should add one for boolean tests.

For that reason, if you adjust the example to test if self.children_name is not None, the false positive goes away, but that's not advisable as you'd have to test the length of the array (and thus isn't pythonic).

CareF commented 3 months ago

Same with dict. See https://github.com/pylint-dev/pylint/issues/9809

CareF commented 3 months ago

Thanks for the report. pylint-dev/astroid#1189 added an inference constraint for is None, but we should add one for boolean tests.

For that reason, if you adjust the example to test if self.children_name is not None, the false positive goes away, but that's not advisable as you'd have to test the length of the array (and thus isn't pythonic).

@jacobtylerwalls Please see another example: #9809 where if foo is None: does not successfully help to infer the optional type. This I think is a very common pattern for lazy loading attributes.