dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.26k stars 4.73k forks source link

System.Text.Json source generator fails if [Required(ErrorMessage = "...")] attribute is present. #62937

Closed Kloizdena closed 1 year ago

Kloizdena commented 2 years ago

Description

I tried to use the Json source generator with a class which has a [Required] attribute with an error message, but the generator succeeds only when the ErrorMessage is not specified. I tried it with other Validation attributes (StringLength, Range), and I got the same result.

Reproduction Steps

Add the following two classes in a .NET 6.0 project, then try to build it.

using System.ComponentModel.DataAnnotations;
using System.Text.Json.Serialization;

public class TestDto
{
    [Required(ErrorMessage = "asdf")]
    public string Name { get; set; }
}

[JsonSerializable(typeof(TestDto))]
public partial class Serializers : JsonSerializerContext
{

}

Expected behavior

The source generator should generate the properties and the build should succeed.

Actual behavior

The source generator and the build fails and I get an error in the Output window:

Build started...
1>------ Build started: Project: SystemTextJsonTest, Configuration: Debug Any CPU ------
1>CSC : warning CS8785: Generator 'JsonSourceGenerator' failed to generate source. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'InvalidOperationException' with message 'Sequence contains no elements'
1>D:\src\SystemTextJsonTest\SystemTextJsonTest\TestDto.cs(11,22,11,33): error CS0534: 'Serializers' does not implement inherited abstract member 'JsonSerializerContext.GetTypeInfo(Type)'
1>D:\src\SystemTextJsonTest\SystemTextJsonTest\TestDto.cs(11,22,11,33): error CS0534: 'Serializers' does not implement inherited abstract member 'JsonSerializerContext.GeneratedSerializerOptions.get'
1>Done building project "SystemTextJsonTest.csproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

Regression?

No response

Known Workarounds

Remove the ErrorMessage, ErrorMessageResourceName and ErrorMessageResourceType properties from the validation attributes.

Configuration

.NET 6.0.100 Microsoft Windows [Version 10.0.19043.1348] x64

Other information

No response

ghost commented 2 years ago

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

Issue Details
### Description I tried to use the Json source generator with a class which has a [Required] attribute with an error message, but the generator succeeds only when the ErrorMessage is not specified. I tried it with other Validation attributes (StringLength, Range), and I got the same result. ### Reproduction Steps Add the following two classes in a .NET 6.0 project, then try to build it. ```csharp using System.ComponentModel.DataAnnotations; using System.Text.Json.Serialization; public class TestDto { [Required(ErrorMessage = "asdf")] public string Name { get; set; } } [JsonSerializable(typeof(TestDto))] public partial class Serializers : JsonSerializerContext { } ``` ### Expected behavior The source generator should generate the properties and the build should succeed. ### Actual behavior The source generator and the build fails and I get an error in the Output window: ``` Build started... 1>------ Build started: Project: SystemTextJsonTest, Configuration: Debug Any CPU ------ 1>CSC : warning CS8785: Generator 'JsonSourceGenerator' failed to generate source. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'InvalidOperationException' with message 'Sequence contains no elements' 1>D:\src\SystemTextJsonTest\SystemTextJsonTest\TestDto.cs(11,22,11,33): error CS0534: 'Serializers' does not implement inherited abstract member 'JsonSerializerContext.GetTypeInfo(Type)' 1>D:\src\SystemTextJsonTest\SystemTextJsonTest\TestDto.cs(11,22,11,33): error CS0534: 'Serializers' does not implement inherited abstract member 'JsonSerializerContext.GeneratedSerializerOptions.get' 1>Done building project "SystemTextJsonTest.csproj" -- FAILED. ========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ========== ``` ### Regression? _No response_ ### Known Workarounds Remove the ErrorMessage, ErrorMessageResourceName and ErrorMessageResourceType properties from the validation attributes. ### Configuration .NET 6.0.100 Microsoft Windows [Version 10.0.19043.1348] x64 ### Other information _No response_
Author: Kloizdena
Assignees: -
Labels: `area-System.Text.Json`, `untriaged`
Milestone: -
layomia commented 2 years ago

