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

System.Text.Json source generator produces nullability warnings for NRT properties #59464

Closed
JakeYallop opened this issue Sep 22, 2021 · 3 comments · Fixed by #59755
Closed
Assignees
Labels
area-System.Text.Json blocking-release source-generator Indicates an issue with a source generator feature
Milestone

Comments

@JakeYallop
Copy link
Contributor

JakeYallop commented Sep 22, 2021

Description

The System.Text.Json source generator produces nullability warnings (CS8260, and CS8604) during a build.

using System.Text.Json.Serialization;

Console.WriteLine("JSON Source Generator repro");

public class Class
{
    public Uri? NullableUri { get; set; }
    public byte[]? ByteArray { get; set; }
}

[JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)]
[JsonSerializable(typeof(Class))]
internal partial class Context : JsonSerializerContext
{

}

The above code produces 2 warnings:

warning CS8620: Argument of type 'JsonTypeInfo<Uri>' cannot be used for parameter 'jsonTypeInfo' of type 'JsonTypeInfo<Uri?>' in 'void JsonSerializer.Serialize<Uri?>(Utf8JsonWriter writer, Uri? value, JsonTypeInfo<Uri?> jsonTypeInfo)' due to differences in the nullability of reference types.
warning CS8604: Possible null reference argument for parameter 'value' in 'void Context.ByteArraySerialize(Utf8JsonWriter writer, byte[] value)'.

Runnable repro here.

Impact

This breaks CI builds set up to fail if there are any warnings present in the project.

Are there any workarounds that can be done in the meantime, other than disabling the warnings in CI builds?

Configuration

.NET Version: NET 6 RC1

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Sep 22, 2021
@ghost
Copy link

ghost commented Sep 22, 2021

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

The System.Text.Json source generator produces nullability warnings (CS8260, and CS8604) during a build.

using System.Text.Json.Serialization;

Console.WriteLine("JSON Source Generator test");

public class Class
{
    public Uri? NullableUri { get; set; }
    public byte[]? ByteArray { get; set; }
}

[JsonSourceGenerationOptions(PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase)]
[JsonSerializable(typeof(Class))]
internal partial class Context : JsonSerializerContext
{

}

The above code produces 2 warnings:

warning CS8620: Argument of type 'JsonTypeInfo<Uri>' cannot be used for parameter 'jsonTypeInfo' of type 'JsonTypeInfo<Uri?>' in 'void JsonSerializer.Serialize<Uri?>(Utf8JsonWriter writer, Uri? value, JsonTypeInfo<Uri?> jsonTypeInfo)' due to differences in the nullability of reference types.
warning CS8604: Possible null reference argument for parameter 'value' in 'void Context.ByteArraySerialize(Utf8JsonWriter writer, byte[] value)'.

Impact

This breaks CI builds set up to fail if there are any warnings present in the project.

Are there any workarounds that can be done in the meantime, other than disabling the warnings in CI builds?

Configuration

.NET Version: NET 6 RC1

Author: JakeYallop
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@layomia layomia added the source-generator Indicates an issue with a source generator feature label Sep 22, 2021
@layomia
Copy link
Contributor

layomia commented Sep 24, 2021

Source generators shouldn't emit code that causes errors in user projects. Looking into whether the relevant nullability info is available through the Roslyn compilation APIs. cc @chsienki @jaredpar for any tips.

@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Sep 24, 2021
@layomia layomia added this to the 6.0.0 milestone Sep 24, 2021
@layomia layomia self-assigned this Sep 24, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 29, 2021
@ericstj
Copy link
Member

ericstj commented Sep 29, 2021

I see https://github.com/dotnet/roslyn/blob/main/docs/compilers/CSharp/Nullability%20Public%20API%20Design%20Notes.md that describes this API in roslyn.
@buyaa-n also recently added this to reflection in #54985.

Presumably the serializer isn't really doing anything different for nullable annotated types today since it wasn't observing that.
Should we disable nullability in the generated code (#nullable disable) for now and see about fixing that in 7.0?

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Sep 30, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json blocking-release source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants