diff --git a/odoo/addons/test_lint/tests/__init__.py b/odoo/addons/test_lint/tests/__init__.py index 0b1bb8fb7bc..681e1270bb5 100644 --- a/odoo/addons/test_lint/tests/__init__.py +++ b/odoo/addons/test_lint/tests/__init__.py @@ -1,3 +1,4 @@ from . import test_pylint from . import test_ecmascript from . import test_markers +from . import test_onchange_domains diff --git a/odoo/addons/test_lint/tests/_odoo_checkers.py b/odoo/addons/test_lint/tests/_odoo_checkers.py deleted file mode 100644 index 5648ac52219..00000000000 --- a/odoo/addons/test_lint/tests/_odoo_checkers.py +++ /dev/null @@ -1,44 +0,0 @@ -#!/usr/bin/env python -# -*- coding: utf-8 -*- - -import tokenize - -from pylint import checkers, interfaces - -class PEP3110TokenChecker(checkers.BaseTokenChecker): - __implements__ = interfaces.ITokenChecker - name = 'python3' - enabled = False - - msgs = { - 'E3110': ('Python 3 uses `as` instead of comma token to catch exceptions', - 'no-comma-exception', - 'See http://www.python.org/dev/peps/pep-3110/', - {'maxversion': (3, 0)}), - } - - def process_tokens(self, tokens): - comma_found = in_except = False - pcount = 0 - for tok_type, token, start, _, _ in tokens: - if tok_type == tokenize.NAME and token == 'except': - in_except = True - comma_found = False - pcount = 0 - - elif tok_type == tokenize.OP and in_except: - if token == '(': - pcount += 1 - elif token == ')': - pcount -= 1 - elif token == ',' and pcount == 0: - comma_found = True - if token == ':': - if in_except and comma_found: - self.add_message('no-comma-exception', line=start[0]) - comma_found = in_except = False - pcount = 0 - - -def register(linter): - linter.register_checker(PEP3110TokenChecker(linter)) diff --git a/odoo/addons/test_lint/tests/lint_case.py b/odoo/addons/test_lint/tests/lint_case.py new file mode 100644 index 00000000000..8e41625c5a8 --- /dev/null +++ b/odoo/addons/test_lint/tests/lint_case.py @@ -0,0 +1,22 @@ +import fnmatch +import os +j = os.path.join + +from odoo.modules import get_modules, get_module_path +from odoo.tests import BaseCase + + +class LintCase(BaseCase): + """ Utility method for lint-type cases + """ + + def iter_module_files(self, *globs): + """ Yields the paths of all the module files matching the provided globs + (AND-ed) + """ + for modroot in map(get_module_path, get_modules()): + for root, _, fnames in os.walk(modroot): + fnames = [j(root, n) for n in fnames] + for glob in globs: + fnames = fnmatch.filter(fnames, glob) + yield from fnames diff --git a/odoo/addons/test_lint/tests/test_ecmascript.py b/odoo/addons/test_lint/tests/test_ecmascript.py index 22e1a8fc8e4..7e7069796b9 100644 --- a/odoo/addons/test_lint/tests/test_ecmascript.py +++ b/odoo/addons/test_lint/tests/test_ecmascript.py @@ -6,10 +6,11 @@ import os import re import subprocess from unittest import skipIf -from odoo.tests.common import TransactionCase from odoo import tools from odoo.modules import get_modules, get_module_path +from . import lint_case + MAX_ES_VERSION = 'es8' _logger = logging.getLogger(__name__) @@ -21,7 +22,7 @@ except IOError: @skipIf(es_check is None, "es-check tool not found on this system") -class TestECMAScriptVersion(TransactionCase): +class TestECMAScriptVersion(lint_case.LintCase): longMessage = True @@ -30,16 +31,12 @@ class TestECMAScriptVersion(TransactionCase): black_re = re.compile(r'summernote.+(intro\.js|outro.js)$') - mod_paths = [get_module_path(m) for m in get_modules()] - files_to_check = [] - for p in mod_paths: - for dp, _, file_names in os.walk(p): - if 'static/test' in dp or "static/src/tests" in dp: - continue - for fn in file_names: - fullpath_name = os.path.join(dp, fn) - if fullpath_name.endswith('.js') and not black_re.search(fullpath_name): - files_to_check.append(fullpath_name) + files_to_check = [ + p for p in self.iter_module_files('*.js') + if not 'static/test' in p + if not 'static/src/tests' in p + if not black_re.search(p) + ] _logger.info('Testing %s js files', len(files_to_check)) cmd = [es_check, MAX_ES_VERSION] + files_to_check diff --git a/odoo/addons/test_lint/tests/test_markers.py b/odoo/addons/test_lint/tests/test_markers.py index 24b8e288d3c..7e52c7ac761 100644 --- a/odoo/addons/test_lint/tests/test_markers.py +++ b/odoo/addons/test_lint/tests/test_markers.py @@ -4,14 +4,15 @@ import logging import os import odoo -from odoo.tests.common import TransactionCase + +from . import lint_case _logger = logging.getLogger(__name__) MARKERS = [b'<' * 7, b'>' * 7] EXTENSIONS = ('.py', '.js', '.xml', '.less', '.sass') -class TestConflictMarkers(TransactionCase): +class TestConflictMarkers(lint_case.LintCase): def check_file(self, fullpath_name): diff --git a/odoo/addons/test_lint/tests/test_onchange_domains.py b/odoo/addons/test_lint/tests/test_onchange_domains.py new file mode 100644 index 00000000000..1e6c561a5f0 --- /dev/null +++ b/odoo/addons/test_lint/tests/test_onchange_domains.py @@ -0,0 +1,60 @@ +import ast +import itertools +import os + +from . import lint_case + +class OnchangeChecker(ast.NodeVisitor): + def visit(self, node): + method = 'visit_' + node.__class__.__name__ + visitor = getattr(self, method, self.generic_visit) + return visitor(node) + + def generic_visit(self, node): + for field, value in ast.iter_fields(node): + if isinstance(value, list): + for item in value: + if isinstance(item, ast.AST): + yield from self.visit(item) + elif isinstance(value, ast.AST): + yield from self.visit(value) + + def matches_onchange(self, node): + if isinstance(node, ast.Call): + if isinstance(node.func, ast.Attribute): + return node.func.attr == 'onchange' + if isinstance(node.func, ast.Name): + return node.func.id == 'onchange' + return False + + def visit_FunctionDef(self, node): + walker = ast.walk(node) if any(map(self.matches_onchange, node.decorator_list)) else [] + # can stop at the first match: an @onchange function either mentions + # domains or does not + return itertools.islice(( + n for n in walker + if isinstance(n, getattr(ast, 'Str', type(None))) and n.s == 'domain' + or isinstance(n, getattr(ast, 'Constant', type(None))) and n.value == 'domain' + ), 1) + +class TestOnchangeDomains(lint_case.LintCase): + """ Would ideally have been a pylint module but that's slow as molasses + (takes minutes to run, and can blow up entirely depending on the pylint + version) + """ + def test_forbid_domains_in_onchanges(self): + """ Dynamic domains (returning a domain from an onchange) are deprecated + and should not be used in "standard" Odoo anymore + """ + checker = OnchangeChecker() + rs = [] + for path in self.iter_module_files('*.py'): + with open(path, 'rb') as f: + t = ast.parse(f.read(), path) + rs.extend(zip(itertools.repeat(os.path.relpath(path)), checker.visit(t))) + + rs.sort(key=lambda t: t[0]) + assert not rs, "probable domains in onchanges at\n" + '\n'.join( + "- %s:%d" % (path, node.lineno) + for path, node in rs + ) diff --git a/odoo/addons/test_lint/tests/test_pylint.py b/odoo/addons/test_lint/tests/test_pylint.py index 747703481ef..844927fabf5 100644 --- a/odoo/addons/test_lint/tests/test_pylint.py +++ b/odoo/addons/test_lint/tests/test_pylint.py @@ -31,6 +31,7 @@ class TestPyLint(TransactionCase): 'mixed-indentation', 'deprecated-method', + # custom checkers 'sql-injection', ] @@ -69,7 +70,7 @@ class TestPyLint(TransactionCase): '--enable=%s' % ','.join(self.ENABLED_CODES), '--reports=n', "--msg-template='{msg} ({msg_id}) at {path}:{line}'", - '--load-plugins=pylint.extensions.bad_builtin,_odoo_checkers,_odoo_checker_sql_injection', + '--load-plugins=pylint.extensions.bad_builtin,_odoo_checker_sql_injection', '--bad-functions=%s' % ','.join(self.BAD_FUNCTIONS), '--deprecated-modules=%s' % ','.join(self.BAD_MODULES) ] diff --git a/odoo/models.py b/odoo/models.py index cb779c32c56..3196fb5209e 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -5894,6 +5894,10 @@ Record ids: %(records)s res['value'].pop('id', None) self.update({key: val for key, val in res['value'].items() if key in self._fields}) if res.get('domain'): + _logger.warning( + "onchange method %s returned a domain, this is deprecated", + method.__qualname__ + ) result.setdefault('domain', {}).update(res['domain']) if res.get('warning'): result['warnings'].add((