Get TrimTest to 0 trim warnings

[LakshanF]

Use the new ComponentModel type registration APIs to make the TrimTest project trim compatible.

• Use a feature switch flag to use the new ComponentModel APIs (non-trimmed applications will continue to use the legacy ComponentModel APIs)
• Some features of WinForms are not supported in trimming and are disabled via feature switches (a complete set can be seen in TrimTest project)
• Primitive types (string, int, bool, double) are not registered with ComponentModel since these are assumed to be supported by default by ComponentModel

Microsoft Reviewers: Open in CodeFlow

[Tanya-Solyanik]

Maybe we need a new helper class that would store "System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization" switch value.

[MichalStrehovsky]

It feels weird to have a feature switch to control use of binary formatter which is also under a feature switch.

We annotated BinaryFormatter as RequiresUnreferencedCode, but at the same time we introduced a feature switch to disable it and defaulted it to "disabled" when trimming. Should we actually drop the RUC on BinaryFormatter? The rationale is that there is no behavioral difference between trimmed and untrimmed app (both throw) and we'll probably generate trimming warnings from BinaryFormatter internals should someone dare to enable the BinaryFormatter feature switch.

It doesn't feel right that people need to create feature switch for a feature switch.

(In a normal PublishTrimmed app, new BinaryFormatter().Deserialize(stream) below is going to throw a NotSupportedException anyway.)

Cc @eerhardt

[eerhardt]

It feels weird to have a feature switch to control use of binary formatter which is also under a feature switch.

DataObject.ComposedDataObject.EnableUnsafeBinaryFormatterInNativeObjectSerialization is just a wrapper over the existing System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization feature switch. It isn't a new feature switch itself.

Should we actually drop the RUC on BinaryFormatter?

Not until BinaryFormatter's logic is actually removed - dotnet/runtime#98245. Once it always throws everywhere - then yes I think we can drop the annotations. No use in giving trim/AOT warnings when it will always throw.

[MichalStrehovsky]

DataObject.ComposedDataObject.EnableUnsafeBinaryFormatterInNativeObjectSerialization is just a wrapper over the existing System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization feature switch. It isn't a new feature switch itself.

There's a comment on this saying it's not the right property to use, so I assume it will change: https://github.com/dotnet/winforms/pull/11407/files#r1607641253

Should we actually drop the RUC on BinaryFormatter?

Not until BinaryFormatter's logic is actually removed - dotnet/runtime#98245. Once it always throws everywhere - then yes I think we can drop the annotations. No use in giving trim/AOT warnings when it will always throw.

The point I'm trying to make is that we generate warnings when there's a difference between F5 Debug time and publish time. In the case of BinaryFormatter in default trimmed configuration, there's no behavior difference. It always throws. The way we've done this elsewhere is that we don't generate a warning and structure things so that if someone changes the feature switch default, then they get a warning (and the warning would be somewhere in the implementation details of it).

But the more I'm thinking about it the more it feels like that's not the right approach either - if someone is a library developer like in this case, they won't know the API could throw until someone uses the library in a trimmed app (it will not be caught in unit tests, etc.). To give a similar example, someone could be using DI with open generics in a library (another spot where we use feature switches to erase differences and make trim unsafe API "safe"), never get any warnings from an analyzer, and only get bug reports from users later that it doesn't work.

[LakshanF]

The feature switch, EnableUnsafeBinaryFormatterInNativeObjectSerialization, using the binary formatter feature switch, is used to control trim warnings related to using BinaryFormatter in WinForms trimming. The trimmer will generate a warning when set to true and will not generate a warning when set to false to work around the issue of the RUC on the BinaryFormatter as mentioned above.

[codecov]

Codecov Report

Attention: Patch coverage is 42.01183% with 98 lines in your changes missing coverage. Please review.

Project coverage is 74.39904%. Comparing base (d8c60bf) to head (4b073f2).
Report is 38 commits behind head on main.

Additional details and impacted files

@@                 Coverage Diff                 @@
##                main      #11407         +/-   ##
===================================================
+ Coverage   74.28601%   74.39904%   +0.11302%     
===================================================
  Files           3026        3030          +4     
  Lines         627075      628039        +964     
  Branches       46755       46803         +48     
===================================================
+ Hits          465829      467255       +1426     
+ Misses        157900      157435        -465     
- Partials        3346        3349          +3     

Flag	Coverage Δ
Debug	74.39904% <42.01183%> (+0.11302%)	⬆️
integration	17.98361% <9.65517%> (-0.01498%)	⬇️
production	47.22281% <33.10345%> (+0.20307%)	⬆️
test	96.98776% <95.83333%> (+0.00075%)	⬆️
unit	44.18112% <33.10345%> (+0.18569%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.