8000 bpo-32912: Upgrade warning for invalid escape sequences from silent to non-silent by Vgr255 · Pull Request #5849 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-32912: Upgrade warning for invalid escape sequences from silent to non-silent #5849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

bpo-32912: Upgrade warning for invalid escape sequences from silent to non-silent #5849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions Lib/test/test_codecs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1210,15 +1210,15 @@ def test_escape(self):
for i in range(97, 123):
b = bytes([i])
if b not in b'abfnrtvx':
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
check(b"\\" + b, b"\\" + b)
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
check(b"\\" + b.upper(), b"\\" + b.upper())
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
check(br"\8", b"\\8")
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
check(br"\9", b"\\9")
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
check(b"\\\xfa", b"\\\xfa")

def test_errors(self):
Expand Down Expand Up @@ -2482,16 +2482,16 @@ def test_escape_decode(self):
for i in range(97, 123):
b = bytes([i])
if b not in b'abfnrtuvx':
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
check(b"\\" + b, "\\" + chr(i))
if b.upper() not in b'UN':
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
check(b"\\" + b.upper(), "\\" + chr(i-32))
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
check(br"\8", "\\8")
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
check(br"\9", "\\9")
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
check(b"\\\xfa", "\\\xfa")

def test_decode_errors(self):
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_fstring.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ def test_backslashes_in_string_part(self):
self.assertEqual(f'2\x203', '2 3')
self.assertEqual(f'\x203', ' 3')

with self.assertWarns(DeprecationWarning): # invalid escape sequence
with self.assertWarns(SyntaxWarning): # invalid escape sequence
value = eval(r"f'\{6*7}'")
self.assertEqual(value, '\\42')
self.assertEqual(f'\\{6*7}', '\\42')
Expand Down
12 changes: 6 additions & 6 deletions Lib/test/test_string_literals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,18 @@ def test_eval_str_invalid_escape(self):
for b in range(1, 128):
if b in b"""\n\r"'01234567NU\\abfnrtuvx""":
continue
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
self.assertEqual(eval(r"'\%c'" % b), '\\' + chr(b))

with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always', category=DeprecationWarning)
warnings.simplefilter('always', category=SyntaxWarning)
eval("'''\n\\z'''")
self.assertEqual(len(w), 1)
self.assertEqual(w[0].filename, '<string>')
self.assertEqual(w[0].lineno, 2)

with warnings.catch_warnings(record=True) as w:
warnings.simplefilter 8000 ('error', category=DeprecationWarning)
warnings.simplefilter('error', category=SyntaxWarning)
with self.assertRaises(SyntaxError) as cm:
eval("'''\n\\z'''")
exc = cm.exception
Expand Down Expand Up @@ -158,18 +158,18 @@ def test_eval_bytes_invalid_escape(self):
for b in range(1, 128):
if b in b"""\n\r"'01234567\\abfnrtvx""":
continue
with self.assertWarns(DeprecationWarning):
with self.assertWarns(SyntaxWarning):
self.assertEqual(eval(r"b'\%c'" % b), b'\\' + bytes([b]))

with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always', category=DeprecationWarning)
warnings.simplefilter('always', category=SyntaxWarning)
eval("b'''\n\\z'''")
self.assertEqual(len(w), 1)
self.assertEqual(w[0].filename, '<string>')
self.assertEqual(w[0].lineno, 2)

with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('error', category=DeprecationWarning)
warnings.simplefilter('error', category=SyntaxWarning)
with self.assertRaises(SyntaxError) as cm:
eval("b'''\n\\z'''")
exc = cm.exception
Expand Down
2 changes: 1 addition & 1 deletion Objects/bytesobject.c
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ PyObject *PyBytes_DecodeEscape(const char *s,
if (result == NULL)
return NULL;
if (first_invalid_escape != NULL) {
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
if (PyErr_WarnFormat(PyExc_SyntaxWarning, 1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not related to syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This warning was added back in 3.6 when we started deprecating the invalid escapes. In 3.8, we're upgrading these to SyntaxWarning so that they're no longer silent by default, and it will eventually be a SyntaxError. The end goal here is to raise a SyntaxError for this use, but this will have to wait for two more releases.

Copy link
Member
8000

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this warning is emitted by codecs.escape_decode() (which is used in pickle).

>>> codecs.escape_decode(b'\\z')
__main__:1: DeprecationWarning: invalid escape sequence '\z'
(b'\\z', 2)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and this warning was added as part of bpo-27364 to keep consistency with the rest of the language. It only makes sense to upgrade this warning along with everything else, or else the codecs module falls out of sync with the rest of the language.

"invalid escape sequence '\\%c'",
(unsigned char)*first_invalid_escape) < 0) {
Py_DECREF(result);
Expand Down
2 changes: 1 addition & 1 deletion Objects/unicodeobject.c
View file
Open in desktop
6D40
Original file line number Diff line number Diff line change
Expand Up @@ -6109,7 +6109,7 @@ PyUnicode_DecodeUnicodeEscape(const char *s,
if (result == NULL)
return NULL;
if (first_invalid_escape != NULL) {
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
if (PyErr_WarnFormat(PyExc_SyntaxWarning, 1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not related to syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This warning was added back in 3.6 when we started deprecating the invalid escapes. In 3.8, we're upgrading these to SyntaxWarning so that they're no longer silent by default, and it will eventually be a SyntaxError. The end goal here is to raise a SyntaxError for this use, but this will have to wait for two more releases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This warning is emitted by the "unicode-escape" codec.

>>> b'\\z'.decode('unicode-escape')
__main__:1: DeprecationWarning: invalid escape sequence '\z'
'\\z'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and this warning was added as part of bpo-27364 to keep consistency with the rest of the language. It only makes sense to upgrade this warning along with everything else, or else this codec falls out of sync with the rest of the language.

"invalid escape sequence '\\%c'",
(unsigned char)*first_invalid_escape) < 0) {
Py_DECREF(result);
Expand Down
4 changes: 2 additions & 2 deletions Python/ast.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4158,11 +4158,11 @@ warn_invalid_escape_sequence(struct compiling *c, const node *n,
if (msg == NULL) {
return -1;
}
ABE9 if (PyErr_WarnExplicitObject(PyExc_DeprecationWarning, msg,
if (PyErr_WarnExplicitObject(PyExc_SyntaxWarning, msg,
c->c_filename, LINENO(n),
NULL, NULL) < 0)
{
if (PyErr_ExceptionMatches(PyExc_DeprecationWarning)) {
if (PyErr_ExceptionMatches(PyExc_SyntaxWarning)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the end goal here is to get a SyntaxError, however this is not yet possible as we have to wait for two releases before this can happen, so we can't yet convert it to a SyntaxError. I probably should update that comment, though (I'll do that tomorrow when I get home).

const char *s;

/* Replace the DeprecationWarning exception with a SyntaxError
Expand Down
0