From a189ae013df9d161710145c7bcd71b678ae133d7 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 19 Mar 2024 11:43:39 +0100 Subject: [PATCH] Don't rely on method monkey patching It is hard to reason about code when monkey patched and methods are overwritten. This had two methods/property: joined() and lines() calling each other, and depending on how the object was constructed one was replaced by an attribute. Is is convoluted and make static inference difficult. Move to private attribute and always storing `_lines` (as a list of strings that are not terminated by newlines. --- lib/python/pyflyby/_file.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/python/pyflyby/_file.py b/lib/python/pyflyby/_file.py index 0e33957d..88ef2b85 100644 --- a/lib/python/pyflyby/_file.py +++ b/lib/python/pyflyby/_file.py @@ -9,7 +9,7 @@ import os import re import sys -from typing import Optional +from typing import Optional, Tuple from pyflyby._util import cmp, memoize @@ -346,6 +346,7 @@ class FileText: filename: Optional[Filename] startpos: FilePos + _lines: Optional[Tuple[str, ...]] = None def __new__(cls, arg, filename=None, startpos=None): """ @@ -380,7 +381,7 @@ def __new__(cls, arg, filename=None, startpos=None): return FileText(arg.__text__(), filename=filename, startpos=startpos) elif isinstance(arg, str): self = object.__new__(cls) - self.joined = arg + self._lines = tuple(arg.split('\n')) else: raise TypeError("%s: unexpected %s" % (cls.__name__, type(arg).__name__)) @@ -400,13 +401,13 @@ def _from_lines(cls, lines, filename: Optional[Filename], startpos: FilePos): assert isinstance(startpos, FilePos), repr(startpos) assert isinstance(filename, (Filename, type(None))), repr(filename) self = object.__new__(cls) - self.lines = lines + self._lines = tuple(lines) self.filename = filename self.startpos = startpos return self @cached_property - def lines(self): + def lines(self) -> Tuple[str, ...]: r""" Lines that have been split by newline. @@ -419,6 +420,8 @@ def lines(self): :rtype: ``tuple`` of ``str`` """ + if self._lines is not None: + return self._lines # Used if only initialized with 'joined'. # We use str.split() instead of str.splitlines() because the latter # doesn't distinguish between strings that end in newline or not @@ -426,9 +429,10 @@ def lines(self): return tuple(self.joined.split('\n')) @cached_property - def joined(self): # used if only initialized with 'lines' + def joined(self) -> str: return '\n'.join(self.lines) + @classmethod def from_filename(cls, filename): return cls.from_lines(Filename(filename)) @@ -446,8 +450,7 @@ def alter(self, filename=None, startpos=None): return self else: result = object.__new__(type(self)) - result.lines = self.lines - result.joined = self.joined + result._lines = self._lines result.filename = filename result.startpos = startpos return result