Skip to content

Commit

Permalink
[ADD] prefer-env-translation: Add new check for odoo v18.0
Browse files Browse the repository at this point in the history 
Related to odoo/odoo#174844
  • Loading branch information
@trisdoan @moylop260
trisdoan authored and moylop260 committed Jan 13, 2025
1 parent 2a40df0 commit 3b63fa9
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 18 deletions.
4 changes: 4 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ To know what version of odoo are you running pylint needs the parameter
with particular odoo version e.g. `"16.0"`
Check valid only for odoo >= 18.0
prefer-env-translation
Checks valid only for odoo >= 14.0
translation-format-interpolation
Expand Down
4 changes: 2 additions & 2 deletions src/pylint_odoo/checkers/custom_logging.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ def add_message(self, msgid, *args, **kwargs):
return super().add_message(msgid, *args, **kwargs)

def visit_call(self, node):
if not isinstance(node.func, nodes.Name):
name = OdooAddons.get_func_name(node.func)
if name != "_":
return
name = node.func.name
with config_logging_modules(self.linter, ("odoo",)):
self._check_log_method(node, name)

Expand Down
20 changes: 14 additions & 6 deletions src/pylint_odoo/checkers/odoo_addons.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,11 @@
"deprecated-odoo-model-method",
CHECK_DESCRIPTION,
),
"W8161": (
"Better using self.env._ More info at https://github.com/odoo/odoo/pull/174844",
"prefer-env-translation",
CHECK_DESCRIPTION,
),
}

DFTL_MANIFEST_REQUIRED_KEYS = ["license"]
Expand Down Expand Up @@ -569,6 +574,7 @@ class OdooAddons(OdooBaseChecker, BaseChecker):
"odoo_maxversion": "13.0",
},
"no-raise-unlink": {"odoo_minversion": "15.0"},
"prefer-env-translation": {"odoo_minversion": "18.0"},
}

def __init__(self, linter: PyLinter):
Expand Down Expand Up @@ -789,6 +795,7 @@ def _get_assignation_nodes(self, node):
"method-inverse",
"method-search",
"no-write-in-compute",
"prefer-env-translation",
"print-used",
"renamed-field-parameter",
"sql-injection",
Expand Down Expand Up @@ -868,11 +875,7 @@ def visit_call(self, node):
)
if method_name and self.class_odoo_models:
self.odoo_computes.add(method_name)
if (
isinstance(argument_aux, nodes.Call)
and isinstance(argument_aux.func, nodes.Name)
and argument_aux.func.name == "_"
):
if isinstance(argument_aux, nodes.Call) and self.get_func_name(argument_aux.func) == "_":
self.add_message("translation-field", node=argument_aux)
index += 1
# Check cr.commit()
Expand Down Expand Up @@ -942,7 +945,12 @@ def visit_call(self, node):
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, nodes.Name) and node.func.name == "_" and node.args:
if self.get_func_name(node.func) == "_" and node.args:
# "_" -> isinstance(node.func, nodes.Name)
# "self.env._" -> isinstance(node.func, nodes.Attribute)
if isinstance(node.func, nodes.Name) and node.func.as_string() == "_":
self.add_message("prefer-env-translation", node=node)

wrong = ""
right = ""
arg = node.args[0]
Expand Down
96 changes: 96 additions & 0 deletions testing/resources/test_repo/broken_module/models/broken_model.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,70 @@ def my_method1(self, variable1):
self.message_post(_('Double method _ and lstrtip %s').lstrip() % (variable1,)) # TODO: Emit message for this case
return error_msg

def my_method11(self, variable1):
# Shouldn't show error of field-argument-translate
self.my_method2(self.env._('hello world'))

