From bce90a3340d10f761a6e7eb88d94a595df103f3d Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Mon, 17 Dec 2018 10:44:24 -0800 Subject: [PATCH] lint: Add lint rule for scripts importing typing improperly. This is a common bug that users might be tempated to introduce. And also fix two instances of this bug that were present in our codebase, including an important one in our upgrade code path. --- puppet/zulip/files/postgresql/pg_backup_and_purge | 6 ++++-- scripts/lib/upgrade-zulip | 3 +-- tools/linter_lib/custom_check.py | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/puppet/zulip/files/postgresql/pg_backup_and_purge b/puppet/zulip/files/postgresql/pg_backup_and_purge index 2c6c7da868..c11497046e 100755 --- a/puppet/zulip/files/postgresql/pg_backup_and_purge +++ b/puppet/zulip/files/postgresql/pg_backup_and_purge @@ -10,13 +10,15 @@ import dateutil.parser import pytz import time from datetime import datetime, timedelta -from typing import Dict, List +if False: + from typing import Dict, List logging.Formatter.converter = time.gmtime logging.basicConfig(format="%(asctime)s %(levelname)s: %(message)s") logger = logging.getLogger(__name__) -def run(args: List[str], dry_run: bool=False) -> str: +def run(args, dry_run=False): + # type: (List[str], bool) -> str if dry_run: print("Would have run: " + " ".join(map(shlex.quote, args))) return "" diff --git a/scripts/lib/upgrade-zulip b/scripts/lib/upgrade-zulip index 92eca7a7d5..b5b2175e36 100755 --- a/scripts/lib/upgrade-zulip +++ b/scripts/lib/upgrade-zulip @@ -6,7 +6,6 @@ import subprocess import logging import time import configparser -from typing import List TARBALL_ARCHIVE_PATH = "/home/zulip/archives" os.environ["PYTHONUNBUFFERED"] = "y" @@ -17,7 +16,7 @@ from scripts.lib.zulip_tools import DEPLOYMENTS_DIR, FAIL, WARNING, ENDC, \ get_config_file, get_deploy_options config_file = get_config_file() # type: configparser.RawConfigParser -deploy_options = get_deploy_options(config_file) # type: List[str] +deploy_options = get_deploy_options(config_file) assert_running_as_root(strip_lib_from_paths=True) diff --git a/tools/linter_lib/custom_check.py b/tools/linter_lib/custom_check.py index 9a0346dd18..37a4be13fb 100644 --- a/tools/linter_lib/custom_check.py +++ b/tools/linter_lib/custom_check.py @@ -479,6 +479,21 @@ def build_custom_checkers(by_lang): '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': '^from typing import', + 'strip': '\n', + 'include_only': set([ + 'scripts/', + 'puppet/', + ]), + 'exclude': set([ + # Not important, but should fix + 'scripts/lib/process-mobile-i18n', + # Uses setup_path_on_import before importing. + 'puppet/zulip/files/nagios_plugins/zulip_app_frontend/check_send_receive_time', + ]), + 'description': 'For scripts run as part of installer, cannot rely on typing existing; use `if False` workaround.', + 'good_lines': ['subprocess.check_call(["ls"])'], + 'bad_lines': ['subprocess.check_call(["sudo", "ls"])']}, {'pattern': 'django.utils.translation', 'include_only': set(['test/']), 'description': 'Test strings should not be tagged for translation',