2017-06-23 02:16:10 +02:00
|
|
|
# -*- coding: utf-8 -*-
|
|
|
|
|
2017-06-05 16:29:04 +02:00
|
|
|
from __future__ import print_function
|
|
|
|
from __future__ import absolute_import
|
|
|
|
|
|
|
|
import os
|
|
|
|
import re
|
|
|
|
import traceback
|
|
|
|
|
2017-07-06 06:54:30 +02:00
|
|
|
from .printer import print_err, colors
|
|
|
|
|
2017-08-08 04:13:25 +02:00
|
|
|
from typing import cast, Any, Callable, Dict, List, Optional, Tuple, Iterable
|
2017-06-05 16:29:04 +02:00
|
|
|
|
2017-11-05 00:48:08 +01:00
|
|
|
RuleList = List[Dict[str, Any]]
|
2017-08-04 02:23:09 +02:00
|
|
|
|
2017-09-11 19:56:19 +02:00
|
|
|
def custom_check_file(fn, identifier, rules, color, skip_rules=None, max_length=None):
|
2017-08-08 04:13:25 +02:00
|
|
|
# type: (str, str, RuleList, str, Optional[Iterable[str]], Optional[int]) -> bool
|
2017-09-11 19:56:19 +02:00
|
|
|
failed = False
|
|
|
|
|
|
|
|
line_tups = []
|
|
|
|
for i, line in enumerate(open(fn)):
|
|
|
|
line_newline_stripped = line.strip('\n')
|
|
|
|
line_fully_stripped = line_newline_stripped.strip()
|
|
|
|
skip = False
|
2017-08-08 04:13:25 +02:00
|
|
|
for skip_rule in skip_rules or []:
|
|
|
|
if re.match(skip_rule, line):
|
2017-09-11 19:56:19 +02:00
|
|
|
skip = True
|
|
|
|
if line_fully_stripped.endswith(' # nolint'):
|
|
|
|
continue
|
|
|
|
if skip:
|
|
|
|
continue
|
|
|
|
tup = (i, line, line_newline_stripped, line_fully_stripped)
|
|
|
|
line_tups.append(tup)
|
|
|
|
|
|
|
|
rules_to_apply = []
|
|
|
|
for rule in rules:
|
2017-11-23 20:53:17 +01:00
|
|
|
excluded = False
|
|
|
|
for item in rule.get('exclude', set()):
|
|
|
|
if fn.startswith(item):
|
|
|
|
excluded = True
|
|
|
|
break
|
|
|
|
if excluded:
|
2017-09-11 19:56:19 +02:00
|
|
|
continue
|
|
|
|
if rule.get("include_only"):
|
|
|
|
found = False
|
|
|
|
for item in rule.get("include_only", set()):
|
|
|
|
if item in fn:
|
|
|
|
found = True
|
|
|
|
if not found:
|
2017-06-05 16:29:04 +02:00
|
|
|
continue
|
2017-09-11 19:56:19 +02:00
|
|
|
rules_to_apply.append(rule)
|
|
|
|
|
|
|
|
for rule in rules_to_apply:
|
|
|
|
exclude_lines = {
|
|
|
|
line for
|
|
|
|
(exclude_fn, line) in rule.get('exclude_line', set())
|
|
|
|
if exclude_fn == fn
|
|
|
|
}
|
|
|
|
|
|
|
|
pattern = rule['pattern']
|
2017-06-21 01:17:00 +02:00
|
|
|
for (i, line, line_newline_stripped, line_fully_stripped) in line_tups:
|
2017-09-11 19:56:19 +02:00
|
|
|
if line_fully_stripped in exclude_lines:
|
|
|
|
exclude_lines.remove(line_fully_stripped)
|
|
|
|
continue
|
|
|
|
try:
|
|
|
|
line_to_check = line_fully_stripped
|
|
|
|
if rule.get('strip') is not None:
|
|
|
|
if rule['strip'] == '\n':
|
|
|
|
line_to_check = line_newline_stripped
|
|
|
|
else:
|
|
|
|
raise Exception("Invalid strip rule")
|
|
|
|
if re.search(pattern, line_to_check):
|
2017-10-06 22:07:47 +02:00
|
|
|
if rule.get("exclude_pattern"):
|
|
|
|
if re.search(rule['exclude_pattern'], line_to_check):
|
|
|
|
continue
|
2017-09-11 19:56:19 +02:00
|
|
|
print_err(identifier, color, '{} at {} line {}:'.format(
|
|
|
|
rule['description'], fn, i+1))
|
|
|
|
print_err(identifier, color, line)
|
|
|
|
failed = True
|
|
|
|
except Exception:
|
|
|
|
print("Exception with %s at %s line %s" % (rule['pattern'], fn, i+1))
|
|
|
|
traceback.print_exc()
|
|
|
|
|
|
|
|
if exclude_lines:
|
|
|
|
print('Please remove exclusions for file %s: %s' % (fn, exclude_lines))
|
|
|
|
|
2017-10-12 18:40:45 +02:00
|
|
|
# TODO: Move the below into more of a framework.
|
|
|
|
firstline = None
|
|
|
|
if line_tups:
|
|
|
|
firstline = line_tups[0][3] # line_fully_stripped for the first line.
|
2017-09-11 19:56:19 +02:00
|
|
|
lastLine = None
|
|
|
|
for (i, line, line_newline_stripped, line_fully_stripped) in line_tups:
|
|
|
|
if isinstance(line, bytes):
|
|
|
|
line_length = len(line.decode("utf-8"))
|
|
|
|
else:
|
|
|
|
line_length = len(line)
|
|
|
|
if (max_length is not None and line_length > max_length and
|
|
|
|
'# type' not in line and 'test' not in fn and 'example' not in fn and
|
2017-11-22 06:53:35 +01:00
|
|
|
# Don't throw errors for markdown format URLs
|
lint: Fix use of re.match in judging long lines.
The `re.match` function in the Python stdlib is a trap for the unwary,
with surprising and asymmetrical semantics; we should probably add a
lint rule to ban it entirely. The docstring says:
> Try to apply the pattern at the start of the string, [...]
In other words, it effectively adds a `^` at the start (or `\A`, where
the distinction matters.) It's bad enough that this differs from what
grep, sed, perl, less, and every other tool I can think of do when
looking for matches to a regex; on top of that, it treats the
beginning of the string differently from the end, for no obvious
reason. The function that does what the rest of the world understands
by "match against this regex" is `re.search`.
In this case, it's unlikely that anyone intended for comments with
URLs, or `api_url` references, to miss out on their respective
exceptions to the long-line rule if they happen to start after the
first column. So fix those rules by just switching to `re.search`
with the same pattern.
I think Markdown URL references may have to start at the beginning of
the line, so I've left a `^` there to preserve -- but now make
explicit -- the `re.match` behavior.
2017-11-28 05:39:24 +01:00
|
|
|
not re.search("^\[[ A-Za-z0-9_:,&()-]*\]: http.*", line) and
|
2017-11-22 06:53:35 +01:00
|
|
|
# Don't throw errors for URLs in code comments
|
lint: Fix use of re.match in judging long lines.
The `re.match` function in the Python stdlib is a trap for the unwary,
with surprising and asymmetrical semantics; we should probably add a
lint rule to ban it entirely. The docstring says:
> Try to apply the pattern at the start of the string, [...]
In other words, it effectively adds a `^` at the start (or `\A`, where
the distinction matters.) It's bad enough that this differs from what
grep, sed, perl, less, and every other tool I can think of do when
looking for matches to a regex; on top of that, it treats the
beginning of the string differently from the end, for no obvious
reason. The function that does what the rest of the world understands
by "match against this regex" is `re.search`.
In this case, it's unlikely that anyone intended for comments with
URLs, or `api_url` references, to miss out on their respective
exceptions to the long-line rule if they happen to start after the
first column. So fix those rules by just switching to `re.search`
with the same pattern.
I think Markdown URL references may have to start at the beginning of
the line, so I've left a `^` there to preserve -- but now make
explicit -- the `re.match` behavior.
2017-11-28 05:39:24 +01:00
|
|
|
not re.search("[#].*http.*", line) and
|
|
|
|
not re.search("`\{\{ api_url \}\}[^`]+`", line) and
|
2017-11-15 21:00:27 +01:00
|
|
|
"# ignorelongline" not in line and 'migrations' not in fn):
|
2017-09-11 19:56:19 +02:00
|
|
|
print("Line too long (%s) at %s line %s: %s" % (len(line), fn, i+1, line_newline_stripped))
|
2017-06-05 16:29:04 +02:00
|
|
|
failed = True
|
2017-09-11 19:56:19 +02:00
|
|
|
lastLine = line
|
|
|
|
|
2017-10-12 18:40:45 +02:00
|
|
|
if firstline:
|
|
|
|
if os.path.splitext(fn)[1] and 'zerver/' in fn:
|
|
|
|
shebang_rules = [{'pattern': '^#!',
|
|
|
|
'description': "zerver library code shouldn't have a shebang line."}]
|
|
|
|
else:
|
|
|
|
shebang_rules = [{'pattern': '#!/usr/bin/python',
|
|
|
|
'description': "Use `#!/usr/bin/env python3` instead of `#!/usr/bin/python`"},
|
|
|
|
{'pattern': '#!/usr/bin/env python$',
|
|
|
|
'description': "Use `#!/usr/bin/env python3` instead of `#!/usr/bin/env python`."}]
|
|
|
|
for rule in shebang_rules:
|
|
|
|
if re.search(rule['pattern'], firstline):
|
|
|
|
print_err(identifier, color,
|
|
|
|
'{} at {} line 1:'.format(rule['description'], fn))
|
|
|
|
print_err(identifier, color, firstline)
|
|
|
|
failed = True
|
|
|
|
|
2017-09-11 19:56:19 +02:00
|
|
|
if lastLine and ('\n' not in lastLine):
|
|
|
|
print("No newline at the end of file. Fix with `sed -i '$a\\' %s`" % (fn,))
|
|
|
|
failed = True
|
2017-06-21 01:17:00 +02:00
|
|
|
|
2017-09-11 19:56:19 +02:00
|
|
|
return failed
|
|
|
|
|
|
|
|
def build_custom_checkers(by_lang):
|
|
|
|
# type: (Dict[str, List[str]]) -> Tuple[Callable[[], bool], Callable[[], bool]]
|
2017-06-05 16:29:04 +02:00
|
|
|
|
2017-09-11 19:48:19 +02:00
|
|
|
# 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.
|
2017-08-15 17:54:29 +02:00
|
|
|
trailing_whitespace_rule = {
|
|
|
|
'pattern': '\s+$',
|
|
|
|
'strip': '\n',
|
|
|
|
'description': 'Fix trailing whitespace'
|
|
|
|
}
|
2017-06-05 16:29:04 +02:00
|
|
|
whitespace_rules = [
|
|
|
|
# This linter should be first since bash_rules depends on it.
|
2017-08-15 17:54:29 +02:00
|
|
|
trailing_whitespace_rule,
|
2017-11-16 19:54:24 +01:00
|
|
|
{'pattern': 'http://zulip.readthedocs.io',
|
|
|
|
'description': 'Use HTTPS when linking to ReadTheDocs',
|
|
|
|
},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': '\t',
|
|
|
|
'strip': '\n',
|
2017-09-11 19:51:47 +02:00
|
|
|
'exclude': set(['tools/travis/success-http-headers.txt']),
|
2017-06-05 16:29:04 +02:00
|
|
|
'description': 'Fix tab-based whitespace'},
|
|
|
|
] # type: RuleList
|
2017-10-18 19:11:33 +02:00
|
|
|
comma_whitespace_rule = [
|
2017-10-18 19:16:41 +02:00
|
|
|
{'pattern': ', {2,}[^#/ ]',
|
|
|
|
'exclude': set(['zerver/tests', 'frontend_tests/node_tests']),
|
2017-10-18 19:11:33 +02:00
|
|
|
'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)']},
|
|
|
|
] # type: RuleList
|
2017-06-05 16:29:04 +02:00
|
|
|
markdown_whitespace_rules = list([rule for rule in whitespace_rules if rule['pattern'] != '\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': '((?<!\s)\s$)|(\s\s\s+$)|(^\s+$)',
|
|
|
|
'strip': '\n',
|
|
|
|
'description': 'Fix trailing whitespace'},
|
|
|
|
{'pattern': '^#+[A-Za-z0-9]',
|
|
|
|
'strip': '\n',
|
2017-09-13 18:02:58 +02:00
|
|
|
'description': 'Missing space after # in heading',
|
|
|
|
'good_lines': ['### some heading', '# another heading'],
|
|
|
|
'bad_lines': ['###some heading', '#another heading']},
|
2017-06-05 16:29:04 +02:00
|
|
|
] # type: RuleList
|
|
|
|
js_rules = cast(RuleList, [
|
|
|
|
{'pattern': '[^_]function\(',
|
|
|
|
'description': 'The keyword "function" should be followed by a space'},
|
|
|
|
{'pattern': '.*blueslip.warning\(.*',
|
|
|
|
'description': 'The module blueslip has no function warning, try using blueslip.warn'},
|
|
|
|
{'pattern': '[)]{$',
|
|
|
|
'description': 'Missing space between ) and {'},
|
2017-09-08 09:35:11 +02:00
|
|
|
{'pattern': 'i18n\.t\([^)]+[^,\{\)]$',
|
2017-09-05 08:37:25 +02:00
|
|
|
'description': 'i18n string should not be a multiline string'},
|
2017-09-08 09:35:11 +02:00
|
|
|
{'pattern': 'i18n\.t\([\'\"].+?[\'\"]\s*\+',
|
|
|
|
'description': 'Do not concatenate arguments within i18n.t()'},
|
|
|
|
{'pattern': 'i18n\.t\(.+\).*\+',
|
2017-09-05 08:37:25 +02:00
|
|
|
'description': 'Do not concatenate i18n strings'},
|
2017-09-08 09:35:11 +02:00
|
|
|
{'pattern': '\+.*i18n\.t\(.+\)',
|
2017-09-05 08:37:25 +02:00
|
|
|
'description': 'Do not concatenate i18n strings'},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'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',
|
2017-10-12 05:39:46 +02:00
|
|
|
'static/js/debug.js',
|
|
|
|
'tools/generate-custom-icon-webfont']),
|
2017-06-05 16:29:04 +02:00
|
|
|
'description': 'console.log and similar should not be used in webapp'},
|
|
|
|
{'pattern': '[.]text\(["\'][a-zA-Z]',
|
|
|
|
'description': 'Strings passed to $().text should be wrapped in i18n.t() for internationalization'},
|
|
|
|
{'pattern': 'compose_error\(["\']',
|
|
|
|
'description': 'Argument to compose_error should be a literal string enclosed '
|
|
|
|
'by i18n.t()'},
|
|
|
|
{'pattern': 'ui.report_success\(',
|
|
|
|
'description': 'Deprecated function, use ui_report.success.'},
|
|
|
|
{'pattern': 'report.success\(["\']',
|
|
|
|
'description': 'Argument to report_success should be a literal string enclosed '
|
|
|
|
'by i18n.t()'},
|
|
|
|
{'pattern': 'ui.report_error\(',
|
|
|
|
'description': 'Deprecated function, use ui_report.error.'},
|
|
|
|
{'pattern': 'report.error\(["\']',
|
|
|
|
'description': 'Argument to report_error should be a literal string enclosed '
|
|
|
|
'by i18n.t()'},
|
2017-10-05 16:03:58 +02:00
|
|
|
{'pattern': '\$\(document\)\.ready\(',
|
|
|
|
'description': "`Use $(f) rather than `$(document).ready(f)`",
|
|
|
|
'good_lines': ['$(function () {foo();}'],
|
|
|
|
'bad_lines': ['$(document).ready(function () {foo();}']},
|
2018-02-14 01:47:09 +01:00
|
|
|
{'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/',
|
|
|
|
]),
|
|
|
|
'good_lines': ['channel.get(...)'],
|
|
|
|
'bad_lines': ['$.get()', '$.post()', '$.ajax()']},
|
2017-10-06 10:12:47 +02:00
|
|
|
{'pattern': 'style ?=',
|
|
|
|
'description': "Avoid using the `style=` attribute; we prefer styling in CSS files",
|
|
|
|
'exclude': set([
|
2017-11-23 21:17:08 +01:00
|
|
|
'frontend_tests/node_tests/copy_and_paste.js',
|
2017-11-25 22:22:36 +01:00
|
|
|
'frontend_tests/node_tests/upload.js',
|
2017-10-06 10:12:47 +02:00
|
|
|
'frontend_tests/node_tests/templates.js',
|
2017-11-23 15:00:05 +01:00
|
|
|
'static/js/upload.js',
|
2017-10-06 10:12:47 +02:00
|
|
|
'static/js/dynamic_text.js',
|
|
|
|
'static/js/stream_color.js',
|
|
|
|
]),
|
|
|
|
'good_lines': ['#my-style {color: blue;}'],
|
|
|
|
'bad_lines': ['<p style="color: blue;">Foo</p>', 'style = "color: blue;"']},
|
2017-10-18 19:16:41 +02:00
|
|
|
]) + whitespace_rules + comma_whitespace_rule
|
2017-06-05 16:29:04 +02:00
|
|
|
python_rules = cast(RuleList, [
|
|
|
|
{'pattern': '^(?!#)@login_required',
|
2017-09-26 15:54:14 +02:00
|
|
|
'description': '@login_required is unsupported; use @zulip_login_required',
|
|
|
|
'good_lines': ['@zulip_login_required', '# foo @login_required'],
|
|
|
|
'bad_lines': ['@login_required', ' @login_required']},
|
2017-11-20 19:52:10 +01:00
|
|
|
{'pattern': '^user_profile[.]save[(][)]',
|
|
|
|
'description': 'Always pass update_fields when saving user_profile objects',
|
|
|
|
'exclude_line': set([
|
|
|
|
('zerver/lib/actions.py', "user_profile.save() # Can't use update_fields because of how the foreign key works."),
|
|
|
|
]),
|
|
|
|
'exclude': set(['zerver/tests', 'zerver/lib/create_user.py']),
|
|
|
|
'good_lines': ['user_profile.save(update_fields=["pointer"])'],
|
|
|
|
'bad_lines': ['user_profile.save()']},
|
2017-10-07 03:33:20 +02:00
|
|
|
{'pattern': '^[^"]*"[^"]*"%\(',
|
2017-09-26 15:54:14 +02:00
|
|
|
'description': 'Missing space around "%"',
|
|
|
|
'good_lines': ['"%s" % ("foo")', '"%s" % (foo)'],
|
|
|
|
'bad_lines': ['"%s"%("foo")', '"%s"%(foo)']},
|
2017-10-07 03:33:20 +02:00
|
|
|
{'pattern': "^[^']*'[^']*'%\(",
|
2017-09-26 15:54:14 +02:00
|
|
|
'description': 'Missing space around "%"',
|
|
|
|
'good_lines': ["'%s' % ('foo')", "'%s' % (foo)"],
|
|
|
|
'bad_lines': ["'%s'%('foo')", "'%s'%(foo)"]},
|
2017-12-08 17:03:58 +01:00
|
|
|
{'pattern': 'self: Any',
|
|
|
|
'description': 'you can omit Any annotation for self',
|
|
|
|
'good_lines': ['def foo (self):'],
|
|
|
|
'bad_lines': ['def foo(self: Any):']},
|
2017-06-05 16:29:04 +02:00
|
|
|
# This rule is constructed with + to avoid triggering on itself
|
|
|
|
{'pattern': " =" + '[^ =>~"]',
|
2017-09-26 15:54:14 +02:00
|
|
|
'description': 'Missing whitespace after "="',
|
|
|
|
'good_lines': ['a = b', '5 == 6'],
|
|
|
|
'bad_lines': ['a =b', 'asdf =42']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': '":\w[^"]*$',
|
2017-09-26 15:54:14 +02:00
|
|
|
'description': 'Missing whitespace after ":"',
|
|
|
|
'good_lines': ['"foo": bar', '"some:string:with:colons"'],
|
|
|
|
'bad_lines': ['"foo":bar', '"foo":1']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': "':\w[^']*$",
|
2017-09-26 15:54:14 +02:00
|
|
|
'description': 'Missing whitespace after ":"',
|
|
|
|
'good_lines': ["'foo': bar", "'some:string:with:colons'"],
|
|
|
|
'bad_lines': ["'foo':bar", "'foo':1"]},
|
2017-09-27 09:47:46 +02:00
|
|
|
{'pattern': "^\s+#\w",
|
2017-06-05 16:29:04 +02:00
|
|
|
'strip': '\n',
|
2017-10-29 02:02:58 +01:00
|
|
|
'exclude': set(['tools/droplets/create.py']),
|
2017-09-26 15:54:14 +02:00
|
|
|
'description': 'Missing whitespace after "#"',
|
|
|
|
'good_lines': ['a = b # some operation', '1+2 # 3 is the result'],
|
|
|
|
'bad_lines': [' #some operation', ' #not valid!!!']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': "assertEquals[(]",
|
2017-09-26 15:54:14 +02:00
|
|
|
'description': 'Use assertEqual, not assertEquals (which is deprecated).',
|
|
|
|
'good_lines': ['assertEqual(1, 2)'],
|
|
|
|
'bad_lines': ['assertEquals(1, 2)']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': "== None",
|
2017-09-26 15:54:14 +02:00
|
|
|
'description': 'Use `is None` to check whether something is None',
|
|
|
|
'good_lines': ['if foo is None'],
|
|
|
|
'bad_lines': ['foo == None']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': "type:[(]",
|
2017-09-26 15:54:14 +02:00
|
|
|
'description': 'Missing whitespace after ":" in type annotation',
|
|
|
|
'good_lines': ['# type: (Any, Any)', 'colon:separated:string:containing:type:as:keyword'],
|
|
|
|
'bad_lines': ['# type:(Any, Any)']},
|
2017-07-28 00:59:36 +02:00
|
|
|
{'pattern': "type: ignore$",
|
|
|
|
'exclude': set(['tools/tests',
|
|
|
|
'zerver/lib/test_runner.py',
|
|
|
|
'zerver/tests']),
|
2017-11-08 19:35:35 +01:00
|
|
|
'description': '"type: ignore" should always end with "# type: ignore # explanation for why"',
|
|
|
|
'good_lines': ['foo = bar # type: ignore # explanation'],
|
|
|
|
'bad_lines': ['foo = bar # type: ignore']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': "# type [(]",
|
2017-11-08 19:35:35 +01:00
|
|
|
'description': 'Missing : after type in type annotation',
|
|
|
|
'good_lines': ['foo = 42 # type: int', '# type: (str, int) -> None'],
|
|
|
|
'bad_lines': ['# type (str, int) -> None']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': "#type",
|
2017-11-08 19:35:35 +01:00
|
|
|
'description': 'Missing whitespace after "#" in type annotation',
|
|
|
|
'good_lines': ['foo = 42 # type: int'],
|
|
|
|
'bad_lines': ['foo = 42 #type: int']},
|
2017-10-05 15:09:46 +02:00
|
|
|
{'pattern': r'\b(if|else|while)[(]',
|
|
|
|
'description': 'Put a space between statements like if, else, etc. and (.',
|
|
|
|
'good_lines': ['if (1 == 2):', 'while (foo == bar):'],
|
|
|
|
'bad_lines': ['if(1 == 2):', 'while(foo == bar):']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': ", [)]",
|
2017-11-08 19:35:35 +01:00
|
|
|
'description': 'Unnecessary whitespace between "," and ")"',
|
|
|
|
'good_lines': ['foo = (1, 2, 3,)', 'foo(bar, 42)'],
|
|
|
|
'bad_lines': ['foo = (1, 2, 3, )']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': "% [(]",
|
2017-11-08 19:35:35 +01:00
|
|
|
'description': 'Unnecessary whitespace between "%" and "("',
|
|
|
|
'good_lines': ['"foo %s bar" % ("baz",)'],
|
|
|
|
'bad_lines': ['"foo %s bar" % ("baz",)']},
|
2017-06-05 16:29:04 +02:00
|
|
|
# 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.
|
2017-11-08 19:34:49 +01:00
|
|
|
{'pattern': ' % [a-zA-Z0-9_."\']*\)?$',
|
2017-06-05 16:29:04 +02:00
|
|
|
'exclude_line': set([
|
|
|
|
('tools/tests/test_template_parser.py', '{% foo'),
|
|
|
|
]),
|
2017-11-08 19:35:35 +01:00
|
|
|
'description': 'Used % comprehension without a tuple',
|
|
|
|
'good_lines': ['"foo %s bar" % ("baz",)'],
|
|
|
|
'bad_lines': ['"foo %s bar" % "baz"']},
|
2017-11-08 19:34:49 +01:00
|
|
|
{'pattern': '.*%s.* % \([a-zA-Z0-9_."\']*\)$',
|
2017-11-08 19:35:35 +01:00
|
|
|
'description': 'Used % comprehension without a tuple',
|
|
|
|
'good_lines': ['"foo %s bar" % ("baz",)"'],
|
|
|
|
'bad_lines': ['"foo %s bar" % ("baz")']},
|
2018-03-02 01:27:08 +01:00
|
|
|
{'pattern': 'sudo',
|
|
|
|
'include_only': set(['scripts/']),
|
|
|
|
'exclude': set(['scripts/lib/setup_venv.py']),
|
|
|
|
'exclude_line': set([
|
|
|
|
('scripts/lib/zulip_tools.py', '# We need sudo here, since the path will be under /srv/ in the'),
|
|
|
|
('scripts/lib/zulip_tools.py', 'subprocess.check_call(["sudo", "/bin/bash", "-c",'),
|
|
|
|
('scripts/lib/zulip_tools.py', 'subprocess.check_call(["sudo", "rm", "-rf", directory])'),
|
|
|
|
]),
|
|
|
|
'description': 'Most scripts are intended to run on systems without sudo.',
|
|
|
|
'good_lines': ['subprocess.check_call(["ls"])'],
|
|
|
|
'bad_lines': ['subprocess.check_call(["sudo", "ls"])']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': 'django.utils.translation',
|
2017-06-20 22:27:49 +02:00
|
|
|
'include_only': set(['test/']),
|
2017-11-08 19:35:35 +01:00
|
|
|
'description': 'Test strings should not be tagged for translation',
|
|
|
|
'good_lines': [''],
|
|
|
|
'bad_lines': ['django.utils.translation']},
|
2017-10-07 17:55:36 +02:00
|
|
|
{'pattern': 'userid',
|
2017-11-08 19:35:35 +01:00
|
|
|
'description': 'We prefer user_id over userid.',
|
|
|
|
'good_lines': ['id = alice.user_id'],
|
|
|
|
'bad_lines': ['id = alice.userid']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': 'json_success\({}\)',
|
2017-11-08 19:35:35 +01:00
|
|
|
'description': 'Use json_success() to return nothing',
|
|
|
|
'good_lines': ['return json_success()'],
|
|
|
|
'bad_lines': ['return json_success({})']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': '\Wjson_error\(_\(?\w+\)',
|
|
|
|
'exclude': set(['zerver/tests']),
|
2017-11-08 19:35:35 +01:00
|
|
|
'description': 'Argument to json_error should be a literal string enclosed by _()',
|
|
|
|
'good_lines': ['return json_error(_("string"))'],
|
|
|
|
'bad_lines': ['return json_error(_variable)', 'return json_error(_(variable))']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': '\Wjson_error\([\'"].+[),]$',
|
|
|
|
'exclude': set(['zerver/tests']),
|
|
|
|
'exclude_line': set([
|
|
|
|
# We don't want this string tagged for translation.
|
|
|
|
('zerver/views/compatibility.py', 'return json_error("Client is too old")'),
|
|
|
|
]),
|
|
|
|
'description': 'Argument to json_error should a literal string enclosed by _()'},
|
|
|
|
# To avoid JsonableError(_variable) and JsonableError(_(variable))
|
|
|
|
{'pattern': '\WJsonableError\(_\(?\w.+\)',
|
|
|
|
'exclude': set(['zerver/tests']),
|
|
|
|
'description': 'Argument to JsonableError should be a literal string enclosed by _()'},
|
|
|
|
{'pattern': '\WJsonableError\(["\'].+\)',
|
|
|
|
'exclude': set(['zerver/tests']),
|
|
|
|
'description': 'Argument to JsonableError should be a literal string enclosed by _()'},
|
|
|
|
{'pattern': '([a-zA-Z0-9_]+)=REQ\([\'"]\\1[\'"]',
|
|
|
|
'description': 'REQ\'s first argument already defaults to parameter name'},
|
|
|
|
{'pattern': '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.
|
2017-09-27 09:47:46 +02:00
|
|
|
{'pattern': '[^r]Message.objects.get',
|
2017-08-02 06:02:06 +02:00
|
|
|
'exclude': set(["zerver/tests",
|
|
|
|
"zerver/lib/onboarding.py",
|
2017-08-06 09:56:29 +02:00
|
|
|
"zilencer/management/commands/add_mock_conversation.py",
|
2017-08-02 06:02:06 +02:00
|
|
|
"zerver/worker/queue_processors.py"]),
|
2017-06-05 16:29:04 +02:00
|
|
|
'description': 'Please use access_message() to fetch Message objects',
|
|
|
|
},
|
2017-09-27 09:47:46 +02:00
|
|
|
{'pattern': 'Stream.objects.get',
|
2017-06-05 16:29:04 +02:00
|
|
|
'include_only': set(["zerver/views/"]),
|
|
|
|
'description': 'Please use access_stream_by_*() to fetch Stream objects',
|
|
|
|
},
|
|
|
|
{'pattern': 'get_stream[(]',
|
|
|
|
'include_only': set(["zerver/views/", "zerver/lib/actions.py"]),
|
|
|
|
'exclude_line': set([
|
|
|
|
# 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)'),
|
2017-10-04 02:01:22 +02:00
|
|
|
('zerver/lib/actions.py', 'get_stream(admin_realm_signup_notifications_stream, admin_realm)'),
|
2017-08-15 18:58:29 +02:00
|
|
|
# Here we need get_stream to access streams you've since unsubscribed from.
|
|
|
|
('zerver/views/messages.py', 'stream = get_stream(operand, self.user_profile.realm)'),
|
2017-09-17 20:19:12 +02:00
|
|
|
# Use stream_id to exclude mutes.
|
|
|
|
('zerver/views/messages.py', 'stream_id = get_stream(stream_name, user_profile.realm).id'),
|
2017-06-05 16:29:04 +02:00
|
|
|
]),
|
|
|
|
'description': 'Please use access_stream_by_*() to fetch Stream objects',
|
|
|
|
},
|
2017-09-27 09:47:46 +02:00
|
|
|
{'pattern': 'Stream.objects.filter',
|
2017-06-05 16:29:04 +02:00
|
|
|
'include_only': set(["zerver/views/"]),
|
|
|
|
'description': 'Please use access_stream_by_*() to fetch Stream objects',
|
|
|
|
},
|
|
|
|
{'pattern': '^from (zerver|analytics|confirmation)',
|
|
|
|
'include_only': set(["/migrations/"]),
|
2017-09-01 14:22:18 +02:00
|
|
|
'exclude': set([
|
|
|
|
'zerver/migrations/0032_verify_all_medium_avatar_images.py',
|
|
|
|
'zerver/migrations/0041_create_attachments_for_old_messages.py',
|
|
|
|
'zerver/migrations/0060_move_avatars_to_be_uid_based.py',
|
|
|
|
'zerver/migrations/0104_fix_unreads.py',
|
|
|
|
]),
|
2017-11-08 17:55:36 +01:00
|
|
|
'description': "Don't import models or other code in migrations; see docs/subsystems/schema-migrations.md",
|
2017-06-05 16:29:04 +02:00
|
|
|
},
|
|
|
|
{'pattern': 'datetime[.](now|utcnow)',
|
|
|
|
'include_only': set(["zerver/", "analytics/"]),
|
|
|
|
'description': "Don't use datetime in backend code.\n"
|
2017-11-08 17:55:36 +01:00
|
|
|
"See https://zulip.readthedocs.io/en/latest/contributing/code-style.html#naive-datetime-objects",
|
2017-06-05 16:29:04 +02:00
|
|
|
},
|
|
|
|
{'pattern': 'render_to_response\(',
|
|
|
|
'description': "Use render() instead of render_to_response().",
|
|
|
|
},
|
2017-09-22 08:15:01 +02:00
|
|
|
{'pattern': 'from os.path',
|
|
|
|
'description': "Don't use from when importing from the standard library",
|
|
|
|
},
|
2017-09-22 18:30:18 +02:00
|
|
|
{'pattern': 'import os.path',
|
|
|
|
'description': "Use import os instead of import os.path",
|
|
|
|
},
|
2017-10-02 11:12:52 +02:00
|
|
|
{'pattern': '(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')"]},
|
2017-10-06 13:17:23 +02:00
|
|
|
{'pattern': '\.pk',
|
|
|
|
'exclude_pattern': '[.]_meta[.]pk',
|
|
|
|
'description': "Use `id` instead of `pk`.",
|
|
|
|
'good_lines': ['if my_django_model.id == 42', 'self.user_profile._meta.pk'],
|
2017-10-18 11:12:55 +02:00
|
|
|
'bad_lines': ['if my_django_model.pk == 42']},
|
2017-10-18 19:11:33 +02:00
|
|
|
]) + whitespace_rules + comma_whitespace_rule
|
2018-03-02 05:31:46 +01:00
|
|
|
bash_rules = cast(RuleList, [
|
2017-06-05 16:29:04 +02:00
|
|
|
{'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'},
|
2018-03-02 01:27:08 +01:00
|
|
|
{'pattern': 'sudo',
|
|
|
|
'description': 'Most scripts are intended to work on systems without sudo',
|
|
|
|
'include_only': set(['scripts/']),
|
|
|
|
'exclude': set([
|
|
|
|
'scripts/lib/install',
|
|
|
|
'scripts/lib/create-zulip-admin',
|
|
|
|
'scripts/setup/terminate-psql-sessions',
|
|
|
|
'scripts/setup/configure-rabbitmq'
|
|
|
|
]), },
|
2018-03-02 05:31:46 +01:00
|
|
|
]) + whitespace_rules[0:1]
|
2017-06-05 16:29:04 +02:00
|
|
|
css_rules = cast(RuleList, [
|
2018-02-25 19:49:13 +01:00
|
|
|
{'pattern': 'calc\([^+]+\+[^+]+\)',
|
|
|
|
'description': "Avoid using calc with '+' operator. See #8403 : in CSS.",
|
|
|
|
'good_lines': ["width: calc(20% - -14px);"],
|
|
|
|
'bad_lines': ["width: calc(20% + 14px);"]},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': '^[^:]*:\S[^:]*;$',
|
2017-12-01 17:11:55 +01:00
|
|
|
'description': "Missing whitespace after : in CSS",
|
|
|
|
'good_lines': ["background-color: white;", "text-size: 16px;"],
|
|
|
|
'bad_lines': ["background-color:white;", "text-size:16px;"]},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': '[a-z]{',
|
2017-12-01 17:11:55 +01:00
|
|
|
'description': "Missing whitespace before '{' in CSS.",
|
|
|
|
'good_lines': ["input {", "body {"],
|
|
|
|
'bad_lines': ["input{", "body{"]},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': 'https://',
|
2017-12-01 17:11:55 +01:00
|
|
|
'description': "Zulip CSS should have no dependencies on external resources",
|
|
|
|
'good_lines': ['background: url(/static/images/landing-page/pycon.jpg);'],
|
|
|
|
'bad_lines': ['background: url(https://example.com/image.png);']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': '^[ ][ ][a-zA-Z0-9]',
|
|
|
|
'description': "Incorrect 2-space indentation in CSS",
|
|
|
|
'exclude': set(['static/third/thirdparty-fonts.css']),
|
2017-12-01 17:11:55 +01:00
|
|
|
'strip': '\n',
|
|
|
|
'good_lines': [" color: white;", "color: white;"],
|
|
|
|
'bad_lines': [" color: white;"]},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': '{\w',
|
2017-12-01 17:11:55 +01:00
|
|
|
'description': "Missing whitespace after '{' in CSS (should be newline).",
|
|
|
|
'good_lines': ["{\n"],
|
|
|
|
'bad_lines': ["{color: LightGoldenRodYellow;"]},
|
|
|
|
{'pattern': ' thin[ ;]',
|
|
|
|
'description': "thin CSS attribute is under-specified, please use 1px.",
|
|
|
|
'good_lines': ["border-width: 1px;"],
|
|
|
|
'bad_lines': ["border-width: thin;", "border-width: thin solid black;"]},
|
|
|
|
{'pattern': ' medium[ ;]',
|
|
|
|
'description': "medium CSS attribute is under-specified, please use pixels.",
|
|
|
|
'good_lines': ["border-width: 3px;"],
|
|
|
|
'bad_lines': ["border-width: medium;", "border: medium solid black;"]},
|
|
|
|
{'pattern': ' thick[ ;]',
|
|
|
|
'description': "thick CSS attribute is under-specified, please use pixels.",
|
|
|
|
'good_lines': ["border-width: 5px;"],
|
|
|
|
'bad_lines': ["border-width: thick;", "border: thick solid black;"]},
|
2017-10-18 19:11:33 +02:00
|
|
|
]) + whitespace_rules + comma_whitespace_rule
|
2017-10-18 19:50:25 +02:00
|
|
|
prose_style_rules = cast(RuleList, [
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': '[^\/\#\-\"]([jJ]avascript)', # exclude usage in hrefs/divs
|
|
|
|
'description': "javascript should be spelled JavaScript"},
|
|
|
|
{'pattern': '[^\/\-\.\"\'\_\=\>]([gG]ithub)[^\.\-\_\"\<]', # exclude usage in hrefs/divs
|
|
|
|
'description': "github should be spelled GitHub"},
|
|
|
|
{'pattern': '[oO]rganisation', # exclude usage in hrefs/divs
|
2017-09-13 18:45:06 +02:00
|
|
|
'description': "Organization is spelled with a z",
|
2017-11-08 17:55:36 +01:00
|
|
|
'exclude_line': [('docs/translating/french.md', '* organization - **organisation**')]},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': '!!! warning',
|
|
|
|
'description': "!!! warning is invalid; it's spelled '!!! warn'"},
|
2017-09-07 11:30:43 +02:00
|
|
|
{'pattern': 'Terms of service',
|
|
|
|
'description': "The S in Terms of Service is capitalized"},
|
2017-10-18 19:50:25 +02:00
|
|
|
]) + comma_whitespace_rule
|
2017-06-05 16:29:04 +02:00
|
|
|
html_rules = whitespace_rules + prose_style_rules + [
|
2018-03-02 18:30:42 +01:00
|
|
|
{'pattern': 'placeholder="[^{#](?:(?!\.com).)+$',
|
2017-06-05 16:29:04 +02:00
|
|
|
'description': "`placeholder` value should be translatable.",
|
|
|
|
'exclude_line': [('templates/zerver/register.html', 'placeholder="acme"'),
|
2018-03-02 18:28:57 +01:00
|
|
|
('templates/zerver/register.html', 'placeholder="Acme or Aκμή"')],
|
2017-12-01 17:11:55 +01:00
|
|
|
'good_lines': ['<input class="stream-list-filter" type="text" placeholder="{{ _(\'Search streams\') }}" />'],
|
2018-02-15 22:16:40 +01:00
|
|
|
'bad_lines': ['<input placeholder="foo">']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': "placeholder='[^{]",
|
2017-12-01 17:11:55 +01:00
|
|
|
'description': "`placeholder` value should be translatable.",
|
|
|
|
'good_lines': ['<input class="stream-list-filter" type="text" placeholder="{{ _(\'Search streams\') }}" />'],
|
2018-02-15 22:16:40 +01:00
|
|
|
'bad_lines': ["<input placeholder='foo'>"]},
|
2017-07-14 23:48:17 +02:00
|
|
|
{'pattern': "aria-label='[^{]",
|
2017-12-01 17:11:55 +01:00
|
|
|
'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>"]},
|
2017-07-14 23:48:17 +02:00
|
|
|
{'pattern': 'aria-label="[^{]',
|
2017-11-21 18:49:45 +01:00
|
|
|
'description': "`aria-label` value should be translatable.",
|
2017-12-01 17:11:55 +01:00
|
|
|
'good_lines': ['<button type="button" class="close close-alert-word-status" aria-label="{{t \'Close\' }}">'],
|
|
|
|
'bad_lines': ['<button aria-label="foo"></button>']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': 'script src="http',
|
2017-12-01 17:11:55 +01:00
|
|
|
'description': "Don't directly load dependencies from CDNs. See docs/subsystems/front-end-build-process.md",
|
2018-01-30 01:30:02 +01:00
|
|
|
'exclude': set(["templates/zilencer/billing.html"]),
|
2017-12-01 17:11:55 +01:00
|
|
|
'good_lines': ["{{ render_bundle('landing-page') }}"],
|
|
|
|
'bad_lines': ['<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': "title='[^{]",
|
2017-12-01 17:11:55 +01:00
|
|
|
'description': "`title` value should be translatable.",
|
|
|
|
'good_lines': ['<link rel="author" title="{{ _(\'About these documents\') }}" />'],
|
|
|
|
'bad_lines': ["<p title='foo'></p>"]},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': 'title="[^{\:]',
|
|
|
|
'exclude_line': set([
|
|
|
|
('templates/zerver/markdown_help.html',
|
|
|
|
'<td><img alt=":heart:" class="emoji" src="/static/generated/emoji/images/emoji/heart.png" title=":heart:" /></td>')
|
|
|
|
]),
|
|
|
|
'exclude': set(["templates/zerver/emails"]),
|
|
|
|
'description': "`title` value should be translatable."},
|
|
|
|
{'pattern': '\Walt=["\'][^{"\']',
|
|
|
|
'description': "alt argument should be enclosed by _() or it should be an empty string.",
|
|
|
|
'exclude': set(['static/templates/settings/display-settings.handlebars',
|
|
|
|
'templates/zerver/keyboard_shortcuts.html',
|
|
|
|
'templates/zerver/markdown_help.html']),
|
2017-12-01 17:11:55 +01:00
|
|
|
'good_lines': ['<img src="{{source_url}}" alt="{{ _(name) }}" />', '<img alg="" />'],
|
|
|
|
'bad_lines': ['<img alt="Foo Image" />']},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': '\Walt=["\']{{ ?["\']',
|
|
|
|
'description': "alt argument should be enclosed by _().",
|
2017-12-01 17:11:55 +01:00
|
|
|
'good_lines': ['<img src="{{source_url}}" alt="{{ _(name) }}" />'],
|
|
|
|
'bad_lines': ['<img alt="{{ " />']},
|
2017-10-05 16:42:08 +02:00
|
|
|
{'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': set(['templates/zerver/dev_login.html']),
|
|
|
|
'good_lines': ["($('#foo').on('click', function () {}"],
|
|
|
|
'bad_lines': ["<button id='foo' onclick='myFunction()'>Foo</button>", "<input onchange='myFunction()'>"]},
|
2017-10-06 10:12:47 +02:00
|
|
|
{'pattern': 'style ?=',
|
|
|
|
'description': "Avoid using the `style=` attribute; we prefer styling in CSS files",
|
2017-10-06 22:07:47 +02:00
|
|
|
'exclude_pattern': r'.*style ?=["' + "'" + '](display: ?none|background: {{|color: {{|background-color: {{).*',
|
2017-10-06 10:12:47 +02:00
|
|
|
'exclude': set([
|
|
|
|
# KaTeX output uses style attribute
|
|
|
|
'templates/zerver/markdown_help.html',
|
|
|
|
# 5xx page doesn't have external CSS
|
|
|
|
'static/html/5xx.html',
|
|
|
|
# Group PMs color is dynamically calculated
|
|
|
|
'static/templates/group_pms.handlebars',
|
|
|
|
|
2017-10-06 22:07:47 +02:00
|
|
|
# exclude_pattern above handles color, but have other issues:
|
2017-10-06 10:12:47 +02:00
|
|
|
'static/templates/draft.handlebars',
|
|
|
|
'static/templates/subscription.handlebars',
|
2017-10-06 22:07:47 +02:00
|
|
|
'static/templates/single_message.handlebars',
|
2017-10-06 10:12:47 +02:00
|
|
|
|
|
|
|
# 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/digest.html',
|
|
|
|
'templates/zerver/emails/missed_message.html',
|
|
|
|
'templates/zerver/emails/email_base_messages.html',
|
|
|
|
|
2017-10-11 19:41:40 +02:00
|
|
|
# Email log templates; should clean up.
|
|
|
|
'templates/zerver/email.html',
|
|
|
|
'templates/zerver/email_log.html',
|
|
|
|
|
2017-10-06 22:07:47 +02:00
|
|
|
# Probably just needs to be changed to display: none so the exclude works
|
|
|
|
'templates/zerver/navbar.html',
|
|
|
|
|
|
|
|
# Needs the width cleaned up; display: none is fine
|
|
|
|
'static/templates/settings/account-settings.handlebars',
|
|
|
|
|
2017-10-06 10:12:47 +02:00
|
|
|
# Inline styling for an svg; could be moved to CSS files?
|
|
|
|
'templates/zerver/landing_nav.html',
|
|
|
|
'templates/zerver/home.html',
|
|
|
|
'templates/zerver/features.html',
|
|
|
|
'templates/zerver/portico-header.html',
|
|
|
|
|
|
|
|
# Miscellaneous violations to be cleaned up
|
|
|
|
'static/templates/user_info_popover_title.handlebars',
|
|
|
|
'static/templates/subscription_invites_warning_modal.handlebars',
|
|
|
|
'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/help/main.html',
|
2017-07-25 02:37:04 +02:00
|
|
|
'templates/zerver/api/main.html',
|
2017-10-06 10:12:47 +02:00
|
|
|
'templates/analytics/realm_summary_table.html',
|
|
|
|
'templates/corporate/zephyr.html',
|
|
|
|
'templates/corporate/zephyr-mirror.html',
|
|
|
|
]),
|
2017-10-06 22:07:47 +02:00
|
|
|
'good_lines': ['#my-style {color: blue;}', 'style="display: none"', "style='display: none"],
|
2017-10-06 10:12:47 +02:00
|
|
|
'bad_lines': ['<p style="color: blue;">Foo</p>', 'style = "color: blue;"']},
|
2017-06-05 16:29:04 +02:00
|
|
|
] # type: RuleList
|
|
|
|
handlebars_rules = html_rules + [
|
|
|
|
{'pattern': "[<]script",
|
|
|
|
'description': "Do not use inline <script> tags here; put JavaScript in static/js instead."},
|
2017-06-10 12:32:06 +02:00
|
|
|
{'pattern': '{{ t ("|\')',
|
|
|
|
'description': 'There should be no spaces before the "t" in a translation tag.'},
|
2017-06-05 16:29:04 +02:00
|
|
|
{'pattern': "{{t '.*' }}[\.\?!]",
|
|
|
|
'description': "Period should be part of the translatable string."},
|
|
|
|
{'pattern': '{{t ".*" }}[\.\?!]',
|
|
|
|
'description': "Period should be part of the translatable string."},
|
|
|
|
{'pattern': "{{/tr}}[\.\?!]",
|
|
|
|
'description': "Period should be part of the translatable string."},
|
2017-06-10 11:56:36 +02:00
|
|
|
{'pattern': '{{t ("|\') ',
|
|
|
|
'description': 'Translatable strings should not have leading spaces.'},
|
2017-06-10 12:11:33 +02:00
|
|
|
{'pattern': "{{t '[^']+ ' }}",
|
|
|
|
'description': 'Translatable strings should not have trailing spaces.'},
|
|
|
|
{'pattern': '{{t "[^"]+ " }}',
|
|
|
|
'description': 'Translatable strings should not have trailing spaces.'},
|
2017-06-05 16:29:04 +02:00
|
|
|
]
|
|
|
|
jinja2_rules = html_rules + [
|
|
|
|
{'pattern': "{% endtrans %}[\.\?!]",
|
|
|
|
'description': "Period should be part of the translatable string."},
|
|
|
|
{'pattern': "{{ _(.+) }}[\.\?!]",
|
|
|
|
'description': "Period should be part of the translatable string."},
|
|
|
|
]
|
2017-08-15 17:54:29 +02:00
|
|
|
json_rules = [
|
2017-11-23 20:56:05 +01:00
|
|
|
# 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).
|
2017-08-15 17:54:29 +02:00
|
|
|
trailing_whitespace_rule,
|
2017-11-23 19:39:58 +01:00
|
|
|
{'pattern': '\t',
|
|
|
|
'strip': '\n',
|
2017-11-23 20:56:05 +01:00
|
|
|
'exclude': set(['zerver/webhooks/']),
|
2017-11-23 19:39:58 +01:00
|
|
|
'description': 'Fix tab-based whitespace'},
|
2017-11-23 20:56:05 +01:00
|
|
|
] # type: RuleList
|
2017-06-05 16:29:04 +02:00
|
|
|
markdown_rules = markdown_whitespace_rules + prose_style_rules + [
|
|
|
|
{'pattern': '\[(?P<url>[^\]]+)\]\((?P=url)\)',
|
2017-11-16 21:44:16 +01:00
|
|
|
'description': 'Linkified markdown URLs should use cleaner <http://example.com> syntax.'},
|
|
|
|
{'pattern': 'https://zulip.readthedocs.io/en/latest/[a-zA-Z0-9]',
|
2017-11-17 06:43:01 +01:00
|
|
|
'exclude': ['docs/overview/contributing.md', 'docs/overview/readme.md'],
|
2017-11-16 22:59:21 +01:00
|
|
|
'include_only': set(['docs/']),
|
2017-11-16 21:44:16 +01:00
|
|
|
'description': "Use relatve links (../foo/bar.html) to other documents in docs/",
|
|
|
|
},
|
|
|
|
{'pattern': '\][(][^#h]',
|
|
|
|
'include_only': set(['README.md', 'CONTRIBUTING.md']),
|
|
|
|
'description': "Use absolute links from docs served by GitHub",
|
|
|
|
},
|
2017-06-05 16:29:04 +02:00
|
|
|
]
|
|
|
|
help_markdown_rules = markdown_rules + [
|
|
|
|
{'pattern': '[a-z][.][A-Z]',
|
|
|
|
'description': "Likely missing space after end of sentence"},
|
|
|
|
{'pattern': '[rR]ealm',
|
|
|
|
'description': "Realms are referred to as Organizations in user-facing docs."},
|
|
|
|
]
|
|
|
|
txt_rules = whitespace_rules
|
|
|
|
|
|
|
|
def check_custom_checks_py():
|
|
|
|
# type: () -> bool
|
|
|
|
failed = False
|
2017-07-28 00:59:53 +02:00
|
|
|
color = next(colors)
|
2017-06-05 16:29:04 +02:00
|
|
|
|
|
|
|
for fn in by_lang['py']:
|
|
|
|
if 'custom_check.py' in fn:
|
|
|
|
continue
|
2017-11-04 13:21:48 +01:00
|
|
|
if custom_check_file(fn, 'py', python_rules, color, max_length=110):
|
2017-06-05 16:29:04 +02:00
|
|
|
failed = True
|
|
|
|
return failed
|
|
|
|
|
|
|
|
def check_custom_checks_nonpy():
|
|
|
|
# type: () -> bool
|
|
|
|
failed = False
|
|
|
|
|
2017-07-28 00:59:53 +02:00
|
|
|
color = next(colors)
|
2017-06-05 16:29:04 +02:00
|
|
|
for fn in by_lang['js']:
|
2017-07-28 00:59:53 +02:00
|
|
|
if custom_check_file(fn, 'js', js_rules, color):
|
2017-06-05 16:29:04 +02:00
|
|
|
failed = True
|
|
|
|
|
2017-07-28 00:59:53 +02:00
|
|
|
color = next(colors)
|
2017-06-05 16:29:04 +02:00
|
|
|
for fn in by_lang['sh']:
|
2017-07-28 00:59:53 +02:00
|
|
|
if custom_check_file(fn, 'sh', bash_rules, color):
|
2017-06-05 16:29:04 +02:00
|
|
|
failed = True
|
|
|
|
|
2017-07-28 00:59:53 +02:00
|
|
|
color = next(colors)
|
2017-06-05 16:29:04 +02:00
|
|
|
for fn in by_lang['css']:
|
2017-07-28 00:59:53 +02:00
|
|
|
if custom_check_file(fn, 'css', css_rules, color):
|
2017-06-05 16:29:04 +02:00
|
|
|
failed = True
|
|
|
|
|
2017-07-28 00:59:53 +02:00
|
|
|
color = next(colors)
|
2017-06-05 16:29:04 +02:00
|
|
|
for fn in by_lang['handlebars']:
|
2017-07-28 00:59:53 +02:00
|
|
|
if custom_check_file(fn, 'handlebars', handlebars_rules, color):
|
2017-06-05 16:29:04 +02:00
|
|
|
failed = True
|
|
|
|
|
2017-07-28 00:59:53 +02:00
|
|
|
color = next(colors)
|
2017-06-05 16:29:04 +02:00
|
|
|
for fn in by_lang['html']:
|
2017-07-28 00:59:53 +02:00
|
|
|
if custom_check_file(fn, 'html', jinja2_rules, color):
|
2017-06-05 16:29:04 +02:00
|
|
|
failed = True
|
|
|
|
|
2017-07-28 00:59:53 +02:00
|
|
|
color = next(colors)
|
2017-06-05 16:29:04 +02:00
|
|
|
for fn in by_lang['json']:
|
2017-07-28 00:59:53 +02:00
|
|
|
if custom_check_file(fn, 'json', json_rules, color):
|
2017-06-05 16:29:04 +02:00
|
|
|
failed = True
|
|
|
|
|
2017-07-28 00:59:53 +02:00
|
|
|
color = next(colors)
|
2017-06-05 16:29:04 +02:00
|
|
|
markdown_docs_length_exclude = {
|
2017-11-16 20:44:00 +01:00
|
|
|
# Has some example Vagrant output that's very long
|
2017-11-08 17:55:36 +01:00
|
|
|
"docs/development/setup-vagrant.md",
|
2017-11-16 20:44:00 +01:00
|
|
|
# Have wide output in code blocks
|
2017-11-08 17:55:36 +01:00
|
|
|
"docs/subsystems/logging.md",
|
2017-11-18 12:37:29 +01:00
|
|
|
"docs/subsystems/migration-renumbering.md",
|
2017-11-16 20:44:00 +01:00
|
|
|
# Have curl commands with JSON that would be messy to wrap
|
2017-06-05 16:29:04 +02:00
|
|
|
"zerver/webhooks/helloworld/doc.md",
|
|
|
|
"zerver/webhooks/trello/doc.md",
|
2017-11-16 20:44:00 +01:00
|
|
|
# Has a very long configuration line
|
2017-06-23 01:04:51 +02:00
|
|
|
"templates/zerver/integrations/perforce.md",
|
2017-11-16 17:36:52 +01:00
|
|
|
# Has some example code that could perhaps be wrapped
|
|
|
|
"templates/zerver/api/webhook-walkthrough.md",
|
2017-11-25 00:02:20 +01:00
|
|
|
# This macro has a long indented URL
|
|
|
|
"templates/zerver/help/include/git-webhook-url-with-branches-indented.md",
|
2017-06-05 16:29:04 +02:00
|
|
|
}
|
|
|
|
for fn in by_lang['md']:
|
|
|
|
max_length = None
|
|
|
|
if fn not in markdown_docs_length_exclude:
|
|
|
|
max_length = 120
|
|
|
|
rules = markdown_rules
|
|
|
|
if fn.startswith("templates/zerver/help"):
|
|
|
|
rules = help_markdown_rules
|
2017-07-28 00:59:53 +02:00
|
|
|
if custom_check_file(fn, 'md', rules, color, max_length=max_length):
|
2017-06-05 16:29:04 +02:00
|
|
|
failed = True
|
|
|
|
|
2017-07-28 00:59:53 +02:00
|
|
|
color = next(colors)
|
2017-06-05 16:29:04 +02:00
|
|
|
for fn in by_lang['txt'] + by_lang['text']:
|
2017-07-28 00:59:53 +02:00
|
|
|
if custom_check_file(fn, 'txt', txt_rules, color):
|
2017-06-05 16:29:04 +02:00
|
|
|
failed = True
|
|
|
|
|
2017-11-16 22:20:50 +01:00
|
|
|
color = next(colors)
|
|
|
|
for fn in by_lang['rst']:
|
|
|
|
if custom_check_file(fn, 'rst', txt_rules, color):
|
|
|
|
failed = True
|
|
|
|
|
2017-07-28 00:59:53 +02:00
|
|
|
color = next(colors)
|
2017-06-05 16:29:04 +02:00
|
|
|
for fn in by_lang['yaml']:
|
2017-07-28 00:59:53 +02:00
|
|
|
if custom_check_file(fn, 'yaml', txt_rules, color):
|
2017-06-05 16:29:04 +02:00
|
|
|
failed = True
|
|
|
|
|
|
|
|
return failed
|
|
|
|
|
|
|
|
return (check_custom_checks_py, check_custom_checks_nonpy)
|