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.

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

Fix ConvertTypeCheckPatternToNullCheck #110973

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xtqqczze
Copy link
Contributor

Use null check pattern instead of a type check succeeding on any not-null value.

Use null check pattern instead of a type check succeeding on any not-null value
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Dec 28, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

@@ -301,7 +301,7 @@ private static Attribute[] InternalParamGetCustomAttributes(ParameterInfo param,
count = 0;
for (int i = 0; i < objAttr.Length; i++)
{
if (objAttr[i] is object attr)
if (objAttr[i] is { } attr)
Copy link
Member

Choose a reason for hiding this comment

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

This is not readable. In the initial version of pattern matching before is not null, is { } is rejected by many projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We cannot use is not null with declaration pattern, i.e. if (objAttr[i] is not null attr) is invalid C#.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
if (objAttr[i] is { } attr)
if (objAttr[i] is not null and var attr)

Copy link
Member

@jkotas jkotas Dec 29, 2024

Choose a reason for hiding this comment

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

Yes, it is unfortunate that there are number of more or less cryptic ways one can express a null check in modern C#. The choice between them is a matter of personal preference. My personal preference is the old fashioned:

object? attr = objAttr[i];
if (attr != null)

I believe that it is the best balance of readable and succinct. (Except for the few types that overload operator==. It is fine to use more verbose is not null in those cases for performance reasons.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Meta community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants