From 8cdf2801f745ace73c4f9dde5559f59b8f472017 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Fri, 8 May 2020 15:10:17 -0700 Subject: [PATCH] python: Convert more variable type annotations to Python 3.6 style. Signed-off-by: Anders Kaseorg --- analytics/views.py | 2 +- docs/documentation/api.md | 4 +- docs/testing/mypy.md | 8 +- docs/tutorials/new-feature-tutorial.md | 21 ++-- docs/tutorials/writing-views.md | 36 ++---- .../files/postgresql/pg_backup_and_purge | 2 +- templates/zerver/api/writing-bots.md | 2 +- tools/linter_lib/custom_check.py | 28 ++--- tools/setup/emoji/import_emoji_names_from_csv | 4 +- zerver/lib/alert_words.py | 2 +- zerver/lib/exceptions.py | 2 +- ...ssed_message_addresses_from_redis_to_db.py | 5 +- zerver/migrations/0277_migrate_alert_word.py | 2 +- zerver/models.py | 6 +- zerver/tests/test_auth_backends.py | 104 ++++++++++-------- zerver/tests/test_slack_importer.py | 3 +- zerver/webhooks/beanstalk/view.py | 4 +- 17 files changed, 122 insertions(+), 113 deletions(-) diff --git a/analytics/views.py b/analytics/views.py index 2145127a6a..941ea9f067 100644 --- a/analytics/views.py +++ b/analytics/views.py @@ -1024,7 +1024,7 @@ def ad_hoc_queries() -> List[Dict[str, str]]: @require_server_admin @has_request_variables def get_activity(request: HttpRequest) -> HttpResponse: - duration_content, realm_minutes = user_activity_intervals() # type: Tuple[mark_safe, Dict[str, float]] + duration_content, realm_minutes = user_activity_intervals() counts_content: str = realm_summary_table(realm_minutes) data = [ ('Counts', counts_content), diff --git a/docs/documentation/api.md b/docs/documentation/api.md index 0167e49fb3..a4d6fd81e1 100644 --- a/docs/documentation/api.md +++ b/docs/documentation/api.md @@ -102,9 +102,7 @@ automatically in Zulip's automated test suite. The code there will look something like this: ``` python -def render_message(client): - # type: (Client) -> None - +def render_message(client: Client) -> None: # {code_example|start} # Render a message request = { diff --git a/docs/testing/mypy.md b/docs/testing/mypy.md index 90ca139d43..f6db236929 100644 --- a/docs/testing/mypy.md +++ b/docs/testing/mypy.md @@ -3,12 +3,12 @@ [mypy](http://mypy-lang.org/) is a compile-time static type checker for Python, allowing optional, gradual typing of Python code. Zulip was fully annotated with mypy's Python 2 syntax in 2016, before our -migration to Python 3 in late 2017. In 2018, we migrated essentially -the entire codebase to the nice PEP-484 (Python 3 only) syntax for -static types: +migration to Python 3 in late 2017. In 2018 and 2020, we migrated +essentially the entire codebase to the nice PEP 484 (Python 3 only) +and PEP 526 (Python 3.6) syntax for static types: ``` -user_dict = {} # type: Dict[str, UserProfile] +user_dict: Dict[str, UserProfile] = {} def get_user(email: str, realm: Realm) -> UserProfile: ... # Actual code of the function here diff --git a/docs/tutorials/new-feature-tutorial.md b/docs/tutorials/new-feature-tutorial.md index 7c817922cb..b3824321a1 100644 --- a/docs/tutorials/new-feature-tutorial.md +++ b/docs/tutorials/new-feature-tutorial.md @@ -173,9 +173,9 @@ boolean field, `mandatory_topics`, to the Realm model in class Realm(models.Model): # ... - emails_restricted_to_domains = models.BooleanField(default=True) # type: bool - invite_required = models.BooleanField(default=False) # type: bool -+ mandatory_topics = models.BooleanField(default=False) # type: bool + emails_restricted_to_domains: bool = models.BooleanField(default=True) + invite_required: bool = models.BooleanField(default=False) ++ mandatory_topics: bool = models.BooleanField(default=False) ``` The Realm model also contains an attribute, `property_types`, which @@ -405,12 +405,15 @@ annotation). # zerver/views/realm.py -def update_realm(request, user_profile, name=REQ(validator=check_string, default=None), - # ..., -+ mandatory_topics=REQ(validator=check_bool, default=None), - # ...): -+ # type: (HttpRequest, UserProfile, ..., Optional[bool], ... - # ... + def update_realm( + request: HttpRequest, + user_profile: UserProfile, + name: Optional[str] = REQ(validator=check_string, default=None), + # ... ++ mandatory_topics: Optional[bool] = REQ(validator=check_bool, default=None), + # ... + ): + # ... ``` If this feature fits the `property_types` framework and does diff --git a/docs/tutorials/writing-views.md b/docs/tutorials/writing-views.md index ffdf904889..0beb46e023 100644 --- a/docs/tutorials/writing-views.md +++ b/docs/tutorials/writing-views.md @@ -68,8 +68,7 @@ views, as an introduction to view decorators. ```py @require_post -def accounts_register(request): - # type: (HttpRequest) -> HttpResponse +def accounts_register(request: HttpRequest) -> HttpResponse: ``` This decorator ensures that the request was a POST--here, we're @@ -91,8 +90,7 @@ specific to Zulip. ```py @zulip_login_required -def home(request): - # type: (HttpRequest) -> HttpResponse +def home(request: HttpRequest) -> HttpResponse: ``` [login-required-link]: https://docs.djangoproject.com/en/1.8/topics/auth/default/#django.contrib.auth.decorators.login_required @@ -261,29 +259,15 @@ For example, in [zerver/views/realm.py](https://github.com/zulip/zulip/blob/mast ```py @require_realm_admin @has_request_variables -def update_realm(request, user_profile, name=REQ(validator=check_string, default=None), ...)): - # type: (HttpRequest, UserProfile, ...) -> HttpResponse +def update_realm( + request: HttpRequest, user_profile: UserProfile, + name: Optional[str]=REQ(validator=check_string, default=None), + # ... +): realm = user_profile.realm - data = {} # type: Dict[str, Any] - if name is not None and realm.name != name: - do_set_realm_name(realm, name) - data['name'] = 'updated' -``` - -and in [zerver/lib/actions.py](https://github.com/zulip/zulip/blob/master/zerver/lib/actions.py): - -```py -def do_set_realm_name(realm, name): - # type: (Realm, str) -> None - realm.name = name - realm.save(update_fields=['name']) - event = dict( - type="realm", - op="update", - property='name', - value=name, - ) - send_event(realm, event, active_user_ids(realm)) + # ... + do_set_realm_property(realm, k, v) + # ... ``` `realm.save()` actually saves the changes to the realm to the diff --git a/puppet/zulip/files/postgresql/pg_backup_and_purge b/puppet/zulip/files/postgresql/pg_backup_and_purge index 7e15cbc3cb..ad873e4ba9 100755 --- a/puppet/zulip/files/postgresql/pg_backup_and_purge +++ b/puppet/zulip/files/postgresql/pg_backup_and_purge @@ -53,7 +53,7 @@ with open('/var/lib/nagios_state/last_postgres_backup', 'w') as f: f.write(now.isoformat()) f.write("\n") -backups = {} # type: Dict[datetime, str] +backups: Dict[datetime, str] = {} lines = run(['env-wal-e', 'backup-list']).split("\n") for line in lines[1:]: if line: diff --git a/templates/zerver/api/writing-bots.md b/templates/zerver/api/writing-bots.md index bc4376b2ea..8cc9ba00d3 100644 --- a/templates/zerver/api/writing-bots.md +++ b/templates/zerver/api/writing-bots.md @@ -392,7 +392,7 @@ refactor them. from zulip_bots.test_lib import StubBotTestCase class TestHelpBot(StubBotTestCase): - bot_name = "helloworld" # type: str + bot_name: str = "helloworld" def test_bot(self) -> None: dialog = [ diff --git a/tools/linter_lib/custom_check.py b/tools/linter_lib/custom_check.py index e9b676b900..06d2044178 100644 --- a/tools/linter_lib/custom_check.py +++ b/tools/linter_lib/custom_check.py @@ -417,26 +417,26 @@ python_rules = RuleList( }, 'description': 'Comment-style function type annotation. Use Python3 style annotations instead.', }, - {'pattern': r' = models[.].*null=True.*\) # type: (?!Optional)', + {'pattern': r': *(?!Optional)[^ ].*= models[.].*null=True', 'include_only': {"zerver/models.py"}, 'description': 'Model variable with null=true not annotated as Optional.', - 'good_lines': ['desc = models.TextField(null=True) # type: Optional[Text]', - 'stream = models.ForeignKey(Stream, null=True, on_delete=CASCADE) # type: Optional[Stream]', - 'desc = models.TextField() # type: Text', - 'stream = models.ForeignKey(Stream, on_delete=CASCADE) # type: Stream'], - 'bad_lines': ['desc = models.CharField(null=True) # type: Text', - 'stream = models.ForeignKey(Stream, null=True, on_delete=CASCADE) # type: Stream'], + 'good_lines': ['desc: Optional[Text] = models.TextField(null=True)', + 'stream: Optional[Stream] = models.ForeignKey(Stream, null=True, on_delete=CASCADE)', + 'desc: Text = models.TextField()', + 'stream: Stream = models.ForeignKey(Stream, on_delete=CASCADE)'], + 'bad_lines': ['desc: Text = models.CharField(null=True)', + 'stream: Stream = models.ForeignKey(Stream, null=True, on_delete=CASCADE)'], }, - {'pattern': r' = models[.].* # type: Optional', # Optional tag + {'pattern': r': *Optional.*= models[.].*\)', 'exclude_pattern': 'null=True', 'include_only': {"zerver/models.py"}, 'description': 'Model variable annotated with Optional but variable does not have null=true.', - 'good_lines': ['desc = models.TextField(null=True) # type: Optional[Text]', - 'stream = models.ForeignKey(Stream, null=True, on_delete=CASCADE) # type: Optional[Stream]', - 'desc = models.TextField() # type: Text', - 'stream = models.ForeignKey(Stream, on_delete=CASCADE) # type: Stream'], - 'bad_lines': ['desc = models.TextField() # type: Optional[Text]', - 'stream = models.ForeignKey(Stream, on_delete=CASCADE) # type: Optional[Stream]'], + 'good_lines': ['desc: Optional[Text] = models.TextField(null=True)', + 'stream: Optional[Stream] = models.ForeignKey(Stream, null=True, on_delete=CASCADE)', + 'desc: Text = models.TextField()', + 'stream: Stream = models.ForeignKey(Stream, on_delete=CASCADE)'], + 'bad_lines': ['desc: Optional[Text] = models.TextField()', + 'stream: Optional[Stream] = models.ForeignKey(Stream, on_delete=CASCADE)'], }, {'pattern': r'[\s([]Text([^\s\w]|$)', 'exclude': { diff --git a/tools/setup/emoji/import_emoji_names_from_csv b/tools/setup/emoji/import_emoji_names_from_csv index b0c13d6f80..e6d0ed78f6 100755 --- a/tools/setup/emoji/import_emoji_names_from_csv +++ b/tools/setup/emoji/import_emoji_names_from_csv @@ -30,9 +30,9 @@ INACTIVE_ENTRY = ( FILE_TEMPLATE = ( "from typing import Any, Dict\n\n" - "EMOJI_NAME_MAPS = {" + "EMOJI_NAME_MAPS: Dict[str, Dict[str, Any]] = {" "%(emoji_entries)s\n" - "} # type: Dict[str, Dict[str, Any]]\n" + "}\n" ) emoji_names: Set[str] = set() diff --git a/zerver/lib/alert_words.py b/zerver/lib/alert_words.py index 28166f5107..51c4c49794 100644 --- a/zerver/lib/alert_words.py +++ b/zerver/lib/alert_words.py @@ -10,7 +10,7 @@ from typing import Dict, Iterable, List def alert_words_in_realm(realm: Realm) -> Dict[int, List[str]]: user_ids_and_words = AlertWord.objects.filter( realm=realm, user_profile__is_active=True).values("user_profile_id", "word") - user_ids_with_words = dict() # type: Dict[int, List[str]] + user_ids_with_words: Dict[int, List[str]] = dict() for id_and_word in user_ids_and_words: user_ids_with_words.setdefault(id_and_word["user_profile_id"], []) user_ids_with_words[id_and_word["user_profile_id"]].append(id_and_word["word"]) diff --git a/zerver/lib/exceptions.py b/zerver/lib/exceptions.py index b9c68ef617..514ad13afa 100644 --- a/zerver/lib/exceptions.py +++ b/zerver/lib/exceptions.py @@ -68,7 +68,7 @@ class JsonableError(Exception): data_fields = ['widget_name'] def __init__(self, widget_name: str) -> None: - self.widget_name = widget_name # type: str + self.widget_name: str = widget_name @staticmethod def msg_format() -> str: diff --git a/zerver/migrations/0260_missed_message_addresses_from_redis_to_db.py b/zerver/migrations/0260_missed_message_addresses_from_redis_to_db.py index c0b35c154b..78bb24026e 100644 --- a/zerver/migrations/0260_missed_message_addresses_from_redis_to_db.py +++ b/zerver/migrations/0260_missed_message_addresses_from_redis_to_db.py @@ -33,7 +33,10 @@ def move_missed_message_addresses_to_database(apps: StateApps, schema_editor: Da redis_client.delete(key) continue - user_profile_id, recipient_id, subject_b = result # type: (bytes, bytes, bytes) + user_profile_id: bytes + recipient_id: bytes + subject_id: bytes + user_profile_id, recipient_id, subject_b = result topic_name = subject_b.decode('utf-8') # The data model for missed-message emails has changed in two diff --git a/zerver/migrations/0277_migrate_alert_word.py b/zerver/migrations/0277_migrate_alert_word.py index 757b8c4678..2ed1d3422f 100644 --- a/zerver/migrations/0277_migrate_alert_word.py +++ b/zerver/migrations/0277_migrate_alert_word.py @@ -28,7 +28,7 @@ def move_back_to_user_profile(apps: StateApps, schema_editor: DatabaseSchemaEdit UserProfile = apps.get_model('zerver', 'UserProfile') user_ids_and_words = AlertWord.objects.all().values("user_profile_id", "word") - user_ids_with_words = dict() # type: Dict[int, List[str]] + user_ids_with_words: Dict[int, List[str]] = dict() for id_and_word in user_ids_and_words: user_ids_with_words.setdefault(id_and_word["user_profile_id"], []) diff --git a/zerver/models.py b/zerver/models.py index 82f57801c2..4013834bfb 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -2894,10 +2894,10 @@ class AlertWord(models.Model): # never move to another realm, so it's static, and having Realm # here optimizes the main query on this table, which is fetching # all the alert words in a realm. - realm = models.ForeignKey(Realm, db_index=True, on_delete=CASCADE) # type: Realm - user_profile = models.ForeignKey(UserProfile, on_delete=CASCADE) # type: UserProfile + realm: Realm = models.ForeignKey(Realm, db_index=True, on_delete=CASCADE) + user_profile: UserProfile = models.ForeignKey(UserProfile, on_delete=CASCADE) # Case-insensitive name for the alert word. - word = models.TextField() # type: str + word: str = models.TextField() class Meta: unique_together = ("user_profile", "word") diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 27a8c64920..72d157b579 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -2229,11 +2229,13 @@ class GoogleAuthBackendTest(SocialAuthBase): def test_redirect_to_next_url_for_log_into_subdomain(self) -> None: def test_redirect_to_next_url(next: str='') -> HttpResponse: - data = {'full_name': 'Hamlet', - 'email': self.example_email("hamlet"), - 'subdomain': 'zulip', - 'is_signup': False, - 'redirect_to': next} # type: ExternalAuthDataDict + data: ExternalAuthDataDict = { + 'full_name': 'Hamlet', + 'email': self.example_email("hamlet"), + 'subdomain': 'zulip', + 'is_signup': False, + 'redirect_to': next, + } user_profile = self.example_user('hamlet') with mock.patch( 'zerver.views.auth.authenticate', @@ -2254,22 +2256,26 @@ class GoogleAuthBackendTest(SocialAuthBase): self.assertEqual(res.url, 'http://zulip.testserver/#narrow/stream/7-test-here') def test_log_into_subdomain_when_token_is_malformed(self) -> None: - data = {'full_name': 'Full Name', - 'email': self.example_email("hamlet"), - 'subdomain': 'zulip', - 'is_signup': False, - 'redirect_to': ''} # type: ExternalAuthDataDict + data: ExternalAuthDataDict = { + 'full_name': 'Full Name', + 'email': self.example_email("hamlet"), + 'subdomain': 'zulip', + 'is_signup': False, + 'redirect_to': '', + } with mock.patch("logging.warning") as mock_warn: result = self.get_log_into_subdomain(data, force_token='nonsense') mock_warn.assert_called_once_with("log_into_subdomain: Malformed token given: %s", "nonsense") self.assertEqual(result.status_code, 400) def test_log_into_subdomain_when_token_not_found(self) -> None: - data = {'full_name': 'Full Name', - 'email': self.example_email("hamlet"), - 'subdomain': 'zulip', - 'is_signup': False, - 'redirect_to': ''} # type: ExternalAuthDataDict + data: ExternalAuthDataDict = { + 'full_name': 'Full Name', + 'email': self.example_email("hamlet"), + 'subdomain': 'zulip', + 'is_signup': False, + 'redirect_to': '', + } with mock.patch("logging.warning") as mock_warn: token = generate_random_token(ExternalAuthResult.LOGIN_TOKEN_LENGTH) result = self.get_log_into_subdomain(data, force_token=token) @@ -2283,11 +2289,13 @@ class GoogleAuthBackendTest(SocialAuthBase): existing_user.email = 'whatever@zulip.com' existing_user.save() - data = {'full_name': 'Full Name', - 'email': 'existing@zulip.com', - 'subdomain': 'zulip', - 'is_signup': True, - 'redirect_to': ''} # type: ExternalAuthDataDict + data: ExternalAuthDataDict = { + 'full_name': 'Full Name', + 'email': 'existing@zulip.com', + 'subdomain': 'zulip', + 'is_signup': True, + 'redirect_to': '', + } result = self.get_log_into_subdomain(data) # Should simply get logged into the existing account: @@ -2295,11 +2303,13 @@ class GoogleAuthBackendTest(SocialAuthBase): self.assert_logged_in_user_id(existing_user.id) def test_log_into_subdomain_when_is_signup_is_true_and_new_user(self) -> None: - data = {'full_name': 'New User Name', - 'email': 'new@zulip.com', - 'subdomain': 'zulip', - 'is_signup': True, - 'redirect_to': ''} # type: ExternalAuthDataDict + data: ExternalAuthDataDict = { + 'full_name': 'New User Name', + 'email': 'new@zulip.com', + 'subdomain': 'zulip', + 'is_signup': True, + 'redirect_to': '', + } result = self.get_log_into_subdomain(data) self.assertEqual(result.status_code, 302) confirmation = Confirmation.objects.all().first() @@ -2318,11 +2328,13 @@ class GoogleAuthBackendTest(SocialAuthBase): self.assert_in_success_response(['id_full_name'], result) def test_log_into_subdomain_when_is_signup_is_false_and_new_user(self) -> None: - data = {'full_name': 'New User Name', - 'email': 'new@zulip.com', - 'subdomain': 'zulip', - 'is_signup': False, - 'redirect_to': ''} # type: ExternalAuthDataDict + data: ExternalAuthDataDict = { + 'full_name': 'New User Name', + 'email': 'new@zulip.com', + 'subdomain': 'zulip', + 'is_signup': False, + 'redirect_to': '', + } result = self.get_log_into_subdomain(data) self.assertEqual(result.status_code, 200) self.assert_in_response('No account found for', result) @@ -2346,11 +2358,13 @@ class GoogleAuthBackendTest(SocialAuthBase): self.assert_in_success_response(['id_full_name'], result) def test_log_into_subdomain_when_using_invite_link(self) -> None: - data = {'full_name': 'New User Name', - 'email': 'new@zulip.com', - 'subdomain': 'zulip', - 'is_signup': True, - 'redirect_to': ''} # type: ExternalAuthDataDict + data: ExternalAuthDataDict = { + 'full_name': 'New User Name', + 'email': 'new@zulip.com', + 'subdomain': 'zulip', + 'is_signup': True, + 'redirect_to': '', + } realm = get_realm("zulip") realm.invite_required = True @@ -2406,11 +2420,13 @@ class GoogleAuthBackendTest(SocialAuthBase): self.assertEqual(sorted(new_streams), stream_names) def test_log_into_subdomain_when_email_is_none(self) -> None: - data = {'full_name': None, - 'email': None, - 'subdomain': 'zulip', - 'is_signup': False, - 'redirect_to': ''} # type: ExternalAuthDataDict + data: ExternalAuthDataDict = { + 'full_name': None, + 'email': None, + 'subdomain': 'zulip', + 'is_signup': False, + 'redirect_to': '', + } with mock.patch('logging.warning') as mock_warn: result = self.get_log_into_subdomain(data) @@ -2418,9 +2434,11 @@ class GoogleAuthBackendTest(SocialAuthBase): mock_warn.assert_called_once() def test_user_cannot_log_into_wrong_subdomain(self) -> None: - data = {'full_name': 'Full Name', - 'email': self.example_email("hamlet"), - 'subdomain': 'zephyr'} # type: ExternalAuthDataDict + data: ExternalAuthDataDict = { + 'full_name': 'Full Name', + 'email': self.example_email("hamlet"), + 'subdomain': 'zephyr', + } result = self.get_log_into_subdomain(data) self.assert_json_error(result, "Invalid subdomain") diff --git a/zerver/tests/test_slack_importer.py b/zerver/tests/test_slack_importer.py index 52d791b103..2129de12e8 100644 --- a/zerver/tests/test_slack_importer.py +++ b/zerver/tests/test_slack_importer.py @@ -677,7 +677,8 @@ class SlackImporter(ZulipTestCase): realm: Dict[str, Any] = {'zerver_subscription': []} user_list: List[Dict[str, Any]] = [] reactions = [{"name": "grinning", "users": ["U061A5N1G"], "count": 1}] - attachments = uploads = [] # type: List[Dict[str, Any]] + attachments: List[Dict[str, Any]] = [] + uploads: List[Dict[str, Any]] = [] zerver_usermessage = [{'id': 3}, {'id': 5}, {'id': 6}, {'id': 9}] diff --git a/zerver/webhooks/beanstalk/view.py b/zerver/webhooks/beanstalk/view.py index a2e2a84590..d70e74d363 100644 --- a/zerver/webhooks/beanstalk/view.py +++ b/zerver/webhooks/beanstalk/view.py @@ -47,7 +47,9 @@ def _transform_commits_list_to_common_format(commits: List[Dict[str, Any]]) -> L def beanstalk_decoder(view_func: ViewFuncT) -> ViewFuncT: @wraps(view_func) def _wrapped_view_func(request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse: - auth_type, encoded_value = request.META['HTTP_AUTHORIZATION'].split() # type: str, str + auth_type: str + encoded_value: str + auth_type, encoded_value = request.META['HTTP_AUTHORIZATION'].split() if auth_type.lower() == "basic": email, api_key = base64.b64decode(encoded_value).decode('utf-8').split(":") email = email.replace('%40', '@')