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 code generation fails on recursive IReadOnlyDictionary #76802

Closed
Tracked by #77019
ejball opened this issue Oct 10, 2022 · 2 comments · Fixed by #87632
Closed
Tracked by #77019

System.Text.Json code generation fails on recursive IReadOnlyDictionary #76802

ejball opened this issue Oct 10, 2022 · 2 comments · Fixed by #87632
Assignees
Labels
area-System.Text.Json bug source-generator Indicates an issue with a source generator feature
Milestone

Comments

@ejball
Copy link

ejball commented Oct 10, 2022

Description

System.Text.Json code generation fails with error CS1031: Type expected if the serializable class has an IReadOnlyDictionary whose value type is that same serializable class.

Reproduction Steps

Include these two classes in a C# library or console app that references System.Text.Json 6.0.6:

public class Dto
{
    public IReadOnlyDictionary<string, Dto>? Data { get; set; }
}

[JsonSerializable(typeof(Dto))]
public partial class JsonContext : JsonSerializerContext
{
}

Expected behavior

Compilation fails with: ...\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\JsonContext.IReadOnlyDictionaryStringDto.g.cs(27,128,27,129): error CS1031: Type expected

That badly generated line of code looks like this: ObjectCreator = () => new global::System.Collections.Generic.Dictionary<global::System.String, >(),

Actual behavior

It should succeed without error, as it does if Dictionary is used instead of IReadOnlyDictionary.

Regression?

No response

Known Workarounds

Use Dictionary instead of IReadOnlyDictionary.

Configuration

.NET 6.0.401 on Windows x64.

Other information

No response

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

ghost commented Oct 10, 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 code generation fails with error CS1031: Type expected if the serializable class has an IReadOnlyDictionary whose values is that same serializable class.

Reproduction Steps

Include these two classes in a C# library or console app that references System.Text.Json 6.0.6:

public class Dto
{
    public IReadOnlyDictionary<string, Dto>? Data { get; set; }
}

[JsonSerializable(typeof(Dto))]
public partial class JsonContext : JsonSerializerContext
{
}

Expected behavior

Compilation fails with: ...\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\JsonContext.IReadOnlyDictionaryStringDto.g.cs(27,128,27,129): error CS1031: Type expected

That badly generated line of code looks like this: ObjectCreator = () => new global::System.Collections.Generic.Dictionary<global::System.String, >(),

Actual behavior

It should succeed without error, as it does if Dictionary is used instead of IReadOnlyDictionary.

Regression?

No response

Known Workarounds

Use Dictionary instead of IReadOnlyDictionary.

Configuration

.NET 6.0.401 on Windows x64.

Other information

No response

Author: ejball
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label Oct 10, 2022
@eiriktsarpalis eiriktsarpalis added this to the 8.0.0 milestone Oct 10, 2022
@eiriktsarpalis
Copy link
Member

I can repro the issue -- fundamentally the issue stems from the fact that the source generator's "parser" component eagerly evaluates properties of the recursive type, even though it hasn't been fully populated yet:

if (needsRuntimeType)
{
runtimeTypeRef = GetDictionaryTypeRef(collectionKeyTypeSpec, collectionValueTypeSpec);
}

I suspect that this issue has the potential to surface in other cases as well, so in fixing this we should carefully audit the parser code for any other instances where it could be reading from uninitialized metadata, perhaps by making such scenaria impossible by construction.

@eiriktsarpalis eiriktsarpalis added the source-generator Indicates an issue with a source generator feature label Oct 10, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 15, 2023
eiriktsarpalis added a commit to eiriktsarpalis/runtime that referenced this issue Jun 15, 2023
@eiriktsarpalis eiriktsarpalis self-assigned this Jun 15, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 15, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 16, 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.

2 participants