Update from OCA/master (#180)

* [REF] invalid-name, unused-argument: Suppress check for migrations path (OCA#348)

The following path:
 - module/x.y.z/migrations/pre-migration.py
has a few false negatives

Considering that standard method is:
    def migrate(cr, version):

- invalid-name (C0103) for module and argument name
  - e.g. "pre-migration.py" instead of pre_migration.py
  - e.g. "cr" for cursor

- unused-argument (W0613) for argument
  - e.g. "version" for version of Odoo

They are valid since that Odoo has already defined as it

So supressing from augmentations

Fix OCA#338

* [ADD] context-overridden: Better using kwargs instead of dictionary (OCA#256)

More info about see docstring of the method:
  - https://github.com/odoo/odoo/blob/ff3064258a320e4008b3fb52a149e6751ae1e7f5/odoo/models.py#L5086-L5105

Using
  - `self.with_context({'key': 'value'})`

The original context is overridden

It should be:
  - `self.with_context(**{'key': 'value'})`
  - `self.with_context(key='value')`

If you need to override the full context only silent this check using:
  - `self.with_context({'key': 'value'})  # pylint: disable=context-overridden`
In order to allow an explicit overridden

* [ADD] dangerous-qweb-replace-wo-priority: Consider qweb view templates (OCA#347)

More info about:
 - https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#inherited-xml

Fix OCA#323

* [REF] README: Auto-update Using pylint_odoo.messages2rst()

* [REF] requirements: Upgrade pylint version (OCA#349)

Add compatibility for pylint between 2.6 and 2.9

The method `_is_in_ignore_list_re` changed for each version

* [FIX] sql-injection: AttributeError: 'NoneType' object has no attribute 'parent' (OCA#350)

Using the following code:

    queries = [
        "SELECT id FROM res_partner",
        "SELECT id FROM res_users",
    ]
    for query in queries:
        self.env.cr.execute(query)

The check sql-injection shows the following error:
 - AttributeError: 'NoneType' object has no attribute 'parent'

So, Now it is validating if it is not None

* [REF] sql-injection: No sql-injection using constants (OCA#351)

The following examples should not be considered as sql-injection:

    self.env.cr.execute("SELECT * FROM %s" % 'table_constant')
    self.env.cr.execute("SELECT * FROM {}".format('table_constant'))
    self.env.cr.execute("SELECT * FROM %(table_variable)s" % {'table_variable': 'table_constant'})

Since that the constant is not possible to inject

* [FIX] sql-injection: AttributeError: 'NoneType' object has no attribute 'parent'

Using the following code:

    queries = [
        "SELECT id FROM res_partner",
        "SELECT id FROM res_users",
    ]
    for query in queries:
        self.env.cr.execute(query)

The check sql-injection shows the following error:
 - AttributeError: 'NoneType' object has no attribute 'parent'

So, Now it is validating if it is not None

* [FIX] sql-injection: Fix false positive using BinOp "+"

Considering the following valid case:

    cr.execute('SELECT ' + operator + ' FROM table' + 'WHERE')

The representation tree is:

    node.repr_tree()
    BinOp(
    op='+',
    left=BinOp(
        op='+',
        left=BinOp(
            op='+',
            left=Const(value='SELECT '),
            right=Name(name='operator')),
        right=Const(value=' FROM table')),
    right=Const(value='WHERE'))

Notice that left node is another BinOp node
So, it need to be considered recursively

* [REF] manifest-version-format: Add new valid odoo v15.0 (OCA#353)

* [REF] requirements: Upgrade pylint version 2.11.1 (OCA#354)

* [REF] .travis.yml: Fix syntax error

.travis.yml

@@ -47,7 +47,6 @@ after_success:
     - coveralls
     - python setup.py sdist  # Build ChangeLog file from git log
 
-<<<<<<< HEAD
 # deploy:
 #   skip_cleanup: true  # Allow to upload Changelog generated from tox
 #   provider: pypi

README.rst

@@ -97,6 +97,8 @@ Enable custom checks for Odoo modules.
 +-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------+
 | W7940 | %s Dangerous use of "replace" from view with priority %s < %s                                                                                                                                                                                                                                                                                       | dangerous-view-replace-wo-priority   |
 +-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------+
+| W7941 | %s Dangerous use of "replace" from view with priority %s < %s                                                                                                                                                                                                                                                                                       | dangerous-qweb-replace-wo-priority   |
++-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------+
 | W7942 | %s Deprecated <tree> xml attribute "%s"                                                                                                                                                                                                                                                                                                             | xml-deprecated-tree-attribute        |
 +-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------+
 | W7943 | %s Deprecated QWeb directive "%s". Use "t-options" instead                                                                                                                                                                                                                                                                                          | xml-deprecated-qweb-directive        |
@@ -137,6 +139,8 @@ Enable custom checks for Odoo modules.
 +-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------+
 | W8120 | Translation method _(%s) is using positional string printf formatting. Use named placeholder ``_("%%(placeholder)s")`` instead.                                                                                                                                                                                                                     | translation-positional-used          |
 +-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------+
+| W8121 | Context overridden using dict. Better using kwargs ``with_context(**%s)`` or ``with_context(key=value)``                                                                                                                                                                                                                                            | context-overridden                   |
++-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------+
 | W8201 | You have a python file with execution permissions but you don't have an interpreter magic comment, or a magic comment but no execution permission. If you really needs a execution permission then add a magic comment ( https://en.wikipedia.org/wiki/Shebang_(Unix) ). If you don't needs a execution permission then remove it with: chmod -x %s | incoherent-interpreter-exec-perm     |
 +-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------+
 | W8202 | Use of vim comment                                                                                                                                                                                                                                                                                                                                  | use-vim-comment                      |

pylint_odoo/augmentations/main.py

@@ -1,9 +1,12 @@
 
 import os
 
-from pylint_plugin_utils import suppress_message
-from pylint.checkers.base import BasicChecker
+from astroid import FunctionDef, Module
+from pylint.checkers.base import BasicChecker, NameChecker
 from pylint.checkers.imports import ImportsChecker
+from pylint.checkers.variables import VariablesChecker
+from pylint_plugin_utils import suppress_message
+
 from .. import settings
 
 
@@ -44,6 +47,35 @@ def is_openerp_import(node):
     return False
 
 
+def is_migration_path(node):
+    """module/x.y.z/migrations/pre-migration.py path has a few false negatives
+
+    Considering that standard method is:
+        def migrate(cr, version):
+
+    - invalid-name (C0103) for module and argument name
+    e.g. "pre-migration.py" instead of pre_migration.py
+    e.g. "cr" for cursor
+
+    - unused-argument (W0613) for argument
+    e.g. "version" for version of Odoo
+
+    node: can be module or functiondef
+    """
+
+    # get 'migrations' from 'module/migrations/x.y.z/pre-migration.py'
+    if os.path.basename(os.path.dirname(os.path.dirname(
+            node.root().file))) != 'migrations':
+        return False
+
+    # pre-migration.py
+    if (isinstance(node, Module) and '-' in node.name or
+            # def migrate(cr, version):
+            isinstance(node, FunctionDef) and node.name == 'migrate'):
+        return True
+    return False
+
+
 def apply_augmentations(linter):
     """Apply suppression rules."""
 
@@ -68,3 +100,9 @@ def apply_augmentations(linter):
     discard = hasattr(ImportsChecker, 'visit_from') and \
         ImportsChecker.visit_from or ImportsChecker.visit_importfrom
     suppress_message(linter, discard, 'E0401', is_openerp_import)
+
+    # C0103 - invalid-name and W0613 - unused-argument for migrations/
+    suppress_message(linter, NameChecker.visit_module, 'C0103', is_migration_path)
+    suppress_message(linter, NameChecker.visit_functiondef, 'C0103', is_migration_path)
+    suppress_message(linter, VariablesChecker.leave_functiondef, 'W0613',
+                     is_migration_path)

pylint_odoo/checkers/modules_odoo.py

@@ -105,6 +105,12 @@
         'dangerous-view-replace-wo-priority',
         settings.DESC_DFLT
     ),
+    'W%d41' % settings.BASE_OMODULE_ID: (
+        '%s Dangerous use of "replace" from view '
+        'with priority %s < %s',
+        'dangerous-qweb-replace-wo-priority',
+        settings.DESC_DFLT
+    ),
     'W%d30' % settings.BASE_OMODULE_ID: (
         '%s not used from manifest',
         'file-not-used',
@@ -906,6 +912,34 @@ def _check_dangerous_view_replace_wo_priority(self):
             return False
         return True
 
+    def _check_dangerous_qweb_replace_wo_priority(self):
+        """Check dangerous qweb view defined with low priority
+        :return: False if exists errors and
+                 add list of errors in self.msg_args
+        """
+        self.msg_args = []
+        xml_files = self.filter_files_ext('xml')
+        for xml_file in self._skip_files_ext('.xml', xml_files):
+            xml_file_path = os.path.join(self.module_path, xml_file)
+
+            # view template
+            xml_doc = self.parse_xml(xml_file_path)
+            for template in xml_doc.xpath("/odoo//template|/openerp//template"):
+                try:
+                    priority = int(template.get('priority'))
+                except (ValueError, TypeError):
+                    priority = 0
+                for child in template.iterchildren():
+                    if (child.get('position') == 'replace' and
+                            priority < self.config.min_priority):
+                        self.msg_args.append((
+                            "%s:%s" % (xml_file, template.sourceline), priority,
+                            self.config.min_priority))
+                        break
+        if self.msg_args:
+            return False
+        return True
+
     def _check_create_user_wo_reset_password(self):
         """Check xml records of user without the context
         'context="{'no_reset_password': True}"'

pylint_odoo/checkers/no_modules.py

@@ -246,6 +246,12 @@
         'translation-positional-used',
         settings.DESC_DFLT
     ),
+    'W%d21' % settings.BASE_NOMODULE_ID: (
+        'Context overridden using dict. '
+        'Better using kwargs `with_context(**%s)` or `with_context(key=value)`',
+        'context-overridden',
+        settings.DESC_DFLT
+    ),
     'F%d01' % settings.BASE_NOMODULE_ID: (
         'File "%s": "%s" not found.',
         'resource-not-exist',
@@ -468,7 +474,10 @@ def _sqli_allowable(self, node):
         # it's a common pattern of reports (self._select, self._group_by, ...)
         return (isinstance(node, astroid.Attribute)
                 and isinstance(node.expr, astroid.Name)
-                and node.attrname.startswith('_'))
+                and node.attrname.startswith('_')
+                # cr.execute('SELECT * FROM %s' % 'table') is OK
+                # since that is a constant and constant can not be injected
+                or isinstance(node, astroid.Const))
 
     def _is_psycopg2_sql(self, node):
         if isinstance(node, astroid.Name):
@@ -480,7 +489,8 @@ def _is_psycopg2_sql(self, node):
             return False
         imported_name = node.func.as_string().split('.')[0]
         imported_node = node.root().locals.get(imported_name)
-        # TODO: Consider "from psycopg2 import *"?
+        # "from psycopg2 import *" not considered since that it is hard
+        # and there is another check detecting these kind of imports
         if not imported_node:
             return None
         imported_node = imported_node[0]
@@ -507,6 +517,22 @@ def _check_node_for_sqli_risk(self, node):
                 # execute("..." % self._table)
                 return True
 
+            # Consider cr.execute('SELECT ' + operator + ' FROM table' + 'WHERE')"
+            # node.repr_tree()
+            # BinOp(
+            #    op='+',
+            #    left=BinOp(
+            #       op='+',
+            #       left=BinOp(
+            #          op='+',
+            #          left=Const(value='SELECT '),
+            #          right=Name(name='operator')),
+            #       right=Const(value=' FROM table')),
+            #    right=Const(value='WHERE'))
+            if (not self._sqli_allowable(node.left) and
+                    self._check_node_for_sqli_risk(node.left)):
+                return True
+
         # check execute("...".format(self._table, table=self._table))
         # ignore sql.SQL().format
         if isinstance(node, astroid.Call) \
@@ -558,12 +584,12 @@ def _get_assignation_nodes(self, node):
             current = node
             while (current and not isinstance(current.parent, astroid.FunctionDef)):
                 current = current.parent
-            parent = current.parent
-
-            # 2) check how was the variable built
-            for assign_node in parent.nodes_of_class(astroid.Assign):
-                if assign_node.targets[0].as_string() == node.as_string():
-                    yield assign_node.value
+            if current:
+                parent = current.parent
+                # 2) check how was the variable built
+                for assign_node in parent.nodes_of_class(astroid.Assign):
+                    if assign_node.targets[0].as_string() == node.as_string():
+                        yield assign_node.value
 
     @utils.check_messages("print-used")
     def visit_print(self, node):
@@ -578,7 +604,7 @@ def visit_print(self, node):
                           'translation-required',
                           'translation-contains-variable',
                           'print-used', 'translation-positional-used',
-                          'str-format-used',
+                          'str-format-used', 'context-overridden',
                           )
     def visit_call(self, node):
         infer_node = utils.safe_infer(node.func)
@@ -652,6 +678,15 @@ def visit_call(self, node):
                 self.get_cursor_name(node.func) in self.config.cursor_expr:
             self.add_message('invalid-commit', node=node)
 
+        if (isinstance(node, astroid.Call) and
+                isinstance(node.func, astroid.Attribute) and
+                node.func.attrname == 'with_context' and
+                not node.keywords and node.args):
+            # with_context(**ctx) is considered a keywords
+            # So, if only one args is received it is overridden
+            self.add_message('context-overridden', node=node,
+                             args=(node.args[0].as_string(),))
+
         # Call the message_post()
         base_dirname = os.path.basename(os.path.normpath(
             os.path.dirname(self.linter.current_file)))

pylint_odoo/misc.py

@@ -14,7 +14,23 @@
 from pylint.checkers import BaseChecker, BaseTokenChecker
 from pylint.interfaces import UNDEFINED
 from pylint.interfaces import IAstroidChecker, ITokenChecker
-from pylint.utils import _basename_in_blacklist_re
+try:
+    # sha 5805a73 pylint 2.9
+    from pylint.lint.expand_modules import _is_in_ignore_list_re
+except ImportError:
+    try:
+        # sha 63ca0597 pylint 2.8
+        from pylint.lint.expand_modules import (
+            _basename_in_ignore_list_re as _is_in_ignore_list_re)
+    except ImportError:
+        try:
+            # sha d19c77337 pylint 2.7
+            from pylint.utils.utils import (
+                _basename_in_ignore_list_re as _is_in_ignore_list_re)
+        except ImportError:
+            # Compatibility with pylint<=2.6.0
+            from pylint.utils import (
+                _basename_in_blacklist_re as _is_in_ignore_list_re)
 from restructuredtext_lint import lint_file as rst_lint
 from translate.misc import wStringIO
 from translate.storage import po, factory, pypo
@@ -40,7 +56,7 @@
 
 DFTL_VALID_ODOO_VERSIONS = [
     '4.2', '5.0', '6.0', '6.1', '7.0', '8.0', '9.0', '10.0', '11.0', '12.0',
-    '13.0', '14.0',
+    '13.0', '14.0', '15.0',
 ]
 DFTL_MANIFEST_VERSION_FORMAT = r"({valid_odoo_versions})\.\d+\.\d+\.\d+$"
 
@@ -204,8 +220,7 @@ def set_ext_files(self):
                 fext = os.path.splitext(filename)[1].lower()
                 fname = os.path.join(root, filename)
                 # If the file is within black_list_re is ignored
-                if _basename_in_blacklist_re(fname,
-                                             self.linter.config.black_list_re):
+                if _is_in_ignore_list_re(fname, self.linter.config.black_list_re):
                     continue
                 # If the file is within ignores is ignored
                 find = False

pylint_odoo/test/main.py

@@ -19,10 +19,12 @@
     'attribute-deprecated': 3,
     'class-camelcase': 1,
     'consider-merging-classes-inherited': 2,
+    'context-overridden': 3,
     'copy-wo-api-one': 2,
     'create-user-wo-reset-password': 1,
     'dangerous-filter-wo-user': 1,
     'dangerous-view-replace-wo-priority': 6,
+    'dangerous-qweb-replace-wo-priority': 2,
     'deprecated-openerp-xml-node': 5,
     'development-status-allowed': 1,
     'duplicate-id-csv': 2,
@@ -398,6 +400,35 @@ def test_130_odoo_namespace_repo(self):
             {"po-msgstr-variables": 1, "missing-readme": 1}
         )
 
+    def test_140_check_suppress_migrations(self):
+        """Test migrations path supress checks"""
+        extra_params = [
+            '--disable=all',
+            '--enable=invalid-name,unused-argument',
+        ]
+        path_modules = [os.path.join(
+            os.path.dirname(os.path.dirname(os.path.realpath(__file__))),
+            'test_repo', 'test_module', 'migrations', '10.0.1.0.0', 'pre-migration.py')]
+
+        # Messages suppressed with plugin for migration
+        pylint_res = self.run_pylint(path_modules, extra_params)
+        real_errors = pylint_res.linter.stats['by_msg']
+        expected_errors = {
+            'invalid-name': 1,
+            'unused-argument': 1,
+        }
+        self.assertDictEqual(real_errors, expected_errors)
+
+        # Messages raised without plugin
+        self.default_options.remove('--load-plugins=pylint_odoo')
+        pylint_res = self.run_pylint(path_modules, extra_params)
+        real_errors = pylint_res.linter.stats['by_msg']
+        expected_errors = {
+            'invalid-name': 3,
+            'unused-argument': 2,
+        }
+        self.assertDictEqual(real_errors, expected_errors)
+
 
 if __name__ == '__main__':
     unittest.main()

pylint_odoo/test_repo/broken_module/__openerp__.py

@@ -15,7 +15,8 @@
         'skip_xml_check.xml',
         'skip_xml_check_2.xml',
         'skip_xml_check_3.xml',
-        'report.xml'
+        'report.xml',
+        'template1.xml',
     ],
     'demo': ['demo/duplicated_id_demo.xml', 'file_no_exist.xml'],
     'test': ['file_no_exist.yml'],