Skip to content

Features/nullchecking binding#36095

Merged
fayrose merged 17 commits intodotnet:features/param-nullcheckingfrom
fayrose:features/nullchecking-binding
Jun 4, 2019
Merged

Features/nullchecking binding#36095
fayrose merged 17 commits intodotnet:features/param-nullcheckingfrom
fayrose:features/nullchecking-binding

Conversation

@fayrose
Copy link
Copy Markdown
Contributor

@fayrose fayrose commented May 31, 2019

Note this exclusively covers error cases of extern/interfaces/abstract classes/delegates. Working cases will be covered in the next PR.

Relates to #36024 (test plan for parameter null-checking)

@fayrose fayrose requested a review from a team as a code owner May 31, 2019 21:19
// delegate void Del(int x!, int y);
Diagnostic(ErrorCode.ERR_MustNullCheckInImplementation, "int x!").WithArguments("int x!").WithLocation(6, 32));
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a basic "happy" case for a concrete method with a null-checked parameter.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See description. Working cases in next PR?

Copy link
Copy Markdown
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

LGTM (commit 13)

Copy link
Copy Markdown
Member

@RikkiGibson RikkiGibson left a comment

Choose a reason for hiding this comment

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

Perhaps the parse errors in NullCheckedAutoProperty could be better, but I feel it's low priority at best

@fayrose fayrose merged commit ca55da0 into dotnet:features/param-nullchecking Jun 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants