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

Diagnostic suppressor seems to work only for open files #75291

Open
lennartb- opened this issue Sep 30, 2024 · 0 comments
Open

Diagnostic suppressor seems to work only for open files #75291

lennartb- opened this issue Sep 30, 2024 · 0 comments
Labels
Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@lennartb-
Copy link

Hi,

this is somewhat of a follow-up to #68885. I've initially reported this to nunit/nunit.analyzers#786 as well, but it doesn't seem to be an issue with the analyzer itself.

For context: The NUnit.Analyzer package provides diagnostic suppressor to suppress nullable warnings for objects that can be null, but have previously been checked with NUnit's asserts that they are not null.

We recently changed a legacy test project that previously had NRTs disabled to enable them. We saw hundreds of "CS8602 - Dereference of a possibly null reference" warnings for objects that can be in fact null, but were checked with some variation of "Assert this is not null". Curiously, the warnings weren't reported at build time and disappeared after the file with the warning was opened.

Here's a repro, mostly based on the Roslyn issue above:
Project file:

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

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
    <IsPackable>false</IsPackable>
    <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
  </PropertyGroup>

  <PropertyGroup>
    <AccelerateBuildsInVisualStudio>true</AccelerateBuildsInVisualStudio>
    <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
    <PackageReference Include="NUnit" Version="4.2.2">
      <NoWarn>NU1608</NoWarn>
    </PackageReference>
    <PackageReference Include="NUnit.Analyzers" Version="4.3.0">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
  </ItemGroup>
</Project>

Test:

using NUnit.Framework;

namespace NUnitNullabilityTest
{
    [TestFixture]
    public class Tests
    {
        private class Model
        {
            public string? Name { get; set; }

            public string? SomeMethod() => Name;
        }

        [Test]
        public void Model_SomeMethod()
        {
            var target = new Model { Name = "name" };

            var result = target.SomeMethod();

            Assert.That(result, Is.Not.Null);
            Assert.That(result.Length, Is.EqualTo(4)); // This is of course a nonsense assertion, but repros the issue
        }
    }
}

result can technically be null, so result.Length could theoretically throw an NRE. Since Is.Not.Null is asserted, we can be sure it's not null.

This produces the warning if the file is not opened:
image

An vanishes if the file is opened:
image

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

1 participant