Skip to content

Param nullchecking binding#36247

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

Param nullchecking binding#36247
fayrose merged 20 commits intodotnet:features/param-nullcheckingfrom
fayrose:param-nullchecking-binding

Conversation

@fayrose
Copy link
Copy Markdown
Contributor

@fayrose fayrose commented Jun 7, 2019

Working cases for binding parameter nullchecking.

@fayrose fayrose requested a review from a team as a code owner June 7, 2019 20:01
@fayrose fayrose assigned fayrose and unassigned fayrose Jun 7, 2019
@fayrose
Copy link
Copy Markdown
Contributor Author

fayrose commented Jun 7, 2019

Currently adding asserts for IsNullChecked.

}
}
internal override bool IsNullChecked
=> this.CSharpSyntaxNode.ExclamationToken.Kind() == SyntaxKind.ExclamationToken;
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.

Can ExclamationToken be null? Also, I think the standard way of checking if a token is present is to call IsMissing instead of checking the Kind.

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.

I believe it's SyntaxKind.None if there's no token.

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.

I'm also wondering about when IsMissing is preferred over Kind() == SyntaxKind.None. It seems like IsMissing is mainly for when the token's absence is a syntax error, not when the token is optional.

@agocke
Copy link
Copy Markdown
Member

agocke commented Jun 10, 2019

Looks like there some copy/paste errors around the asserts, should be simple to fix up

Copy link
Copy Markdown
Member

@agocke agocke left a comment

Choose a reason for hiding this comment

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

LGTM

@fayrose fayrose merged commit 3e72bb0 into dotnet:features/param-nullchecking Jun 11, 2019
@fayrose fayrose deleted the param-nullchecking-binding branch June 11, 2019 16:43
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.

3 participants