Skip to content

Commit

Permalink
Fix memoization when called with different args
Browse files Browse the repository at this point in the history
Previous implementation of the memoize function was not aware of
function arguments. This could lead to incorrect return values,
especially when the memoize was used on class instance methods.

With current implementation, in order to optimize cache size, the
memoize function is used only on class and static methods.
  • Loading branch information
arkq committed Sep 17, 2022
1 parent c0cee6a commit 70022cc
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 52 deletions.
104 changes: 56 additions & 48 deletions src/flake8_requirements/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ def memoize(f):
"""Cache value returned by the function."""
@wraps(f)
def w(*args, **kw):
if f not in memoize.mem:
memoize.mem[f] = f(*args, **kw)
return memoize.mem[f]
k = (f, repr(args), repr(kw))
if k not in memoize.mem:
memoize.mem[k] = f(*args, **kw)
return memoize.mem[k]
return w


Expand Down Expand Up @@ -437,6 +438,15 @@ def discover_project_root_dir(path):
path = os.path.abspath(os.path.join(path, ".."))
return ""

@staticmethod
def is_project_setup_py(project_root_dir, filename):
"""Determine whether given file is project's setup.py file."""
project_setup_py = os.path.join(project_root_dir, "setup.py")
try:
return os.path.samefile(filename, project_setup_py)
except OSError:
return False

_requirement_match_option = re.compile(
r"(-[\w-]+)(.*)").match

Expand Down Expand Up @@ -549,9 +559,10 @@ def get_pyproject_toml_pep621(cls):
cfg_pep518 = cls.get_pyproject_toml()
return cfg_pep518.get('project', {})

def get_pyproject_toml_pep621_requirements(self):
@classmethod
def get_pyproject_toml_pep621_requirements(cls):
"""Try to get PEP 621 metadata requirements."""
pep621 = self.get_pyproject_toml_pep621()
pep621 = cls.get_pyproject_toml_pep621()
requirements = []
requirements.extend(parse_requirements(
pep621.get("dependencies", ())))
Expand All @@ -565,9 +576,10 @@ def get_pyproject_toml_poetry(cls):
cfg_pep518 = cls.get_pyproject_toml()
return cfg_pep518.get('tool', {}).get('poetry', {})

def get_pyproject_toml_poetry_requirements(self):
@classmethod
def get_pyproject_toml_poetry_requirements(cls):
"""Try to get poetry configuration requirements."""
poetry = self.get_pyproject_toml_poetry()
poetry = cls.get_pyproject_toml_poetry()
requirements = []
requirements.extend(parse_requirements(
poetry.get('dependencies', ())))
Expand All @@ -580,7 +592,6 @@ def get_pyproject_toml_poetry_requirements(self):
return requirements

@classmethod
@memoize
def get_requirements_txt(cls):
"""Try to load requirements from text file."""
path = cls.requirements_file or "requirements.txt"
Expand Down Expand Up @@ -610,9 +621,10 @@ def get_setup_cfg(cls):
LOG.debug("Couldn't load setup configuration: setup.cfg")
return config

def get_setup_cfg_requirements(self):
@classmethod
def get_setup_cfg_requirements(cls, is_setup_py):
"""Try to load standard configuration file requirements."""
config = self.get_setup_cfg()
config = cls.get_setup_cfg()
requirements = []
requirements.extend(parse_requirements(
config.get('options', 'install_requires')))
Expand All @@ -621,7 +633,7 @@ def get_setup_cfg_requirements(self):
for _, r in config.items('options.extras_require'):
requirements.extend(parse_requirements(r))
setup_requires = config.get('options', 'setup_requires')
if setup_requires and self.processing_setup_py:
if setup_requires and is_setup_py:
requirements.extend(parse_requirements(setup_requires))
return requirements

Expand All @@ -636,13 +648,14 @@ def get_setup_py(cls):
LOG.debug("Couldn't load project setup: %s", e)
return SetupVisitor(ast.parse(""), cls.root_dir)

def get_setup_py_requirements(self):
@classmethod
def get_setup_py_requirements(cls, is_setup_py):
"""Try to load standard setup file requirements."""
setup = self.get_setup_py()
setup = cls.get_setup_py()
if not setup.redirected:
return []
return setup.get_requirements(
setup=self.processing_setup_py,
setup=is_setup_py,
tests=True,
)

Expand All @@ -663,53 +676,48 @@ def get_mods_1st_party(cls):
mods_1st_party.add(modsplit(module), True)
return mods_1st_party