# Message post with new translation function
self.message_post(subject=self.env._('Subject translatable'),
body=self.env._('Body translatable'))
self.message_post(self.env._('Body translatable'),
self.env._('Subject translatable'))
self.message_post(self.env._('Body translatable'),
subject=self.env._('Subject translatable'))
self.message_post(self.env._('A CDR has been recovered for %s') % (variable1,))
self.message_post(self.env._('A CDR has been recovered for %s') % variable1)
self.message_post(self.env._('Var {a}').format(a=variable1))
self.message_post(self.env._('Var %(variable)s') % {'variable': variable1})
self.message_post(subject=self.env._('Subject translatable'),
body=self.env._('Body translatable %s') % variable1)
self.message_post(subject=self.env._('Subject translatable %(variable)s') %
{'variable': variable1},
message_type='notification')
self.message_post(self.env._('Body translatable'),
self.env._('Subject translatable {a}').format(a=variable1))
self.message_post(self.env._('Body translatable %s') % variable1,
self.env._('Subject translatable %(variable)s') %
{'variable': variable1})
self.message_post('<p>%s</p>' % self.env._('Body translatable'))
self.message_post(body='<p>%s</p>' % self.env._('Body translatable'))

# translation new function with variables in the term
variable2 = variable1
self.message_post(self.env._('Variable not translatable: %s' % variable1))
self.message_post(self.env._('Variables not translatable: %s, %s' % (
variable1, variable2)))
self.message_post(body=self.env._('Variable not translatable: %s' % variable1))
self.message_post(body=self.env._('Variables not translatable: %s %s' % (
variable1, variable2)))
error_msg = self.env._('Variable not translatable: %s' % variable1)
error_msg = self.env._('Variables not translatable: %s, %s' % (
variable1, variable2))
error_msg = self.env._('Variable not translatable: {}'.format(variable1))
error_msg = self.env._('Variables not translatable: {}, {variable2}'.format(
variable1, variable2=variable2))

# string with parameters without name
# so you can't change the order in the translation
self.env._('%s %d') % ('hello', 3)
self.env._('%s %s') % ('hello', 'world')
self.env._('{} {}').format('hello', 3)
self.env._('{} {}').format('hello', 'world')

# Valid cases
self.env._('%(strname)s') % {'strname': 'hello'}
self.env._('%(strname)s %(intname)d') % {'strname': 'hello', 'intname': 3}
self.env._('%s') % 'hello'
self.env._('%d') % 3
self.env._('{}').format('hello')
self.env._('{}').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(self.env._('Double method _ and lstrtip %s').lstrip() % (variable1,)) # TODO: Emit message for this case
return error_msg

def my_method2(self, variable2):
return variable2

Expand Down Expand Up @@ -432,6 +496,12 @@ def my_method7(self):
# Method with translation
raise UserError(_('String with translation'))

def my_method71(self):
user_id = 1
if user_id != 99:
# Method with translation
raise UserError(self.env._('String with translation'))

def my_method8(self):
user_id = 1
if user_id != 99:
Expand Down Expand Up @@ -476,6 +546,17 @@ def my_method13(self):
raise exceptions.Warning(_(
'String with params format %(p1)s' % {'p1': 'v1'}))

def my_method131(self):
# Shouldn't show error
raise exceptions.Warning(self.env._(
'String with params format {p1}').format(p1='v1'))
raise exceptions.Warning(self.env._(
'String with params format {p1}'.format(p1='v1')))
raise exceptions.Warning(self.env._(
'String with params format %(p1)s') % {'p1': 'v1'})
raise exceptions.Warning(self.env._(
'String with params format %(p1)s' % {'p1': 'v1'}))

def my_method14(self):
_("String with missing args %s %s", "param1")
_("String with missing kwargs %(param1)s", param2="hola")
Expand All @@ -491,6 +572,21 @@ def my_method14(self):
_("String with correct args %s", "param1")
_("String with correct kwargs %(param1)s", param1="hola")

def my_method141(self):
self.env._("String with missing args %s %s", "param1")
self.env._("String with missing kwargs %(param1)s", param2="hola")
self.env._(f"String with f-interpolation {self.param1}")
self.env._("String unsupported character %y", "param1")
self.env._("format truncated %s%", 'param1')
self.env._("too many args %s", 'param1', 'param2')

