Skip to content

Commit

Permalink
[ADD] deprecated-odoo-model-method: New check to detect deprecated me…
Browse files Browse the repository at this point in the history
…thod for each Odoo version (#470)

* [ADD] deprecated-odoo-model-method: New check to detect deprecated method for each Odoo version (#470)

This new check looks for model methods that have been deprecated and
should not be overriden in model classes. It takes into account odoo
versions and comes with sane defaults which can be replaced by a custom
list in the configuration file.

Closes #469.

* [REF] tests: support custom rcfiles

Tests can now specify their own rcfiles, previously it was always
/dev/null (no rcfile). This makes it possible to test the behavior of
tests when options are provided through rcfiles.

There is also no need to supply /dev/null as a rcfile. Pylint
internally uses a default RC file if not provided. It also makes it
redundant to use `_add_rcfile_default_pylintrc` in tests.
  • Loading branch information
antonag32 committed Oct 2, 2023
1 parent 84513d2 commit 3926faa
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 67 deletions.
108 changes: 57 additions & 51 deletions README.md

Large diffs are not rendered by default.

53 changes: 49 additions & 4 deletions src/pylint_odoo/checkers/odoo_addons.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
import validators
from astroid import ClassDef, FunctionDef, NodeNG, nodes
from pylint.checkers import BaseChecker, utils
from pylint.lint import PyLinter

from .. import misc
from .odoo_base_checker import OdooBaseChecker
Expand Down Expand Up @@ -251,6 +252,11 @@
"bad-builtin-groupby",
CHECK_DESCRIPTION,
),
"W8160": (
"%s has been deprecated by Odoo. Please look for alternatives.",
"deprecated-odoo-model-method",
CHECK_DESCRIPTION,
),
}

DFTL_MANIFEST_REQUIRED_KEYS = ["license"]
Expand Down Expand Up @@ -346,6 +352,8 @@
"suds.client.Client",
"urllib.request.urlopen",
]
DFTL_DEPRECATED_ODOO_MODEL_METHODS = {"16.0": {"fields_view_get"}}

# Regex used from https://github.com/translate/translate/blob/9de0d72437/translate/filters/checks.py#L50-L62 # noqa
PRINTF_PATTERN = re.compile(
r"""
Expand Down Expand Up @@ -517,6 +525,15 @@ class OdooAddons(OdooBaseChecker, BaseChecker):
"help": "List of valid odoo versions separated by a comma.",
},
),
(
"deprecated-odoo-model-methods",
{
"type": "string",
"metavar": "python_expression",
"default": "",
"help": "Dictionary consisting of versions (keys) and methods that have been marked as deprecated.",
},
),
)

checks_maxmin_odoo_version = {
Expand All @@ -527,8 +544,13 @@ class OdooAddons(OdooBaseChecker, BaseChecker):
"no-raise-unlink": {"odoo_minversion": "15.0"},
}

def __init__(self, linter: PyLinter):
super().__init__(linter)
self._deprecated_odoo_methods = set()

def close(self):
"""Final process get all cached values and add messages"""
self.linter.config.deprecated_odoo_model_methods = set()
for (_manifest_path, odoo_class_inherit), inh_nodes in self._odoo_inherit_items.items():
# Skip _inherit='other.model' _name='model.name' because is valid
inh_nodes = {
Expand Down Expand Up @@ -567,6 +589,16 @@ def open(self):
self.linter.config.deprecated_field_parameters
)

if self.linter.config.deprecated_odoo_model_methods:
deprecated_model_methods = ast.literal_eval(self.linter.config.deprecated_odoo_model_methods)
else:
deprecated_model_methods = DFTL_DEPRECATED_ODOO_MODEL_METHODS

max_valid_version = float(sorted(self.linter.config.valid_odoo_versions, key=float)[-1])
for version, checks in deprecated_model_methods.items():
if float(version) <= max_valid_version:
self._deprecated_odoo_methods.update(checks)

def colon_list_to_dict(self, colon_list):
"""Converts a colon list to a dictionary.
Expand Down Expand Up @@ -1077,10 +1109,20 @@ def visit_dict(self, node):
):
self.add_message("manifest-maintainers-list", node=manifest_keys_nodes.get("maintainers") or node)

@utils.only_required_for_messages(
"method-required-super",
"missing-return",
)
def check_deprecated_odoo_method(self, node: NodeNG) -> bool:
"""Verify the given method is not marked as deprecated under the set Odoo versions.
:param node: Function definition to be checked
:return: True if the method is deprecated, false otherwise.
"""
try:
if not self.get_odoo_models_class(node.parent):
return False
except AttributeError:
return False

return node.name in self._deprecated_odoo_methods

