Skip to content

Restore nullable warnings for unset required members from base types#63286

Merged
333fred merged 6 commits intodotnet:mainfrom
333fred:base-required-nullable
Aug 16, 2022
Merged

Restore nullable warnings for unset required members from base types#63286
333fred merged 6 commits intodotnet:mainfrom
333fred:base-required-nullable

Conversation

@333fred
Copy link
Member

@333fred 333fred commented Aug 9, 2022 •
edited
Loading

Fixes #61718.

@333fred
Restore nullable warnings for unset required members from base types
1b75d91 
Fixes dotnet#61718. Note that there is still a hole for field-like events coming from metadata, but there's not much we can do about that as we don't know whether the event was field-like.
@333fred 333fred requested a review from a team as a code owner August 9, 2022 21:57
@ghost ghost added the Area-Compilers label Aug 9, 2022
@333fred
Copy link
Member Author

333fred commented Aug 9, 2022

@dotnet/roslyn-compiler @RikkiGibson for review.

@333fred 333fred added the Feature - Required Members Required properties and fields label Aug 9, 2022
RikkiGibson
RikkiGibson reviewed Aug 9, 2022
View reviewed changes
@RikkiGibson
Copy link
Member

RikkiGibson commented Aug 9, 2022

Note that there is still a hole for field-like events coming from metadata

Is there a test for this scenario? I'm not sure I understand it.

@333fred
Copy link
Member Author

333fred commented Aug 9, 2022 •
edited
Loading

Is there a test for this scenario? I'm not sure I understand it.

I can add one. I lied, this is not a scenario, as events cannot be marked required. Just had a bit of brain fog when I wrote that I guess.

@333fred @RikkiGibson
Grammar
db6d386 
Co-authored-by: Rikki Gibson <rikkigibson@gmail.com>
@RikkiGibson RikkiGibson self-assigned this Aug 9, 2022
RikkiGibson
RikkiGibson reviewed Aug 9, 2022
View reviewed changes
@333fred
Copy link
Member Author

333fred commented Aug 10, 2022

@dotnet/roslyn-compiler for a second review please.

Youssef1313
Youssef1313 reviewed Aug 10, 2022
View reviewed changes
Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

Does this need a spec update?

@333fred
Copy link
Member Author

333fred commented Aug 12, 2022

@RikkiGibson please take another look. @dotnet/roslyn-compiler for reviews.

RikkiGibson
RikkiGibson reviewed Aug 12, 2022
View reviewed changes
@RikkiGibson RikkiGibson self-requested a review August 14, 2022 00:54
@333fred
Copy link
Member Author

333fred commented Aug 15, 2022

@RikkiGibson @dotnet/roslyn-compiler for reviews please.

RikkiGibson
RikkiGibson approved these changes Aug 15, 2022
View reviewed changes
@333fred
Copy link
Member Author

333fred commented Aug 16, 2022

@dotnet/roslyn-compiler for a second review please.

@333fred 333fred merged commit 84acdbf into dotnet:main Aug 16, 2022
@333fred 333fred deleted the base-required-nullable branch August 16, 2022 21:37
@ghost ghost added this to the Next milestone Aug 16, 2022
333fred added a commit that referenced this pull request Aug 17, 2022
@333fred
Merge remote-tracking branch 'upstream/main' into deref-unused
f1e1da5 
* upstream/main:
  Fix compilation of exe RefFieldTests (#63431)
  Implement IAttributeOperation (#59369)
  Restore nullable warnings for unset required members from base types (#63286)
  Remove portion of incremental-analyzer-infrastructure that has no usage.
@dibarbet dibarbet modified the milestones: Next, 17.4 P2 Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RikkiGibson RikkiGibson RikkiGibson approved these changes

+2 more reviewers

@Youssef1313 Youssef1313 Youssef1313 left review comments

@cston cston cston approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@RikkiGibson RikkiGibson

Labels

Area-Compilers Feature - Required Members Required properties and fields

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

No warning when base required member is not initialized in constructor with "SetsRequiredMembers" attribute

5 participants

@333fred @RikkiGibson @cston @Youssef1313 @dibarbet