zulip · gnprice · Oct 12, 2024 · Sep 10, 2024 · Sep 12, 2024 · Sep 12, 2024
diff --git a/analysis_options.yaml b/analysis_options.yaml
@@ -25,3 +25,4 @@ linter:
     no_literal_bool_comparisons: true
     prefer_relative_imports: true
     unnecessary_statements: true
+    unawaited_futures: true
diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart
@@ -615,7 +615,7 @@ class TopicAutocompleteView extends AutocompleteView<TopicAutocompleteQuery, Top
     final result = await getStreamTopics(store.connection, streamId: streamId);
     _topics = result.topics.map((e) => e.name);
     _isFetching = false;
-    if (_query != null) _startSearch();
+    if (_query != null) return _startSearch();
   }
 
   @override

diff --git a/lib/model/store.dart b/lib/model/store.dart
@@ -121,7 +121,7 @@ abstract class GlobalStore extends ChangeNotifier {
     _perAccountStoresLoading[accountId] = future;
     store = await future;
     _setPerAccount(accountId, store);
-    _perAccountStoresLoading.remove(accountId);
+    unawaited(_perAccountStoresLoading.remove(accountId));
     return store;
   }
 

diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart
@@ -1,3 +1,4 @@
+import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 
@@ -377,10 +378,9 @@ class NotificationDisplayManager {
 
     assert(debugLog('  account: $account, narrow: $narrow'));
     // TODO(nav): Better interact with existing nav stack on notif open
-    navigator.push(MaterialAccountWidgetRoute<void>(accountId: account.id,
+    unawaited(navigator.push(MaterialAccountWidgetRoute<void>(accountId: account.id,
       // TODO(#82): Open at specific message, not just conversation
-      page: MessageListPage(initNarrow: narrow)));
-    return;
+      page: MessageListPage(initNarrow: narrow))));
   }
 
   static Future<Uint8List?> _fetchBitmap(Uri url) async {

diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart
@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import 'package:flutter/material.dart';
 import 'package:flutter/services.dart';
 import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
@@ -303,7 +305,7 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton {
 
   @override void onPressed(BuildContext context) async {
     Navigator.of(context).pop();
-    markNarrowAsUnreadFromMessage(messageListContext, message, narrow);
+    unawaited(markNarrowAsUnreadFromMessage(messageListContext, message, narrow));
   }
 }
 

diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart
@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import 'package:flutter/cupertino.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/gestures.dart';
@@ -1213,9 +1215,9 @@ void _launchUrl(BuildContext context, String urlString) async {
 
   final internalNarrow = parseInternalLink(url, store);
   if (internalNarrow != null) {
-    Navigator.push(context,
+    unawaited(Navigator.push(context,
       MessageListPage.buildRoute(context: context,
-        narrow: internalNarrow));
+        narrow: internalNarrow)));
     return;
   }
 

diff --git a/lib/widgets/login.dart b/lib/widgets/login.dart
@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/services.dart';
@@ -142,7 +144,7 @@ class _AddAccountPageState extends State<AddAccountPage> {
     super.dispose();
   }
 
-  Future<void> _onSubmitted(BuildContext context) async {
+  void _onSubmitted(BuildContext context) async {
     final zulipLocalizations = ZulipLocalizations.of(context);
     final url = _parseResult.url;
     final error = _parseResult.error;
@@ -184,8 +186,8 @@ class _AddAccountPageState extends State<AddAccountPage> {
         return;
       }
 
-      Navigator.push(context,
-        LoginPage.buildRoute(serverSettings: serverSettings));
+      unawaited(Navigator.push(context,
+        LoginPage.buildRoute(serverSettings: serverSettings)));
     } finally {
       setState(() {
         _inProgress = false;
@@ -393,9 +395,9 @@ class _LoginPageState extends State<LoginPage> {
       return;
     }
 
-    Navigator.of(context).pushAndRemoveUntil(
+    unawaited(Navigator.of(context).pushAndRemoveUntil(
       HomePage.buildRoute(accountId: accountId),
-      (route) => (route is! _LoginSequenceRoute),
+      (route) => (route is! _LoginSequenceRoute)),
     );
   }
 

diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart
@@ -601,11 +601,11 @@ class ScrollToBottomButton extends StatelessWidget {
   final ValueNotifier<bool> visibleValue;
   final ScrollController scrollController;
 
-  Future<void> _navigateToBottom() async {
+  Future<void> _navigateToBottom() {
     final distance = scrollController.position.pixels;
     final durationMsAtSpeedLimit = (1000 * distance / 8000).ceil();
     final durationMs = max(300, durationMsAtSpeedLimit);
-    scrollController.animateTo(
+    return scrollController.animateTo(
       0,
       duration: Duration(milliseconds: durationMs),
       curve: Curves.ease);

diff --git a/test/api/core_test.dart b/test/api/core_test.dart
@@ -1,3 +1,4 @@
+import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 
@@ -64,15 +65,17 @@ void main() {
     });
 
     test('send rejects off-realm URL (with default useAuth)', () async {
-      Future<void> checkAllow(String realmUrl, String requestUrl) async {
-        check(await makeRequest(realmUrl, requestUrl))
-          .isA<http.Request>()
-          .url.asString.equals(requestUrl);
+      void checkAllow(String realmUrl, String requestUrl) {
+        // No need to await directly; `check` ensures the future completes
+        // before the enclosing test is considered complete.
+        unawaited(check(makeRequest(realmUrl, requestUrl))
+          .completes((it) => it.isA<http.Request>()
+            .url.asString.equals(requestUrl)));
       }
 
-      Future<void> checkDeny(String realmUrl, String requestUrl) async {
-        await check(makeRequest(realmUrl, requestUrl))
-          .throws<StateError>();
+      void checkDeny(String realmUrl, String requestUrl) {
+        unawaited(check(makeRequest(realmUrl, requestUrl))
+          .throws<StateError>());
       }
 
       // Baseline: normal requests are allowed.
@@ -242,7 +245,7 @@ void main() {
   test('API network errors', () async {
     void checkRequest<T extends Object>(
         T exception, Condition<NetworkException> condition) {
-      finish(check(tryRequest(exception: exception))
+      unawaited(check(tryRequest(exception: exception))
         .throws<NetworkException>((it) => it
           ..routeName.equals(kExampleRouteName)
           ..cause.equals(exception)
@@ -296,7 +299,7 @@ void main() {
     void checkMalformed({
         int httpStatus = 400, Map<String, dynamic>? json, String? body}) {
       assert((json == null) != (body == null));
-      finish(check(tryRequest(httpStatus: httpStatus, json: json, body: body))
+      unawaited(check(tryRequest(httpStatus: httpStatus, json: json, body: body))
         .throws<MalformedServerResponseException>((it) => it
           ..routeName.equals(kExampleRouteName)
           ..httpStatus.equals(httpStatus)

diff --git a/test/api/fake_api_test.dart b/test/api/fake_api_test.dart
@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import 'package:checks/checks.dart';
 import 'package:test/scaffolding.dart';
 import 'package:zulip/api/exception.dart';
@@ -29,8 +31,8 @@ void main() {
       json: {'a': 3});
 
     Map<String, dynamic>? result;
-    connection.get('aRoute', (json) => json, '/', null)
-      .then((r) { result = r; });
+    unawaited(connection.get('aRoute', (json) => json, '/', null)
+      .then((r) { result = r; }));
 
     async.elapse(const Duration(seconds: 1));
     check(result).isNull();
@@ -45,8 +47,8 @@ void main() {
       exception: Exception("oops"));
 
     Object? error;
-    connection.get('aRoute', (json) => null, '/', null)
-      .catchError((Object e) { error = e; });
+    unawaited(connection.get('aRoute', (json) => null, '/', null)
+      .catchError((Object e) { error = e; }));
 
     async.elapse(const Duration(seconds: 1));
     check(error).isNull();

diff --git a/test/model/channel_test.dart b/test/model/channel_test.dart
@@ -264,15 +264,15 @@ void main() {
                 await store.addSubscription(
                   eg.subscription(stream1, isMuted: streamMuted));
               }
-              store.handleEvent(mkEvent(oldPolicy));
+              await store.handleEvent(mkEvent(oldPolicy));
               final oldVisibleInStream = store.isTopicVisibleInStream(stream1.streamId, 'topic');
               final oldVisible         = store.isTopicVisible(stream1.streamId, 'topic');
 
               final event = mkEvent(newPolicy);
               final willChangeInStream = store.willChangeIfTopicVisibleInStream(event);
               final willChange         = store.willChangeIfTopicVisible(event);
 
-              store.handleEvent(event);
+              await store.handleEvent(event);
               final newVisibleInStream = store.isTopicVisibleInStream(stream1.streamId, 'topic');
               final newVisible         = store.isTopicVisible(stream1.streamId, 'topic');
 

diff --git a/test/model/store_test.dart b/test/model/store_test.dart
@@ -155,11 +155,11 @@ void main() {
 
     test('reject changing id, realmUrl, or userId', () async {
       final globalStore = eg.globalStore(accounts: [eg.selfAccount]);
-      check(globalStore.updateAccount(eg.selfAccount.id, const AccountsCompanion(
+      await check(globalStore.updateAccount(eg.selfAccount.id, const AccountsCompanion(
         id: Value(1234)))).throws();
-      check(globalStore.updateAccount(eg.selfAccount.id, AccountsCompanion(
+      await check(globalStore.updateAccount(eg.selfAccount.id, AccountsCompanion(
         realmUrl: Value(Uri.parse('https://other.example'))))).throws();
-      check(globalStore.updateAccount(eg.selfAccount.id, const AccountsCompanion(
+      await check(globalStore.updateAccount(eg.selfAccount.id, const AccountsCompanion(
         userId: Value(1234)))).throws();
     });
 
@@ -295,7 +295,7 @@ void main() {
       connection.prepare(exception: Exception('failed'));
       final future = UpdateMachine.load(globalStore, eg.selfAccount.id);
       bool complete = false;
-      future.whenComplete(() => complete = true);
+      unawaited(future.whenComplete(() => complete = true));
       async.flushMicrotasks();
       checkLastRequest();
       check(complete).isFalse();
@@ -363,7 +363,7 @@ void main() {
       connection.prepare(exception: Exception('failed'));
       final future = updateMachine.fetchEmojiData(emojiDataUrl);
       bool complete = false;
-      future.whenComplete(() => complete = true);
+      unawaited(future.whenComplete(() => complete = true));
       async.flushMicrotasks();
       checkLastRequest();
       check(complete).isFalse();

diff --git a/test/model/test_store.dart b/test/model/test_store.dart
@@ -80,6 +80,7 @@ class TestGlobalStore extends GlobalStore {
     // Check for duplication is typically handled by the database but since
     // we're not using a real database, this needs to be handled here.
     // See [AppDatabase.createAccount].
+    // TODO: Ensure that parallel account insertions do not bypass this check.
     if (accounts.any((account) =>
           data.realmUrl.value == account.realmUrl
           && (data.userId.value == account.userId