tempfile: Fix TypeVar usage

[AlexWaygood]

#7928

I find the name _DirT really confusing, considering it's a type alias rather than a TypeVar, but decided against fixing that in this PR in order to keep the diff small.

[AlexWaygood]

Diff from mypy_primer, showing the effect of this PR on open source code:

manticore (https://github.com/trailofbits/manticore)
+ manticore/core/workspace.py:220: error: Argument 1 to "exists" has incompatible type "Optional[str]"; expected "Union[Union[str, bytes, PathLike[str], PathLike[bytes]], int]"+ manticore/core/workspace.py:221: error: Argument 1 to "isdir" has incompatible type "Optional[str]"; expected "Union[Union[str, bytes, PathLike[str], PathLike[bytes]], int]"+ manticore/core/workspace.py:223: error: Argument 1 to "mkdir" has incompatible type "Optional[str]"; expected "Union[str, bytes, PathLike[str], PathLike[bytes]]"

I have no idea why this change makes mypy think that the result of os.path.abspath(tempfile.mkdtemp(prefix='foo', dir='bar')) can be None.

https://github.com/trailofbits/manticore/blob/9e11bc9ae9cc961d38c2a4932372d89fec2c6b26/manticore/core/workspace.py#L212-L223

[AlexWaygood]

Okay, I've managed to reproduce the manticore error:

fromtypingimportAny, AnyStr, TypeAlias, overloadimportosconsts: Any_DirT: TypeAlias=AnyStr|os.PathLike[AnyStr]
@overloaddefmkdtemp(suffix: None= ..., prefix: None= ..., dir: None= ...) ->str: ...
@overloaddefmkdtemp(suffix: AnyStr, prefix: AnyStr|None= ..., dir: _DirT[AnyStr] |None= ...) ->AnyStr: ...
@overloaddefmkdtemp(*, prefix: AnyStr, dir: _DirT[AnyStr] |None= ...) ->AnyStr: ...
@overloaddefmkdtemp(*, dir: _DirT[AnyStr]) ->AnyStr: ...
reveal_type(mkdtemp(prefix=consts.prefix, suffix=consts.suffix)) # Revealed type is "Any"reveal_type(os.path.abspath(mkdtemp(prefix=consts.prefix, suffix=consts.suffix))) # Revealed type is "Any"x: str|Noneifnotx:
x=os.path.abspath(mkdtemp(prefix=consts.prefix, suffix=consts.suffix)) # Revealed type is "Union[builtins.str, None]"reveal_type(x)

https://mypy-play.net/?mypy=latest&python=3.10&gist=67faf105c3c17e72591a947bf45aeb64

Mypy appears to be refusing to do type narrowing for the if not x: guard due to the presence of the Anys.

---

This is how mypy currently infers the types, by contrast:

fromtypingimportAny, AnyStr, TypeAlias, overloadfromtempfileimportmkdtempimportosconsts: Anyreveal_type(mkdtemp(prefix=consts.prefix, suffix=consts.suffix)) # Revealed type is "builtins.str"reveal_type(os.path.abspath(mkdtemp(prefix=consts.prefix, suffix=consts.suffix))) # Revealed type is "builtins.str"x: str|Noneifnotx:
x=os.path.abspath(mkdtemp(prefix=consts.prefix, suffix=consts.suffix))
reveal_type(x) # Revealed type is "builtins.str"

https://mypy-play.net/?mypy=latest&python=3.10&gist=2bae188586be16ac81bd72c96244f017

I.e., currently mypy incorrectly infers that the return type of mkdtemp will definitely be str if the values passed into the prefix and suffix parameters are both of type Any.

[hauntsaninja]

Looks like it was one of those cases where it's sensitive to the ordering of the contraints in AnyStr, if we had AnyStr = TypeVar("AnyStr", bytes, str) we'd get a different result. cc @JelleZijlstra

[JelleZijlstra]

This disallows some valid calls, such as tempfile.mkstemp(None, b"x"). I think we can fix this and work around the mypy bug with three overloads:

1. One where all the args are None (you already have it)
2. One where all the args are str | None, returning tuple[int, str]
3. One where all the args are bytes | None, returning tuple[int, bytes]

mkdtemp would need similar treatment.

mktemp indeed doesn't accept bytes, so it was correct to remove the AnyStr for it.

[hauntsaninja]

nit: I'd remove the "according to mypy" since these overloads genuinely do overlap (just to disambiguate from other comments where we workaround incorrect mypy behaviour).

[AlexWaygood]

nit: I'd remove the "according to mypy" since these overloads genuinely do overlap (just to disambiguate from other comments where we workaround incorrect mypy behaviour).

I think the only reason pyright never complains about this is we have the check switched off in pyrightconfig.json.

[AlexWaygood]

This disallows some valid calls, such as tempfile.mkstemp(None, b"x").

Good spot! I'm fine with the new approach :)

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

manticore (https://github.com/trailofbits/manticore)
+ manticore/core/workspace.py:220: error: Argument 1 to "exists" has incompatible type "Optional[str]"; expected "Union[Union[str, bytes, PathLike[str], PathLike[bytes]], int]"+ manticore/core/workspace.py:221: error: Argument 1 to "isdir" has incompatible type "Optional[str]"; expected "Union[Union[str, bytes, PathLike[str], PathLike[bytes]], int]"+ manticore/core/workspace.py:223: error: Argument 1 to "mkdir" has incompatible type "Optional[str]"; expected "Union[str, bytes, PathLike[str], PathLike[bytes]]"