Can repro. This bug can affect lots of users and a bug fix should be considered for 6.0.

deeprobin commented 2 years ago

If it's just a small fix and I can more or less take that from your other PR, of course I can take that. Is there anything to keep in mind for PRs on v6.0 other than choosing the right branch?

deeprobin commented 2 years ago

Oh I see the bot has already created a backport PR.

veertien commented 2 years ago

This issue did not surface for me until I used .NET SDK 6.0.2 (dotnet/core#7172).

sdk setup result
6.0.1 JsonSerializerContext in project A, type to serialize with RequiredAttribute in project B both projects compile without error
6.0.1 JsonSerializerContext and type to serialize both in the same project build error due to 'JsonSourceGenerator' failed to generate source
6.0.2 JsonSerializerContext in project A, type to serialize with RequiredAttribute in project B build error due to 'JsonSourceGenerator' failed to generate source
6.0.2 JsonSerializerContext and type to serialize both in the same project build error due to 'JsonSourceGenerator' failed to generate source
layomia commented 2 years ago

Taking a look.

JakeYallop commented 2 years ago

I've just ran into this issue as well. I'm able to repro the issue in 6.0.1 as well as 6.0.2.

Easy repro here, switching to 6.0.1 of STJ still reproduces the issue. https://github.com/JakeYallop/JsonSerializerContextInvalidOperationExceptionRepro

public class MyClass
{
    [Derived(BaseProperty = "")] //source generator does not compile
    //[Derived(DerivedProperty = "", BaseProperty = "")] //does not compile
    //[Derived] //compiles fine
    //[Derived(DerivedProperty = "")] //compiles fine
    public string Property { get; set; } = null!;
}

public abstract class BaseAttribute : Attribute
{
    public string BaseProperty { get; set; } = null!;
}

[AttributeUsage(AttributeTargets.All)]
public class DerivedAttribute : BaseAttribute
{
    public string DerivedProperty { get; set; } = null!;
}

[JsonSourceGenerationOptions]
[JsonSerializable(typeof(MyClass))]
public partial class JsonContext : JsonSerializerContext
{

}

As a workaround for the short term, I've added an explicit reference to version 7.0.0-preview.2.22152.2 of STJ in my .NET 6 project which includes the fix made above (I usually just use the in-box version, rather than an explicit reference).

Will this fix make it into a 6.0.x version of the STJ package in future?

eiriktsarpalis commented 2 years ago

Moving to .NET 8 milestone.

JakeYallop commented 2 years ago

I'm not sure if this issue should still be open - it seems like this is fixed in .NET 7.0.

eiriktsarpalis commented 2 years ago

@layomia According to https://github.com/dotnet/runtime/issues/62937#issuecomment-1037259847 this is a regression in servicing (although I couldn't pinpoint a specific change that might have introduced this). Should we consider backporting #63837 to .NET 6?

cc @ericstj

onionhammer commented 2 years ago

.net 7's new 'required' & init-only props are also unsupported;

i.e. public required string Test { get; init; }

Is there something tracking this I can subscribe to?

eiriktsarpalis commented 2 years ago

Yes, see https://github.com/dotnet/runtime/issues/58770

eiriktsarpalis commented 1 year ago

@veertien I checked this particular scenario using sdk version 6.0.101

sdk setup result
6.0.1 JsonSerializerContext in project A, type to serialize with RequiredAttribute in project B both projects compile without error

On my machine the compile error is reproducing with that configuration as well. I don't believe we're dealing with a servicing regression here, so I'm going to close this issue. .NET 6 impacted by this issue can try referencing the v7 System.Text.Json NuGet package to incorporate the fix.

xlievo commented 1 year ago

CS0534: So far, it is still easy to reproduce this problem. Referencing Business.SourceGenerator.Test v7 System.Text.Json NuGet package It didn't work!

eiriktsarpalis commented 1 year ago

@xlievo please create a new issue containing a minimal reproduction. Thanks!