Skip to content

Commit

Permalink
Address review feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
KlausLoeffelmann committed Aug 11, 2024
1 parent 7c8aab6 commit 36e5c27
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace System.Windows.Forms.Analyzers.Diagnostics;

internal static class DiagnosticIDs
{
public const string UrlFormat = "https://aka.ms/winforms-experimental/{0}";
public const string UrlFormat = "https://aka.ms/winforms-warnings/{0}";

// Application Configuration, number group 0001+
public const string UnsupportedProjectType = "WFO0001";
Expand Down
7 changes: 7 additions & 0 deletions src/System.Windows.Forms/src/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -260,3 +260,10 @@
[assembly: SuppressMessage("Design", "CA1051:Do not declare visible instance fields", Justification = "Public API", Scope = "member", Target = "~F:System.Windows.Forms.BindingManagerBase.onPositionChangedHandler")]
[assembly: SuppressMessage("Design", "CA1051:Do not declare visible instance fields", Justification = "Public API", Scope = "member", Target = "~F:System.Windows.Forms.CurrencyManager.listposition")]
[assembly: SuppressMessage("Design", "CA1051:Do not declare visible instance fields", Justification = "Public API", Scope = "member", Target = "~F:System.Windows.Forms.CurrencyManager.finalType")]
[assembly: SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Analyzer wrongly complains for new APIs - known issue.", Scope = "member", Target = "~M:System.Windows.Forms.Control.InvokeAsync``1(System.Func{``0},System.Threading.CancellationToken)~System.Threading.Tasks.Task{``0}")]
[assembly: SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Analyzer wrongly complains for new APIs - known issue.", Scope = "member", Target = "~M:System.Windows.Forms.Control.InvokeAsync(System.Func{System.Threading.CancellationToken,System.Threading.Tasks.ValueTask},System.Threading.CancellationToken)~System.Threading.Tasks.Task")]
[assembly: SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Analyzer wrongly complains for new APIs - known issue.", Scope = "member", Target = "~M:System.Windows.Forms.Control.InvokeAsync``1(System.Func{System.Threading.CancellationToken,System.Threading.Tasks.ValueTask{``0}},System.Threading.CancellationToken)~System.Threading.Tasks.Task{``0}")]
[assembly: SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Analyzer wrongly complains for new APIs - known issue.", Scope = "member", Target = "~M:System.Windows.Forms.Control.InvokeAsync(System.Action,System.Threading.CancellationToken)~System.Threading.Tasks.Task")]
[assembly: SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Analyzer wrongly complains for new APIs - known issue.", Scope = "member", Target = "~M:System.Windows.Forms.TaskDialog.ShowDialogAsync(System.Windows.Forms.TaskDialogPage,System.Windows.Forms.TaskDialogStartupLocation)~System.Threading.Tasks.Task{System.Windows.Forms.TaskDialogButton}")]
[assembly: SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Analyzer wrongly complains for new APIs - known issue.", Scope = "member", Target = "~M:System.Windows.Forms.TaskDialog.ShowDialogAsync(System.Windows.Forms.IWin32Window,System.Windows.Forms.TaskDialogPage,System.Windows.Forms.TaskDialogStartupLocation)~System.Threading.Tasks.Task{System.Windows.Forms.TaskDialogButton}")]
[assembly: SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Analyzer wrongly complains for new APIs - known issue.", Scope = "member", Target = "~M:System.Windows.Forms.TaskDialog.ShowDialogAsync(System.IntPtr,System.Windows.Forms.TaskDialogPage,System.Windows.Forms.TaskDialogStartupLocation)~System.Threading.Tasks.Task{System.Windows.Forms.TaskDialogButton}")]
7 changes: 3 additions & 4 deletions src/System.Windows.Forms/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
static System.Windows.Forms.TaskDialog.ShowDialogAsync(nint hwndOwner, System.Windows.Forms.TaskDialogPage! page) -> System.Threading.Tasks.Task<System.Windows.Forms.TaskDialogButton!>!
static System.Windows.Forms.TaskDialog.ShowDialogAsync(nint hwndOwner, System.Windows.Forms.TaskDialogPage! page, System.Windows.Forms.TaskDialogStartupLocation startupLocation) -> System.Threading.Tasks.Task<System.Windows.Forms.TaskDialogButton!>!
static System.Windows.Forms.TaskDialog.ShowDialogAsync(System.Windows.Forms.TaskDialogPage! page) -> System.Threading.Tasks.Task<System.Windows.Forms.TaskDialogButton!>!
static System.Windows.Forms.TaskDialog.ShowDialogAsync(System.Windows.Forms.TaskDialogPage! page, System.Windows.Forms.TaskDialogStartupLocation startupLocation) -> System.Threading.Tasks.Task<System.Windows.Forms.TaskDialogButton!>!
static System.Windows.Forms.TaskDialog.ShowDialogAsync(nint hwndOwner, System.Windows.Forms.TaskDialogPage! page, System.Windows.Forms.TaskDialogStartupLocation startupLocation = System.Windows.Forms.TaskDialogStartupLocation.CenterOwner) -> System.Threading.Tasks.Task<System.Windows.Forms.TaskDialogButton!>!
static System.Windows.Forms.TaskDialog.ShowDialogAsync(System.Windows.Forms.IWin32Window! owner, System.Windows.Forms.TaskDialogPage! page, System.Windows.Forms.TaskDialogStartupLocation startupLocation = System.Windows.Forms.TaskDialogStartupLocation.CenterOwner) -> System.Threading.Tasks.Task<System.Windows.Forms.TaskDialogButton!>!
static System.Windows.Forms.TaskDialog.ShowDialogAsync(System.Windows.Forms.TaskDialogPage! page, System.Windows.Forms.TaskDialogStartupLocation startupLocation = System.Windows.Forms.TaskDialogStartupLocation.CenterScreen) -> System.Threading.Tasks.Task<System.Windows.Forms.TaskDialogButton!>!
System.Windows.Forms.Control.InvokeAsync(System.Action! callback, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task!
System.Windows.Forms.Control.InvokeAsync(System.Func<System.Threading.CancellationToken, System.Threading.Tasks.ValueTask>! callback, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task!
System.Windows.Forms.Control.InvokeAsync<T>(System.Func<System.Threading.CancellationToken, System.Threading.Tasks.ValueTask<T>>! callback, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<T>!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ public partial class Control
/// <see cref="InvokeAsync{T}(Func{CancellationToken, ValueTask{T}}, CancellationToken)"/>.
/// </para>
/// </remarks>
#pragma warning disable RS0026 // API with optional parameter(s) should have the most parameters amongst its public overloads
public async Task InvokeAsync(Action callback, CancellationToken cancellationToken = default)
#pragma warning restore RS0026 // API with optional parameter(s) should have the most parameters amongst its public overloads
{
ArgumentNullException.ThrowIfNull(callback);

Expand All @@ -38,12 +36,12 @@ public async Task InvokeAsync(Action callback, CancellationToken cancellationTok
return;
}

TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously);
TaskCompletionSource completion = new(TaskCreationOptions.RunContinuationsAsynchronously);

using (cancellationToken.Register(() => tcs.SetCanceled(), useSynchronizationContext: false))
using (cancellationToken.Register(() => completion.SetCanceled(), useSynchronizationContext: false))
{
BeginInvoke(WrappedAction);
await tcs.Task.ConfigureAwait(false);
await completion.Task.ConfigureAwait(false);
}

void WrappedAction()
Expand All @@ -52,16 +50,16 @@ void WrappedAction()
{
if (cancellationToken.IsCancellationRequested)
{
tcs.SetCanceled(cancellationToken);
completion.SetCanceled(cancellationToken);
return;
}

callback();
tcs.TrySetResult();
completion.TrySetResult();
}
catch (Exception ex)
{
tcs.TrySetException(ex);
completion.TrySetException(ex);
}
}
}
Expand Down Expand Up @@ -95,9 +93,7 @@ void WrappedAction()
/// <see cref="InvokeAsync{T}(Func{CancellationToken, ValueTask{T}}, CancellationToken)"/>.
/// </para>
/// </remarks>
#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
public async Task<T> InvokeAsync<T>(Func<T> callback, CancellationToken cancellationToken = default)
#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
{
ArgumentNullException.ThrowIfNull(callback);

Expand All @@ -106,12 +102,12 @@ public async Task<T> InvokeAsync<T>(Func<T> callback, CancellationToken cancella
return default!;
}

TaskCompletionSource<T> tcs = new(TaskCreationOptions.RunContinuationsAsynchronously);
TaskCompletionSource<T> completion = new(TaskCreationOptions.RunContinuationsAsynchronously);

using (cancellationToken.Register(() => tcs.SetCanceled(), useSynchronizationContext: false))
using (cancellationToken.Register(() => completion.SetCanceled(), useSynchronizationContext: false))
{
BeginInvoke(WrappedCallback);
return await tcs.Task.ConfigureAwait(false);
return await completion.Task.ConfigureAwait(false);
}

void WrappedCallback()
Expand All @@ -120,16 +116,16 @@ void WrappedCallback()
{
if (cancellationToken.IsCancellationRequested)
{
tcs.TrySetCanceled(cancellationToken);
completion.TrySetCanceled(cancellationToken);
return;
}

T result = callback();
tcs.TrySetResult(result);
completion.TrySetResult(result);
}
catch (Exception ex)
{
tcs.TrySetException(ex);
completion.TrySetException(ex);
}
}
}
Expand Down Expand Up @@ -162,9 +158,7 @@ void WrappedCallback()
/// <see cref="InvokeAsync(Action, CancellationToken)"/>.
/// </para>
/// </remarks>
#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
public async Task InvokeAsync(Func<CancellationToken, ValueTask> callback, CancellationToken cancellationToken = default)
#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
{
ArgumentNullException.ThrowIfNull(callback);

Expand All @@ -173,12 +167,12 @@ public async Task InvokeAsync(Func<CancellationToken, ValueTask> callback, Cance
return;
}

TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously);
TaskCompletionSource completion = new(TaskCreationOptions.RunContinuationsAsynchronously);

using (cancellationToken.Register(() => tcs.SetCanceled(), useSynchronizationContext: false))
using (cancellationToken.Register(() => completion.SetCanceled(), useSynchronizationContext: false))
{
BeginInvoke(async () => await WrappedCallbackAsync().ConfigureAwait(false));
await tcs.Task.ConfigureAwait(false);
await completion.Task.ConfigureAwait(false);
}

async Task WrappedCallbackAsync()
Expand All @@ -187,16 +181,16 @@ async Task WrappedCallbackAsync()
{
if (cancellationToken.IsCancellationRequested)
{
tcs.TrySetCanceled(cancellationToken);
completion.TrySetCanceled(cancellationToken);
return;
}

await callback(cancellationToken).ConfigureAwait(false);
tcs.TrySetResult();
completion.TrySetResult();
}
catch (Exception ex)
{
tcs.TrySetException(ex);
completion.TrySetException(ex);
}
}
}
Expand Down Expand Up @@ -228,9 +222,7 @@ async Task WrappedCallbackAsync()
/// <see cref="InvokeAsync(Action, CancellationToken)"/>.
/// </para>
/// </remarks>
#pragma warning disable RS0026 // API with optional parameter(s) should have the most parameters amongst its public overloads
public async Task<T> InvokeAsync<T>(Func<CancellationToken, ValueTask<T>> callback, CancellationToken cancellationToken = default)
#pragma warning restore RS0026 // API with optional parameter(s) should have the most parameters amongst its public overloads
{
ArgumentNullException.ThrowIfNull(callback);

Expand All @@ -239,24 +231,24 @@ public async Task<T> InvokeAsync<T>(Func<CancellationToken, ValueTask<T>> callba
return default!;
}

TaskCompletionSource<T> tcs = new(TaskCreationOptions.RunContinuationsAsynchronously);
TaskCompletionSource<T> completion = new(TaskCreationOptions.RunContinuationsAsynchronously);

using (cancellationToken.Register(() => tcs.SetCanceled(), useSynchronizationContext: false))
using (cancellationToken.Register(() => completion.SetCanceled(), useSynchronizationContext: false))
{
BeginInvoke(async () => await WrappedCallbackAsync().ConfigureAwait(false));
return await tcs.Task.ConfigureAwait(false);
return await completion.Task.ConfigureAwait(false);
}

async Task WrappedCallbackAsync()
{
try
{
var returnValue = await callback(cancellationToken).ConfigureAwait(false);
tcs.TrySetResult(returnValue);
completion.TrySetResult(returnValue);
}
catch (Exception ex)
{
tcs.TrySetException(ex);
completion.TrySetException(ex);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,35 +304,8 @@ private static TaskDialogButton CreatePlaceholderButton(TaskDialogResult result)
/// <param name="page">
/// The page instance that contains the contents which this task dialog will display.
/// </param>
/// <remarks>
/// <para>
/// Showing the dialog will bind the <paramref name="page"/> and its controls until
/// this method returns or the dialog is navigated to a different page.
/// </para>
/// </remarks>
/// <returns>
/// The <see cref="TaskDialogButton"/> which was clicked by the user to close the dialog.
/// </returns>
/// <exception cref="ArgumentNullException">
/// <paramref name="page"/> is <see langword="null"/>.
/// </exception>
/// <exception cref="InvalidOperationException">
/// The specified <paramref name="page"/> contains an invalid configuration.
/// </exception>
[Experimental(DiagnosticIDs.ExperimentalAsync, UrlFormat = DiagnosticIDs.UrlFormat)]
public static Task<TaskDialogButton> ShowDialogAsync(
TaskDialogPage page)
=> ShowDialogAsync(IntPtr.Zero, page.OrThrowIfNull(), TaskDialogStartupLocation.CenterOwner);

/// <summary>
/// Shows the task dialog with the specified owner asynchronously.
/// </summary>
/// <param name="page">
/// The page instance that contains the contents which this task dialog will display.
/// </param>
/// <param name="hwndOwner">
/// The handle of the owner window, or <see cref="IntPtr.Zero"/> to show a
/// modeless dialog.
/// <param name="startupLocation">
/// Gets or sets the position of the task dialog when it is shown.
/// </param>
/// <remarks>
/// <para>
Expand All @@ -351,16 +324,19 @@ public static Task<TaskDialogButton> ShowDialogAsync(
/// </exception>
[Experimental(DiagnosticIDs.ExperimentalAsync, UrlFormat = DiagnosticIDs.UrlFormat)]
public static Task<TaskDialogButton> ShowDialogAsync(
IntPtr hwndOwner,
TaskDialogPage page)
=> ShowDialogAsync(IntPtr.Zero, page.OrThrowIfNull(), TaskDialogStartupLocation.CenterOwner);
TaskDialogPage page,
TaskDialogStartupLocation startupLocation = TaskDialogStartupLocation.CenterScreen)
=> ShowDialogAsync(IntPtr.Zero, page.OrThrowIfNull(), startupLocation);

/// <summary>
/// Shows the task dialog with the specified owner asynchronously.
/// </summary>
/// <param name="page">
/// The page instance that contains the contents which this task dialog will display.
/// </param>
/// <param name="owner">
/// The owner window.
/// </param>
/// <param name="startupLocation">
/// Gets or sets the position of the task dialog when it is shown.
/// </param>
Expand All @@ -381,9 +357,10 @@ public static Task<TaskDialogButton> ShowDialogAsync(
/// </exception>
[Experimental(DiagnosticIDs.ExperimentalAsync, UrlFormat = DiagnosticIDs.UrlFormat)]
public static Task<TaskDialogButton> ShowDialogAsync(
IWin32Window owner,
TaskDialogPage page,
TaskDialogStartupLocation startupLocation)
=> ShowDialogAsync(IntPtr.Zero, page.OrThrowIfNull(), startupLocation);
TaskDialogStartupLocation startupLocation = TaskDialogStartupLocation.CenterOwner) =>
ShowDialogAsync(owner.Handle, page, startupLocation);

/// <summary>
/// Shows the task dialog with the specified owner asynchronously.
Expand All @@ -392,8 +369,7 @@ public static Task<TaskDialogButton> ShowDialogAsync(
/// The page instance that contains the contents which this task dialog will display.
/// </param>
/// <param name="hwndOwner">
/// The handle of the owner window, or <see cref="IntPtr.Zero"/> to show a
/// modeless dialog.
/// The handle of the owner window, or <see cref="IntPtr.Zero"/> to show a modeless dialog.
/// </param>
/// <param name="startupLocation">
/// Gets or sets the position of the task dialog when it is shown.
Expand All @@ -413,14 +389,15 @@ public static Task<TaskDialogButton> ShowDialogAsync(
/// <exception cref="InvalidOperationException">
/// The specified <paramref name="page"/> contains an invalid configuration.
/// </exception>
[Experimental(DiagnosticIDs.ExperimentalAsync, UrlFormat = DiagnosticIDs.UrlFormat)]
public static async Task<TaskDialogButton> ShowDialogAsync(
IntPtr hwndOwner,
nint hwndOwner,
TaskDialogPage page,
TaskDialogStartupLocation startupLocation)
TaskDialogStartupLocation startupLocation = TaskDialogStartupLocation.CenterOwner)
{
ArgumentNullException.ThrowIfNull(page);

var tcs = new TaskCompletionSource<TaskDialogButton>();
var completion = new TaskCompletionSource<TaskDialogButton>();

TaskDialog? dialog = null;

Expand All @@ -436,19 +413,19 @@ public static async Task<TaskDialogButton> ShowDialogAsync(

TaskDialogButton result;

result = await tcs.Task.ConfigureAwait(true);
result = await completion.Task.ConfigureAwait(true);
return result;

void ShowDialogProc()
{
try
{
dialog = new();
tcs.TrySetResult(dialog.ShowDialogInternal(hwndOwner, page, startupLocation));
completion.TrySetResult(dialog.ShowDialogInternal(hwndOwner, page, startupLocation));
}
catch (Exception ex)
{
tcs.TrySetException(ex);
completion.TrySetException(ex);
}
}
}
Expand Down
Loading

0 comments on commit 36e5c27

Please sign in to comment.