From dce1f02e3245788e29929745f62a70214bb822da Mon Sep 17 00:00:00 2001 From: = Date: Fri, 29 Dec 2023 23:32:57 +0000 Subject: [PATCH 01/12] Fixed a bug in `reprlib.repr` where it incorrectly called the repr method on shadowed Python built-in types. --- Lib/reprlib.py | 26 ++++++++++++++++++++------ Lib/test/test_reprlib.py | 10 ++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/Lib/reprlib.py b/Lib/reprlib.py index 05bb1a0eb01795..af882f1d05919d 100644 --- a/Lib/reprlib.py +++ b/Lib/reprlib.py @@ -60,12 +60,26 @@ def repr(self, x): return self.repr1(x, self.maxlevel) def repr1(self, x, level): - typename = type(x).__name__ - if ' ' in typename: - parts = typename.split() - typename = '_'.join(parts) - if hasattr(self, 'repr_' + typename): - return getattr(self, 'repr_' + typename)(x, level) + + _lookup = { + ("builtins", "tuple"): self.repr_tuple, + ("builtins", "list"): self.repr_list, + ("array", "array"): self.repr_array, + ("builtins", "set"): self.repr_set, + ("builtins", "frozenset"): self.repr_frozenset, + ("collections", "deque"): self.repr_deque, + ("builtins", "dict"): self.repr_dict, + ("builtins", "str"): self.repr_str, + ("builtins", "int"): self.repr_int, + } + + _type = type(x) + module = _type.__module__ + typename = _type.__name__ + + predefined_repr = _lookup.get((module, typename)) + if predefined_repr: + return predefined_repr(x, level) else: return self.repr_instance(x, level) diff --git a/Lib/test/test_reprlib.py b/Lib/test/test_reprlib.py index 3e93b561c143d8..236196aa255264 100644 --- a/Lib/test/test_reprlib.py +++ b/Lib/test/test_reprlib.py @@ -580,6 +580,16 @@ def test_invalid_indent(self): with self.assertRaisesRegex(expected_error, expected_msg): r.repr(test_object) + def test_shadowed_builtin(self): + # Issue #113570: repr() should not be fooled by a shadowed builtin + # function + class array: + def __repr__(self): + return "not array.array" + + self.assertEqual(r(array()), "not array.array") + + def write_file(path, text): with open(path, 'w', encoding='ASCII') as fp: fp.write(text) From 5a4691b2e9be35174808f3ff79ed404d2d7f420a Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 30 Dec 2023 00:21:46 +0000 Subject: [PATCH 02/12] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2023-12-30-00-21-45.gh-issue-113570._XQgsW.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-12-30-00-21-45.gh-issue-113570._XQgsW.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-12-30-00-21-45.gh-issue-113570._XQgsW.rst b/Misc/NEWS.d/next/Core and Builtins/2023-12-30-00-21-45.gh-issue-113570._XQgsW.rst new file mode 100644 index 00000000000000..d95dd5656a907c --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-12-30-00-21-45.gh-issue-113570._XQgsW.rst @@ -0,0 +1 @@ +Fixed a bug in `reprlib.repr` where it incorrectly called the repr method on shadowed Python built-in types. From 04ed3cddcf3a54d11a7dc8feb8c682e617e4aca5 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 30 Dec 2023 00:29:33 +0000 Subject: [PATCH 03/12] use double backticks in news entry --- .../2023-12-30-00-21-45.gh-issue-113570._XQgsW.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-12-30-00-21-45.gh-issue-113570._XQgsW.rst b/Misc/NEWS.d/next/Core and Builtins/2023-12-30-00-21-45.gh-issue-113570._XQgsW.rst index d95dd5656a907c..6e0f0afe05369b 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-12-30-00-21-45.gh-issue-113570._XQgsW.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-12-30-00-21-45.gh-issue-113570._XQgsW.rst @@ -1 +1 @@ -Fixed a bug in `reprlib.repr` where it incorrectly called the repr method on shadowed Python built-in types. +Fixed a bug in ``reprlib.repr`` where it incorrectly called the repr method on shadowed Python built-in types. From 7a694674cbbc8d19979c738d46bce4741bc569c5 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 30 Dec 2023 17:32:56 +0000 Subject: [PATCH 04/12] fix so that subclasses that define repr_{type} methods still work --- Lib/reprlib.py | 47 +++++++++++++++++++++++++--------------- Lib/test/test_reprlib.py | 10 +++++++++ 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/Lib/reprlib.py b/Lib/reprlib.py index af882f1d05919d..feaa5ba29c6116 100644 --- a/Lib/reprlib.py +++ b/Lib/reprlib.py @@ -36,6 +36,17 @@ def wrapper(self): return decorating_function class Repr: + _lookup = { + "repr_tuple": ("builtins", "tuple"), + "repr_list": ("builtins", "list"), + "repr_array": ("array", "array"), + "repr_set": ("builtins", "set"), + "repr_frozenset": ("builtins", "frozenset"), + "repr_deque": ("collections", "deque"), + "repr_dict": ("builtins", "dict"), + "repr_str": ("builtins", "str"), + "repr_int": ("builtins", "int"), + } def __init__( self, *, maxlevel=6, maxtuple=6, maxlist=6, maxarray=5, maxdict=4, @@ -60,29 +71,29 @@ def repr(self, x): return self.repr1(x, self.maxlevel) def repr1(self, x, level): - - _lookup = { - ("builtins", "tuple"): self.repr_tuple, - ("builtins", "list"): self.repr_list, - ("array", "array"): self.repr_array, - ("builtins", "set"): self.repr_set, - ("builtins", "frozenset"): self.repr_frozenset, - ("collections", "deque"): self.repr_deque, - ("builtins", "dict"): self.repr_dict, - ("builtins", "str"): self.repr_str, - ("builtins", "int"): self.repr_int, - } - _type = type(x) - module = _type.__module__ typename = _type.__name__ + predefined_method = "repr_" + typename - predefined_repr = _lookup.get((module, typename)) - if predefined_repr: - return predefined_repr(x, level) - else: + if not hasattr(self, method_name): return self.repr_instance(x, level) + # we have a predefined method for this type, + # but it has been set in a subclass + if method_name not in self._lookup: + return getattr(self, method_name)(x, level) + + module = getattr(_type, "__module__", None) + is_shadowed = (module, typename) != self._lookup[method_name] + + if is_shadowed: + # we have a predefined method for a class with + # the same name as x, but it is not x + return self.repr_instance(x, level) + + return getattr(self, method_name)(x, level) + + def _join(self, pieces, level): if self.indent is None: return ', '.join(pieces) diff --git a/Lib/test/test_reprlib.py b/Lib/test/test_reprlib.py index 236196aa255264..a987efefe8a22a 100644 --- a/Lib/test/test_reprlib.py +++ b/Lib/test/test_reprlib.py @@ -589,6 +589,16 @@ def __repr__(self): self.assertEqual(r(array()), "not array.array") + def test_custom_repr(self): + class MyRepr(Repr): + + def repr_TextIOWrapper(self, obj, level): + if obj.name in {'', '', ''}: + return obj.name + return repr(obj) + + aRepr = MyRepr() + self.assertEqual(aRepr.repr(sys.stdin), "") def write_file(path, text): with open(path, 'w', encoding='ASCII') as fp: From c7ae05baba50d84d511feaea301f7a27a4798486 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 30 Dec 2023 17:39:22 +0000 Subject: [PATCH 05/12] fix variable name --- Lib/reprlib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/reprlib.py b/Lib/reprlib.py index feaa5ba29c6116..98869027a41b8a 100644 --- a/Lib/reprlib.py +++ b/Lib/reprlib.py @@ -73,7 +73,7 @@ def repr(self, x): def repr1(self, x, level): _type = type(x) typename = _type.__name__ - predefined_method = "repr_" + typename + method_name = "repr_" + typename if not hasattr(self, method_name): return self.repr_instance(x, level) From 8badac9cfb9926e70e3a6549e043e963baa0f49a Mon Sep 17 00:00:00 2001 From: = Date: Wed, 3 Jan 2024 17:46:27 +0000 Subject: [PATCH 06/12] add back handling of type(x).__name__ containing spaces --- Lib/reprlib.py | 6 +++++- Lib/test/test_reprlib.py | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/Lib/reprlib.py b/Lib/reprlib.py index 98869027a41b8a..1a2752c145791b 100644 --- a/Lib/reprlib.py +++ b/Lib/reprlib.py @@ -73,6 +73,11 @@ def repr(self, x): def repr1(self, x, level): _type = type(x) typename = _type.__name__ + + if " " in typename: + parts = typename.split() + typename = "_".join(parts) + method_name = "repr_" + typename if not hasattr(self, method_name): @@ -93,7 +98,6 @@ def repr1(self, x, level): return getattr(self, method_name)(x, level) - def _join(self, pieces, level): if self.indent is None: return ', '.join(pieces) diff --git a/Lib/test/test_reprlib.py b/Lib/test/test_reprlib.py index a987efefe8a22a..7f3f1b560a1b66 100644 --- a/Lib/test/test_reprlib.py +++ b/Lib/test/test_reprlib.py @@ -600,6 +600,22 @@ def repr_TextIOWrapper(self, obj, level): aRepr = MyRepr() self.assertEqual(aRepr.repr(sys.stdin), "") + def test_custom_repr_class_with_spaces(self): + class TypeWithSpaces: + pass + + t = TypeWithSpaces() + type(t).__name__ = "type with spaces" + self.assertEqual(type(t).__name__, "type with spaces") + + class MyRepr(Repr): + def repr_type_with_spaces(self, obj, level): + return "Type With Spaces" + + + aRepr = MyRepr() + self.assertEqual(aRepr.repr(t), "Type With Spaces") + def write_file(path, text): with open(path, 'w', encoding='ASCII') as fp: fp.write(text) From 660030e2a27ce7771cf100783a3375efd01788ea Mon Sep 17 00:00:00 2001 From: George Pittock Date: Sun, 28 Jan 2024 10:56:40 +0000 Subject: [PATCH 07/12] refactor multiple hasattr, getattr calls into one --- Lib/reprlib.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/reprlib.py b/Lib/reprlib.py index 1a2752c145791b..66459e56b655a9 100644 --- a/Lib/reprlib.py +++ b/Lib/reprlib.py @@ -80,13 +80,14 @@ def repr1(self, x, level): method_name = "repr_" + typename - if not hasattr(self, method_name): + _method = getattr(self, method_name, None) + if not _method: return self.repr_instance(x, level) # we have a predefined method for this type, # but it has been set in a subclass if method_name not in self._lookup: - return getattr(self, method_name)(x, level) + return _method(x, level) module = getattr(_type, "__module__", None) is_shadowed = (module, typename) != self._lookup[method_name] @@ -96,7 +97,7 @@ def repr1(self, x, level): # the same name as x, but it is not x return self.repr_instance(x, level) - return getattr(self, method_name)(x, level) + return _method(x, level) def _join(self, pieces, level): if self.indent is None: From 5b718e30a14ea4e9ed37b8e2a6247e9b5cf4ef77 Mon Sep 17 00:00:00 2001 From: George Pittock Date: Mon, 29 Jan 2024 06:43:30 +0000 Subject: [PATCH 08/12] add a test case for that reprlib.repr on a user defined class shadowing builtin "list" calls its own __repr__ --- Lib/test/test_reprlib.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_reprlib.py b/Lib/test/test_reprlib.py index 7f3f1b560a1b66..ffeb1fba7b80c6 100644 --- a/Lib/test/test_reprlib.py +++ b/Lib/test/test_reprlib.py @@ -580,15 +580,23 @@ def test_invalid_indent(self): with self.assertRaisesRegex(expected_error, expected_msg): r.repr(test_object) - def test_shadowed_builtin(self): - # Issue #113570: repr() should not be fooled by a shadowed builtin - # function + def test_shadowed_stdlib_array(self): + # Issue #113570: repr() should not be fooled by an array class array: def __repr__(self): return "not array.array" self.assertEqual(r(array()), "not array.array") + def test_shadowed_builtin(self): + # Issue #113570: repr() should not be fooled + # by a shadowed builtin function + class list: + def __repr__(self): + return "not builtins.list" + + self.assertEqual(r(list()), "not builtins.list") + def test_custom_repr(self): class MyRepr(Repr): From 24ac6387dd55e1869bbe52f1c51b6531b5b89dc1 Mon Sep 17 00:00:00 2001 From: George Pittock Date: Thu, 17 Oct 2024 08:32:33 +0100 Subject: [PATCH 09/12] Address review comments to remove unrelated style changes --- Lib/reprlib.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/reprlib.py b/Lib/reprlib.py index 66459e56b655a9..c75275f523f020 100644 --- a/Lib/reprlib.py +++ b/Lib/reprlib.py @@ -73,10 +73,9 @@ def repr(self, x): def repr1(self, x, level): _type = type(x) typename = _type.__name__ - - if " " in typename: + if ' ' in typename: parts = typename.split() - typename = "_".join(parts) + typename = '_'.join(parts) method_name = "repr_" + typename From 59af49a008b94755fc30060636fe8e6d01149674 Mon Sep 17 00:00:00 2001 From: George Pittock Date: Thu, 17 Oct 2024 15:52:30 +0100 Subject: [PATCH 10/12] remove redundancy of lookup dictionary plus a few style changes as per review --- Lib/reprlib.py | 56 ++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/Lib/reprlib.py b/Lib/reprlib.py index c75275f523f020..38964122860f20 100644 --- a/Lib/reprlib.py +++ b/Lib/reprlib.py @@ -37,16 +37,16 @@ def wrapper(self): class Repr: _lookup = { - "repr_tuple": ("builtins", "tuple"), - "repr_list": ("builtins", "list"), - "repr_array": ("array", "array"), - "repr_set": ("builtins", "set"), - "repr_frozenset": ("builtins", "frozenset"), - "repr_deque": ("collections", "deque"), - "repr_dict": ("builtins", "dict"), - "repr_str": ("builtins", "str"), - "repr_int": ("builtins", "int"), - } + 'tuple': 'builtins', + 'list': 'builtins', + 'array': 'array', + 'set': 'builtins', + 'frozenset': 'builtins', + 'deque': 'collections', + 'dict': 'builtins', + 'str': 'builtins', + 'int': 'builtins' + } def __init__( self, *, maxlevel=6, maxtuple=6, maxlist=6, maxarray=5, maxdict=4, @@ -71,32 +71,24 @@ def repr(self, x): return self.repr1(x, self.maxlevel) def repr1(self, x, level): - _type = type(x) - typename = _type.__name__ + cls = type(x) + typename = cls.__name__ + if ' ' in typename: parts = typename.split() typename = '_'.join(parts) - method_name = "repr_" + typename - - _method = getattr(self, method_name, None) - if not _method: - return self.repr_instance(x, level) - - # we have a predefined method for this type, - # but it has been set in a subclass - if method_name not in self._lookup: - return _method(x, level) - - module = getattr(_type, "__module__", None) - is_shadowed = (module, typename) != self._lookup[method_name] - - if is_shadowed: - # we have a predefined method for a class with - # the same name as x, but it is not x - return self.repr_instance(x, level) - - return _method(x, level) + method = getattr(self, 'repr_' + typename, None) + if method: + # not defined in this class + if typename not in self._lookup: + return method(x, level) + module = getattr(cls, '__module__', None) + # defined in this class and is the module intended + if module == self._lookup.get(typename): + return method(x, level) + + return self.repr_instance(x, level) def _join(self, pieces, level): if self.indent is None: From 0bcf8dbb826713fcc2ea53c47c7fafdbfe507ee6 Mon Sep 17 00:00:00 2001 From: George Pittock Date: Thu, 17 Oct 2024 16:02:36 +0100 Subject: [PATCH 11/12] remove trailing whitespace --- Lib/reprlib.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Lib/reprlib.py b/Lib/reprlib.py index 38964122860f20..56a1fafd52719a 100644 --- a/Lib/reprlib.py +++ b/Lib/reprlib.py @@ -37,14 +37,14 @@ def wrapper(self): class Repr: _lookup = { - 'tuple': 'builtins', - 'list': 'builtins', - 'array': 'array', - 'set': 'builtins', - 'frozenset': 'builtins', - 'deque': 'collections', - 'dict': 'builtins', - 'str': 'builtins', + 'tuple': 'builtins', + 'list': 'builtins', + 'array': 'array', + 'set': 'builtins', + 'frozenset': 'builtins', + 'deque': 'collections', + 'dict': 'builtins', + 'str': 'builtins', 'int': 'builtins' } @@ -87,7 +87,7 @@ def repr1(self, x, level): # defined in this class and is the module intended if module == self._lookup.get(typename): return method(x, level) - + return self.repr_instance(x, level) def _join(self, pieces, level): From 70ba74c9413da1ab44163bb50b431ae2bcaa05f2 Mon Sep 17 00:00:00 2001 From: George Pittock <66332098+georgepittock@users.noreply.github.com> Date: Thu, 17 Oct 2024 16:40:25 +0100 Subject: [PATCH 12/12] Change .get to __getitem__ as we already know typename is in the dict Co-authored-by: Serhiy Storchaka --- Lib/reprlib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/reprlib.py b/Lib/reprlib.py index 56a1fafd52719a..19dbe3a07eb618 100644 --- a/Lib/reprlib.py +++ b/Lib/reprlib.py @@ -85,7 +85,7 @@ def repr1(self, x, level): return method(x, level) module = getattr(cls, '__module__', None) # defined in this class and is the module intended - if module == self._lookup.get(typename): + if module == self._lookup[typename]: return method(x, level) return self.repr_instance(x, level)