Improve type hints for SFTPClient directory methods (`listdir`, `scandir`, `readdir`) using `@overload`

[vivodi]

I would like to suggest an improvement to the type hints for directory-reading methods in SFTPClient, specifically listdir, scandir, and readdir.

The Problem:

Currently, the signatures for these methods look something like this:

• async def listdir(self, path: PurePath | str | bytes) -> list[str | bytes]
• async def scandir(self, path: PurePath | str | bytes) -> AsyncIterator[SFTPName] (where SFTPName.filename is str | bytes)

While this accurately reflects that the methods can handle both strings and bytes, it causes friction during static type checking (with tools like Mypy or Pyright).

Because the return type is a broad union (str | bytes), users who pass a str or PurePath as the input path are forced to add isinstance(name, str) checks or # type: ignore comments before they can use standard string methods (like .endswith()) on the results.

The Proposed Solution:

We can align the typing behavior with Python's standard library (e.g., os.listdir and os.scandir), where the input type dictates the output type.

This can be achieved by:

1. Making SFTPName a Generic class to preserve the type of filename and longname.
2. Using typing.overload on listdir, scandir, and readdir.

Example Implementation:

import os
from typing import overload, Union, AsyncIterator, Sequence, Generic, TypeVar
from pathlib import PurePath

# Use AnyStr-like TypeVar to bind the type
T = TypeVar("T", str, bytes)

class SFTPName(Generic[T]):
    filename: T
    longname: T
    attrs: 'SFTPAttrs'
    # ... existing implementation

class SFTPClient:
    
    # --- listdir ---
    @overload
    async def listdir(self, path: Union[PurePath, str] = '.') -> list[str]: ...
    
    @overload
    async def listdir(self, path: bytes) -> list[bytes]: ...

    async def listdir(self, path: Union[PurePath, str, bytes] = '.') -> Union[list[str], list[bytes]]:
        # ... existing implementation ...
        pass

    # --- scandir ---
    @overload
    def scandir(self, path: Union[PurePath, str] = '.') -> AsyncIterator[SFTPName[str]]: ...

    @overload
    def scandir(self, path: bytes) -> AsyncIterator[SFTPName[bytes]]: ...

    def scandir(self, path: Union[PurePath, str, bytes] = '.') -> AsyncIterator[SFTPName]:
        # ... existing implementation ...
        pass

    # --- readdir ---
    @overload
    async def readdir(self, path: Union[PurePath, str] = '.') -> Sequence[SFTPName[str]]: ...

    @overload
    async def readdir(self, path: bytes) -> Sequence[SFTPName[bytes]]: ...

    async def readdir(self, path: Union[PurePath, str, bytes] = '.') -> Sequence[SFTPName]:
        # ... existing implementation ...
        pass

Benefits of this change:

1. Strict Type Safety: It eliminates false-positive type errors in user code and removes the need for boilerplate type-narrowing when iterating over directories.
2. Better IDE Support: Developers will get immediate, accurate autocompletion for str or bytes depending on what they passed in.
3. Zero Runtime Cost: This is a purely static typing enhancement (can be done in .pyi stubs or inline) and won't affect performance.

[ronf]

Thanks for the suggestion!

Which version of AsyncSSH are you looking at? The latest version has the following signature for listdir, and I think it has been that way for a number of years now.

    async def listdir(self, path: bytes) -> Sequence[bytes]
    async def listdir(self, path: FilePath = ...) -> Sequence[str]

However, as you noted, the scandir and readdir methods are returning SFTPName objects. I believe I tried once or twice to parameterize this type as you show here, but the challenge is that whether we return bytes or str depends on whether the application sets path_encoding when starting up the SFTP client. In some cases, the path argument being str or not is also checked, but we only return a string when both conditions are true, and the type checking can't check a runtime value to determine the type of SFTPName objects being returned.

That said, the code will actually fail if you try to pass a path of type str or PurePath as an input when path_encoding is set to None, requiring that you only pass in bytes values in this case. So, we might be able to get away claiming that the type is only conditional on the path input argument and not path_encoding, but I think I ran into trouble the last time I tried to do that.

There's also an edge case where that doesn't work. The return value of getcwd() depends solely on path_encoding. Since it takes no arguments, there isn't any other indicator about which type to return.

To further complicate things, SFTPName is also used in the SFTP server code, but there all of the path information passed around is always bytes in that case. It may be possible to just always use the bytes variant in that code.

I can give this a try again and see if there's a way to make it work, but I'm not sure what to do for the return type of getcwd() in this case.

[ronf]

I spent a little time on this again today.

As expected, there are a few different issues with trying to make the filename and longname types be variable:

• Currently we assign a default value of '' to filename and longname. We'd potentially need different initial values depending on which variant of SFTPName this was, and I'm not really sure how to do that, especially in the context of the Record class I'm using here. This default value needs to be chosen before we know anything about whether the types should be bytes or str.
• There's code in scandir() which conditionally calls the SFTPClient.decode() function on filename and longname, modifying the values in place by calling SFTPClient.decode(). However, that function sometimes converts bytes to str and sometimes leaves the values as bytes, depending on whether path_encoding was set. I don't see how to do that without the fields remaining BytesOrStr.
• I'm also getting an error I can't explain in SFTPName.decode where it thinks the return value is the str variant of SFTPName, even though in that case the filename and longname will always be bytes.