On Python 3.3+, replace pipes.quote with shlex.quote
The pipes.quote() function was undocumented, and the pipes module was deprecated in Python 3.11 and will be removed in Python 3.13.
Fixes #341.
This is a one-to-one replacement of pipes.quote with shlex.quote; I did not attempt to audit the shell escaping for correctness and safety.
I would suggest to switch to if/else for version detection because that pattern can be automatically cleaned with modern tooling (pyupgrade, ruff), while a try/except cannot (to the best of my knowledge)
See #344
I would suggest to switch to
if/elsefor version detection because that pattern can be automatically cleaned with modern tooling (pyupgrade, ruff), while atry/exceptcannot (to the best of my knowledge)
That’s an interesting point. I tested it to be sure. With pyupgrade --py38-plus nodeenv.py, we would see:
try:
from shlex import quote as _quote # Python 3.3+
except ImportError:
- from pipes import quote as _quote # Python 2.7
+ from shlex import quote as _quote # Python 2.7
import platform
which should point out the need remove the try/except to upgrade to a human reviewer, but doesn’t fully handle it by itself. (This makes sense: there can be a lot of surprising and exotic reasons for an ImportError in general.)
If I switch to:
if sys.version_info < (3, 3):
from pipes import quote as _quote
else:
from shlex import quote as _quote
I get:
import argparse
import subprocess
import tarfile
-if sys.version_info < (3, 3):
- from pipes import quote as _quote
-else:
- from shlex import quote as _quote
+from shlex import quote as _quote
import platform
import zipfile
import shutil
A human reviewer might still choose to change this back to import shlex and call shlex.quote for consistency with the overall style.
I did notice that if I slice version_info, e.g. testing sys.version_info[:2], it breaks pyupgrade’s heuristic and the if/else is not removed.
If ruff knows how to do “upgrades” like this, I don’t know how to ask for it.
Anyway, it seems like this basically supports your suggestion. I’ve added a commit to switch to an if/else for now, but I’m curious what @ekalinin thinks.
If ruff knows how to do “upgrades” like this, I don’t know how to ask for it.
ruff's UP rule set is a reimplementation of pyupgrade, which includes the refactor you just illustrated. The difference is that ruff is also able to read metadata from pyproject.toml, so it "just knows" what refactors can be safely applied.
The checks are broken due to #347, this PR is not the culprit.
Thanks!