Attempt to fix nullability for e.g. #973 by jdb0123 · Pull Request #976 · nsubstitute/NSubstitute

jdb0123 · 2026-07-13T19:45:28Z

No description provided.

… types Change predicate signatures from Predicate<T?> to Predicate<T> in Arg.Is and ExpressionArgumentMatcher, aligning with the compatibility API which already used non-nullable predicates. Replace the unboxing cast in IsSatisfiedBy with an 'argument is T typed' type check in both ExpressionArgumentMatcher and the GenericToNonGenericMatcherProxy. A null argument passed for a non-nullable type now returns false instead of throwing or coercing to default.

it already throws when it's not assignable

Before it had to be banged

dtchepak · 2026-07-14T10:59:09Z

Thanks for picking this up @jdb0123 . Would it be simplest to revert #856 ?

Let me know when you're ready for a review of this. 🙇

Before it had to be banged

jdb0123 · 2026-07-14T13:50:29Z

@dtchepak no problem, happy to help.

I believe the current changes should be sufficient to fix the issues. However, I only detected and fixed things which showed up in our own usage of nsubstitute so I might have missed things.

I think reverting the changes would be a bit of waste, given that the direction of enabling nullable is a good idea.

Anyhow, these changes are ready for review.

dtchepak · 2026-07-18T13:24:51Z

Testing this out now. Is the sub.DoSomethingNullable(Arg.Is<string?>(x => x == null)).Returns(456); behaviour expected?

using NUnit.Framework;

namespace NSubstitute.Acceptance.Specs.FieldReports;

public class Issue973_MatchingWithNullability
{

    #nullable enable

    public interface ISomething
    {
        int DoSomething(string s);
        int DoSomethingNullable(string? s);
    }

    [Test]
    public void Match_non_null()
    {
        var sub = Substitute.For<ISomething>();

        sub.DoSomething(Arg.Is<string>(x => x.StartsWith("12"))).Returns(42);

        Assert.That(sub.DoSomething("123"), Is.EqualTo(42));
        Assert.That(sub.DoSomething("abc"), Is.EqualTo(0));
    }

    [Test]
    public void Match_nullable()
    {
        var sub = Substitute.For<ISomething>();

        sub.DoSomethingNullable(Arg.Is<string>(x => x.StartsWith("12"))).Returns(42);
        sub.DoSomethingNullable(Arg.Is<string?>(x => x == null)).Returns(456);

        Assert.That(sub.DoSomethingNullable("123"), Is.EqualTo(42));
        Assert.That(sub.DoSomethingNullable("hi"), Is.EqualTo(0));
        Assert.That(sub.DoSomethingNullable(null), Is.EqualTo(456));
        /*
failed Match_nullable 
    Assert.That(sub.DoSomethingNullable(null), Is.EqualTo(456))
  Expected: 456
  But was:  0
*/

    #nullable restore
}

Romfos · 2026-07-19T09:12:11Z

what is purpose of GenericTypeArgumentsAreCompatible. Is it related to #974 ? Maybe better to revert old PR until author will propose fixed version?

Romfos · 2026-07-19T09:12:22Z

Other changes looks good

Romfos · 2026-07-19T09:14:17Z

Testing this out now. Is the sub.DoSomethingNullable(Arg.Is<string?>(x => x == null)).Returns(456); behaviour expected?

using NUnit.Framework;

namespace NSubstitute.Acceptance.Specs.FieldReports;

public class Issue973_MatchingWithNullability
{

    #nullable enable

    public interface ISomething
    {
        int DoSomething(string s);
        int DoSomethingNullable(string? s);
    }

    [Test]
    public void Match_non_null()
    {
        var sub = Substitute.For<ISomething>();

        sub.DoSomething(Arg.Is<string>(x => x.StartsWith("12"))).Returns(42);

        Assert.That(sub.DoSomething("123"), Is.EqualTo(42));
        Assert.That(sub.DoSomething("abc"), Is.EqualTo(0));
    }