@utils.only_required_for_messages("method-required-super", "missing-return", "deprecated-odoo-model-method")
def visit_functiondef(self, node):
"""Check that `api.one` and `api.multi` decorators not exists together
Check that method `copy` exists `api.one` decorator
Expand All @@ -1089,6 +1131,9 @@ def visit_functiondef(self, node):
if not node.is_method():
return

if self.is_odoo_message_enabled("deprecated-odoo-model-method") and self.check_deprecated_odoo_method(node):
self.add_message("deprecated-odoo-model-method", node=node, args=(node.name,))

if node.name in self.linter.config.method_required_super:
calls = [
call_func.func.name
Expand Down
5 changes: 5 additions & 0 deletions testing/resources/.pylintrc-odoo-deprecated-model-methods
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[ODOOLINT]
deprecated-odoo-model-methods={
'15.0': {'custom_deprecated_method_just_because', 'another_deprecated_model_method'},
'16.0': {'fields_view_get'},
}
18 changes: 15 additions & 3 deletions testing/resources/test_repo/broken_module/models/broken_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ def function_no_method():
return broken_model1, broken_module1, broken_module2, broken_model2


def fields_view_get():
return "Not part of a model, i'm free"

class TestModel2(odoo.models.Model):
def _compute_name2(self):
# Compute called from string with write defined before
Expand All @@ -82,9 +85,18 @@ def _compute_name2(self):
f_obj.write("write file allowed")
unknown_type_object = self._get_object()
unknown_type_object.write('write not self.browse allowed')

name2 = fields.Char(compute='_compute_name2')

def fields_view_get(self):
return "Deprecated in Odoo 16.0!!!"

def custom_deprecated_method_just_because(self):
return "this method is deprecated because i said so in the options" + self.name2

def another_deprecated_model_method(self):
return self.name2

class TestModel(models.Model):
_name = 'test.model'

Expand All @@ -111,7 +123,7 @@ def _compute_name(self):
unknown_type_object = self._get_object()
unknown_type_object.write('write not self.browse allowed')
self.write({"name": "hello"}) # pylint: disable=no-write-in-compute

def _compute_with_method_def(self):
# Compute called from funct-def with write
self.write({"name": "hello"})
Expand Down Expand Up @@ -458,7 +470,7 @@ def my_method13(self):
'String with params format %(p1)s') % {'p1': 'v1'})
raise exceptions.Warning(_(
'String with params format %(p1)s' % {'p1': 'v1'}))

def my_method14(self):
_("String with missing args %s %s", "param1")
_("String with missing kwargs %(param1)s", param2="hola")
Expand Down
3 changes: 3 additions & 0 deletions testing/resources/test_repo/eleven_module/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,6 @@ class EleveModel(models.Model):
def method1(self):
self.const = isinstance(astroid.Const, Const)
self.bo = isinstance(astroid.BinOp, bo)

def fields_view_get(self):
return self.bo
34 changes: 25 additions & 9 deletions tests/test_main.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import os
import re
import sys
Expand All @@ -10,7 +12,7 @@

import pytest
from pylint.reporters.text import TextReporter
from pylint.testutils._run import _add_rcfile_default_pylintrc, _Run as Run
from pylint.testutils._run import _Run as Run
from pylint.testutils.utils import _patch_streams

from pylint_odoo import __version__ as version, plugin
Expand Down Expand Up @@ -64,6 +66,7 @@
"use-vim-comment": 1,
"website-manifest-key-not-valid-uri": 1,
"no-raise-unlink": 2,
"deprecated-odoo-model-method": 2,
}


Expand All @@ -74,7 +77,7 @@ def setUp(self):
"--reports=no",
"--score=no",
"--msg-template={path}:{line} {msg} - [{symbol}]",
"--rcfile=%s" % os.devnull,
"--persistent=no",
]
self.root_path_modules = os.path.join(
os.path.dirname(os.path.dirname(os.path.realpath(__file__))), "testing", "resources", "test_repo"
Expand All @@ -99,12 +102,16 @@ def setUp(self):
def tearDown(self):
sys.path = list(self.sys_path_origin)

def run_pylint(self, paths, extra_params=None, verbose=False):
def run_pylint(self, paths, extra_params: list | None = None, verbose=False, rcfile: str = ""):
for path in paths:
if not os.path.exists(path):
raise OSError('Path "{path}" not found.'.format(path=path))

if extra_params is None:
extra_params = self.default_extra_params
if rcfile:
extra_params.append(f"--rcfile={rcfile}")

sys.path.extend(paths)
cmd = self.default_options + extra_params + paths
reporter = TextReporter(StringIO())
Expand All @@ -117,8 +124,6 @@ def run_pylint(self, paths, extra_params=None, verbose=False):

@staticmethod
def _run_pylint(args, out, reporter):
# copied directly from pylint tests/test_self.py
args = _add_rcfile_default_pylintrc(args + ["--persistent=no"])
with _patch_streams(out):
with pytest.raises(SystemExit) as ctx_mgr:
with warnings.catch_warnings():
Expand Down Expand Up @@ -149,6 +154,7 @@ def test_25_checks_excluding_by_odoo_version(self):
"translation-too-many-args",
"translation-unsupported-format",
"no-raise-unlink",
"deprecated-odoo-model-method",
}
self.default_extra_params += ["--valid-odoo-versions=13.0"]
pylint_res = self.run_pylint(self.paths_modules)
Expand All @@ -166,10 +172,7 @@ def test_35_checks_emiting_by_odoo_version(self):
real_errors = pylint_res.linter.stats.by_msg
expected_errors = self.expected_errors.copy()
expected_errors.update({"manifest-version-format": 6})
excluded_msgs = {
"no-raise-unlink",
"translation-contains-variable",
}
excluded_msgs = {"no-raise-unlink", "translation-contains-variable", "deprecated-odoo-model-method"}
for excluded_msg in excluded_msgs:
expected_errors.pop(excluded_msg)
self.assertEqual(expected_errors, real_errors)
Expand Down Expand Up @@ -379,6 +382,19 @@ def test_165_no_raises_unlink(self):
extra_params.append("--valid-odoo-versions=14.0")
self.assertFalse(self.run_pylint([test_repo], extra_params).linter.stats.by_msg)

def test_option_odoo_deprecated_model_method(self):
pylint_res = self.run_pylint(
self.paths_modules,
rcfile=os.path.abspath(
os.path.join(__file__, "..", "..", "testing", "resources", ".pylintrc-odoo-deprecated-model-methods")
),
)

self.assertEqual(
4,
pylint_res.linter.stats.by_msg["deprecated-odoo-model-method"],
)

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

Please sign in to comment.