From ef601e1189f2752017687954b2173a9b0c78eec2 Mon Sep 17 00:00:00 2001 From: antonag32 Date: Fri, 18 Nov 2022 17:18:50 -0600 Subject: [PATCH] [ADD] new `unused-module` check Fixes #177 This new check looks for python files (modules) which are not referenced anywhere in the codebase, making them useless. --- README.md | 9 ++ src/pylint_odoo/checkers/__init__.py | 4 +- src/pylint_odoo/checkers/import_checker.py | 128 ++++++++++++++++++ src/pylint_odoo/plugin.py | 2 + .../test_repo/unused_module/__init__.py | 1 + .../test_repo/unused_module/__manifest__.py | 10 ++ .../unused_module/controllers/__init__.py | 0 .../unused_module/controllers/test.py | 4 + .../migrations/11.0.1.0.1/pre-migration.py | 1 + .../unused_module/models/__init__.py | 1 + .../unused_module/models/fail_model.py | 6 + .../test_repo/unused_module/models/foo.py | 1 + .../unused_module/models/res_partner.py | 11 ++ .../unused_module/models/unused_utils.py | 1 + .../test_repo/unused_module/tests/__init__.py | 1 + .../unused_module/tests/test_res_partner.py | 7 + .../test_repo/unused_module/useful.py | 1 + .../unused_module/wizard/__init__.py | 1 + .../unused_module/wizard/fail_wizard.py | 7 + .../unused_module/wizard/pass_wizard.py | 13 ++ .../test_repo/unused_module/wizard/utils.py | 2 + tests/test_main.py | 5 + 22 files changed, 213 insertions(+), 3 deletions(-) create mode 100644 src/pylint_odoo/checkers/import_checker.py create mode 100644 testing/resources/test_repo/unused_module/__init__.py create mode 100644 testing/resources/test_repo/unused_module/__manifest__.py create mode 100644 testing/resources/test_repo/unused_module/controllers/__init__.py create mode 100644 testing/resources/test_repo/unused_module/controllers/test.py create mode 100644 testing/resources/test_repo/unused_module/migrations/11.0.1.0.1/pre-migration.py create mode 100644 testing/resources/test_repo/unused_module/models/__init__.py create mode 100644 testing/resources/test_repo/unused_module/models/fail_model.py create mode 100644 testing/resources/test_repo/unused_module/models/foo.py create mode 100644 testing/resources/test_repo/unused_module/models/res_partner.py create mode 100644 testing/resources/test_repo/unused_module/models/unused_utils.py create mode 100644 testing/resources/test_repo/unused_module/tests/__init__.py create mode 100644 testing/resources/test_repo/unused_module/tests/test_res_partner.py create mode 100644 testing/resources/test_repo/unused_module/useful.py create mode 100644 testing/resources/test_repo/unused_module/wizard/__init__.py create mode 100644 testing/resources/test_repo/unused_module/wizard/fail_wizard.py create mode 100644 testing/resources/test_repo/unused_module/wizard/pass_wizard.py create mode 100644 testing/resources/test_repo/unused_module/wizard/utils.py diff --git a/README.md b/README.md index be4ac39b..59c41410 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,7 @@ translation-required | String parameter on "%s" requires translation. Use %s_(%s translation-too-few-args | Not enough arguments for odoo._ format string | E8306 translation-too-many-args | Too many arguments for odoo._ format string | E8305 translation-unsupported-format | Unsupported odoo._ format character %r (%#02x) at index %d | E8300 +unused-module | This python module is not imported and has no effect | E8401 use-vim-comment | Use of vim comment | W8202 website-manifest-key-not-valid-uri | Website "%s" in manifest key is not a valid URI | W8114 @@ -242,6 +243,7 @@ Checks valid only for odoo <= 13.0 * missing-readme - https://github.com/OCA/pylint-odoo/blob/v8.0.19/testing/resources/test_repo/broken_module/__openerp__.py#L2 Missing ./README.rst file. Template here: https://github.com/OCA/maintainer-tools/blob/master/template/module/README.rst + - https://github.com/OCA/pylint-odoo/blob/v8.0.19/testing/resources/test_repo/unused_module/__manifest__.py#L1 Missing ./README.rst file. Template here: https://github.com/OCA/maintainer-tools/blob/master/template/module/README.rst * missing-return @@ -272,6 +274,7 @@ Checks valid only for odoo <= 13.0 * print-used - https://github.com/OCA/pylint-odoo/blob/v8.0.19/testing/resources/test_repo/test_module/except_pass.py#L20 Print used. Use `logger` instead. + - https://github.com/OCA/pylint-odoo/blob/v8.0.19/testing/resources/test_repo/unused_module/migrations/11.0.1.0.1/pre-migration.py#L1 Print used. Use `logger` instead. * renamed-field-parameter @@ -351,6 +354,12 @@ Checks valid only for odoo <= 13.0 - https://github.com/OCA/pylint-odoo/blob/v8.0.19/testing/resources/test_repo/broken_module/models/broken_model.py#L466 Unsupported odoo._ format character 'y' (0x79) at index 30 + * unused-module + + - https://github.com/OCA/pylint-odoo/blob/v8.0.19/testing/resources/test_repo/broken_module/coding_latin.py#L1 This python module is not imported and has no effect + - https://github.com/OCA/pylint-odoo/blob/v8.0.19/testing/resources/test_repo/broken_module/encoding_utf8.py#L1 This python module is not imported and has no effect + - https://github.com/OCA/pylint-odoo/blob/v8.0.19/testing/resources/test_repo/broken_module/interpreter_wox.py#L1 This python module is not imported and has no effect + * use-vim-comment - https://github.com/OCA/pylint-odoo/blob/v8.0.19/testing/resources/test_repo/broken_module/pylint_oca_broken.py#L108 Use of vim comment diff --git a/src/pylint_odoo/checkers/__init__.py b/src/pylint_odoo/checkers/__init__.py index 7585f2b5..ba82641b 100644 --- a/src/pylint_odoo/checkers/__init__.py +++ b/src/pylint_odoo/checkers/__init__.py @@ -1,3 +1 @@ -from . import odoo_addons -from . import vim_comment -from . import custom_logging +from . import custom_logging, import_checker, odoo_addons, vim_comment diff --git a/src/pylint_odoo/checkers/import_checker.py b/src/pylint_odoo/checkers/import_checker.py new file mode 100644 index 00000000..a475d87c --- /dev/null +++ b/src/pylint_odoo/checkers/import_checker.py @@ -0,0 +1,128 @@ +import re +from typing import Union + +from astroid import Import, ImportFrom, Module +from astroid.modutils import modpath_from_file +from pylint.checkers.utils import only_required_for_messages +from pylint.lint import PyLinter + +from .odoo_base_checker import OdooBaseChecker + +ODOO_MSGS = { + # C->convention R->refactor W->warning E->error F->fatal + "E8401": ( + "This python module is not imported and has no effect", + "unused-module", + "", + ), +} + +EXCLUDED = re.compile("__manifest__.py$|__openerp__.py$|__init__.py$|tests/|migrations/|tests\\\\|migrations\\\\") + + +class ImportChecker(OdooBaseChecker): + msgs = ODOO_MSGS + name = "odoolint" + + def __init__(self, linter: PyLinter): + self.imports = set() + self.modules = set() + + super().__init__(linter) + + @staticmethod + def get_module_from_node(node, max_attempts: int = 15) -> Union[None, Module]: + """Get the module a node belongs to. + + :param node: Node that belongs to the module to be extracted. + :param int max_attempts: Amount of attempts that will be made to obtain the module. Nodes that are + nested deeper than max_attempts won't be found. + + :returns: Module if found, otherwise None. + """ + attempts = 0 + while attempts < max_attempts: + if not getattr(node, "parent", False): + return None + + if isinstance(node.parent, Module): + return node.parent + + node = node.parent + attempts += 1 + + return None + + def store_imported_modules(self, node): + """Store all modules that are imported by 'from x import y' and 'import x,y,z' statements. + + Relative paths are taken into account for ImportFrom nodes. For example, the following statements + would import the following modules (consider the file which contains these statements is module.models.partner): + + from ..wizard import cap, hello --> module.wizard, module.wizard.cap, module.wizard.hello + from ..utils.legacy import db --> module.utils.legacy, module.utils.legacy.db + from . import sale_order --> module.models.sale_order + + import foo --> module.models.foo + """ + module = ImportChecker.get_module_from_node(node) + if not module: + return + if "tests" in module.name: + return + + if isinstance(node, ImportFrom): + level = 0 if not node.level else node.level + elif isinstance(node, Import): + level = 1 + else: + raise ValueError("Node must be either ImportFrom or Import") + + module_name = ".".join(modpath_from_file(module.file)) + if level > module_name.count("."): + return + + slice_index = 0 + current_level = 0 + for char in reversed(module_name): + if current_level >= level: + break + if char == ".": + current_level += 1 + + slice_index += 1 + + if slice_index != 0: + root_module = module_name[:-slice_index] + else: + root_module = module_name + + modname_separator = "" + modname = getattr(node, "modname", "") + if getattr(node, "modname", False): + self.imports.add(f"{root_module}.{modname}") + modname_separator = "." + + for name in node.names: + self.imports.add(f"{root_module}{modname_separator}{modname}.{name[0]}") + + @only_required_for_messages("unused-module") + def visit_module(self, node): + if EXCLUDED.search(node.file): + return + + self.modules.add(node) + + @only_required_for_messages("unused-module") + def visit_importfrom(self, node): + self.store_imported_modules(node) + + @only_required_for_messages("unused-module") + def visit_import(self, node): + self.store_imported_modules(node) + + @only_required_for_messages("unused-module") + def close(self): + for module in self.modules: + if module.name not in self.imports: + self.add_message("unused-module", node=module) diff --git a/src/pylint_odoo/plugin.py b/src/pylint_odoo/plugin.py index d35fd379..61014355 100644 --- a/src/pylint_odoo/plugin.py +++ b/src/pylint_odoo/plugin.py @@ -7,6 +7,7 @@ def register(linter): linter.register_checker(checkers.odoo_addons.OdooAddons(linter)) linter.register_checker(checkers.vim_comment.VimComment(linter)) linter.register_checker(checkers.custom_logging.CustomLoggingChecker(linter)) + linter.register_checker(checkers.import_checker.ImportChecker(linter)) # register any checking fiddlers apply_augmentations(linter) @@ -18,6 +19,7 @@ def get_all_messages(): all_msgs.update(checkers.odoo_addons.ODOO_MSGS) all_msgs.update(checkers.vim_comment.ODOO_MSGS) all_msgs.update(checkers.custom_logging.ODOO_MSGS) + all_msgs.update(checkers.import_checker.ODOO_MSGS) return all_msgs diff --git a/testing/resources/test_repo/unused_module/__init__.py b/testing/resources/test_repo/unused_module/__init__.py new file mode 100644 index 00000000..0650744f --- /dev/null +++ b/testing/resources/test_repo/unused_module/__init__.py @@ -0,0 +1 @@ +from . import models diff --git a/testing/resources/test_repo/unused_module/__manifest__.py b/testing/resources/test_repo/unused_module/__manifest__.py new file mode 100644 index 00000000..a2e8b78e --- /dev/null +++ b/testing/resources/test_repo/unused_module/__manifest__.py @@ -0,0 +1,10 @@ +{ + 'name': 'Test Import Messages', + 'license': 'AGPL-3', + 'author': 'Vauxoo, Odoo Community Association (OCA)', + 'version': '11.0.0.0.0', + 'depends': [ + 'base', + ], + 'data': [], +} diff --git a/testing/resources/test_repo/unused_module/controllers/__init__.py b/testing/resources/test_repo/unused_module/controllers/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/testing/resources/test_repo/unused_module/controllers/test.py b/testing/resources/test_repo/unused_module/controllers/test.py new file mode 100644 index 00000000..a3d2bfa7 --- /dev/null +++ b/testing/resources/test_repo/unused_module/controllers/test.py @@ -0,0 +1,4 @@ +class MyController(odoo.http.Controller): + @odoo.http.route('/some_url', auth='public') + def handler(self): + return True diff --git a/testing/resources/test_repo/unused_module/migrations/11.0.1.0.1/pre-migration.py b/testing/resources/test_repo/unused_module/migrations/11.0.1.0.1/pre-migration.py new file mode 100644 index 00000000..2f9a147d --- /dev/null +++ b/testing/resources/test_repo/unused_module/migrations/11.0.1.0.1/pre-migration.py @@ -0,0 +1 @@ +print("Hello") diff --git a/testing/resources/test_repo/unused_module/models/__init__.py b/testing/resources/test_repo/unused_module/models/__init__.py new file mode 100644 index 00000000..91fed54d --- /dev/null +++ b/testing/resources/test_repo/unused_module/models/__init__.py @@ -0,0 +1 @@ +from . import res_partner diff --git a/testing/resources/test_repo/unused_module/models/fail_model.py b/testing/resources/test_repo/unused_module/models/fail_model.py new file mode 100644 index 00000000..72302a29 --- /dev/null +++ b/testing/resources/test_repo/unused_module/models/fail_model.py @@ -0,0 +1,6 @@ +from odoo.models import BaseModel + +class FailModel(BaseModel): + _name = "fail.model" + + not_imported = fields.Boolean(default=True) diff --git a/testing/resources/test_repo/unused_module/models/foo.py b/testing/resources/test_repo/unused_module/models/foo.py new file mode 100644 index 00000000..d117c22c --- /dev/null +++ b/testing/resources/test_repo/unused_module/models/foo.py @@ -0,0 +1 @@ +THREE = 2 + 1 diff --git a/testing/resources/test_repo/unused_module/models/res_partner.py b/testing/resources/test_repo/unused_module/models/res_partner.py new file mode 100644 index 00000000..e4b955b4 --- /dev/null +++ b/testing/resources/test_repo/unused_module/models/res_partner.py @@ -0,0 +1,11 @@ +from odoo import fields +from odoo.models import BaseModel + +from ..useful import USEFUL +import foo + + +class ResPartner(BaseModel): + _name = "res.partner" + + random_field = fields.Char(string=USEFUL, size=foo.THREE) diff --git a/testing/resources/test_repo/unused_module/models/unused_utils.py b/testing/resources/test_repo/unused_module/models/unused_utils.py new file mode 100644 index 00000000..fcc47b20 --- /dev/null +++ b/testing/resources/test_repo/unused_module/models/unused_utils.py @@ -0,0 +1 @@ +RANDOM_STRING = "HELLO" diff --git a/testing/resources/test_repo/unused_module/tests/__init__.py b/testing/resources/test_repo/unused_module/tests/__init__.py new file mode 100644 index 00000000..d57d215f --- /dev/null +++ b/testing/resources/test_repo/unused_module/tests/__init__.py @@ -0,0 +1 @@ +from . import test_res_partner diff --git a/testing/resources/test_repo/unused_module/tests/test_res_partner.py b/testing/resources/test_repo/unused_module/tests/test_res_partner.py new file mode 100644 index 00000000..bbca1da3 --- /dev/null +++ b/testing/resources/test_repo/unused_module/tests/test_res_partner.py @@ -0,0 +1,7 @@ +from ..models import unused_utils + + +class TestResPartner: + + def test_something(self): + return unused_utils.RANDOM_STRING diff --git a/testing/resources/test_repo/unused_module/useful.py b/testing/resources/test_repo/unused_module/useful.py new file mode 100644 index 00000000..fdf077f1 --- /dev/null +++ b/testing/resources/test_repo/unused_module/useful.py @@ -0,0 +1 @@ +USEFUL = "foo" diff --git a/testing/resources/test_repo/unused_module/wizard/__init__.py b/testing/resources/test_repo/unused_module/wizard/__init__.py new file mode 100644 index 00000000..7860d9dc --- /dev/null +++ b/testing/resources/test_repo/unused_module/wizard/__init__.py @@ -0,0 +1 @@ +from . import pass_wizard diff --git a/testing/resources/test_repo/unused_module/wizard/fail_wizard.py b/testing/resources/test_repo/unused_module/wizard/fail_wizard.py new file mode 100644 index 00000000..ae9ca7a9 --- /dev/null +++ b/testing/resources/test_repo/unused_module/wizard/fail_wizard.py @@ -0,0 +1,7 @@ +from odoo import fields, models + + +class FailWizard(models.AbstractModel): + _name = "fail.wizard" + + name = fields.Char() diff --git a/testing/resources/test_repo/unused_module/wizard/pass_wizard.py b/testing/resources/test_repo/unused_module/wizard/pass_wizard.py new file mode 100644 index 00000000..75a9f109 --- /dev/null +++ b/testing/resources/test_repo/unused_module/wizard/pass_wizard.py @@ -0,0 +1,13 @@ +from odoo.models import AbstractModel +from odoo import fields + +from .utils import func + + +class PassWizard(AbstractModel): + _name = "pass.wizard" + + name = fields.Char(required=True) + + def foo(self): + return func(self) diff --git a/testing/resources/test_repo/unused_module/wizard/utils.py b/testing/resources/test_repo/unused_module/wizard/utils.py new file mode 100644 index 00000000..035005cf --- /dev/null +++ b/testing/resources/test_repo/unused_module/wizard/utils.py @@ -0,0 +1,2 @@ +def func(*_args): + return False diff --git a/tests/test_main.py b/tests/test_main.py index 60ac9e4f..929de937 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -381,6 +381,11 @@ def test_160_check_only_disabled_one_check(self): expected_errors.pop(disable_error) self.assertDictEqual(real_errors, expected_errors) + def test_165_unused_module(self): + extra_params = ["--disable=all", "--enable=unused-module"] + pylint_res = self.run_pylint([os.path.join(self.root_path_modules, "unused_module")], extra_params) + self.assertDictEqual(pylint_res.linter.stats.by_msg, {"unused-module": 4}) + @staticmethod def re_replace(sub_start, sub_end, substitution, content): re_sub = re.compile(rf"^{re.escape(sub_start)}$.*^{re.escape(sub_end)}$", re.M | re.S)