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

CS8619 for anonymous types with nullable ref types after VS update #51886

Open
davidkarlsson opened this issue Mar 15, 2021 · 10 comments
Open

CS8619 for anonymous types with nullable ref types after VS update #51886

davidkarlsson opened this issue Mar 15, 2021 · 10 comments

Comments

@davidkarlsson
Copy link

davidkarlsson commented Mar 15, 2021

Version Used:
.NET SDK: 5.0.201
Visual Studio: 16.9.1
Compiler: '3.9.0-6.21124.20 (db94f4c)'. Language version: 9.0

Steps to Reproduce:
After updating Visual Studio to version 16.9.1 (from 16.8.5 I think) I've started getting false positives when creating anonymous types with nullable reference types.

  1. Enable nullable in csproj file
  2. Create an anonymous type from a type with nullable reference types using a lambda expression:
class Test
{
    public string? NullableString { get; set; }
}

var test = new List<Test>() { new Test() };

var anon = test.Select(a =>
    new
    {
        a.NullableString
    }
);

Creating an anonymous type like this with Select or GroupBy etc causes a CS8619 warning but I have a ruleset referenced in my csproj file that treats CS8619 with error severity so my project doesn't compile at all any longer.

Expected Behavior:
It should be possible to create anonymous types from types that has nullable reference types without getting a CS8619 warning.

Actual Behavior:
The compiler/analyzer wrongly wants the anonymous type to match an anonymous type with non nullable reference types: CS8619 Nullability of reference types in value of type '<anonymous type: string NullableString>' doesn't match target type '<anonymous type: string? NullableString>'.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 15, 2021
@Youssef1313
Copy link
Member

I can't reproduce in the current main branch. See SharpLab.

@davidkarlsson
Copy link
Author

@Youssef1313 I did some more testing with a blank project and could not reproduce it either at first but I now think I've managed to pinpoint the root cause of this issue.

It seems like the problem happens with EF Core (v. 5.0.4) in async functions where you try to save your changes in a loop with SaveChangesAsync(). Here's a complete repro:

using Microsoft.EntityFrameworkCore;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

namespace NullabilityTest
{
    class Program
    {
        static async Task Main(string[] args)
        {
            await using var dbContext = new TestDbContext();

            var test = new List<Test>() { new Test() };

            var anon = test.Select(a =>
                new
                {
                    a.NullableString
                }
            );

            for (var i = 0; i < 1; i++)
            {
                await dbContext.SaveChangesAsync();
            }
        }
    }

    public class TestDbContext : DbContext
    {
    }

    class Test
    {
        public string? NullableString { get; set; }
    }
}

I'm not sure if this has already been fixed in the efcore repo or if I should make a new issue about it there? It seems related to this issue: dotnet/efcore#19007

@roji
Copy link
Member

roji commented Mar 16, 2021

This does repro for me, producing the odd exception, but although EF is being used to trigger the error it doesn't seem to be involved in any way. Indeed looks like a compiler issue to me...

@RikkiGibson
Copy link
Contributor

I tried making some minimal modifications to make the sample build in SharpLab. However, it doesn't seem to produce a diagnostic. If you are able to produce a SharpLab which does produce the undesired diagnostic, that would be really helpful.

@davidkarlsson
Copy link
Author

@RikkiGibson If you add CancellationToken cancellationToken = default as a parameter to the method you call in the loop it should produce the diagnostic. You can see it in this SharpLab.

@Illya-Clearpoint
Copy link

Illya-Clearpoint commented Mar 17, 2021

Came across this today after upgrading Visual Studio to 16.9.2

Using .NET Core 3.1 I can reproduce the issue with the following:

var nrts = new[] {new {Test1 = (string?)"x", Test2 = 1}, new {Test1 = (string?) null, Test2 = 2}};
nrts.Where(nrt => nrt.Test1 != null).Select(nrt => new {nrt.Test1});

Error CS8619 Nullability of reference types in value of type '' doesn't match target type '<anonymous type: string? Test1>'.

It seems the issue comes about when a property of an object is a nullable reference type, but is subsequently able to be inferred to be non-nullable (via an explicit check for null or via nullability attributes of calling method). The inferred type of the field in anonymous object is then determined to be non-nullable, which causes the mismatch.

@RikkiGibson
Copy link
Contributor

I'll need to look at this more closely to be sure I understand the issue, and particularly determine what we changed that made this start to become a problem, but at a glance I am wondering if this is another scenario where "nullable-covariant" conversions should be allowed. That is, if we simply allowed the anonymous type new { string Name } to convert to the anonymous type new { string? Name }, would the problem go away? cc @jcouv.

@Illya-Clearpoint
Copy link

@RikkiGibson My understanding is that the anonymous types that differ only by property nullability are not created as separate types, but rather the exact same type generated by compiler. The following tests pass:

        [Fact]
        public void test1()
        {
            var x = new {Test1 = (string?) null};
            var y = new {Test1 = "xyz"};

            x.GetType().Should().BeSameAs(y.GetType());
        }

        [Fact]
        public void test2()
        {
            var x = new[] {new {Test1 = (string?) null}};
            var y = new[] {new {Test1 = "xyz"}};

            x.GetType().Should().BeSameAs(y.GetType());
        }

@jaredpar jaredpar added Bug Feature - Nullable Reference Types Nullable Reference Types and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 18, 2021
@jaredpar jaredpar added this to the C# 10 milestone Mar 18, 2021
@RikkiGibson
Copy link
Contributor

RikkiGibson commented Mar 19, 2021

I noticed that the presence of the warning depends on whether 'default' is passed for the CancellationToken parameter. If you pass a non-'default' CancellationToken, there is no warning.

Passing default
Passing non-default

@cston I wondered if this is potentially related to the recent nullable lambda state optimizations. Is there a remote chance that the state of the default argument is being assigned to the slot for the Test.NullableString and causing this warning?

@jaredpar jaredpar modified the milestones: C# 10, 17.0 Jul 13, 2021
@jaredpar jaredpar modified the milestones: 17.0, 17.1 Aug 3, 2021
@jinujoseph jinujoseph modified the milestones: 17.1, 17.3 Apr 27, 2022
@jaredpar jaredpar modified the milestones: 17.3, Backlog Jul 1, 2022
@Youssef1313
Copy link
Member

@RikkiGibson Here is a SharpLab repro that doesn't involve default parameters

#62500 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants