Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add hollow types for deprecated UI controls #10913

Conversation

SimonZhao888
Copy link
Member

@SimonZhao888 SimonZhao888 commented Feb 20, 2024

Fixes #3783

API review

notes - #3783 (comment) and in the video

Proposed changes

Add controls removed in NET3.1 Back to .NET10 for binary compatibility.

  • None of the re-added types can be constructed, all constructors throw new PlatformNotSupportedException(), default constructors are suppressed by introducing throwing public constructors if needed.
  • All attributes that have been present on the .NET Framework types, are preserved, as they are accessible by reflection.
  • All re-introduced types are decorated with [EditorBrowsable(EditorBrowsableState.Never)] attribute that hides them from the intellisense. Type names can be typed in by the developer manually and intellisense would show the same members as it did for .NET Framework projects.
  • All re-introduced types are decorated with the ObsoleteAttribute that results in a compile time warning. All types share a single deprecation warning Id, to simplify suppression, but have different explanation messages.
  • All re-introduced types are decorated with [Browsable(false)] attribute to not show custom control properties of these types in the property browser.
  • All public members are re-introduced with their metadata, except for the private attributes.
  • Members inherited from the base classes (Control, for example) are not re-introduced even if they had been overridden in the past because they are not required for binary compatibility, unless they are decorated with different attributes.
  • Members that are re-added to the existing types (Form and Control) are returning defaults or doing nothing.
  • Non-void public or protected instance members on the restored types throw null, not the PlatformNotSupportedException to save space.
  • Void public or protected instance members do nothing.
  • Public or protected fields return default values.
  • Nullability is disabled for all re-added classes, structs and delegates for compatibility with the .NET Framework.

Test methodology

  • manually
  • added a new project that targets .NET Framework UnsupportedTypes that instantiate all re-added types. This project is referenced from the unit test assembly. Unit tests verify that methods from NETFX assembly can be resolved, and types are type-forwarded to the hollow implementation in NET10 instead of throwing the missing member exception.
  • compared NETFX public surface generated by Microsoft.DotNet.GenAPI.Tool SDK tool against NET10 public surface generated by the same tool
Microsoft Reviewers: Open in CodeFlow

@Tanya-Solyanik Tanya-Solyanik added the waiting-author-feedback The team requires more information from the author label Mar 1, 2024
@dotnet-policy-service dotnet-policy-service bot removed the waiting-author-feedback The team requires more information from the author label Mar 5, 2024
Copy link

codecov bot commented Mar 11, 2024

Codecov Report

Attention: Patch coverage is 24.62094% with 1044 lines in your changes missing coverage. Please review.

Project coverage is 76.09947%. Comparing base (2b2ce68) to head (355118f).
Report is 14 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #10913         +/-   ##
===================================================
- Coverage   76.21334%   76.09947%   -0.11388%     
===================================================
  Files           3197        3227         +30     
  Lines         640464      641831       +1367     
  Branches       47220       47252         +32     
===================================================
+ Hits          488119      488430        +311     
- Misses        148825      149852       +1027     
- Partials        3520        3549         +29     
Flag Coverage Δ
Debug 76.09947% <24.62094%> (-0.11388%) ⬇️
integration 18.10449% <0.32086%> (-0.07023%) ⬇️
production 50.02050% <11.33690%> (-0.13523%) ⬇️
test 96.98515% <52.22222%> (-0.05365%) ⬇️
unit 47.45664% <11.33690%> (-0.09235%) ⬇️

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

@SimonZhao888 SimonZhao888 marked this pull request as ready for review March 11, 2024 08:24
@SimonZhao888 SimonZhao888 requested a review from a team as a code owner March 11, 2024 08:24
@dotnet-policy-service dotnet-policy-service bot removed the draft draft PR label Mar 11, 2024
@Tanya-Solyanik Tanya-Solyanik added the waiting-author-feedback The team requires more information from the author label Mar 12, 2024
@SimonZhao888 SimonZhao888 force-pushed the Issue_3783_AddUnimplementedTypeForObsoleteControls branch from 287033f to 8dc151f Compare April 2, 2024 01:56
@dotnet-policy-service dotnet-policy-service bot removed the waiting-author-feedback The team requires more information from the author label Apr 2, 2024
@SimonZhao888 SimonZhao888 requested a review from LeafShi1 April 7, 2024 02:52
@ricardobossan
Copy link
Member

LGTM

