Port remaining FxCop rules into .NET as Roslyn Analyzers

[Evangelink]

In roslyn-analyzers there is a bunch of tickets open related to porting some old FxCop rules. Because most of these are dated from 2015, we were wondering if it would be worth having them under review as their relevance or the context in which they apply may have changed.

I have tried to filter down the ones related to .NET API but I may have missed or included incorrect ones (if you want to review them all, please look at https://github.com/dotnet/roslyn-analyzers/issues?q=is%3Aopen+label%3AFxCop-Port+-label%3A%22Needs-Fixer%22):

(tagging @mavasani )

Native resources/Interop

• Port FxCop rule CA1049: TypesThatOwnNativeResourcesShouldBeDisposable roslyn-analyzers#395
• Port FxCop rule CA1402: AvoidOverloadsInComVisibleInterfaces roslyn-analyzers#418
• Port FxCop rule CA1403: AutoLayoutTypesShouldNotBeComVisible roslyn-analyzers#419
• Port FxCop rule CA1404: CallGetLastErrorImmediatelyAfterPInvoke roslyn-analyzers#420
• Port FxCop rule CA1405: ComVisibleTypeBaseTypesShouldBeComVisible roslyn-analyzers#421
• Port FxCop rule CA1407: AvoidStaticMembersInComVisibleTypes roslyn-analyzers#423
• Port FxCop rule CA1408: DoNotUseAutoDualClassInterfaceType roslyn-analyzers#424
• Port FxCop rule CA1409: ComVisibleTypesShouldBeCreatable roslyn-analyzers#425
• Port FxCop rule CA1410: ComRegistrationMethodsShouldBeMatched roslyn-analyzers#426
• Port FxCop rule CA1411: ComRegistrationMethodsShouldNotBeVisible roslyn-analyzers#427
• Port FxCop rule CA1412: MarkComSourceInterfacesAsIDispatch roslyn-analyzers#428
• Port FxCop rule CA1413: AvoidNonpublicFieldsInComVisibleValueTypes roslyn-analyzers#429
• Port FxCop rule CA1414: MarkBooleanPInvokeArgumentsWithMarshalAs roslyn-analyzers#430
• Port FxCop rule CA1415: DeclarePInvokesCorrectly roslyn-analyzers#431
• Port FxCop rule CA2004: RemoveCallsToGCKeepAlive roslyn-analyzers#479
• Port FxCop rule CA2006: UseSafeHandleToEncapsulateNativeResources roslyn-analyzers#480
• Port FxCop rule CA2205: UseManagedEquivalentsOfWin32Api roslyn-analyzers#530

WinForms

• Port FxCop rule CA1300: SpecifyMessageBoxOptions roslyn-analyzers#408
• Port FxCop rule CA1301: AvoidDuplicateAccelerators roslyn-analyzers#409
• Port FxCop rule CA2232: MarkWindowsFormsEntryPointsWithStaThread roslyn-analyzers#545

Serialization

• Port FxCop rule CA2236: CallBaseClassMethodsOnISerializableTypes roslyn-analyzers#548
• Port FxCop rule CA2238: ImplementSerializationMethodsCorrectly roslyn-analyzers#549
• Port FxCop rule CA2239: ProvideDeserializationMethodsForOptionalFields roslyn-analyzers#550
• Port FxCop rule CA2240: ImplementISerializableCorrectly roslyn-analyzers#551

Others

• Port FxCop rule CA1013: OverloadOperatorEqualsOnOverloadingAddAndSubtract #64647
• Port FxCop rule CA1023: IndexersShouldNotBeMultidimensional roslyn-analyzers#378
• Port FxCop rule CA1057: StringUriOverloadsCallSystemUriOverloads roslyn-analyzers#401
• Port FxCop rule CA1306: SetLocaleForDataTypes roslyn-analyzers#414
• Port FxCop rule CA1600: DoNotUseIdleProcessPriority roslyn-analyzers#438
• Port FxCop rule CA1601: DoNotUseTimersThatPreventPowerStateChanges roslyn-analyzers#439
• Port FxCop rule CA2001: AvoidCallingProblematicMethods roslyn-analyzers#477

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[mavasani]

@jmarolf @mikadumont

[stephentoub]

We should just close any of these that are no longer relevant / stale / etc.

@AaronRobinsonMSFT / @jkoritzinsky / @elinor-fung to comment on the interop rules.

I suggest we close all of the serialization ones. We're in the process of deprecating BinaryFormatter, we're no longer encouraging implementation of ISerializable nor implementing it ourselves on new types, etc. @GrabYourPitchforks

@merriemcgaw for WinForms.

[Evangelink]

@stephentoub Out of curiosity, are you simply no longer encouraging the implementation of ISerializable or shall we go even one step further and have analyzers guarding against (there could be a dotnet version check)?

[merriemcgaw]

@RussKie can you look at the WinForms ones and see if they make any sense in the current context?

[GrabYourPitchforks]

@Evangelink Basically, we're no longer marking any new types as [Serializable] or applying the ISerializable interface. There are some exceptions for the sake of compatibility, but those use cases are definitely in the minority.

[RussKie]

WinForms:

• Port FxCop rule CA1300: SpecifyMessageBoxOptions roslyn-analyzers#408
• Port FxCop rule CA1301: AvoidDuplicateAccelerators roslyn-analyzers#409
• Port FxCop rule CA2232: MarkWindowsFormsEntryPointsWithStaThread roslyn-analyzers#545

These look worthy of a port.

Over the months we had several team discussions and bounced ideas about how we can enhance the developer and end user experiences with various Windows Forms analyzers. I also had an offline thread with @jmarolf @sharwell @mavasani about the same subject, and for the general purpose analyzers (like these) https://github.com/dotnet/roslyn-analyzers looked like a good home.

[AaronRobinsonMSFT]

The Interop team has gone through the list. The below list contains what we think makes sense to port. All others can be closed.

dotnet/roslyn-analyzers#420
dotnet/roslyn-analyzers#421
dotnet/roslyn-analyzers#425
dotnet/roslyn-analyzers#428
dotnet/roslyn-analyzers#430

[Evangelink]

@AaronRobinsonMSFT Would it be possible to get a short description on why the team thinks the following two rules are not worth being implemented?
dotnet/roslyn-analyzers#395
dotnet/roslyn-analyzers#530

[Evangelink]

@stephentoub @GrabYourPitchforks Could you please confirm that's ok to close all the tickets about serialization?

The tickets in the Other section are still waiting for review/decision.

[AaronRobinsonMSFT]

@Evangelink Sure. Would you like the comments here or in the specific issues?

[Evangelink]

Maybe directly on the tickets but I am fine here too. The two rules seem to make sense to me (like if you own native or disposable resources you must be disposable, and for the 2nd what's the point of using an interop if the fwk allows for the same fnctionality).

[AaronRobinsonMSFT]

@Evangelink I've commented on each issue directly.

[stephentoub]

@stephentoub @GrabYourPitchforks Could you please confirm that's ok to close all the tickets about serialization?

Yes, go ahead. Thanks.

[jeffhandley]

@buyaa-n & @carlossanlop Please review the analyzers in the 'Other' category to determine if we want to bring those for API Review and include them in our plans. Thanks!

[jeffhandley]

that's ok to close all the tickets about serialization

Done

[jeffhandley]

We were not able to finish this list. Changing the milestone to Future to keep the epic alive. More of these analyzers will be considered as part of .NET 8 planning.