From 5aa8f7c57a103d5db546f7ab6a7d369e60f42a53 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 | 2 + src/pylint_odoo/checkers/__init__.py | 4 +- src/pylint_odoo/checkers/import_checker.py | 57 +++++++++++++++++++ src/pylint_odoo/plugin.py | 2 + .../test_repo/unused_module/__init__.py | 1 + .../test_repo/unused_module/__manifest__.py | 10 ++++ .../unused_module/models/__init__.py | 1 + .../unused_module/models/fail_model.py | 6 ++ .../unused_module/models/res_partner.py | 8 +++ .../unused_module/models/unused_utils.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 ++ 15 files changed, 117 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/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/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/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..53c0f01e 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 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..7b0229bc --- /dev/null +++ b/src/pylint_odoo/checkers/import_checker.py @@ -0,0 +1,57 @@ +import re + +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", + "", + ), +} + +UNUSED_MODULE_EXCLUDE = re.compile("migrations/|__init__.py$|__manifest__.py$|__openerp__.py$") + + +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_package(node) -> str: + if "." in node.parent.name: + return node.parent.name[: node.parent.name.rfind(".")] + + return node.parent.name + + @only_required_for_messages("unused-module") + def visit_module(self, node) -> None: + if UNUSED_MODULE_EXCLUDE.search(node.file): + return + + self.modules.add(node) + + @only_required_for_messages("unused-module") + def visit_importfrom(self, node) -> None: + if node.modname: + self.imports.add(f"{self.get_package(node)}.{node.modname}") + return + + for name in node.names: + self.imports.add(f"{node.parent.name}.{name[0]}") + + @only_required_for_messages("unused-module") + def close(self) -> None: + 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/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/res_partner.py b/testing/resources/test_repo/unused_module/models/res_partner.py new file mode 100644 index 00000000..62b16417 --- /dev/null +++ b/testing/resources/test_repo/unused_module/models/res_partner.py @@ -0,0 +1,8 @@ +from odoo import fields +from odoo.models import BaseModel + + +class ResPartner(BaseModel): + _name = "res.partner" + + random_field = fields.Char() 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/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 17dd5d05..55f0d973 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": 3}) + @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)