From 889e8aacb73f9c059cd5d39d88c3a612c44fbae8 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 25 May 2021 12:51:10 +0530 Subject: [PATCH 1/2] ci(semgrep): false +ve translation on templates separated JS and python rules for granuarilty. Ignore matches with microtemplating that have this structure: `{{.*_.*}}` in string. --- .github/helper/semgrep_rules/ux.js | 9 +++++++++ .github/helper/semgrep_rules/ux.py | 18 +++++++++--------- .github/helper/semgrep_rules/ux.yml | 21 ++++++++++++++++++--- 3 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 .github/helper/semgrep_rules/ux.js diff --git a/.github/helper/semgrep_rules/ux.js b/.github/helper/semgrep_rules/ux.js new file mode 100644 index 0000000000..ae73f9cc60 --- /dev/null +++ b/.github/helper/semgrep_rules/ux.js @@ -0,0 +1,9 @@ + +// ok: frappe-missing-translate-function-js +frappe.msgprint('{{ _("Both login and password required") }}'); + +// ruleid: frappe-missing-translate-function-js +frappe.msgprint('What'); + +// ok: frappe-missing-translate-function-js +frappe.throw(' {{ _("Both login and password required") }}. '); diff --git a/.github/helper/semgrep_rules/ux.py b/.github/helper/semgrep_rules/ux.py index 4a74457435..a00d3cd8ae 100644 --- a/.github/helper/semgrep_rules/ux.py +++ b/.github/helper/semgrep_rules/ux.py @@ -2,30 +2,30 @@ import frappe from frappe import msgprint, throw, _ -# ruleid: frappe-missing-translate-function +# ruleid: frappe-missing-translate-function-python throw("Error Occured") -# ruleid: frappe-missing-translate-function +# ruleid: frappe-missing-translate-function-python frappe.throw("Error Occured") -# ruleid: frappe-missing-translate-function +# ruleid: frappe-missing-translate-function-python frappe.msgprint("Useful message") -# ruleid: frappe-missing-translate-function +# ruleid: frappe-missing-translate-function-python msgprint("Useful message") -# ok: frappe-missing-translate-function +# ok: frappe-missing-translate-function-python translatedmessage = _("Hello") -# ok: frappe-missing-translate-function +# ok: frappe-missing-translate-function-python throw(translatedmessage) -# ok: frappe-missing-translate-function +# ok: frappe-missing-translate-function-python msgprint(translatedmessage) -# ok: frappe-missing-translate-function +# ok: frappe-missing-translate-function-python msgprint(_("Helpful message")) -# ok: frappe-missing-translate-function +# ok: frappe-missing-translate-function-python frappe.throw(_("Error occured")) diff --git a/.github/helper/semgrep_rules/ux.yml b/.github/helper/semgrep_rules/ux.yml index ed06a6a80c..dd667f36c0 100644 --- a/.github/helper/semgrep_rules/ux.yml +++ b/.github/helper/semgrep_rules/ux.yml @@ -1,15 +1,30 @@ rules: -- id: frappe-missing-translate-function +- id: frappe-missing-translate-function-python pattern-either: - patterns: - pattern: frappe.msgprint("...", ...) - pattern-not: frappe.msgprint(_("..."), ...) - - pattern-not: frappe.msgprint(__("..."), ...) - patterns: - pattern: frappe.throw("...", ...) - pattern-not: frappe.throw(_("..."), ...) + message: | + All user facing text must be wrapped in translate function. Please refer to translation documentation. https://frappeframework.com/docs/user/en/guides/basics/translations + languages: [python] + severity: ERROR + +- id: frappe-missing-translate-function-js + pattern-either: + - patterns: + - pattern: frappe.msgprint("...", ...) + - pattern-not: frappe.msgprint(__("..."), ...) + # ignore microtemplating e.g. msgprint("{{ _("server side translation") }}") + - pattern-not: frappe.msgprint("=~/\{\{.*\_.*\}\}/i", ...) + - patterns: + - pattern: frappe.throw("...", ...) - pattern-not: frappe.throw(__("..."), ...) + # ignore microtemplating + - pattern-not: frappe.throw("=~/\{\{.*\_.*\}\}/i", ...) message: | All user facing text must be wrapped in translate function. Please refer to translation documentation. https://frappeframework.com/docs/user/en/guides/basics/translations - languages: [python, javascript, json] + languages: [javascript] severity: ERROR From faa347461d7e09136803d82931f179b90aabcfb7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 25 May 2021 13:00:02 +0530 Subject: [PATCH 2/2] chore: fix all findings flaged by ux rule --- frappe/desk/doctype/dashboard/dashboard.py | 2 +- frappe/desk/doctype/dashboard_chart/dashboard_chart.py | 2 +- frappe/templates/includes/contact.js | 10 ++++------ frappe/tests/ui_test_helpers.py | 3 ++- frappe/utils/jinja.py | 4 ++-- frappe/utils/password.py | 2 +- frappe/utils/safe_exec.py | 3 ++- 7 files changed, 13 insertions(+), 13 deletions(-) diff --git a/frappe/desk/doctype/dashboard/dashboard.py b/frappe/desk/doctype/dashboard/dashboard.py index 4e66318769..90620ed3c5 100644 --- a/frappe/desk/doctype/dashboard/dashboard.py +++ b/frappe/desk/doctype/dashboard/dashboard.py @@ -22,7 +22,7 @@ class Dashboard(Document): def validate(self): if not frappe.conf.developer_mode and self.is_standard: - frappe.throw('Cannot edit Standard Dashboards') + frappe.throw(_('Cannot edit Standard Dashboards')) if self.is_standard: non_standard_docs_map = { diff --git a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py index 48b34e6cd9..cd934c54e6 100644 --- a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py @@ -326,7 +326,7 @@ class DashboardChart(Document): def validate(self): if not frappe.conf.developer_mode and self.is_standard: - frappe.throw('Cannot edit Standard charts') + frappe.throw(_('Cannot edit Standard charts')) if self.chart_type != 'Custom' and self.chart_type != 'Report': self.check_required_field() self.check_document_type() diff --git a/frappe/templates/includes/contact.js b/frappe/templates/includes/contact.js index 130d8bfc0c..fb0b73ff80 100644 --- a/frappe/templates/includes/contact.js +++ b/frappe/templates/includes/contact.js @@ -12,14 +12,12 @@ frappe.ready(function() { var message = $('[name="message"]').val(); if(!(email && message)) { - frappe.msgprint("{{ _("Please enter both your email and message so that we \ - can get back to you. Thanks!") }}"); + frappe.msgprint('{{ _("Please enter both your email and message so that we can get back to you. Thanks!") }}'); return false; } if(!validate_email(email)) { - frappe.msgprint("{{ _("You seem to have written your name instead of your email. \ - Please enter a valid email address so that we can get back.") }}"); + frappe.msgprint('{{ _("You seem to have written your name instead of your email. Please enter a valid email address so that we can get back.") }}'); $('[name="email"]').focus(); return false; } @@ -31,9 +29,9 @@ frappe.ready(function() { message: message, callback: function(r) { if(r.message==="okay") { - frappe.msgprint("{{ _("Thank you for your message") }}"); + frappe.msgprint('{{ _("Thank you for your message") }}'); } else { - frappe.msgprint("{{ _("There were errors") }}"); + frappe.msgprint('{{ _("There were errors") }}'); console.log(r.exc); } $(':input').val(''); diff --git a/frappe/tests/ui_test_helpers.py b/frappe/tests/ui_test_helpers.py index f56311b2e3..7670f99698 100644 --- a/frappe/tests/ui_test_helpers.py +++ b/frappe/tests/ui_test_helpers.py @@ -1,4 +1,5 @@ import frappe +from frappe import _ from frappe.utils import add_to_date, now @frappe.whitelist() @@ -10,7 +11,7 @@ def create_if_not_exists(doc): ''' if not frappe.local.dev_server: - frappe.throw('This method can only be accessed in development', frappe.PermissionError) + frappe.throw(_('This method can only be accessed in development'), frappe.PermissionError) doc = frappe.parse_json(doc) diff --git a/frappe/utils/jinja.py b/frappe/utils/jinja.py index a77eca4977..c276526c52 100644 --- a/frappe/utils/jinja.py +++ b/frappe/utils/jinja.py @@ -67,7 +67,7 @@ def render_template(template, context, is_path=None, safe_render=True): :param safe_render: (optional) prevent server side scripting via jinja templating ''' - from frappe import get_traceback, throw + from frappe import get_traceback, throw, _ from jinja2 import TemplateError if not template: @@ -77,7 +77,7 @@ def render_template(template, context, is_path=None, safe_render=True): return get_jenv().get_template(template).render(context) else: if safe_render and ".__" in template: - throw("Illegal template") + throw(_("Illegal template")) try: return get_jenv().from_string(template).render(context) except TemplateError: diff --git a/frappe/utils/password.py b/frappe/utils/password.py index fbed3cd8e7..fef0bab8af 100644 --- a/frappe/utils/password.py +++ b/frappe/utils/password.py @@ -61,7 +61,7 @@ def set_encrypted_password(doctype, name, pwd, fieldname='password'): except frappe.db.DataError as e: if ((frappe.db.db_type == 'mariadb' and e.args[0] == DATA_TOO_LONG) or (frappe.db.db_type == 'postgres' and e.pgcode == STRING_DATA_RIGHT_TRUNCATION)): - frappe.throw("Most probably your password is too long.", exc=e) + frappe.throw(_("Most probably your password is too long."), exc=e) raise e diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index 643812b226..21508706c9 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -5,6 +5,7 @@ from html2text import html2text from RestrictedPython import compile_restricted, safe_globals import RestrictedPython.Guards import frappe +from frappe import _ import frappe.utils import frappe.utils.data from frappe.website.utils import (get_shade, get_toc, get_next_link) @@ -31,7 +32,7 @@ class NamespaceDict(frappe._dict): def safe_exec(script, _globals=None, _locals=None): # script reports must be enabled via site_config.json if not frappe.conf.server_script_enabled: - frappe.throw('Please Enable Server Scripts', ServerScriptNotEnabled) + frappe.throw(_('Please Enable Server Scripts'), ServerScriptNotEnabled) # build globals exec_globals = get_safe_globals()