[FIX] *: support for pylint 4

- astroid 4 deprecates toplevel exports of nodes, thankfully that was
  never actually necessary so we can just import that unconditionally
- remove support for pre-jammy pylint / astroid, specifically
  `astroid.nodes` was added in astroid 2.7.0 and
  `astroid.node_classes` deprecated then and removed in 3.0, this can
  affect Bullseye users as it shipped with astroid 2.5
- Astroid 4 changes `spec.Finder.find_module` in order to cache it
  (pylint-dev/astroid#2509), we can just make our method static for
  all versions as we don't need `self` anyway.
- The mail test triggers `function-redefined` (E0102), fix it.
- Skip the escpos script thing which triggers a bunch of
  `undefined-variable` (E0602) false positives.

closes odoo#236993

X-original-commit: 628b693
Related: odoo/enterprise#100228
Signed-off-by: Xavier Morel (xmo) <[email protected]>

Author: xmo-odoo

addons/lunch/models/lunch_product.py

@@ -90,13 +90,13 @@ def _sync_active_from_related(self):
         self.filtered(lambda p: not p.active and (p.category_id.active and p.supplier_id.active)).action_unarchive()
 
     @api.constrains('active', 'category_id')
-    def _check_active(self):
+    def _check_active_categories(self):
         invalid_products = self.filtered(lambda product: product.active and not product.category_id.active)
         if invalid_products:
             raise UserError(_("The following product categories are archived. You should either unarchive the categories or change the category of the product.\n%s", '\n'.join(invalid_products.category_id.mapped('name'))))
 
     @api.constrains('active', 'supplier_id')
-    def _check_active(self):
+    def _check_active_suppliers(self):
         invalid_products = self.filtered(lambda product: product.active and not product.supplier_id.active)
         if invalid_products:
             raise UserError(_("The following suppliers are archived. You should either unarchive the suppliers or change the supplier of the product.\n%s", '\n'.join(invalid_products.supplier_id.mapped('name'))))

addons/test_mail/tests/test_mail_mail.py

@@ -1,3 +1,4 @@
+
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 import pytz
@@ -744,16 +745,16 @@ def _connect(*args, **kwargs):
         # SMTP sending issues
         with self.mock_mail_gateway():
             _send_current = self.send_email_mocked.side_effect
+            self.addCleanup(setattr, self.send_email_mocked, 'side_effect', _send_current)
+
             self._reset_data()
             mail.write({'email_to': '[email protected]'})
 
             # should always raise for those errors, even with raise_exception=False
             for error, error_class in [
                     (smtplib.SMTPServerDisconnected("Some exception"), smtplib.SMTPServerDisconnected),
                     (MemoryError("Some exception"), MemoryError)]:
-                def _send_email(*args, **kwargs):
-                    raise error
-                self.send_email_mocked.side_effect = _send_email
+                self.send_email_mocked.side_effect = error
 
                 with self.assertRaises(error_class):
                     mail.send(raise_exception=False)
@@ -770,9 +771,7 @@ def _send_email(*args, **kwargs):
                     (MailDeliveryException("Some exception"), 'Some exception', 'unknown'),
                     (MailDeliveryException("OutboundSpamException"), 'OutboundSpamException', 'mail_spam'),
                     (ValueError("Unexpected issue"), 'Unexpected issue', 'unknown')]:
-                def _send_email(*args, **kwargs):
-                    raise error
-                self.send_email_mocked.side_effect = _send_email
+                self.send_email_mocked.side_effect = error
 
                 self._reset_data()
                 mail.send(raise_exception=False)
@@ -783,8 +782,6 @@ def _send_email(*args, **kwargs):
                 self.assertEqual(notification.failure_type, failure_type)
                 self.assertEqual(notification.notification_status, 'exception')
 
-            self.send_email_mocked.side_effect = _send_current
-
     def test_mail_mail_values_misc(self):
         """ Test various values on mail.mail, notably default values """
         msg = self.env['mail.mail'].create({})

odoo/addons/test_lint/tests/_odoo_checker_sql_injection.py

