From e9ff9e34b342d69fd37e2647053a53a71159d45d Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Fri, 21 Jun 2019 22:10:38 +0530 Subject: [PATCH] lint: Use --groups to specify specific groups to run. This helps generalize the use of groups inside zulint. Introduce list_files to return `by_lang` files dict. Add feature to create custom groups. Make custom groups for backend and frontend files. --- tools/ci/backend | 2 +- tools/ci/frontend | 2 +- tools/lint | 30 +++++++----------------------- tools/test-all | 4 ++-- tools/zulint/README.md | 7 +++++-- tools/zulint/command.py | 28 ++++++++++++++++++++++++---- 6 files changed, 40 insertions(+), 33 deletions(-) diff --git a/tools/ci/backend b/tools/ci/backend index 1ea7251972..e180cf3343 100755 --- a/tools/ci/backend +++ b/tools/ci/backend @@ -6,7 +6,7 @@ echo "Test suite is running under $(python --version)." set -e set -x -./tools/lint --backend --skip=gitlint,mypy # gitlint disabled because flaky +./tools/lint --groups=backend --skip=gitlint,mypy # gitlint disabled because flaky ./tools/test-tools # We need to pass a parallel level to test-backend because CircleCI's # docker setup means the auto-detection logic sees the ~36 processes diff --git a/tools/ci/frontend b/tools/ci/frontend index 22e0866f39..d4448510d4 100755 --- a/tools/ci/frontend +++ b/tools/ci/frontend @@ -5,7 +5,7 @@ source tools/ci/activate-venv set -e set -x -./tools/lint --frontend --skip=gitlint # gitlint disabled because flaky +./tools/lint --groups=frontend --skip=gitlint # gitlint disabled because flaky # Run the node tests first, since they're fast and deterministic ./tools/test-js-with-node --coverage diff --git a/tools/lint b/tools/lint index 25c50dafac..559f3ac7b4 100755 --- a/tools/lint +++ b/tools/lint @@ -10,9 +10,7 @@ from lib import sanity_check sanity_check.check_venv(__file__) from linter_lib.custom_check import python_rules, non_py_rules -from zulint import lister from zulint.command import add_default_linter_arguments, LinterConfig -from typing import cast, Dict, List import random def run(): @@ -24,13 +22,6 @@ def run(): parser.add_argument('--full', action='store_true', help='Check some things we typically ignore') - limited_tests_group = parser.add_mutually_exclusive_group() - limited_tests_group.add_argument('--frontend', - action='store_true', - help='Only check files relevant to frontend') - limited_tests_group.add_argument('--backend', - action='store_true', - help='Only check files relevant to backend') add_default_linter_arguments(parser) args = parser.parse_args() @@ -50,23 +41,16 @@ def run(): assert_provisioning_status_ok(args.force) - backend_file_types = ['py', 'sh', 'pp', 'json', 'md', 'txt', 'text', 'yaml', 'rst'] - frontend_file_types = ['js', 'ts', 'css', 'scss', 'handlebars', 'html'] - file_types = backend_file_types + frontend_file_types - if args.backend: - file_types = backend_file_types - if args.frontend: - file_types = frontend_file_types - - by_lang = cast(Dict[str, List[str]], - lister.list_files(args.targets, modified_only=args.modified, - ftypes=file_types, - 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. - linter_config = LinterConfig(args, by_lang) + linter_config = LinterConfig(args) + + by_lang = linter_config.list_files(groups={ + 'backend': ['py', 'sh', 'pp', 'json', 'md', 'txt', 'text', 'yaml', 'rst'], + 'frontend': ['js', 'ts', 'css', 'scss', 'handlebars', 'html'], + }, exclude=EXCLUDED_FILES) + linter_config.external_linter('add_class', ['tools/find-add-class'], ['js']) linter_config.external_linter('css', ['node', 'node_modules/.bin/stylelint'], ['css', 'scss']) linter_config.external_linter('eslint', ['node', 'node_modules/.bin/eslint', diff --git a/tools/test-all b/tools/test-all index d91111f4b8..f886be48f3 100755 --- a/tools/test-all +++ b/tools/test-all @@ -39,7 +39,7 @@ run ./tools/check-provision $FORCEARG run ./tools/clean-repo # ci/backend -run ./tools/lint --backend $FORCEARG +run ./tools/lint --groups=backend $FORCEARG run ./tools/test-tools run ./tools/test-backend --include-webhooks $FORCEARG run ./tools/test-migrations @@ -57,7 +57,7 @@ run ./tools/test-api # run ./tools/test-queue-worker-reload # ci/frontend -run ./tools/lint --frontend $FORCEARG +run ./tools/lint --groups=frontend $FORCEARG run ./tools/test-js-with-node run ./manage.py makemessages --locale en run env PYTHONWARNINGS=ignore ./tools/check-capitalization --no-generate diff --git a/tools/zulint/README.md b/tools/zulint/README.md index 959d4591d9..b2f4d6411d 100644 --- a/tools/zulint/README.md +++ b/tools/zulint/README.md @@ -60,8 +60,8 @@ script with at least the following options: ``` (zulip-py3-venv) tabbott@coset:~/zulip$ ./tools/lint --help -usage: lint [-h] [--force] [--full] [--frontend | --backend] [--modified] - [--verbose-timing] [--skip SKIP] [--only ONLY] [--list] +usage: lint [-h] [--force] [--full] [--modified] [--verbose-timing] + [--skip SKIP] [--only ONLY] [--list] [--groups GROUPS] [targets [targets ...]] positional arguments: @@ -76,6 +76,9 @@ optional arguments: --skip SKIP Specify linters to skip, eg: --skip=mypy,gitlint --only ONLY Specify linters to run, eg: --only=mypy,gitlint --list, -l List all the registered linters + --groups GROUPS, -g GROUPS + Only run linter for languages in the group(s), e.g.: + --groups=backend,other_group ``` ### pre-commit hook mode diff --git a/tools/zulint/command.py b/tools/zulint/command.py index 5bfc03b4a1..fa098d1e26 100644 --- a/tools/zulint/command.py +++ b/tools/zulint/command.py @@ -11,9 +11,10 @@ import sys if False: # See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts - from typing import Any, Callable, Dict, List + from typing import Callable, Dict, List from zulint.printer import print_err, colors +from zulint import lister def add_default_linter_arguments(parser): # type: (argparse.ArgumentParser) -> None @@ -37,6 +38,11 @@ def add_default_linter_arguments(parser): parser.add_argument('--list', '-l', action='store_true', help='List all the registered linters') + parser.add_argument('--groups', '-g', + default=[], + type=split_arg_into_list, + help='Only run linter for languages in the group(s), e.g.: ' + '--groups=backend,frontend') def split_arg_into_list(arg): # type: (str) -> List[str] @@ -66,10 +72,24 @@ def run_parallel(lint_functions): class LinterConfig: lint_functions = {} # type: Dict[str, Callable[[], int]] - def __init__(self, args, by_lang): - # type: (argparse.Namespace, Any) -> None + def __init__(self, args): + # type: (argparse.Namespace) -> None self.args = args - self.by_lang = by_lang # type: Dict[str, List[str]] + self.by_lang = {} # type: Dict[str, List[str]] + + def list_files(self, file_types=[], groups={}, use_shebang=True, group_by_ftype=True, exclude=[]): + # type: (List[str], Dict[str, List[str]], bool, bool, List[str]) -> Dict[str, List[str]] + assert file_types or groups, "Atleast one of `file_types` or `groups` must be specified." + + if self.args.groups: + file_types = [ft for group in self.args.groups for ft in groups[group]] + else: + file_types.extend({ft for group in groups.values() for ft in group}) + + self.by_lang = lister.list_files(self.args.targets, modified_only=self.args.modified, + ftypes=file_types, use_shebang=use_shebang, + group_by_ftype=group_by_ftype, exclude=exclude) + return self.by_lang def lint(self, func): # type: (Callable[[], int]) -> Callable[[], int]