    [Test]
    public void Match_nullable()
    {
        var sub = Substitute.For<ISomething>();

        sub.DoSomethingNullable(Arg.Is<string>(x => x.StartsWith("12"))).Returns(42);
        sub.DoSomethingNullable(Arg.Is<string?>(x => x == null)).Returns(456);

        Assert.That(sub.DoSomethingNullable("123"), Is.EqualTo(42));
        Assert.That(sub.DoSomethingNullable("hi"), Is.EqualTo(0));
        Assert.That(sub.DoSomethingNullable(null), Is.EqualTo(456));
        /*
failed Match_nullable 
    Assert.That(sub.DoSomethingNullable(null), Is.EqualTo(456))
  Expected: 456
  But was:  0
*/

    #nullable restore
}

For me looks correct

cremor · 2026-07-19T10:11:54Z

For me looks correct

How is it correct that it doesn't return 456? How else would I setup an Arg.Is check for null?

Null argument also matched on the non nullable variant

jdb0123 · 2026-07-19T11:34:24Z

@dtchepak sub.DoSomethingNullable(Arg.Is<string?>(x => x == null)).Returns(456); No i believe that to be incorrect. Added a fix to the review

…_argument

jdb0123 · 2026-07-19T12:01:08Z

what is purpose of GenericTypeArgumentsAreCompatible. Is it related to #974 ? Maybe better to revert old PR until author will propose fixed version?

No it was not related to #974
I just put all fixes we required to get our tests running / compiling again in this PR.

GenericTypeArgumentsAreCompatible was required to not use a derived setup when generic functions were called with a base type. I've added a test for this, see Stub_for_derived_type_argument_is_not_returned_for_base_type_argument

dtchepak · 2026-07-26T05:52:06Z

Hi @zvirja, was hoping to get your thoughts here based on your initial concern about enabling nullability. I'm a bit torn between reverting the initial nullability change vs. proceeding with @jdb0123's fix. @jdb0123's fix addresses the issues found i their code base, but we're not sure what other cases will emerge. Any thoughts on whether to press on with this, or revert and maybe take another look at this later?

Romfos · 2026-07-26T11:36:11Z

@dtchepak

I propose following:

merge this
merge Fix return type equivalency checking #977

and release as 6.0.1

and then lets process next feedback if any

zvirja · 2026-07-30T14:24:41Z

@dtchepak My personal suggestion would be to revert nullability for the public API introduced in #856. This is a testing library and nullability is always weird in the unit test projects, as we don't track it strictly there anyway. Another argument is that by the library's nature we don't have any info about nullability of the code we are mocking. Like when we have Arg.Any<T>(), we literally don't know if it's nullable or not. So the safest would be to play pessimistically (like .NET did with .ToString() marking it nullable) - but that is causing unnecessary inconveniences without bringing any visible benefits.

This PR is a clear demonstration of that deeper issue, as it starts marking types non-nullable which in fact could be null. Look at CallInfo.Arg<T>() - it could be null and the suggested nullability annotation is misleading. Mow we are working around the issue we introduced ourselves :)

So I vote for reverting that PR. We gave it a shot, we saw the issues and the proposed fix shows that there is no a clean way of fixing it.

As for #977 - it's a different beast.

dtchepak · 2026-08-02T05:54:22Z

@zvirja @jdb0123 @Romfos I created #985. Please take a look 🙇

Should be a non-breaking change? Releasing as 6.1.0 ok?

jdb0123 · 2026-08-02T13:02:20Z

@dtchepak

@zvirja @jdb0123 @Romfos I created #985. Please take a look 🙇

Should be a non-breaking change? Releasing as 6.1.0 ok?

I'm not sure why you would choose the revert the changes. As far as we are aware there are not further issues once we merge #976 and #977.

So I would just go for which @Romfos already suggested:

@dtchepak

I propose following:

merge this

merge Fix return type equivalency checking #977

and release as 6.0.1

and then lets process next feedback if any

If you merge the 2 PR's, release, close the existing nullable related issues. Then we will hear back from other use cases which were missed, if any, and we can fix those.

zvirja · 2026-08-06T16:44:44Z

If you merge the 2 PR's, release, close the existing nullable related issues. Then we will hear back from other use cases which were missed, if any, and we can fix those.

Current PR offers hacky solution, as now it states things are non-nullable while they could be nullable. See the .Arg<>() example from above - it just don't work 😕

Also NSubstitute is not really compatible with nullability. Take for example this syntax:

sut.Method(default, default).ReturnsForAnyArgs(42);

That's how you usually use ReturnsForAnyArgs(), but with nullability it will highlight the case.

Again, one of many examples.

The issue is about nullability being incorrectly assigned - the problem is that nullability is just unsolvable with the API we provide and instead of returning wrong nullability (like we do in this PR) we should rather disable it at all.

jdb0123 · 2026-08-06T17:23:52Z

@zvirja can you give me a full code example of the issues you mention. They are not clear to me from your text.
Just add what you would expect and what the current behavior or issue is.

cremor · 2026-08-07T11:38:23Z

May I suggest unlisting version 6.0.0 from NuGet if this discussion goes on? More and more users are updating to 6.0.0 every day and waste time trying to fix the new nullable warnings.

This reverts commit 6c37da9 from PR #856, reversing changes made to ee11ef0. #976 has demonstrated that we need more fundamental lib changes to effectively support nullability, so we'll revert to the pre v6.0 behaviour and revisit later. # Conflicts: # BreakingChanges.md # CHANGELOG.md # src/NSubstitute/Compatibility/Arg.Compat.cs # src/NSubstitute/Core/CallInfo.cs

dtchepak · 2026-08-08T10:02:19Z

@jdb0123 I understand wanting to keep nullability support, but based on the 6.0.0 attempt I think we might be better off starting with the internals. Once the internals are nullability-aware i think we'll be in a better position to enable it for callers where it makes sense (keeping in mind examples like @zvirja's where we may explicitly want to ignore nullables for some extension methods.)

