Skip to content

Commit b1e0635

Browse files
authored
Fix type hints, add mypy to CI, fix as_bytes DKIM bug (#194)
* Fix type hints issues from review 1. Include py.typed in published package: add package_data in setup.py and include in MANIFEST.in (without this, PEP 561 support was not actually working) 2. Widen date callback type from Callable[[], str] to Callable[..., str | datetime | float] to match existing behavior (callbacks can return datetime/float, not just str) 3. Fix SMTPResponse annotations: esmtp_opts and rcpt_options are lists (not str), and rename refused_recipient to refused_recipients to match actual usage in client.py * Add mypy type checking to CI Configure mypy in setup.cfg with per-module overrides: - emails.message: suppress mixin pattern false positives - emails.packages: ignore vendored DKIM code - emails.backend.smtp.client: private smtplib attrs - emails.template, emails.django: optional deps Add targeted type: ignore comments for known safe patterns. Add tox typecheck environment and CI job. mypy now passes clean on 26 source files. * Fix mypy CI: add per-module ignore for requests stubs The global ignore_missing_imports does not suppress import-untyped for inline imports. Add explicit per-module override for requests. * Remove redundant type annotations on property-backed attributes The uri, filename, and data attributes are defined as properties via get/set + property(). The type annotation in __init__ conflicted with the property descriptor, causing mypy no-redef errors. Removing the annotation lets mypy infer the type from the property. * Replace getattr/hasattr with direct access and isinstance in BaseFile.get_data Use self._data directly (type known from setter) and isinstance checks that mypy can narrow, removing type: ignore comments. * Reduce type: ignore comments from 26 to 14 Replace suppressions with proper fixes where possible: - Use isinstance checks instead of hasattr for type narrowing - Use assert for None-safety where input guarantees non-None - Access self._data directly instead of getattr - Remove redundant to_unicode calls on already-str values - Use explicit if/else instead of tricky and/or expressions - Use dict[] instead of .get() where key is guaranteed present Remaining 14 ignores are genuine mypy limitations: vendored dkim API, private stdlib attrs, intentional method overrides, and decorator typing. * Add tests for changed logic, fix as_bytes DKIM signing bug New tests: - BaseFile.get_data() with str, bytes, IO, and None - SMTPResponse: defaults, set_status, success, refused_recipients - Message.as_bytes() with DKIM signing The as_bytes test uncovered a bug: as_bytes() called sign_string() (expects str) instead of sign_bytes() (expects bytes), causing TypeError when DKIM signing was enabled. Fixed. * Fix SMTPResponse.status_text type: str → bytes smtplib.SMTP.mail/rcpt/data return (int, bytes), so status_text and the text parameter of set_status() should be bytes, not str. * Reduce type: ignore from 14 to 5 - store/file.py: handle None payload explicitly, use get_mime_type() instead of super().mime_type, narrow LazyHTTPFile.get_data types - signers.py: normalize privkey to bytes before parsing, use cast(bytes, ...) for vendored dkim.sign, rewrite sign methods with explicit if/else - backend/smtp/client.py: add return type annotation for sendmail, add from __future__ import annotations - backend/smtp/backend.py: remove no-any-return ignore (now typed via client.py), assert sendmail result is not None - utils.py: add overloads for to_unicode, use cast(F, wrapper) for renderable decorator, build decode_header result with explicit loop and assert Remaining 5 ignores are private API access (msg._headers, email.utils internals), MIMEMixin inheritance, and a dead code path in to_unicode. * Simplify LazyHTTPFile.get_data control flow * Simplify decode_header: remove to_native/to_unicode wrappers Use direct isinstance check and bytes.decode() instead of to_native/to_unicode. Clearer control flow, no type: ignore needed. * Remove allow_none_charset from to_unicode Dead code path: never called with allow_none_charset=True anywhere in the codebase. It also violated its own return type (returned bytes while promising str | None). Removing it simplifies the function and eliminates the last type: ignore in to_unicode. * Remove to_native/to_unicode calls from message, signers, utils These were Python 2 compatibility helpers. On Python 3: - to_native(s) where s is str → noop, removed - to_native(s) where s is bytes → replaced with s.decode() - to_unicode(s) where s is str → noop, removed - to_unicode(value) for non-str → replaced with explicit bytes.decode() / str() branching Functions kept in utils.py for now (still used by loader/). * Remove to_bytes calls from signers.py All inputs are known str — use .encode() directly instead of the Python 2 compatibility wrapper. signers.py no longer imports anything from utils. * Remove assert in _send, let None propagate honestly _send() can return None when client.sendmail() gets empty to_addrs. Instead of asserting this away, widen the return type to SMTPResponse | None and propagate through retry_on_disconnect and sendmail. * Replace cast with typed variable for dkim.sign result
1 parent 0628c3c commit b1e0635

17 files changed

Lines changed: 238 additions & 64 deletions

File tree

.github/workflows/tests.yaml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,24 @@ jobs:
3636
- name: run tests
3737
run: tox -e ${{ matrix.tox }} -- -m "not e2e"
3838

39+
typecheck:
40+
name: "typecheck"
41+
runs-on: ubuntu-latest
42+
steps:
43+
- uses: actions/checkout@v4
44+
- uses: actions/setup-python@v5
45+
with:
46+
python-version: '3.12'
47+
cache: pip
48+
- name: update pip
49+
run: |
50+
pip install -U wheel
51+
pip install -U setuptools
52+
python -m pip install -U pip
53+
- run: pip install tox
54+
- name: run mypy
55+
run: tox -e typecheck
56+
3957
e2e:
4058
name: "e2e / ${{ matrix.name }}"
4159
runs-on: ${{ matrix.os }}

MANIFEST.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
include README.rst LICENSE requirements.txt
2+
include emails/py.typed

emails/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
__license__ = 'Apache 2.0'
4343
__copyright__ = 'Copyright 2013-2026 Sergey Lavrinenko'
4444

45-
USER_AGENT = 'python-emails/%s' % __version__
45+
USER_AGENT: str = 'python-emails/%s' % __version__
4646

4747
from .message import Message, html
4848
from .utils import MessageID

emails/backend/response.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,15 @@ def __init__(self, exception: Exception | None = None, backend: Any = None) -> N
3737

3838
self.responses: list[list] = []
3939

40-
self.esmtp_opts: str | None = None
41-
self.rcpt_options: str | None = None
40+
self.esmtp_opts: list[str] | None = None
41+
self.rcpt_options: list[str] | None = None
4242

4343
self.status_code: int | None = None
44-
self.status_text: str | None = None
44+
self.status_text: bytes | None = None
4545
self.last_command: str | None = None
46-
self.refused_recipient: dict[str, tuple[int, str]] = {}
46+
self.refused_recipients: dict[str, tuple[int, bytes]] = {}
4747

48-
def set_status(self, command: str, code: int, text: str, **kwargs: Any) -> None:
48+
def set_status(self, command: str, code: int, text: bytes, **kwargs: Any) -> None:
4949
self.responses.append([command, code, text, kwargs])
5050
self.status_code = code
5151
self.status_text = text

emails/backend/smtp/backend.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class SMTPBackend:
3434
def __init__(self, ssl: bool = False, fail_silently: bool = True,
3535
mail_options: list[str] | None = None, **kwargs: Any) -> None:
3636

37-
self.smtp_cls = ssl and self.connection_ssl_cls or self.connection_cls
37+
self.smtp_cls = self.connection_ssl_cls if ssl else self.connection_cls
3838

3939
self.ssl = ssl
4040
self.tls = kwargs.get('tls')
@@ -50,7 +50,7 @@ def __init__(self, ssl: bool = False, fail_silently: bool = True,
5050
self.smtp_cls_kwargs = kwargs
5151

5252
self.host: str | None = kwargs.get('host')
53-
self.port: int = kwargs.get('port')
53+
self.port: int = kwargs['port'] # always set as int two lines above
5454
self.fail_silently = fail_silently
5555
self.mail_options = mail_options or []
5656

@@ -80,9 +80,9 @@ def close(self) -> None:
8080
def make_response(self, exception: Exception | None = None) -> SMTPResponse:
8181
return self.response_cls(backend=self, exception=exception)
8282

83-
def retry_on_disconnect(self, func: Callable[..., SMTPResponse]) -> Callable[..., SMTPResponse]:
83+
def retry_on_disconnect(self, func: Callable[..., SMTPResponse | None]) -> Callable[..., SMTPResponse | None]:
8484
@wraps(func)
85-
def wrapper(*args: Any, **kwargs: Any) -> SMTPResponse:
85+
def wrapper(*args: Any, **kwargs: Any) -> SMTPResponse | None:
8686
try:
8787
return func(*args, **kwargs)
8888
except smtplib.SMTPServerDisconnected:
@@ -92,7 +92,7 @@ def wrapper(*args: Any, **kwargs: Any) -> SMTPResponse:
9292
return func(*args, **kwargs)
9393
return wrapper
9494

95-
def _send(self, **kwargs: Any) -> SMTPResponse:
95+
def _send(self, **kwargs: Any) -> SMTPResponse | None:
9696

9797
response = None
9898
try:
@@ -131,7 +131,7 @@ def sendmail(self, from_addr: str, to_addrs: str | list[str],
131131
mail_options=mail_options or self.mail_options,
132132
rcpt_options=rcpt_options)
133133

134-
if not self.fail_silently:
134+
if response and not self.fail_silently:
135135
response.raise_if_needed()
136136

137137
return response

emails/backend/smtp/client.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
# encoding: utf-8
2+
from __future__ import annotations
3+
24
__all__ = ['SMTPClientWithResponse', 'SMTPClientWithResponse_SSL']
35

46
import smtplib
5-
from smtplib import _have_ssl, SMTP
7+
from smtplib import _have_ssl, SMTP # noqa: private API
68
import logging
7-
from ... utils import sanitize_email
9+
from ..response import SMTPResponse
10+
from ...utils import sanitize_email
811

912
logger = logging.getLogger(__name__)
1013

@@ -55,7 +58,9 @@ def _rset(self):
5558
except smtplib.SMTPServerDisconnected:
5659
pass
5760

58-
def sendmail(self, from_addr, to_addrs, msg, mail_options=None, rcpt_options=None):
61+
def sendmail(self, from_addr: str, to_addrs: list[str] | str,
62+
msg: bytes, mail_options: list[str] | None = None,
63+
rcpt_options: list[str] | None = None) -> SMTPResponse | None:
5964

6065
if not to_addrs:
6166
return None

emails/message.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from email.utils import getaddresses
77
from typing import Any, IO
88

9-
from .utils import (formataddr, to_unicode, to_native,
9+
from .utils import (formataddr,
1010
SafeMIMEText, SafeMIMEMultipart, sanitize_address,
1111
parse_name_and_email, load_email_charsets,
1212
encode_header as encode_header_,
@@ -39,7 +39,7 @@ class BaseMessage:
3939
def __init__(self,
4040
charset: str | None = None,
4141
message_id: str | MessageID | bool | None = None,
42-
date: str | datetime | float | bool | Callable[[], str] | None = None,
42+
date: str | datetime | float | bool | Callable[..., str | datetime | float] | None = None,
4343
subject: str | None = None,
4444
mail_from: _Address = None,
4545
mail_to: _AddressList = None,
@@ -158,7 +158,7 @@ def get_subject(self) -> str | None:
158158
def render(self, **kwargs: Any) -> None:
159159
self.render_data = kwargs
160160

161-
def set_date(self, value: str | datetime | float | bool | Callable[[], str] | None) -> None:
161+
def set_date(self, value: str | datetime | float | bool | Callable[..., str | datetime | float] | None) -> None:
162162
self._date = value
163163

164164
def get_date(self) -> str | None:
@@ -231,7 +231,7 @@ def set_header(self, msg: SafeMIMEMultipart, key: str,
231231
return
232232

233233
if not isinstance(value, str):
234-
value = to_unicode(value)
234+
value = value.decode() if isinstance(value, bytes) else str(value)
235235

236236
# Prevent header injection
237237
if '\n' in value or '\r' in value:
@@ -280,13 +280,15 @@ def _build_html_part(self) -> SafeMIMEText | None:
280280
p = SafeMIMEText(text, 'html', charset=self.charset)
281281
p.set_charset(self.charset)
282282
return p
283+
return None
283284

284285
def _build_text_part(self) -> SafeMIMEText | None:
285286
text = self.text_body
286287
if text:
287288
p = SafeMIMEText(text, 'plain', charset=self.charset)
288289
p.set_charset(self.charset)
289290
return p
291+
return None
290292

291293
def build_message(self, message_cls: type | None = None) -> SafeMIMEMultipart:
292294

@@ -343,7 +345,7 @@ def as_string(self, message_cls: type | None = None) -> str:
343345
Note: this method costs one less message-to-string conversions
344346
for dkim in compare to self.as_message().as_string()
345347
"""
346-
r = to_native(self.build_message(message_cls=message_cls).as_string())
348+
r = self.build_message(message_cls=message_cls).as_string()
347349
if self._signer:
348350
r = self.sign_string(r)
349351
return r
@@ -354,7 +356,7 @@ def as_bytes(self, message_cls: type | None = None) -> bytes:
354356
"""
355357
r = self.build_message(message_cls=message_cls).as_bytes()
356358
if self._signer:
357-
r = self.sign_string(r)
359+
r = self.sign_bytes(r)
358360
return r
359361

360362

emails/signers.py

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from .packages import dkim
1010
from .packages.dkim import DKIMException, UnparsableKeyError
1111
from .packages.dkim.crypto import parse_pem_private_key
12-
from .utils import to_bytes, to_native
1312

1413

1514
class DKIMSigner:
@@ -28,27 +27,32 @@ def __init__(self, selector: str, domain: str, key: str | bytes | IO[bytes] | No
2827
if privkey and hasattr(privkey, 'read'):
2928
privkey = privkey.read()
3029

30+
# Normalize to bytes before parsing
31+
privkey_bytes = privkey if isinstance(privkey, bytes) else str(privkey).encode()
32+
3133
# Compile private key
3234
try:
33-
privkey = parse_pem_private_key(to_bytes(privkey))
35+
privkey_parsed = parse_pem_private_key(privkey_bytes)
3436
except UnparsableKeyError as exc:
3537
raise DKIMException(exc)
3638

37-
self._sign_params.update({'privkey': privkey,
38-
'domain': to_bytes(domain),
39-
'selector': to_bytes(selector)})
39+
self._sign_params.update({'privkey': privkey_parsed,
40+
'domain': domain.encode(),
41+
'selector': selector.encode()})
4042

4143
def get_sign_string(self, message: bytes) -> bytes | None:
4244
try:
4345
# pydkim module parses message and privkey on each signing
4446
# this is not optimal for mass operations
4547
# TODO: patch pydkim or use another signing module
46-
return dkim.sign(message=message, **self._sign_params)
48+
result: bytes = dkim.sign(message=message, **self._sign_params)
49+
return result
4750
except DKIMException:
4851
if self.ignore_sign_errors:
4952
logging.exception('Error signing message')
5053
else:
5154
raise
55+
return None
5256

5357
def get_sign_bytes(self, message: bytes) -> bytes | None:
5458
return self.get_sign_string(message)
@@ -57,10 +61,11 @@ def get_sign_header(self, message: bytes) -> tuple[str, str] | None:
5761
# pydkim returns string, so we should split
5862
s = self.get_sign_string(message)
5963
if s:
60-
(header, value) = to_native(s).split(': ', 1)
64+
(header, value) = s.decode().split(': ', 1)
6165
if value.endswith("\r\n"):
6266
value = value[:-2]
6367
return header, value
68+
return None
6469

6570
def sign_message(self, msg: MIMEMultipart) -> MIMEMultipart:
6671
"""
@@ -71,9 +76,9 @@ def sign_message(self, msg: MIMEMultipart) -> MIMEMultipart:
7176
# but py3 smtplib requires str to send DATA command (#
7277
# so we have to convert msg.as_string
7378

74-
dkim_header = self.get_sign_header(to_bytes(msg.as_string()))
79+
dkim_header = self.get_sign_header(msg.as_string().encode())
7580
if dkim_header:
76-
msg._headers.insert(0, dkim_header)
81+
msg._headers.insert(0, dkim_header) # type: ignore[attr-defined]
7782
return msg
7883

7984
def sign_message_string(self, message_string: str) -> str:
@@ -85,12 +90,16 @@ def sign_message_string(self, message_string: str) -> str:
8590
# but py3 smtplib requires str to send DATA command
8691
# so we have to convert message_string
8792

88-
s = self.get_sign_string(to_bytes(message_string))
89-
return s and to_native(s) + message_string or message_string
93+
s = self.get_sign_string(message_string.encode())
94+
if s:
95+
return s.decode() + message_string
96+
return message_string
9097

9198
def sign_message_bytes(self, message_bytes: bytes) -> bytes:
9299
"""
93100
Insert DKIM header to message bytes
94101
"""
95102
s = self.get_sign_bytes(message_bytes)
96-
return s and to_bytes(s) + message_bytes or message_bytes
103+
if s:
104+
return s + message_bytes
105+
return message_bytes

emails/store/file.py

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,18 @@ class BaseFile:
2929
Store base "attachment-file" information.
3030
"""
3131

32+
_data: bytes | str | IO[bytes] | None
33+
3234
def __init__(self, **kwargs: Any) -> None:
3335
"""
3436
uri and filename are connected properties.
3537
if no filename set, filename extracted from uri.
3638
if no uri, but filename set, then uri==filename
3739
"""
38-
self.uri: str | None = kwargs.get('uri', None)
40+
self.uri = kwargs.get('uri', None)
3941
self.absolute_url: str | None = kwargs.get('absolute_url', None) or self.uri
40-
self.filename: str | None = kwargs.get('filename', None)
41-
self.data: bytes | str | IO[bytes] | None = kwargs.get('data', None)
42+
self.filename = kwargs.get('filename', None)
43+
self.data = kwargs.get('data', None)
4244
self._mime_type: str | None = kwargs.get('mime_type')
4345
self._headers: dict[str, str] = kwargs.get('headers', {})
4446
self._content_id: str | None = kwargs.get('content_id')
@@ -52,13 +54,13 @@ def as_dict(self, fields: tuple[str, ...] | None = None) -> dict[str, Any]:
5254
return dict([(k, getattr(self, k)) for k in fields])
5355

5456
def get_data(self) -> bytes | str | None:
55-
_data = getattr(self, '_data', None)
56-
if isinstance(_data, str):
57+
_data = self._data
58+
if isinstance(_data, (str, bytes)):
5759
return _data
58-
elif hasattr(_data, 'read'):
59-
return _data.read()
60+
elif _data is None:
61+
return None
6062
else:
61-
return _data
63+
return _data.read()
6264

6365
def set_data(self, value: bytes | str | IO[bytes] | None) -> None:
6466
self._data = value
@@ -142,7 +144,8 @@ def mime(self) -> MIMEBase | None:
142144
if p is None:
143145
filename_header = encode_header(self.filename)
144146
p = MIMEBase(*self.mime_type.split('/', 1), name=filename_header)
145-
p.set_payload(to_bytes(self.data))
147+
payload = to_bytes(self.data) or b''
148+
p.set_payload(payload)
146149
encode_base64(p)
147150
if 'content-disposition' not in self._headers:
148151
p.add_header('Content-Disposition', self.content_disposition, filename=filename_header)
@@ -186,7 +189,12 @@ def fetch(self) -> None:
186189

187190
def get_data(self) -> bytes | str:
188191
self.fetch()
189-
return self._data or ''
192+
data = self._data
193+
if data is None:
194+
return ''
195+
if isinstance(data, (str, bytes)):
196+
return data
197+
return data.read()
190198

191199
def set_data(self, v: bytes | str | IO[bytes] | None) -> None:
192200
self._data = v
@@ -196,7 +204,7 @@ def set_data(self, v: bytes | str | IO[bytes] | None) -> None:
196204
@property
197205
def mime_type(self) -> str:
198206
self.fetch()
199-
return super(LazyHTTPFile, self).mime_type
207+
return self.get_mime_type()
200208

201209
@property
202210
def headers(self) -> dict[str, str]:

emails/store/store.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ def remove(self, uri: BaseFile | str) -> None:
5454
del self._filenames[filename]
5555
del self._files[uri]
5656

57-
def unique_filename(self, filename: str | None, uri: str | None = None) -> str:
57+
def unique_filename(self, filename: str | None, uri: str | None = None) -> str | None:
5858

5959
if filename in self._filenames:
6060
n = 1
@@ -66,7 +66,8 @@ def unique_filename(self, filename: str | None, uri: str | None = None) -> str:
6666
if filename not in self._filenames:
6767
break
6868

69-
self._filenames[filename] = uri
69+
if filename is not None:
70+
self._filenames[filename] = uri
7071

7172
return filename
7273

@@ -94,6 +95,7 @@ def by_filename(self, filename: str) -> BaseFile | None:
9495
uri = self._filenames.get(filename)
9596
if uri:
9697
return self.by_uri(uri)
98+
return None
9799

98100
def __getitem__(self, uri: str) -> BaseFile | None:
99101
return self.by_uri(uri) or self.by_filename(uri)

0 commit comments

Comments
 (0)