From fc39ad43c9bba3849860ac9c04ecd5374257629a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mois=C3=A9s=20L=C3=B3pez=20-=20https=3A//www=2Evauxoo=2Eco?= =?UTF-8?q?m/?= Date: Thu, 19 Dec 2024 12:30:51 -0600 Subject: [PATCH] [FIX] translation-not-lazy: Fix AstroidSyntaxError (#509) There is a corner case where pylint_odoo could raise errors It was considered in the unittest and it was already fixed Using the following code: ```python "%s".lstrip() % "value" ``` `binop.left` is returning the structure different if the method is called `method("%s" % value)` and `"%s".method() % value` So the code above is malformed `'%s'.lstrip( % 'value')` Raising the following error: ```txt Traceback (most recent call last): File "site-packages/astroid/builder.py", line 181, in _data_build node, parser_module = _parse_string( ^^^^^^^^^^^^^^ File "site-packages/astroid/builder.py", line 480, in _parse_string parsed = parser_module.parse( ^^^^^^^^^^^^^^^^^^^^ File "site-packages/astroid/_ast.py", line 30, in parse return ast.parse(string, type_comments=type_comments) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "python3.11/ast.py", line 50, in parse return compile(source, filename, mode, flags, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "", line 1 '%s'.lstrip( % 'value') ^ SyntaxError: invalid syntax The above exception was the direct cause of the following exception: Traceback (most recent call last): File "site-packages/pylint/lint/pylinter.py", line 830, in _check_file check_astroid_module(ast_node) File "site-packages/pylint/lint/pylinter.py", line 1016, in check_astroid_module retval = self._check_astroid_module( ^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "site-packages/pylint/lint/pylinter.py", line 1068, in _check_astroid_module walker.walk(node) File "site-packages/pylint/utils/ast_walker.py", line 94, in walk self.walk(child) File "site-packages/pylint/utils/ast_walker.py", line 94, in walk self.walk(child) File "site-packages/pylint/utils/ast_walker.py", line 94, in walk self.walk(child) [Previous line repeated 1 more time] File "site-packages/pylint/utils/ast_walker.py", line 91, in walk callback(astroid) File "pylint_odoo/checkers/custom_logging.py", line 91, in visit_binop self.visit_call(self.transform_binop2call(node)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "pylint_odoo/checkers/custom_logging.py", line 82, in transform_binop2call new_node = builder.extract_node(new_code) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "site-packages/astroid/builder.py", line 446, in extract_node tree = parse(code, module_name=module_name) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "site-packages/astroid/builder.py", line 303, in parse return builder.string_build(code, modname=module_name, path=path) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "site-packages/astroid/builder.py", line 151, in string_build module, builder = self._data_build(data, modname, path) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "site-packages/astroid/builder.py", line 185, in _data_build raise AstroidSyntaxError( astroid.exceptions.AstroidSyntaxError: Parsing Python code failed: invalid syntax (, line 1) The above exception was the direct cause of the following exception: Traceback (most recent call last): File "python3.11/concurrent/futures/process.py", line 261, in _process_worker r = call_item.fn(*call_item.args, **call_item.kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "python3.11/concurrent/futures/process.py", line 210, in _process_chunk return [fn(*args) for args in chunk] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "python3.11/concurrent/futures/process.py", line 210, in return [fn(*args) for args in chunk] ^^^^^^^^^ File "site-packages/pylint/lint/parallel.py", line 79, in _worker_check_single_file _worker_linter.check_single_file_item(file_item) File "site-packages/pylint/lint/pylinter.py", line 739, in check_single_file_item self._check_file(self.get_ast, check_astroid_module, file) File "site-packages/pylint/lint/pylinter.py", line 832, in _check_file raise astroid.AstroidError from e astroid.exceptions.AstroidError """ The above exception was the direct cause of the following exception: tests/test_main.py:430: in test_180_jobs pylint_res = self.run_pylint(self.paths_modules, verbose=True) tests/test_main.py:120: in run_pylint self._run_pylint(cmd, f_dummy, reporter=reporter) tests/test_main.py:132: in _run_pylint Run(args, reporter=reporter) pylint/testutils/_run.py:41: in __init__ super().__init__(args, reporter, exit) pylint/lint/run.py:211: in __init__ linter.check(args) pylint/lint/pylinter.py:678: in check check_parallel( pylint/lint/parallel.py:153: in check_parallel for ( python3.11/concurrent/futures/process.py:620: in _chain_from_iterable_of_lists for element in iterable: python3.11/concurrent/futures/_base.py:619: in result_iterator yield _result_or_cancel(fs.pop()) python3.11/concurrent/futures/_base.py:317: in _result_or_cancel return fut.result(timeout) python3.11/concurrent/futures/_base.py:456: in result return self.__get_result() python3.11/concurrent/futures/_base.py:[401](https://github.com/OCA/pylint-odoo/actions/runs/12407071620/job/34636504075#step:7:402): in __get_result raise self._exception E astroid.exceptions.AstroidError ``` --- README.md | 32 +++++++++---------- src/pylint_odoo/checkers/custom_logging.py | 14 +++++--- src/pylint_odoo/checkers/odoo_addons.py | 3 +- .../broken_module/models/broken_model.py | 5 +++ 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 1b37ab2e..20734952 100644 --- a/README.md +++ b/README.md @@ -187,15 +187,15 @@ Checks valid only for odoo <= 13.0 * external-request-timeout - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L685 Use of external request method `requests.delete` without timeout. It could wait for a long time - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L686 Use of external request method `requests.get` without timeout. It could wait for a long time - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L687 Use of external request method `requests.head` without timeout. It could wait for a long time + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L690 Use of external request method `requests.delete` without timeout. It could wait for a long time + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L691 Use of external request method `requests.get` without timeout. It could wait for a long time + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L692 Use of external request method `requests.head` without timeout. It could wait for a long time * invalid-commit - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L404 Use of cr.commit() directly - More info https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#never-commit-the-transaction - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L405 Use of cr.commit() directly - More info https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#never-commit-the-transaction - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L406 Use of cr.commit() directly - More info https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#never-commit-the-transaction + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L409 Use of cr.commit() directly - More info https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#never-commit-the-transaction + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L410 Use of cr.commit() directly - More info https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#never-commit-the-transaction + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L411 Use of cr.commit() directly - More info https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#never-commit-the-transaction * license-allowed @@ -270,7 +270,7 @@ Checks valid only for odoo <= 13.0 * no-wizard-in-models - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L824 No wizard class for model directory. See the complete structure https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#complete-structure + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L829 No wizard class for model directory. See the complete structure https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#complete-structure * no-write-in-compute @@ -307,9 +307,9 @@ Checks valid only for odoo <= 13.0 * sql-injection - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L571 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 - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L573 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 - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L575 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 + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L576 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 + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L578 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 + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L580 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 * test-folder-imported @@ -332,15 +332,15 @@ Checks valid only for odoo <= 13.0 - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L380 Use lazy % or .format() or % formatting in odoo._ functions - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L381 Use lazy % or .format() or % formatting in odoo._ functions - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L468 Use lazy % or .format() or % formatting in odoo._ functions + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L473 Use lazy % or .format() or % formatting in odoo._ functions * translation-format-truncated - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L479 Logging format string ends in middle of conversion specifier + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L484 Logging format string ends in middle of conversion specifier * translation-fstring-interpolation - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L477 Use lazy % or .format() or % formatting in odoo._ functions + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L482 Use lazy % or .format() or % formatting in odoo._ functions * translation-not-lazy @@ -362,15 +362,15 @@ Checks valid only for odoo <= 13.0 * translation-too-few-args - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L475 Not enough arguments for odoo._ format string + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L480 Not enough arguments for odoo._ format string * translation-too-many-args - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L480 Too many arguments for odoo._ format string + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L485 Too many arguments for odoo._ format string * translation-unsupported-format - - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L478 Unsupported odoo._ format character 'y' (0x79) at index 30 + - https://github.com/OCA/pylint-odoo/blob/v9.1.4/testing/resources/test_repo/broken_module/models/broken_model.py#L483 Unsupported odoo._ format character 'y' (0x79) at index 30 * use-vim-comment diff --git a/src/pylint_odoo/checkers/custom_logging.py b/src/pylint_odoo/checkers/custom_logging.py index a0a793a4..9eb32350 100644 --- a/src/pylint_odoo/checkers/custom_logging.py +++ b/src/pylint_odoo/checkers/custom_logging.py @@ -2,9 +2,10 @@ from contextlib import contextmanager from unittest.mock import patch -from astroid import builder, nodes +from astroid import builder, exceptions as astroid_exceptions, nodes from pylint.checkers import logging +from .odoo_addons import OdooAddons from .odoo_base_checker import OdooBaseChecker @@ -79,16 +80,21 @@ def transform_binop2call(self, node): _("lazy detectable: %s" % es_err.error) """ new_code = f"{node.left.as_string()[:-1]} {node.op} {node.right.as_string()})" - new_node = builder.extract_node(new_code) + try: + new_node = builder.extract_node(new_code) + except astroid_exceptions.AstroidSyntaxError: + return node_attrs = ["lineno", "col_offset", "parent", "end_lineno", "end_col_offset", "position", "fromlineno"] for node_attr in node_attrs: setattr(new_node, node_attr, getattr(node, node_attr, None)) return new_node def visit_binop(self, node): - if not isinstance(node.left, nodes.Call) or node.op != "%": + if not isinstance(node.left, nodes.Call) or node.op != "%" or OdooAddons.get_func_name(node.left.func) != "_": return - self.visit_call(self.transform_binop2call(node)) + new_node = self.transform_binop2call(node) + if new_node: + self.visit_call(new_node) def _check_log_method(self, *args, **kwargs): with patch("pylint.checkers.logging.CHECKED_CONVENIENCE_FUNCTIONS", {"_"}): diff --git a/src/pylint_odoo/checkers/odoo_addons.py b/src/pylint_odoo/checkers/odoo_addons.py index 61f5bef4..04c0beb6 100644 --- a/src/pylint_odoo/checkers/odoo_addons.py +++ b/src/pylint_odoo/checkers/odoo_addons.py @@ -1298,7 +1298,8 @@ def visit_assign(self, node): if manifest_path: self._odoo_inherit_items[(manifest_path, odoo_class_inherit)].add(node) - def get_func_name(self, node): + @staticmethod + def get_func_name(node): func_name = ( isinstance(node, nodes.Name) and node.name or isinstance(node, nodes.Attribute) and node.attrname or "" ) diff --git a/testing/resources/test_repo/broken_module/models/broken_model.py b/testing/resources/test_repo/broken_module/models/broken_model.py index 411df08c..c781ab15 100644 --- a/testing/resources/test_repo/broken_module/models/broken_model.py +++ b/testing/resources/test_repo/broken_module/models/broken_model.py @@ -395,6 +395,11 @@ def my_method1(self, variable1): _('%d') % 3 _('{}').format('hello') _('{}').format(3) + + # It raised exception but it was already fixed + msg = "Invalid not _ method %s".lstrip() % "value" + # It should emit message but binop.left is showing "lstrip" only instead of "_" + self.message_post(_('Double method _ and lstrtip %s').lstrip() % (variable1,)) # TODO: Emit message for this case return error_msg def my_method2(self, variable2):