From 70022cc699ead88808c38c1e0b47a013faad04af Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy <arkadiusz.bokowy@gmail.com> Date: Sat, 17 Sep 2022 12:32:25 +0200 Subject: [PATCH] Fix memoization when called with different args 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. --- src/flake8_requirements/checker.py | 104 ++++++++++++++++------------- test/test_pep621.py | 2 +- test/test_poetry.py | 4 +- test/test_setup.py | 2 +- 4 files changed, 60 insertions(+), 52 deletions(-) diff --git a/src/flake8_requirements/checker.py b/src/flake8_requirements/checker.py index 5b341ab..00d32d9 100644 --- a/src/flake8_requirements/checker.py +++ b/src/flake8_requirements/checker.py @@ -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 @@ -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 @@ -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", ()))) @@ -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', ()))) @@ -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" @@ -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'))) @@ -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 @@ -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, ) @@ -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 @@ -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]) diff --git a/test/test_pep621.py b/test/test_pep621.py index 256b6fe..fd96592 100644 --- a/test/test_pep621.py +++ b/test/test_pep621.py @@ -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": {}})) diff --git a/test/test_poetry.py b/test/test_poetry.py index 61d3652..a387b7d 100644 --- a/test/test_poetry.py +++ b/test/test_poetry.py @@ -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): @@ -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": {}})) diff --git a/test/test_setup.py b/test/test_setup.py index 0dbd710..cf49d2b 100644 --- a/test/test_setup.py +++ b/test/test_setup.py @@ -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\"",