@Tanya-Solyanik Tanya-Solyanik added waiting-review This item is waiting on review by one or more members of team waiting-on-team This work item needs to be discussed with team or is waiting on team action in order to proceed labels Apr 23, 2024
@lonitra lonitra added this to the .NET 9.0 milestone Jul 10, 2024
Simon Zhao (BEYONDSOFT CONSULTING INC) and others added 17 commits January 17, 2025 14:09
ensure that nullability is disabled on all re-added classes
…oked, thus I changed test to call as many methods as possible directly.
…perties of that type on custom classes (UserControl with a ContextMenu property) are not visible to the PropertyBrowser.

There is no reason to define designer Browsable attribute as false on the properties of the re-added types because the types can't be instantiated on the designer surface since these types can't be constructed.
The EditorBrowsable attribute is more interesting. If it's defined on the class as Never, that class name is not suggested. However if the user types in the class name, all members will be visible to intellisense. To get rid of all members in the intellisense, we should add attribute set to Never to all properties that come from all base classes (using the new modifier). These properties are not needed for binary compatibility. I don't consider the scenario where the user types in the class name as something we should support at the cost of increased size.
… because these attributes are accessible from the type.
… types to the re-added types because otherwise these property would show in the intellisense where they were not shown in the Framework.

Unfortunately, this required re-adding some of the overrides from the base classes.
… override methods or properties are missing on the NET10 types
use single diagnostics ID for add deprecated controls
add/remove methods don't have to throw as they are void
all void methods, don't have to throw, they can do nothing {};
all returning methods can save space and throw null; instead of a real exception; This is the same thing as the ref assemblies do
restore attributes on public members except for private attributes and edditor and property browser attributes that are overriden o the type level.
@Tanya-Solyanik Tanya-Solyanik force-pushed the Issue_3783_AddUnimplementedTypeForObsoleteControls branch from fc3ebbb to 130a079 Compare January 17, 2025 22:09
@Tanya-Solyanik Tanya-Solyanik force-pushed the Issue_3783_AddUnimplementedTypeForObsoleteControls branch from 130a079 to 355118f Compare January 17, 2025 22:54

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 52 out of 67 changed files in this pull request and generated no comments.

Files not reviewed (15)
  • Winforms.sln: Language not supported
  • src/System.Windows.Forms/src/System/Windows/Forms/Controls/Unsupported/DataGrid/DataGridColumnStyle.cs: Evaluated as low risk
  • src/System.Windows.Forms.Design/src/System/ComponentModel/Design/Serialization/CodeDomComponentSerializationService.CodeDomSerializationStore.cs: Evaluated as low risk
  • src/System.Windows.Forms/src/System/Windows/Forms/Controls/Unsupported/ContextMenu/ContextMenu.cs: Evaluated as low risk
  • src/System.Windows.Forms/src/System/Windows/Forms/Controls/Unsupported/DataGrid/DataGridLineStyle.cs: Evaluated as low risk
  • src/System.Windows.Forms.Design/src/System/Resources/Tools/StronglyTypedResourceBuilder.cs: Evaluated as low risk
  • src/System.Windows.Forms/src/GlobalSuppressions.cs: Evaluated as low risk
  • docs/list-of-diagnostics.md: Evaluated as low risk
  • src/Common/src/Obsoletions.cs: Evaluated as low risk
  • src/System.Windows.Forms/src/System/Windows/Forms/Control.cs: Evaluated as low risk
  • src/System.Windows.Forms/src/System/Windows/Forms/Controls/Unsupported/ContextMenu/MenuItem.cs: Evaluated as low risk
  • src/System.Windows.Forms/src/System/Windows/Forms/Controls/Unsupported/ContextMenu/Menu.cs: Evaluated as low risk
  • src/System.Windows.Forms/src/System/Windows/Forms/Controls/Unsupported/DataGrid/DataGridBoolColumn.cs: Evaluated as low risk
  • src/System.Windows.Forms/src/System/Windows/Forms/Controls/Unsupported/DataGrid/DataGridColumnStyle.CompModSwitches.cs: Evaluated as low risk
  • src/System.Windows.Forms/src/System/Windows/Forms/Controls/Unsupported/ContextMenu/MenuMerge.cs: Evaluated as low risk
Comments suppressed due to low confidence (1)

src/System.Windows.Forms/src/System/Windows/Forms/Controls/Unsupported/ContextMenu/Menu.MenuItemCollection.cs:58

  • [nitpick] Add a comment for the AddRange method to maintain consistency with the rest of the code.
public virtual void AddRange(MenuItem[] items) { }
@Tanya-Solyanik
Copy link
Member

To the copilot comment -

Add a comment for the AddRange method to maintain consistency with the rest of the code.

I'm not adding comments to re-added methods inside the re-added types.

Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add unimplemented types for deprecated UI controls
7 participants