Forbid shlex.quote returning a quoted empty string (`”””`) for falsey non-`str` data

[jb2170]

Bug report

Bug description:

Full discussion and history of the bug is on discuss.python.org.

tldr:

shlex.quote's opening check for the empty string as a special case is still placed before the isinstance type check. This is intentional as when the isinstance check was added, the patch was intended to not break compatibility with eg people expecting shlex.quote(None) to return "''", implicitly coercing None to the empty string "" and then quoting it as "''".

def quote(s):
    if not s:
        return "''"

    if not isinstance(s, str):
        raise TypeError(f"expected string object, got {type(s).__name__!r}")
    ...

We think either of the following is a best course of action:

• Print a DeprecationWarning when one calls quote on falsey non-str data
• Leave the function alone, as people may now be relying on this as a feature, not a bug, and we don't gain much by doing a patch.

Or we could do a hard-break and just swap the order of the isinstance and if not s checks around, which would break anybody relying on None -> "''" (not recommended!).

---

I have a couple of git tags that I will link below with reference implementation for 'fixing' this issue (if it needs to be fixed). I just need the gh- number of this issue first for the blurb entries.

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

[picnixz]

I think I was the one who suggested raising the TypeError explicitly because I wanted to prevent exceptions types changing, not because I wanted to preserve the None -> "" behavior (if I said so, though, I was wrong then: ideally, we should avoid changing raised exceptions).

I can think of people doing list(map(shlex.quote, ...)) and by chance, there is an item that is an empty list, and instead of having, say, no argument at all, you end up with some quoted argument. So, I would suggest deprecating non-string because the docs say:

Return a shell-escaped version of the string s. The returned value is a string that can safely be used as one token in a shell command line, for cases where you cannot use a list.

[jb2170]

Yeah I guess the explicit raising of TypeError, implemented in #138809 (addressing #138804), was the correct thing to do to keep things consistent, but it leads to a weird situation where shlex.quote looks like it is intended to work for falsey non-str data, eg None. It's worked for falsey data since before and after both #138809 (raise TypeError) and #132036 (PR which caused AttributeError etc instead of TypeError).

The manual raising of TypeError was just about making the type of exception raised for data of an incorrect type consistent between versions. The behavior of quote for falsey non-str data is just an artefact of not wanting to break compatibility, not purposely trying to permit a special case.

It was this review post of #138809 that decided to not raise TypeError for falsey data as it would be a breaking change, but it does make one think: now that we have the type check, should it come before the special case, and deprecate falsey non-str data being treated nicely? I can take it or leave it 😅 :

I gave a couple of examples in the discussion of why we may or may not want to treat None -> "''" as a feature rather than a bug lest we break existing workflows, albeit that we don't currently and shouldn't in the future encourage it in the docs.

---

For the record:

Tag and comparison for the deprecation route.

Tag and comparison for the hard-break route, though I wouldn't recommend this version; we're better deprecating than breaking. The deprecation tag is based off of this one.