def test_rule_patterns(self) -> None:
"""Verifies that the search regex specified in a custom rule actually matches
the expectation and doesn't throw false positives."""
for rule in self.all_rules:
pattern = rule['pattern']
for line in rule.get('good_lines', []):
# create=True is superfluous when mocking built-ins in Python >= 3.5
with patch('builtins.open',
return_value=iter((line + '\n\n').splitlines()),
create=True,
autospec=True):
self.assertFalse(
RuleList([], [rule]).custom_check_file(
'foo.bar', 'baz', ''),
"The pattern '{}' matched the line '{}' while it shouldn't."
.format(pattern, line))
for line in rule.get('bad_lines', []):
# create=True is superfluous when mocking built-ins in Python >= 3.5
with patch('builtins.open',
return_value=iter((line + '\n\n').splitlines()),
create=True,
autospec=True), patch('builtins.print'):
filename = list(rule.get('include_only', {'foo.bar'}))[0]
self.assertTrue(
RuleList([], [rule]).custom_check_file(
filename, 'baz', ''),
"The pattern '{}' didn't match the line '{}' while it should."
.format(pattern, line))
def test_rule_patterns(self) -> None:
"""Verifies that the search regex specified in a custom rule actually matches
the expectation and doesn't throw false positives."""
for rule in self.all_rules:
pattern = rule["pattern"]
for line in rule.get("good_lines", []):
# create=True is superfluous when mocking built-ins in Python >= 3.5
with patch(
"builtins.open",
return_value=StringIO(line + "\n\n"),
create=True,
autospec=True,
):
self.assertFalse(
RuleList([], [rule]).custom_check_file("foo.bar", "baz", ""),
f"The pattern '{pattern}' matched the line '{line}' while it shouldn't.",
)
for line in rule.get("bad_lines", []):
# create=True is superfluous when mocking built-ins in Python >= 3.5
with patch(
"builtins.open",
return_value=StringIO(line + "\n\n"),
create=True,
autospec=True,
), patch("builtins.print"):
filename = list(rule.get("include_only", {"foo.bar"}))[0]
self.assertTrue(
RuleList([], [rule]).custom_check_file(filename, "baz", ""),
f"The pattern '{pattern}' didn't match the line '{line}' while it should.",
)
js_rules = RuleList(
langs=['js'],
rules=[
{'pattern': 'subject|SUBJECT',
'exclude': set(['static/js/util.js',
'frontend_tests/']),
'exclude_pattern': 'emails',
'description': 'avoid subject in JS code',
'good_lines': ['topic_name'],
'bad_lines': ['subject="foo"', ' MAX_SUBJECT_LEN']},
{'pattern': r'[^_]function\(',
'description': 'The keyword "function" should be followed by a space'},
{'pattern': 'msgid|MSGID',
'description': 'Avoid using "msgid" as a variable name; use "message_id" instead.'},
{'pattern': r'.*blueslip.warning\(.*',
'description': 'The module blueslip has no function warning, try using blueslip.warn'},
{'pattern': '[)]{$',
'description': 'Missing space between ) and {'},
{'pattern': r'i18n\.t\([^)]+[^,\{\)]$',
'description': 'i18n string should not be a multiline string'},
{'pattern': r'''i18n\.t\(['"].+?['"]\s*\+''',
'description': 'Do not concatenate arguments within i18n.t()'},
{'pattern': r'i18n\.t\(.+\).*\+',
'description': 'Do not concatenate i18n strings'},
{'pattern': r'\+.*i18n\.t\(.+\)',
'description': 'Do not concatenate i18n strings'},
{'pattern': '[.]includes[(]',
'exclude': {'frontend_tests/'},
'description': '.includes() is incompatible with Internet Explorer. Use .indexOf() !== -1 instead.'},
{'pattern': '[.]html[(]',
'exclude_pattern': r'''[.]html[(]("|'|render_|html|message.content|sub.rendered_description|i18n.t|rendered_|$|[)]|error_text|widget_elem|[$]error|[$][(]"<p>"[)])''',
'exclude': {'static/js/portico', 'static/js/lightbox.js', 'static/js/ui_report.js',
'static/js/confirm_dialog.js',
'frontend_tests/'},
'description': 'Setting HTML content with jQuery .html() can lead to XSS security bugs. Consider .text() or using rendered_foo as a variable name if content comes from handlebars and thus is already sanitized.'},
{'pattern': '["\']json/',
'description': 'Relative URL for JSON route not supported by i18n'},
# This rule is constructed with + to avoid triggering on itself
{'pattern': " =" + '[^ =>~"]',
'description': 'Missing whitespace after "="'},
{'pattern': '^[ ]*//[A-Za-z0-9]',
'description': 'Missing space after // in comment'},
{'pattern': 'if[(]',
'description': 'Missing space between if and ('},
{'pattern': 'else{$',
'description': 'Missing space between else and {'},
{'pattern': '^else {$',
'description': 'Write JS else statements on same line as }'},
{'pattern': '^else if',
'description': 'Write JS else statements on same line as }'},
{'pattern': 'console[.][a-z]',
'exclude': set(['static/js/blueslip.js',
'frontend_tests/zjsunit',
'frontend_tests/casper_lib/common.js',
'frontend_tests/node_tests',
'static/js/debug.js']),
'description': 'console.log and similar should not be used in webapp'},
{'pattern': r'''[.]text\(["'][a-zA-Z]''',
'description': 'Strings passed to $().text should be wrapped in i18n.t() for internationalization',
'exclude': set(['frontend_tests/node_tests/'])},
{'pattern': r'''compose_error\(["']''',
'description': 'Argument to compose_error should be a literal string enclosed '
'by i18n.t()'},
{'pattern': r'ui.report_success\(',
'description': 'Deprecated function, use ui_report.success.'},
{'pattern': r'''report.success\(["']''',
'description': 'Argument to report_success should be a literal string enclosed '
'by i18n.t()'},
{'pattern': r'ui.report_error\(',
'description': 'Deprecated function, use ui_report.error.'},
{'pattern': r'''report.error\(["'][^'"]''',
'description': 'Argument to ui_report.error should be a literal string enclosed '
'by i18n.t()',
'good_lines': ['ui_report.error("")', 'ui_report.error(_("text"))'],
'bad_lines': ['ui_report.error("test")']},
{'pattern': r'\$\(document\)\.ready\(',
'description': "`Use $(f) rather than `$(document).ready(f)`",
'good_lines': ['$(function () {foo();}'],
'bad_lines': ['$(document).ready(function () {foo();}']},
{'pattern': '[$][.](get|post|patch|delete|ajax)[(]',
'description': "Use channel module for AJAX calls",
'exclude': set([
# Internal modules can do direct network calls
'static/js/blueslip.js',
'static/js/channel.js',
# External modules that don't include channel.js
'static/js/stats/',
'static/js/portico/',
'static/js/billing/',
]),
'good_lines': ['channel.get(...)'],
'bad_lines': ['$.get()', '$.post()', '$.ajax()']},
{'pattern': 'style ?=',
'description': "Avoid using the `style=` attribute; we prefer styling in CSS files",
'exclude': set([
'frontend_tests/node_tests/copy_and_paste.js',
'frontend_tests/node_tests/upload.js',
'frontend_tests/node_tests/templates.js',
'static/js/upload.js',
'static/js/stream_color.js',
]),
'good_lines': ['#my-style {color: blue;}'],
'bad_lines': ['<p style="color: blue;">Foo</p>', 'style = "color: blue;"']},
*whitespace_rules,
*comma_whitespace_rule,
],
)
js_rules = RuleList(
langs=['js'],
rules=[
{
'pattern': 'subject|SUBJECT',
'exclude': {'static/js/util.js', 'frontend_tests/'},
'exclude_pattern': 'emails',
'description': 'avoid subject in JS code',
'good_lines': ['topic_name'],
'bad_lines': ['subject="foo"', ' MAX_SUBJECT_LEN']
},
{
'pattern': r'[^_]function\(',
'description':
'The keyword "function" should be followed by a space'
},
{
'pattern':
'msgid|MSGID',
'description':
'Avoid using "msgid" as a variable name; use "message_id" instead.'
},
{
'pattern':
r'.*blueslip.warning\(.*',
'description':
'The module blueslip has no function warning, try using blueslip.warn'
},
{
'pattern': r'i18n\.t\([^)]+[^,\{\)]$',
'description': 'i18n string should not be a multiline string'
},
{
'pattern': r'''i18n\.t\(['"].+?['"]\s*\+''',
'description': 'Do not concatenate arguments within i18n.t()'
},
{
'pattern':
r'''i18n\.t\([a-zA-Z]''',
'exclude': {'static/js/templates.js'},
'description':
'Do not pass a variable into i18n.t; it will not be exported to Transifex for translation.'
},
{
'pattern': r'i18n\.t\(.+\).*\+',
'description': 'Do not concatenate i18n strings'
},
{
'pattern': r'\+.*i18n\.t\(.+\)',
'description': 'Do not concatenate i18n strings'
},
{
'pattern':
'[.]html[(]',
'exclude_pattern':
r'''\.html\(("|'|render_|html|message\.content|util\.clean_user_content_links|i18n\.t|rendered_|$|\)|error_text|widget_elem|\$error|\$\("<p>"\))''',
'exclude': {
'static/js/portico', 'static/js/lightbox.js',
'static/js/ui_report.js', 'static/js/confirm_dialog.js',
'frontend_tests/'
},
'description':
'Setting HTML content with jQuery .html() can lead to XSS security bugs. Consider .text() or using rendered_foo as a variable name if content comes from handlebars and thus is already sanitized.'
},
{
'pattern': '["\']json/',
'description': 'Relative URL for JSON route not supported by i18n'
},
# This rule is constructed with + to avoid triggering on itself
{
'pattern': '^[ ]*//[A-Za-z0-9]',
'description': 'Missing space after // in comment'
},
{
'pattern': r'''[.]text\(["'][a-zA-Z]''',
'description':
'Strings passed to $().text should be wrapped in i18n.t() for internationalization',
'exclude': {'frontend_tests/node_tests/'}
},
{
'pattern':
r'''compose_error\(["']''',
'description':
'Argument to compose_error should be a literal string enclosed '
'by i18n.t()'
},
{
'pattern': r'ui.report_success\(',
'description': 'Deprecated function, use ui_report.success.'
},
{
'pattern':
r'''report.success\(["']''',
'description':
'Argument to report_success should be a literal string enclosed '
'by i18n.t()'
},
{
'pattern': r'ui.report_error\(',
'description': 'Deprecated function, use ui_report.error.'
},
{
'pattern':
r'''report.error\(["'][^'"]''',
'description':
'Argument to ui_report.error should be a literal string enclosed '
'by i18n.t()',
'good_lines':
['ui_report.error("")', 'ui_report.error(_("text"))'],
'bad_lines': ['ui_report.error("test")']
},
{
'pattern': r'\$\(document\)\.ready\(',
'description': "`Use $(f) rather than `$(document).ready(f)`",
'good_lines': ['$(function () {foo();}'],
'bad_lines': ['$(document).ready(function () {foo();}']
},
{
'pattern': '[$][.](get|post|patch|delete|ajax)[(]',
'description': "Use channel module for AJAX calls",
'exclude': {
# Internal modules can do direct network calls
'static/js/blueslip.js',
'static/js/channel.js',
# External modules that don't include channel.js
'static/js/stats/',
'static/js/portico/',
'static/js/billing/',
},
'good_lines': ['channel.get(...)'],
'bad_lines': ['$.get()', '$.post()', '$.ajax()']
},
{
'pattern':
'style ?=',
'description':
"Avoid using the `style=` attribute; we prefer styling in CSS files",
'exclude': {
'frontend_tests/node_tests/copy_and_paste.js',
'frontend_tests/node_tests/upload.js',
'static/js/upload.js',
'static/js/stream_color.js',
},
'good_lines': ['#my-style {color: blue;}'],
'bad_lines':
['<p style="color: blue;">Foo</p>', 'style = "color: blue;"']
},
*whitespace_rules,
*comma_whitespace_rule,
],
)
js_rules = RuleList(
langs=["js"],
rules=[
{
"pattern": "subject|SUBJECT",
"exclude": {"static/js/util.js", "frontend_tests/"},
"exclude_pattern": "emails",
"description": "avoid subject in JS code",
"good_lines": ["topic_name"],
"bad_lines": ['subject="foo"', " MAX_SUBJECT_LEN"],
},
{
"pattern":
"msgid|MSGID",
"description":
'Avoid using "msgid" as a variable name; use "message_id" instead.',
},
{
"pattern": r"\$t\(.+\).*\+",
"description": "Do not concatenate i18n strings",
},
{
"pattern": r"\+.*\$t\(.+\)",
"description": "Do not concatenate i18n strings"
},
{
"pattern":
"[.]html[(]",
"exclude_pattern":
r"""\.html\(("|'|render_|html|message\.content|util\.clean_user_content_links|rendered_|$|\)|error_html|widget_elem|\$error|\$\("<p>"\))""",
"exclude": {
"static/js/portico",
"static/js/lightbox.js",
"static/js/ui_report.js",
"static/js/confirm_dialog.js",
"frontend_tests/",
},
"description":
"Setting HTML content with jQuery .html() can lead to XSS security bugs. Consider .text() or using rendered_foo as a variable name if content comes from handlebars and thus is already sanitized.",
},
{
"pattern": "[\"']json/",
"description": "Relative URL for JSON route not supported by i18n",
},
{
"pattern": r"""[.]text\(["'][a-zA-Z]""",
"description":
"Strings passed to $().text should be wrapped in $t() for internationalization",
"exclude": {"frontend_tests/node_tests/"},
},
{
"pattern":
r"""compose_error\(["']""",
"description":
"Argument to compose_error should be a literal string translated "
"by $t_html()",
},
{
"pattern": r"ui.report_success\(",
"description": "Deprecated function, use ui_report.success.",
},
{
"pattern":
r"""report.success\(["']""",
"description":
"Argument to ui_report.success should be a literal string translated "
"by $t_html()",
},
{
"pattern": r"ui.report_error\(",
"description": "Deprecated function, use ui_report.error.",
},
{
"pattern":
r"""report.error\(["'][^'"]""",
"description":
"Argument to ui_report.error should be a literal string translated "
"by $t_html()",
"good_lines":
['ui_report.error("")', 'ui_report.error(_("text"))'],
"bad_lines": ['ui_report.error("test")'],
},
{
"pattern":
r"""report.client_error\(["'][^'"]""",
"description":
"Argument to ui_report.client_error should be a literal string translated "
"by $t_html()",
"good_lines": [
'ui_report.client_error("")',
'ui_report.client_error(_("text"))'
],
"bad_lines": ['ui_report.client_error("test")'],
},
{
"pattern": r"\$\(document\)\.ready\(",
"description": "`Use $(f) rather than `$(document).ready(f)`",
"good_lines": ["$(function () {foo();}"],
"bad_lines": ["$(document).ready(function () {foo();}"],
},
{
"pattern": "[$][.](get|post|patch|delete|ajax)[(]",
"description": "Use channel module for AJAX calls",
"exclude": {
# Internal modules can do direct network calls
"static/js/blueslip.js",
"static/js/channel.js",
# External modules that don't include channel.js
"static/js/stats/",
"static/js/portico/",
"static/js/billing/",
},
"good_lines": ["channel.get(...)"],
"bad_lines": ["$.get()", "$.post()", "$.ajax()"],
},
{
"pattern":
"style ?=",
"description":
"Avoid using the `style=` attribute; we prefer styling in CSS files",
"exclude": {
"frontend_tests/node_tests/copy_and_paste.js",
"frontend_tests/node_tests/upload.js",
"static/js/upload.js",
"static/js/stream_color.js",
},
"good_lines": ["#my-style {color: blue;}"],
"bad_lines":
['<p style="color: blue;">Foo</p>', 'style = "color: blue;"'],
},
*whitespace_rules,
],
)
python_rules = RuleList(
langs=['py'],
rules=[
{
'pattern': r'".*"%\([a-z_].*\)?$',
'description': 'Missing space around "%"'
},
{
'pattern': r"'.*'%\([a-z_].*\)?$",
'description': 'Missing space around "%"'
},
# This rule is constructed with + to avoid triggering on itself
{
'pattern': r" =" + r'[^ =>~"]',
'description': 'Missing whitespace after "="'
},
{
'pattern': r'":\w[^"]*$',
'description': 'Missing whitespace after ":"'
},
{
'pattern': r"':\w[^']*$",
'description': 'Missing whitespace after ":"'
},
{
'pattern': r"^\s+[#]\w",
'strip': '\n',
'description': 'Missing whitespace after "#"'
},
{
'pattern':
r"assertEquals[(]",
'description':
'Use assertEqual, not assertEquals (which is deprecated).'
},
{
'pattern': r'self: Any',
'description': 'you can omit Any annotation for self',
'good_lines': ['def foo (self):'],
'bad_lines': ['def foo(self: Any):']
},
{
'pattern': r"== None",
'description': 'Use `is None` to check whether something is None'
},
{
'pattern': r"type:[(]",
'description': 'Missing whitespace after ":" in type annotation'
},
{
'pattern': r"# type [(]",
'description': 'Missing : after type in type annotation'
},
{
'pattern': r"#type",
'description': 'Missing whitespace after "#" in type annotation'
},
{
'pattern': r'if[(]',
'description': 'Missing space between if and ('
},
{
'pattern': r", [)]",
'description': 'Unnecessary whitespace between "," and ")"'
},
{
'pattern': r"% [(]",
'description': 'Unnecessary whitespace between "%" and "("'
},
# This next check could have false positives, but it seems pretty
# rare; if we find any, they can be added to the exclude list for
# this rule.
{
'pattern': r' % [a-zA-Z0-9_.]*\)?$',
'description': 'Used % comprehension without a tuple'
},
{
'pattern': r'.*%s.* % \([a-zA-Z0-9_.]*\)$',
'description': 'Used % comprehension without a tuple'
},
{
'pattern': r'__future__',
'include_only': {'zulip_bots/zulip_bots/bots/'},
'description': 'Bots no longer need __future__ imports.'
},
{
'pattern': r'#!/usr/bin/env python$',
'include_only': {'zulip_bots/'},
'description': 'Python shebangs must be python3'
},
{
'pattern':
r'(^|\s)open\s*\(',
'description':
'open() should not be used in Zulip\'s bots. Use functions'
' provided by the bots framework to access the filesystem.',
'include_only': {'zulip_bots/zulip_bots/bots/'}
},
{
'pattern':
r'pprint',
'description':
'Used pprint, which is most likely a debugging leftover. For user output, use print().'
},
{
'pattern':
r'\(BotTestCase\)',
'bad_lines': ['class TestSomeBot(BotTestCase):'],
'description':
'Bot test cases should directly inherit from BotTestCase *and* DefaultTests.'
},
{
'pattern':
r'\(DefaultTests, BotTestCase\)',
'bad_lines': ['class TestSomeBot(DefaultTests, BotTestCase):'],
'good_lines': ['class TestSomeBot(BotTestCase, DefaultTests):'],
'description':
'Bot test cases should inherit from BotTestCase before DefaultTests.'
},
*whitespace_rules,
],
max_length=140,
)