pallets/markupsafe

Return type for methods covered by `_simple_escaping_wrapper` is wrong

Daverball opened this issue · 1 comments

Currently you are using _simple_escaping_wrapper to extend most of the existing string methods, which is absolutely fine (although the type annotation for the decorator could be better), but the way you are applying the decorator is problematic, since mypy will not be able to infer that the return type for these methods has changed. So whenever you use these methods the type will be downgraded to str from Markup. Luckily most of these methods you are unlikely to use on markup, but replace is one that I've wanted to use on several occasions and had to use type:ignore to fix the typing issues.

html = Markup('<a href="${url}">some link</a>')
reveal_type(html)  # Markup -> OK
still_html = html.replace('${url}', 'http://example.com')
reveal_type(still_html)  # str -> Not OK

The second reveal_type should still be Markup, since _simple_escaping_wrapper preserves markup-safety.

The simple way to fix this would just be to explicitly write out each method explicitly, rather than the hacky loop using locals. It wouldn't even really be that much more verbose than it is right now, in many ways it would actually be more simple to just write:

__getitem__ = _simple_escaping_wrapper(str.__getitem__)
capitalize = _simple_escaping_wrapper(str.capitalize)
title = _simple_escaping_wrapper(str.title)
lower = _simple_escaping_wrapper(str.lower)
upper = _simple_escaping_wrapper(str.upper)
replace = _simple_escaping_wrapper(str.replace)
ljust = _simple_escaping_wrapper(str.ljust)
rjust = _simple_escaping_wrapper(str.rjust)
lstrip = _simple_escaping_wrapper(str.lstrip)
rstrip = _simple_escaping_wrapper(str.rstrip)
center = _simple_escaping_wrapper(str.center)
strip = _simple_escaping_wrapper(str.strip)
translate = _simple_escaping_wrapper(str.translate)
expandtabs = _simple_escaping_wrapper(str.expandtabs)
swapcase = _simple_escaping_wrapper(str.swapcase)
zfill = _simple_escaping_wrapper(str.zfill)

Environment:

  • Python version: 3.10
  • MarkupSafe version: 2.1.2

Of course _simple_escaping_wrapper would need to be rewritten as a proper decorator and could use ParamSpec to preserve the original function's parameters while upgrading the return type from str to Markup.