self.env._("multi-positional args without placeholders %s %s", 'param1', 'param2')

self.env._("multi-positional args without placeholders {} {}".format('param1', 'param2'))

self.env._("String with correct args %s", "param1")
self.env._("String with correct kwargs %(param1)s", param1="hola")

def old_api_method_alias(self, cursor, user, ids, context=None): # old api
pass

Expand Down
43 changes: 33 additions & 10 deletions tests/test_main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,23 @@
"no-write-in-compute": 16,
"odoo-addons-relative-import": 4,
"odoo-exception-warning": 4,
"prefer-env-translation": 57,
"print-used": 1,
"renamed-field-parameter": 2,
"resource-not-exist": 4,
"sql-injection": 21,
"test-folder-imported": 3,
"translation-contains-variable": 11,
"translation-contains-variable": 22,
"translation-field": 2,
"translation-format-interpolation": 4,
"translation-format-truncated": 1,
"translation-fstring-interpolation": 1,
"translation-not-lazy": 21,
"translation-positional-used": 10,
"translation-format-interpolation": 8,
"translation-format-truncated": 2,
"translation-fstring-interpolation": 2,
"translation-not-lazy": 42,
"translation-positional-used": 20,
"translation-required": 15,
"translation-too-few-args": 1,
"translation-too-many-args": 1,
"translation-unsupported-format": 1,
"translation-too-few-args": 2,
"translation-too-many-args": 2,
"translation-unsupported-format": 2,
"use-vim-comment": 1,
"website-manifest-key-not-valid-uri": 1,
"no-raise-unlink": 2,
Expand Down Expand Up @@ -150,6 +151,7 @@ def test_20_expected_errors(self):
def test_25_checks_excluding_by_odoo_version(self):
"""All odoolint errors vs found but excluding based on Odoo version"""
excluded_msgs = {
"prefer-env-translation",
"translation-format-interpolation",
"translation-format-truncated",
"translation-fstring-interpolation",
Expand All @@ -176,7 +178,12 @@ def test_35_checks_emiting_by_odoo_version(self):
real_errors = pylint_res.linter.stats.by_msg
expected_errors = self.expected_errors.copy()
expected_errors.update({"manifest-version-format": 6})
excluded_msgs = {"no-raise-unlink", "translation-contains-variable", "deprecated-odoo-model-method"}
excluded_msgs = {
"no-raise-unlink",
"translation-contains-variable",
"deprecated-odoo-model-method",
"prefer-env-translation",
}
for excluded_msg in excluded_msgs:
expected_errors.pop(excluded_msg)
self.assertEqual(expected_errors, real_errors)
Expand Down Expand Up @@ -322,6 +329,22 @@ def test_140_check_migrations_is_not_odoo_module(self):
expected_errors = {}
self.assertDictEqual(real_errors, expected_errors)

def test_gettext_env(self):
"""prefer-env-translation is only valid for odoo v18.0+ but not for older odoo versions"""
pylint_res = self.run_pylint(
self.paths_modules, ["--valid-odoo-versions=18.0", "--disable=all", "--enable=prefer-env-translation"]
)
real_errors = pylint_res.linter.stats.by_msg
expected_errors = {"prefer-env-translation": self.expected_errors.get("prefer-env-translation")}
self.assertEqual(expected_errors, real_errors)

pylint_res = self.run_pylint(
self.paths_modules, ["--valid-odoo-versions=17.0", "--disable=all", "--enable=prefer-env-translation"]
)
real_errors = pylint_res.linter.stats.by_msg
expected_errors = {}
self.assertEqual(expected_errors, real_errors)

@unittest.skipUnless(not sys.platform.startswith("win"), "TOOD: Fix with windows") # TODO: Fix it
def test_145_check_fstring_sqli(self):
"""Verify the linter is capable of finding SQL Injection vulnerabilities
Expand Down

0 comments on commit 3b63fa9

Please sign in to comment.