Skip to content

Re-enable discarded_futures lint #9117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elliette merged 8 commits into from
Apr 10, 2025
Merged

Re-enable discarded_futures lint #9117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
13ce11b
Enable lint
elliette Apr 7, 2025
0b55d24
Add safeUnawaited method
elliette Apr 7, 2025
3d40e17
Fix instances of discarded_futures
elliette Apr 7, 2025
6ceeee8
Switch awaits to unawaits
elliette Apr 8, 2025
d350224
Update storage.setValue
elliette Apr 8, 2025
30c64ad
Don't await tree refresh
elliette Apr 8, 2025
ea74a31
Partial revert of "Update storage.setValue"
elliette Apr 8, 2025
3d1abbc
Remove async
elliette Apr 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion analysis_options.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ linter:
# - constant_identifier_names # https://github.com/dart-lang/linter/issues/204
- control_flow_in_finally
- directives_ordering
# - discarded_futures # TODO(https://github.com/flutter/devtools/issues/9102): Re-enable.
- discarded_futures
- empty_catches
- empty_constructor_bodies
- empty_statements
Expand Down
3 changes: 2 additions & 1 deletion packages/devtools_app/lib/src/framework/home_screen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import '../shared/primitives/blocking_action_mixin.dart';
import '../shared/primitives/utils.dart';
import '../shared/title.dart';
import '../shared/ui/vm_flag_widgets.dart';
import '../shared/utils/utils.dart';
import 'framework_core.dart';

