Please just change something. Weird combination of dead code and inferred None

[Akuli]

Feature

foo = None

def bar() -> str:
    global foo
    if foo is not None:
        print("Hello")
    else:
        foo = "bar"
        return "lol"

This program type-checks, even though it's obviously wrong. It's not a bug, it's a feature:

• Inferred type of foo is None, even though Optional[str] was intended.
• Because foo has type None, the if foo is not None: part is eliminated as dead code, and therefore the missing return isn't detected.
• After setting foo = "bar", reveal_type(foo) shows str. Perhaps the assignment should be an error?

Pitch

A friend submitted a PR like this and thought it was fine.

[Akuli]

I set warn_unreachable = True and discovered other similar bugs in existing code. Can we enable warn_unreachable by default?

[scaramallion]

Having warn_unreachable as the default would flag errors in code that performs checking along the lines of:

def foo(val: str) -> None:
    if not isinstance(val, str):
        raise TypeError("Bad!")

[Akuli]

I think warn_unreachable should be included in --strict, and I don't see this as a bad thing in a project that uses --strict. A --strict project doesn't need checks like that because of the type checker. If a function like this is really needed, then it would be better to annotate val as object or Any.

[sobolevn]

related #11223

[pranavrajpal]

I think the problem here is that mypy is inferring a partial None for foo, and then inconsistently using that fact when type checking bar.

I suppose mypy could try rechecking bar repeatedly until all partial types stopped changing using something similar to what we do for type checking loops, but I think the better fix is just disallowing partial types that can't be inferred completely using only the scope they were defined in by enabling --local-partial-types.

With --local-partial-types enabled, the output I'm getting is:

foo = None # E: Need type annotation for "foo"

def bar() -> str: # E: Missing return statement
    global foo
    if foo is not None:
        print("Hello")
    else:
        foo = "bar"
        return "lol"

which is a lot closer to what I'd expect.

Merging #10169 would make --local-partial-types the default, which should fix this issue.

[JukkaL]

I agree that --local-partial-types is the right fix. Making it the default is blocked for now since the switch can be quite disruptive for users. Improving the error messages could help. For example, we could suggest --no-local-partial-types or similar in a note. We could also perhaps allow disabling most of the new errors selectively, while still otherwise supporting local partial types.