
On Tue, Feb 1, 2022 at 10:50 PM David Foster <davidfstr@gmail.com> wrote:
This proposal is looking really good. Just a few nits from me:
# Backwards Compatibility
As PEP 586 mentions, type checkers "should feel free to experiment with more sophisticated inference techniques". So, if the type checker infers a literal string type for an unannotated variable that is initialized with a literal string, the following example should be OK:
``` x = "hello" expect_literal_string(x) # OK, because x is inferred to have type ``Literal["hello"]``. ```
Nit: In this example I don't see how a typechecker could reasonably infer x is of type `Literal["hello"]` when `expect_literal_string` has signature `Callable[[LiteralString], None]`.
Inferring the type for `x` as `Literal["hello"]` after `x = "hello"` is sound. Pyre and Pyright do in fact infer it that way. PEP 586 leaves type checkers free to infer precise literal types as long as it isn't "disruptive":
For example, given the statement x = "blue", should the inferred type of x be str or Literal["blue"]? ... This is not the only viable strategy: type checkers should feel free to experiment with more sophisticated inference techniques. This PEP does not mandate any particular strategy; it only emphasizes the importance of backwards compatibility. -- https://www.python.org/dev/peps/pep-0586/#backwards-compatibility
Note that inferring `Literal["hello"]` does not cause a type error for `expect_literal_string(x)` since `Literal["hello"]` is compatible with the expected type `LiteralString`.
# Runtime Behavior We propose an implementation for typing.LiteralString similar to that for typing.Self from PEP 673.
Nit: Would be nice to link the text "PEP 673" to the specific section in that PEP it references: https://www.python.org/dev/peps/pep-0673/#runtime-behavior
New reST syntax was added recently to the PEPs repo to support linking to specific sections of PEPs. Something like: :pep:`PEP 673 <673#runtime-behavior>`
Nit: Since this PEP refers to another PEP within the same Python release (i.e. PEP 673), it should probably add the header field "Requires: 673".
Hmm... this PEP doesn't really depend on that PEP. I can just remove the dependency instead of adding a misleading "Requires" heading.
If you want to mention some more prior art RE marking strings as "safe"/"unsafe" at runtime/statically, Perl has had the concept of "tainted" strings (and other values) for a long time. [2]
Thanks, I'll mention it. (Took me quite a while to understand that Perl man page! Thank goodness we have Python :) ).
[1]:
https://mail.python.org/archives/list/typing-sig@python.org/message/JTO3WKRK... [2]: https://metacpan.org/pod/Taint
-- David Foster | Seattle, WA, USA Contributor to TypedDict support for mypy
-- S Pradeep Kumar