@@ -7,12 +7,7 @@
 from contextlib import ExitStack
 from typing import Optional
 
-import astroid
-
-try:
-    from astroid import NodeNG
-except ImportError:
-    from astroid.node_classes import NodeNG
+from astroid import nodes
 
 import pylint.interfaces
 from pylint.checkers import BaseChecker, utils
@@ -40,10 +35,10 @@
 
 function_definitions = collections.defaultdict(list)
 callsites_for_queries = collections.defaultdict(list)
-root_call: contextvars.ContextVar[Optional[astroid.Call]] =\
+root_call: contextvars.ContextVar[Optional[nodes.Call]] =\
     contextvars.ContextVar('root_call', default=None)
 @contextlib.contextmanager
-def push_call(node: astroid.Call):
+def push_call(node: nodes.Call):
     with ExitStack() as s:
         if root_call.get() is None:
             t = root_call.set(node)
@@ -77,39 +72,39 @@ def _infer_filename(self, node):
         return node.file
     def _get_return_node(self, node):
         ret = []
-        nodes = deque([node])
-        while nodes:
-            node = nodes.popleft()
-            if isinstance(node, astroid.Return):
+        q = deque([node])
+        while q:
+            node = q.popleft()
+            if isinstance(node, nodes.Return):
                 ret.append(node)
             else:
-                nodes.extend(node.get_children())
+                q.extend(node.get_children())
         return ret
 
     def _is_asserted(self, node): # If there is an assert on the value of the node, it's very likely to be safe
-        asserted = deque((assert_.test for assert_ in node.scope().nodes_of_class(astroid.Assert)))
+        asserted = deque(assert_.test for assert_ in node.scope().nodes_of_class(nodes.Assert))
         while asserted:
             n = asserted.popleft()
-            if isinstance(n, astroid.Name) and n.name == node.name:
+            if isinstance(n, nodes.Name) and n.name == node.name:
                 return True
             else:
                 asserted.extend(n.get_children())
         return False
 
     def _get_attribute_chain(self, node):
-        if isinstance(node, astroid.Attribute):
+        if isinstance(node, nodes.Attribute):
             return self._get_attribute_chain(node.expr) + '.' + node.attrname
-        elif isinstance(node, astroid.Name):
+        elif isinstance(node, nodes.Name):
             return node.name
-        elif isinstance(node, astroid.Call):
+        elif isinstance(node, nodes.Call):
             return self._get_attribute_chain(node.func)
         return '' #FIXME
 
     def _evaluate_function_call(self, node, args_allowed, position):
-        name = node.func.attrname if isinstance(node.func, astroid.Attribute) else node.func.name
+        name = node.func.attrname if isinstance(node.func, nodes.Attribute) else node.func.name
         if name == 'SQL':
             return True
-        if isinstance(node.scope(), astroid.GeneratorExp):
+        if isinstance(node.scope(), nodes.GeneratorExp):
             return True
         if name == node.scope().name:
             return True
@@ -125,13 +120,13 @@ def _evaluate_function_call(self, node, args_allowed, position):
             )
         return True
 
-    def _is_fstring_cst(self, node: astroid.JoinedStr, args_allowed=False, position=None):
+    def _is_fstring_cst(self, node: nodes.JoinedStr, args_allowed=False, position=None):
         # an fstring is constant if all its FormattedValue are constant, or
         # are access to private attributes (nb: whitelist?)
         return self.all_const((
             node.value for node in node.values
-            if isinstance(node, astroid.FormattedValue)
-            if not (isinstance(node.value, astroid.Attribute) and node.value.attrname.startswith('_'))
+            if isinstance(node, nodes.FormattedValue)
+            if not (isinstance(node.value, nodes.Attribute) and node.value.attrname.startswith('_'))
         ),
             args_allowed=args_allowed,
             position=position
@@ -143,72 +138,70 @@ def all_const(self, nodes, args_allowed=False, position=None):
             for node in nodes
         )
 
