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

Check for nullity using "is null" or "is object" #1624

Merged
merged 1 commit into from Sep 12, 2019

Conversation

blairconrad
Copy link
Member

Jared Parsons lays out the reasons in
https://twitter.com/jaredpar/status/1171478965355024384:

... "x is object" is either a) a null check or b) does
not compile. In the case "x" is known to be a struct the compiler will
emit a diagnostic.

Similarly "x is null" has the same properties except it's always a
null check vs. a non-null check

@blairconrad
Copy link
Member Author

blairconrad commented Sep 12, 2019

You seemed excited when I mentioned this, @thomaslevesque. Feel like making @jaredpar happier by two developers?

thomaslevesque
thomaslevesque previously approved these changes Sep 12, 2019
Copy link
Member

@thomaslevesque thomaslevesque left a comment

Choose a reason for hiding this comment

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

You seemed excited when I mentioned this, @thomaslevesque.

Not excited enough to go over the whole code base to replace usages of == null and != null, but since you were motivated enough to do it... Why not 😉

Thanks @blairconrad, LGTM!

src/FakeItEasy/ServiceLocator.cs Show resolved Hide resolved
@thomaslevesque
Copy link
Member

You'll need to squash before I can merge, though

Jared Parsons lays out the reasons in
https://twitter.com/jaredpar/status/1171478965355024384:

> ... "x is object" is either a) a null check or b) does
> not compile. In the case "x" is known to be a struct the compiler will
> emit a diagnostic.
>
> Similarly "x is null" has the same properties except it's always a
> null check vs. a non-null check
@blairconrad
Copy link
Member Author

Not excited enough to go over the whole code base to replace usages of == null and != null

Search and replace is a wonderful thing.

You mean with the latest SDK we can do (x is null) even when x could be a struct?

You can at least do it when T is an unconstrained typeparam.

Ah! I just tried with an int, and the i is null test caused the build to fail.

Squashed! Thanks for the review and eventual merge.

@blairconrad blairconrad added this to the vNext milestone Sep 12, 2019
@thomaslevesque thomaslevesque merged commit dc00727 into FakeItEasy:master Sep 12, 2019
@thomaslevesque
Copy link
Member

Thanks @blairconrad !

@jaredpar
Copy link

This makes me happy 😄

@blairconrad blairconrad deleted the null-check branch September 12, 2019 17:19
@blairconrad
Copy link
Member Author

This change has been released as part of FakeItEasy 5.3.0.

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.

None yet

3 participants