From d94b6ec5bd4a161cb7c93a37610717cff421645d Mon Sep 17 00:00:00 2001 From: antonag32 Date: Fri, 6 Jan 2023 22:48:54 -0600 Subject: [PATCH] [FIX] Exclude missing-return checks for computed methods Fixes #450. Compute methods are not supposed to return any value, even if there is a super() in it, since super() shouldn't return anything either. The only reliable way to ensure a field is computed is if has been declared in the same python module (file), so this exclusion only applies to a limited set of cases. --- src/pylint_odoo/checkers/odoo_addons.py | 38 +++++++++++++------ .../test_module/missing-return-compute.py | 16 ++++++++ 2 files changed, 43 insertions(+), 11 deletions(-) create mode 100644 testing/resources/test_repo/test_module/missing-return-compute.py diff --git a/src/pylint_odoo/checkers/odoo_addons.py b/src/pylint_odoo/checkers/odoo_addons.py index 87029f9b..5d1aef97 100644 --- a/src/pylint_odoo/checkers/odoo_addons.py +++ b/src/pylint_odoo/checkers/odoo_addons.py @@ -105,8 +105,9 @@ from collections import Counter, defaultdict import validators -from astroid import nodes +from astroid import Const, nodes from pylint.checkers import BaseChecker, utils +from pylint.lint import PyLinter from .. import misc from .odoo_base_checker import OdooBaseChecker @@ -361,7 +362,6 @@ class OdooAddons(OdooBaseChecker, BaseChecker): - _from_imports = None name = "odoolint" msgs = ODOO_MSGS @@ -521,8 +521,15 @@ class OdooAddons(OdooBaseChecker, BaseChecker): }, } + def __init__(self, linter: PyLinter): + super().__init__(linter) + self._odoo_inherit_items = None + self._compute_method_names = set() + def close(self): - """Final process get all cached values and add messages""" + """Final process: cleanup, get all cached values and add messages""" + self._compute_method_names.clear() + 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 = { @@ -733,8 +740,16 @@ def _get_assignation_nodes(self, node): "translation-field", "translation-positional-used", "translation-required", + "missing-return", ) def visit_call(self, node): + # compute method names must be stored to avoid false positives if super is called inside them + if self.linter.is_message_enabled("missing-return"): + for keyword in getattr(node, "keywords", []): + if keyword.arg == "compute" and isinstance(keyword.value, Const): + self._compute_method_names.add(keyword.value.value) + break + if ( self.linter.is_message_enabled("print-used", node.lineno) and isinstance(node.func, nodes.Name) @@ -1099,14 +1114,15 @@ def visit_functiondef(self, node): there_is_super = True break - there_is_return = any(node.nodes_of_class(nodes.Return, skip_klass=(nodes.FunctionDef, nodes.ClassDef))) - if ( - there_is_super - and not there_is_return - and not node.is_generator() - and node.name not in self.linter.config.no_missing_return - ): - self.add_message("missing-return", node=node, args=(node.name)) + if self.linter.is_message_enabled("missing-return") and node.name not in self._compute_method_names: + there_is_return = any(node.nodes_of_class(nodes.Return, skip_klass=(nodes.FunctionDef, nodes.ClassDef))) + if ( + there_is_super + and not there_is_return + and not node.is_generator() + and node.name not in self.linter.config.no_missing_return + ): + self.add_message("missing-return", node=node, args=(node.name)) @utils.only_required_for_messages( "external-request-timeout", "odoo-addons-relative-import", "test-folder-imported" diff --git a/testing/resources/test_repo/test_module/missing-return-compute.py b/testing/resources/test_repo/test_module/missing-return-compute.py new file mode 100644 index 00000000..e6f995ad --- /dev/null +++ b/testing/resources/test_repo/test_module/missing-return-compute.py @@ -0,0 +1,16 @@ +# 'missing-return' not necessary inside compute methods, even if super is called +# we can only make sure the method is used to compute fields if said fields/methods +# are declared in the same python module (file) +from odoo import api, fields, models + + +class MissingReturnCompute(models.Model): + + discount = fields.Float(compute="_compute_discount", store=True) + + @api.depends("order_id", "order_id.general_discount") + def _compute_discount(self): + if hasattr(super(), "_compute_discount"): + super()._compute_discount() + for line in self: + line.discount = line.order_id.general_discount