Skip to content

Commit

Permalink
Don't rely on method monkey patching
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Carreau committed Mar 19, 2024
1 parent 631f083 commit a189ae0
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions lib/python/pyflyby/_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -346,6 +346,7 @@ class FileText:

filename: Optional[Filename]
startpos: FilePos
_lines: Optional[Tuple[str, ...]] = None

def __new__(cls, arg, filename=None, startpos=None):
"""
Expand Down Expand Up @@ -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__))
Expand All @@ -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.
Expand All @@ -419,16 +420,19 @@ 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
# (or requires extra work to process if we use splitlines(True)).
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))
Expand All @@ -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
Expand Down

0 comments on commit a189ae0

Please sign in to comment.