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

Default value for optional ctor params should be used when matching prop is ignored #60082

Closed
Tracked by #73255
layomia opened this issue Oct 6, 2021 · 4 comments · Fixed by #81842
Closed
Tracked by #73255

Comments

@layomia
Copy link
Contributor

layomia commented Oct 6, 2021

Repro:

public class Point
{
    public int X { get; }

    [JsonIgnore]
    public int Y { get; }

    public Point(int x, int y = 5) => (X, Y) = (x, y);
}

string json = "{}";
var point = JsonSerializer.Deserialize<Point>(json);
Console.WriteLine(point.Y);
// Expected: 5
// Actual: 0

The behavior should be consistent with then the matching prop is not ignored:

public class Point
{
    public int X { get; }

    public int Y { get; }

    public Point(int x, int y = 5) => (X, Y) = (x, y);
}

string json = "{}";
var point = JsonSerializer.Deserialize<Point>(json);
Console.WriteLine(point.Y);
// Expected: 5
// Actual: 5
@layomia layomia added this to the 7.0.0 milestone Oct 6, 2021
@layomia layomia self-assigned this Oct 6, 2021
@ghost
Copy link

ghost commented Oct 6, 2021

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

Issue Details

Repro:

public class Point
{
    public int X { get; }

    [JsonIgnore]
    public int Y { get; }

    public Point(int x, int y = 5) => (X, Y) = (x, y);
}

string json = "{}";
var point = JsonSerializer.Deserialize<Point>(json);
Console.WriteLine(point.Y);
// Expected: 5
// Actual: 0

The behavior should be consistent with then the matching prop is not ignored:

public class Point
{
    public int X { get; }

    public int Y { get; }

    public Point(int x, int y = 5) => (X, Y) = (x, y);
}

string json = "{}";
var point = JsonSerializer.Deserialize<Point>(json);
Console.WriteLine(point.Y);
// Expected: 5
// Actual: 5
Author: layomia
Assignees: layomia
Labels:

area-System.Text.Json

Milestone: 7.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Oct 6, 2021
@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Oct 6, 2021
@krwq
Copy link
Member

krwq commented Oct 7, 2021

should we consider this for 6.0? it might lead to data loss

@eiriktsarpalis
Copy link
Member

should we consider this for 6.0? it might lead to data loss

Is this a regression from 5.0 behavior?

@eiriktsarpalis eiriktsarpalis added bug Priority:1 Work that is critical for the release, but we could probably ship without labels Oct 7, 2021
@layomia
Copy link
Contributor Author

layomia commented Oct 7, 2021

Is this a regression from 5.0 behavior?

No this behavior shipped in 5.0. A fix for this can be considered a breaking change. I recommend addressing early in 7.0 as a bug fix so we have time to react to any feedback.

@jeffhandley jeffhandley removed the Priority:1 Work that is critical for the release, but we could probably ship without label Jul 9, 2022
@jeffhandley jeffhandley modified the milestones: 7.0.0, Future Jul 9, 2022
@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 8.0.0 Aug 1, 2022
@eiriktsarpalis eiriktsarpalis modified the milestones: 8.0.0, Future Sep 28, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 8, 2023
@eiriktsarpalis eiriktsarpalis self-assigned this Feb 8, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants