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

JSON source gen doesn't detect [JsonNumberHandling] when applied to POCO #58638

Closed
Tracked by #77019
steveharter opened this issue Sep 3, 2021 · 2 comments · Fixed by #87484
Closed
Tracked by #77019

JSON source gen doesn't detect [JsonNumberHandling] when applied to POCO #58638

steveharter opened this issue Sep 3, 2021 · 2 comments · Fixed by #87484
Labels
area-System.Text.Json bug source-generator Indicates an issue with a source generator feature
Milestone

Comments

@steveharter
Copy link
Member

The attribute is ignored, causing number-based properties to be written as integers, for example, instead of a string.

Repro:

using System.Text.Json;
using System.Text.Json.Serialization;

Console.WriteLine(JsonSerializer.Serialize(new MyPOCO(), MyContext.Default.MyPOCO));

[JsonSerializable(typeof(MyPOCO))]
public partial class MyContext : JsonSerializerContext { }

[JsonNumberHandling(JsonNumberHandling.WriteAsString)]
public class MyPOCO
{
    public int Id {  get; set; }
}

Outputs {"Id":0} instead of the expected {"Id":"0"}

Also when applying [JsonSourceGenerationOptions(GenerationMode =JsonSourceGenerationMode.Serialization)] to the context, a compile-time (or run-time based on current expected behavior) should occur since fast-path doesn't support number handling.

@steveharter steveharter added this to the 6.0.0 milestone Sep 3, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Sep 3, 2021
@ghost
Copy link

ghost commented Sep 3, 2021

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

Issue Details

The attribute is ignored, causing number-based properties to be written as integers, for example, instead of a string.

Repro:

using System.Text.Json;
using System.Text.Json.Serialization;

Console.WriteLine(JsonSerializer.Serialize(new MyPOCO(), MyContext.Default.MyPOCO));

[JsonSerializable(typeof(MyPOCO))]
public partial class MyContext : JsonSerializerContext { }

[JsonNumberHandling(JsonNumberHandling.WriteAsString)]
public class MyPOCO
{
    public int Id {  get; set; }
}

Outputs {"Id":0} instead of the expected {"Id":"0"}

Also when applying [JsonSourceGenerationOptions(GenerationMode =JsonSourceGenerationMode.Serialization)] to the context, a compile-time (or run-time based on current expected behavior) should occur since fast-path doesn't support number handling.

Author: steveharter
Assignees: -
Labels:

area-System.Text.Json, blocking-release

Milestone: 6.0.0

@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Sep 3, 2021
@layomia layomia self-assigned this Sep 3, 2021
@eiriktsarpalis
Copy link
Member

The issue can be worked around by applying the attribute on the property level. As such I don't believe it meets the bar for a 6.0.0 fix. cc @ericstj

@eiriktsarpalis eiriktsarpalis modified the milestones: 6.0.0, 7.0.0 Oct 4, 2021
@eiriktsarpalis eiriktsarpalis added bug Priority:1 Work that is critical for the release, but we could probably ship without and removed blocking-release labels Oct 4, 2021
@layomia layomia added the source-generator Indicates an issue with a source generator feature label Jan 21, 2022
@jeffhandley jeffhandley modified the milestones: 7.0.0, Future Jul 10, 2022
@jeffhandley jeffhandley removed the Priority:1 Work that is critical for the release, but we could probably ship without label Jul 10, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 8, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 6, 2023
@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 8.0.0 Jun 7, 2023
@ghost ghost added in-pr There is an active PR which will close this issue when it is merged and removed in-pr There is an active PR which will close this issue when it is merged labels Jun 13, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 13, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 13, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 14, 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
5 participants