Skip to content
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

termios: Definition of tcgetattr causes false positives #4661

Closed
cebtenzzre opened this issue Oct 12, 2020 · 1 comment · Fixed by #4662
Closed

termios: Definition of tcgetattr causes false positives #4661

cebtenzzre opened this issue Oct 12, 2020 · 1 comment · Fixed by #4662

Comments

@cebtenzzre
Copy link
Contributor

Example Code

The example from the termios docs:

def getpass(prompt="Password: "):
    import termios, sys
    fd = sys.stdin.fileno()
    old = termios.tcgetattr(fd)
    new = termios.tcgetattr(fd)
    new[3] = new[3] & ~termios.ECHO          # lflags
    try:
        termios.tcsetattr(fd, termios.TCSADRAIN, new)
        passwd = input(prompt)
    finally:
        termios.tcsetattr(fd, termios.TCSADRAIN, old)
    return passwd

Actual Behavior

$ mypy --pretty --show-error-codes --check-untyped-defs example.py
example.py:6: error: Unsupported operand types for & ("List[bytes]" and "int")  [operator]
        new[3] = new[3] & ~termios.ECHO          # lflags
                 ^
example.py:6: note: Left operand is of type "Union[int, List[bytes]]"
Found 1 error in 1 file (checked 1 source file)

Expected Behavior

The code typechecks successfully, because it is correct. In the reality of runtime, I never have to worry that new[3] will be a list, because it won't.


The mypy error is caused by the definition of _Attr, which is the return type of tcgetattr:

_Attr = List[Union[int, List[bytes]]]

Typeshed's own contribution guidelines, python/mypy#1693, and mypy's docs agree that union return types are bad news. I can get around this with an assert isinstance(new[3], int), but at runtime new[0] through new[5] will always be int, and new[6] will always be List[bytes] *, so the assertion is pointless.

It would be clearer if tcgetattr returned an object so the different values could have their own names and types, but that decision predates typeshed. It would be nice if we at least had an AnyOf type (python/typing#566) so we could write List[AnyOf[int, List[AnyOf[bytes, int]]]], but we don't. So we are left with List[Any], which is not very specific but is at least not incorrect.

* Except for new[6][termios.VTIME] and new[6][termios.VMIN], which are int in noncanonical mode (when new[3] & termios.ICANON is zero). The stubs currently ignore this possibility.

@hauntsaninja
Copy link
Collaborator

Thanks for the report! I'd merge a PR changing this to List[Any] :-)

cebtenzzre added a commit to cebtenzzre/typeshed that referenced this issue Oct 12, 2020
Returning a union from tcgetattr forces the caller to use isinstance or
a type: ignore comment if they modify the returned list. Return
List[Any] instead.

The cc field has ints at cc[termios.VTIME] and cc[termios.VMIN] if
lflag & termios.ICANON is zero. Change _Attr to allow this.

Fixes python#4661
hauntsaninja pushed a commit that referenced this issue Oct 13, 2020
Returning a union from tcgetattr forces the caller to use isinstance or
a type: ignore comment if they modify the returned list. Return
List[Any] instead.

The cc field has ints at cc[termios.VTIME] and cc[termios.VMIN] if
lflag & termios.ICANON is zero. Change _Attr to allow this.

Fixes #4661
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants