Port FxCop rule CA1300: SpecifyMessageBoxOptions

[Youssef1313]

Title: Specify MessageBoxOptions

Description:

To correctly display a message box for cultures that use a right-to-left reading order, the RightAlign and RtlReading members of the MessageBoxOptions enumeration must be passed to the Show method.

https://learn.microsoft.com/en-us/visualstudio/code-quality/ca1300?view=vs-2022&tabs=csharp

[Youssef1313]

If it was decided not to implement this analyzer in dotnet/winforms, please make sure dotnet/roslyn-analyzers#408 is reopened.

[elachlan]

Youssef, are you going to tackle this effort?

[Youssef1313]

I can take a look, sure. Though might not be very soon.

[Youssef1313]

From documentation: https://learn.microsoft.com/en-us/visualstudio/code-quality/ca1300
It seems this rule will flag any call to MessageBox.Show overload that doesn't have MessageBoxOptions parameter. Isn't it easier to just Obsolete those overloads? In both cases, the user will see a warning that he can suppress?

[elachlan]

Good suggestion. I don't ever use MessageBoxOptions myself. I'd say an analyzer is better because its the more correct option and it can be turned off.

Edit: more correct in that obsolete probably isn't the best approach.

[Youssef1313]

and it can be turned off.

Both can be turned off.

public class C
{
    [Obsolete("Don't use this.", DiagnosticId = "MyID")]
    public void M() { }
    
    // This produces a warning with the custom id "MyID"
    // So it can be turned of in .editorconfig via:
    // dotnet_diagnostic.MyID.severity = none
    public void M2() => M();
}

[elachlan]

@RussKie What are your thoughts on using Obsolete? If yes, I'll do a PR today.

[elachlan]

Also, do we still use CA1300 as the DiagnosticId?

[elachlan]

I ran a test with this, I don't like that it specifies that it is obsolete. Because it isn't. Many apps aren't localized. Also the Obsolete attribute can't accept a resource string from SR. Meaning the error description isn't localized. Additionally we don't get to specify a "Title" and the ObsoleteAttribute shows up as a warning by default. Which breaks builds.

[Obsolete("To correctly display a message box for cultures that use a right-to-left reading order, the RightAlign and RtlReading members of the MessageBoxOptions enumeration must be passed to the Show method.", DiagnosticId = "WFMB0001", UrlFormat = "https://learn.microsoft.com/visualstudio/code-quality/ca1300")]

[Youssef1313]

Makes sense @elachlan.

This should be an analyzer then.

Also, do we still use CA1300 as the DiagnosticId?

I'm not sure about that. CAxxxx are known to come from Microsoft.CodeAnalysis.NetAnalyzers, it might be confusing to have CA1300 in winforms. @mavasani What do you think?

[elachlan]

I'd say we would follow the standard set by winforms. That makes the most sense.