Skip to content

Commit

Permalink
[ADD] new unused-module check
Browse files Browse the repository at this point in the history
Fixes Vauxoo#177

This new check looks for python files (modules) which are not referenced
anywhere in the codebase, making  them useless.
  • Loading branch information
antonag32 committed Jun 2, 2023
1 parent 5baba79 commit 27ce79e
Show file tree
Hide file tree
Showing 20 changed files with 189 additions and 3 deletions.
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions src/pylint_odoo/checkers/__init__.py
Original file line number Diff line number Diff line change
@@ -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
108 changes: 108 additions & 0 deletions src/pylint_odoo/checkers/import_checker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import re

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=15):
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):
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)
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': [],
}
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 @@ -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)
Expand Down

0 comments on commit 27ce79e

Please sign in to comment.