From 3ec02c814d7e03fbc8cd8c098d86d09f9c6dbb23 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Sun, 25 Jun 2023 14:52:57 +0100 Subject: [PATCH 1/4] gh-106046: Improve error message from `os.fspath` if `__fspath__` is set to `None` --- Lib/os.py | 6 ++++ Lib/test/test_os.py | 35 +++++++++++++++++++ ...-06-23-22-52-24.gh-issue-106046.OdLiLJ.rst | 2 ++ Modules/posixmodule.c | 14 ++++---- 4 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-06-23-22-52-24.gh-issue-106046.OdLiLJ.rst diff --git a/Lib/os.py b/Lib/os.py index 31b957f13215d5..d8c9ba4b15400a 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -1061,6 +1061,12 @@ def _fspath(path): else: raise TypeError("expected str, bytes or os.PathLike object, " "not " + path_type.__name__) + except TypeError: + if path_type.__fspath__ is None: + raise TypeError("expected str, bytes or os.PathLike object, " + "not " + path_type.__name__) from None + else: + raise if isinstance(path_repr, (str, bytes)): return path_repr else: diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 8de4ef7270b754..a29806fae8ab54 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -4647,6 +4647,41 @@ def __fspath__(self): return '' self.assertFalse(hasattr(A(), '__dict__')) + def test_fspath_set_to_None(self): + class Foo: + __fspath__ = None + + class Bar: + def __fspath__(self): + return 'bar' + + class Baz(Bar): + __fspath__ = None + + good_error_msg = ( + r"((expected)|(should be)) str, bytes or os.PathLike( object)?, not {}".format + ) + + with self.assertRaisesRegex(TypeError, good_error_msg("Foo")): + self.fspath(Foo()) + + self.assertEqual(self.fspath(Bar()), 'bar') + + with self.assertRaisesRegex(TypeError, good_error_msg("Baz")): + self.fspath(Baz()) + + with self.assertRaisesRegex(TypeError, good_error_msg("Foo")): + open(Foo()) + + with self.assertRaisesRegex(TypeError, good_error_msg("Baz")): + open(Baz()) + + with self.assertRaisesRegex(TypeError, good_error_msg("Foo")): + os.rename(Foo(), "foooo") + + with self.assertRaisesRegex(TypeError, good_error_msg("Baz")): + os.rename(Baz(), "bazzz") + class TimesTests(unittest.TestCase): def test_times(self): times = os.times() diff --git a/Misc/NEWS.d/next/Library/2023-06-23-22-52-24.gh-issue-106046.OdLiLJ.rst b/Misc/NEWS.d/next/Library/2023-06-23-22-52-24.gh-issue-106046.OdLiLJ.rst new file mode 100644 index 00000000000000..ce10a9d81dc64c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-06-23-22-52-24.gh-issue-106046.OdLiLJ.rst @@ -0,0 +1,2 @@ +Improve the error message from :func:`os.fspath` if called on an object +where ``__fspath__`` is set to ``None``. Patch by Alex Waygood. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 694cff19d2286c..81b5b702f7ce88 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -1197,7 +1197,7 @@ path_converter(PyObject *o, void *p) PyObject *func, *res; func = _PyObject_LookupSpecial(o, &_Py_ID(__fspath__)); - if (NULL == func) { + if ((NULL == func) || (func == Py_None)) { goto error_format; } res = _PyObject_CallNoArgs(func); @@ -1271,11 +1271,11 @@ path_converter(PyObject *o, void *p) path->function_name ? path->function_name : "", path->function_name ? ": " : "", path->argument_name ? path->argument_name : "path", - path->allow_fd && path->nullable ? "string, bytes, os.PathLike, " - "integer or None" : - path->allow_fd ? "string, bytes, os.PathLike or integer" : - path->nullable ? "string, bytes, os.PathLike or None" : - "string, bytes or os.PathLike", + path->allow_fd && path->nullable ? "str, bytes, os.PathLike, " + "int or None" : + path->allow_fd ? "str, bytes, os.PathLike or int" : + path->nullable ? "str, bytes, os.PathLike or None" : + "str, bytes or os.PathLike", _PyType_Name(Py_TYPE(o))); goto error_exit; } @@ -15430,7 +15430,7 @@ PyOS_FSPath(PyObject *path) } func = _PyObject_LookupSpecial(path, &_Py_ID(__fspath__)); - if (NULL == func) { + if ((NULL == func) || (func == Py_None)) { return PyErr_Format(PyExc_TypeError, "expected str, bytes or os.PathLike object, " "not %.200s", From 1e9ebd4ba2654cce0be1e4cca51e0785afc512a9 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 25 Jun 2023 15:10:42 +0100 Subject: [PATCH 2/4] Update Lib/test/test_os.py Co-authored-by: Jelle Zijlstra --- Lib/test/test_os.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index a29806fae8ab54..e19b7b8375188f 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -4659,7 +4659,7 @@ class Baz(Bar): __fspath__ = None good_error_msg = ( - r"((expected)|(should be)) str, bytes or os.PathLike( object)?, not {}".format + r"(expected|should be) str, bytes or os.PathLike( object)?, not {}".format ) with self.assertRaisesRegex(TypeError, good_error_msg("Foo")): From dbfb232a5e13dcc77076b62d7b47afd7dad536e8 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Sun, 25 Jun 2023 15:21:41 +0100 Subject: [PATCH 3/4] Revert change to make error message more consistent: it makes other tests fail elsewhere --- Lib/test/test_os.py | 10 +++++++--- Modules/posixmodule.c | 10 +++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index a29806fae8ab54..99e9ed213e5615 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -4659,7 +4659,7 @@ class Baz(Bar): __fspath__ = None good_error_msg = ( - r"((expected)|(should be)) str, bytes or os.PathLike( object)?, not {}".format + r"expected str, bytes or os.PathLike object, not {}".format ) with self.assertRaisesRegex(TypeError, good_error_msg("Foo")): @@ -4676,10 +4676,14 @@ class Baz(Bar): with self.assertRaisesRegex(TypeError, good_error_msg("Baz")): open(Baz()) - with self.assertRaisesRegex(TypeError, good_error_msg("Foo")): + other_good_error_msg = ( + r"should be string, bytes or os.PathLike, not {}".format + ) + + with self.assertRaisesRegex(TypeError, other_good_error_msg("Foo")): os.rename(Foo(), "foooo") - with self.assertRaisesRegex(TypeError, good_error_msg("Baz")): + with self.assertRaisesRegex(TypeError, other_good_error_msg("Baz")): os.rename(Baz(), "bazzz") class TimesTests(unittest.TestCase): diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 81b5b702f7ce88..d73886f14cb9ec 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -1271,11 +1271,11 @@ path_converter(PyObject *o, void *p) path->function_name ? path->function_name : "", path->function_name ? ": " : "", path->argument_name ? path->argument_name : "path", - path->allow_fd && path->nullable ? "str, bytes, os.PathLike, " - "int or None" : - path->allow_fd ? "str, bytes, os.PathLike or int" : - path->nullable ? "str, bytes, os.PathLike or None" : - "str, bytes or os.PathLike", + path->allow_fd && path->nullable ? "string, bytes, os.PathLike, " + "integer or None" : + path->allow_fd ? "string, bytes, os.PathLike or integer" : + path->nullable ? "string, bytes, os.PathLike or None" : + "string, bytes or os.PathLike", _PyType_Name(Py_TYPE(o))); goto error_exit; } From 1dcf5e39d5ba0e14a3100d116c87165b69aca793 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Sun, 25 Jun 2023 15:32:36 +0100 Subject: [PATCH 4/4] Document it --- Doc/reference/datamodel.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Doc/reference/datamodel.rst b/Doc/reference/datamodel.rst index e8f9775dd33ce1..7f5edbbcadca6c 100644 --- a/Doc/reference/datamodel.rst +++ b/Doc/reference/datamodel.rst @@ -3179,8 +3179,9 @@ An example of an asynchronous context manager class:: lead to some very strange behaviour if it is handled incorrectly. .. [#] The :meth:`~object.__hash__`, :meth:`~object.__iter__`, - :meth:`~object.__reversed__`, and :meth:`~object.__contains__` methods have - special handling for this; others + :meth:`~object.__reversed__`, :meth:`~object.__contains__`, + :meth:`~object.__class_getitem__` and :meth:`~os.PathLike.__fspath__` + methods have special handling for this. Others will still raise a :exc:`TypeError`, but may do so by relying on the behavior that ``None`` is not callable.