-    def _is_constexpr(self, node: NodeNG, *, args_allowed=False, position=None):
-        if isinstance(node, astroid.Const): # astroid.const is always safe
+    def _is_constexpr(self, node: nodes.NodeNG, *, args_allowed=False, position=None):
+        if isinstance(node, nodes.Const):  # astroid.const is always safe
             return True
-        elif isinstance(node, (astroid.List, astroid.Set)):
+        elif isinstance(node, (nodes.List, nodes.Set)):
             return self.all_const(node.elts, args_allowed=args_allowed)
-        elif isinstance(node, astroid.Tuple):
+        elif isinstance(node, nodes.Tuple):
             if position is None:
                 return self.all_const(node.elts, args_allowed=args_allowed)
             else:
                 return self._is_constexpr(node.elts[position], args_allowed=args_allowed)
-        elif isinstance(node, astroid.Dict):
+        elif isinstance(node, nodes.Dict):
             return all(
                 self._is_constexpr(k, args_allowed=args_allowed) and self._is_constexpr(v, args_allowed=args_allowed)
                 for k, v in node.items
             )
-        elif isinstance(node, astroid.Starred):
+        elif isinstance(node, nodes.Starred):
             return self._is_constexpr(node.value, args_allowed=args_allowed, position=position)
-        elif isinstance(node, astroid.BinOp): # recusively infer both side of the operation. Failing if either side is not inferable
+        elif isinstance(node, nodes.BinOp):  # recusively infer both side of the operation. Failing if either side is not inferable
             left_operand = self._is_constexpr(node.left, args_allowed=args_allowed)
             # This case allows to always consider a string formatted with %d to be safe
             if node.op == '%' and \
-                isinstance(node.left, astroid.Const) and \
+                isinstance(node.left, nodes.Const) and \
                 node.left.pytype() == 'builtins.str' and \
                 '%d' in node.left.value and \
                 not '%s' in node.left.value:
                 return True
             right_operand = self._is_constexpr(node.right, args_allowed=args_allowed)
             return left_operand and right_operand
-        elif isinstance(node, astroid.Name) or isinstance(node, astroid.AssignName): # Variable: find the assignement instruction in the AST and infer its value.
-            assignements = node.lookup(node.name)
+        elif isinstance(node, (nodes.Name, nodes.AssignName)):  # Variable: find the assignement instruction in the AST and infer its value.
+            assignment = node.lookup(node.name)
             assigned_node = []
-            for n in assignements[1]: #assignement[0] contains the scope, so assignment[1] contains the assignement nodes
+            for n in assignment[1]:  # assignment[0] contains the scope, so assignment[1] contains the assignement nodes
                 # FIXME: makes no sense, assuming this gets
                 #        `visit_functiondef`'d we should just ignore it
-                if isinstance(n.parent, astroid.FunctionDef):
+                if isinstance(n.parent, (nodes.FunctionDef, nodes.Arguments)):
                     assigned_node += [args_allowed]
-                elif isinstance(n.parent, astroid.Arguments):
-                    assigned_node += [args_allowed]
-                elif isinstance(n.parent, astroid.Tuple): # multi assign a,b = (a,b)
+                elif isinstance(n.parent, nodes.Tuple):  # multi assign a,b = (a,b)
                     statement = n.statement()
-                    if isinstance(statement, astroid.For):
+                    if isinstance(statement, nodes.For):
                         assigned_node += [self._is_constexpr(statement.iter, args_allowed=args_allowed)]
-                    elif isinstance(statement, astroid.Assign):
+                    elif isinstance(statement, nodes.Assign):
                         assigned_node += [self._is_constexpr(statement.value, args_allowed=args_allowed, position=n.parent.elts.index(n))]
                     else:
                         raise TypeError(f"Expected statement Assign or For, got {statement}")
-                elif isinstance(n.parent, astroid.For):
+                elif isinstance(n.parent, nodes.For):
                     assigned_node.append(self._is_constexpr(n.parent.iter, args_allowed=args_allowed))
