From f24777c2b329974b69d2a3bf5cfc37e0fcace36c Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Sat, 10 Jul 2021 01:29:29 +0100 Subject: [PATCH 1/2] bpo-44317: Improve tokenizer errors with more informative locations (GH-26555) --- Lib/test/test_exceptions.py | 4 +- .../2021-06-06-00-29-14.bpo-44317.xPPhcZ.rst | 1 + Parser/tokenizer.c | 72 ++++++++++++++----- 3 files changed, 57 insertions(+), 20 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-06-06-00-29-14.bpo-44317.xPPhcZ.rst diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 8f689546a62296..f92637f9930bfd 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -226,9 +226,9 @@ def testSyntaxErrorOffset(self): # Errors thrown by tokenizer.c check('(0x+1)', 1, 3) check('x = 0xI', 1, 6) - check('0010 + 2', 1, 4) + check('0010 + 2', 1, 1) check('x = 32e-+4', 1, 8) - check('x = 0o9', 1, 6) + check('x = 0o9', 1, 7) check('\u03b1 = 0xI', 1, 6) check(b'\xce\xb1 = 0xI', 1, 6) check(b'# -*- coding: iso8859-7 -*-\n\xe1 = 0xI', 2, 6, diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-06-06-00-29-14.bpo-44317.xPPhcZ.rst b/Misc/NEWS.d/next/Core and Builtins/2021-06-06-00-29-14.bpo-44317.xPPhcZ.rst new file mode 100644 index 00000000000000..8ac32adf8b5535 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-06-06-00-29-14.bpo-44317.xPPhcZ.rst @@ -0,0 +1 @@ +Improve tokenizer error with improved locations. Patch by Pablo Galindo. diff --git a/Parser/tokenizer.c b/Parser/tokenizer.c index ba9366402babd3..50a0afc05cb839 100644 --- a/Parser/tokenizer.c +++ b/Parser/tokenizer.c @@ -1071,19 +1071,13 @@ tok_backup(struct tok_state *tok, int c) } } - static int -syntaxerror(struct tok_state *tok, const char *format, ...) +_syntaxerror_range(struct tok_state *tok, const char *format, + int col_offset, int end_col_offset, + va_list vargs) { PyObject *errmsg, *errtext, *args; - va_list vargs; -#ifdef HAVE_STDARG_PROTOTYPES - va_start(vargs, format); -#else - va_start(vargs); -#endif errmsg = PyUnicode_FromFormatV(format, vargs); - va_end(vargs); if (!errmsg) { goto error; } @@ -1093,7 +1087,14 @@ syntaxerror(struct tok_state *tok, const char *format, ...) if (!errtext) { goto error; } - int offset = (int)PyUnicode_GET_LENGTH(errtext); + + if (col_offset == -1) { + col_offset = (int)PyUnicode_GET_LENGTH(errtext); + } + if (end_col_offset == -1) { + end_col_offset = col_offset; + } + Py_ssize_t line_len = strcspn(tok->line_start, "\n"); if (line_len != tok->cur - tok->line_start) { Py_DECREF(errtext); @@ -1104,8 +1105,8 @@ syntaxerror(struct tok_state *tok, const char *format, ...) goto error; } - args = Py_BuildValue("(O(OiiN))", errmsg, - tok->filename, tok->lineno, offset, errtext); + args = Py_BuildValue("(O(OiiNii))", errmsg, tok->filename, tok->lineno, + col_offset, errtext, tok->lineno, end_col_offset); if (args) { PyErr_SetObject(PyExc_SyntaxError, args); Py_DECREF(args); @@ -1117,6 +1118,38 @@ syntaxerror(struct tok_state *tok, const char *format, ...) return ERRORTOKEN; } +static int +syntaxerror(struct tok_state *tok, const char *format, ...) +{ + va_list vargs; +#ifdef HAVE_STDARG_PROTOTYPES + va_start(vargs, format); +#else + va_start(vargs); +#endif + int ret = _syntaxerror_range(tok, format, -1, -1, vargs); + va_end(vargs); + return ret; +} + +static int +syntaxerror_known_range(struct tok_state *tok, + int col_offset, int end_col_offset, + const char *format, ...) +{ + va_list vargs; +#ifdef HAVE_STDARG_PROTOTYPES + va_start(vargs, format); +#else + va_start(vargs); +#endif + int ret = _syntaxerror_range(tok, format, col_offset, end_col_offset, vargs); + va_end(vargs); + return ret; +} + + + static int indenterror(struct tok_state *tok) { @@ -1692,12 +1725,12 @@ tok_get(struct tok_state *tok, const char **p_start, const char **p_end) c = tok_nextc(tok); } if (c < '0' || c >= '8') { - tok_backup(tok, c); if (isdigit(c)) { return syntaxerror(tok, "invalid digit '%c' in octal literal", c); } else { + tok_backup(tok, c); return syntaxerror(tok, "invalid octal literal"); } } @@ -1721,12 +1754,12 @@ tok_get(struct tok_state *tok, const char **p_start, const char **p_end) c = tok_nextc(tok); } if (c != '0' && c != '1') { - tok_backup(tok, c); if (isdigit(c)) { return syntaxerror(tok, "invalid digit '%c' in binary literal", c); } else { + tok_backup(tok, c); return syntaxerror(tok, "invalid binary literal"); } } @@ -1759,6 +1792,7 @@ tok_get(struct tok_state *tok, const char **p_start, const char **p_end) } c = tok_nextc(tok); } + char* zeros_end = tok->cur; if (isdigit(c)) { nonzero = 1; c = tok_decimal_tail(tok); @@ -1779,10 +1813,12 @@ tok_get(struct tok_state *tok, const char **p_start, const char **p_end) else if (nonzero) { /* Old-style octal: now disallowed. */ tok_backup(tok, c); - return syntaxerror(tok, - "leading zeros in decimal integer " - "literals are not permitted; " - "use an 0o prefix for octal integers"); + return syntaxerror_known_range( + tok, (int)(tok->start + 1 - tok->line_start), + (int)(zeros_end - tok->line_start), + "leading zeros in decimal integer " + "literals are not permitted; " + "use an 0o prefix for octal integers"); } if (!verify_end_of_number(tok, c, "decimal")) { return ERRORTOKEN; From 248173cc0483a9ad9261353302f1234cf9eb2ebe Mon Sep 17 00:00:00 2001 From: andrei kulakov Date: Fri, 9 Jul 2021 23:47:41 -0400 Subject: [PATCH 2/2] bpo-43219: shutil.copyfile, raise a less confusing exception instead of IsADirectoryError (GH-27049) Fixes the misleading IsADirectoryError to be FileNotFoundError. --- Lib/shutil.py | 50 +++++++++++-------- Lib/test/test_shutil.py | 9 ++++ .../2021-07-09-07-14-37.bpo-41928.Q1jMrr.rst | 4 ++ 3 files changed, 42 insertions(+), 21 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2021-07-09-07-14-37.bpo-41928.Q1jMrr.rst diff --git a/Lib/shutil.py b/Lib/shutil.py index 1982b1c626e966..2cb5ef848c2781 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -253,28 +253,36 @@ def copyfile(src, dst, *, follow_symlinks=True): if not follow_symlinks and _islink(src): os.symlink(os.readlink(src), dst) else: - with open(src, 'rb') as fsrc, open(dst, 'wb') as fdst: - # macOS - if _HAS_FCOPYFILE: - try: - _fastcopy_fcopyfile(fsrc, fdst, posix._COPYFILE_DATA) - return dst - except _GiveupOnFastCopy: - pass - # Linux - elif _USE_CP_SENDFILE: - try: - _fastcopy_sendfile(fsrc, fdst) + try: + with open(src, 'rb') as fsrc, open(dst, 'wb') as fdst: + # macOS + if _HAS_FCOPYFILE: + try: + _fastcopy_fcopyfile(fsrc, fdst, posix._COPYFILE_DATA) + return dst + except _GiveupOnFastCopy: + pass + # Linux + elif _USE_CP_SENDFILE: + try: + _fastcopy_sendfile(fsrc, fdst) + return dst + except _GiveupOnFastCopy: + pass + # Windows, see: + # https://github.com/python/cpython/pull/7160#discussion_r195405230 + elif _WINDOWS and file_size > 0: + _copyfileobj_readinto(fsrc, fdst, min(file_size, COPY_BUFSIZE)) return dst - except _GiveupOnFastCopy: - pass - # Windows, see: - # https://github.com/python/cpython/pull/7160#discussion_r195405230 - elif _WINDOWS and file_size > 0: - _copyfileobj_readinto(fsrc, fdst, min(file_size, COPY_BUFSIZE)) - return dst - - copyfileobj(fsrc, fdst) + + copyfileobj(fsrc, fdst) + + # Issue 43219, raise a less confusing exception + except IsADirectoryError as e: + if os.path.exists(dst): + raise + else: + raise FileNotFoundError(f'Directory does not exist: {dst}') from e return dst diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 493c7e5f9f1379..e2574253d961f9 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1249,6 +1249,15 @@ def test_copyfile_same_file(self): # Make sure file is not corrupted. self.assertEqual(read_file(src_file), 'foo') + @unittest.skipIf(MACOS or _winapi, 'On MACOS and Windows the errors are not confusing (though different)') + def test_copyfile_nonexistent_dir(self): + # Issue 43219 + src_dir = self.mkdtemp() + src_file = os.path.join(src_dir, 'foo') + dst = os.path.join(src_dir, 'does_not_exist/') + write_file(src_file, 'foo') + self.assertRaises(FileNotFoundError, shutil.copyfile, src_file, dst) + class TestArchives(BaseTest, unittest.TestCase): diff --git a/Misc/NEWS.d/next/Library/2021-07-09-07-14-37.bpo-41928.Q1jMrr.rst b/Misc/NEWS.d/next/Library/2021-07-09-07-14-37.bpo-41928.Q1jMrr.rst new file mode 100644 index 00000000000000..e6bd758980b003 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-07-09-07-14-37.bpo-41928.Q1jMrr.rst @@ -0,0 +1,4 @@ +Update :func:`shutil.copyfile` to raise :exc:`FileNotFoundError` instead of +confusing :exc:`IsADirectoryError` when a path ending with a +:const:`os.path.sep` does not exist; :func:`shutil.copy` and +:func:`shutil.copy2` are also affected.