Skip to content

Commit

Permalink
[FIX] translation-not-lazy: Fix AstroidSyntaxError (#509)
Browse files Browse the repository at this point in the history
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 "<unknown>", 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 (<unknown>, 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 <listcomp>
    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
```
  • Loading branch information
moylop260 authored Dec 19, 2024
1 parent 08a3ea3 commit fc39ad4
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 21 deletions.
32 changes: 16 additions & 16 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand Down
14 changes: 10 additions & 4 deletions src/pylint_odoo/checkers/custom_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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", {"_"}):
Expand Down
3 changes: 2 additions & 1 deletion src/pylint_odoo/checkers/odoo_addons.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit fc39ad4

Please sign in to comment.