-                elif isinstance(n.parent, astroid.AugAssign):
+                elif isinstance(n.parent, nodes.AugAssign):
                     left = self._is_constexpr(n.parent.target, args_allowed=args_allowed)
                     right = self._is_constexpr(n.parent.value, args_allowed=args_allowed)
                     assigned_node.append(left and right)
-                elif isinstance(n.parent, astroid.Module):
+                elif isinstance(n.parent, nodes.Module):
                     return True
                 else:
-                    if isinstance(n.parent, astroid.Comprehension):
+                    if isinstance(n.parent, nodes.Comprehension):
                         assigned_node += [self._is_constexpr(n.parent.iter, args_allowed=args_allowed)]
                     else:
                         assigned_node += [self._is_constexpr(n.parent.value, args_allowed=args_allowed)]
             if assigned_node and all(assigned_node):
                 return True
             return self._is_asserted(node)
-        elif isinstance(node, astroid.JoinedStr):
+        elif isinstance(node, nodes.JoinedStr):
             return self._is_fstring_cst(node, args_allowed)
-        elif isinstance(node, astroid.Call):
-            if isinstance(node.func, astroid.Attribute):
+        elif isinstance(node, nodes.Call):
+            if isinstance(node.func, nodes.Attribute):
                 if node.func.attrname == 'append':
                     return self._is_constexpr(node.args[0])
                 elif node.func.attrname == 'format':
@@ -219,16 +212,16 @@ def _is_constexpr(self, node: NodeNG, *, args_allowed=False, position=None):
                     )
             with push_call(node):
                 return self._evaluate_function_call(node, args_allowed=args_allowed, position=position)
-        elif isinstance(node, astroid.IfExp):
+        elif isinstance(node, nodes.IfExp):
             body = self._is_constexpr(node.body, args_allowed=args_allowed)
             orelse = self._is_constexpr(node.orelse, args_allowed=args_allowed)
             return body and orelse
-        elif isinstance(node, astroid.Subscript):
+        elif isinstance(node, nodes.Subscript):
             return self._is_constexpr(node.value, args_allowed=args_allowed)
-        elif isinstance(node, astroid.BoolOp):
+        elif isinstance(node, nodes.BoolOp):
             return self.all_const(node.values, args_allowed=args_allowed)
 
-        elif isinstance(node, astroid.Attribute):
+        elif isinstance(node, nodes.Attribute):
             attr_chain = self._get_attribute_chain(node)
             while attr_chain:
                 if attr_chain in ATTRIBUTE_WHITELIST or attr_chain.startswith('_'):
@@ -243,20 +236,17 @@ def _is_constexpr(self, node: NodeNG, *, args_allowed=False, position=None):
     def _get_cursor_name(self, node):
         expr_list = []
         node_expr = node.expr
-        while isinstance(node_expr, astroid.Attribute):
+        while isinstance(node_expr, nodes.Attribute):
             expr_list.insert(0, node_expr.attrname)
             node_expr = node_expr.expr
-        if isinstance(node_expr, astroid.Name):
+        if isinstance(node_expr, nodes.Name):
             expr_list.insert(0, node_expr.name)
         cursor_name = '.'.join(expr_list)
         return cursor_name
 
-    def _allowable(self, node):
-        """
-        :type node: NodeNG
-        """
+    def _allowable(self, node: nodes.NodeNG) -> bool:
         scope = node.scope()
-        if isinstance(scope, astroid.FunctionDef) and (scope.name.startswith("_") or scope.name == 'init'):
+        if isinstance(scope, nodes.FunctionDef) and (scope.name.startswith("_") or scope.name == 'init'):
             return True
 
         infered = utils.safe_infer(node)
@@ -269,23 +259,23 @@ def _allowable(self, node):
 
         # self._thing is OK (mostly self._table), self._thing() also because
         # it's a common pattern of reports (self._select, self._group_by, ...)
