mirror of
https://github.com/zulip/zulip.git
synced 2025-11-04 14:03:30 +00:00
729c09074a
As seen in https://chat.zulip.org/#narrow/stream/9-issues/topic/edit.20history.20bug/near/1320430, clicking such a link takes you to the user's default view if the click handler throws an exception before doing preventDefault(). There hrefs also have the negative effect of having your browser claim that clicking the link will navigate you to the default view, which it won't. Comes with a linter rule to prevent future instances, since it seems there are some recently added ones, though they are likely the result of copy/paste.
955 lines
38 KiB
Python
955 lines
38 KiB
Python
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([<path>, ...])' - if <path> is a filename, excludes that file.
|
|
# if <path> is a directory, excludes all files directly below the directory <path>.
|
|
# 'exclude_line': 'set([(<path>, <line>), ...])' - excludes all lines matching <line> in the file <path> from linting.
|
|
# 'include_only': 'set([<path>, ...])' - includes only those files where <path> 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"((?<!\s)\s$)|(\s\s\s+$)|(^\s+$)",
|
|
"strip": "\n",
|
|
"description": "Fix trailing whitespace",
|
|
},
|
|
{
|
|
"pattern": "^#+[A-Za-z0-9]",
|
|
"strip": "\n",
|
|
"description": "Missing space after # in heading",
|
|
"exclude_line": {
|
|
("docs/subsystems/hotspots.md", "#hotspot_new_hotspot_name_icon {"),
|
|
},
|
|
"good_lines": ["### some heading", "# another heading"],
|
|
"bad_lines": ["###some heading", "#another heading"],
|
|
},
|
|
]
|
|
|
|
|
|
js_rules = RuleList(
|
|
langs=["js", "ts"],
|
|
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.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": ['<p style="color: blue;">Foo</p>', '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"],
|
|
},
|
|
# 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": r"exit[(][1-9]\d*[)]",
|
|
"include_only": {"/management/commands/"},
|
|
"description": "Raise CommandError to exit with failure in management commands",
|
|
"exclude": {"zerver/management/commands/process_queue.py"},
|
|
},
|
|
{
|
|
"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_"',
|
|
},
|
|
{
|
|
"pattern": "@transaction.atomic\\(\\)",
|
|
"description": "Use @transaction.atomic as function decorator for consistency.",
|
|
},
|
|
*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", "templates/corporate/policies/privacy.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-business.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": [
|
|
'<input class="stream-list-filter" type="text" placeholder="{{ _(\'Filter streams\') }}" />'
|
|
],
|
|
"bad_lines": ['<input placeholder="foo">'],
|
|
},
|
|
{
|
|
"pattern": "={",
|
|
"description": "Likely missing quoting in HTML attribute",
|
|
"good_lines": ['<a href="{{variable}}">'],
|
|
"bad_lines": ["<a href={{variable}}>"],
|
|
},
|
|
{
|
|
"pattern": " '}}",
|
|
"description": "Likely misplaced quoting in translation tags",
|
|
"good_lines": ["{{t 'translatable string' }}"],
|
|
"bad_lines": ["{{t 'translatable string '}}"],
|
|
},
|
|
{
|
|
"pattern": "placeholder='[^{]",
|
|
"description": "`placeholder` value should be translatable.",
|
|
"good_lines": [
|
|
'<input class="stream-list-filter" type="text" placeholder="{{ _(\'Filter streams\') }}" />'
|
|
],
|
|
"bad_lines": ["<input placeholder='foo'>"],
|
|
},
|
|
{
|
|
"pattern": "aria-label='[^{]",
|
|
"description": "`aria-label` value should be translatable.",
|
|
"good_lines": [
|
|
'<button type="button" class="close close-alert-word-status" aria-label="{{t \'Close\' }}">'
|
|
],
|
|
"bad_lines": ["<button aria-label='foo'></button>"],
|
|
},
|
|
{
|
|
"pattern": 'aria-label="[^{]',
|
|
"description": "`aria-label` value should be translatable.",
|
|
"good_lines": [
|
|
'<button type="button" class="close close-alert-word-status" aria-label="{{t \'Close\' }}">'
|
|
],
|
|
"bad_lines": ['<button aria-label="foo"></button>'],
|
|
},
|
|
{
|
|
"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",
|
|
"templates/corporate/event_status.html",
|
|
},
|
|
"bad_lines": [
|
|
'<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>'
|
|
],
|
|
},
|
|
{
|
|
"pattern": "title='[^{]",
|
|
"description": "`title` value should be translatable.",
|
|
"good_lines": ['<link rel="author" title="{{ _(\'About these documents\') }}" />'],
|
|
"bad_lines": ["<p title='foo'></p>"],
|
|
},
|
|
{
|
|
"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",
|
|
'<img alt=":thumbs_up:"',
|
|
),
|
|
},
|
|
"good_lines": ['<img src="{{source_url}}" alt="{{ _(name) }}" />', '<img alg="" />'],
|
|
"bad_lines": ['<img alt="Foo Image" />'],
|
|
},
|
|
{
|
|
"pattern": r"""\Walt=["']{{ ?["']""",
|
|
"description": "alt argument should be enclosed by _().",
|
|
"good_lines": ['<img src="{{source_url}}" alt="{{ _(name) }}" />'],
|
|
"bad_lines": ['<img alt="{{ " />'],
|
|
},
|
|
{
|
|
"pattern": r"\bon\w+ ?=",
|
|
"description": "Don't use inline event handlers (onclick=, etc. attributes) in HTML. Instead,"
|
|
"attach a jQuery event handler ($('#foo').on('click', function () {...})) when "
|
|
"the DOM is ready (inside a $(function () {...}) block).",
|
|
"exclude": {
|
|
"templates/zerver/development/dev_login.html",
|
|
"templates/corporate/upgrade.html",
|
|
},
|
|
"good_lines": ["($('#foo').on('click', function () {}"],
|
|
"bad_lines": [
|
|
"<button id='foo' onclick='myFunction()'>Foo</button>",
|
|
"<input onchange='myFunction()'>",
|
|
],
|
|
},
|
|
{
|
|
"pattern": "style ?=",
|
|
"description": "Avoid using the `style=` attribute; we prefer styling in CSS files",
|
|
"exclude_pattern": r'.*style ?=["'
|
|
+ "'"
|
|
+ "](display: ?none|background: {{|color: {{|background-color: {{).*",
|
|
"exclude": {
|
|
# 5xx page doesn't have external CSS
|
|
"static/html/5xx.html",
|
|
# exclude_pattern above handles color, but have other issues:
|
|
"static/templates/draft.hbs",
|
|
"static/templates/stream_settings/browse_streams_list_item.hbs",
|
|
"static/templates/single_message.hbs",
|
|
# Old-style email templates need to use inline style
|
|
# attributes; it should be possible to clean these up
|
|
# when we convert these templates to use premailer.
|
|
"templates/zerver/emails/email_base_messages.html",
|
|
# Email log templates; should clean up.
|
|
"templates/zerver/email.html",
|
|
"templates/zerver/development/email_log.html",
|
|
# Social backend logos are dynamically loaded
|
|
"templates/zerver/accounts_home.html",
|
|
"templates/zerver/login.html",
|
|
# Needs the width cleaned up; display: none is fine
|
|
"static/templates/dialog_change_password.hbs",
|
|
# background image property is dynamically generated
|
|
"static/templates/user_profile_modal.hbs",
|
|
"static/templates/pm_list_item.hbs",
|
|
# Inline styling for an svg; could be moved to CSS files?
|
|
"templates/zerver/landing_nav.html",
|
|
"templates/zerver/billing_nav.html",
|
|
"templates/zerver/features.html",
|
|
"templates/zerver/portico-header.html",
|
|
"templates/corporate/billing.html",
|
|
"templates/corporate/upgrade.html",
|
|
# Miscellaneous violations to be cleaned up
|
|
"static/templates/user_info_popover_title.hbs",
|
|
"static/templates/confirm_dialog/confirm_subscription_invites_warning.hbs",
|
|
"templates/zerver/reset_confirm.html",
|
|
"templates/zerver/config_error.html",
|
|
"templates/zerver/dev_env_email_access_details.html",
|
|
"templates/zerver/confirm_continue_registration.html",
|
|
"templates/zerver/register.html",
|
|
"templates/zerver/accounts_send_confirm.html",
|
|
"templates/zerver/integrations/index.html",
|
|
"templates/zerver/documentation_main.html",
|
|
"templates/analytics/realm_summary_table.html",
|
|
"templates/corporate/zephyr.html",
|
|
"templates/corporate/zephyr-mirror.html",
|
|
},
|
|
"good_lines": ["#my-style {color: blue;}", 'style="display: none"', "style='display: none"],
|
|
"bad_lines": ['<p style="color: blue;">Foo</p>', 'style = "color: blue;"'],
|
|
},
|
|
]
|
|
|
|
handlebars_rules = RuleList(
|
|
langs=["hbs"],
|
|
rules=[
|
|
*html_rules,
|
|
{
|
|
"pattern": "[<]script",
|
|
"description": "Do not use inline <script> tags here; put JavaScript in static/js instead.",
|
|
},
|
|
{
|
|
"pattern": "{{ t (\"|')",
|
|
"description": 'There should be no spaces before the "t" in a translation tag.',
|
|
},
|
|
{
|
|
"pattern": r"{{t '.*' }}[\.\?!]",
|
|
"description": "Period should be part of the translatable string.",
|
|
},
|
|
{
|
|
"pattern": r'{{t ".*" }}[\.\?!]',
|
|
"description": "Period should be part of the translatable string.",
|
|
},
|
|
{
|
|
"pattern": r"{{/tr}}[\.\?!]",
|
|
"description": "Period should be part of the translatable string.",
|
|
},
|
|
{
|
|
"pattern": "{{t (\"|') ",
|
|
"description": "Translatable strings should not have leading spaces.",
|
|
},
|
|
{
|
|
"pattern": "{{t '[^']+ ' }}",
|
|
"description": "Translatable strings should not have trailing spaces.",
|
|
},
|
|
{
|
|
"pattern": '{{t "[^"]+ " }}',
|
|
"description": "Translatable strings should not have trailing spaces.",
|
|
},
|
|
{
|
|
"pattern": r'"{{t "',
|
|
"description": "Invalid quoting for HTML element with translated string.",
|
|
},
|
|
{
|
|
"pattern": r'href="#"',
|
|
"description": 'Avoid href="#" for elements with a JavaScript click handler.',
|
|
"exclude": {"static/templates/navbar.hbs"},
|
|
},
|
|
],
|
|
)
|
|
|
|
jinja2_rules = RuleList(
|
|
langs=["html"],
|
|
rules=[
|
|
*html_rules,
|
|
{
|
|
"pattern": r"{% endtrans %}[\.\?!]",
|
|
"description": "Period should be part of the translatable string.",
|
|
},
|
|
{
|
|
"pattern": r"{{ _(.+) }}[\.\?!]",
|
|
"description": "Period should be part of the translatable string.",
|
|
},
|
|
{
|
|
"pattern": r'{% set entrypoint = "dev-',
|
|
"exclude": {"templates/zerver/development/"},
|
|
"description": "Development entry points (dev-) must not be imported in production.",
|
|
},
|
|
],
|
|
)
|
|
|
|
json_rules = RuleList(
|
|
langs=["json"],
|
|
rules=[
|
|
# Here, we don't use `whitespace_rules`, because the tab-based
|
|
# whitespace rule flags a lot of third-party JSON fixtures
|
|
# under zerver/webhooks that we want preserved verbatim. So
|
|
# we just include the trailing whitespace rule and a modified
|
|
# version of the tab-based whitespace rule (we can't just use
|
|
# exclude in whitespace_rules, since we only want to ignore
|
|
# JSON files with tab-based whitespace, not webhook code).
|
|
trailing_whitespace_rule,
|
|
{
|
|
"pattern": "\t",
|
|
"strip": "\n",
|
|
"exclude": {"zerver/webhooks/"},
|
|
"description": "Fix tab-based whitespace",
|
|
},
|
|
{
|
|
"pattern": r'":["\[\{]',
|
|
"exclude": {"zerver/webhooks/", "zerver/tests/fixtures/"},
|
|
"description": "Require space after : in JSON",
|
|
},
|
|
],
|
|
)
|
|
|
|
markdown_docs_length_exclude = {
|
|
# Has some example Vagrant output that's very long
|
|
"docs/development/setup-vagrant.md",
|
|
# Have wide output in code blocks
|
|
"docs/subsystems/logging.md",
|
|
"docs/subsystems/schema-migrations.md",
|
|
# Have curl commands with JSON that would be messy to wrap
|
|
"zerver/webhooks/helloworld/doc.md",
|
|
"zerver/webhooks/trello/doc.md",
|
|
# Has a very long configuration line
|
|
"templates/zerver/integrations/perforce.md",
|
|
# Has some example code that could perhaps be wrapped
|
|
"templates/zerver/api/incoming-webhooks-walkthrough.md",
|
|
"templates/zerver/api/get-messages.md",
|
|
# This macro has a long indented URL
|
|
"templates/zerver/help/include/git-webhook-url-with-branches-indented.md",
|
|
# These two are the same file and have some too-long lines for GitHub badges
|
|
"README.md",
|
|
"docs/overview/readme.md",
|
|
}
|
|
|
|
markdown_rules = RuleList(
|
|
langs=["md"],
|
|
rules=[
|
|
*markdown_whitespace_rules,
|
|
*prose_style_rules,
|
|
{
|
|
"pattern": r"\[(?P<url>[^\]]+)\]\((?P=url)\)",
|
|
"description": "Linkified Markdown URLs should use cleaner <http://example.com> syntax.",
|
|
},
|
|
{
|
|
"pattern": "https://zulip.readthedocs.io/en/latest/[a-zA-Z0-9]",
|
|
"exclude": {
|
|
"docs/overview/contributing.md",
|
|
"docs/overview/readme.md",
|
|
"docs/README.md",
|
|
"docs/subsystems/email.md",
|
|
},
|
|
"exclude_line": {
|
|
(
|
|
"docs/overview/changelog.md",
|
|
"[latest-changelog]: https://zulip.readthedocs.io/en/latest/overview/changelog.html",
|
|
),
|
|
},
|
|
"include_only": {"docs/"},
|
|
"description": "Use relative links (../foo/bar.html) to other documents in docs/",
|
|
},
|
|
{
|
|
"pattern": "su zulip -c [^']",
|
|
"include_only": {"docs/"},
|
|
"description": "Always quote arguments using `su zulip -c '` to avoid confusion about how su works.",
|
|
},
|
|
{
|
|
"pattern": r"\][(][^#h]",
|
|
"exclude_pattern": "mailto:",
|
|
"include_only": {"README.md", "CONTRIBUTING.md"},
|
|
"description": "Use absolute links from docs served by GitHub",
|
|
},
|
|
{
|
|
"pattern": r"\.(py|js)#L\d+",
|
|
"include_only": {"docs/"},
|
|
"description": "Don't link directly to line numbers",
|
|
},
|
|
],
|
|
max_length=120,
|
|
length_exclude=markdown_docs_length_exclude,
|
|
exclude_files_in="templates/zerver/help/",
|
|
)
|
|
|
|
help_markdown_rules = RuleList(
|
|
langs=["md"],
|
|
rules=[
|
|
*markdown_rules.rules,
|
|
{
|
|
"pattern": "[a-z][.][A-Z]",
|
|
"description": "Likely missing space after end of sentence",
|
|
"include_only": {"templates/zerver/help/"},
|
|
"exclude_pattern": "Rocket.Chat",
|
|
},
|
|
{
|
|
"pattern": r"\b[rR]ealm[s]?\b",
|
|
"include_only": {"templates/zerver/help/"},
|
|
"exclude": {"templates/zerver/help/change-organization-url.md"},
|
|
"good_lines": ["Organization", "deactivate_realm", "realm_filter"],
|
|
"bad_lines": ["Users are in a realm", "Realm is the best model"],
|
|
"description": "Realms are referred to as Organizations in user-facing docs.",
|
|
# Keycloak uses the term realm as well.
|
|
# Additionally, we allow -realm- as that appears in /api/ doc URLs.
|
|
"exclude_pattern": "(-realm-|[kK]eycloak)",
|
|
},
|
|
],
|
|
length_exclude=markdown_docs_length_exclude,
|
|
)
|
|
|
|
puppet_rules = RuleList(
|
|
langs=["pp"],
|
|
rules=[
|
|
{
|
|
"pattern": r"(include\s+|\$)zulip::(profile|base)\b",
|
|
"exclude": {
|
|
"puppet/zulip/manifests/profile/",
|
|
"puppet/zulip_ops/manifests/",
|
|
"puppet/zulip/manifests/dockervoyager.pp",
|
|
},
|
|
"description": "Abstraction layering violation; only profiles should reference profiles or zulip::base",
|
|
},
|
|
{
|
|
"pattern": r"(include\s+|\$)zulip_ops::(profile|base)\b",
|
|
"exclude": {
|
|
"puppet/zulip/manifests/",
|
|
"puppet/zulip_ops/manifests/profile/",
|
|
},
|
|
"description": "Abstraction layering violation; only profiles should reference profiles or zulip_ops::base",
|
|
},
|
|
],
|
|
)
|
|
|
|
txt_rules = RuleList(
|
|
langs=["txt", "text", "yaml", "rst", "yml"],
|
|
rules=whitespace_rules,
|
|
)
|
|
non_py_rules = [
|
|
handlebars_rules,
|
|
jinja2_rules,
|
|
css_rules,
|
|
js_rules,
|
|
json_rules,
|
|
markdown_rules,
|
|
help_markdown_rules,
|
|
bash_rules,
|
|
txt_rules,
|
|
puppet_rules,
|
|
]
|