[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

@@ -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

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

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/")
+
+
+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]:
+        f"""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)

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
 
 

testing/resources/test_repo/unused_module/__init__.py

@@ -0,0 +1 @@
+from . import models

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': [],
+}

testing/resources/test_repo/unused_module/migrations/11.0.1.0.1/pre-migration.py

@@ -0,0 +1 @@
+print("Hello")

testing/resources/test_repo/unused_module/models/__init__.py

@@ -0,0 +1 @@
+from . import res_partner

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)

testing/resources/test_repo/unused_module/models/foo.py

@@ -0,0 +1 @@
+THREE = 2 + 1

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)

testing/resources/test_repo/unused_module/models/unused_utils.py

@@ -0,0 +1 @@
+RANDOM_STRING = "HELLO"

testing/resources/test_repo/unused_module/tests/__init__.py

@@ -0,0 +1 @@
+from . import test_res_partner

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

testing/resources/test_repo/unused_module/useful.py

@@ -0,0 +1 @@
+USEFUL = "foo"

testing/resources/test_repo/unused_module/wizard/__init__.py

@@ -0,0 +1 @@
+from . import pass_wizard

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()

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)

testing/resources/test_repo/unused_module/wizard/utils.py

@@ -0,0 +1,2 @@
+def func(*_args):
+    return False

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)