python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.57k stars 2.84k forks source link

False negative: potentially unbound instance attribute doesn't trigger `possibly-undefined` error #17837

Open nsoranzo opened 2 months ago

nsoranzo commented 2 months ago

Bug Report

When enabling the possibly-undefined error code, a potentially undefined instance attribute doesn't trigger the error, while a potentially unbound variable does.

To Reproduce

def test_possibly_undefined(flag: bool):
    if flag:
        a = 1
    print(a)  # error: Name "a" may be undefined  [possibly-undefined]

class Foo:
    def test_possibly_undefined(self, flag: bool):
        if flag:
            self.a = 1
        print(self.a)  # false negative, should also be a possibly-undefined error

try:
    test_possibly_undefined(False)
except Exception as e:
    print(f"{type(e).__name__}: {e}")  # UnboundLocalError: cannot access local variable 'a' where it is not associated with a value

f = Foo()
try:
    f.test_possibly_undefined(False)
except Exception as e:
    print(f"{type(e).__name__}: {e}")  # AttributeError: 'Foo' object has no attribute 'a'

Also available as mypy Playground, but cannot enable enable_error_code = possibly-undefined there.

Expected Behavior

A possible-undefined error should be reported for line 11 as well, e.g.:

test.py:4: error: Name "a" may be undefined  [possibly-undefined]
test.py:11: error: Attribute "a" may be undefined  [possibly-undefined]
Found 2 errors in 1 file (checked 1 source file)

Actual Behavior

No error reported for line 11.

test.py:4: error: Name "a" may be undefined  [possibly-undefined]
Found 1 error in 1 file (checked 1 source file)

Your Environment

brianschubert commented 2 months ago

Related: #4019, #10736

Tracking conditionally defined attributes is hard, both for mypy and for humans. Generally it's preferable to have attributes always be defined, and if necessary give them a default value / optional type that can be guarded against. Though that wouldn't have helped in the linked issue, since the attribute wasn't supposed to be conditionally defined to begin with.

This sort of error can be easier to catch if you use a pattern like

if x:
    foo = 1
elif y:
    foo = 2

self.foo = foo

or

self.foo = _resolve_foo(x, y)
nsoranzo commented 2 months ago

@brianschubert Thanks for the quick reply and the links. Agreed that it's hard, that's why we need a tool like mypy to help :)

As you can guess, in a project with hundreds of contributors it is nearly impossible to ensure the use of such patterns every time an instance attribute is initialised. It's OK if there's no plan to address this, I think it's worth to have an issue specifically related to the the possibly-undefined error code.