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

False positive NS1004 with When().Do() #160

Closed
mcarton opened this issue Apr 27, 2021 · 4 comments
Closed

False positive NS1004 with When().Do() #160

mcarton opened this issue Apr 27, 2021 · 4 comments
Labels
bug Something isn't working
Milestone

Comments

@mcarton
Copy link

mcarton commented Apr 27, 2021

The following gives a NS1004 warning, even though the test works as expected:

using NUnit.Framework;
using NSubstitute;
using System;

namespace dlsfkn
{
    public interface IFoo
    {
        string Foo { set; }
    }

    public class Tests
    {
        [Test]
        public void Test1()
        {
            var foo = NSubstitute.Substitute.For<IFoo>();

            foo
                .When(x => x.Foo = Arg.Any<string>())
                .Do(x => throw new Exception("This does fail"));

            foo.Foo = "whynot";
        }
    }
}

gives the following warning:

/tmp/dlsfkn/UnitTest1.cs(20,36): warning NS1004: Argument matcher used with a non-virtual member of a class. [/tmp/dlsfkn/dlsfkn.csproj]

with the following project:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net5.0</TargetFramework>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="NUnit" Version="3.12.0" />
    <PackageReference Include="NUnit3TestAdapter" Version="3.16.1" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0"/>

    <PackageReference Include="NSubstitute" Version="4.2.2" />
    <PackageReference Include="NSubstitute.Analyzers.CSharp" Version="1.0.14" />
  </ItemGroup>

</Project>

This is probably related to #159, but with a different method, so I though I'd report it anyway.

@tpodolak tpodolak added the bug Something isn't working label Apr 27, 2021
@tpodolak
Copy link
Member

Hi @mcarton thanks for reporting. As you said this is related with #159. @dtchepak could you confirm that this is correct usage of Arg.Any? I didn't find any usage examples of Arg.Any Arg.Do etc in docs.

@mcarton
Copy link
Author

mcarton commented Apr 27, 2021

I also tried to test that a property was not set using the following, which doesn't use Do. It also gives a NS1004, even though Foo is from an interface:

using NUnit.Framework;
using NSubstitute;

namespace dlsfkn
{
    public interface IFoo
    {
        string Foo { set; }
    }

    public class Tests
    {
        [Test]
        public void Test1()
        {
            var foo = NSubstitute.Substitute.For<IFoo>();
            foo.Foo = "whynot";
            foo.DidNotReceive().Foo = Arg.Any<string>();
        }
    }
}

@dtchepak could you confirm that this is correct usage of Arg.Any? I didn't find any usage examples of Arg.Any Arg.Do etc in docs.

What would you recommend instead to test that a property is not set?

@dtchepak
Copy link
Member

@tpodolak I haven't expected that use but it seems like it is valid.

What would you recommend instead to test that a property is not set?

@mcarton If possible I think asserting on the property value is cleaner. Alternatively using an explicit setter method if the act of setting is significant. But these are more subjective design opinions rather than anything concrete. 😄

@tpodolak
Copy link
Member

Fixed in #169

@tpodolak tpodolak added this to the 1.0.15 milestone Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants