Files
zulip/tools/lint-all
Robert Hönig 789ae8648a Add wrapper and log file output for provisioning.
Before this commit, provisioning was done by executing provision.py,
which printed the log directly to stdout, making debugging harder.
This commit creates a wrapper bash script 'provision' in tools, which
calls 'zulip/scripts/tools/provision_vm.py' (the new location of
provision.py) and prints all the output to
'zulip/var/log/zulip/zulip_provision.log' via 'tee'.
Travis tests and docs have been modified accordingly.
2017-01-17 14:23:28 -08:00

621 lines
26 KiB
Python
Executable File

#!/usr/bin/env python
from __future__ import print_function
from __future__ import absolute_import
from contextlib import contextmanager
import logging
import os
import re
import sys
import optparse
import subprocess
import traceback
try:
import lister
from typing import cast, Any, Callable, Dict, Iterator, List, Optional, Tuple
except ImportError as e:
print("ImportError: {}".format(e))
print("You need to run the Zulip linters inside a Zulip dev environment.")
print("If you are using Vagrant, you can `vagrant ssh` to enter the Vagrant guest.")
sys.exit(1)
# Exclude some directories and files from lint checking
EXCLUDED_FILES = [
# Third-party code that doesn't match our style
"api/integrations/perforce/git_p4.py",
"puppet/apt/.forge-release",
"puppet/apt/README.md",
"static/third",
# Transifex syncs translation.json files without trailing
# newlines; there's nothing other than trailing newlines we'd be
# checking for in these anyway.
"static/locale",
]
@contextmanager
def bright_red_output():
# type: () -> Iterator[None]
# Make the lint output bright red
sys.stdout.write('\x1B[1;31m')
sys.stdout.flush()
try:
yield
finally:
# Restore normal terminal colors
sys.stdout.write('\x1B[0m')
def check_pyflakes(options, by_lang):
# type: (Any, Dict[str, List[str]]) -> bool
if not by_lang['py']:
return False
failed = False
pyflakes = subprocess.Popen(['pyflakes'] + by_lang['py'],
stdout = subprocess.PIPE,
stderr = subprocess.PIPE,
universal_newlines = True)
# pyflakes writes some output (like syntax errors) to stderr. :/
for pipe in (pyflakes.stdout, pyflakes.stderr):
for ln in pipe:
if options.full or not (
('imported but unused' in ln or
'redefinition of unused' in ln or
# Our ipython startup pythonrc file intentionally imports *
("scripts/lib/pythonrc.py" in ln and
" import *' used; unable to detect undefined names" in ln) or
# Special dev_settings.py import
"from .prod_settings_template import *" in ln or
("settings.py" in ln and
("settings import *' used; unable to detect undefined names" in ln or
"may be undefined, or defined from star imports" in ln)) or
("zerver/tornado/ioloop_logging.py" in ln and
"redefinition of function 'instrument_tornado_ioloop'" in ln) or
("zephyr_mirror_backend.py:" in ln and
"redefinition of unused 'simplejson' from line" in ln))):
sys.stdout.write(ln)
failed = True
return failed
def check_pep8(files):
# type: (List[str]) -> bool
failed = False
ignored_rules = [
'E402', 'E501', 'W503', 'E711', 'E226',
'E126', 'E121', 'E123', 'E266', 'E265', 'E261', 'E221',
'E241', 'E712', 'E702', 'E401', 'E115', 'E114', 'E731', 'E302',
'E741', 'E714', 'W391', 'E713', 'E305', 'E251', 'E306',
]
pep8 = subprocess.Popen(
['pycodestyle'] + files + ['--ignore={rules}'.format(rules=','.join(ignored_rules))],
stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
for pipe in (pep8.stdout, pep8.stderr):
for ln in pipe:
sys.stdout.write(ln)
failed = True
return failed
def run_parallel(lint_functions):
# type: (Dict[str, Callable[[], int]]) -> bool
pids = []
for name, func in lint_functions.items():
pid = os.fork()
if pid == 0:
logging.info("start " + name)
result = func()
logging.info("finish " + name)
sys.stdout.flush()
sys.stderr.flush()
os._exit(result)
pids.append(pid)
failed = False
for pid in pids:
(_, status) = os.waitpid(pid, 0)
if status != 0:
failed = True
return failed
def build_custom_checkers(by_lang):
# type: (Dict[str, List[str]]) -> Tuple[Callable[[], bool], Callable[[], bool]]
RuleList = List[Dict[str, Any]]
def custom_check_file(fn, rules, skip_rules=None, max_length=None):
# type: (str, RuleList, Optional[Any], Optional[int]) -> bool
failed = False
lineFlag = False
for i, line in enumerate(open(fn)):
line_newline_stripped = line.strip('\n')
line_fully_stripped = line_newline_stripped.strip()
skip = False
lineFlag = True
for rule in skip_rules or []:
if re.match(rule, line):
skip = True
if skip:
continue
for rule in rules:
exclude_list = rule.get('exclude', set())
if fn in exclude_list or os.path.dirname(fn) in exclude_list:
continue
exclude_list = rule.get('exclude_line', set())
if (fn, line_fully_stripped) in exclude_list:
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(rule['pattern'], line_to_check):
sys.stdout.write(rule['description'] + ' at %s line %s:\n' % (fn, i+1))
print(line)
failed = True
except Exception:
print("Exception with %s at %s line %s" % (rule['pattern'], fn, i+1))
traceback.print_exc()
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
not re.match("\[[ A-Za-z0-9_:,&()-]*\]: http.*", line) and
"#ignorelongline" not in line and 'migrations' not in fn):
print("Line too long (%s) at %s line %s: %s" % (len(line), fn, i+1, line_newline_stripped))
failed = True
lastLine = line
if lineFlag and '\n' not in lastLine:
print("No newline at the end of file. Fix with `sed -i '$a\\' %s`" % (fn,))
failed = True
return failed
whitespace_rules = [
# This linter should be first since bash_rules depends on it.
{'pattern': '\s+$',
'strip': '\n',
'description': 'Fix trailing whitespace'},
{'pattern': '\t',
'strip': '\n',
'exclude': set(['zerver/lib/bugdown/codehilite.py',
'tools/travis/success-http-headers.txt']),
'description': 'Fix tab-based whitespace'},
] # type: RuleList
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',
'description': 'Missing space after # in heading'},
] # 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 {'},
{'pattern': '["\']json/',
'description': 'Relative URL for JSON route not supported by i18n'},
# This rule is constructed with + to avoid triggering on itself
{'pattern': " =" + '[^ =>~"]',
'description': 'Missing whitespace after "="'},
{'pattern': '^[ ]*//[A-Za-z0-9]',
'description': 'Missing space after // in comment'},
{'pattern': 'if[(]',
'description': 'Missing space between if and ('},
{'pattern': 'else{$',
'description': 'Missing space between else and {'},
{'pattern': '^else {$',
'description': 'Write JS else statements on same line as }'},
{'pattern': '^else if',
'description': 'Write JS else statements on same line as }'},
{'pattern': 'console[.][a-z]',
'exclude': set(['static/js/blueslip.js',
'frontend_tests/zjsunit',
'frontend_tests/casper_lib/common.js',
'frontend_tests/node_tests',
'static/js/debug.js']),
'description': 'console.log and similar should not be used in webapp'},
{'pattern': 'button\.text\(["\']',
'exclude': set(['tools/lint-all',
'frontend_tests/node_tests/templates.js']),
'description': 'Argument to button.text should be a literal string enclosed by i18n.t()'},
{'pattern': 'compose_error\(["\']',
'exclude': set(['tools/lint-all']),
'description': 'Argument to compose_error should be a literal string enclosed '
'by i18n.t()'},
{'pattern': 'report_success\(["\']',
'exclude': set(['tools/lint-all']),
'description': 'Argument to report_success should be a literal string enclosed '
'by i18n.t()'},
{'pattern': 'report_error\(["\']',
'exclude': set(['tools/lint-all']),
'description': 'Argument to report_error should be a literal string enclosed '
'by i18n.t()'},
]) + whitespace_rules
python_rules = cast(RuleList, [
{'pattern': '^(?!#)@login_required',
'description': '@login_required is unsupported; use @zulip_login_required'},
{'pattern': '".*"%\([a-z_].*\)?$',
'description': 'Missing space around "%"'},
{'pattern': "'.*'%\([a-z_].*\)?$",
'exclude': set(['tools/lint-all',
'analytics/lib/counts.py',
'analytics/tests/test_counts.py',
]),
'exclude_line': set([
('zerver/views/users.py',
"return json_error(_(\"Email '%(email)s' does not belong to domain '%(domain)s'\") %"),
('zproject/settings.py',
"'format': '%(asctime)s %(levelname)-8s %(message)s'"),
]),
'description': 'Missing space around "%"'},
# This rule is constructed with + to avoid triggering on itself
{'pattern': " =" + '[^ =>~"]',
'description': 'Missing whitespace after "="'},
{'pattern': '":\w[^"]*$',
'description': 'Missing whitespace after ":"'},
{'pattern': "':\w[^']*$",
'description': 'Missing whitespace after ":"'},
{'pattern': "^\s+[#]\w",
'strip': '\n',
'description': 'Missing whitespace after "#"'},
{'pattern': "== None",
'exclude': 'tools/lint-all',
'description': 'Use `is None` to check whether something is None'},
{'pattern': "type:[(]",
'description': 'Missing whitespace after ":" in type annotation'},
{'pattern': "# type [(]",
'description': 'Missing : after type in type annotation'},
{'pattern': "#type",
'exclude': 'tools/lint-all',
'description': 'Missing whitespace after "#" in type annotation'},
{'pattern': 'if[(]',
'description': 'Missing space between if and ('},
{'pattern': ", [)]",
'description': 'Unnecessary whitespace between "," and ")"'},
{'pattern': "% [(]",
'description': 'Unnecessary whitespace between "%" and "("'},
# This next check could have false positives, but it seems pretty
# rare; if we find any, they can be added to the exclude list for
# this rule.
{'pattern': '% [a-zA-Z0-9_.]*\)?$',
'exclude_line': set([
('tools/tests/test_template_parser.py', '{% foo'),
]),
'description': 'Used % comprehension without a tuple'},
{'pattern': '.*%s.* % \([a-zA-Z0-9_.]*\)$',
'description': 'Used % comprehension without a tuple'},
{'pattern': 'json_success\({}\)',
'description': 'Use json_success() to return nothing'},
# To avoid json_error(_variable) and json_error(_(variable))
{'pattern': '\Wjson_error\(_\(?\w+\)',
'exclude': set(['tools/lint-all']),
'description': 'Argument to json_error should be a literal string enclosed by _()'},
{'pattern': '\Wjson_error\([^_].+[),]$',
'exclude': set(['tools/lint-all']),
'exclude_line': set([
# function definition
('zerver/lib/response.py', 'def json_error(msg, data=None, status=400):'),
# No need to worry about the following as the translation strings
# are already captured
('zerver/middleware.py',
'return json_error(exception.to_json_error_msg(), status=status_code)'),
('zerver/tornado/views.py', 'return json_error(result["message"])'),
('zerver/views/invite.py',
'return json_error(data=error_data, msg=ret_error)'),
('zerver/views/streams.py', 'return json_error(property_conversion)'),
# We can't do anything about this.
('zerver/views/realm_emoji.py', 'return json_error(e.messages[0])'),
('zerver/views/realm_filters.py', 'return json_error(e.messages[0], data={"errors": dict(e)})'),
]),
'description': 'Argument to json_error should a literal string enclosed by _()'},
# To avoid JsonableError(_variable) and JsonableError(_(variable))
{'pattern': '\WJsonableError\(_\(?\w.+\)',
'exclude': set(['tools/lint-all']),
'description': 'Argument to JsonableError should be a literal string enclosed by _()'},
{'pattern': '\WJsonableError\([^_].+\)',
'exclude': set(['tools/lint-all']),
'exclude_line': set([
# class definition
('zerver/lib/request.py', 'class JsonableError(Exception):'),
# No need to worry about the following as the translation strings
# are already captured
('zerver/decorator.py', 'raise JsonableError(reason % (role,))'),
('zerver/lib/actions.py', 'raise JsonableError(e.messages[0])'),
('zerver/views/messages.py', 'raise JsonableError(error)'),
('zerver/lib/request.py', 'raise JsonableError(error)'),
('zerver/views/streams.py', 'raise JsonableError(response.content)'),
]),
'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)',
'exclude': set(['zilencer/tests.py']),
'description': \
'''Do not call self.client directly for put/patch/post/get.
See WRAPPER_COMMENT in test_helpers.py for details.
'''},
# This rule might give false positives in virtualenv setup files which should be excluded,
# and comments which should be rewritten to avoid use of "python2", "python3", etc.
{'pattern': 'python[23]',
'exclude': set(['tools/lib/provision.py',
'tools/setup/setup_venvs.py',
'scripts/lib/setup_venv.py',
'tools/lint-all']),
'description': 'Explicit python invocations should not include a version'}
]) + whitespace_rules
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'},
] + whitespace_rules[0:1] # type: RuleList
css_rules = cast(RuleList, [
{'pattern': '^[^:]*:\S[^:]*;$',
'description': "Missing whitespace after : in CSS"},
{'pattern': '[a-z]{',
'description': "Missing whitespace before '{' in CSS."},
{'pattern': 'https://',
'description': "Zulip CSS should have no dependencies on external resources"},
{'pattern': '^[ ][ ][a-zA-Z0-9]',
'description': "Incorrect 2-space indentation in CSS",
'exclude': set(['static/styles/thirdparty-fonts.css']),
'strip': '\n'},
{'pattern': '{\w',
'description': "Missing whitespace after '{' in CSS (should be newline)."},
]) + whitespace_rules # type: RuleList
prose_style_rules = [
{'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
'description': "Organization is spelled with a z"},
{'pattern': '!!! warning',
'description': "!!! warning is invalid; it's spelled '!!! warn'"},
] # type: RuleList
html_rules = whitespace_rules + prose_style_rules + [
{'pattern': 'placeholder="[^{]',
'description': "`placeholder` value should be translatable.",
'exclude': set(["static/templates/settings/emoji-settings-admin.handlebars",
"static/templates/settings/realm-filter-settings-admin.handlebars"])},
{'pattern': "placeholder='[^{]",
'description': "`placeholder` value should be translatable."},
{'pattern': "title='[^{]",
'description': "`title` value should be translatable."},
{'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>')
]),
'description': "`title` value should be translatable."},
] # type: RuleList
handlebars_rules = html_rules + [
{'pattern': "[<]script",
'description': "Do not use inline <script> tags here; put JavaScript in static/js instead."},
]
json_rules = [] # type: RuleList # fix newlines at ends of files
# It is okay that json_rules is empty, because the empty list
# ensures we'll still check JSON files for whitespace.
markdown_rules = markdown_whitespace_rules + prose_style_rules
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
for fn in by_lang['py']:
if custom_check_file(fn, python_rules, max_length=140):
failed = True
return failed
def check_custom_checks_nonpy():
# type: () -> bool
failed = False
for fn in by_lang['js']:
if custom_check_file(fn, js_rules):
failed = True
for fn in by_lang['sh']:
if custom_check_file(fn, bash_rules):
failed = True
for fn in by_lang['css']:
if custom_check_file(fn, css_rules):
failed = True
for fn in by_lang['handlebars']:
if custom_check_file(fn, handlebars_rules):
failed = True
for fn in by_lang['html']:
if custom_check_file(fn, html_rules):
failed = True
for fn in by_lang['json']:
if custom_check_file(fn, json_rules):
failed = True
markdown_docs_length_exclude = {
"contrib_bots/lib/ConverterBot/docs.md",
"docs/bots-guide.md",
"docs/dev-env-first-time-contributors.md",
"docs/integration-guide.md",
"docs/life-of-a-request.md",
"docs/logging.md",
"docs/migration-renumbering.md",
"docs/readme-symlink.md",
"README.md",
}
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
if custom_check_file(fn, rules, max_length=max_length):
failed = True
for fn in by_lang['txt'] + by_lang['text']:
if custom_check_file(fn, txt_rules):
failed = True
return failed
return (check_custom_checks_py, check_custom_checks_nonpy)
def run():
# type: () -> None
parser = optparse.OptionParser()
parser.add_option('--force', default=False,
action="store_true",
help='Run tests despite possible problems.')
parser.add_option('--full',
action='store_true',
help='Check some things we typically ignore')
parser.add_option('--pep8',
action='store_true',
help='Run the pep8 checker')
parser.add_option('--modified', '-m',
action='store_true',
help='Only check modified files')
parser.add_option('--verbose', '-v',
action='store_true',
help='Print verbose timing output')
(options, args) = parser.parse_args()
tools_dir = os.path.dirname(os.path.abspath(__file__))
root_dir = os.path.dirname(tools_dir)
sys.path.insert(0, root_dir)
from tools.lib.test_script import (
get_provisioning_status,
)
os.chdir(root_dir)
if not options.force:
ok, msg = get_provisioning_status()
if not ok:
print(msg)
print('If you really know what you are doing, use --force to run anyway.')
sys.exit(1)
by_lang = cast(Dict[str, List[str]],
lister.list_files(args, modified_only=options.modified,
ftypes=['py', 'sh', 'js', 'pp', 'css', 'handlebars',
'html', 'json', 'md', 'txt', 'text'],
use_shebang=True, group_by_ftype=True, exclude=EXCLUDED_FILES))
# Invoke the appropriate lint checker for each language,
# and also check files for extra whitespace.
logging.basicConfig(format="%(asctime)s %(message)s")
logger = logging.getLogger()
if options.verbose:
logger.setLevel(logging.INFO)
else:
logger.setLevel(logging.WARNING)
check_custom_checks_py, check_custom_checks_nonpy = build_custom_checkers(by_lang)
lint_functions = {} # type: Dict[str, Callable[[], int]]
def lint(func):
# type: (Callable[[], int]) -> Callable[[], int]
lint_functions[func.__name__] = func
return func
with bright_red_output():
@lint
def check_urls():
# type: () -> int
result = subprocess.call(['tools/check-urls'])
return result
@lint
def templates():
# type: () -> int
args = ['tools/check-templates']
if options.modified:
args.append('-m')
result = subprocess.call(args)
return result
@lint
def add_class():
# type: () -> int
result = subprocess.call(['tools/find-add-class'])
return result
@lint
def css():
# type: () -> int
result = subprocess.call(['tools/check-css'])
return result
@lint
def eslint():
# type: () -> int
if len(by_lang['js']) == 0:
return 0
result = subprocess.call(['node', 'node_modules/.bin/eslint', '--quiet']
+ by_lang['js'])
return result
@lint
def puppet():
# type: () -> int
if not by_lang['pp']:
return 0
result = subprocess.call(['puppet', 'parser', 'validate'] + by_lang['pp'])
return result
@lint
def custom_py():
# type: () -> int
failed = check_custom_checks_py()
return 1 if failed else 0
@lint
def custom_nonpy():
# type: () -> int
failed = check_custom_checks_nonpy()
return 1 if failed else 0
@lint
def pyflakes():
# type: () -> int
failed = check_pyflakes(options, by_lang)
return 1 if failed else 0
if options.pep8:
@lint
def pep8():
# type: () -> int
failed = check_pep8(by_lang['py'])
return 1 if failed else 0
failed = run_parallel(lint_functions)
sys.exit(1 if failed else 0)
if __name__ == '__main__':
run()