Repository navigation
bytes formatting incorrect in python 3 #2070
Description
The following runs properly:
def foo(a: bytes, b:bytes) -> bytes:
return b'%s:%s' % (a, b)
assert foo(b'a', b'b') == b'a:b'
However mypy complains:
example.py: note: In function "foo":
example.py:2: error: Incompatible return value type (got "str", expected "bytes")
Activity
gvanrossum commentedon Aug 31, 2016
Oooh, good catch! I suspect the % checking code for bytes was too hastily adapted from the same for strings. There are many things wrong there: the checker always assumes the full formatting language used for str, while bytes have a separate (weaker) language. It always returns
builtins.stras the return type (which is actually the error you're seeing). And I don't think the checker is invoked for unicode strings in PY2 mode...gvanrossum commentedon Aug 31, 2016
(Also, though not affecting mypy's behavior, typeshed has no
__mod__operator for bytes defined. It's new in Python 3.5, so mypy really should check python_version too.)gvanrossum commentedon Sep 15, 2016
@buckbaskin If you want to tackle this that's great! IIRC it should be a fairly simple refactoring and doing this will get you up to speed with how to run the tests. (Hint:
pip install -r test-requirements.txtand then you can run tests with py.test, see README.md.)buckbaskin commentedon Sep 21, 2016
It looks like the type checking code for bytes is just to use the string version (see checkexpr.py). The lines effectively check to see if it either matches
StrExprorBytesExprand casts toStrExprif it matches either one. This leads to the bug, where the BytesExpr is supposed to stay a BytesExpr.Would the correct way to fix this be to create a new checker for Bytes? Or possibly to add a method to what is now StringFormatterChecker that checks bytes instead?
gvanrossum commentedon Sep 22, 2016
The quick and dirty fix would be to just change the return type to be the same as the arg type. But the bytes % operator supports a smaller mini-language, so technically StringFormatterChecker should grow another method to handle that. Or maybe a subclass? The main thing that ought to change seems to be conversion_type().
But I'll settle for the quick and dirty fix as a first installment!
buckbaskin commentedon Sep 22, 2016
In the pull request, I added a separate method that handles BytesExpr. I don't think that the type checking for what can be interpolated into a
b''is correct (it still uses the logic for a string) but the code example given in the bug no longer has an error.On a parallel thought, is there a place where I could include a test that will check to see that something like the above example passes without error? I ran a manual check before I made the pull request.
gvanrossum commentedon Sep 22, 2016