def get_mods_3rd_party_requirements(self):
@classmethod
@memoize
def get_mods_3rd_party(cls, is_setup_py):
mods_3rd_party = ModuleSet()
# Get 3rd party module names based on requirements.
for requirement in cls.get_mods_3rd_party_requirements(is_setup_py):
modules = [project2module(requirement.project_name)]
if modules[0] in cls.known_modules:
modules = cls.known_modules[modules[0]]
elif modules[0] in cls.known_3rd_parties:
modules = cls.known_3rd_parties[modules[0]]
elif modules[0] in cls.known_host_3rd_parties:
modules = cls.known_host_3rd_parties[modules[0]]
for module in modules:
mods_3rd_party.add(modsplit(module), requirement)
return mods_3rd_party

@classmethod
def get_mods_3rd_party_requirements(cls, is_setup_py):
"""Get list of 3rd party requirements."""
# Use user provided requirements text file.
if self.requirements_file:
return self.get_requirements_txt()
if cls.requirements_file:
return cls.get_requirements_txt()
return (
# Use requirements from setup if available.
self.get_setup_py_requirements() or
cls.get_setup_py_requirements(is_setup_py) or
# Check setup configuration file for requirements.
self.get_setup_cfg_requirements() or
cls.get_setup_cfg_requirements(is_setup_py) or
# Check PEP 621 metadata for requirements.
self.get_pyproject_toml_pep621_requirements() or
cls.get_pyproject_toml_pep621_requirements() or
# Check project configuration for requirements.
self.get_pyproject_toml_poetry_requirements() or
cls.get_pyproject_toml_poetry_requirements() or
# Fall-back to requirements.txt in our root directory.
self.get_requirements_txt()
cls.get_requirements_txt()
)

@memoize
def get_mods_3rd_party(self):
mods_3rd_party = ModuleSet()
# Get 3rd party module names based on requirements.
for requirement in self.get_mods_3rd_party_requirements():
modules = [project2module(requirement.project_name)]
if modules[0] in self.known_modules:
modules = self.known_modules[modules[0]]
elif modules[0] in self.known_3rd_parties:
modules = self.known_3rd_parties[modules[0]]
elif modules[0] in self.known_host_3rd_parties:
modules = self.known_host_3rd_parties[modules[0]]
for module in modules:
mods_3rd_party.add(modsplit(module), requirement)
return mods_3rd_party

@property
def processing_setup_py(self):
"""Determine whether we are processing setup.py file."""
try:
return os.path.samefile(self.filename, "setup.py")
except OSError:
return False

def check_I900(self, node):
"""Run missing requirement checker."""
if node.module[0] in STDLIB:
return
if node.module in self.get_mods_3rd_party():
is_setup_py = self.is_project_setup_py(self.root_dir, self.filename)
if node.module in self.get_mods_3rd_party(is_setup_py):
return
if node.module in self.get_mods_1st_party():
return
Expand All @@ -718,7 +726,7 @@ def check_I900(self, node):
# project, even though it is not listed as a requirement - this
# package is required to run setup.py, so listing it as a setup
# requirement would be pointless.
if (self.processing_setup_py and
if (is_setup_py and
node.module[0] in KNOWN_3RD_PARTIES["setuptools"]):
return
return ERRORS['I900'].format(pkg=node.module[0])
Expand Down
2 changes: 1 addition & 1 deletion test/test_pep621.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,5 @@ def test_3rd_party(self):
)

checker = Flake8Checker(None, None)
mods = checker.get_mods_3rd_party()
mods = checker.get_mods_3rd_party(False)
self.assertEqual(mods, ModuleSet({"tools": {}, "dev_tools": {}}))
4 changes: 2 additions & 2 deletions test/test_poetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def test_3rd_party(self):
)

checker = Flake8Checker(None, None)
mods = checker.get_mods_3rd_party()
mods = checker.get_mods_3rd_party(False)
self.assertEqual(mods, ModuleSet({"tools": {}, "dev_tools": {}}))

def test_3rd_party_groups(self):
Expand All @@ -64,5 +64,5 @@ def test_3rd_party_groups(self):
)

checker = Flake8Checker(None, None)
mods = checker.get_mods_3rd_party()
mods = checker.get_mods_3rd_party(False)
self.assertEqual(mods, ModuleSet({"tools": {}, "dev_tools": {}}))
2 changes: 1 addition & 1 deletion test/test_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_get_setup_cfg_requirements(self):
with mock.patch(builtins_open, mock.mock_open(read_data=content)):
checker = Flake8Checker(None, None)
self.assertEqual(
checker.get_setup_cfg_requirements(),
checker.get_setup_cfg_requirements(False),
list(parse_requirements([
"requests",
"importlib; python_version == \"2.6\"",
Expand Down

0 comments on commit 70022cc

Please sign in to comment.