Skip to content

Commit

Permalink
STJ: Avoid duplicate initialization of required or init-only properti…
Browse files Browse the repository at this point in the history
…es (#97726)

* STJ: Avoid duplicate initialization of required or init-only properties

* Add more test combinations

* Move shadowed properties check

* Update metadata flow accordingly

* Add init value for all properties

* Add more shadowing test cases

* Remove extra empty line

* Move deduplication logic to property initializer parser.

* Revert DefaultJsonTypeInfoResolver changes

* Update src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs

* Update src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs

---------

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
  • Loading branch information
manandre and eiriktsarpalis authored Feb 5, 2024
1 parent f28bcf1 commit 8ac26af
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 0 deletions.
13 changes: 13 additions & 0 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1437,6 +1437,7 @@ private void ProcessMember(
return null;
}

HashSet<string>? memberInitializerNames = null;
List<PropertyInitializerGenerationSpec>? propertyInitializers = null;
int paramCount = constructorParameters?.Length ?? 0;

Expand All @@ -1450,6 +1451,18 @@ private void ProcessMember(

if ((property.IsRequired && !constructorSetsRequiredMembers) || property.IsInitOnlySetter)
{
if (!(memberInitializerNames ??= new()).Add(property.MemberName))
{
// We've already added another member initializer with the same name to our spec list.
// Duplicates can occur here because the provided list of properties includes shadowed members.
// This is because we generate metadata for *all* members, including shadowed or ignored ones,
// since we need to re-run the deduplication algorithm taking run-time configuration into account.
// This is a simple deduplication that keeps the first result for each member name --
// this should be fine since the properties are listed from most derived to least derived order,
// so the second instance of a member name is always shadowed by the first.
continue;
}

ParameterGenerationSpec? matchingConstructorParameter = GetMatchingConstructorParameter(property, constructorParameters);

if (property.IsRequired || matchingConstructorParameter is null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,52 @@ public class ClassWithCustomRequiredPropertyName
public required int PropertyWithInitOnlySetter { get; init; }
}

[Fact]
public async Task DerivedClassWithRequiredProperty()
{
var options = Serializer.CreateOptions(includeFields: true);
var value = new DerivedClassWithRequiredInitOnlyProperty { MyInt = 42, MyBool = true, MyString = "42", MyProp = 42.0M, MyLong = 4242, MyMember1 = 1, MyMember2 = 2, MyField = "42" };
string json = await Serializer.SerializeWrapper(value, options);
Assert.Equal("""{"MyInt":42,"MyBool":true,"MyString":"42","MyProp":42.0,"MyLong":4242,"MyMember1":1,"MyMember2":2,"MyField":"42"}""", json);

value = await Serializer.DeserializeWrapper<DerivedClassWithRequiredInitOnlyProperty>(json, options);
Assert.Equal(42, value.MyInt);
Assert.True(value.MyBool);
Assert.Equal("42", value.MyString);
Assert.Equal(42.0M, value.MyProp);
Assert.Equal(4242, value.MyLong);
Assert.Equal(1, value.MyMember1);
Assert.Equal(2, value.MyMember2);
Assert.Equal("42", value.MyField);
}

public class BaseClassWithInitOnlyProperty
{
public int MyInt { get; init; }
public bool MyBool { get; init; }
public string MyString { get; set; }
public string MyProp { get; init; }

public string MyMember1;
public string MyMember2 { get; init; }

public string MyField;
}

public class DerivedClassWithRequiredInitOnlyProperty : BaseClassWithInitOnlyProperty
{
public new required int MyInt { get; init; }
public new required bool MyBool { get; set; }
public new string MyString { get; init; }
public new required decimal MyProp { get; init; }
public required long MyLong { get; init; }

public new required int MyMember1 { get; init; }
public new required int MyMember2;

public new required string MyField;
}

public static IEnumerable<object[]> InheritedPersonWithRequiredMembersSetsRequiredMembersWorksAsExpectedSources()
{
yield return new object[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public RequiredKeywordTests_SourceGen()
[JsonSerializable(typeof(ClassWithRequiredExtensionDataProperty))]
[JsonSerializable(typeof(ClassWithRequiredKeywordAndJsonRequiredCustomAttribute))]
[JsonSerializable(typeof(ClassWithCustomRequiredPropertyName))]
[JsonSerializable(typeof(DerivedClassWithRequiredInitOnlyProperty))]
internal sealed partial class RequiredKeywordTestsContext : JsonSerializerContext
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -768,5 +768,32 @@ internal partial class JsonContext : JsonSerializerContext
Compilation compilation = CompilationHelper.CreateCompilation(source);
CompilationHelper.RunJsonSourceGenerator(compilation);
}

#if ROSLYN4_4_OR_GREATER && NETCOREAPP
[Fact]
public void ShadowedMemberInitializers()
{
string source = """
using System.Text.Json.Serialization;
public record Base
{
public string Value { get; init; }
}
public record Derived : Base
{
public new string Value { get; init; }
}
[JsonSerializable(typeof(Derived))]
public partial class MyContext : JsonSerializerContext
{
}
""";

Compilation compilation = CompilationHelper.CreateCompilation(source, parseOptions: CompilationHelper.CreateParseOptions(LanguageVersion.CSharp11));
CompilationHelper.RunJsonSourceGenerator(compilation);
}
#endif
}
}

0 comments on commit 8ac26af

Please sign in to comment.