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, )