Skip to content

Commit

Permalink
[ADD] New missing-python-import check
Browse files Browse the repository at this point in the history
This new check finds all files that have a class which inherits
from one of Odoo's models classes and ensures they are imported.

This is required since classes that interact with the ORM
(create models, or inherit them) MUST be imported in the
corresponding __init__.py file, otherwise they won't have
an effect.
  • Loading branch information
antonag32 committed Nov 23, 2022
1 parent dd98797 commit d862e56
Show file tree
Hide file tree
Showing 15 changed files with 228 additions and 5 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
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
78 changes: 78 additions & 0 deletions src/pylint_odoo/checkers/import_checker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
from functools import lru_cache
from pathlib import Path
from typing import Set

from astroid.nodes import ImportFrom, Module
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 file is not imported and has no effect",
"missing-python-import",
"Import it on the corresponding __init__.py file",
),
}


class ImportChecker(OdooBaseChecker):
msgs = ODOO_MSGS
name = "odoolint"

def __init__(self, linter: PyLinter):
self.imports = {}
self.orm_files = {}

super().__init__(linter)

@lru_cache()
def _get_odoo_import_orm_names(self, module: Module) -> Set[str]:
results = set()
for import_stmt in filter(lambda node: isinstance(node, ImportFrom), module.body):
if import_stmt.modname == "odoo":
if any("models" in name for name in import_stmt.names):
results.add("models")
elif import_stmt.modname == "odoo.models":
results.update([name[0] for name in import_stmt.names])

return results

@only_required_for_messages("missing-python-import")
def visit_classdef(self, node) -> None:
odoo_imports = self._get_odoo_import_orm_names(node.parent)
if "models" in odoo_imports:
not_orm = not any(
(basename.startswith("models.") or basename in odoo_imports) for basename in node.basenames
)
else:
not_orm = not any(basename in odoo_imports for basename in node.basenames)

if not_orm:
return

node_dir = str(Path(node.parent.file).parent)
if not self.orm_files.get(node_dir, False):
self.orm_files[node_dir] = set()

self.orm_files[node_dir].add(node.parent)

@only_required_for_messages("missing-python-import")
def visit_importfrom(self, node) -> None:
if node.modname or Path(node.parent.file).name != "__init__.py":
return

node_dir = str(Path(node.parent.file).parent)
if not self.imports.get(node_dir, False):
self.imports[node_dir] = set()

for name in node.names:
self.imports[node_dir].add(name[0])

def close(self) -> None:
for key, values in self.orm_files.items():
for value in values:
if Path(value.file).name[:-3] not in self.imports.get(key, {}):
self.add_message("missing-python-import", node=value)
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)
14 changes: 14 additions & 0 deletions testing/resources/test_repo/imports_module/models/project_task.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from odoo import models, fields
import argparse


class MyParser(argparse.ArgumentParser):

def random_function(self):
return f"{self.name} -- This class does not need importing. But the other one in this file does!"


class ProjectTask(models.Model):
_name = "project.task"

random_field = fields.Boolean()
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()
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)
84 changes: 79 additions & 5 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 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,78 @@ def test_160_check_only_disabled_one_check(self):
expected_errors.pop(disable_error)
self.assertDictEqual(real_errors, expected_errors)

def test_165_missing_python_import(self):
extra_params = ["--disable=all", "--enable=missing-python-import"]

odoo_classdef = dedent(
"""
from {base_odoo_import} import {odoo_imports}
class {class_name}({base_class}):
_name = "{odoo_model}"
def random_function(self):
return self.name
"""
)

util_classdef = dedent(
"""
from argparse import ArgumentParser
class UtilClass(ArgumentParser):
def util_method(self):
return "Very util output"
"""
)

py_orm = "res_partner.py"
py_orm2 = "sale_order.py"
py_orm3 = "project_task.py"
py_no_orm = "utils.py"
with TemporaryDirectory() as tmpdir:
with open(os.path.join(tmpdir, py_orm), "w", encoding="utf-8") as py_orm_fd:
py_orm_fd.write(
odoo_classdef.format(
base_odoo_import="odoo",
odoo_imports="models",
class_name="ResPartner",
base_class="models.Model",
odoo_model="res.partner",
)
)

with open(os.path.join(tmpdir, py_orm2), "w", encoding="utf-8") as py_orm2_fd:
py_orm2_fd.write(
odoo_classdef.format(
base_odoo_import="odoo",
odoo_imports="models",
class_name="SaleOrder",
base_class="models.Model",
odoo_model="sale.order",
)
)
with open(os.path.join(tmpdir, py_orm3), "w", encoding="utf-8") as py_orm3_fd:
py_orm3_fd.write(
odoo_classdef.format(
base_odoo_import="odoo.models",
odoo_imports="Model",
class_name="ProjectTask",
base_class="Model",
odoo_model="project.task",
)
)

with open(os.path.join(tmpdir, py_no_orm), "w", encoding="utf-8") as py_no_orm_fd:
py_no_orm_fd.write(util_classdef)

with open(os.path.join(tmpdir, "__init__.py"), "w", encoding="utf-8") as init_fd:
init_fd.write("from . import sale_order\nfrom pathlib import Path\n")

pylint_res = self.run_pylint([tmpdir], extra_params).linter.stats.by_msg
self.assertDictEqual(pylint_res, {"missing-python-import": 2})

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

Please sign in to comment.