diff --git a/.flake8 b/.flake8 index ca92e5c3..41390dcb 100644 --- a/.flake8 +++ b/.flake8 @@ -7,6 +7,8 @@ select = C,E,F,W,B,B9 # E203: whitespace before ':' (black behaviour) # E501: flake8 line length (covered by bugbear B950) # W503: line break before binary operator (black behaviour) -ignore = E203,E501,W503 +# W504: line break after binary operator (black behaviour) +# C901: too complex is enabled from pylint +ignore = E203,E501,W503,W504,C901 per-file-ignores= __init__.py:F401 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 04c41cee..562af8f7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,7 +11,7 @@ exclude: | # Library files can have extraneous formatting (even minimized) /static/(src/)?lib/| # test_repo is expected to break lints - test_repo/| + testing/resources/| # Repos using Sphinx to generate docs don't need prettying ^docs/_templates/.*\.html$| # You don't usually want a bot to modify your legal texts diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index a13b0132..abd0a4db 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -1,6 +1,6 @@ -- id: pylint_odoo - name: Check for Odoo modules using pylint - entry: pylint - language: python - types: [python] - require_serial: true +- id: pylint_odoo + name: Check for Odoo modules using pylint + entry: pylint + language: python + types: [python] + require_serial: true diff --git a/.pylintrc b/.pylintrc index 53c615bc..ea4406e6 100644 --- a/.pylintrc +++ b/.pylintrc @@ -17,6 +17,7 @@ disable=c-extension-no-member, missing-class-docstring, missing-function-docstring, missing-module-docstring, + protected-access, suppressed-message, too-few-public-methods, too-many-ancestors, @@ -36,6 +37,15 @@ disable=c-extension-no-member, too-many-return-statements, too-many-star-expressions, too-many-statements, + unused-argument, + # TODO: Re-enable the following one + attribute-defined-outside-init, + consider-using-f-string, + implicit-str-concat, + too-complex, + no-member, + abstract-method, + arguments-differ, [REPORTS] msg-template={path}:{line}: [{msg_id}({symbol}), {obj}] {msg} @@ -54,3 +64,6 @@ ignore-docstrings=yes # but that in some circumstances it may be appropriate to relax the restriction # and permit modules with a complexity as high as 15 max-complexity=15 + +[BASIC] +good-names=maxDiff diff --git a/install.sh b/install.sh deleted file mode 100755 index e8df05a6..00000000 --- a/install.sh +++ /dev/null @@ -1,4 +0,0 @@ -wget -qO- https://deb.nodesource.com/setup | bash - \ - && apt-get install nodejs -npm install -g eslint -pip install . diff --git a/pytest.ini b/pytest.ini index eb679324..27a592e8 100644 --- a/pytest.ini +++ b/pytest.ini @@ -49,4 +49,4 @@ filterwarnings = # You can add exclusions, some examples: # ignore:'pylint_odoo' defines default_app_config:PendingDeprecationWarning:: # ignore:The {{% if::: -# ignore:Coverage disabled via --no-cov switch! \ No newline at end of file +# ignore:Coverage disabled via --no-cov switch! diff --git a/setup.py b/setup.py index cba1d162..67bee75e 100644 --- a/setup.py +++ b/setup.py @@ -1,12 +1,14 @@ -from os.path import basename, dirname, join, splitext import re -from setuptools import find_packages, setup +from os.path import dirname, join + +from setuptools import setup try: from pbr import git except ImportError: git = None + def generate_changelog(): fname = "ChangeLog" if not git: @@ -14,13 +16,13 @@ def generate_changelog(): with open(fname, "w", encoding="UTF-8") as fchg: fchg.write(changelog_str) return changelog_str - # pylint: disable=protected-access changelog = git._iter_log_oneline() changelog = git._iter_changelog(changelog) git.write_git_changelog(changelog=changelog) # git.generate_authors() return read(fname) + def generate_dependencies(): return read("requirements.txt").splitlines() diff --git a/src/pylint_odoo/__init__.py b/src/pylint_odoo/__init__.py index 080ae40e..cb80f3ea 100644 --- a/src/pylint_odoo/__init__.py +++ b/src/pylint_odoo/__init__.py @@ -1,7 +1,7 @@ __version__ = "8.0.0" from . import checkers -from . augmentations.main import apply_augmentations +from .augmentations.main import apply_augmentations def register(linter): @@ -25,11 +25,10 @@ def get_all_messages(): def messages2md(): all_msgs = get_all_messages() - md_msgs = 'Code | Description | short name\n--- | --- | ---' - for msg_code, (title, name_key, description) in \ - sorted(all_msgs.items()): - md_msgs += "\n{0} | {1} | {2}".format(msg_code, title, name_key) - md_msgs += '\n' + md_msgs = "Code | Description | short name\n--- | --- | ---" + for msg_code, (title, name_key, _description) in sorted(all_msgs.items()): + md_msgs += "\n{} | {} | {}".format(msg_code, title, name_key) + md_msgs += "\n" return md_msgs @@ -49,26 +48,32 @@ def messages2rst(): lines = [] max_col_sizes = [len(item) for item in title_list] for msg_code, msg_items in sorted(all_msgs.items()): - title, name_key, description = msg_items[0:3] - line = [item.replace('`', '``') - for item in [msg_code, title, name_key]] + title, name_key, _description = msg_items[0:3] + line = [item.replace("`", "``") for item in [msg_code, title, name_key]] + # TODO: consider using enumerate :) + # pylint: disable=consider-using-enumerate for index in range(len(max_col_sizes)): if len(line[index]) > max_col_sizes[index]: max_col_sizes[index] = len(line[index]) lines.append(line) - def rst_spaces(max_col_sizes, line=None, sep='|', fill=' '): + def rst_spaces(max_col_sizes, line=None, sep="|", fill=" "): if line is None: - line = [''] * len(max_col_sizes) - return ''.join( - [sep + fill + line[index] + - fill * (max_col_sizes[index] - len(line[index]) + 1) - for index in range(len(max_col_sizes))]) + sep + '\n' - rst_msgs = rst_spaces(max_col_sizes, sep='+', fill='-') + line = [""] * len(max_col_sizes) + return ( + "".join( + [ + sep + fill + line[index] + fill * (max_col_sizes[index] - len(line[index]) + 1) + for index in range(len(max_col_sizes)) + ] + ) + + sep + + "\n" + ) + + rst_msgs = rst_spaces(max_col_sizes, sep="+", fill="-") rst_msgs += rst_spaces(max_col_sizes, title_list) - rst_msgs += rst_spaces(max_col_sizes, sep='+', fill='=') - rst_msgs += rst_spaces(max_col_sizes, sep='+', fill='-').join( - [rst_spaces(max_col_sizes, item) for item in lines]) - rst_msgs += rst_spaces(max_col_sizes, sep='+', fill='-') - rst_msgs = rst_msgs + rst_msgs += rst_spaces(max_col_sizes, sep="+", fill="=") + rst_msgs += rst_spaces(max_col_sizes, sep="+", fill="-").join([rst_spaces(max_col_sizes, item) for item in lines]) + rst_msgs += rst_spaces(max_col_sizes, sep="+", fill="-") return rst_msgs diff --git a/src/pylint_odoo/augmentations/main.py b/src/pylint_odoo/augmentations/main.py index 46715d62..828c759b 100644 --- a/src/pylint_odoo/augmentations/main.py +++ b/src/pylint_odoo/augmentations/main.py @@ -1,9 +1,7 @@ - import os 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 @@ -35,14 +33,12 @@ def migrate(cr, version): """ # 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': + 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'): + # def migrate(cr, version): + if isinstance(node, Module) and "-" in node.name or isinstance(node, FunctionDef) and node.name == "migrate": return True return False @@ -52,12 +48,10 @@ def apply_augmentations(linter): # W0104 - pointless-statement # manifest file have a valid pointless-statement dict - discard = hasattr(BasicChecker, 'visit_discard') and \ - BasicChecker.visit_discard or BasicChecker.visit_expr - suppress_message(linter, discard, 'W0104', is_manifest_file) + discard = BasicChecker.visit_discard if hasattr(BasicChecker, "visit_discard") else BasicChecker.visit_expr + suppress_message(linter, discard, "W0104", is_manifest_file) # 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) + 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) diff --git a/src/pylint_odoo/checkers/__init__.py b/src/pylint_odoo/checkers/__init__.py index f8eab67c..3057433a 100644 --- a/src/pylint_odoo/checkers/__init__.py +++ b/src/pylint_odoo/checkers/__init__.py @@ -1,3 +1,4 @@ +# pylint:disable=redefined-builtin from . import modules_odoo from . import no_modules from . import format diff --git a/src/pylint_odoo/checkers/format.py b/src/pylint_odoo/checkers/format.py index 1cfa88fb..79bfd9db 100644 --- a/src/pylint_odoo/checkers/format.py +++ b/src/pylint_odoo/checkers/format.py @@ -1,19 +1,12 @@ - -import os import tokenize -from sys import platform from .. import settings from ..misc import PylintOdooTokenChecker ODOO_MSGS = { # C->convention R->refactor W->warning E->error F->fatal - - 'W%d02' % settings.BASE_FORMAT_ID: ( - 'Use of vim comment', - 'use-vim-comment', - settings.DESC_DFLT - ), + "W%d02" + % settings.BASE_FORMAT_ID: ("Use of vim comment", "use-vim-comment", settings.DESC_DFLT), } MAGIC_COMMENT_CODING = 1 @@ -29,15 +22,11 @@ class FormatChecker(PylintOdooTokenChecker): msgs = ODOO_MSGS def is_vim_comment(self, comment): - return True if comment.strip('# ').lower().startswith('vim:') \ - else False + return comment.strip("# ").lower().startswith("vim:") def process_tokens(self, tokens): - tokens_identified = {} - for idx, (tok_type, token_content, - start_line_col, end_line_col, - line_content) in enumerate(tokens): + for tok_type, token_content, start_line_col, _end_line_col, _line_content in tokens: if tokenize.COMMENT == tok_type: line_num = start_line_col[0] if self.is_vim_comment(token_content): - self.add_message('use-vim-comment', line=line_num) + self.add_message("use-vim-comment", line=line_num) diff --git a/src/pylint_odoo/checkers/modules_odoo.py b/src/pylint_odoo/checkers/modules_odoo.py index f595ac0b..dec9becd 100644 --- a/src/pylint_odoo/checkers/modules_odoo.py +++ b/src/pylint_odoo/checkers/modules_odoo.py @@ -2,99 +2,102 @@ """ import os -import re import astroid -from collections import defaultdict from pylint.checkers import utils from .. import misc, settings ODOO_MSGS = { # C->convention R->refactor W->warning E->error F->fatal - # Visit odoo module with settings.BASE_OMODULE_ID - 'E%d03' % settings.BASE_OMODULE_ID: ( - 'Test folder imported in module %s', - 'test-folder-imported', - settings.DESC_DFLT - ), - 'R%d80' % settings.BASE_OMODULE_ID: ( + "E%d03" + % settings.BASE_OMODULE_ID: ("Test folder imported in module %s", "test-folder-imported", settings.DESC_DFLT), + "R%d80" + % settings.BASE_OMODULE_ID: ( 'Consider merging classes inherited to "%s" from %s.', - 'consider-merging-classes-inherited', - settings.DESC_DFLT + "consider-merging-classes-inherited", + settings.DESC_DFLT, ), - 'W%d50' % settings.BASE_OMODULE_ID: ( - 'Same Odoo module absolute import. You should use ' + "W%d50" + % settings.BASE_OMODULE_ID: ( + "Same Odoo module absolute import. You should use " 'relative import with "." ' 'instead of "openerp.addons.%s"', - 'odoo-addons-relative-import', - settings.DESC_DFLT + "odoo-addons-relative-import", + settings.DESC_DFLT, ), - 'W%d38' % settings.BASE_OMODULE_ID: ( - 'pass into block except. ' - 'If you really need to use the pass consider logging that exception', - 'except-pass', - settings.DESC_DFLT + "W%d38" + % settings.BASE_OMODULE_ID: ( + "pass into block except. " "If you really need to use the pass consider logging that exception", + "except-pass", + settings.DESC_DFLT, ), } -DFTL_README_TMPL_URL = 'https://github.com/OCA/maintainer-tools' + \ - '/blob/master/template/module/README.rst' -DFTL_README_FILES = ['README.rst', 'README.md', 'README.txt'] +DFTL_README_TMPL_URL = "https://github.com/OCA/maintainer-tools" + "/blob/master/template/module/README.rst" +DFTL_README_FILES = ["README.rst", "README.md", "README.txt"] DFTL_MIN_PRIORITY = 99 # Files supported from manifest to convert # Extracted from openerp/tools/convert.py:def convert_file -DFLT_EXTFILES_CONVERT = ['csv', 'sql', 'xml', 'yml'] -DFTL_MANIFEST_DATA_KEYS = ['data', 'demo', 'demo_xml', 'init_xml', 'test', - 'update_xml'] +DFLT_EXTFILES_CONVERT = ["csv", "sql", "xml", "yml"] +DFTL_MANIFEST_DATA_KEYS = ["data", "demo", "demo_xml", "init_xml", "test", "update_xml"] class ModuleChecker(misc.WrapperModuleChecker): name = settings.CFG_SECTION msgs = ODOO_MSGS options = ( - ('readme_template_url', { - 'type': 'string', - 'metavar': '', - 'default': DFTL_README_TMPL_URL, - 'help': 'URL of README.rst template file', - }), - ('min-priority', { - 'type': 'int', - 'metavar': '', - 'default': DFTL_MIN_PRIORITY, - 'help': 'Minimum priority number of a view with replace of fields.' - }), - ('extfiles_convert', { - 'type': 'csv', - 'metavar': '', - 'default': DFLT_EXTFILES_CONVERT, - 'help': 'List of extension files supported to convert ' - 'from manifest separated by a comma.' - }), + ( + "readme_template_url", + { + "type": "string", + "metavar": "", + "default": DFTL_README_TMPL_URL, + "help": "URL of README.rst template file", + }, + ), + ( + "min-priority", + { + "type": "int", + "metavar": "", + "default": DFTL_MIN_PRIORITY, + "help": "Minimum priority number of a view with replace of fields.", + }, + ), + ( + "extfiles_convert", + { + "type": "csv", + "metavar": "", + "default": DFLT_EXTFILES_CONVERT, + "help": "List of extension files supported to convert " "from manifest separated by a comma.", + }, + ), ) class_inherit_names = [] - @utils.check_messages('consider-merging-classes-inherited') + @utils.check_messages("consider-merging-classes-inherited") def visit_assign(self, node): if not self.odoo_node: return - if not self.linter.is_message_enabled( - 'consider-merging-classes-inherited', node.lineno): + if not self.linter.is_message_enabled("consider-merging-classes-inherited", node.lineno): return node_left = node.targets[0] - if not isinstance(node_left, astroid.node_classes.AssignName) or \ - node_left.name not in ('_inherit', '_name') or \ - not isinstance(node.value, astroid.node_classes.Const) or \ - not isinstance(node.parent, astroid.ClassDef): + if ( + not isinstance(node_left, astroid.node_classes.AssignName) + or node_left.name not in ("_inherit", "_name") + or not isinstance(node.value, astroid.node_classes.Const) + or not isinstance(node.parent, astroid.ClassDef) + ): return - if node_left.name == '_name': + if node_left.name == "_name": node.parent.odoo_attribute_name = node.value.value return - _name = getattr(node.parent, 'odoo_attribute_name', None) + _name = getattr(node.parent, "odoo_attribute_name", None) _inherit = node.value.value if _name and _name != _inherit: # Skip _name='model.name' _inherit='other.model' because is valid @@ -106,7 +109,7 @@ def visit_assign(self, node): def open(self): """Define variables to use cache""" self.inh_dup = {} - super(ModuleChecker, self).open() + super().open() def close(self): """Final process get all cached values and add messages""" @@ -115,28 +118,26 @@ def close(self): continue path_nodes = [] for node in nodes[1:]: - relpath = os.path.relpath(node.file, - os.path.dirname(odoo_node.file)) + relpath = os.path.relpath(node.file, os.path.dirname(odoo_node.file)) path_nodes.append("%s:%d" % (relpath, node.lineno)) - self.add_message('consider-merging-classes-inherited', - node=nodes[0], - args=(class_dup_name, ', '.join(path_nodes))) + self.add_message( + "consider-merging-classes-inherited", node=nodes[0], args=(class_dup_name, ", ".join(path_nodes)) + ) def _get_odoo_module_imported(self, node): odoo_module = [] - if self.manifest_file and hasattr(node.parent, 'file'): - relpath = os.path.relpath( - node.parent.file, os.path.dirname(self.manifest_file)) - if os.path.dirname(relpath) == 'tests': + if self.manifest_file and hasattr(node.parent, "file"): + relpath = os.path.relpath(node.parent.file, os.path.dirname(self.manifest_file)) + if os.path.dirname(relpath) == "tests": # import errors rules don't apply to the test files # since these files are loaded only when running tests # and in such a case your # module and their external dependencies are installed. return odoo_module - if isinstance(node, astroid.ImportFrom) and \ - ('openerp.addons' in node.modname or - 'odoo.addons' in node.modname): - packages = node.modname.split('.') + if isinstance(node, astroid.ImportFrom) and ( + "openerp.addons" in node.modname or "odoo.addons" in node.modname + ): + packages = node.modname.split(".") if len(packages) >= 3: # from openerp.addons.odoo_module import models odoo_module.append(packages[2]) @@ -145,9 +146,9 @@ def _get_odoo_module_imported(self, node): odoo_module.append(node.names[0][0]) elif isinstance(node, astroid.Import): for name, _ in node.names: - if 'openerp.addons' not in name and 'odoo.addons' not in name: + if "openerp.addons" not in name and "odoo.addons" not in name: continue - packages = name.split('.') + packages = name.split(".") if len(packages) >= 3: # import openerp.addons.odoo_module odoo_module.append(packages[2]) @@ -155,43 +156,36 @@ def _get_odoo_module_imported(self, node): def check_odoo_relative_import(self, node): if self.odoo_module_name in self._get_odoo_module_imported(node): - self.add_message('odoo-addons-relative-import', node=node, - args=(self.odoo_module_name)) + self.add_message("odoo-addons-relative-import", node=node, args=(self.odoo_module_name)) def check_folder_test_imported(self, node): - if (hasattr(node.parent, 'file') - and os.path.basename(node.parent.file) == '__init__.py'): + if hasattr(node.parent, "file") and os.path.basename(node.parent.file) == "__init__.py": package_names = [] if isinstance(node, astroid.ImportFrom): if node.modname: # from .tests import test_file - package_names = node.modname.split('.')[:1] + package_names = node.modname.split(".")[:1] else: # from . import tests package_names = [name for name, alias in node.names] elif isinstance(node, astroid.Import): - package_names = [name[0].split('.')[0] for name in node.names] + package_names = [name[0].split(".")[0] for name in node.names] if "tests" in package_names: - self.add_message('test-folder-imported', node=node, - args=(node.parent.name,)) + self.add_message("test-folder-imported", node=node, args=(node.parent.name,)) - @utils.check_messages('odoo-addons-relative-import', - 'test-folder-imported') + @utils.check_messages("odoo-addons-relative-import", "test-folder-imported") def visit_importfrom(self, node): self.check_odoo_relative_import(node) self.check_folder_test_imported(node) - @utils.check_messages('odoo-addons-relative-import', - 'test-folder-imported') + @utils.check_messages("odoo-addons-relative-import", "test-folder-imported") def visit_import(self, node): self.check_odoo_relative_import(node) self.check_folder_test_imported(node) - @utils.check_messages('except-pass') + @utils.check_messages("except-pass") def visit_tryexcept(self, node): """Visit block try except""" for handler in node.handlers: - if (not handler.name and - len(handler.body) == 1 and - isinstance(handler.body[0], astroid.node_classes.Pass)): - self.add_message('except-pass', node=handler) + if not handler.name and len(handler.body) == 1 and isinstance(handler.body[0], astroid.node_classes.Pass): + self.add_message("except-pass", node=handler) diff --git a/src/pylint_odoo/checkers/no_modules.py b/src/pylint_odoo/checkers/no_modules.py index 3f559164..4b3bc50d 100644 --- a/src/pylint_odoo/checkers/no_modules.py +++ b/src/pylint_odoo/checkers/no_modules.py @@ -51,7 +51,6 @@ for more info visit pylint doc """ -import validators import ast import itertools import os @@ -59,233 +58,260 @@ from collections import Counter import astroid +import validators from pylint.checkers import utils from pylint.interfaces import IAstroidChecker -from .. import settings -from .. import misc +from .. import misc, settings from .modules_odoo import DFTL_MANIFEST_DATA_KEYS ODOO_MSGS = { # C->convention R->refactor W->warning E->error F->fatal - - 'R%d01' % settings.BASE_NOMODULE_ID: ( - 'Import `Warning` should be renamed as UserError ' - '`from openerp.exceptions import Warning as UserError`', - 'openerp-exception-warning', - settings.DESC_DFLT + "R%d01" + % settings.BASE_NOMODULE_ID: ( + "Import `Warning` should be renamed as UserError " "`from openerp.exceptions import Warning as UserError`", + "openerp-exception-warning", + settings.DESC_DFLT, ), - 'W%d03' % settings.BASE_NOMODULE_ID: ( + "W%d03" + % settings.BASE_NOMODULE_ID: ( 'Translation method _("string") in fields is not necessary.', - 'translation-field', - settings.DESC_DFLT - ), - 'W%d05' % settings.BASE_NOMODULE_ID: ( - 'attribute "%s" deprecated', - 'attribute-deprecated', - settings.DESC_DFLT + "translation-field", + settings.DESC_DFLT, ), - 'W%d06' % settings.BASE_NOMODULE_ID: ( - 'Missing `super` call in "%s" method.', - 'method-required-super', - settings.DESC_DFLT + "W%d05" % settings.BASE_NOMODULE_ID: ('attribute "%s" deprecated', "attribute-deprecated", settings.DESC_DFLT), + "W%d06" + % settings.BASE_NOMODULE_ID: ('Missing `super` call in "%s" method.', "method-required-super", settings.DESC_DFLT), + "W%d10" + % settings.BASE_NOMODULE_ID: ( + "Missing `return` (`super` is used) in method %s.", + "missing-return", + settings.DESC_DFLT, ), - 'W%d10' % settings.BASE_NOMODULE_ID: ( - 'Missing `return` (`super` is used) in method %s.', - 'missing-return', - settings.DESC_DFLT + "E%d01" + % settings.BASE_NOMODULE_ID: ( + "The author key in the manifest file must be a string " "(with comma separated values)", + "manifest-author-string", + settings.DESC_DFLT, ), - 'E%d01' % settings.BASE_NOMODULE_ID: ( - 'The author key in the manifest file must be a string ' - '(with comma separated values)', - 'manifest-author-string', - settings.DESC_DFLT + "E%d02" + % settings.BASE_NOMODULE_ID: ( + "Use of cr.commit() directly - More info " + "https://github.com/OCA/odoo-community.org/blob/master/website/" + "Contribution/CONTRIBUTING.rst" + "#never-commit-the-transaction", + "invalid-commit", + settings.DESC_DFLT, ), - 'E%d02' % settings.BASE_NOMODULE_ID: ( - 'Use of cr.commit() directly - More info ' - 'https://github.com/OCA/odoo-community.org/blob/master/website/' - 'Contribution/CONTRIBUTING.rst' - '#never-commit-the-transaction', - 'invalid-commit', - settings.DESC_DFLT + "E%d03" + % settings.BASE_NOMODULE_ID: ( + "SQL injection risk. " + "Use parameters if you can. - More info " + "https://github.com/OCA/odoo-community.org/blob/master/website/" + "Contribution/CONTRIBUTING.rst" + "#no-sql-injection", + "sql-injection", + settings.DESC_DFLT, ), - 'E%d03' % settings.BASE_NOMODULE_ID: ( - 'SQL injection risk. ' - 'Use parameters if you can. - More info ' - 'https://github.com/OCA/odoo-community.org/blob/master/website/' - 'Contribution/CONTRIBUTING.rst' - '#no-sql-injection', - 'sql-injection', - settings.DESC_DFLT + "E%d04" + % settings.BASE_NOMODULE_ID: ( + "The maintainers key in the manifest file must be a list of strings", + "manifest-maintainers-list", + settings.DESC_DFLT, ), - 'E%d04' % settings.BASE_NOMODULE_ID: ( - 'The maintainers key in the manifest file must be a list of strings', - 'manifest-maintainers-list', - settings.DESC_DFLT + "E%d06" + % settings.BASE_NOMODULE_ID: ( + "Use of external request method `%s` without timeout. " "It could wait for a long time", + "external-request-timeout", + settings.DESC_DFLT, ), - 'E%d06' % settings.BASE_NOMODULE_ID: ( - 'Use of external request method `%s` without timeout. ' - 'It could wait for a long time', - 'external-request-timeout', - settings.DESC_DFLT + "C%d01" + % settings.BASE_NOMODULE_ID: ( + "One of the following authors must be present in manifest: %s", + "manifest-required-author", + settings.DESC_DFLT, ), - 'C%d01' % settings.BASE_NOMODULE_ID: ( - 'One of the following authors must be present in manifest: %s', - 'manifest-required-author', - settings.DESC_DFLT - ), - 'C%d02' % settings.BASE_NOMODULE_ID: ( + "C%d02" + % settings.BASE_NOMODULE_ID: ( 'Missing required key "%s" in manifest file', - 'manifest-required-key', - settings.DESC_DFLT + "manifest-required-key", + settings.DESC_DFLT, ), - 'C%d03' % settings.BASE_NOMODULE_ID: ( + "C%d03" + % settings.BASE_NOMODULE_ID: ( 'Deprecated key "%s" in manifest file', - 'manifest-deprecated-key', - settings.DESC_DFLT + "manifest-deprecated-key", + settings.DESC_DFLT, ), - 'C%d04' % settings.BASE_NOMODULE_ID: ( + "C%d04" + % settings.BASE_NOMODULE_ID: ( 'Use `CamelCase` "%s" in class name "%s". ' - 'You can use oca-autopep8 of https://github.com/OCA/maintainer-tools' - ' to auto fix it.', - 'class-camelcase', - settings.DESC_DFLT - ), - 'C%d05' % settings.BASE_NOMODULE_ID: ( - 'License "%s" not allowed in manifest file.', - 'license-allowed', - settings.DESC_DFLT + "You can use oca-autopep8 of https://github.com/OCA/maintainer-tools" + " to auto fix it.", + "class-camelcase", + settings.DESC_DFLT, ), - 'C%d06' % settings.BASE_NOMODULE_ID: ( - 'Wrong Version Format "%s" in manifest file. ' - 'Regex to match: "%s"', - 'manifest-version-format', - settings.DESC_DFLT + "C%d05" + % settings.BASE_NOMODULE_ID: ('License "%s" not allowed in manifest file.', "license-allowed", settings.DESC_DFLT), + "C%d06" + % settings.BASE_NOMODULE_ID: ( + 'Wrong Version Format "%s" in manifest file. ' 'Regex to match: "%s"', + "manifest-version-format", + settings.DESC_DFLT, ), - 'C%d07' % settings.BASE_NOMODULE_ID: ( + "C%d07" + % settings.BASE_NOMODULE_ID: ( 'String parameter on "%s" requires translation. Use %s_(%s)', - 'translation-required', - settings.DESC_DFLT + "translation-required", + settings.DESC_DFLT, ), - 'C%d08' % settings.BASE_NOMODULE_ID: ( + "C%d08" + % settings.BASE_NOMODULE_ID: ( 'Name of compute method should start with "_compute_"', - 'method-compute', - settings.DESC_DFLT + "method-compute", + settings.DESC_DFLT, ), - 'C%d09' % settings.BASE_NOMODULE_ID: ( + "C%d09" + % settings.BASE_NOMODULE_ID: ( 'Name of search method should start with "_search_"', - 'method-search', - settings.DESC_DFLT + "method-search", + settings.DESC_DFLT, ), - 'C%d10' % settings.BASE_NOMODULE_ID: ( + "C%d10" + % settings.BASE_NOMODULE_ID: ( 'Name of inverse method should start with "_inverse_"', - 'method-inverse', - settings.DESC_DFLT + "method-inverse", + settings.DESC_DFLT, ), - 'C%d11' % settings.BASE_NOMODULE_ID: ( - 'Manifest key development_status "%s" not allowed. ' - 'Use one of: %s.', - 'development-status-allowed', - settings.DESC_DFLT + "C%d11" + % settings.BASE_NOMODULE_ID: ( + 'Manifest key development_status "%s" not allowed. ' "Use one of: %s.", + "development-status-allowed", + settings.DESC_DFLT, ), - 'W%d11' % settings.BASE_NOMODULE_ID: ( + "W%d11" + % settings.BASE_NOMODULE_ID: ( 'Field parameter "%s" is no longer supported. Use "%s" instead.', - 'renamed-field-parameter', - settings.DESC_DFLT + "renamed-field-parameter", + settings.DESC_DFLT, ), - 'W%d12' % settings.BASE_NOMODULE_ID: ( - '"eval" referenced detected.', - 'eval-referenced', - settings.DESC_DFLT + "W%d12" % settings.BASE_NOMODULE_ID: ('"eval" referenced detected.', "eval-referenced", settings.DESC_DFLT), + "W%d13" + % settings.BASE_NOMODULE_ID: ( + "The attribute string is redundant. " "String parameter equal to name of variable", + "attribute-string-redundant", + settings.DESC_DFLT, ), - 'W%d13' % settings.BASE_NOMODULE_ID: ( - 'The attribute string is redundant. ' - 'String parameter equal to name of variable', - 'attribute-string-redundant', - settings.DESC_DFLT - ), - 'W%d14' % settings.BASE_NOMODULE_ID: ( + "W%d14" + % settings.BASE_NOMODULE_ID: ( 'Website "%s" in manifest key is not a valid URI', - 'website-manifest-key-not-valid-uri', - settings.DESC_DFLT + "website-manifest-key-not-valid-uri", + settings.DESC_DFLT, ), - 'W%d15' % settings.BASE_NOMODULE_ID: ( + "W%d15" + % settings.BASE_NOMODULE_ID: ( 'Translatable term in "%s" contains variables. Use %s instead', - 'translation-contains-variable', - settings.DESC_DFLT - ), - 'W%d16' % settings.BASE_NOMODULE_ID: ( - 'Print used. Use `logger` instead.', - 'print-used', - settings.DESC_DFLT + "translation-contains-variable", + settings.DESC_DFLT, ), - 'W%d20' % settings.BASE_NOMODULE_ID: ( - 'Translation method _(%s) is using positional string printf formatting. ' + "W%d16" % settings.BASE_NOMODULE_ID: ("Print used. Use `logger` instead.", "print-used", settings.DESC_DFLT), + "W%d20" + % settings.BASE_NOMODULE_ID: ( + "Translation method _(%s) is using positional string printf formatting. " 'Use named placeholder `_("%%(placeholder)s")` instead.', - 'translation-positional-used', - settings.DESC_DFLT + "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 + "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, ), - 'W%d25' % settings.BASE_NOMODULE_ID: ( + "W%d25" + % settings.BASE_NOMODULE_ID: ( 'The file "%s" is duplicated %d times from manifest key "%s"', - 'manifest-data-duplicated', - settings.DESC_DFLT + "manifest-data-duplicated", + settings.DESC_DFLT, ), - 'F%d01' % settings.BASE_NOMODULE_ID: ( - 'File "%s": "%s" not found.', - 'resource-not-exist', - settings.DESC_DFLT - ) + "F%d01" % settings.BASE_NOMODULE_ID: ('File "%s": "%s" not found.', "resource-not-exist", settings.DESC_DFLT), } -DFTL_MANIFEST_REQUIRED_KEYS = ['license'] -DFTL_MANIFEST_REQUIRED_AUTHORS = ['Odoo Community Association (OCA)'] -DFTL_MANIFEST_DEPRECATED_KEYS = ['description'] +DFTL_MANIFEST_REQUIRED_KEYS = ["license"] +DFTL_MANIFEST_REQUIRED_AUTHORS = ["Odoo Community Association (OCA)"] +DFTL_MANIFEST_DEPRECATED_KEYS = ["description"] DFTL_LICENSE_ALLOWED = [ - 'AGPL-3', 'GPL-2', 'GPL-2 or any later version', - 'GPL-3', 'GPL-3 or any later version', 'LGPL-3', - 'Other OSI approved licence', 'Other proprietary', - 'OEEL-1', + "AGPL-3", + "GPL-2", + "GPL-2 or any later version", + "GPL-3", + "GPL-3 or any later version", + "LGPL-3", + "Other OSI approved licence", + "Other proprietary", + "OEEL-1", ] DFTL_DEVELOPMENT_STATUS_ALLOWED = [ - 'Alpha', 'Beta', 'Production/Stable', 'Mature', + "Alpha", + "Beta", + "Production/Stable", + "Mature", ] DFTL_ATTRIBUTE_DEPRECATED = [ - '_columns', '_defaults', 'length', + "_columns", + "_defaults", + "length", ] DFTL_METHOD_REQUIRED_SUPER = [ - 'create', 'write', 'read', 'unlink', 'copy', - 'setUp', 'setUpClass', 'tearDown', 'tearDownClass', 'default_get', + "create", + "write", + "read", + "unlink", + "copy", + "setUp", + "setUpClass", + "tearDown", + "tearDownClass", + "default_get", ] DFTL_CURSOR_EXPR = [ - 'self.env.cr', 'self._cr', # new api - 'self.cr', # controllers and test - 'cr', # old api + "self.env.cr", + "self._cr", # new api + "self.cr", # controllers and test + "cr", # old api ] DFTL_ODOO_EXCEPTIONS = [ # Extracted from openerp/exceptions.py of 8.0 and master - 'AccessDenied', 'AccessError', 'DeferredException', 'except_orm', - 'MissingError', 'QWebException', 'RedirectWarning', 'UserError', - 'ValidationError', 'Warning', + "AccessDenied", + "AccessError", + "DeferredException", + "except_orm", + "MissingError", + "QWebException", + "RedirectWarning", + "UserError", + "ValidationError", + "Warning", ] DFTL_NO_MISSING_RETURN = [ - '__init__', 'setUp', 'setUpClass', 'tearDown', 'tearDownClass', '_register_hook', + "__init__", + "setUp", + "setUpClass", + "tearDown", + "tearDownClass", + "_register_hook", ] FIELDS_METHOD = { - 'Many2many': 4, - 'One2many': 2, - 'Many2one': 1, - 'Reference': 1, - 'Selection': 1, + "Many2many": 4, + "One2many": 2, + "Many2one": 1, + "Reference": 1, + "Selection": 1, } DFTL_DEPRECATED_FIELD_PARAMETERS = [ # From odoo/odoo 10.0: odoo/odoo/fields.py:29 - 'digits_compute:digits', 'select:index' + "digits_compute:digits", + "select:index", ] DFTL_EXTERNAL_REQUEST_TIMEOUT_METHODS = [ "ftplib.FTP", @@ -311,119 +337,160 @@ class NoModuleChecker(misc.PylintOdooChecker): __implements__ = IAstroidChecker + _from_imports = None name = settings.CFG_SECTION msgs = ODOO_MSGS options = ( - ('manifest_required_authors', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_MANIFEST_REQUIRED_AUTHORS, - 'help': 'Author names, at least one is required in manifest file.' - }), - ('manifest_required_author', { - 'type': 'string', - 'metavar': '', - 'default': '', - 'help': ('Name of author required in manifest file. ' - 'This parameter is deprecated use ' - '"manifest_required_authors" instead.') - }), - ('manifest_required_keys', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_MANIFEST_REQUIRED_KEYS, - 'help': 'List of keys required in manifest file, ' + - 'separated by a comma.' - }), - ('manifest_deprecated_keys', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_MANIFEST_DEPRECATED_KEYS, - 'help': 'List of keys deprecated in manifest file, ' + - 'separated by a comma.' - }), - ('license_allowed', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_LICENSE_ALLOWED, - 'help': 'List of license allowed in manifest file, ' + - 'separated by a comma.' - }), - ('development_status_allowed', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_DEVELOPMENT_STATUS_ALLOWED, - 'help': 'List of development status allowed in manifest file, ' + - 'separated by a comma.' - }), - ('attribute_deprecated', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_ATTRIBUTE_DEPRECATED, - 'help': 'List of attributes deprecated, ' + - 'separated by a comma.' - }), - ('deprecated_field_parameters', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_DEPRECATED_FIELD_PARAMETERS, - 'help': 'List of deprecated field parameters, separated by a ' - 'comma. If the param was renamed, separate the old and ' - 'new name with a colon. If the param was removed, keep ' - 'the right side of the colon empty. ' - '"deprecated_param:" means that "deprecated_param" was ' - 'deprecated and it doesn\'t have a new alternative. ' - '"deprecated_param:new_param" means that it was ' - 'deprecated and renamed as "new_param". ' - }), - ('method_required_super', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_METHOD_REQUIRED_SUPER, - 'help': 'List of methods where call to `super` is required.' + - 'separated by a comma.' - }), - ('manifest_version_format', { - 'type': 'string', - 'metavar': '', - 'default': misc.DFTL_MANIFEST_VERSION_FORMAT, - 'help': 'Regex to check version format in manifest file. ' - 'Use "{valid_odoo_versions}" to check the parameter of ' - '"valid_odoo_versions"' - }), - ('cursor_expr', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_CURSOR_EXPR, - 'help': 'List of cursor expr separated by a comma.' - }), - ('odoo_exceptions', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_ODOO_EXCEPTIONS, - 'help': 'List of odoo exceptions separated by a comma.' - }), - ('valid_odoo_versions', { - 'type': 'csv', - 'metavar': '', - 'default': misc.DFTL_VALID_ODOO_VERSIONS, - 'help': 'List of valid odoo versions separated by a comma.' - }), - ('no_missing_return', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_NO_MISSING_RETURN, - 'help': 'List of valid missing return method names, ' - 'separated by a comma.' - }), - ('external_request_timeout_methods', { - 'type': 'csv', - 'metavar': '', - 'default': DFTL_EXTERNAL_REQUEST_TIMEOUT_METHODS, - 'help': 'List of library.method that must have a timeout ' - 'parameter defined, separated by a comma. ' - 'e.g. "requests.get,requests.post"' - }), + ( + "manifest_required_authors", + { + "type": "csv", + "metavar": "", + "default": DFTL_MANIFEST_REQUIRED_AUTHORS, + "help": "Author names, at least one is required in manifest file.", + }, + ), + ( + "manifest_required_author", + { + "type": "string", + "metavar": "", + "default": "", + "help": ( + "Name of author required in manifest file. " + "This parameter is deprecated use " + '"manifest_required_authors" instead.' + ), + }, + ), + ( + "manifest_required_keys", + { + "type": "csv", + "metavar": "", + "default": DFTL_MANIFEST_REQUIRED_KEYS, + "help": "List of keys required in manifest file, " + "separated by a comma.", + }, + ), + ( + "manifest_deprecated_keys", + { + "type": "csv", + "metavar": "", + "default": DFTL_MANIFEST_DEPRECATED_KEYS, + "help": "List of keys deprecated in manifest file, " + "separated by a comma.", + }, + ), + ( + "license_allowed", + { + "type": "csv", + "metavar": "", + "default": DFTL_LICENSE_ALLOWED, + "help": "List of license allowed in manifest file, " + "separated by a comma.", + }, + ), + ( + "development_status_allowed", + { + "type": "csv", + "metavar": "", + "default": DFTL_DEVELOPMENT_STATUS_ALLOWED, + "help": "List of development status allowed in manifest file, " + "separated by a comma.", + }, + ), + ( + "attribute_deprecated", + { + "type": "csv", + "metavar": "", + "default": DFTL_ATTRIBUTE_DEPRECATED, + "help": "List of attributes deprecated, " + "separated by a comma.", + }, + ), + ( + "deprecated_field_parameters", + { + "type": "csv", + "metavar": "", + "default": DFTL_DEPRECATED_FIELD_PARAMETERS, + "help": "List of deprecated field parameters, separated by a " + "comma. If the param was renamed, separate the old and " + "new name with a colon. If the param was removed, keep " + "the right side of the colon empty. " + '"deprecated_param:" means that "deprecated_param" was ' + "deprecated and it doesn't have a new alternative. " + '"deprecated_param:new_param" means that it was ' + 'deprecated and renamed as "new_param". ', + }, + ), + ( + "method_required_super", + { + "type": "csv", + "metavar": "", + "default": DFTL_METHOD_REQUIRED_SUPER, + "help": "List of methods where call to `super` is required." + "separated by a comma.", + }, + ), + ( + "manifest_version_format", + { + "type": "string", + "metavar": "", + "default": misc.DFTL_MANIFEST_VERSION_FORMAT, + "help": "Regex to check version format in manifest file. " + 'Use "{valid_odoo_versions}" to check the parameter of ' + '"valid_odoo_versions"', + }, + ), + ( + "cursor_expr", + { + "type": "csv", + "metavar": "", + "default": DFTL_CURSOR_EXPR, + "help": "List of cursor expr separated by a comma.", + }, + ), + ( + "odoo_exceptions", + { + "type": "csv", + "metavar": "", + "default": DFTL_ODOO_EXCEPTIONS, + "help": "List of odoo exceptions separated by a comma.", + }, + ), + ( + "valid_odoo_versions", + { + "type": "csv", + "metavar": "", + "default": misc.DFTL_VALID_ODOO_VERSIONS, + "help": "List of valid odoo versions separated by a comma.", + }, + ), + ( + "no_missing_return", + { + "type": "csv", + "metavar": "", + "default": DFTL_NO_MISSING_RETURN, + "help": "List of valid missing return method names, " "separated by a comma.", + }, + ), + ( + "external_request_timeout_methods", + { + "type": "csv", + "metavar": "", + "default": DFTL_EXTERNAL_REQUEST_TIMEOUT_METHODS, + "help": "List of library.method that must have a timeout " + "parameter defined, separated by a comma. " + 'e.g. "requests.get,requests.post"', + }, + ), ) def visit_module(self, node): @@ -440,9 +507,8 @@ def leave_module(self, node): self._from_imports = {} def open(self): - super(NoModuleChecker, self).open() - self.config.deprecated_field_parameters = \ - self.colon_list_to_dict(self.config.deprecated_field_parameters) + super().open() + self.config.deprecated_field_parameters = self.colon_list_to_dict(self.config.deprecated_field_parameters) def colon_list_to_dict(self, colon_list): """Converts a colon list to a dictionary. @@ -466,30 +532,31 @@ def _sqli_allowable(self, node): if self._is_psycopg2_sql(node): return True if isinstance(node, astroid.FormattedValue): - if hasattr(node, 'value'): + if hasattr(node, "value"): return self._sqli_allowable(node.value) - if hasattr(node, 'values'): + if hasattr(node, "values"): return all(self._sqli_allowable(v) for v in node.values) if isinstance(node, astroid.Call): node = node.func # 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) - 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)) + return ( + isinstance(node, astroid.Attribute) + and isinstance(node.expr, astroid.Name) + 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): for assignation_node in self._get_assignation_nodes(node): if self._is_psycopg2_sql(assignation_node): return True - if (not isinstance(node, astroid.Call) or - not isinstance(node.func, (astroid.Attribute, astroid.Name))): + if not isinstance(node, astroid.Call) or not isinstance(node.func, (astroid.Attribute, astroid.Name)): return False - imported_name = node.func.as_string().split('.')[0] + imported_name = node.func.as_string().split(".")[0] imported_node = node.root().locals.get(imported_name) # "from psycopg2 import *" not considered since that it is hard # and there is another check detecting these kind of imports @@ -497,16 +564,16 @@ def _is_psycopg2_sql(self, node): return None imported_node = imported_node[0] if isinstance(imported_node, astroid.ImportFrom): - package_names = imported_node.modname.split('.')[:1] + package_names = imported_node.modname.split(".")[:1] elif isinstance(imported_node, astroid.Import): - package_names = [name[0].split('.')[0] for name in imported_node.names] + package_names = [name[0].split(".")[0] for name in imported_node.names] else: return False - if 'psycopg2' in package_names: + if "psycopg2" in package_names: return True def _check_node_for_sqli_risk(self, node): - if isinstance(node, astroid.BinOp) and node.op in ('%', '+'): + if isinstance(node, astroid.BinOp) and node.op in ("%", "+"): if isinstance(node.right, astroid.Tuple): # execute("..." % (self._table, thing)) if not all(map(self._sqli_allowable, node.right.elts)): @@ -531,30 +598,28 @@ def _check_node_for_sqli_risk(self, node): # 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)): + 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) \ - and isinstance(node.func, astroid.Attribute) \ - and node.func.attrname == 'format': + if ( + isinstance(node, astroid.Call) + and isinstance(node.func, astroid.Attribute) + and node.func.attrname == "format" + ): if not all(map(self._sqli_allowable, node.args or [])): return True - if not all( - self._sqli_allowable(keyword.value) - for keyword in (node.keywords or []) - ): + if not all(self._sqli_allowable(keyword.value) for keyword in (node.keywords or [])): return True # Check fstrings (PEP 498). Only Python >= 3.6 if isinstance(node, astroid.JoinedStr): - if hasattr(node, 'value'): + if hasattr(node, "value"): return self._sqli_allowable(node.value) - elif hasattr(node, 'values'): + if hasattr(node, "values"): return not all(self._sqli_allowable(v) for v in node.values) return False @@ -565,16 +630,20 @@ 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') and + isinstance(node, astroid.Call) + and node.args + and isinstance(node.func, astroid.Attribute) + and node.func.attrname in ("execute", "executemany") + and # cursor expr (see above) - self.get_cursor_name(node.func) in DFTL_CURSOR_EXPR and + self.get_cursor_name(node.func) in DFTL_CURSOR_EXPR + and # cr.execute("select * from %s" % foo, [bar]) -> probably a good reason # for string formatting - len(node.args) <= 1 and + len(node.args) <= 1 + and # ignore in test files, probably not accessible - not current_file_bname.startswith('test_') + not current_file_bname.startswith("test_") ): return False first_arg = node.args[0] @@ -591,7 +660,7 @@ def _get_assignation_nodes(self, node): if isinstance(node, (astroid.Name, astroid.Subscript)): # 1) look for parent method / controller current = node - while (current and not isinstance(current.parent, astroid.FunctionDef)): + while current and not isinstance(current.parent, astroid.FunctionDef): current = current.parent if current: parent = current.parent @@ -604,178 +673,181 @@ def _get_assignation_nodes(self, node): def visit_print(self, node): self.add_message("print-used", node=node) - @utils.check_messages('translation-field', 'invalid-commit', - 'method-compute', 'method-search', 'method-inverse', - 'sql-injection', - 'attribute-string-redundant', - 'renamed-field-parameter', - 'translation-required', - 'translation-contains-variable', - 'print-used', 'translation-positional-used', - 'context-overridden', - 'external-request-timeout', - ) + @utils.check_messages( + "translation-field", + "invalid-commit", + "method-compute", + "method-search", + "method-inverse", + "sql-injection", + "attribute-string-redundant", + "renamed-field-parameter", + "translation-required", + "translation-contains-variable", + "print-used", + "translation-positional-used", + "context-overridden", + "external-request-timeout", + ) def visit_call(self, node): infer_node = utils.safe_infer(node.func) - if utils.is_builtin_object(infer_node) and infer_node.name == 'print': - self.add_message('print-used', node=node) - if ('fields' == self.get_func_lib(node.func) and - isinstance(node.parent, astroid.Assign) and - isinstance(node.parent.parent, astroid.ClassDef)): + if utils.is_builtin_object(infer_node) and infer_node.name == "print": + self.add_message("print-used", node=node) + if ( + "fields" == self.get_func_lib(node.func) + and isinstance(node.parent, astroid.Assign) + and isinstance(node.parent.parent, astroid.ClassDef) + ): args = misc.join_node_args_kwargs(node) index = 0 - field_name = '' - if (isinstance(node.parent, astroid.Assign) and - node.parent.targets and - isinstance(node.parent.targets[0], astroid.AssignName)): - field_name = (node.parent.targets[0].name - .replace('_', ' ')) - is_related = bool([1 for kw in node.keywords or [] if kw.arg == 'related']) + field_name = "" + if ( + isinstance(node.parent, astroid.Assign) + and node.parent.targets + and isinstance(node.parent.targets[0], astroid.AssignName) + ): + field_name = node.parent.targets[0].name.replace("_", " ") + is_related = bool([1 for kw in node.keywords or [] if kw.arg == "related"]) for argument in args: argument_aux = argument # Check this 'name = fields.Char("name")' - if (not is_related and isinstance(argument, astroid.Const) and - (index == - FIELDS_METHOD.get(argument.parent.func.attrname, 0)) and - (argument.value == - field_name.title())): - self.add_message('attribute-string-redundant', node=node) + if ( + not is_related + and isinstance(argument, astroid.Const) + and (index == FIELDS_METHOD.get(argument.parent.func.attrname, 0)) + and (argument.value == field_name.title()) + ): + self.add_message("attribute-string-redundant", node=node) if isinstance(argument, astroid.Keyword): argument_aux = argument.value deprecated = self.config.deprecated_field_parameters - if argument.arg in ['compute', 'search', 'inverse'] and \ - isinstance(argument_aux, astroid.Const) and \ - isinstance(argument_aux.value, str) and \ - not argument_aux.value.startswith( - '_' + argument.arg + '_'): - self.add_message('method-' + argument.arg, - node=argument_aux) + if ( + argument.arg in ["compute", "search", "inverse"] + and isinstance(argument_aux, astroid.Const) + and isinstance(argument_aux.value, str) + and not argument_aux.value.startswith("_" + argument.arg + "_") + ): + self.add_message("method-" + argument.arg, node=argument_aux) # Check if the param string is equal to the name # of variable - elif not is_related and argument.arg == 'string' and \ - (isinstance(argument_aux, astroid.Const) and - argument_aux.value == field_name.title()): - self.add_message( - 'attribute-string-redundant', node=node) - elif (argument.arg in deprecated): + elif ( + not is_related + and argument.arg == "string" + and (isinstance(argument_aux, astroid.Const) and argument_aux.value == field_name.title()) + ): + self.add_message("attribute-string-redundant", node=node) + elif argument.arg in deprecated: self.add_message( - 'renamed-field-parameter', node=node, - args=(argument.arg, deprecated[argument.arg]) + "renamed-field-parameter", node=node, args=(argument.arg, deprecated[argument.arg]) ) - if isinstance(argument_aux, astroid.Call) and \ - isinstance(argument_aux.func, astroid.Name) and \ - argument_aux.func.name == '_': - self.add_message('translation-field', node=argument_aux) + if ( + isinstance(argument_aux, astroid.Call) + and isinstance(argument_aux.func, astroid.Name) + and argument_aux.func.name == "_" + ): + self.add_message("translation-field", node=argument_aux) index += 1 # Check cr.commit() - if isinstance(node, astroid.Call) and \ - isinstance(node.func, astroid.Attribute) and \ - node.func.attrname == 'commit' and \ - 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): + if ( + isinstance(node, astroid.Call) + and isinstance(node.func, astroid.Attribute) + and node.func.attrname == "commit" + and 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(),)) + 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))) - if (base_dirname != 'tests' and isinstance(node, astroid.Call) and - isinstance(node.func, astroid.Attribute) and - node.func.attrname == 'message_post'): + base_dirname = os.path.basename(os.path.normpath(os.path.dirname(self.linter.current_file))) + if ( + base_dirname != "tests" + and isinstance(node, astroid.Call) + and isinstance(node.func, astroid.Attribute) + and node.func.attrname == "message_post" + ): for arg in itertools.chain(node.args, node.keywords or []): if isinstance(arg, astroid.Keyword): keyword = arg.arg value = arg.value else: - keyword = '' + keyword = "" value = arg - if keyword and keyword not in ('subject', 'body'): + if keyword and keyword not in ("subject", "body"): continue - as_string = '' + as_string = "" # case: message_post(body='String') if isinstance(value, astroid.Const): as_string = value.as_string() # case: message_post(body='String %s' % (...)) - elif (isinstance(value, astroid.BinOp) - and value.op == '%' - and isinstance(value.left, astroid.Const) - # The right part is translatable only if it's a - # function or a list of functions - and not ( - isinstance(value.right, ( - astroid.Call, astroid.Tuple, astroid.List)) - and all([ - isinstance(child, astroid.Call) - for child in getattr(value.right, 'elts', []) - ]))): + elif ( + isinstance(value, astroid.BinOp) + and value.op == "%" + and isinstance(value.left, astroid.Const) + # The right part is translatable only if it's a + # function or a list of functions + and not ( + isinstance(value.right, (astroid.Call, astroid.Tuple, astroid.List)) + and all(isinstance(child, astroid.Call) for child in getattr(value.right, "elts", [])) + ) + ): as_string = value.left.as_string() # case: message_post(body='String {...}'.format(...)) - elif (isinstance(value, astroid.Call) - and isinstance(value.func, astroid.Attribute) - and isinstance(value.func.expr, astroid.Const) - and value.func.attrname == 'format'): + elif ( + isinstance(value, astroid.Call) + and isinstance(value.func, astroid.Attribute) + and isinstance(value.func.expr, astroid.Const) + and value.func.attrname == "format" + ): as_string = value.func.expr.as_string() if as_string: - keyword = keyword and '%s=' % keyword - self.add_message( - 'translation-required', node=node, - args=('message_post', keyword, as_string)) + keyword = keyword and "%s=" % keyword + self.add_message("translation-required", node=node, args=("message_post", keyword, as_string)) # Call _(...) with variables into the term to be translated - if (isinstance(node.func, astroid.Name) - and node.func.name == '_' - and node.args): - wrong = '' - right = '' + if isinstance(node.func, astroid.Name) and node.func.name == "_" and node.args: + wrong = "" + right = "" arg = node.args[0] # case: _('...' % (variables)) - if isinstance(arg, astroid.BinOp) and arg.op == '%': - wrong = '%s %% %s' % ( - arg.left.as_string(), arg.right.as_string()) - right = '_(%s) %% %s' % ( - arg.left.as_string(), arg.right.as_string()) + if isinstance(arg, astroid.BinOp) and arg.op == "%": + wrong = "%s %% %s" % (arg.left.as_string(), arg.right.as_string()) + right = "_(%s) %% %s" % (arg.left.as_string(), arg.right.as_string()) # Case: _('...'.format(variables)) - elif (isinstance(arg, astroid.Call) - and isinstance(arg.func, astroid.Attribute) - and isinstance(arg.func.expr, astroid.Const) - and arg.func.attrname == 'format'): + elif ( + isinstance(arg, astroid.Call) + and isinstance(arg.func, astroid.Attribute) + and isinstance(arg.func.expr, astroid.Const) + and arg.func.attrname == "format" + ): wrong = arg.as_string() - params_as_string = ', '.join([ - x.as_string() - for x in itertools.chain(arg.args, arg.keywords or [])]) - right = '_(%s).format(%s)' % ( - arg.func.expr.as_string(), params_as_string) + params_as_string = ", ".join([x.as_string() for x in itertools.chain(arg.args, arg.keywords or [])]) + right = "_(%s).format(%s)" % (arg.func.expr.as_string(), params_as_string) if wrong and right: - self.add_message( - 'translation-contains-variable', node=node, - args=(wrong, right)) + self.add_message("translation-contains-variable", node=node, args=(wrong, right)) # translation-positional-used: Check "string to translate" # to check "%s %s..." used where the position can't be changed str2translate = arg.as_string() - printf_args = ( - misc.WrapperModuleChecker. - _get_printf_str_args_kwargs(str2translate)) - format_args = ( - misc.WrapperModuleChecker. - _get_format_str_args_kwargs(str2translate)[0]) - if (isinstance(printf_args, tuple) and len(printf_args) >= 2 or - len(format_args) >= 2): + printf_args = misc.WrapperModuleChecker._get_printf_str_args_kwargs(str2translate) + format_args = misc.WrapperModuleChecker._get_format_str_args_kwargs(str2translate)[0] + if isinstance(printf_args, tuple) and len(printf_args) >= 2 or len(format_args) >= 2: # Return tuple for %s and dict for %(varname)s # Check just the following cases "%s %s..." - self.add_message('translation-positional-used', - node=node, args=(str2translate,)) + self.add_message("translation-positional-used", node=node, args=(str2translate,)) # SQL Injection if self._check_sql_injection_risky(node): - self.add_message('sql-injection', node=node) + self.add_message("sql-injection", node=node) # external-request-timeout lib_alias = self.get_func_lib(node.func) @@ -784,82 +856,83 @@ def visit_call(self, node): func_name = self.get_func_name(node.func) lib_original_func_name = ( # If it using "requests.request()" - "%s.%s" % (lib_original, func_name) if lib_original + "%s.%s" % (lib_original, func_name) + if lib_original # If it using "from requests import request;request()" - else self._from_imports.get(func_name)) + else self._from_imports.get(func_name) + ) if lib_original_func_name in self.config.external_request_timeout_methods: for argument in misc.join_node_args_kwargs(node): if not isinstance(argument, astroid.Keyword): continue - if argument.arg == 'timeout': + if argument.arg == "timeout": break else: - self.add_message( - 'external-request-timeout', node=node, - args=(lib_original_func_name,)) + self.add_message("external-request-timeout", node=node, args=(lib_original_func_name,)) @utils.check_messages( - 'license-allowed', 'manifest-author-string', 'manifest-deprecated-key', - 'manifest-required-author', 'manifest-required-key', - 'manifest-version-format', 'resource-not-exist', - 'website-manifest-key-not-valid-uri', 'development-status-allowed', - 'manifest-maintainers-list', 'manifest-data-duplicated') + "license-allowed", + "manifest-author-string", + "manifest-deprecated-key", + "manifest-required-author", + "manifest-required-key", + "manifest-version-format", + "resource-not-exist", + "website-manifest-key-not-valid-uri", + "development-status-allowed", + "manifest-maintainers-list", + "manifest-data-duplicated", + ) def visit_dict(self, node): - if not os.path.basename(self.linter.current_file) in \ - settings.MANIFEST_FILES \ - or not isinstance(node.parent, astroid.Expr): + if not os.path.basename(self.linter.current_file) in settings.MANIFEST_FILES or not isinstance( + node.parent, astroid.Expr + ): return manifest_dict = ast.literal_eval(node.as_string()) # Check author is a string - author = manifest_dict.get('author', '') + author = manifest_dict.get("author", "") if not isinstance(author, str): - self.add_message('manifest-author-string', node=node) + self.add_message("manifest-author-string", node=node) else: # Check author required - authors = set([auth.strip() for auth in author.split(',')]) + authors = {auth.strip() for auth in author.split(",")} if self.config.manifest_required_author: # Support compatibility for deprecated attribute - required_authors = set((self.config.manifest_required_author,)) + required_authors = {self.config.manifest_required_author} else: required_authors = set(self.config.manifest_required_authors) - if not (authors & required_authors): + if not authors & required_authors: # None of the required authors is present in the manifest # Authors will be printed as 'author1', 'author2', ... - authors_str = ", ".join([ - "'%s'" % auth for auth in required_authors - ]) - self.add_message('manifest-required-author', node=node, - args=(authors_str,)) + authors_str = ", ".join(["'%s'" % auth for auth in required_authors]) + self.add_message("manifest-required-author", node=node, args=(authors_str,)) # Check keys required required_keys = self.config.manifest_required_keys for required_key in required_keys: if required_key not in manifest_dict: - self.add_message('manifest-required-key', node=node, - args=(required_key,)) + self.add_message("manifest-required-key", node=node, args=(required_key,)) # Check keys deprecated deprecated_keys = self.config.manifest_deprecated_keys for deprecated_key in deprecated_keys: if deprecated_key in manifest_dict: - self.add_message('manifest-deprecated-key', node=node, - args=(deprecated_key,)) + self.add_message("manifest-deprecated-key", node=node, args=(deprecated_key,)) # Check license allowed - license = manifest_dict.get('license', None) - if license and license not in self.config.license_allowed: - self.add_message('license-allowed', - node=node, args=(license,)) + license_str = manifest_dict.get("license", None) + if license_str and license_str not in self.config.license_allowed: + self.add_message("license-allowed", node=node, args=(license_str,)) # Check version format - version_format = manifest_dict.get('version', '') + version_format = manifest_dict.get("version", "") formatrgx = self.formatversion(version_format) if version_format and not formatrgx: - self.add_message('manifest-version-format', node=node, - args=(version_format, - self.config.manifest_version_format_parsed)) + self.add_message( + "manifest-version-format", node=node, args=(version_format, self.config.manifest_version_format_parsed) + ) # Check if resource exist # Check manifest-data-duplicated @@ -867,38 +940,34 @@ def visit_dict(self, node): for key in DFTL_MANIFEST_DATA_KEYS: for resource, coincidences in Counter(manifest_dict.get(key) or []).items(): if coincidences >= 2: - self.add_message('manifest-data-duplicated', node=node, - args=(resource, coincidences, key)) + self.add_message("manifest-data-duplicated", node=node, args=(resource, coincidences, key)) if os.path.isfile(os.path.join(dirname, resource)): continue - self.add_message('resource-not-exist', node=node, - args=(key, resource)) + self.add_message("resource-not-exist", node=node, args=(key, resource)) # Check if the website is valid URI - website = manifest_dict.get('website', '') + website = manifest_dict.get("website", "") url_is_valid = bool(validators.url(website, public=True)) - if website and ',' not in website and not url_is_valid: - self.add_message('website-manifest-key-not-valid-uri', - node=node, args=(website)) + if website and "," not in website and not url_is_valid: + self.add_message("website-manifest-key-not-valid-uri", node=node, args=(website)) # Check valid development_status values - dev_status = manifest_dict.get('development_status') - if (dev_status and - dev_status not in self.config.development_status_allowed): + dev_status = manifest_dict.get("development_status") + if dev_status and dev_status not in self.config.development_status_allowed: valid_status = ", ".join(self.config.development_status_allowed) - self.add_message('development-status-allowed', - node=node, args=(dev_status, valid_status)) + self.add_message("development-status-allowed", node=node, args=(dev_status, valid_status)) # Check maintainers key is a list of strings - maintainers = manifest_dict.get('maintainers') - if(maintainers and (not isinstance(maintainers, list) - or any(not isinstance(item, str) for item in maintainers))): - self.add_message('manifest-maintainers-list', - node=node) + maintainers = manifest_dict.get("maintainers") + if maintainers and ( + not isinstance(maintainers, list) or any(not isinstance(item, str) for item in maintainers) + ): + self.add_message("manifest-maintainers-list", node=node) - @utils.check_messages('method-required-super', - 'missing-return', - ) + @utils.check_messages( + "method-required-super", + "missing-return", + ) def visit_functiondef(self, node): """Check that `api.one` and `api.multi` decorators not exists together Check that method `copy` exists `api.one` decorator @@ -907,80 +976,69 @@ def visit_functiondef(self, node): if not node.is_method(): return - decor_names = self.get_decorators_names(node.decorators) - decor_lastnames = [ - decor.split('.')[-1] - for decor in decor_names] - if node.name in self.config.method_required_super: calls = [ call_func.func.name for call_func in node.nodes_of_class((astroid.Call,)) - if isinstance(call_func.func, astroid.Name)] - if 'super' not in calls: - self.add_message('method-required-super', node=node, - args=(node.name)) + if isinstance(call_func.func, astroid.Name) + ] + if "super" not in calls: + self.add_message("method-required-super", node=node, args=(node.name)) there_is_super = False for stmt in node.nodes_of_class(astroid.Call): func = stmt.func - if isinstance(func, astroid.Name) and func.name == 'super': + if isinstance(func, astroid.Name) and func.name == "super": there_is_super = True break - there_is_return = False - for stmt in node.nodes_of_class(astroid.Return, - skip_klass=(astroid.FunctionDef, - astroid.ClassDef)): - there_is_return = True - break - if there_is_super and not there_is_return and \ - not node.is_generator() and \ - node.name not in self.config.no_missing_return: - self.add_message('missing-return', node=node, args=(node.name)) - @utils.check_messages('external-request-timeout') + there_is_return = any(node.nodes_of_class(astroid.Return, skip_klass=(astroid.FunctionDef, astroid.ClassDef))) + if ( + there_is_super + and not there_is_return + and not node.is_generator() + and node.name not in self.config.no_missing_return + ): + self.add_message("missing-return", node=node, args=(node.name)) + + @utils.check_messages("external-request-timeout") def visit_import(self, node): - self._from_imports.update({ - alias or name: "%s" % name - for name, alias in node.names - }) + self._from_imports.update({alias or name: "%s" % name for name, alias in node.names}) - @utils.check_messages('openerp-exception-warning', 'external-request-timeout') + @utils.check_messages("openerp-exception-warning", "external-request-timeout") def visit_importfrom(self, node): - if node.modname == 'openerp.exceptions': + if node.modname == "openerp.exceptions": for (import_name, import_as_name) in node.names: - if import_name == 'Warning' and import_as_name != 'UserError': - self.add_message('openerp-exception-warning', node=node) - self._from_imports.update({ - alias or name: "%s.%s" % (node.modname, name) - for name, alias in node.names}) + if import_name == "Warning" and import_as_name != "UserError": + self.add_message("openerp-exception-warning", node=node) + self._from_imports.update({alias or name: "%s.%s" % (node.modname, name) for name, alias in node.names}) - @utils.check_messages('class-camelcase') + @utils.check_messages("class-camelcase") def visit_classdef(self, node): camelized = self.camelize(node.name) if camelized != node.name: - self.add_message('class-camelcase', node=node, - args=(camelized, node.name)) + self.add_message("class-camelcase", node=node, args=(camelized, node.name)) - @utils.check_messages('attribute-deprecated') + @utils.check_messages("attribute-deprecated") def visit_assign(self, node): node_left = node.targets[0] - if (isinstance(node.parent, astroid.ClassDef) and - isinstance(node_left, astroid.AssignName) and - [1 for m in node.parent.basenames if 'Model' in m]): + if ( + isinstance(node.parent, astroid.ClassDef) + and isinstance(node_left, astroid.AssignName) + and [1 for m in node.parent.basenames if "Model" in m] + ): if node_left.name in self.config.attribute_deprecated: - self.add_message('attribute-deprecated', - node=node_left, args=(node_left.name,)) + self.add_message("attribute-deprecated", node=node_left, args=(node_left.name,)) - @utils.check_messages('eval-referenced') + @utils.check_messages("eval-referenced") def visit_name(self, node): """Detect when a "bad" built-in is referenced.""" node_infer = utils.safe_infer(node) if not utils.is_builtin_object(node_infer): # Skip not builtin objects return - if node_infer.name == 'eval': - self.add_message('eval-referenced', node=node) + if node_infer.name == "eval": + self.add_message("eval-referenced", node=node) def camelize(self, string): return re.sub(r"(?:^|_)(.)", lambda m: m.group(1).upper(), string) @@ -989,21 +1047,20 @@ def get_decorators_names(self, decorators): nodes = [] if decorators: nodes = decorators.nodes - return [getattr(decorator, 'attrname', '') - for decorator in nodes if decorator is not None] + return [getattr(decorator, "attrname", "") for decorator in nodes if decorator is not None] def get_func_name(self, node): - func_name = isinstance(node, astroid.Name) and node.name or \ - isinstance(node, astroid.Attribute) and node.attrname or '' + func_name = ( + isinstance(node, astroid.Name) and node.name or isinstance(node, astroid.Attribute) and node.attrname or "" + ) return func_name def get_func_lib(self, node): - if isinstance(node, astroid.Attribute) and \ - isinstance(node.expr, astroid.Name): + if isinstance(node, astroid.Attribute) and isinstance(node.expr, astroid.Name): return node.expr.name return "" - @utils.check_messages('translation-required') + @utils.check_messages("translation-required") def visit_raise(self, node): """Visit raise and search methods with a string parameter without a method. @@ -1025,18 +1082,13 @@ def visit_raise(self, node): func_name = self.get_func_name(expr.func) argument = expr.args[0] - if isinstance(argument, astroid.Call) and \ - 'format' == self.get_func_name(argument.func): + if isinstance(argument, astroid.Call) and "format" == self.get_func_name(argument.func): argument = argument.func.expr elif isinstance(argument, astroid.BinOp): argument = argument.left - if isinstance(argument, astroid.Const) and \ - argument.name == 'str' and \ - func_name in self.config.odoo_exceptions: - self.add_message( - 'translation-required', node=node, - args=(func_name, '', argument.as_string())) + if isinstance(argument, astroid.Const) and argument.name == "str" and func_name in self.config.odoo_exceptions: + self.add_message("translation-required", node=node, args=(func_name, "", argument.as_string())) def get_cursor_name(self, node): expr_list = [] @@ -1046,5 +1098,5 @@ def get_cursor_name(self, node): node_expr = node_expr.expr if isinstance(node_expr, astroid.Name): expr_list.insert(0, node_expr.name) - cursor_name = '.'.join(expr_list) + cursor_name = ".".join(expr_list) return cursor_name diff --git a/src/pylint_odoo/misc.py b/src/pylint_odoo/misc.py index 8bb0c7d5..46da835c 100644 --- a/src/pylint_odoo/misc.py +++ b/src/pylint_odoo/misc.py @@ -4,21 +4,31 @@ import string from pylint.checkers import BaseChecker, BaseTokenChecker -from pylint.interfaces import UNDEFINED -from pylint.interfaces import IAstroidChecker, ITokenChecker - +from pylint.interfaces import UNDEFINED, IAstroidChecker, ITokenChecker from . import settings - 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', '15.0', '16.0', + "4.2", + "5.0", + "6.0", + "6.1", + "7.0", + "8.0", + "9.0", + "10.0", + "11.0", + "12.0", + "13.0", + "14.0", + "15.0", + "16.0", ] DFTL_MANIFEST_VERSION_FORMAT = r"({valid_odoo_versions})\.\d+\.\d+\.\d+$" # Regex used from https://github.com/translate/translate/blob/9de0d72437/translate/filters/checks.py#L50-L62 # noqa -PRINTF_PATTERN = re.compile(r''' +PRINTF_PATTERN = re.compile( + r""" %( # initial % (?P\d+)% # boost::format style variable order, like %1% | @@ -30,7 +40,9 @@ (?:\.\d+)? # precision (hh\|h\|l\|ll)? # length formatting (?P[\w@])) # type (%s, %d, etc.) - )''', re.VERBOSE) + )""", + re.VERBOSE, +) class StringParseError(TypeError): @@ -42,7 +54,6 @@ def get_plugin_msgs(pylint_run_res): :param pylint_run_res: Object returned by pylint.run method. :return: List of strings with message name. """ - msgs_store = pylint_run_res.linter.msgs_store def get_messages(): return pylint_run_res.linter.msgs_store._messages_definitions @@ -52,7 +63,7 @@ def get_messages(): all_plugin_msgs = [] for key in messages: message = messages[key] - checker_name = message.msgid + checker_name = message.msgid if checker_name == settings.CFG_SECTION: all_plugin_msgs.append(key) return all_plugin_msgs @@ -63,8 +74,7 @@ def join_node_args_kwargs(node): :param node: node to get args and keywords :return: List of args """ - args = (getattr(node, 'args', None) or []) + \ - (getattr(node, 'keywords', None) or []) + args = (getattr(node, "args", None) or []) + (getattr(node, "keywords", None) or []) return args @@ -78,16 +88,14 @@ class PylintOdooChecker(BaseChecker): manifest_file = None manifest_dict = {} - def formatversion(self, string): - valid_odoo_versions = self.linter._all_options[ - 'valid_odoo_versions'].config.valid_odoo_versions - valid_odoo_versions = '|'.join( - map(re.escape, valid_odoo_versions)) - manifest_version_format = self.linter._all_options[ - 'manifest_version_format'].config.manifest_version_format - self.config.manifest_version_format_parsed = ( - manifest_version_format.format(valid_odoo_versions=valid_odoo_versions)) - return re.match(self.config.manifest_version_format_parsed, string) + def formatversion(self, version_string): + valid_odoo_versions = self.linter._all_options["valid_odoo_versions"].config.valid_odoo_versions + valid_odoo_versions = "|".join(map(re.escape, valid_odoo_versions)) + manifest_version_format = self.linter._all_options["manifest_version_format"].config.manifest_version_format + self.config.manifest_version_format_parsed = manifest_version_format.format( + valid_odoo_versions=valid_odoo_versions + ) + return re.match(self.config.manifest_version_format_parsed, version_string) def get_manifest_file(self, node): """Get manifest file path @@ -102,9 +110,9 @@ def get_manifest_file(self, node): if "odoo.addons." in node_name: # we are into a namespace package... node_name = node_name.split("odoo.addons.")[1] - if os.path.basename(node.file) == '__init__.py': - node_name += '.__init__' - for _ in range(node_name.count('.')): + if os.path.basename(node.file) == "__init__.py": + node_name += ".__init__" + for _ in range(node_name.count(".")): module_path = os.path.dirname(module_path) for manifest_basename in settings.MANIFEST_FILES: @@ -128,17 +136,13 @@ def _check_{NAME_KEY}(self, module_path) if manifest_file: self.manifest_file = manifest_file self.odoo_node = node - self.odoo_module_name = os.path.basename( - os.path.dirname(manifest_file)) - self.odoo_module_name_with_ns = "odoo.addons.{}".format( - self.odoo_module_name - ) - with open(self.manifest_file) as f_manifest: + self.odoo_module_name = os.path.basename(os.path.dirname(manifest_file)) + self.odoo_module_name_with_ns = "odoo.addons.{}".format(self.odoo_module_name) + with open(self.manifest_file, encoding="UTF-8") as f_manifest: self.manifest_dict = ast.literal_eval(f_manifest.read()) elif self.odoo_node and os.path.commonprefix( - [os.path.dirname(self.odoo_node.file), - os.path.dirname(node.file)]) != os.path.dirname( - self.odoo_node.file): + [os.path.dirname(self.odoo_node.file), os.path.dirname(node.file)] + ) != os.path.dirname(self.odoo_node.file): # It's not a sub-module python of a odoo module and # it's not a odoo module self.odoo_node = None @@ -146,9 +150,10 @@ def _check_{NAME_KEY}(self, module_path) self.manifest_dict = {} self.manifest_file = None self.is_main_odoo_module = False - if self.manifest_file and os.path.basename(node.file) == '__init__.py' and ( - node.name.count('.') == 0 or - node.name.endswith(self.odoo_module_name_with_ns) + if ( + self.manifest_file + and os.path.basename(node.file) == "__init__.py" + and (node.name.count(".") == 0 or node.name.endswith(self.odoo_module_name_with_ns)) ): self.is_main_odoo_module = True self.node = node @@ -161,29 +166,19 @@ def _check_{NAME_KEY}(self, module_path) self.msg_args = None if not self.linter.is_message_enabled(msg_code): continue - check_method = getattr( - self, '_check_' + name_key.replace('-', '_'), - None) - is_odoo_check = self.is_main_odoo_module and \ - msg_code[1:3] == str(settings.BASE_OMODULE_ID) - is_py_check = msg_code[1:3] == str(settings.BASE_PYMODULE_ID) + getattr(self, "_check_" + name_key.replace("-", "_"), None) def visit_module(self, node): self.wrapper_visit_module(node) - def add_message(self, msg_id, line=None, node=None, args=None, - confidence=UNDEFINED): - version = (self.manifest_dict.get('version') or '' - if isinstance(self.manifest_dict, dict) else '') + def add_message(self, msg_id, line=None, node=None, args=None, confidence=UNDEFINED): + version = self.manifest_dict.get("version") or "" if isinstance(self.manifest_dict, dict) else "" match = self.formatversion(version) - short_version = match.group(1) if match else '' + short_version = match.group(1) if match else "" if not short_version: - valid_odoo_versions = self.linter._all_options[ - 'valid_odoo_versions'].config.valid_odoo_versions - short_version = (valid_odoo_versions[0] if - len(valid_odoo_versions) == 1 else '') - return super(PylintOdooChecker, self).add_message( - msg_id, line, node, args, confidence) + valid_odoo_versions = self.linter._all_options["valid_odoo_versions"].config.valid_odoo_versions + short_version = valid_odoo_versions[0] if len(valid_odoo_versions) == 1 else "" + return super().add_message(msg_id, line, node, args, confidence) class PylintOdooTokenChecker(BaseTokenChecker, PylintOdooChecker): @@ -194,6 +189,7 @@ class PylintOdooTokenChecker(BaseTokenChecker, PylintOdooChecker): # TODO: Change all methods here + class WrapperModuleChecker(PylintOdooChecker): node = None @@ -222,9 +218,7 @@ def _get_format_str_args_kwargs(format_str): placeholders = [] for line in format_str.splitlines(): try: - placeholders.extend( - name for _, name, _, _ in string.Formatter().parse(line) - if name is not None) + placeholders.extend(name for _, name, _, _ in string.Formatter().parse(line) if name is not None) except ValueError: continue for placeholder in placeholders: @@ -242,8 +236,7 @@ def _get_format_str_args_kwargs(format_str): # named "{var0} {var1} {var2} {var0}" format_str_kwargs[placeholder] = 0 if format_str_args: - format_str_args = (range(len(format_str_args)) if max(format_str_args) == 0 - else range(max(format_str_args))) + format_str_args = range(len(format_str_args)) if max(format_str_args) == 0 else range(max(format_str_args)) return format_str_args, format_str_kwargs @staticmethod @@ -261,13 +254,13 @@ def _get_printf_str_args_kwargs(printf_str): kwargs = {} # Remove all escaped %% - printf_str = re.sub('%%', '', printf_str) + printf_str = re.sub("%%", "", printf_str) for line in printf_str.splitlines(): for match in PRINTF_PATTERN.finditer(line): match_items = match.groupdict() - var = '' if match_items['type'] == 's' else 0 - if match_items['key'] is None: + var = "" if match_items["type"] == "s" else 0 + if match_items["key"] is None: args.append(var) else: - kwargs[match_items['key']] = var + kwargs[match_items["key"]] = var return tuple(args) or kwargs diff --git a/src/pylint_odoo/settings.py b/src/pylint_odoo/settings.py index 70bc1b2c..47400210 100644 --- a/src/pylint_odoo/settings.py +++ b/src/pylint_odoo/settings.py @@ -1,4 +1,3 @@ - # pylint plugin global msg number for odoo modules msgs BASE_OMODULE_ID = 79 @@ -13,15 +12,17 @@ # Manifest files of odoo MANIFEST_FILES = [ - '__manifest__.py', - '__odoo__.py', - '__openerp__.py', - '__terp__.py', + "__manifest__.py", + "__odoo__.py", + "__openerp__.py", + "__terp__.py", ] # Message description default -DESC_DFLT = 'You can review guidelines here: ' + \ - 'https://github.com/OCA/odoo-community.org/blob/master/website/' + \ - 'Contribution/CONTRIBUTING.rst' +DESC_DFLT = ( + "You can review guidelines here: " + + "https://github.com/OCA/odoo-community.org/blob/master/website/" + + "Contribution/CONTRIBUTING.rst" +) -CFG_SECTION = 'odoolint' +CFG_SECTION = "odoolint" diff --git a/test-requirements.txt b/test-requirements.txt index d38ebb49..4ddf09e4 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -8,4 +8,4 @@ pytest-cov pytest-xdist tox twine -wheel \ No newline at end of file +wheel diff --git a/tests/test_main.py b/tests/test_main.py index a270304e..6d18db05 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,46 +1,81 @@ import os -import stat import sys -from tempfile import gettempdir, NamedTemporaryFile - import unittest -from contextlib import contextmanager +from tempfile import NamedTemporaryFile, gettempdir from pylint.lint import Run from pylint_odoo import misc - EXPECTED_ERRORS = { - 'website-manifest-key-not-valid-uri': 1, - 'except-pass': 3, 'print-used': 1, 'test-folder-imported': 3, 'use-vim-comment': 1, 'openerp-exception-warning': 3, 'class-camelcase': 1, 'missing-return': 1, 'method-required-super': 8, 'manifest-required-author': 1, 'manifest-required-key': 1, 'manifest-deprecated-key': 1, 'manifest-version-format': 3, 'resource-not-exist': 4, 'manifest-data-duplicated': 1, 'odoo-addons-relative-import': 8, 'attribute-deprecated': 6, 'translation-field': 4, 'method-compute': 2, 'method-search': 2, 'method-inverse': 2, 'attribute-string-redundant': 62, 'context-overridden': 6, 'renamed-field-parameter': 4, 'translation-required': 30, 'translation-contains-variable': 20, 'translation-positional-used': 14, 'invalid-commit': 8, 'sql-injection': 42, 'external-request-timeout': 102, 'eval-referenced': 5, 'manifest-author-string': 1, 'manifest-maintainers-list': 1, 'license-allowed': 1, 'development-status-allowed': 1, 'consider-merging-classes-inherited': 5} - + "website-manifest-key-not-valid-uri": 1, + "except-pass": 3, + "print-used": 1, + "test-folder-imported": 3, + "use-vim-comment": 1, + "openerp-exception-warning": 3, + "class-camelcase": 1, + "missing-return": 1, + "method-required-super": 8, + "manifest-required-author": 1, + "manifest-required-key": 1, + "manifest-deprecated-key": 1, + "manifest-version-format": 3, + "resource-not-exist": 4, + "manifest-data-duplicated": 1, + "odoo-addons-relative-import": 8, + "attribute-deprecated": 6, + "translation-field": 4, + "method-compute": 2, + "method-search": 2, + "method-inverse": 2, + "attribute-string-redundant": 62, + "context-overridden": 6, + "renamed-field-parameter": 4, + "translation-required": 30, + "translation-contains-variable": 20, + "translation-positional-used": 14, + "invalid-commit": 8, + "sql-injection": 42, + "external-request-timeout": 102, + "eval-referenced": 5, + "manifest-author-string": 1, + "manifest-maintainers-list": 1, + "license-allowed": 1, + "development-status-allowed": 1, + "consider-merging-classes-inherited": 5, +} class MainTest(unittest.TestCase): def setUp(self): - dummy_cfg = os.path.join(gettempdir(), 'nousedft.cfg') - with open(dummy_cfg, "w") as f_dummy: + dummy_cfg = os.path.join(gettempdir(), "nousedft.cfg") + with open(dummy_cfg, "w", encoding="UTF-8") as f_dummy: f_dummy.write("") self.default_options = [ - '--load-plugins=pylint_odoo', '--reports=no', '--msg-template=' - '"{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"', - '--output-format=colorized', '--rcfile=%s' % os.devnull, + "--load-plugins=pylint_odoo", + "--reports=no", + "--msg-template=" '"{path}:{line}: [{msg_id}({symbol}), {obj}] {msg}"', + "--output-format=colorized", + "--rcfile=%s" % os.devnull, ] path_modules = os.path.join( - os.path.dirname(os.path.dirname(os.path.realpath(__file__))), - "testing", "resources", 'test_repo') + os.path.dirname(os.path.dirname(os.path.realpath(__file__))), "testing", "resources", "test_repo" + ) self.paths_modules = [] for root, dirs, _ in os.walk(path_modules): for path in dirs: self.paths_modules.append(os.path.join(root, path)) self.odoo_namespace_addons_path = os.path.join( os.path.dirname(os.path.dirname(os.path.realpath(__file__))), - "testing", "resources", - 'test_repo_odoo_namespace', 'odoo') + "testing", + "resources", + "test_repo_odoo_namespace", + "odoo", + ) self.default_extra_params = [ - '--disable=all', - '--enable=odoolint,pointless-statement,trailing-newlines', + "--disable=all", + "--enable=odoolint,pointless-statement,trailing-newlines", ] self.sys_path_origin = list(sys.path) self.maxDiff = None @@ -62,10 +97,8 @@ def run_pylint(self, paths, extra_params=None): def test_10_path_dont_exist(self): """test if path don't exist""" - path_unexist = u'/tmp/____unexist______' - with self.assertRaisesRegex( - OSError, - r'Path "{path}" not found.$'.format(path=path_unexist)): + path_unexist = "/tmp/____unexist______" + with self.assertRaisesRegex(OSError, r'Path "{path}" not found.$'.format(path=path_unexist)): self.run_pylint([path_unexist]) def test_20_expected_errors(self): @@ -79,32 +112,30 @@ def test_25_checks_without_coverage(self): # Some messages can be excluded as they are only applied on certain # Odoo versions (not necessarily 8.0). excluded_msgs = { - 'unnecessary-utf8-coding-comment', - 'xml-deprecated-qweb-directive', + "unnecessary-utf8-coding-comment", + "xml-deprecated-qweb-directive", } - extra_params = ['--valid_odoo_versions=8.0'] + extra_params = ["--valid_odoo_versions=8.0"] pylint_res = self.run_pylint(self.paths_modules, extra_params) msgs_found = pylint_res.linter.stats.by_msg.keys() plugin_msgs = set(misc.get_plugin_msgs(pylint_res)) - excluded_msgs test_missed_msgs = sorted(list(plugin_msgs - set(msgs_found))) self.assertFalse( - test_missed_msgs, - "Checks without test case: {test_missed_msgs}".format( - test_missed_msgs=test_missed_msgs)) + test_missed_msgs, "Checks without test case: {test_missed_msgs}".format(test_missed_msgs=test_missed_msgs) + ) def test_85_valid_odoo_version_format(self): """Test --manifest_version_format parameter""" # First, run Pylint for version 8.0 extra_params = [ - r'--manifest_version_format="8\.0\.\d+\.\d+.\d+$"' - '--valid_odoo_versions=""', - '--disable=all', - '--enable=manifest-version-format', + r'--manifest_version_format="8\.0\.\d+\.\d+.\d+$"' '--valid_odoo_versions=""', + "--disable=all", + "--enable=manifest-version-format", ] pylint_res = self.run_pylint(self.paths_modules, extra_params) real_errors = pylint_res.linter.stats.by_msg expected_errors = { - 'manifest-version-format': 6, + "manifest-version-format": 6, } self.assertDictEqual(real_errors, expected_errors) @@ -113,7 +144,7 @@ def test_85_valid_odoo_version_format(self): pylint_res = self.run_pylint(self.paths_modules, extra_params) real_errors = pylint_res.linter.stats.by_msg expected_errors = { - 'manifest-version-format': 5, + "manifest-version-format": 5, } self.assertDictEqual(real_errors, expected_errors) @@ -121,88 +152,94 @@ def test_90_valid_odoo_versions(self): """Test --valid_odoo_versions parameter when it's '8.0' & '11.0'""" # First, run Pylint for version 8.0 extra_params = [ - '--valid_odoo_versions=8.0', - '--disable=all', - '--enable=manifest-version-format', + "--valid_odoo_versions=8.0", + "--disable=all", + "--enable=manifest-version-format", ] pylint_res = self.run_pylint(self.paths_modules, extra_params) real_errors = pylint_res.linter.stats.by_msg expected_errors = { - 'manifest-version-format': 6, + "manifest-version-format": 6, } self.assertDictEqual(real_errors, expected_errors) # Now for version 11.0 - extra_params[0] = '--valid_odoo_versions=11.0' + extra_params[0] = "--valid_odoo_versions=11.0" pylint_res = self.run_pylint(self.paths_modules, extra_params) real_errors = pylint_res.linter.stats.by_msg expected_errors = { - 'manifest-version-format': 5, + "manifest-version-format": 5, } self.assertDictEqual(real_errors, expected_errors) def test_110_manifest_required_authors(self): - """ Test --manifest_required_authors using a different author and - multiple authors separated by commas + """Test --manifest_required_authors using a different author and + multiple authors separated by commas """ # First, run Pylint using a different author extra_params = [ - '--manifest_required_authors=Vauxoo', - '--disable=all', - '--enable=manifest-required-author', + "--manifest_required_authors=Vauxoo", + "--disable=all", + "--enable=manifest-required-author", ] pylint_res = self.run_pylint(self.paths_modules, extra_params) real_errors = pylint_res.linter.stats.by_msg expected_errors = { - 'manifest-required-author': 4, + "manifest-required-author": 4, } self.assertDictEqual(real_errors, expected_errors) # Then, run it using multiple authors - extra_params[0] = '--manifest_required_authors=Vauxoo,Other' + extra_params[0] = "--manifest_required_authors=Vauxoo,Other" pylint_res = self.run_pylint(self.paths_modules, extra_params) real_errors = pylint_res.linter.stats.by_msg - expected_errors['manifest-required-author'] = 3 + expected_errors["manifest-required-author"] = 3 self.assertDictEqual(real_errors, expected_errors) # Testing deprecated attribute - extra_params[0] = ('--manifest_required_author=' - 'Odoo Community Association (OCA)') + extra_params[0] = "--manifest_required_author=" "Odoo Community Association (OCA)" pylint_res = self.run_pylint(self.paths_modules, extra_params) real_errors = pylint_res.linter.stats.by_msg expected_errors_deprecated = { - 'manifest-required-author': ( - EXPECTED_ERRORS['manifest-required-author']), + "manifest-required-author": (EXPECTED_ERRORS["manifest-required-author"]), } self.assertDictEqual(real_errors, expected_errors_deprecated) def test_140_check_suppress_migrations(self): """Test migrations path supress checks""" extra_params = [ - '--disable=all', - '--enable=invalid-name,unused-argument', + "--disable=all", + "--enable=invalid-name,unused-argument", + ] + path_modules = [ + os.path.join( + os.path.dirname(os.path.dirname(os.path.realpath(__file__))), + "testing", + "resources", + "test_repo", + "test_module", + "migrations", + "10.0.1.0.0", + "pre-migration.py", + ) ] - path_modules = [os.path.join( - os.path.dirname(os.path.dirname(os.path.realpath(__file__))), - "testing", "resources", - '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, + "invalid-name": 1, + "unused-argument": 1, } self.assertDictEqual(real_errors, expected_errors) # Messages raised without plugin - self.default_options.remove('--load-plugins=pylint_odoo') + 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, + "invalid-name": 3, + "unused-argument": 2, } self.assertDictEqual(real_errors, expected_errors) @@ -210,16 +247,20 @@ def test_140_check_migrations_is_not_odoo_module(self): """Checking that migrations folder is not considered a odoo module Related to https://github.com/OCA/pylint-odoo/issues/357""" extra_params = [ - '--disable=all', - '--enable=missing-readme', + "--disable=all", + "--enable=missing-readme", ] test_module = os.path.join( os.path.dirname(os.path.dirname(os.path.realpath(__file__))), - "testing", "resources", - 'test_repo', 'test_module') + "testing", + "resources", + "test_repo", + "test_module", + ) path_modules = [ - os.path.join(test_module, '__init__.py'), - os.path.join(test_module, 'migrations', '10.0.1.0.0', 'pre-migration.py')] + os.path.join(test_module, "__init__.py"), + os.path.join(test_module, "migrations", "10.0.1.0.0", "pre-migration.py"), + ] pylint_res = self.run_pylint(path_modules, extra_params) real_errors = pylint_res.linter.stats.by_msg expected_errors = {} @@ -229,11 +270,8 @@ def test_145_check_fstring_sqli(self): """Verify the linter is capable of finding SQL Injection vulnerabilities when using fstrings. Related to https://github.com/OCA/pylint-odoo/issues/363""" - extra_params = [ - '--disable=all', - '--enable=sql-injection' - ] - queries = ''' + extra_params = ["--disable=all", "--enable=sql-injection"] + queries = """ def fstring_sqli(self): self.env.cr.execute(f"SELECT * FROM TABLE WHERE SQLI = {self.table}") self.env.cr.execute( @@ -248,25 +286,25 @@ def fstring_no_sqli(self): f"CREATE VIEW {self._table} AS (SELECT * FROM res_partner)" ) self.env.cr.execute(f"SELECT NAME FROM res_partner LIMIT 10") - ''' - with NamedTemporaryFile(mode='w') as f: - f.write(queries) - f.flush() - pylint_res = self.run_pylint([f.name], extra_params) + """ + with NamedTemporaryFile(mode="w") as tmp_f: + tmp_f.write(queries) + tmp_f.flush() + pylint_res = self.run_pylint([tmp_f.name], extra_params) real_errors = pylint_res.linter.stats.by_msg - self.assertDictEqual(real_errors, {'sql-injection': 4}) + self.assertDictEqual(real_errors, {"sql-injection": 4}) def test_150_check_only_enabled_one_check(self): """Checking -d all -e ONLY-ONE-CHECK""" - disable = '--disable=all' + disable = "--disable=all" for expected_error_name, expected_error_value in EXPECTED_ERRORS.items(): - enable = '--enable=%s' % expected_error_name + enable = "--enable=%s" % expected_error_name pylint_res = self.run_pylint(self.paths_modules, [disable, enable]) real_errors = pylint_res.linter.stats.by_msg expected_errors = {expected_error_name: expected_error_value} self.assertDictEqual(real_errors, expected_errors) -if __name__ == '__main__': +if __name__ == "__main__": unittest.main()