Skip to content

Regression of os.scandir() typecheck #11964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
achimnol opened this issue Jan 10, 2022 · 8 comments
Open

Regression of os.scandir() typecheck #11964

achimnol opened this issue Jan 10, 2022 · 8 comments
Labels
bug mypy got something wrong topic-type-variables

Comments

@achimnol
Copy link
Contributor

Bug Report

Type variable inference related with os.scandir() seems to generate false-positives.

To Reproduce

Here is a minimal reproduction example, which is accepted by mypy 0.910 but rejected by 0.930 and 0.931.

from __future__ import annotations

import os
from pathlib import Path


def _calc_usage(target_path: Path | os.DirEntry) -> None:
    with os.scandir(target_path) as scanner:
        for entry in scanner:
            if entry.is_dir():
                _calc_usage(target_path)
            else:
                print('f')


_calc_usage(Path('.'))

Expected Behavior

Success: no issues found in 1 source file

Actual Behavior

test.py:8: error: Value of type variable "AnyStr" of "scandir" cannot be "Union[DirEntry[Any], Any]"

Your Environment

  • Mypy version used: 0.931
  • Mypy command-line flags: python -m mypy test.py
  • Mypy configuration options from mypy.ini (and other config files):
  • Python version used: 3.9.6
  • Operating system and version: macOS 12.1
@achimnol achimnol added the bug mypy got something wrong label Jan 10, 2022
@JelleZijlstra
Copy link
Member

For what it's worth, the only significant recent change to the stubs for scandir was to use | syntax: https://github.com/python/typeshed/blame/master/stdlib/os/__init__.pyi#L711

@ktbarrett
Copy link

@achimnol Are you certain the original code is valid? typeshed does not mention that os.scandir takes a os.DirEntry.

@JelleZijlstra
Copy link
Member

@ktbarrett DirEntry is an os.PathLike.

@ktbarrett
Copy link

ktbarrett commented Jan 10, 2022

The change from Union to | did not affect anything. Confirmed it broke in 0.920.

Changing the signature of scandir to the following works.

@overload
def scandir(path: str | PathLike[str]]) ->  _ScandirIterator[str]: ...
@overload
def scandir(path: bytes | PathLike[bytes]) ->  _ScandirIterator[bytes]: ...

Changing the signature of AnyStr to use a Union bound also works. So the problem must be with how listed types are checked between 0.910 and 0.920.

@ktbarrett
Copy link

git bisect shows 7808e82 (#10887).

@ktbarrett
Copy link

ktbarrett commented Jan 11, 2022

Started debugging a simpler example against the passing example below it that uses a Union bound instead of a list.

Fails:

a: os.DirEntry[Any]
os.scandir(a)

Passes:

from typing import *
import os

MyAnyStr = TypeVar("MyAnyStr", bound=Union[bytes, str])
MyAnyStr_co = TypeVar("MyAnyStr_co", bound=Union[bytes, str], covariant=True)

class MyPathLike(Protocol[MyAnyStr_co]):
    def __fspath__(self) -> MyAnyStr_co: ...

def wew(t: Union[MyAnyStr, MyPathLike[MyAnyStr]]) -> None:
    ...
    
test: os.DirEntry
wew(test)

After much debugging (which is not easy at all because of how coarse-grained the switch on incremental compile is) I found that the difference was in how the type variable was inferred. Example 1 inferred the TypeVar as Union[os.DirEntry[Any], Any] and Example 2 as Any. After debugging some more I discovered that only TypeVar bounds, and not the list, are considered in the below case, which ends up deciding the TypeVar constraints for the call.

mypy/mypy/constraints.py

Lines 198 to 202 in 48d810d

if (direction == SUPERTYPE_OF and isinstance(template, TypeVarType) and
not mypy.subtypes.is_subtype(actual, erase_typevars(template.upper_bound))):
# This is not caught by the above branch because of the erase_typevars() call,
# that would return 'Any' for a type variable.
return None

@achimnol
Copy link
Contributor Author

How is this going? Just checking in. 😉

@ktbarrett
Copy link

I spent a couple hours looking into it and I think it's due to a lack of support for type lists in infer_constraints. It's not a trivial fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-type-variables
Projects
None yet
Development

No branches or pull requests

4 participants