diff --git a/requirements.txt b/requirements.txt index a22c3a9..ebd5adb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,3 +7,4 @@ pytest -e ./zulip -e ./zulip_bots -e ./zulip_botserver +-e git+https://github.com/zulip/zulint@aaed679f1ad38b230090eadd3870b7682500f60c#egg=zulint==1.0.0 diff --git a/tools/custom_check.py b/tools/custom_check.py index c8bd3d6..0983e0a 100644 --- a/tools/custom_check.py +++ b/tools/custom_check.py @@ -1,126 +1,36 @@ from __future__ import print_function from __future__ import absolute_import -import os -import re -import traceback +from typing import cast, Any, Dict, List, Tuple +from zulint.custom_rules import RuleList -from server_lib.printer import print_err, colors +Rule = List[Dict[str, Any]] -from typing import cast, Any, Callable, Dict, List, Optional, Tuple +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', + 'description': 'Fix tab-based whitespace'}, +] # type: Rule -def build_custom_checkers(by_lang): - # type: (Dict[str, List[str]]) -> Tuple[Callable[[], bool], Callable[[], bool]] - RuleList = List[Dict[str, Any]] +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': '((? bool - failed = False - color = next(colors) - - line_tups = [] - for i, line in enumerate(open(fn)): - line_newline_stripped = line.strip('\n') - line_fully_stripped = line_newline_stripped.strip() - skip = False - for rule in skip_rules or []: - if re.match(rule, line): - 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 = [] - fn_dirname = os.path.dirname(fn) - for rule in rules: - exclude_list = rule.get('exclude', set()) - if fn in exclude_list or fn_dirname in exclude_list: - 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: - continue - 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'] - for (i, line, line_newline_stripped, line_fully_stripped) in line_tups: - 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): - 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)) - - 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 - not re.match("\[[ A-Za-z0-9_:,&()-]*\]: http.*", line) and - not re.match("`\{\{ external_api_uri_subdomain \}\}[^`]+`", 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 lastLine 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', - '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': '((?]([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'"}, - {'pattern': '[^-_]botserver(?!rc)|bot server', - 'description': "Use Botserver instead of botserver or Botserver."}, - ] # type: RuleList - 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 + [ + ]) + whitespace_rules[0:1], +) + + +json_rules = RuleList( + langs=['json'], + # Here, we don't check tab-based whitespace, 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). + rules= whitespace_rules[0:1], +) + +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'"}, + {'pattern': '[^-_]botserver(?!rc)|bot server', + 'description': "Use Botserver instead of botserver or Botserver."}, +] # type: Rule + +markdown_docs_length_exclude = { + "zulip_bots/zulip_bots/bots/converter/doc.md", + "tools/server_lib/README.md", +} + +markdown_rules = RuleList( + langs=['md'], + rules=markdown_whitespace_rules + prose_style_rules + cast(Rule, [ {'pattern': '\[(?P[^\]]+)\]\((?P=url)\)', 'description': 'Linkified markdown URLs should use cleaner syntax.'} - ] - 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 + ]), + max_length=120, + length_exclude=markdown_docs_length_exclude, +) - def check_custom_checks_py(): - # type: () -> bool - failed = False +txt_rules = RuleList( + langs=['txt'], + rules=whitespace_rules, +) - for fn in by_lang['py']: - if 'custom_check.py' in fn: - continue - if custom_check_file(fn, 'py', python_rules, max_length=140): - failed = True - return failed - - def check_custom_checks_nonpy(): - # type: () -> bool - failed = False - - for fn in by_lang['sh']: - if custom_check_file(fn, 'sh', bash_rules): - failed = True - - for fn in by_lang['json']: - if custom_check_file(fn, 'json', json_rules): - failed = True - - markdown_docs_length_exclude = { - "zulip_bots/zulip_bots/bots/converter/doc.md", - "tools/server_lib/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, 'md', rules, max_length=max_length): - failed = True - - for fn in by_lang['txt'] + by_lang['text']: - if custom_check_file(fn, 'txt', txt_rules): - failed = True - - for fn in by_lang['yaml']: - if custom_check_file(fn, 'yaml', txt_rules): - failed = True - - return failed - - return (check_custom_checks_py, check_custom_checks_nonpy) +non_py_rules = [ + json_rules, + markdown_rules, + bash_rules, + txt_rules, +] diff --git a/tools/lint b/tools/lint index 17d098b..28ba647 100755 --- a/tools/lint +++ b/tools/lint @@ -1,39 +1,54 @@ #! /usr/bin/env python -from pep8 import check_pep8 -from custom_check import build_custom_checkers -from server_lib import lister +from __future__ import print_function +from __future__ import absolute_import +import argparse -import sys -import optparse -from typing import cast, Callable, Dict, Iterator, List +from zulint.command import add_default_linter_arguments, LinterConfig + +from custom_check import python_rules, non_py_rules +from pep8 import check_pep8 EXCLUDED_FILES = [ # This is an external file that doesn't comply with our codestyle 'zulip/integrations/perforce/git_p4.py', ] -def lint_all(args, options): - - 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', 'yaml'], - use_shebang=True, group_by_ftype=True, exclude=EXCLUDED_FILES)) - check_custom_checks_py, check_custom_checks_nonpy = build_custom_checkers(by_lang) - return any([check_pep8(by_lang['py']), - check_custom_checks_py(), - check_custom_checks_nonpy()]) - def run(): # type: () -> None - parser = optparse.OptionParser() - parser.add_option('--modified', '-m', - action='store_true', - help='Only check modified files') - (options, args) = parser.parse_args() - failed = lint_all(args, options) - sys.exit(1 if failed else 0) + parser = argparse.ArgumentParser() + add_default_linter_arguments(parser) + args = parser.parse_args() + + linter_config = LinterConfig(args) + + by_lang = linter_config.list_files(file_types=['py', 'sh', 'json', 'md', 'txt'], + exclude=EXCLUDED_FILES) + + @linter_config.lint + def custom_py(): + # type: () -> int + """Runs custom checks for python files (config: tools/linter_lib/custom_check.py)""" + failed = python_rules.check(by_lang, verbose=args.verbose) + return 1 if failed else 0 + + @linter_config.lint + def custom_nonpy(): + # type: () -> int + """Runs custom checks for non-python files (config: tools/linter_lib/custom_check.py)""" + failed = False + for rule in non_py_rules: + failed = failed or rule.check(by_lang, verbose=args.verbose) + return 1 if failed else 0 + + @linter_config.lint + def pep8(): + # type: () -> int + """Standard Python style linter on 50% of files (config: tools/linter_lib/pep8.py)""" + failed = check_pep8(by_lang['py']) + return 1 if failed else 0 + + linter_config.do_lint() if __name__ == '__main__': run()