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

Using StringValues with JsonSerializerContext produces warnings with 7.0.0-preview* #74652

Open
austindrenski opened this issue Aug 26, 2022 · 7 comments · Fixed by #74801
Open
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions source-generator Indicates an issue with a source generator feature
Milestone

Comments

@austindrenski
Copy link
Contributor

austindrenski commented Aug 26, 2022

Description

JsonSerializerContext produces warnings when models contain StringValues due to nullable annotation mismatch.

Reproduction Steps

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

  <PropertyGroup>
    <LangVersion>latest</LangVersion>
    <Nullable>enable</Nullable>
    <OutputType>exe</OutputType>
    <TargetFramework>net7.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Primitives" Version="7.0.0-preview.7.22375.6" />
    <PackageReference Include="System.Text.Json" Version="7.0.0-preview.7.22375.6" />
  </ItemGroup>

</Project>
using System;
using System.Text.Json.Serialization;
using Microsoft.Extensions.Primitives;

Console.WriteLine("Hello, world!");

[JsonSerializable(typeof(SomeClass))]
[JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Default)]
public sealed partial class SomeJsonSerializerContext : JsonSerializerContext
{
}

public sealed class SomeClass
{
    public StringValues SomeStringValues { get; set; }
}
$ dotnet build
MSBuild version 17.4.0-preview-22368-02+c8492483a for .NET
  Determining projects to restore...
  All projects are up-to-date for restore.
C:\Program Files\dotnet\sdk\7.0.100-preview.7.22377.5\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.RuntimeIdentifierInference.targets(219,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\system-text-json-source-generator-fails-on-stringvalues.csproj]
C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\SomeJsonSerializerContext.StringValues.g.cs(31,32): warning CS8631: The type 'Microsoft.Extensions.Primitives.StringValues' cannot be used as type parameter 'TCollection' in the generic type or method 'JsonMetadataServices.CreateIListInfo<TCollection, TElement>(JsonSerializerOptions, JsonCollectionInfoValues<TCollection>)'. Nullability of type argument 'Microsoft.Extensions.Primitives.StringValues' doesn't match constraint type 'System.Collections.Generic.IList<string>'. [C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\system-text-json-source-generator-fails-on-stringvalues.csproj]
  system-text-json-source-generator-fails-on-stringvalues -> C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\bin\Debug\net7.0\system-text-json-source-generator-fails-on-stringvalues.dll

Build succeeded.

C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\SomeJsonSerializerContext.StringValues.g.cs(31,32): warning CS8631: The type 'Microsoft.Extensions.Primitives.StringValues' cannot be used as type parameter 'TCollection' in the generic type or method 'JsonMetadataServices.CreateIListInfo<TCollection, TElement>(JsonSerializerOptions, JsonCollectionInfoValues<TCollection>)'. Nullability of type argument 'Microsoft.Extensions.Primitives.StringValues' doesn't match constraint type 'System.Collections.Generic.IList<string>'. [C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\system-text-json-source-generator-fails-on-stringvalues.csproj]
    1 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.41

Expected behavior

Using StringValues in conjunction with JsonSerializerContext does not produce warnings.

Actual behavior

Using StringValues in conjunction with JsonSerializerContext produces warnings.

warning CS8631: The type 'Microsoft.Extensions.Primitives.StringValues' cannot be used as type parameter 'TCollection' in the generic type or method 'JsonMetadataServices.CreateIListInfo<TCollection, TElement>(JsonSerializerOptions, JsonCollectionInfoValues)'. Nullability of type argument 'Microsoft.Extensions.Primitives.StringValues' doesn't match constraint type 'System.Collections.Generic.IList'.

Regression?

This works when using version 6.0.0 of Microsoft.Extensions.Primitives because the interfaces for StringValues were updated with nullable annotations for 7.0.0-preview*.

Known Workarounds

  • Suppress CS8631
  • Stay on version 6.0.0 of Microsoft.Extensions.Primitives

Configuration

$ dotnet --info
.NET SDK:
 Version:   7.0.100-preview.7.22377.5
 Commit:    ba310d9309

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22000
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\7.0.100-preview.7.22377.5\

Host:
  Version:      7.0.0-preview.7.22375.6
  Architecture: x64
  Commit:       eecb028078

Other information

Not sure where the fix belongs, but here's where the warning bubbles from:

public static JsonTypeInfo<TCollection> CreateIListInfo<TCollection, TElement>(
JsonSerializerOptions options,
JsonCollectionInfoValues<TCollection> collectionInfo)
where TCollection : IList<TElement>

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 26, 2022
@ghost
Copy link

ghost commented Aug 26, 2022

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

