Skip to content

Commit

Permalink
[FIX] Exclude missing-return checks for computed methods
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
antonag32 committed Jan 9, 2023
1 parent 0ac19f4 commit d94b6ec
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 11 deletions.
38 changes: 27 additions & 11 deletions src/pylint_odoo/checkers/odoo_addons.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -361,7 +362,6 @@


class OdooAddons(OdooBaseChecker, BaseChecker):

_from_imports = None
name = "odoolint"
msgs = ODOO_MSGS
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"
Expand Down
16 changes: 16 additions & 0 deletions testing/resources/test_repo/test_module/missing-return-compute.py
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit d94b6ec

Please sign in to comment.