dtchepak · 2026-08-08T10:03:34Z

May I suggest unlisting version 6.0.0 from NuGet if this discussion goes on? More and more users are updating to 6.0.0 every day and waste time trying to fix the new nullable warnings.

Definitely something i considered @cremor . I'm about to publish v6.1.0 with nullability reverted, so I'm tempted to keep 6.0 published for people running it without nullability as they still get new matchers etc. Sound reasonable?

cremor · 2026-08-08T10:33:41Z

@dtchepak Yes, sure. My intention was just for the latest version to not be broken.

Jbontt added 2 commits July 13, 2026 13:53

Fix 973: Make callInfo arg not return nullable

5240732

it already throws when it's not assignable

jdb0123 mentioned this pull request Jul 13, 2026

Arg.Is<T> forces Predicate<T?> even for non-nullable T causing CS8602 and CS8604 at every call #973

Closed

Jbontt added 3 commits July 13, 2026 23:07

Allow ReturnsNull calls to be called on nullable members

7ebbfe0

Before it had to be banged

Allow null for sender of event raises

81f0ba5

Allow null for sender of event raises

63e3953

dtchepak mentioned this pull request Jul 14, 2026

[Hotfix] Make Arg.Is predicate as not nullable #975

Closed

Jbontt added 5 commits July 14, 2026 13:05

Allow ReturnsNull calls to be called on nullable members

dce080f

Before it had to be banged

Fix generic method matching to compare type arguments directionally

50a0edc

Allow Returns calls to be called on nullable members

fc1c891

Allow Throws calls to be called on nullable members

455cc0f

Match generic method type arguments directionally test

b4e503d

dtchepak mentioned this pull request Jul 16, 2026

Substitute.For and Substitute.ForPartsOf no longer allow null parameters #978

Closed

dtchepak requested review from Romfos and dtchepak July 16, 2026 12:23

Fix argument matcher for mixed nullability

5f72153

Null argument also matched on the non nullable variant

Add test Stub_for_derived_type_argument_is_not_returned_for_base_type…

c168821

…_argument

This was referenced Jul 20, 2026

chore(deps): bump package and GitHub Actions versions roryprimrose/Neovolve.Streamline#225

Merged

Bump NSubstitute from 5.3.0 to 6.0.0 roryprimrose/Neovolve.Streamline#221

Closed

This was referenced Jul 30, 2026

fix: update Arg.Is predicate parameter type to avoid CS8602/CS8604 nullability warnings (#973) #982

Closed

Generic constraint on ReturnsNull and ReturnsNullForAnyArgs does not allow nullable classes, leading to CS8634 #983

Closed

dtchepak mentioned this pull request Aug 2, 2026

Revert nullability changes #985

Merged

jdb0123 closed this Aug 8, 2026

Uh oh!

Conversation

jdb0123 commented Jul 13, 2026

Uh oh!

dtchepak commented Jul 14, 2026

Uh oh!

jdb0123 commented Jul 14, 2026

Uh oh!

dtchepak commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Romfos commented Jul 19, 2026

Uh oh!

Romfos commented Jul 19, 2026

Uh oh!

Romfos commented Jul 19, 2026

Uh oh!

cremor commented Jul 19, 2026

Uh oh!

jdb0123 commented Jul 19, 2026

Uh oh!

jdb0123 commented Jul 19, 2026

Uh oh!

dtchepak commented Jul 26, 2026

Uh oh!

Romfos commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zvirja commented Jul 30, 2026

Uh oh!

dtchepak commented Aug 2, 2026

Uh oh!

jdb0123 commented Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zvirja commented Aug 6, 2026

Uh oh!

jdb0123 commented Aug 6, 2026

Uh oh!

cremor commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtchepak commented Aug 8, 2026

Uh oh!

dtchepak commented Aug 8, 2026

Uh oh!

cremor commented Aug 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dtchepak commented Jul 18, 2026 •

edited

Loading

Romfos commented Jul 26, 2026 •

edited

Loading

jdb0123 commented Aug 2, 2026 •

edited

Loading

cremor commented Aug 7, 2026 •

edited

Loading