System.Text.Json.JsonSerializerContext produces warnings when models contain StringValues due to nullable annotation mismatch.

Reproduction Steps

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

  <PropertyGroup>
    <LangVersion>latest</LangVersion>
    <Nullable>enable</Nullable>
    <OutputType>exe</OutputType>
    <TargetFramework>net7.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Primitives" Version="7.0.0-preview.7.22375.6" />
    <PackageReference Include="System.Text.Json" Version="7.0.0-preview.7.22375.6" />
  </ItemGroup>

</Project>
using System;
using System.Text.Json.Serialization;
using Microsoft.Extensions.Primitives;

Console.WriteLine("Hello, world!");

[JsonSerializable(typeof(SomeClass))]
[JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Default)]
public sealed partial class SomeJsonSerializerContext : JsonSerializerContext
{
}

public sealed class SomeClass
{
    public StringValues SomeStringValues { get; set; }
}
$ dotnet build
MSBuild version 17.4.0-preview-22368-02+c8492483a for .NET
  Determining projects to restore...
  All projects are up-to-date for restore.
C:\Program Files\dotnet\sdk\7.0.100-preview.7.22377.5\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.RuntimeIdentifierInference.targets(219,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\system-text-json-source-generator-fails-on-stringvalues.csproj]
C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\SomeJsonSerializerContext.StringValues.g.cs(31,32): warning CS8631: The type 'Microsoft.Extensions.Primitives.StringValues' cannot be used as type parameter 'TCollection' in the generic type or method 'JsonMetadataServices.CreateIListInfo<TCollection, TElement>(JsonSerializerOptions, JsonCollectionInfoValues<TCollection>)'. Nullability of type argument 'Microsoft.Extensions.Primitives.StringValues' doesn't match constraint type 'System.Collections.Generic.IList<string>'. [C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\system-text-json-source-generator-fails-on-stringvalues.csproj]
  system-text-json-source-generator-fails-on-stringvalues -> C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\bin\Debug\net7.0\system-text-json-source-generator-fails-on-stringvalues.dll

Build succeeded.

C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\SomeJsonSerializerContext.StringValues.g.cs(31,32): warning CS8631: The type 'Microsoft.Extensions.Primitives.StringValues' cannot be used as type parameter 'TCollection' in the generic type or method 'JsonMetadataServices.CreateIListInfo<TCollection, TElement>(JsonSerializerOptions, JsonCollectionInfoValues<TCollection>)'. Nullability of type argument 'Microsoft.Extensions.Primitives.StringValues' doesn't match constraint type 'System.Collections.Generic.IList<string>'. [C:\Users\adren\rider\system-text-json-source-generator-fails-on-stringvalues\system-text-json-source-generator-fails-on-stringvalues.csproj]
    1 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.41

Expected behavior

Using StringValues in conjunction with JsonSerializerContext does not produce warnings.

Actual behavior

Using StringValues in conjunction with JsonSerializerContext produces warnings.

warning CS8631: The type 'Microsoft.Extensions.Primitives.StringValues' cannot be used as type parameter 'TCollection' in the generic type or method 'JsonMetadataServices.CreateIListInfo<TCollection, TElement>(JsonSerializerOptions, JsonCollectionInfoValues)'. Nullability of type argument 'Microsoft.Extensions.Primitives.StringValues' doesn't match constraint type 'System.Collections.Generic.IList'.

Regression?

This works when using version 6.0.0 of Microsoft.Extensions.Primitives because the interfaces for StringValues were updated with nullable annotations for 7.0.0-preview*.

Known Workarounds

  • Suppress CS8631
  • Stay on version 6.0.0 of Microsoft.Extensions.Primitives

Configuration

$ dotnet --info
.NET SDK:
 Version:   7.0.100-preview.7.22377.5
 Commit:    ba310d9309

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22000
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\7.0.100-preview.7.22377.5\

Host:
  Version:      7.0.0-preview.7.22375.6
  Architecture: x64
  Commit:       eecb028078

Other information

Not sure where the fix belongs, but here's where the warning bubbles from:

public static JsonTypeInfo<TCollection> CreateIListInfo<TCollection, TElement>(
JsonSerializerOptions options,
JsonCollectionInfoValues<TCollection> collectionInfo)
where TCollection : IList<TElement>

Author: austindrenski
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Aug 26, 2022

Related to #61734. I don't believe this is a regression in .NET 7, the warnings have surfaced because of updates to the nullability annotation of StringValues.

@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label Aug 26, 2022
@eiriktsarpalis eiriktsarpalis added this to the 8.0.0 milestone Aug 26, 2022
@eiriktsarpalis eiriktsarpalis added the enhancement Product code improvement that does NOT require public API changes/additions label Aug 26, 2022
@austindrenski
Copy link
Contributor Author

austindrenski commented Aug 26, 2022

Related to #61734. I don't believe this is a regression in .NET 7, the warnings have surfaced because of updates to the nullability annotation of StringValues.

@eiriktsarpalis I agree this isn't an STJ regression in .NET 7 (traces back to #55566 which shipped with .NET 6), but I was surprised to run into it using a combination of System.Text.Json + Microsoft.Extensions.Primitives (i.e. no shade, just the same repo, so had assumed there would have been a test case somewhere).

Given RC1 is right around the corner, I'm assuming this won't meet the bar for shipping with 7.0.0, but since it looks like it impacts any IList<T?> where T : class, would it meet the bar for an STJ 7.0.1 release?

Investigation stuff

Collecting snippets here to see if this is something I can contribute to, or if it's out of my depth.

Here's the generated source causing the issue from SomeJsonSerializerContext.StringValues.g.cs:

// <auto-generated/>
#nullable enable

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0618
    public sealed partial class SomeJsonSerializerContext
    {
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Microsoft.Extensions.Primitives.StringValues>? _StringValues;
        public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Microsoft.Extensions.Primitives.StringValues> StringValues
        {
            get => _StringValues ??= Create_StringValues(Options);
        }
        
        private static global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Microsoft.Extensions.Primitives.StringValues> Create_StringValues(global::System.Text.Json.JsonSerializerOptions options)
        {
            global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::Microsoft.Extensions.Primitives.StringValues>? jsonTypeInfo = null;
            global::System.Text.Json.Serialization.JsonConverter? customConverter;
            if (options.Converters.Count > 0 && (customConverter = GetRuntimeProvidedCustomConverter(options, typeof(global::Microsoft.Extensions.Primitives.StringValues))) != null)
            {
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::Microsoft.Extensions.Primitives.StringValues>(options, customConverter);
            }
            else
            {
                global::System.Text.Json.Serialization.Metadata.JsonCollectionInfoValues<global::Microsoft.Extensions.Primitives.StringValues> info = new global::System.Text.Json.Serialization.Metadata.JsonCollectionInfoValues<global::Microsoft.Extensions.Primitives.StringValues>()
                {
                    ObjectCreator = () => new global::Microsoft.Extensions.Primitives.StringValues(),
                    NumberHandling = default,
                    SerializeHandler = StringValuesSerializeHandler
                };
        
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateIListInfo<global::Microsoft.Extensions.Primitives.StringValues, global::System.String>(options, info);
        
            }
        
            return jsonTypeInfo;
        }
        
        
        private static void StringValuesSerializeHandler(global::System.Text.Json.Utf8JsonWriter writer, global::Microsoft.Extensions.Primitives.StringValues value)
        {
            
            writer.WriteStartArray();
        
            for (int i = 0; i < value.Count; i++)
            {
                writer.WriteStringValue(value[i]);
            }
        
            writer.WriteEndArray();
        }
    }

where the offending line is:

                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateIListInfo<global::Microsoft.Extensions.Primitives.StringValues, global::System.String>(options, info);

and the literal patch would be:

-               jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateIListInfo<global::Microsoft.Extensions.Primitives.StringValues, global::System.String>(options, info);
+               jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateIListInfo<global::Microsoft.Extensions.Primitives.StringValues, global::System.String?>(options, info);

Back tracking through the generator source code:

So it seems like the patch would probably involve making TypeGenerationSpec (more) aware of the annotated nullability of collection types being passed into it. Or maybe preserving the exact annotations when the type is an implemented interface (i.e. StringValues : IList<string?>).

Not sure how much work that would involve, but going to keep poking at this until/unless someone with more experience wants to jump in.

@eiriktsarpalis
Copy link
Member

Honestly, I don't think this would meet the bar for a .NET 7 fix at this stage. Realistically the only safe intervention might be to introduce a #nullable disable warnings declaration in all source generated files cc @layomia @krwq

@krwq
Copy link
Member

krwq commented Aug 29, 2022

Given though technically it isn't directly our regression it is a regression from the overall product perspective. IMO we should at least bar check this and I agree the appropriate fix for 7.0 (if any) seems like disabling warnings for the generated code since we don't know all the edge cases

@eiriktsarpalis
Copy link
Member

Let's try to mitigate the problem (and the related issue in #61734) by disabling nullability warnings in .NET 7.

@eiriktsarpalis eiriktsarpalis modified the milestones: 8.0.0, 7.0.0 Aug 29, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 30, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 31, 2022
@eiriktsarpalis
Copy link
Member

See #74801 (comment). We should keep this one open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants