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.

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

Issue 385: Detect indirect assignment of non-nullable fields. #386

Merged
merged 1 commit into from
Aug 28, 2021
Merged

Issue 385: Detect indirect assignment of non-nullable fields. #386

merged 1 commit into from
Aug 28, 2021

Conversation

manfred-brands
Copy link
Member

Fixes #385

mikkelbu
mikkelbu approved these changes Aug 25, 2021
View reviewed changes
Copy link
Member

@mikkelbu mikkelbu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good @manfred-brands and I'm happy to merge it as it is. I guess it will not work with methods taking a params argument (and I'm unsure about optional parameters), but I think this is a very good first step.

EDIT: Just let me know if you are fine with merging the commits as they are.

@mikkelbu mikkelbu added this to the Release 3.2 / 2.2 milestone Aug 25, 2021
@manfred-brands
Copy link
Member Author

@mikkelbu Dropped the parameter count/type checking and ask the compiler what method it resolved. Then check from there.
Updated test to show that it works with a params method.

mikkelbu
mikkelbu approved these changes Aug 28, 2021
View reviewed changes
Copy link
Member

@mikkelbu mikkelbu left a comment

Choose a reason for hiding this comment

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

Looks great @manfred-brands (and I like that the new solution is simpler).

I'll merge it when you are happy.

@manfred-brands manfred-brands force-pushed the Issue385_SuppressIndirectlyAssignedFields branch from 0a1f59c to f20ac84 Compare August 28, 2021 11:00
@manfred-brands
Issue 385: Detect indirect assignment of non-nullable fields.
f20ac84 
Deal with method overloads.
Let the compiler find the actual overload called.
@manfred-brands
Copy link
Member Author

@mikkelbu Thanks. Commits Squashed. Ready for merge.

@mikkelbu mikkelbu merged commit 7f8ab8a into nunit:master Aug 28, 2021
@mikkelbu
Copy link
Member

Merged. Thanks for the contribution @manfred-brands 👍

@manfred-brands manfred-brands deleted the Issue385_SuppressIndirectlyAssignedFields branch August 31, 2021 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikkelbu mikkelbu mikkelbu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release 3.2 / 2.2
Development

Successfully merging this pull request may close these issues.

Supress (even more) 'Non-nullable field must contain a non-null value when exiting constructor (CS8618)'
2 participants
@manfred-brands @mikkelbu