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 fails for covariant properties when [JsonIgnore] is specified #63443

Closed
Tracked by #73255
Kloizdena opened this issue Jan 6, 2022 · 8 comments
Labels
area-System.Text.Json bug source-generator Indicates an issue with a source generator feature
Milestone

Comments

@Kloizdena
Copy link
Contributor

Description

I have a class hierarchy where the derived class overrides one of the properties using covariant return type. I want to omit this property from the JSON, so I added [JsonIgnore] attributes to both properties:

public class A
{
	[JsonIgnore]
	public virtual object Id { get; }
}
public class GenericB<T> : A
	where T : class
{
	[JsonIgnore]
	public override T Id { get; }
}

When I try to use the Json source generator for the derived type, it fails. I tried without generics, but it also fails.

Reproduction Steps

Add the following classes to a .NET 6 project and try to build it:

using System.Text.Json.Serialization;

public class A
{
	[JsonIgnore]
	public virtual object Id { get; }
}
public class B : A
{
	[JsonIgnore]
	public override string Id { get; }
}
public class GenericB<T> : A
	where T : class
{
	[JsonIgnore]
	public override T Id { get; }
}

[JsonSerializable(typeof(B))]
[JsonSerializable(typeof(GenericB<string>))]
public partial class Context : JsonSerializerContext
{

}

Expected behavior

The project should build without errors.

Actual behavior

The source generator fails with an error:

Build started...
1>------ Build started: Project: JsonTest, 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 'ArgumentException' with message 'An item with the same key has already been added.'
1>C:\src\JsonTest\JsonTest\JsonClasses.cs(22,22,22,29): error CS0534: 'Context' does not implement inherited abstract member 'JsonSerializerContext.GeneratedSerializerOptions.get'
1>C:\src\JsonTest\JsonTest\JsonClasses.cs(22,22,22,29): error CS0534: 'Context' does not implement inherited abstract member 'JsonSerializerContext.GetTypeInfo(Type)'
1>Done building project "JsonTest.csproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

Regression?

No response

Known Workarounds

No response

Configuration

.NET 6.0.101
Microsoft Windows [Version 10.0.19044.1415]
x64

Other information

No response

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

ghost commented Jan 6, 2022

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 have a class hierarchy where the derived class overrides one of the properties using covariant return type. I want to omit this property from the JSON, so I added [JsonIgnore] attributes to both properties:

public class A
{
	[JsonIgnore]
	public virtual object Id { get; }
}
public class GenericB<T> : A
	where T : class
{
	[JsonIgnore]
	public override T Id { get; }
}

When I try to use the Json source generator for the derived type, it fails. I tried without generics, but it also fails.

Reproduction Steps

Add the following classes to a .NET 6 project and try to build it:

using System.Text.Json.Serialization;

public class A
{
	[JsonIgnore]
	public virtual object Id { get; }
}
public class B : A
{
	[JsonIgnore]
	public override string Id { get; }
}
public class GenericB<T> : A
	where T : class
{
	[JsonIgnore]
	public override T Id { get; }
}

[JsonSerializable(typeof(B))]
[JsonSerializable(typeof(GenericB<string>))]
public partial class Context : JsonSerializerContext
{

}

Expected behavior

The project should build without errors.

Actual behavior

The source generator fails with an error:

Build started...
1>------ Build started: Project: JsonTest, 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 'ArgumentException' with message 'An item with the same key has already been added.'
1>C:\src\JsonTest\JsonTest\JsonClasses.cs(22,22,22,29): error CS0534: 'Context' does not implement inherited abstract member 'JsonSerializerContext.GeneratedSerializerOptions.get'
1>C:\src\JsonTest\JsonTest\JsonClasses.cs(22,22,22,29): error CS0534: 'Context' does not implement inherited abstract member 'JsonSerializerContext.GetTypeInfo(Type)'
1>Done building project "JsonTest.csproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

Regression?

No response

Known Workarounds

No response

Configuration

.NET 6.0.101
Microsoft Windows [Version 10.0.19044.1415]
x64

Other information

No response

Author: Kloizdena
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@krwq krwq added this to the 7.0.0 milestone Jan 14, 2022
@krwq
Copy link
Member

krwq commented Jan 14, 2022

Agreed this looks incorrect, any chance you'd be interested in contributing a fix?

@krwq krwq added bug and removed untriaged New issue has not been triaged by the area owner labels Jan 14, 2022
@Kloizdena
Copy link
Contributor Author

Yes, I can take a look.

@Kloizdena
Copy link
Contributor Author

Kloizdena commented Jan 14, 2022

The exception is thrown at this line:

if (propGenSpec.DefaultIgnoreCondition == JsonIgnoreCondition.Always)
{
(ignoredMembers ??= new Dictionary<string, PropertyGenerationSpec>()).Add(memberName, propGenSpec);
}

Based on this, I tested a similar case and found that the generator runs into the same problem when there is no covariance involved, only property hiding without overriding:

    public class IgnoredBase
    {
        [JsonIgnore]
        public string Id { get; set; }
    }
    public class IgnoredBase_IgnoredDerived : IgnoredBase
    {
        [JsonIgnore]
        public new string Id { get; set; }
    }

Modifying the Add to TryAdd would solve this issue, but what about other cases when only one of the properties has the [JsonIgnore] attribute?

    public class IgnoredBase
    {
        [JsonIgnore]
        public string Id { get; set; }
    }

    public class IgnoredBase_NotIgnoredDerived : IgnoredBase
    {
        public new string Id { get; set; }
    }

    public class NotIgnoredBase
    {
        public string Id { get; set; }
    }
    public class NotIgnoredBase_IgnoredDerived : NotIgnoredBase
    {
        [JsonIgnore]
        public new string Id { get; set; }
    }

NotIgnoredBase a = new NotIgnoredBase_IgnoredDerived();
a.Id = "Test"; // Should be serialized?
var b = new NotIgnoredBase_IgnoredDerived();
b.Id = "Test"; // Should be ignored?
IgnoredBase c = new IgnoredBase_NotIgnoredDerived();
c.Id = "Test"; // Should be ignored?
var d = new IgnoredBase_NotIgnoredDerived();
d.Id = "Test"; // Should be serialized?

@eiriktsarpalis
Copy link
Member

Slightly related to #59364 which also tracks JsonIgnoreAttribute consistency issues.

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jan 24, 2022

More relevant, #50078

@krwq
Copy link
Member

krwq commented May 31, 2022

As per #63823 (comment) we need to look at this problem holistically and write down all scenarios and their expected behavior before we start making any fixes

@eiriktsarpalis
Copy link
Member

Fixed by #84756

@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 8.0.0 Jun 22, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json bug source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants