from typing import List from zulint.custom_rules import Rule, RuleList # Rule help: # By default, a rule applies to all files within the extension for which it is specified (e.g. all .py files) # There are three operators we can use to manually include or exclude files from linting for a rule: # 'exclude': 'set([, ...])' - if is a filename, excludes that file. # if is a directory, excludes all files directly below the directory . # 'exclude_line': 'set([(, ), ...])' - excludes all lines matching in the file from linting. # 'include_only': 'set([, ...])' - includes only those files where is a substring of the filepath. FILES_WITH_LEGACY_SUBJECT = { # This basically requires a big DB migration: "zerver/lib/topic.py", # This is for backward compatibility. "zerver/tests/test_legacy_subject.py", # Other migration-related changes require extreme care. "zerver/lib/fix_unreads.py", "zerver/tests/test_migrations.py", # These use subject in the email sense, and will # probably always be exempt: "zerver/lib/email_mirror.py", "zerver/lib/send_email.py", "zerver/tests/test_new_users.py", "zerver/tests/test_email_mirror.py", "zerver/tests/test_email_notifications.py", # This uses subject in authentication protocols sense: "zerver/tests/test_auth_backends.py", # These are tied more to our API than our DB model. "zerver/openapi/python_examples.py", "zerver/tests/test_openapi.py", # This has lots of query data embedded, so it's hard # to fix everything until we migrate the DB to "topic". "zerver/tests/test_message_fetch.py", } shebang_rules: List["Rule"] = [ { "pattern": "^#!", "description": "zerver library code shouldn't have a shebang line.", "include_only": {"zerver/"}, }, # /bin/sh and /usr/bin/env are the only two binaries # that NixOS provides at a fixed path (outside a # buildFHSUserEnv sandbox). { "pattern": "^#!(?! *(?:/usr/bin/env|/bin/sh)(?: |$))", "description": "Use `#!/usr/bin/env foo` instead of `#!/path/foo`" " for interpreters other than sh.", }, { "pattern": "^#!/usr/bin/env python$", "description": "Use `#!/usr/bin/env python3` instead of `#!/usr/bin/env python`.", }, ] trailing_whitespace_rule: "Rule" = { "pattern": r"\s+$", "strip": "\n", "exclude": {"tools/ci/success-http-headers.template.txt"}, "description": "Fix trailing whitespace", } whitespace_rules: List["Rule"] = [ # This linter should be first since bash_rules depends on it. trailing_whitespace_rule, { "pattern": "http://zulip.readthedocs.io", "description": "Use HTTPS when linking to ReadTheDocs", }, { "pattern": "\t", "strip": "\n", "description": "Fix tab-based whitespace", }, ] comma_whitespace_rule: List["Rule"] = [ { "pattern": ", {2,}[^#/ ]", "exclude": {"zerver/tests", "frontend_tests/node_tests", "corporate/tests"}, "description": "Remove multiple whitespaces after ','", "good_lines": ["foo(1, 2, 3)", "foo = bar # some inline comment"], "bad_lines": ["foo(1, 2, 3)", "foo(1, 2, 3)"], }, ] markdown_whitespace_rules: List["Rule"] = [ *(rule for rule in whitespace_rules if rule["pattern"] != r"\s+$"), # Two spaces trailing a line with other content is okay--it's a Markdown line break. # This rule finds one space trailing a non-space, three or more trailing spaces, and # spaces on an empty line. { "pattern": r"((?"\))""", "exclude": { "static/js/portico", "static/js/lightbox.js", "static/js/ui_report.ts", "static/js/dialog_widget.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.ts", "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 ?=", "exclude_pattern": r"(const |\S)style ?=", "description": "Avoid using the `style=` attribute; we prefer styling in CSS files", "exclude": { "frontend_tests/node_tests/copy_and_paste.js", }, "good_lines": ["#my-style {color: blue;}", "const style =", 'some_style = "test"'], "bad_lines": ['

Foo

', 'style = "color: blue;"'], }, { "pattern": r"assert\(", "description": "Use 'assert.ok' instead of 'assert'. We avoid the use of 'assert' as it can easily be confused with 'assert.equal'.", "good_lines": ["assert.ok(...)"], "bad_lines": ["assert(...)"], }, *whitespace_rules, ], ) python_rules = RuleList( langs=["py"], rules=[ { "pattern": "subject|SUBJECT", "exclude_pattern": "subject to the|email|outbox", "description": "avoid subject as a var", "good_lines": ["topic_name"], "bad_lines": ['subject="foo"', " MAX_SUBJECT_LEN"], "exclude": FILES_WITH_LEGACY_SUBJECT, "include_only": { "zerver/data_import/", "zerver/lib/", "zerver/tests/", "zerver/views/", }, }, { "pattern": "msgid|MSGID", "exclude": {"tools/check-capitalization"}, "description": 'Avoid using "msgid" as a variable name; use "message_id" instead.', }, { "pattern": "^(?!#)@login_required", "description": "@login_required is unsupported; use @zulip_login_required", "good_lines": ["@zulip_login_required", "# foo @login_required"], "bad_lines": ["@login_required", " @login_required"], }, { "pattern": "^user_profile[.]save[(][)]", "description": "Always pass update_fields when saving user_profile objects", "exclude_line": { ( "zerver/lib/actions.py", "user_profile.save() # Can't use update_fields because of how the foreign key works.", ), }, "exclude": {"zerver/tests", "zerver/lib/create_user.py"}, "good_lines": ['user_profile.save(update_fields=["pointer"])'], "bad_lines": ["user_profile.save()"], }, { "pattern": "self: Any", "description": "you can omit Any annotation for self", "good_lines": ["def foo (self):"], "bad_lines": ["def foo(self: Any):"], }, { "pattern": "assertEquals[(]", "description": "Use assertEqual, not assertEquals (which is deprecated).", "good_lines": ["assertEqual(1, 2)"], "bad_lines": ["assertEquals(1, 2)"], }, { "pattern": "assertEqual[(]len[(][^ ]*[)],", "description": "Use the assert_length helper instead of assertEqual(len(..), ..).", "good_lines": ["assert_length(data, 2)"], "bad_lines": ["assertEqual(len(data), 2)"], }, { "pattern": "assertTrue[(]len[(][^ ]*[)]", "description": "Use assert_length or assertGreater helper instead of assertTrue(len(..) ..).", "good_lines": ["assert_length(data, 2)", "assertGreater(len(data), 2)"], "bad_lines": [ "assertTrue(len(data) == 2)", "assertTrue(len(data) >= 2)", "assertTrue(len(data) > 2)", ], }, { "pattern": r"#\s*type:\s*ignore(?!\[[^][]+\] +# +\S)", "exclude": {"tools/tests", "zerver/lib/test_runner.py", "zerver/tests"}, "description": '"type: ignore" should always end with "# type: ignore[code] # explanation for why"', "good_lines": ["foo = bar # type: ignore[code] # explanation"], "bad_lines": [ "foo = bar # type: ignore", "foo = bar # type: ignore[code]", "foo = bar # type: ignore # explanation", ], }, { "pattern": r"\bsudo\b", "include_only": {"scripts/"}, "exclude": {"scripts/lib/setup_venv.py"}, "exclude_line": { ("scripts/lib/zulip_tools.py", 'args = ["sudo", *sudo_args, "--", *args]'), }, "description": "Most scripts are intended to run on systems without sudo.", "good_lines": ['subprocess.check_call(["ls"])'], "bad_lines": ['subprocess.check_call(["sudo", "ls"])'], }, { "pattern": "django.utils.translation", "include_only": {"test/", "zerver/views/development/"}, "exclude": {"zerver/views/development/dev_login.py"}, "description": "Test strings should not be tagged for translation", "good_lines": [""], "bad_lines": ["django.utils.translation"], }, { "pattern": "userid", "description": "We prefer user_id over userid.", "good_lines": ["id = alice.user_id"], "bad_lines": ["id = alice.userid"], }, { "pattern": r"json_success${}$", "description": "Use json_success() to return nothing", "good_lines": ["return json_success()"], "bad_lines": ["return json_success({})"], }, # To avoid JsonableError(_variable) and JsonableError(_(variable)) { "pattern": r"\WJsonableError$_\(?\w.+$", "exclude": {"zerver/tests", "zerver/views/development/"}, "description": "Argument to JsonableError should be a literal string enclosed by _()", }, { "pattern": r"""\WJsonableError$["'].+$""", "exclude": {"zerver/tests", "zerver/views/development/"}, "description": "Argument to JsonableError should be a literal string enclosed by _()", }, { "pattern": r"""([a-zA-Z0-9_]+)=REQ\(['"]\1['"]""", "description": "REQ's first argument already defaults to parameter name", }, { "pattern": r"self\.client\.(get|post|patch|put|delete)", "description": """Do not call self.client directly for put/patch/post/get. See WRAPPER_COMMENT in test_helpers.py for details. """, }, # Directly fetching Message objects in e.g. views code is often a security bug. { "pattern": "[^r]Message.objects.get", "exclude": { "zerver/tests", "zerver/lib/onboarding.py", "zilencer/management/commands/add_mock_conversation.py", "zerver/worker/queue_processors.py", "zerver/management/commands/export.py", "zerver/lib/export.py", }, "description": "Please use access_message() to fetch Message objects", }, { "pattern": "Stream.objects.get", "include_only": {"zerver/views/"}, "description": "Please use access_stream_by_*() to fetch Stream objects", }, { "pattern": "get_stream[(]", "include_only": {"zerver/views/", "zerver/lib/actions.py"}, "exclude_line": { # This one in check_message is kinda terrible, since it's # how most instances are written, but better to exclude something than nothing ("zerver/lib/actions.py", "stream = get_stream(stream_name, realm)"), ("zerver/lib/actions.py", 'return get_stream("signups", realm)'), }, "description": "Please use access_stream_by_*() to fetch Stream objects", }, { "pattern": "datetime[.](now|utcnow)", "include_only": {"zerver/", "analytics/"}, "description": "Don't use datetime in backend code.\n" "See https://zulip.readthedocs.io/en/latest/contributing/code-style.html#naive-datetime-objects", }, { "pattern": "from os.path", "description": "Don't use from when importing from the standard library", }, { "pattern": "import os.path", "description": "Use import os instead of import os.path", }, { "pattern": r"(logging|logger)\.warn\W", "description": "Logger.warn is a deprecated alias for Logger.warning; Use 'warning' instead of 'warn'.", "good_lines": ["logging.warning('I am a warning.')", "logger.warning('warning')"], "bad_lines": ["logging.warn('I am a warning.')", "logger.warn('warning')"], }, { "pattern": r"\.pk", "exclude_pattern": "[.]_meta[.]pk", "description": "Use `id` instead of `pk`.", "good_lines": ["if my_django_model.id == 42", "self.user_profile._meta.pk"], "bad_lines": ["if my_django_model.pk == 42"], }, { "pattern": r"^\s*#\s*type:", "description": "Comment-style function type annotation. Use Python3 style annotations instead.", }, { "pattern": r"\S\s*#\s*type:(?!\s*ignore)", "exclude": { "scripts/lib/hash_reqs.py", "scripts/lib/setup_venv.py", "scripts/lib/zulip_tools.py", "tools/lib/provision.py", }, "description": "Comment-style variable type annotation. Use Python 3.6 style annotations instead.", "good_lines": ["a: List[int] = []"], "bad_lines": ["a = [] # type: List[int]"], }, { "pattern": r": *(?!Optional)[^ ].*= models[.].*null=True", "include_only": {"zerver/models.py"}, "description": "Model variable with null=true not annotated as Optional.", "good_lines": [ "desc: Optional[Text] = models.TextField(null=True)", "stream: Optional[Stream] = models.ForeignKey(Stream, null=True, on_delete=CASCADE)", "desc: Text = models.TextField()", "stream: Stream = models.ForeignKey(Stream, on_delete=CASCADE)", ], "bad_lines": [ "desc: Text = models.CharField(null=True)", "stream: Stream = models.ForeignKey(Stream, null=True, on_delete=CASCADE)", ], }, { "pattern": r": *Optional.*= models[.].*\)", "exclude_pattern": "null=True", "include_only": {"zerver/models.py"}, "description": "Model variable annotated with Optional but variable does not have null=true.", "good_lines": [ "desc: Optional[Text] = models.TextField(null=True)", "stream: Optional[Stream] = models.ForeignKey(Stream, null=True, on_delete=CASCADE)", "desc: Text = models.TextField()", "stream: Stream = models.ForeignKey(Stream, on_delete=CASCADE)", ], "bad_lines": [ "desc: Optional[Text] = models.TextField()", "stream: Optional[Stream] = models.ForeignKey(Stream, on_delete=CASCADE)", ], }, { "pattern": "exit[(]1[)]", "include_only": {"/management/commands/"}, "description": "Raise CommandError to exit with failure in management commands", }, { "pattern": ".is_realm_admin =", "description": "Use do_change_user_role function rather than setting UserProfile's is_realm_admin attribute directly.", "exclude": { "zerver/migrations/0248_userprofile_role_start.py", "zerver/tests/test_users.py", }, }, { "pattern": ".is_guest =", "description": "Use do_change_user_role function rather than setting UserProfile's is_guest attribute directly.", "exclude": { "zerver/migrations/0248_userprofile_role_start.py", "zerver/tests/test_users.py", }, }, { "pattern": "\\.(called(_once|_with|_once_with)?|not_called|has_calls|not_called)[(]", "description": 'A mock function is missing a leading "assert_"', }, *whitespace_rules, ], max_length=110, shebang_rules=shebang_rules, ) bash_rules = RuleList( langs=["bash"], rules=[ { "pattern": "#!.*sh [-xe]", "description": "Fix shebang line with proper call to /usr/bin/env for Bash path, change -x|-e switches" " to set -x|set -e", }, { "pattern": "sudo", "description": "Most scripts are intended to work on systems without sudo", "include_only": {"scripts/"}, "exclude": { "scripts/lib/install", "scripts/setup/configure-rabbitmq", }, }, *whitespace_rules[0:1], ], shebang_rules=shebang_rules, ) css_rules = RuleList( langs=["css"], rules=[ *whitespace_rules, ], ) prose_style_rules: List["Rule"] = [ { "pattern": r'^[^{].*?[^\/\#\-"]([jJ]avascript)', # exclude usage in hrefs/divs/custom-markdown "exclude": {"docs/documentation/api.md"}, "description": "javascript should be spelled JavaScript", }, { "pattern": r"""[^\/\-\."'\_\=\>]([gG]ithub)[^\.\-\_"\<]""", # exclude usage in hrefs/divs "description": "github should be spelled GitHub", }, { "pattern": "[oO]rganisation", # exclude usage in hrefs/divs "description": "Organization is spelled with a z", "exclude_line": {("docs/translating/french.md", "- organization - **organisation**")}, }, {"pattern": "!!! warning", "description": "!!! warning is invalid; it's spelled '!!! warn'"}, {"pattern": "Terms of service", "description": "The S in Terms of Service is capitalized"}, { "pattern": "[^-_p]botserver(?!rc)|bot server", "description": "Use Botserver instead of botserver or bot server.", }, *comma_whitespace_rule, ] html_rules: List["Rule"] = [ *whitespace_rules, *prose_style_rules, { "pattern": "subject|SUBJECT", "exclude": { "templates/zerver/email.html", "zerver/tests/fixtures/email", "templates/zerver/for-companies.html", "templates/corporate/support_request.html", "templates/corporate/support_request_thanks.html", "templates/zerver/emails/support_request.html", }, "exclude_pattern": "email subject", "description": "avoid subject in templates", "good_lines": ["topic_name"], "bad_lines": ['subject="foo"', " MAX_SUBJECT_LEN"], }, { "pattern": r'placeholder="[^{#](?:(?!\.com).)+$', "description": "`placeholder` value should be translatable.", "exclude_line": { ("templates/zerver/register.html", 'placeholder="acme"'), ("templates/zerver/register.html", 'placeholder="Acme or Ακμή"'), }, "exclude": { "templates/analytics/support.html", # We have URL prefix and Pygments language name as placeholders # in the below template which we don't want to be translatable. "static/templates/settings/playground_settings_admin.hbs", }, "good_lines": [ '' ], "bad_lines": [''], }, { "pattern": "={", "description": "Likely missing quoting in HTML attribute", "good_lines": [''], "bad_lines": [""], }, { "pattern": " '}}", "description": "Likely misplaced quoting in translation tags", "good_lines": ["{{t 'translateable string' }}"], "bad_lines": ["{{t 'translateable string '}}"], }, { "pattern": "placeholder='[^{]", "description": "`placeholder` value should be translatable.", "good_lines": [ '' ], "bad_lines": [""], }, { "pattern": "aria-label='[^{]", "description": "`aria-label` value should be translatable.", "good_lines": [ '"], }, { "pattern": 'aria-label="[^{]', "description": "`aria-label` value should be translatable.", "good_lines": [ ''], }, { "pattern": 'script src="http', "description": "Don't directly load dependencies from CDNs. See docs/subsystems/html-css.md", "exclude": { "templates/corporate/billing.html", "templates/zerver/hello.html", "templates/corporate/upgrade.html", }, "bad_lines": [ '' ], }, { "pattern": "title='[^{]", "description": "`title` value should be translatable.", "good_lines": [''], "bad_lines": ["

"], }, { "pattern": r'title="[^{\:]', "exclude": { "templates/zerver/emails", "templates/analytics/realm_details.html", "templates/analytics/support.html", }, "description": "`title` value should be translatable.", }, { "pattern": r"""\Walt=["'][^{"']""", "description": "alt argument should be enclosed by _() or it should be an empty string.", "exclude_line": { ( # Emoji should not be tagged for translation. "static/templates/keyboard_shortcuts.hbs", ' :thumbs_up:

', ''], "bad_lines": [' Foo Image

'], }, { "pattern": r"""\Walt=["']{{ ?["']""", "description": "alt argument should be enclosed by _().", "good_lines": [' ${{ _(name) }}$ '], "bad_lines": ['

Foo

', 'style = "color: blue;"'], }, ] handlebars_rules = RuleList( langs=["hbs"], rules=[ *html_rules, { "pattern": "[<]script", "description": "Do not use inline