From e1bbf5405c57a058a1f6a3b275a8db1112ea51c6 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Levesque Date: Mon, 30 Oct 2023 08:41:04 -0400 Subject: [PATCH] fix: Add Microsoft.VisualStudio.Threading.Analyzers to check for async void usages. --- .editorconfig | 9 + CHANGELOG.md | 2 + Directory.Build.props | 2 + .../ApplicationTemplate.Mobile.csproj | 1 + .../Reactive/IObservable.Extensions.cs | 3 +- .../DadJokes/DadJokesFiltersPageViewModel.cs | 11 +- .../ConfigurationDebuggerViewModel.cs | 2 +- ...ApplicationTemplate.Shared.Views.projitems | 1 - .../Behaviors/FormattingTextBoxBehavior.cs | 3 +- .../Controls/AppBarBackButton.cs | 3 +- .../Controls/Validation/DataValidationView.cs | 3 +- .../ExtendedSplashscreenController.cs | 5 +- .../Framework/Launcher/LauncherService.cs | 29 +-- .../Helpers/DispatcherQueueExtensions.cs | 237 ------------------ .../Startup.cs | 7 +- .../ApplicationTemplate.Windows.csproj | 1 + 16 files changed, 35 insertions(+), 284 deletions(-) delete mode 100644 src/app/ApplicationTemplate.Shared.Views/Helpers/DispatcherQueueExtensions.cs diff --git a/.editorconfig b/.editorconfig index a7a799e9..6feff84e 100644 --- a/.editorconfig +++ b/.editorconfig @@ -391,3 +391,12 @@ dotnet_diagnostic.SA1210.severity = suggestion dotnet_diagnostic.CA1308.severity = none # CS1587: XML comment is not placed on a valid language element dotnet_diagnostic.CS1587.severity = none + +# VSTHRD110: This one is bugged: https://github.com/microsoft/vs-threading/issues/899 +dotnet_diagnostic.VSTHRD110.severity = none +# VSTHRD200: Use `Async` naming convention +dotnet_diagnostic.VSTHRD200.severity = none +# VSTHRD100: Avoid async void methods +dotnet_diagnostic.VSTHRD100.severity = error +# VSTHRD101: Avoid unsupported async delegates +dotnet_diagnostic.VSTHRD101.severity = error diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d821e2c..28c3af7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) Prefix your items with `(Template)` if the change is about the template and not the resulting application. ## 2.1.X +- Replace local `DispatcherQueue` extension methods with the ones from the WinUI and Uno.WinUI Community Toolkit. +- Add `Microsoft.VisualStudio.Threading.Analyzers` to check for async void usages and fix async void usages. - Enable `TreatWarningsAsErrors` for the Access, Business, and Presentation projects. - Update analyzers packages and severity of rules. diff --git a/Directory.Build.props b/Directory.Build.props index 7484b9ee..2a55efca 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -8,6 +8,8 @@ all runtime; build; native; contentfiles; analyzers + + diff --git a/src/app/ApplicationTemplate.Mobile/ApplicationTemplate.Mobile.csproj b/src/app/ApplicationTemplate.Mobile/ApplicationTemplate.Mobile.csproj index 9ed7d618..f571d104 100644 --- a/src/app/ApplicationTemplate.Mobile/ApplicationTemplate.Mobile.csproj +++ b/src/app/ApplicationTemplate.Mobile/ApplicationTemplate.Mobile.csproj @@ -28,6 +28,7 @@ + diff --git a/src/app/ApplicationTemplate.Presentation/Framework/Reactive/IObservable.Extensions.cs b/src/app/ApplicationTemplate.Presentation/Framework/Reactive/IObservable.Extensions.cs index e2eb458f..ba25ab57 100644 --- a/src/app/ApplicationTemplate.Presentation/Framework/Reactive/IObservable.Extensions.cs +++ b/src/app/ApplicationTemplate.Presentation/Framework/Reactive/IObservable.Extensions.cs @@ -21,8 +21,7 @@ public static class ObservableExtensions2 /// Time delay /// Scheduler /// Output observable - [SuppressMessage("nventive.Globalization", "NV2005:NV2005 - Simple cyclomatic complexity​​", Justification = "Imported code")] - [SuppressMessage("nventive.Reliability", "NV0016:NV0016 - Do not create an async void lambda expression", Justification = "Imported code")] + [SuppressMessage("Usage", "VSTHRD101:Avoid unsupported async delegates", Justification = "Imported code")] public static IObservable ThrottleOrImmediate(this IObservable source, TimeSpan delay, IScheduler scheduler) { // Throttle behavior: diff --git a/src/app/ApplicationTemplate.Presentation/ViewModels/DadJokes/DadJokesFiltersPageViewModel.cs b/src/app/ApplicationTemplate.Presentation/ViewModels/DadJokes/DadJokesFiltersPageViewModel.cs index 05e6ca0d..2e8b98b6 100644 --- a/src/app/ApplicationTemplate.Presentation/ViewModels/DadJokes/DadJokesFiltersPageViewModel.cs +++ b/src/app/ApplicationTemplate.Presentation/ViewModels/DadJokes/DadJokesFiltersPageViewModel.cs @@ -16,15 +16,6 @@ public class DadJokesFiltersPageViewModel : ViewModel { public DadJokesFiltersPageViewModel() { - var pt = this.GetService().GetAndObservePostTypeFilter(); - var postType = GetPostType(pt); - - PostTypeFilter = postType.Result; - - async Task GetPostType(ReplaySubject ptArg) - { - return await ptArg.FirstAsync(); - } } public IDynamicCommand HandleCheck => this.GetCommand((string pt) => @@ -40,7 +31,7 @@ async Task GetPostType(ReplaySubject ptArg) public PostTypes PostTypeFilter { - get => this.Get(initialValue: PostTypes.Hot); + get => this.GetFromTask(ct => this.GetService().GetAndObservePostTypeFilter().FirstAsync(ct), initialValue: PostTypes.Hot); set => this.Set(value); } } diff --git a/src/app/ApplicationTemplate.Presentation/ViewModels/Diagnostics/Configuration/ConfigurationDebuggerViewModel.cs b/src/app/ApplicationTemplate.Presentation/ViewModels/Diagnostics/Configuration/ConfigurationDebuggerViewModel.cs index f399b796..8d8aa219 100644 --- a/src/app/ApplicationTemplate.Presentation/ViewModels/Diagnostics/Configuration/ConfigurationDebuggerViewModel.cs +++ b/src/app/ApplicationTemplate.Presentation/ViewModels/Diagnostics/Configuration/ConfigurationDebuggerViewModel.cs @@ -88,7 +88,7 @@ public string SelectedKey ConfigurationValue = this.GetService()[value]; // Erase the selection from the ComboBox because we put it in the TextBox. - Task.Run(() => RaisePropertyChanged(nameof(SelectedKey))); + _ = Task.Run(() => RaisePropertyChanged(nameof(SelectedKey))); } } diff --git a/src/app/ApplicationTemplate.Shared.Views/ApplicationTemplate.Shared.Views.projitems b/src/app/ApplicationTemplate.Shared.Views/ApplicationTemplate.Shared.Views.projitems index 7d8960b4..4c6d4436 100644 --- a/src/app/ApplicationTemplate.Shared.Views/ApplicationTemplate.Shared.Views.projitems +++ b/src/app/ApplicationTemplate.Shared.Views/ApplicationTemplate.Shared.Views.projitems @@ -109,7 +109,6 @@ - diff --git a/src/app/ApplicationTemplate.Shared.Views/Behaviors/FormattingTextBoxBehavior.cs b/src/app/ApplicationTemplate.Shared.Views/Behaviors/FormattingTextBoxBehavior.cs index 77b5e614..8a589eb2 100644 --- a/src/app/ApplicationTemplate.Shared.Views/Behaviors/FormattingTextBoxBehavior.cs +++ b/src/app/ApplicationTemplate.Shared.Views/Behaviors/FormattingTextBoxBehavior.cs @@ -1,6 +1,7 @@ using System; using System.Linq; using System.Text; +using CommunityToolkit.WinUI; using Microsoft.UI.Dispatching; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; @@ -196,7 +197,7 @@ private static void FormatText(TextBox textbox, string textFormat) return; } - _ = textbox.DispatcherQueue.RunAsync(DispatcherQueuePriority.Normal, () => + _ = textbox.DispatcherQueue.EnqueueAsync(() => { textbox.Text = formattedText; textbox.SelectionStart = selectionStart; diff --git a/src/app/ApplicationTemplate.Shared.Views/Controls/AppBarBackButton.cs b/src/app/ApplicationTemplate.Shared.Views/Controls/AppBarBackButton.cs index 18d2a40d..2ca26bbf 100644 --- a/src/app/ApplicationTemplate.Shared.Views/Controls/AppBarBackButton.cs +++ b/src/app/ApplicationTemplate.Shared.Views/Controls/AppBarBackButton.cs @@ -3,6 +3,7 @@ using System.Windows.Input; using Chinook.DynamicMvvm; using Chinook.SectionsNavigation; +using CommunityToolkit.WinUI; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.UI.Dispatching; @@ -77,7 +78,7 @@ private IDisposable ObserveBackButtonVisibility() void OnStateChanged(bool canNavigateBackOrCloseModal) { var dispatcherQueue = ServiceProvider.GetRequiredService(); - _ = dispatcherQueue.RunAsync(DispatcherQueuePriority.Normal, UpdateBackButtonUI); + _ = dispatcherQueue.EnqueueAsync(UpdateBackButtonUI); void UpdateBackButtonUI() // Runs on UI thread. { diff --git a/src/app/ApplicationTemplate.Shared.Views/Controls/Validation/DataValidationView.cs b/src/app/ApplicationTemplate.Shared.Views/Controls/Validation/DataValidationView.cs index 431afc4f..4fe53c38 100644 --- a/src/app/ApplicationTemplate.Shared.Views/Controls/Validation/DataValidationView.cs +++ b/src/app/ApplicationTemplate.Shared.Views/Controls/Validation/DataValidationView.cs @@ -4,6 +4,7 @@ using Microsoft.UI.Dispatching; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; +using CommunityToolkit.WinUI; namespace ApplicationTemplate; @@ -78,7 +79,7 @@ private void OnPropertyNamedChanged() private void OnErrorsChanged(object sender, DataErrorsChangedEventArgs e) { - _ = DispatcherQueue.RunAsync(DispatcherQueuePriority.Normal, ErrorsChangedUI); + _ = DispatcherQueue.EnqueueAsync(ErrorsChangedUI); void ErrorsChangedUI() { diff --git a/src/app/ApplicationTemplate.Shared.Views/Framework/ExtendedSplashscreenController.cs b/src/app/ApplicationTemplate.Shared.Views/Framework/ExtendedSplashscreenController.cs index 6dcfb590..75f4a473 100644 --- a/src/app/ApplicationTemplate.Shared.Views/Framework/ExtendedSplashscreenController.cs +++ b/src/app/ApplicationTemplate.Shared.Views/Framework/ExtendedSplashscreenController.cs @@ -1,4 +1,5 @@ -using Microsoft.UI.Dispatching; +using CommunityToolkit.WinUI; +using Microsoft.UI.Dispatching; namespace ApplicationTemplate; @@ -13,7 +14,7 @@ public ExtendedSplashscreenController(DispatcherQueue dispatcherQueue) public void Dismiss() { - _ = _dispatcherQueue.RunAsync(DispatcherQueuePriority.Normal, DismissSplashScreen); + _ = _dispatcherQueue.EnqueueAsync(DismissSplashScreen); void DismissSplashScreen() // Runs on UI thread. { diff --git a/src/app/ApplicationTemplate.Shared.Views/Framework/Launcher/LauncherService.cs b/src/app/ApplicationTemplate.Shared.Views/Framework/Launcher/LauncherService.cs index 8eba2985..e1c75f81 100644 --- a/src/app/ApplicationTemplate.Shared.Views/Framework/Launcher/LauncherService.cs +++ b/src/app/ApplicationTemplate.Shared.Views/Framework/Launcher/LauncherService.cs @@ -1,5 +1,6 @@ using System; using System.Threading.Tasks; +using CommunityToolkit.WinUI; using Microsoft.UI.Dispatching; namespace ApplicationTemplate; @@ -15,37 +16,15 @@ public LauncherService(DispatcherQueue dispatcherQueue) public async Task Launch(Uri uri) { - var launchSucceeded = await DispatcherRunTaskAsync(DispatcherQueuePriority.Normal, async () => await Windows.System.Launcher.LaunchUriAsync(uri)); + var launchSucceeded = await _dispatcherQueue.EnqueueAsync(InnerLaunch, DispatcherQueuePriority.Normal); if (!launchSucceeded) { throw new LaunchFailedException($"Failed to launch URI: {uri}"); } - } - - /// - /// This method allows for executing an async Task with result on the . - /// - /// The result type. - /// . - /// A function that will be execute on the . - /// of . - private async Task DispatcherRunTaskAsync(DispatcherQueuePriority dispatcherQueuePriority, Func> asyncFunc) - { - var completion = new TaskCompletionSource(); - await _dispatcherQueue.RunAsync(dispatcherQueuePriority, RunActionUI); - return await completion.Task; - async void RunActionUI() + async Task InnerLaunch() { - try - { - var result = await asyncFunc(); - completion.SetResult(result); - } - catch (Exception exception) - { - completion.SetException(new LaunchFailedException("An error occured while trying to launch an URI on the UI thread.", exception)); - } + return await Windows.System.Launcher.LaunchUriAsync(uri); } } } diff --git a/src/app/ApplicationTemplate.Shared.Views/Helpers/DispatcherQueueExtensions.cs b/src/app/ApplicationTemplate.Shared.Views/Helpers/DispatcherQueueExtensions.cs deleted file mode 100644 index 1a8d1b15..00000000 --- a/src/app/ApplicationTemplate.Shared.Views/Helpers/DispatcherQueueExtensions.cs +++ /dev/null @@ -1,237 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/main/License.md -// See reference: https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/main/Microsoft.Toolkit.Uwp/Extensions/DispatcherQueueExtensions.cs - -using System; -using System.Runtime.CompilerServices; -using System.Threading.Tasks; - -namespace Microsoft.UI.Dispatching; - -internal static class DispatcherQueueExtensions -{ - /// - /// Invokes a given function on the target and returns a - /// that completes when the invocation of the function is completed. - /// - /// The target to invoke the code on. - /// The priority level for the function to invoke. - /// The to invoke. - /// A that completes when the invocation of is over. - /// If the current thread has access to , will be invoked directly. - internal static Task RunAsync(this DispatcherQueue dispatcher, DispatcherQueuePriority priority = DispatcherQueuePriority.Normal, DispatcherQueueHandler handler = default) - { - // Run the function directly when we have thread access. - // Also reuse Task.CompletedTask in case of success, - // to skip an unnecessary heap allocation for every invocation. - if (dispatcher.HasThreadAccess) - { - try - { - handler.Invoke(); - - return Task.CompletedTask; - } - catch (Exception e) - { - return Task.FromException(e); - } - } - - return TryEnqueueAsync(dispatcher, handler, priority); - } - - /// - /// Invokes a given function on the target and returns a - /// that completes when the invocation of the function is completed. - /// - /// The target to invoke the code on. - /// The priority level for the function to invoke. - /// The to invoke. - /// A that completes when the invocation of is over. - /// If the current thread has access to , will be invoked directly. - internal static Task RunTaskAsync(this DispatcherQueue dispatcher, DispatcherQueuePriority priority, Func asyncAction) - { - return EnqueueAsync(dispatcher, asyncAction, priority); - } - - /// - /// Invokes a given function on the target and returns a - /// that completes when the invocation of the function is completed. - /// - /// The target to invoke the code on. - /// The priority level for the function to invoke. - /// The to invoke. - /// The return type of the task. - /// A that completes when the invocation of is over. - /// If the current thread has access to , will be invoked directly. - internal static Task RunTaskAsync(this DispatcherQueue dispatcher, DispatcherQueuePriority priority, Func> asyncAction) - { - return EnqueueAsync(dispatcher, asyncAction, priority); - } - - internal static Task TryEnqueueAsync(DispatcherQueue dispatcher, DispatcherQueueHandler handler, DispatcherQueuePriority priority) - { - var taskCompletionSource = new TaskCompletionSource(); - - if (!dispatcher.TryEnqueue(() => - { - try - { - handler(); - - taskCompletionSource.SetResult(null); - } - catch (Exception e) - { - taskCompletionSource.SetException(e); - } - })) - { - taskCompletionSource.SetException(GetEnqueueException("Failed to enqueue the operation")); - } - - return taskCompletionSource.Task; - } - - /// - /// Invokes a given function on the target and returns a - /// that acts as a proxy for the one returned by the given function. - /// - /// The target to invoke the code on. - /// The to invoke. - /// The priority level for the function to invoke. - /// A that acts as a proxy for the one returned by . - /// If the current thread has access to , will be invoked directly. - internal static Task EnqueueAsync(this DispatcherQueue dispatcher, Func function, DispatcherQueuePriority priority = DispatcherQueuePriority.Normal) - { - // If we have thread access, we can retrieve the task directly. - // We don't use ConfigureAwait(false) in this case, in order - // to let the caller continue its execution on the same thread - // after awaiting the task returned by this function. - if (dispatcher.HasThreadAccess) - { - try - { - if (function() is Task awaitableResult) - { - return awaitableResult; - } - - return Task.FromException(GetEnqueueException("The Task returned by function cannot be null.")); - } - catch (Exception e) - { - return Task.FromException(e); - } - } - - static Task TryEnqueueAsync(DispatcherQueue dispatcher, Func function, DispatcherQueuePriority priority) - { - var taskCompletionSource = new TaskCompletionSource(); - - if (!dispatcher.TryEnqueue(priority, async () => - { - try - { - if (function() is Task awaitableResult) - { - await awaitableResult.ConfigureAwait(false); - - taskCompletionSource.SetResult(null); - } - else - { - taskCompletionSource.SetException(GetEnqueueException("The Task returned by function cannot be null.")); - } - } - catch (Exception e) - { - taskCompletionSource.SetException(e); - } - })) - { - taskCompletionSource.SetException(GetEnqueueException("Failed to enqueue the operation")); - } - - return taskCompletionSource.Task; - } - - return TryEnqueueAsync(dispatcher, function, priority); - } - - /// - /// Invokes a given function on the target and returns a - /// that acts as a proxy for the one returned by the given function. - /// - /// The return type of to relay through the returned . - /// The target to invoke the code on. - /// The to invoke. - /// The priority level for the function to invoke. - /// A that relays the one returned by . - /// If the current thread has access to , will be invoked directly. - internal static Task EnqueueAsync(this DispatcherQueue dispatcher, Func> function, DispatcherQueuePriority priority = DispatcherQueuePriority.Normal) - { - if (dispatcher.HasThreadAccess) - { - try - { - if (function() is Task awaitableResult) - { - return awaitableResult; - } - - return Task.FromException(GetEnqueueException("The Task returned by function cannot be null.")); - } - catch (Exception e) - { - return Task.FromException(e); - } - } - - static Task TryEnqueueAsync(DispatcherQueue dispatcher, Func> function, DispatcherQueuePriority priority) - { - var taskCompletionSource = new TaskCompletionSource(); - - if (!dispatcher.TryEnqueue(priority, async () => - { - try - { - if (function() is Task awaitableResult) - { - var result = await awaitableResult.ConfigureAwait(false); - - taskCompletionSource.SetResult(result); - } - else - { - taskCompletionSource.SetException(GetEnqueueException("The Task returned by function cannot be null.")); - } - } - catch (Exception e) - { - taskCompletionSource.SetException(e); - } - })) - { - taskCompletionSource.SetException(GetEnqueueException("Failed to enqueue the operation")); - } - - return taskCompletionSource.Task; - } - - return TryEnqueueAsync(dispatcher, function, priority); - } - - /// - /// Creates an to return when an enqueue operation fails. - /// - /// The message of the exception. - /// An with a specified message. - [MethodImpl(MethodImplOptions.NoInlining)] - internal static InvalidOperationException GetEnqueueException(string message) - { - return new InvalidOperationException(message); - } -} diff --git a/src/app/ApplicationTemplate.Shared.Views/Startup.cs b/src/app/ApplicationTemplate.Shared.Views/Startup.cs index ce100b23..570df1d0 100644 --- a/src/app/ApplicationTemplate.Shared.Views/Startup.cs +++ b/src/app/ApplicationTemplate.Shared.Views/Startup.cs @@ -9,6 +9,7 @@ using Chinook.DataLoader; using Chinook.DynamicMvvm; using Chinook.SectionsNavigation; +using CommunityToolkit.WinUI; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; @@ -125,7 +126,7 @@ private static void HandleUnhandledExceptions(IServiceProvider services) private static async Task SetShellViewModel() { - await App.Instance.Shell.DispatcherQueue.RunAsync(DispatcherQueuePriority.Normal, SetDataContextUI); + await App.Instance.Shell.DispatcherQueue.EnqueueAsync(SetDataContextUI); static void SetDataContextUI() // Runs on UI thread. { @@ -159,7 +160,7 @@ private void HandleSystemBackVisibility(IServiceProvider services) void OnStateChanged(bool canNavigateBackOrCloseModal) { var dispatcherQueue = services.GetRequiredService(); - _ = dispatcherQueue.RunAsync(DispatcherQueuePriority.Normal, UpdateBackButtonUI); + _ = dispatcherQueue.EnqueueAsync(UpdateBackButtonUI); void UpdateBackButtonUI() // Runs on UI thread. { @@ -181,7 +182,7 @@ private async Task AddSystemBackButtonSource(IServiceProvider services) #if __ANDROID__ || __IOS__ var dispatcherQueue = services.GetRequiredService(); var backButtonManager = services.GetRequiredService(); - await dispatcherQueue.RunAsync(DispatcherQueuePriority.High, AddSystemBackButtonSourceInner); + await dispatcherQueue.EnqueueAsync(AddSystemBackButtonSourceInner, DispatcherQueuePriority.High); // Runs on main thread. void AddSystemBackButtonSourceInner() diff --git a/src/app/ApplicationTemplate.Windows/ApplicationTemplate.Windows.csproj b/src/app/ApplicationTemplate.Windows/ApplicationTemplate.Windows.csproj index e7f82383..0d754b43 100644 --- a/src/app/ApplicationTemplate.Windows/ApplicationTemplate.Windows.csproj +++ b/src/app/ApplicationTemplate.Windows/ApplicationTemplate.Windows.csproj @@ -72,6 +72,7 @@ +