microsoft / pyright

Static Type Checker for Python
Other
13.09k stars 1.4k forks source link

TypedDict with optional keys errors despite guarded access #1739

Closed Rapptz closed 3 years ago

Rapptz commented 3 years ago

Describe the bug When using TypedDict with total=False, guarded access with try except reports an error.

To Reproduce

from typing import TypedDict
class A(TypedDict, total=False):
    test: str

def get_test(a: A) -> str:
    try:
        value = a['test'] # error here
    except KeyError:
        return 'ok'
    else:
        return f'received: {value}'

Expected behavior No error since the code is being guarded by a try-except.

Screenshots or Code image

VS Code extension or command-line VSCode. Pylance 2021.3.4.

erictraut commented 3 years ago

Pyright doesn't support exception-based guards. If you want this to type check, you should use a regular (non-exception-based) check:

def get_test(a: A) -> str:
    if 'test' in a:
        value = a['test']
    else:
        return 'ok'
    return f'received: {value}'

Even better, use the get method. This is much cleaner and more readable:

def get_test(a: A) -> str:
    return a.get('test', 'ok')
Rapptz commented 3 years ago

The actual code is a bit more complicated, I had kind of shown it a little bit with the value being used in the found case but not in the exception case.

In Python things are usually done using EAFP rather than LBYL. Likewise the behaviour of the two aren't equivalent, in the try except case you can avoid certain high cost evaluations whereas the get or setdefault cases would always pay the price for the default parameter.

Ultimately I can just use # type: ignore but I don't think it looks good to do this on idiomatic Python.

erictraut commented 3 years ago

Yeah, that's a reasonable point.

While I think it's a bad idea to use exceptions for normal code flow, I unfortunately do see this commonly in Python code. It's especially bad if you're interested in the benefits of type checking because it harms the type checker's ability to understand the code flow. Because of this, you're going to mask bugs that would otherwise be caught by the tools. It's also just harder to read and maintain, IMO.

I verified that mypy suppresses this check when it's within a try block, so I've changed pyright to do the same.

This will be in the next release.

erictraut commented 3 years ago

This is included in Pyright 1.1.130, which I just published. It will also be part of the next release of Pylance.

apodznoev commented 3 months ago

Just for the same snippet as originally posted there is the same error:

error: Could not access item in TypedDict
    "test" is not a required key in "A", so access may result in runtime exception (reportTypedDictNotRequiredAccess)

starting from 1.1.360 onwards (up to 1.1.366).

The issue was not there in 1.1.359.

erictraut commented 3 months ago

@apodznoev, yes this change was intentional. See this issue for details. I decided to reverse the original decision because it was inconsistent with the general philosophy of type checking. Code within a try block should not be exempt from type checks.

If you're interested in making your code type safe, do not use exception handling for expected code flow. Instead, use regular if statements to check for conditions like the presence of specific keys.

Rapptz commented 3 months ago

That's an unfortunate change considering how overwhelmingly common try-except blocks are for accessing keys in a dictionary or other type of EAFP.

erictraut commented 3 months ago

I think EAFP is a very unfortunate and ill-advised practice. If you're interested in type safety and code robustness, I recommend avoiding this pattern. Exceptions should be used for exceptional cases, not anticipated code flow.

Rapptz commented 3 months ago

The type checker for a language should complement the predominant style of the programming language it's type checking for. I don't really want to discuss the merits of EAFP vs LBYL here but the choice is often there and commonly taken to prevent bugs like TOCTOU or for performance reasons. I understand that this is not your preferred programming style but this is honestly very common for Python. Even the notion of "exceptional cases only" for exceptions doesn't hold true here, exceptions are used all over the language for control flow internally such as StopIteration and the various other related exceptions when working with coroutines.