Skip to content

Commit

Permalink
[ADD] new unused-module check
Browse files Browse the repository at this point in the history
This new check looks for python files (modules) which are not referenced
anywhere in the codebase, making  them useless.

Closes Vauxoo#177.
  • Loading branch information
antonag32 committed Feb 14, 2024
1 parent 813663d commit a45128f
Show file tree
Hide file tree
Showing 22 changed files with 210 additions and 0 deletions.
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,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

Expand Down Expand Up @@ -257,6 +258,7 @@ Checks valid only for odoo <= 13.0
* missing-readme

- https://github.com/OCA/pylint-odoo/blob/v9.0.5/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/v9.0.5/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

Expand Down Expand Up @@ -292,6 +294,7 @@ Checks valid only for odoo <= 13.0
* print-used

- https://github.com/OCA/pylint-odoo/blob/v9.0.5/testing/resources/test_repo/test_module/except_pass.py#L20 Print used. Use `logger` instead.
- https://github.com/OCA/pylint-odoo/blob/v9.0.5/testing/resources/test_repo/unused_module/migrations/11.0.1.0.1/pre-migration.py#L1 Print used. Use `logger` instead.

* renamed-field-parameter

Expand Down Expand Up @@ -371,6 +374,12 @@ Checks valid only for odoo <= 13.0

- https://github.com/OCA/pylint-odoo/blob/v9.0.5/testing/resources/test_repo/broken_module/models/broken_model.py#L478 Unsupported odoo._ format character 'y' (0x79) at index 30

* unused-module

- https://github.com/OCA/pylint-odoo/blob/v9.0.5/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/v9.0.5/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/v9.0.5/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/v9.0.5/testing/resources/test_repo/broken_module/pylint_oca_broken.py#L108 Use of vim comment
Expand Down
1 change: 1 addition & 0 deletions src/pylint_odoo/checkers/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from . import odoo_addons
from . import vim_comment
from . import custom_logging
from . import import_checker
125 changes: 125 additions & 0 deletions src/pylint_odoo/checkers/import_checker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
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/|docs/|tests\\\\|migrations\\\\|docs\\\\"
)


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]:
"""Obtain the module which contains the given node.
:param node: Node that belongs to the module which wil be obtained.
:param int max_attempts: Number 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.
"""
for _attempt in range(max_attempts):
if not getattr(node, "parent", False):
return None

if isinstance(node.parent, Module):
return node.parent

node = node.parent

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 or "tests" in module.name:
return

if isinstance(node, ImportFrom):
level = node.level or 0
elif isinstance(node, Import):
level = 1
else:
return

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

root_module = module_name[:-slice_index] if slice_index else 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)
2 changes: 2 additions & 0 deletions src/pylint_odoo/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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


Expand Down
1 change: 1 addition & 0 deletions testing/resources/test_repo/unused_module/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from . import models
10 changes: 10 additions & 0 deletions testing/resources/test_repo/unused_module/__manifest__.py
Original file line number Diff line number Diff line change
@@ -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': [],
}
Empty file.
4 changes: 4 additions & 0 deletions testing/resources/test_repo/unused_module/controllers/test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class MyController(odoo.http.Controller):
@odoo.http.route('/some_url', auth='public')
def handler(self):
return True
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
print("Hello")
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from . import res_partner
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from odoo.models import BaseModel

class FailModel(BaseModel):
_name = "fail.model"

not_imported = fields.Boolean(default=True)
1 change: 1 addition & 0 deletions testing/resources/test_repo/unused_module/models/foo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
THREE = 2 + 1
11 changes: 11 additions & 0 deletions testing/resources/test_repo/unused_module/models/res_partner.py
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
RANDOM_STRING = "HELLO"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from . import test_res_partner
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from ..models import unused_utils


class TestResPartner:

def test_something(self):
return unused_utils.RANDOM_STRING
1 change: 1 addition & 0 deletions testing/resources/test_repo/unused_module/useful.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
USEFUL = "foo"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from . import pass_wizard
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from odoo import fields, models


class FailWizard(models.AbstractModel):
_name = "fail.wizard"

name = fields.Char()
13 changes: 13 additions & 0 deletions testing/resources/test_repo/unused_module/wizard/pass_wizard.py
Original file line number Diff line number Diff line change
@@ -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)
2 changes: 2 additions & 0 deletions testing/resources/test_repo/unused_module/wizard/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def func(*_args):
return False
5 changes: 5 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,11 @@ def test_option_odoo_deprecated_model_method(self):
pylint_res.linter.stats.by_msg["deprecated-odoo-model-method"],
)

def test_175_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)
Expand Down

0 comments on commit a45128f

Please sign in to comment.