Skip to content

Enable nullability in DesignerActionMethodItem #12676

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 26, 2025
Merged

Enable nullability in DesignerActionMethodItem #12676

merged 1 commit into from
Feb 26, 2025

Conversation

gpetrou
Copy link
Contributor

@gpetrou gpetrou commented Dec 23, 2024 •
edited by dotnet-policy-service bot
Loading

Proposed changes

  • Enable nullability in DesignerActionMethodItem.
Microsoft Reviewers: Open in CodeFlow

@gpetrou gpetrou requested a review from a team as a code owner December 23, 2024 11:41
@gpetrou gpetrou force-pushed the DesignerActionMethodItem branch from 909d588 to e03833e Compare December 23, 2024 11:44
@codecov Codecov
Copy link

codecov bot commented Dec 23, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.04200%. Comparing base (92919a1) to head (cc9a3d1).
Report is 17 commits behind head on main.

Additional details and impacted files 
@@                 Coverage Diff                 @@
##                main      #12676         +/-   ##
===================================================
- Coverage   76.05370%   76.04200%   -0.01171%     
===================================================
  Files           3269        3269                 
  Lines         643544      643572         +28     
  Branches       47429       47430          +1     
===================================================
- Hits          489439      489385         -54     
- Misses        150545      150622         +77     
- Partials        3560        3565          +5
Flag Coverage Δ
Debug 76.04200% <100.00000%> (-0.01171%) ⬇️
integration 18.03442% <0.00000%> (-0.01502%) ⬇️
production 50.01692% <100.00000%> (-0.02357%) ⬇️
test 96.93778% <ø> (+0.00052%) ⬆️
unit 47.47548% <100.00000%> (+0.01057%) ⬆️

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

ricardobossan
ricardobossan reviewed Dec 24, 2024
View reviewed changes
src/System.Windows.Forms.Design/src/System/ComponentModel/Design/DesignerActionMethodItem.cs Outdated

public virtual bool IncludeAsDesignerVerb { get; }

public virtual void Invoke()
{
_methodInfo ??= _actionList?.GetType()?.GetMethod(MemberName, BindingFlags.Default | BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
_methodInfo ??= _actionList?.GetType()?.GetMethod(MemberName!, BindingFlags.Default | BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment explaining in detail why MemberName! can never be null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can actually be null, but I did not want to add an extra exception to it.
@lonitra should I throw an exception instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's appropriate to add a more descriptive exception in this case. InvalidOperationException with message "Could not find method '{0}'."

@Tanya-Solyanik Tanya-Solyanik added the waiting-author-feedback The team requires more information from the author label Jan 7, 2025
@dotnet-policy-service dotnet-policy-service bot removed the waiting-author-feedback The team requires more information from the author label Jan 11, 2025
@LeafShi1 LeafShi1 added the waiting-review This item is waiting on review by one or more members of team label Jan 20, 2025
Copilot
Copilot AI reviewed Jan 21, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • src/System.Windows.Forms.Design/src/PublicAPI.Shipped.txt: Language not supported

@Tanya-Solyanik Tanya-Solyanik added waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels Jan 23, 2025
@gpetrou gpetrou force-pushed the DesignerActionMethodItem branch from e03833e to 6688d48 Compare February 16, 2025 06:45
@gpetrou gpetrou force-pushed the DesignerActionMethodItem branch from 6688d48 to 092797c Compare February 16, 2025 06:47
@Tanya-Solyanik
Copy link
Contributor

System.ComponentModel.Design.Tests.DesignerActionMethodItemTests.Invoke_NullMemberName_ThrowsArgumentNullException

Assert.Throws() Failure: Exception type was not an exact match\r\nExpected: typeof(System.ArgumentNullException)\r\nActual: typeof(System.InvalidOperationException)\r\n---- System.InvalidOperationException : Operation is not valid due to the current state of the object.

Stack trace
at System.ComponentModel.Design.Tests.DesignerActionMethodItemTests.Invoke_NullMemberName_ThrowsArgumentNullException() in D:\a_work\1\s\src\System.Windows.Forms.Design\tests\UnitTests\System\ComponentModel\Design\DesignerActionMethodItemTests.cs:line 227
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
----- Inner Stack Trace -----
at System.ComponentModel.Design.DesignerActionMethodItem.Invoke() in D:\a_work\1\s\src\System.Windows.Forms.Design\src\System\ComponentModel\Design\DesignerActionMethodItem.cs:line 113

@gpetrou gpetrou force-pushed the DesignerActionMethodItem branch from 092797c to cc9a3d1 Compare February 22, 2025 07:01
Tanya-Solyanik
Tanya-Solyanik approved these changes Feb 26, 2025
View reviewed changes
Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik left a comment

Choose a reason for hiding this comment

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

Thank you!

@Tanya-Solyanik Tanya-Solyanik merged commit e9f1c43 into dotnet:main Feb 26, 2025
8 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0 Preview3 milestone Feb 26, 2025
@gpetrou gpetrou deleted the DesignerActionMethodItem branch February 26, 2025 20:00
LeafShi1 pushed a commit to LeafShi1/winforms that referenced this pull request Mar 6, 2025
@gpetrou @LeafShi1
Enable nullability in DesignerActionMethodItem (dotnet#12676)
d920717
@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ricardobossan ricardobossan ricardobossan left review comments

Copilot code review Copilot Copilot left review comments

+1 more reviewer

@Tanya-Solyanik Tanya-Solyanik Tanya-Solyanik approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@gpetrou gpetrou

Labels
waiting-author-feedback The team requires more information from the author
Projects
None yet
Milestone
10.0 Preview3
Development

Successfully merging this pull request may close these issues.
4 participants
@gpetrou @Tanya-Solyanik @ricardobossan @LeafShi1