Skip to content

Type mismatch in email.charset.Charset.body_encode #10429

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

Closed
cushionbadak opened this issue Jul 10, 2023 · 1 comment · Fixed by #10437
Closed

Type mismatch in email.charset.Charset.body_encode #10429

cushionbadak opened this issue Jul 10, 2023 · 1 comment · Fixed by #10437
Labels
stubs: false positive Type checkers report false errors

Comments

@cushionbadak
Copy link
Contributor

Summary

Modification Target: Charset.body_encode in stdlib/email/charset.pyi
Current type hint: (self, string: str) -> str
Proposed type hint: (self, string: Optional[str]) -> Optional[str]

See the existing implementation here: charset.pyi#L21

Description

The method Charset.body_encode can return None if it's invoked with None as the string parameter.

    def body_encode(self, string):
        if not string:
            return string
        ...

Although the type hint currently does not allow for None to be passed as an argument, there's a mismatch with how it's used in the Message.set_charset method from the standard library's email/message.py, which can pass None to body_encode. This is because the Message class's attribute self._payload is initially set to None.

            self._payload = charset.body_encode(self._payload)
class Message:
    def __init__(self, policy=compat32):
        self._payload = None
        ...

The mismatch (passing None as an argument) was discovered in the CPython email unit test, TestMessageAPI.test_getset_charset, found in test/test_email/test_email.py. The minimized example below demonstrates the type mismatch:

    def test_getset_charset(self):
        msg = Message()
        charset = Charset('iso-8859-1')
        msg.set_charset(charset)
        ...

P.S. All CPython links refer to commit hash 0481b8... from the current CPython 3.12 branch.

@srittau
Copy link
Collaborator

srittau commented Jul 10, 2023

Thanks! I believe our best option is to use an overload for the None -> None case. Just changing the return type without an overload would be unergonomic (and unnecessary) for what is most likely the common case: Passing in a str. So something like:

    @overload
    def body_encode(self, string: None) -> None: ...
    @overload
    def body_encode(self, string: str) -> str: ...

PR welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: false positive Type checkers report false errors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants