From 1397887a17db8d485e3eb897b680d585603a0487 Mon Sep 17 00:00:00 2001 From: Pavel savchenko Date: Mon, 5 Feb 2024 12:26:46 +0100 Subject: [PATCH 1/7] test: allow specifying name to the check_code helper + update test_stdin to make use of the updated helper --- run_tests.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/run_tests.py b/run_tests.py index 522edec..a297caa 100644 --- a/run_tests.py +++ b/run_tests.py @@ -18,7 +18,7 @@ def __init__(self, ignore_list='', builtins=None): self.builtins = builtins -def check_code(source, expected_codes=None, ignore_list=None, builtins=None): +def check_code(source, expected_codes=None, ignore_list=None, builtins=None, filename='/home/script.py'): """Check if the given source code generates the given flake8 errors If `expected_codes` is a string is converted to a list, @@ -37,7 +37,7 @@ def check_code(source, expected_codes=None, ignore_list=None, builtins=None): if ignore_list is None: ignore_list = [] tree = ast.parse(textwrap.dedent(source)) - checker = BuiltinsChecker(tree, '/home/script.py') + checker = BuiltinsChecker(tree, filename) checker.parse_options(FakeOptions(ignore_list=ignore_list, builtins=builtins)) return_statements = list(checker.run()) @@ -463,12 +463,11 @@ async def bla(): def test_stdin(stdin_get_value): source = 'max = 4' stdin_get_value.return_value = source - checker = BuiltinsChecker('', 'stdin') - checker.parse_options(FakeOptions()) - ret = list(checker.run()) - assert len(ret) == 1 + check_code('', expected_codes='A001', filename='stdin') def test_tuple_unpacking(): source = 'a, *(b, c) = 1, 2, 3' check_code(source) + + From 6f4e77f38aaba21d9ba3c272a4b46043ad30baed Mon Sep 17 00:00:00 2001 From: Pavel savchenko Date: Mon, 5 Feb 2024 12:59:31 +0100 Subject: [PATCH 2/7] feat: detect builtin module name shadowing --- flake8_builtins.py | 47 +++++++++++++++++++++++++++++++++++++++++++--- run_tests.py | 21 +++++++++++++++++++-- 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/flake8_builtins.py b/flake8_builtins.py index 1473b9f..d942c56 100644 --- a/flake8_builtins.py +++ b/flake8_builtins.py @@ -1,8 +1,10 @@ from flake8 import utils as stdin_utils +from pathlib import Path import ast import builtins import inspect +import sys class BuiltinsChecker: @@ -12,6 +14,7 @@ class BuiltinsChecker: argument_msg = 'A002 argument "{0}" is shadowing a Python builtin' class_attribute_msg = 'A003 class attribute "{0}" is shadowing a Python builtin' import_msg = 'A004 import statement "{0}" is shadowing a Python builtin' + module_name_msg = 'A005 the module is shadowing a Python builtin module "{0}"' names = [] ignore_list = { @@ -20,6 +23,7 @@ class BuiltinsChecker: 'credits', '_', } + ignored_module_names = {} def __init__(self, tree, filename): self.tree = tree @@ -34,6 +38,20 @@ def add_options(cls, option_manager): comma_separated_list=True, help='A comma separated list of builtins to skip checking', ) + option_manager.add_option( + '--builtins-allowed-modules', + metavar='builtins', + parse_from_config=True, + comma_separated_list=True, + help='A comma separated list of builtin module names to allow', + ) + option_manager.add_option( + '--builtins-allowed-modules', + metavar='builtins', + parse_from_config=True, + comma_separated_list=True, + help='A comma separated list of builtin module names to allow', + ) @classmethod def parse_options(cls, options): @@ -47,12 +65,24 @@ def parse_options(cls, options): if flake8_builtins: cls.names.update(flake8_builtins) + if options.builtins_allowed_modules is not None: + cls.ignored_module_names.update(options.builtins_allowed_modules) + + known_module_names = getattr( + sys, 'stdlib_module_names', sys.builtin_module_names + ) + cls.module_names = { + m for m in known_module_names if m not in cls.ignored_module_names + } + def run(self): tree = self.tree if self.filename == 'stdin': lines = stdin_utils.stdin_get_value() tree = ast.parse(lines) + else: + yield from self.check_module_name(self.filename) for statement in ast.walk(tree): for child in ast.iter_child_nodes(statement): @@ -234,13 +264,24 @@ def check_class(self, statement): if statement.name in self.names: yield self.error(statement, variable=statement.name) - def error(self, statement, variable, message=None): + def error(self, statement=None, variable=None, message=None): if not message: message = self.assign_msg + # lineno and col_offset must be integers return ( - statement.lineno, - statement.col_offset, + statement.lineno if statement else 0, + statement.col_offset if statement else 0, message.format(variable), type(self), ) + + def check_module_name(self, filename: str): + path = Path(filename) + module_name = path.name.removesuffix('.py') + if module_name in self.module_names: + yield self.error( + None, + module_name, + message=self.module_name_msg, + ) diff --git a/run_tests.py b/run_tests.py index a297caa..9e5280b 100644 --- a/run_tests.py +++ b/run_tests.py @@ -10,15 +10,24 @@ class FakeOptions: builtins_ignorelist = [] builtins = None + builtins_allowed_modules = None - def __init__(self, ignore_list='', builtins=None): + def __init__(self, ignore_list='', builtins=None, builtins_allowed_modules=None): if ignore_list: self.builtins_ignorelist = ignore_list if builtins: self.builtins = builtins + if builtins_allowed_modules: + self.builtins_allowed_modules = builtins_allowed_modules -def check_code(source, expected_codes=None, ignore_list=None, builtins=None, filename='/home/script.py'): +def check_code( + source, + expected_codes=None, + ignore_list=None, + builtins=None, + filename='/home/script.py', +): """Check if the given source code generates the given flake8 errors If `expected_codes` is a string is converted to a list, @@ -471,3 +480,11 @@ def test_tuple_unpacking(): check_code(source) +def test_module_name(): + source = '' + check_code(source, expected_codes='A005', filename='./temp/logging.py') + + +def test_module_name_not_builtin(): + source = '' + check_code(source, filename='log_config') From 33bc80d8c52a65da7193af83812860a04cd8dfd2 Mon Sep 17 00:00:00 2001 From: Pavel savchenko Date: Mon, 25 Mar 2024 12:23:50 +0000 Subject: [PATCH 3/7] fix: remove duplicate --builtins-allowed-modules option --- flake8_builtins.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/flake8_builtins.py b/flake8_builtins.py index d942c56..e92a5e7 100644 --- a/flake8_builtins.py +++ b/flake8_builtins.py @@ -45,13 +45,6 @@ def add_options(cls, option_manager): comma_separated_list=True, help='A comma separated list of builtin module names to allow', ) - option_manager.add_option( - '--builtins-allowed-modules', - metavar='builtins', - parse_from_config=True, - comma_separated_list=True, - help='A comma separated list of builtin module names to allow', - ) @classmethod def parse_options(cls, options): From 246537c4d1d533de35fb0c2562ce29e9b218ca0f Mon Sep 17 00:00:00 2001 From: Pavel savchenko Date: Mon, 25 Mar 2024 12:24:29 +0000 Subject: [PATCH 4/7] fix: check modules only in python 3.10 and above --- flake8_builtins.py | 16 ++++++++++------ run_tests.py | 4 ++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/flake8_builtins.py b/flake8_builtins.py index e92a5e7..c9038bd 100644 --- a/flake8_builtins.py +++ b/flake8_builtins.py @@ -61,12 +61,14 @@ def parse_options(cls, options): if options.builtins_allowed_modules is not None: cls.ignored_module_names.update(options.builtins_allowed_modules) - known_module_names = getattr( - sys, 'stdlib_module_names', sys.builtin_module_names - ) - cls.module_names = { - m for m in known_module_names if m not in cls.ignored_module_names - } + if hasattr(sys, 'stdlib_module_names'): + # stdlib_module_names is only available in Python 3.10+ + known_module_names = sys.stdlib_module_names + cls.module_names = { + m for m in known_module_names if m not in cls.ignored_module_names + } + else: + cls.module_names = set() def run(self): tree = self.tree @@ -270,6 +272,8 @@ def error(self, statement=None, variable=None, message=None): ) def check_module_name(self, filename: str): + if not self.module_names: + return path = Path(filename) module_name = path.name.removesuffix('.py') if module_name in self.module_names: diff --git a/run_tests.py b/run_tests.py index 9e5280b..ad527e8 100644 --- a/run_tests.py +++ b/run_tests.py @@ -480,6 +480,10 @@ def test_tuple_unpacking(): check_code(source) +@pytest.mark.skipif( + sys.version_info < (3, 10), + reason='Skip A005, module testing is only supported in Python 3.10 and above', +) def test_module_name(): source = '' check_code(source, expected_codes='A005', filename='./temp/logging.py') From 99ef5176e18bac002a11c3b53a943f5242e54b6a Mon Sep 17 00:00:00 2001 From: Pavel savchenko Date: Mon, 25 Mar 2024 12:45:16 +0000 Subject: [PATCH 5/7] fix: correct ignoring module names and test that using the option works as expected --- flake8_builtins.py | 2 +- run_tests.py | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/flake8_builtins.py b/flake8_builtins.py index c9038bd..ed23b74 100644 --- a/flake8_builtins.py +++ b/flake8_builtins.py @@ -23,7 +23,7 @@ class BuiltinsChecker: 'credits', '_', } - ignored_module_names = {} + ignored_module_names = set() def __init__(self, tree, filename): self.tree = tree diff --git a/run_tests.py b/run_tests.py index ad527e8..e46b57b 100644 --- a/run_tests.py +++ b/run_tests.py @@ -26,6 +26,7 @@ def check_code( expected_codes=None, ignore_list=None, builtins=None, + builtins_allowed_modules=None, filename='/home/script.py', ): """Check if the given source code generates the given flake8 errors @@ -47,7 +48,13 @@ def check_code( ignore_list = [] tree = ast.parse(textwrap.dedent(source)) checker = BuiltinsChecker(tree, filename) - checker.parse_options(FakeOptions(ignore_list=ignore_list, builtins=builtins)) + checker.parse_options( + FakeOptions( + ignore_list=ignore_list, + builtins=builtins, + builtins_allowed_modules=builtins_allowed_modules, + ) + ) return_statements = list(checker.run()) assert len(return_statements) == len(expected_codes) @@ -489,6 +496,19 @@ def test_module_name(): check_code(source, expected_codes='A005', filename='./temp/logging.py') +@pytest.mark.skipif( + sys.version_info < (3, 10), + reason='Skip A005, module testing is only supported in Python 3.10 and above', +) +def test_module_name_ignore_module(): + source = '' + check_code( + source, + filename='./temp/logging.py', + builtins_allowed_modules=['logging'], + ) + + def test_module_name_not_builtin(): source = '' check_code(source, filename='log_config') From 62971eefa0af0d564ad1829a7c6c0c307aff6a7d Mon Sep 17 00:00:00 2001 From: Pavel savchenko Date: Mon, 25 Mar 2024 12:48:24 +0000 Subject: [PATCH 6/7] docs: add description for the new A005 check --- README.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.rst b/README.rst index f867f63..8268d4f 100644 --- a/README.rst +++ b/README.rst @@ -101,6 +101,9 @@ A003: A004: An import statement is shadowing a Python builtin. +A005: + A module is shadowing a Python builtin module (e.g: `logging` or `socket`) + License ------- GPL 2.0 From 58b85275d1b2d590dfbbedd31ee1ce6444a1206d Mon Sep 17 00:00:00 2001 From: Pavel savchenko Date: Mon, 25 Mar 2024 12:50:01 +0000 Subject: [PATCH 7/7] chore: add feature description to CHANGES.rst --- CHANGES.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 29b1e45..dcf9ae7 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,7 +6,8 @@ Changelog 2.2.1 (unreleased) ------------------ -- Nothing changed yet. +- Add rule for builtin module name shadowing (`A005`). + [asfaltboy] 2.2.0 (2023-11-03)