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 1, 2023
1 parent dd98797 commit 910a8ee
Show file tree
Hide file tree
Showing 16 changed files with 132 additions and 10 deletions.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
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
58 changes: 58 additions & 0 deletions src/pylint_odoo/checkers/import_checker.py
Original file line number Diff line number Diff line change
@@ -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)
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
5 changes: 5 additions & 0 deletions testing/resources/test_repo/imports_module/README.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
This module is designed to test messages related to import errors.
Expected Messages:

* missing-python-import: 2

1 change: 1 addition & 0 deletions testing/resources/test_repo/imports_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/imports_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': [],
}
2 changes: 2 additions & 0 deletions testing/resources/test_repo/imports_module/models/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
from . import res_partner
from . import project_task
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)
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from odoo import fields
from odoo.models import BaseModel


class ResPartner(BaseModel):
_name = "res.partner"

random_field = fields.Char()
Empty file.
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()
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from odoo.models import AbstractModel
from odoo import fields


class PassWizard(AbstractModel):
_name = "pass.wizard"

name = fields.Char(required=True)
Empty file.
24 changes: 17 additions & 7 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -63,6 +64,7 @@
"translation-unsupported-format": 1,
"use-vim-comment": 1,
"website-manifest-key-not-valid-uri": 1,
"missing-python-import": 2,
}


Expand All @@ -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",
Expand Down Expand Up @@ -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):
Expand All @@ -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",
}
Expand All @@ -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)

Expand All @@ -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)

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

0 comments on commit 910a8ee

Please sign in to comment.