class HomeScreen extends Screen {
Expand Down Expand Up @@ -259,7 +260,7 @@ class _ConnectInputState extends State<ConnectInput> with BlockingActionMixin {
}

assert(() {
storage.setValue(_debugVmServiceUriKey, uri);
safeUnawaited(storage.setValue(_debugVmServiceUriKey, uri));
return true;
}());

Expand Down
7 changes: 4 additions & 3 deletions packages/devtools_app/lib/src/screens/debugger/debugger_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import '../../shared/framework/screen_controllers.dart';
import '../../shared/globals.dart';
import '../../shared/primitives/message_bus.dart';
import '../../shared/primitives/utils.dart';
import '../../shared/utils/utils.dart';
import 'codeview_controller.dart';
import 'debugger_model.dart';

Expand Down Expand Up @@ -399,9 +400,9 @@ class DebuggerController extends DevToolsScreenController
/// This method ensures that the source for the script is populated in our
/// cache, in order to reduce flashing in the editor view.
void _populateScriptAndShowLocation(ScriptRef scriptRef) {
unawaited(
scriptManager.getScript(scriptRef).then((script) {
codeViewController.showScriptLocation(ScriptLocation(scriptRef));
safeUnawaited(
scriptManager.getScript(scriptRef).then((script) async {
await codeViewController.showScriptLocation(ScriptLocation(scriptRef));
}),
);
}
Expand Down
15 changes: 9 additions & 6 deletions packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import '../../shared/diagnostics/primitives/instance_ref.dart';
import '../../shared/globals.dart';
import '../../shared/primitives/query_parameters.dart';
import '../../shared/primitives/utils.dart';
import '../../shared/utils/utils.dart';
import '../inspector_shared/inspector_screen.dart';
import 'inspector_tree_controller.dart';

Expand Down Expand Up @@ -105,12 +106,14 @@ class InspectorController extends DisposableController
// won't interfere with users.
addAutoDisposeListener(_supportsToggleSelectWidgetMode, () {
if (_supportsToggleSelectWidgetMode.value) {
serviceConnection.serviceManager.serviceExtensionManager
.setServiceExtensionState(
extensions.enableOnDeviceInspector.extension,
enabled: true,
value: true,
);
safeUnawaited(
serviceConnection.serviceManager.serviceExtensionManager
.setServiceExtensionState(
extensions.enableOnDeviceInspector.extension,
enabled: true,
value: true,
),
);
}
});
}
Expand Down
15 changes: 9 additions & 6 deletions packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import '../../shared/globals.dart';
import '../../shared/managers/notifications.dart';
import '../../shared/primitives/query_parameters.dart';
import '../../shared/primitives/utils.dart';
import '../../shared/utils/utils.dart';
import '../inspector_shared/inspector_screen.dart';
import 'inspector_data_models.dart';
import 'inspector_tree_controller.dart';
Expand Down Expand Up @@ -112,12 +113,14 @@ class InspectorController extends DisposableController
// won't interfere with users.
addAutoDisposeListener(_supportsToggleSelectWidgetMode, () {
if (_supportsToggleSelectWidgetMode.value) {
serviceConnection.serviceManager.serviceExtensionManager
.setServiceExtensionState(
extensions.enableOnDeviceInspector.extension,
enabled: true,
value: true,
);
safeUnawaited(
serviceConnection.serviceManager.serviceExtensionManager
.setServiceExtensionState(
extensions.enableOnDeviceInspector.extension,
enabled: true,
value: true,
),
);
}
});

Expand Down
3 changes: 2 additions & 1 deletion packages/devtools_app/lib/src/screens/inspector_v2/inspector_screen_body.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import '../../shared/managers/error_badge_manager.dart';
import '../../shared/primitives/blocking_action_mixin.dart';
import '../../shared/ui/common_widgets.dart';
import '../../shared/ui/search.dart';
import '../../shared/utils/utils.dart';
import '../inspector_shared/inspector_controls.dart';
import '../inspector_shared/inspector_screen.dart';
import 'inspector_controller.dart';
Expand Down Expand Up @@ -97,7 +98,7 @@ class InspectorScreenBodyState extends State<InspectorScreenBody>
addAutoDisposeListener(preferences.inspector.pubRootDirectories, () {
if (serviceConnection.serviceManager.connectedState.value.connected &&
controller.firstInspectorTreeLoadCompleted) {
controller.refreshInspector();
safeUnawaited(controller.refreshInspector());
}
});

Expand Down
11 changes: 8 additions & 3 deletions ...tools_app/lib/src/screens/performance/panes/controls/enhance_tracing/enhance_tracing.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import '../../../../../service/service_extensions.dart' as extensions;
import '../../../../../shared/globals.dart';
import '../../../../../shared/primitives/utils.dart';
import '../../../../../shared/ui/common_widgets.dart';
import '../../../../../shared/utils/utils.dart';
import '../performance_controls.dart';
import 'enhance_tracing_controller.dart';

Expand Down Expand Up @@ -142,7 +143,7 @@ class _TraceWidgetBuildsSettingState extends State<TraceWidgetBuildsSetting>
for (final type in TraceWidgetBuildsScope.values) {
final extension = _traceWidgetBuildsExtensions[type]!;

unawaited(
safeUnawaited(
serviceConnection.serviceManager.serviceExtensionManager
.waitForServiceExtensionAvailable(extension.extension)
.then((isServiceAvailable) {
Expand All @@ -154,9 +155,13 @@ class _TraceWidgetBuildsSettingState extends State<TraceWidgetBuildsSetting>
.serviceExtensionManager
.getServiceExtensionState(extension.extension);

_updateForServiceExtensionState(state.value, type);
safeUnawaited(
_updateForServiceExtensionState(state.value, type),
);
addAutoDisposeListener(state, () {
_updateForServiceExtensionState(state.value, type);
safeUnawaited(
_updateForServiceExtensionState(state.value, type),
);
});
}
}),
Expand Down
13 changes: 8 additions & 5 deletions packages/devtools_app/lib/src/service/vm_service_wrapper.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import '../screens/vm_developer/vm_service_private_extensions.dart';
import '../shared/feature_flags.dart';
import '../shared/globals.dart';
import '../shared/primitives/utils.dart';
import '../shared/utils/utils.dart';
import 'json_to_service_cache.dart';

final _log = Logger('vm_service_wrapper');
Expand Down Expand Up @@ -188,8 +189,8 @@ class VmServiceWrapper extends VmService {
}

@override
Future<Success> streamCancel(String streamId) {
_activeStreams.remove(streamId);
Future<Success> streamCancel(String streamId) async {
await _activeStreams.remove(streamId);
Copy link
Member Author

@elliette elliette Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it should have always been awaited

return super.streamCancel(streamId);
}

Expand Down Expand Up @@ -380,9 +381,11 @@ class VmServiceWrapper extends VmService {
}
}

localFuture.then(
(value) => futureComplete(),
onError: (error) => futureComplete(),
safeUnawaited(
localFuture.then(
(value) => futureComplete(),
onError: (error) => futureComplete(),
),
);
return localFuture;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools_app/lib/src/shared/managers/survey.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class SurveyService {
allowDuplicates: false,
);
if (didPush) {
server.incrementSurveyShownCount();
safeUnawaited(server.incrementSurveyShownCount());
}
});
}
Expand Down
8 changes: 5 additions & 3 deletions packages/devtools_app/lib/src/shared/preferences/_extension_preferences.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ class ExtensionsPreferencesController extends DisposableController
@override
Future<void> init() async {
addAutoDisposeListener(showOnlyEnabledExtensions, () {
storage.setValue(
_showOnlyEnabledExtensionsId,
showOnlyEnabledExtensions.value.toString(),
safeUnawaited(
storage.setValue(
_showOnlyEnabledExtensionsId,
showOnlyEnabledExtensions.value.toString(),
),
);
ga.select(
gac.DevToolsExtensionEvents.extensionScreenId.name,
Expand Down
8 changes: 5 additions & 3 deletions packages/devtools_app/lib/src/shared/preferences/_inspector_preferences.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,11 @@ class InspectorPreferencesController extends DisposableController
await _updateInspectorDetailsViewSelection();

addAutoDisposeListener(_defaultDetailsView, () {
storage.setValue(
_defaultDetailsViewStorageId,
_defaultDetailsView.value.name.toString(),
safeUnawaited(
storage.setValue(
_defaultDetailsViewStorageId,
_defaultDetailsView.value.name.toString(),
),
);
});
}
Expand Down
12 changes: 8 additions & 4 deletions packages/devtools_app/lib/src/shared/preferences/_logging_preferences.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,11 @@ class LoggingPreferencesController extends DisposableController
int.tryParse(await storage.getValue(_retentionLimitStorageId) ?? '') ??
_defaultRetentionLimit;
addAutoDisposeListener(retentionLimit, () {
storage.setValue(
_retentionLimitStorageId,
retentionLimit.value.toString(),
safeUnawaited(
storage.setValue(
_retentionLimitStorageId,
retentionLimit.value.toString(),
),
);
ga.select(
gac.logging,
Expand All @@ -59,7 +61,9 @@ class LoggingPreferencesController extends DisposableController
) ??
_defaultDetailsFormat;
addAutoDisposeListener(detailsFormat, () {
storage.setValue(detailsFormatStorageId, detailsFormat.value.name);
safeUnawaited(
storage.setValue(detailsFormatStorageId, detailsFormat.value.name),
);
ga.select(
gac.logging,
gac.LoggingEvents.changeDetailsFormat.name,
Expand Down
16 changes: 11 additions & 5 deletions packages/devtools_app/lib/src/shared/preferences/_memory_preferences.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ class MemoryPreferencesController extends DisposableController
@override
Future<void> init() async {
addAutoDisposeListener(androidCollectionEnabled, () {
storage.setValue(
_androidCollectionEnabledStorageId,
androidCollectionEnabled.value.toString(),
safeUnawaited(
storage.setValue(
_androidCollectionEnabledStorageId,
androidCollectionEnabled.value.toString(),
),
);
if (androidCollectionEnabled.value) {
ga.select(gac.memory, gac.MemoryEvents.androidChart.name);
Expand All @@ -40,7 +42,9 @@ class MemoryPreferencesController extends DisposableController
);

addAutoDisposeListener(showChart, () {
storage.setValue(_showChartStorageId, showChart.value.toString());
safeUnawaited(
storage.setValue(_showChartStorageId, showChart.value.toString()),
);

ga.select(
gac.memory,
Expand All @@ -55,7 +59,9 @@ class MemoryPreferencesController extends DisposableController
);

addAutoDisposeListener(refLimit, () {
storage.setValue(_refLimitStorageId, refLimit.value.toString());
safeUnawaited(
storage.setValue(_refLimitStorageId, refLimit.value.toString()),
);

ga.select(gac.memory, gac.MemoryEvents.browseRefLimit.name);
});
Expand Down
16 changes: 10 additions & 6 deletions packages/devtools_app/lib/src/shared/preferences/_performance_preferences.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ class PerformancePreferencesController extends DisposableController
@override
Future<void> init() async {
addAutoDisposeListener(showFlutterFramesChart, () {
storage.setValue(
_showFlutterFramesChartId,
showFlutterFramesChart.value.toString(),
safeUnawaited(
storage.setValue(
_showFlutterFramesChartId,
showFlutterFramesChart.value.toString(),
),
);
ga.select(
gac.performance,
Expand All @@ -38,9 +40,11 @@ class PerformancePreferencesController extends DisposableController
);

addAutoDisposeListener(includeCpuSamplesInTimeline, () {
storage.setValue(
_includeCpuSamplesInTimelineId,
includeCpuSamplesInTimeline.value.toString(),
safeUnawaited(
storage.setValue(
_includeCpuSamplesInTimelineId,
includeCpuSamplesInTimeline.value.toString(),
),
);
ga.select(
gac.performance,
Expand Down
24 changes: 15 additions & 9 deletions packages/devtools_app/lib/src/shared/preferences/preferences.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,11 @@ class PreferencesController extends DisposableController
ga.impression(gac.devToolsMain, gac.startingTheme(darkMode: useDarkMode));
toggleDarkModeTheme(useDarkMode);
addAutoDisposeListener(darkModeEnabled, () {
storage.setValue(
_UiPreferences.darkMode.storageKey,
'${darkModeEnabled.value}',
safeUnawaited(
storage.setValue(
_UiPreferences.darkMode.storageKey,
'${darkModeEnabled.value}',
),
);
});
}
Expand All @@ -149,9 +151,11 @@ class PreferencesController extends DisposableController
);
toggleVmDeveloperMode(vmDeveloperModeValue);
addAutoDisposeListener(vmDeveloperModeEnabled, () {
storage.setValue(
_UiPreferences.vmDeveloperMode.storageKey,
'${vmDeveloperModeEnabled.value}',
safeUnawaited(
storage.setValue(
Copy link
Member

@kenzieschmoll kenzieschmoll Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we ever await storage.setValue? If not, we could change the return type of setValue to void and then add the safeUnawatied wrapper inside that method.

Copy link
Member Author

@elliette elliette Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, setValue now returns void instead of Future<void>

Copy link
Member

@kenzieschmoll kenzieschmoll Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we were awaiting storage.setValue in some places though. My comment was only if we weren't awaiting this method call anywhere but it looks like we were. I don't know if we want to change the behavior of places where we were awaiting.

Copy link
Member Author

@elliette elliette Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah got it! Reverted those changes

_UiPreferences.vmDeveloperMode.storageKey,
'${vmDeveloperModeEnabled.value}',
),
);
});
}
Expand Down Expand Up @@ -237,9 +241,11 @@ class PreferencesController extends DisposableController
);
toggleVerboseLogging(verboseLoggingEnabledValue);
addAutoDisposeListener(verboseLoggingEnabled, () {
storage.setValue(
_GeneralPreferences.verboseLogging.name,
verboseLoggingEnabled.value.toString(),
safeUnawaited(
storage.setValue(
_GeneralPreferences.verboseLogging.name,
verboseLoggingEnabled.value.toString(),
),
);
});
}
Expand Down
13 changes: 12 additions & 1 deletion packages/devtools_app/lib/src/shared/utils/utils.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import 'package:vm_service/vm_service.dart';

import '../../../devtools.dart' as devtools;
import '../../service/connected_app/connected_app.dart';
import '../framework/app_error_handling.dart';
import '../globals.dart';
import '../primitives/query_parameters.dart';
import '../primitives/utils.dart';
Expand Down Expand Up @@ -343,7 +344,7 @@ class InterruptableChunkWorker {
}

progressCallback(0.0);
doChunkWork(0);
safeUnawaited(doChunkWork(0));

return completer.future;
}
Expand All @@ -354,3 +355,13 @@ class InterruptableChunkWorker {
}

String get devToolsVersion => devtools.version;

/// Unawaits the given [future] and catches any errors thrown.
void safeUnawaited(
Future<void> future, {
void Function(Object?, StackTrace)? onError,
}) {
onError ??=
(e, st) => reportError('Error in unawaited Future: $e', stack: st);
unawaited(future.catchError(onError));
}
Loading