Mark the Form.OnClosed and OnClosing methods with the ObsoleteAttribute

[geniuszxy]

Background and motivation

Since the document mentions that these two methods have been obsolete for years, marking them with the ObsoleteAttribute can help us avoid using them when coding.

API Proposal

namespace System.Windows.Forms;

public partial class Form
{
    [System.Obsolete("The OnClosed method is obsolete starting with the .NET Framework 2.0; use the OnFormClosed method instead.")]
    protected virtual void OnClosed (EventArgs e);

    [System.Obsolete("The OnClosing method is obsolete starting with the .NET Framework 2.0; use the OnFormClosing method instead.")]
    protected virtual void OnClosing (System.ComponentModel.CancelEventArgs e);

    [System.ComponentModel.Browsable(false)]
    [System.Obsolete("The Closed event is obsolete in the .NET Framework version 2.0; use the FormClosed event instead.")]
    public event EventHandler? Closed;

    [System.ComponentModel.Browsable(false)]
    [System.Obsolete("The Closing event is obsolete starting with the .NET Framework 2.0; use the FormClosing event instead.")]
    public event System.ComponentModel.CancelEventHandler? Closing;
}

API Usage

No.

Alternative Designs

No response

Risks

No response

Will this feature affect UI controls?

No.

[paul1956]

Does Visual Studio understand Obsolete Attribute? Will it still show up in form DropDown list if they are not used, is it different if they are used?

[geniuszxy]

@paul1956 Visual Studio understand ObsoleteAttribute, decorated members will still appear in the dropdown list, but they will be marked as deprecated.

If you use them, the compiler will also provide warnings.

[elachlan]

@geniuszxy would you like to submit a PR?

[PaulusParssinen]

geniuszxy would you like to submit a PR?

I believe making public API surface obsolete requires an API review first? (atleast in runtime)

[geniuszxy]

@geniuszxy would you like to submit a PR?

Can I submit directly? Or is it lonitra who is making this change now?

[lonitra]

@geniuszxy Thank you for submitting this issue! If you are interested, please feel free to submit a PR and I can help shuffle this along to API review to get approval.

[terrajobst]

• The APIs should be marked [EditorBrowsable(BrowsableState.Never)] to avoid them showing up in the code editor
• Assign a single diagnostic ID to all of them, as they are logically the same obsoletion
• We should remove the mention of when it was obsoleted because it makes the message longer and doesn't add anything

namespace System.Windows.Forms;

public partial class Form
{
    [EditorBrowsable(BrowsableState.Never)]
    [Obsolete("The OnClosed method is obsolete; use the OnFormClosed method instead.",
              DiagnosticId="<Pick Next ID>")]
    protected virtual void OnClosed(EventArgs e);

    [EditorBrowsable(BrowsableState.Never)]
    [Obsolete("The OnClosing method is obsolete; use the OnFormClosing method instead.",
              DiagnosticId="<Pick Next ID>"))]
    protected virtual void OnClosing(CancelEventArgs e);

    [Browsable(false)]
    [EditorBrowsable(BrowsableState.Never)]
    [Obsolete("The Closed event is obsolete; use the FormClosed event instead.",
              DiagnosticId="<Pick Next ID>"))]
    public event EventHandler? Closed;

    [Browsable(false)]
    [EditorBrowsable(BrowsableState.Never)]
    [Obsolete("The Closing event is obsolete; use the FormClosing event instead.",
              DiagnosticId="<Pick Next ID>"))]
    public event CancelEventHandler? Closing;
}

[RussKie]

Does the obsoletion need to be an error - that is, [Obsolete(..., error: true, ...)]? Or should it remain as a warning?

[terrajobst]

We generally don't obsolete as error as it disrupts the inner loop (developer has to fix errors before they can build/run). So unless there is a strong reason I wouldn't do it here either.

[Syareel-Sukeri]

Verified this issue in the latest .NET9.0 SDK build: 9.0.100-rc.2.24420.21 + dlls built from Winforms main branch, the Form.OnClosed and Form.OnClosing methods, as well as their override methods, are now marked with the [Obsolete] attribute.