-        return (isinstance(node, astroid.Attribute)
-            and isinstance(node.expr, astroid.Name)
+        return (isinstance(node, nodes.Attribute)
+            and isinstance(node.expr, nodes.Name)
             and node.attrname.startswith('_')
         )
 
-    def _check_concatenation(self, node):
+    def _check_concatenation(self, node: nodes.NodeNG) -> bool | None:
         node = self.resolve(node)
 
         if self._allowable(node):
             return False
 
-        if isinstance(node, astroid.BinOp) and node.op in ('%', '+'):
-            if isinstance(node.right, astroid.Tuple):
+        if isinstance(node, nodes.BinOp) and node.op in ('%', '+'):
+            if isinstance(node.right, nodes.Tuple):
                 # execute("..." % (self._table, thing))
                 if not all(map(self._allowable, node.right.elts)):
                     return True
-            elif isinstance(node.right, astroid.Dict):
+            elif isinstance(node.right, nodes.Dict):
                 # execute("..." % {'table': self._table}
                 if not all(self._allowable(v) for _, v in node.right.items):
                     return True
@@ -308,8 +298,8 @@ def _check_concatenation(self, node):
             return self._check_concatenation(node.left)
 
         # check execute("...".format(self._table, table=self._table))
-        if isinstance(node, astroid.Call) \
-                and isinstance(node.func, astroid.Attribute) \
+        if isinstance(node, nodes.Call) \
+                and isinstance(node.func, nodes.Attribute) \
                 and node.func.attrname == 'format':
 
             return not (
@@ -318,18 +308,20 @@ def _check_concatenation(self, node):
             )
 
         # check execute(f'foo {...}')
-        if isinstance(node, astroid.JoinedStr):
+        if isinstance(node, nodes.JoinedStr):
             return not all(
                 self._allowable(formatted.value)
-                for formatted in node.nodes_of_class(astroid.FormattedValue)
+                for formatted in node.nodes_of_class(nodes.FormattedValue)
             )
 
+        return None
+
     def resolve(self, node):
         # if node is a variable, find how it was built
-        if isinstance(node, astroid.Name):
+        if isinstance(node, nodes.Name):
             for target in node.lookup(node.name)[1]:
                 # could also be e.g. arguments (if the source is a function parameter)
-                if isinstance(target.parent, astroid.Assign):
+                if isinstance(target.parent, nodes.Assign):
                     # FIXME: handle multiple results (e.g. conditional assignment)
                     return target.parent.value
         # otherwise just return the original node for checking
@@ -341,9 +333,9 @@ def _check_sql_injection_risky(self, node):
         current_file_bname = os.path.basename(self.linter.current_file)
         if not (
             # .execute() or .executemany()
-            isinstance(node, astroid.Call) and node.args and
-            ((isinstance(node.func, astroid.Attribute) and node.func.attrname in ('execute', 'executemany', 'SQL') and self._get_cursor_name(node.func) in DFTL_CURSOR_EXPR) or
-            (isinstance(node.func, astroid.Name) and node.func.name == 'SQL')) and
+            isinstance(node, nodes.Call) and node.args and
+            ((isinstance(node.func, nodes.Attribute) and node.func.attrname in ('execute', 'executemany', 'SQL') and self._get_cursor_name(node.func) in DFTL_CURSOR_EXPR) or
+            (isinstance(node.func, nodes.Name) and node.func.name == 'SQL')) and
             # ignore in test files, probably not accessible
             not current_file_bname.startswith('test_')
         ):
@@ -387,7 +379,7 @@ def visit_functiondef(self, node):
                 )
 
     @functools.lru_cache(None)
-    def _is_const_def(self, node: astroid.FunctionDef, /, *, position: Optional[int], const_args: bool = False) -> bool:
+    def _is_const_def(self, node: nodes.FunctionDef, /, *, position: Optional[int], const_args: bool = False) -> bool:
         if node.name.startswith('__') or node.name in FUNCTION_WHITELIST:
             return True
 

odoo/addons/test_lint/tests/_pylint_path_setup.py

@@ -43,8 +43,8 @@ class AddonsPackageFinder(spec.Finder):
     forcefully discovered as namespace packages since otherwise they get
     discovered as directory packages.
     """
+    @staticmethod
     def find_module(
-        self,
         modname: str,
         module_parts: Sequence[str],
         processed: list[str],