From 910a8eea46070607605c003450fc11455b27db7c 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 | 6 ++ src/pylint_odoo/checkers/__init__.py | 4 +- src/pylint_odoo/checkers/import_checker.py | 58 +++++++++++++++++++ src/pylint_odoo/plugin.py | 2 + .../test_repo/imports_module/README.rst | 5 ++ .../test_repo/imports_module/__init__.py | 1 + .../test_repo/imports_module/__manifest__.py | 10 ++++ .../imports_module/models/__init__.py | 2 + .../imports_module/models/fail_model.py | 6 ++ .../imports_module/models/res_partner.py | 8 +++ .../imports_module/models/unused_utils.py | 0 .../imports_module/wizard/__init__.py | 1 + .../imports_module/wizard/fail_wizard.py | 7 +++ .../imports_module/wizard/pass_wizard.py | 8 +++ .../test_repo/imports_module/wizard/utils.py | 0 tests/test_main.py | 24 +++++--- 16 files changed, 132 insertions(+), 10 deletions(-) create mode 100644 src/pylint_odoo/checkers/import_checker.py create mode 100644 testing/resources/test_repo/imports_module/README.rst create mode 100644 testing/resources/test_repo/imports_module/__init__.py create mode 100644 testing/resources/test_repo/imports_module/__manifest__.py create mode 100644 testing/resources/test_repo/imports_module/models/__init__.py create mode 100644 testing/resources/test_repo/imports_module/models/fail_model.py create mode 100644 testing/resources/test_repo/imports_module/models/res_partner.py create mode 100644 testing/resources/test_repo/imports_module/models/unused_utils.py create mode 100644 testing/resources/test_repo/imports_module/wizard/__init__.py create mode 100644 testing/resources/test_repo/imports_module/wizard/fail_wizard.py create mode 100644 testing/resources/test_repo/imports_module/wizard/pass_wizard.py create mode 100644 testing/resources/test_repo/imports_module/wizard/utils.py diff --git a/README.md b/README.md index 38baa814..bd18ec5c 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,7 @@ method-compute | Name of compute method should start with "_compute_" | C8108 method-inverse | Name of inverse method should start with "_inverse_" | C8110 method-required-super | Missing `super` call in "%s" method. | W8106 method-search | Name of search method should start with "_search_" | C8109 +missing-python-import | This python file is not imported and has no effect | E8401 missing-readme | Missing ./README.rst file. Template here: %s | C8112 missing-return | Missing `return` (`super` is used) in method %s. | W8110 no-wizard-in-models | No wizard class for model directory. See the complete structure https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#complete-structure | C8113 @@ -239,6 +240,11 @@ Checks valid only for odoo <= 13.0 - https://github.com/OCA/pylint-odoo/blob/v8.0.17/testing/resources/test_repo/broken_module/models/broken_model.py#L144 Name of search method should start with "_search_" + * missing-python-import + + - https://github.com/OCA/pylint-odoo/blob/v8.0.17/testing/resources/test_repo/imports_module/models/fail_model.py#L1 This python file is not imported and has no effect + - https://github.com/OCA/pylint-odoo/blob/v8.0.17/testing/resources/test_repo/imports_module/wizard/fail_wizard.py#L1 This python file is not imported and has no effect + * missing-readme - https://github.com/OCA/pylint-odoo/blob/v8.0.17/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 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..6a37cba1 --- /dev/null +++ b/src/pylint_odoo/checkers/import_checker.py @@ -0,0 +1,58 @@ +import re +from os.path import basename, dirname + +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$") + + +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 + else: + 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/imports_module/README.rst b/testing/resources/test_repo/imports_module/README.rst new file mode 100644 index 00000000..39a90cdd --- /dev/null +++ b/testing/resources/test_repo/imports_module/README.rst @@ -0,0 +1,5 @@ +This module is designed to test messages related to import errors. +Expected Messages: + +* missing-python-import: 2 + diff --git a/testing/resources/test_repo/imports_module/__init__.py b/testing/resources/test_repo/imports_module/__init__.py new file mode 100644 index 00000000..0650744f --- /dev/null +++ b/testing/resources/test_repo/imports_module/__init__.py @@ -0,0 +1 @@ +from . import models diff --git a/testing/resources/test_repo/imports_module/__manifest__.py b/testing/resources/test_repo/imports_module/__manifest__.py new file mode 100644 index 00000000..a2e8b78e --- /dev/null +++ b/testing/resources/test_repo/imports_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/imports_module/models/__init__.py b/testing/resources/test_repo/imports_module/models/__init__.py new file mode 100644 index 00000000..c58b9039 --- /dev/null +++ b/testing/resources/test_repo/imports_module/models/__init__.py @@ -0,0 +1,2 @@ +from . import res_partner +from . import project_task diff --git a/testing/resources/test_repo/imports_module/models/fail_model.py b/testing/resources/test_repo/imports_module/models/fail_model.py new file mode 100644 index 00000000..72302a29 --- /dev/null +++ b/testing/resources/test_repo/imports_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/imports_module/models/res_partner.py b/testing/resources/test_repo/imports_module/models/res_partner.py new file mode 100644 index 00000000..62b16417 --- /dev/null +++ b/testing/resources/test_repo/imports_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/imports_module/models/unused_utils.py b/testing/resources/test_repo/imports_module/models/unused_utils.py new file mode 100644 index 00000000..e69de29b diff --git a/testing/resources/test_repo/imports_module/wizard/__init__.py b/testing/resources/test_repo/imports_module/wizard/__init__.py new file mode 100644 index 00000000..7860d9dc --- /dev/null +++ b/testing/resources/test_repo/imports_module/wizard/__init__.py @@ -0,0 +1 @@ +from . import pass_wizard diff --git a/testing/resources/test_repo/imports_module/wizard/fail_wizard.py b/testing/resources/test_repo/imports_module/wizard/fail_wizard.py new file mode 100644 index 00000000..ae9ca7a9 --- /dev/null +++ b/testing/resources/test_repo/imports_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/imports_module/wizard/pass_wizard.py b/testing/resources/test_repo/imports_module/wizard/pass_wizard.py new file mode 100644 index 00000000..10c2bc5a --- /dev/null +++ b/testing/resources/test_repo/imports_module/wizard/pass_wizard.py @@ -0,0 +1,8 @@ +from odoo.models import AbstractModel +from odoo import fields + + +class PassWizard(AbstractModel): + _name = "pass.wizard" + + name = fields.Char(required=True) diff --git a/testing/resources/test_repo/imports_module/wizard/utils.py b/testing/resources/test_repo/imports_module/wizard/utils.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_main.py b/tests/test_main.py index c45566f3..58842b67 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -6,7 +6,8 @@ from collections import defaultdict from glob import glob from io import StringIO -from tempfile import NamedTemporaryFile +from tempfile import NamedTemporaryFile, TemporaryDirectory +from textwrap import dedent import pytest from pylint.reporters.text import TextReporter @@ -63,6 +64,7 @@ "translation-unsupported-format": 1, "use-vim-comment": 1, "website-manifest-key-not-valid-uri": 1, + "missing-python-import": 2, } @@ -75,11 +77,11 @@ def setUp(self): "--msg-template={path}:{line} {msg} - [{symbol}]", "--rcfile=%s" % os.devnull, ] - path_modules = os.path.join( + self.root_path_modules = os.path.join( os.path.dirname(os.path.dirname(os.path.realpath(__file__))), "testing", "resources", "test_repo" ) # Similar to pre-commit way - self.paths_modules = glob(os.path.join(path_modules, "**", "*.py"), recursive=True) + self.paths_modules = glob(os.path.join(self.root_path_modules, "**", "*.py"), recursive=True) self.odoo_namespace_addons_path = os.path.join( os.path.dirname(os.path.dirname(os.path.realpath(__file__))), "testing", @@ -154,7 +156,7 @@ def test_25_checks_excluding_by_odoo_version(self): expected_errors = self.expected_errors.copy() for excluded_msg in excluded_msgs: expected_errors.pop(excluded_msg) - expected_errors.update({"manifest-version-format": 6}) + expected_errors.update({"manifest-version-format": 7}) self.assertEqual(expected_errors, real_errors) def test_35_checks_emiting_by_odoo_version(self): @@ -163,7 +165,7 @@ def test_35_checks_emiting_by_odoo_version(self): pylint_res = self.run_pylint(self.paths_modules) real_errors = pylint_res.linter.stats.by_msg expected_errors = self.expected_errors.copy() - expected_errors.update({"manifest-version-format": 6}) + expected_errors.update({"manifest-version-format": 7}) excluded_msgs = { "translation-contains-variable", } @@ -182,7 +184,7 @@ def test_85_valid_odoo_version_format(self): pylint_res = self.run_pylint(self.paths_modules, extra_params) real_errors = pylint_res.linter.stats.by_msg expected_errors = { - "manifest-version-format": 6, + "manifest-version-format": 7, } self.assertDictEqual(real_errors, expected_errors) @@ -206,7 +208,7 @@ def test_90_valid_odoo_versions(self): pylint_res = self.run_pylint(self.paths_modules, extra_params) real_errors = pylint_res.linter.stats.by_msg expected_errors = { - "manifest-version-format": 6, + "manifest-version-format": 7, } self.assertDictEqual(real_errors, expected_errors) @@ -363,6 +365,14 @@ 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, "imports_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)