From 12c1ead8736c9e3738e7742392c6a806aa5607c6 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Mon, 20 Jun 2022 08:38:23 +0100 Subject: [PATCH] Fixes #64159 - JsonSerializerContext source generation fails with keyword identifiers (#66876) * Fixes #64159 - initial implementation * Tidy up, and add test for ignored reserved-keyword-named property * PR feedback - use same approach as logging generator * PR feedback * Update src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com> * PR feedback - rename ExactNameSpecifiedInSourceCode * PR feedback - use extracted (and renamed) local variable * Remove commented code * Added `IsVerbatimName` as extension method * Support fields with verbatim names (@) * Use alternative method for checking for verbatim names * Uses `SyntaxFacts` to determine if escaping is needed * Remove extension method * Modified source generator test to include a verbatim field * Minor typo Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com> --- .../gen/JsonSourceGenerator.Emitter.cs | 14 ++--- .../gen/JsonSourceGenerator.Parser.cs | 10 +++- .../System.Text.Json/gen/PropertyGenerationSpec.cs | 7 +++ .../gen/Reflection/FieldInfoWrapper.cs | 5 ++ .../gen/Reflection/PropertyInfoWrapper.cs | 7 ++- .../JsonSerializerContextTests.cs | 53 +++++++++++++++++ .../JsonSourceGeneratorTests.cs | 66 +++++++++++++++++++++- 7 files changed, 150 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs index f18b452..a68bc3b 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs @@ -718,7 +718,7 @@ private static {JsonPropertyInfoTypeRef}[] {propInitMethodName}({JsonSerializerC TypeGenerationSpec memberTypeMetadata = memberMetadata.TypeGenerationSpec; - string clrPropertyName = memberMetadata.ClrName; + string nameSpecifiedInSourceCode = memberMetadata.NameSpecifiedInSourceCode; string declaringTypeCompilableName = memberMetadata.DeclaringTypeRef; @@ -733,9 +733,9 @@ private static {JsonPropertyInfoTypeRef}[] {propInitMethodName}({JsonSerializerC string getterValue = memberMetadata switch { { DefaultIgnoreCondition: JsonIgnoreCondition.Always } => "null", - { CanUseGetter: true } => $"static (obj) => (({declaringTypeCompilableName})obj).{clrPropertyName}{(memberMetadata.TypeGenerationSpec.CanContainNullableReferenceAnnotations ? "!" : "")}", + { CanUseGetter: true } => $"static (obj) => (({declaringTypeCompilableName})obj).{nameSpecifiedInSourceCode}{(memberMetadata.TypeGenerationSpec.CanContainNullableReferenceAnnotations ? "!" : "")}", { CanUseGetter: false, HasJsonInclude: true } - => @$"static (obj) => throw new {InvalidOperationExceptionTypeRef}(""{string.Format(ExceptionMessages.InaccessibleJsonIncludePropertiesNotSupported, typeGenerationSpec.Type.Name, memberMetadata.ClrName)}"")", + => @$"static (obj) => throw new {InvalidOperationExceptionTypeRef}(""{string.Format(ExceptionMessages.InaccessibleJsonIncludePropertiesNotSupported, typeGenerationSpec.Type.Name, nameSpecifiedInSourceCode)}"")", _ => "null" }; @@ -745,9 +745,9 @@ private static {JsonPropertyInfoTypeRef}[] {propInitMethodName}({JsonSerializerC { CanUseSetter: true, IsInitOnlySetter: true } => @$"static (obj, value) => throw new {InvalidOperationExceptionTypeRef}(""{ExceptionMessages.InitOnlyPropertyDeserializationNotSupported}"")", { CanUseSetter: true } when typeGenerationSpec.IsValueType - => $@"static (obj, value) => {UnsafeTypeRef}.Unbox<{declaringTypeCompilableName}>(obj).{clrPropertyName} = value!", + => $@"static (obj, value) => {UnsafeTypeRef}.Unbox<{declaringTypeCompilableName}>(obj).{nameSpecifiedInSourceCode} = value!", { CanUseSetter: true } - => @$"static (obj, value) => (({declaringTypeCompilableName})obj).{clrPropertyName} = value!", + => @$"static (obj, value) => (({declaringTypeCompilableName})obj).{nameSpecifiedInSourceCode} = value!", { CanUseSetter: false, HasJsonInclude: true } => @$"static (obj, value) => throw new {InvalidOperationExceptionTypeRef}(""{string.Format(ExceptionMessages.InaccessibleJsonIncludePropertiesNotSupported, typeGenerationSpec.Type.Name, memberMetadata.ClrName)}"")", _ => "null", @@ -781,7 +781,7 @@ private static {JsonPropertyInfoTypeRef}[] {propInitMethodName}({JsonSerializerC HasJsonInclude = {FormatBool(memberMetadata.HasJsonInclude)}, IsExtensionData = {FormatBool(memberMetadata.IsExtensionData)}, NumberHandling = {GetNumberHandlingAsStr(memberMetadata.NumberHandling)}, - PropertyName = ""{clrPropertyName}"", + PropertyName = ""{memberMetadata.ClrName}"", JsonPropertyName = {jsonPropertyNameValue} }}; @@ -894,7 +894,7 @@ private static {JsonParameterInfoValuesTypeRef}[] {typeGenerationSpec.TypeInfoPr Type propertyType = propertyTypeSpec.Type; string? objectRef = castingRequiredForProps ? $"(({propertyGenSpec.DeclaringTypeRef}){ValueVarName})" : ValueVarName; - string propValue = $"{objectRef}.{propertyGenSpec.ClrName}"; + string propValue = $"{objectRef}.{propertyGenSpec.NameSpecifiedInSourceCode}"; string methodArgs = $"{propVarName}, {propValue}"; string? methodToCall = GetWriterMethod(propertyType); diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs index 5880347..b539258 100644 --- a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs +++ b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs @@ -1212,13 +1212,21 @@ namespace System.Text.Json.SourceGeneration out bool setterIsVirtual, out bool setterIsInitOnly); + bool needsAtSign = memberInfo switch + { + PropertyInfoWrapper prop => prop.NeedsAtSign, + FieldInfoWrapper field => field.NeedsAtSign, + _ => false + }; + string clrName = memberInfo.Name; string runtimePropertyName = DetermineRuntimePropName(clrName, jsonPropertyName, _currentContextNamingPolicy); string propertyNameVarName = DeterminePropNameIdentifier(runtimePropertyName); return new PropertyGenerationSpec { - ClrName = clrName, + NameSpecifiedInSourceCode = needsAtSign ? "@" + memberInfo.Name : memberInfo.Name, + ClrName = memberInfo.Name, IsProperty = memberInfo.MemberType == MemberTypes.Property, IsPublic = isPublic, IsVirtual = isVirtual, diff --git a/src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs b/src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs index c9c4cbd..62d6ff7 100644 --- a/src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs +++ b/src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs @@ -9,6 +9,13 @@ namespace System.Text.Json.SourceGeneration [DebuggerDisplay("Name={Name}, Type={TypeMetadata}")] internal sealed class PropertyGenerationSpec { + /// + /// The exact name specified in the source code. This might be different + /// from the because source code might be decorated + /// with '@' for reserved keywords, e.g. public string @event { get; set; } + /// + public string NameSpecifiedInSourceCode { get; init; } + public string ClrName { get; init; } /// diff --git a/src/libraries/System.Text.Json/gen/Reflection/FieldInfoWrapper.cs b/src/libraries/System.Text.Json/gen/Reflection/FieldInfoWrapper.cs index b2fd062..92f0530 100644 --- a/src/libraries/System.Text.Json/gen/Reflection/FieldInfoWrapper.cs +++ b/src/libraries/System.Text.Json/gen/Reflection/FieldInfoWrapper.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Reflection; using Microsoft.CodeAnalysis; using System.Globalization; +using Microsoft.CodeAnalysis.CSharp; namespace System.Text.Json.Reflection { @@ -16,6 +17,8 @@ namespace System.Text.Json.Reflection { _field = parameter; _metadataLoadContext = metadataLoadContext; + + NeedsAtSign = SyntaxFacts.GetKeywordKind(_field.Name) != SyntaxKind.None || SyntaxFacts.GetContextualKeywordKind(_field.Name) != SyntaxKind.None; } private FieldAttributes? _attributes; @@ -64,6 +67,8 @@ namespace System.Text.Json.Reflection public override string Name => _field.Name; + public bool NeedsAtSign { get; } + public override Type ReflectedType => throw new NotImplementedException(); public override object[] GetCustomAttributes(bool inherit) diff --git a/src/libraries/System.Text.Json/gen/Reflection/PropertyInfoWrapper.cs b/src/libraries/System.Text.Json/gen/Reflection/PropertyInfoWrapper.cs index 120daa5..086c7df 100644 --- a/src/libraries/System.Text.Json/gen/Reflection/PropertyInfoWrapper.cs +++ b/src/libraries/System.Text.Json/gen/Reflection/PropertyInfoWrapper.cs @@ -5,18 +5,21 @@ using System.Collections.Generic; using System.Globalization; using System.Reflection; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; namespace System.Text.Json.Reflection { internal sealed class PropertyInfoWrapper : PropertyInfo { private readonly IPropertySymbol _property; - private MetadataLoadContextInternal _metadataLoadContext; + private readonly MetadataLoadContextInternal _metadataLoadContext; public PropertyInfoWrapper(IPropertySymbol property, MetadataLoadContextInternal metadataLoadContext) { _property = property; _metadataLoadContext = metadataLoadContext; + + NeedsAtSign = SyntaxFacts.GetKeywordKind(_property.Name) != SyntaxKind.None || SyntaxFacts.GetContextualKeywordKind(_property.Name) != SyntaxKind.None; } public override PropertyAttributes Attributes => throw new NotImplementedException(); @@ -31,6 +34,8 @@ namespace System.Text.Json.Reflection public override string Name => _property.Name; + public bool NeedsAtSign { get; } + public override Type ReflectedType => throw new NotImplementedException(); public override MethodInfo[] GetAccessors(bool nonPublic) diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs index ea17141..4b3c128 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs @@ -61,6 +61,36 @@ namespace System.Text.Json.SourceGeneration.Tests } [Fact] + public static void SupportsReservedLanguageKeywordsAsProperties() + { + GreetingCard card = new() + { + @event = "Birthday", + message = @"Happy Birthday!" + }; + + byte[] utf8Json = JsonSerializer.SerializeToUtf8Bytes(card, GreetingCardJsonContext.Default.GreetingCard); + + card = JsonSerializer.Deserialize(utf8Json, GreetingCardJsonContext.Default.GreetingCard); + Assert.Equal("Birthday", card.@event); + Assert.Equal("Happy Birthday!", card.message); + } + + [Fact] + public static void SupportsReservedLanguageKeywordsAsFields() + { + var options = new JsonSerializerOptions { IncludeFields = true }; + + GreetingCardWithFields card = new() {@event = "Birthday", message = @"Happy Birthday!"}; + + byte[] utf8Json = JsonSerializer.SerializeToUtf8Bytes(card, GreetingCardWithFieldsJsonContext.Default.GreetingCardWithFields); + + card = JsonSerializer.Deserialize(utf8Json, GreetingCardWithFieldsJsonContext.Default.GreetingCardWithFields); + Assert.Equal("Happy Birthday!", card.message); + Assert.Equal("Birthday", card.@event); + } + + [Fact] public static void SupportsPositionalRecords() { Person person = new(FirstName: "Jane", LastName: "Doe"); @@ -91,6 +121,29 @@ namespace System.Text.Json.SourceGeneration.Tests { } + internal class GreetingCard + { + public string @event { get;set; } + public string message { get;set; } + } + + internal class GreetingCardWithFields + { + public string @event; + public string message; + } + + [JsonSerializable(typeof(GreetingCard))] + internal partial class GreetingCardJsonContext : JsonSerializerContext + { + } + + [JsonSourceGenerationOptions(IncludeFields = true)] + [JsonSerializable(typeof(GreetingCardWithFields))] + internal partial class GreetingCardWithFieldsJsonContext : JsonSerializerContext + { + } + // Regression test for https://github.com/dotnet/runtime/issues/62079 [Fact] public static void SupportsPropertiesWithCustomConverterFactory() diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs index f1d5b19..245c1a4 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs @@ -324,7 +324,7 @@ namespace System.Text.Json.Serialization CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Info, generatorDiags, Array.Empty<(Location, string)>()); CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Warning, generatorDiags, Array.Empty<(Location, string)>()); - CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Error, generatorDiags, Array.Empty<(Location, string)>()); + CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Error, generatorDiags, Array.Empty<(Location, string)>()); } [Fact] @@ -647,7 +647,7 @@ namespace System.Text.Json.Serialization receivedFields = fields.ToArray(); receivedProperties = props.ToArray(); } - + string[] receivedMethods = type.GetMethods().Select(method => method.Name).OrderBy(s => s).ToArray(); Array.Sort(receivedFields); @@ -722,7 +722,7 @@ namespace System.Text.Json.Serialization [Fact] public static void NoWarningsDueToObsoleteMembers() { - string source = @"using System; + string source = @"using System; using System.Text.Json.Serialization; namespace Test @@ -749,5 +749,65 @@ namespace Test CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Warning, generatorDiags, Array.Empty<(Location, string)>()); CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Error, generatorDiags, Array.Empty<(Location, string)>()); } + + [Fact] + public static void NoErrorsWhenUsingReservedCSharpKeywords() + { + string source = @"using System; +using System.Text.Json.Serialization; + +namespace Test +{ + [JsonSerializable(typeof(ClassWithPropertiesAndFieldsThatAreReservedKeywords))] + public partial class JsonContext : JsonSerializerContext { } + + public class ClassWithPropertiesAndFieldsThatAreReservedKeywords + { + public int @class; + public string @event { get; set; } + } +} +"; + Compilation compilation = CompilationHelper.CreateCompilation(source); + JsonSourceGenerator generator = new JsonSourceGenerator(); + + Compilation newCompilation = CompilationHelper.RunGenerators(compilation, out _, generator); + ImmutableArray generatorDiags = newCompilation.GetDiagnostics(); + + // No diagnostics expected. + CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Info, generatorDiags, Array.Empty<(Location, string)>()); + CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Warning, generatorDiags, Array.Empty<(Location, string)>()); + CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Error, generatorDiags, Array.Empty<(Location, string)>()); + } + + [Fact] + public static void NoErrorsWhenUsingIgnoredReservedCSharpKeywords() + { + string source = @"using System; +using System.Text.Json.Serialization; + +namespace Test +{ + [JsonSerializable(typeof(ClassWithPropertyNameThatIsAReservedKeyword))] + public partial class JsonContext : JsonSerializerContext { } + + public class ClassWithPropertyNameThatIsAReservedKeyword + { + [JsonIgnore] + public string @event { get; set; } + } +} +"; + Compilation compilation = CompilationHelper.CreateCompilation(source); + JsonSourceGenerator generator = new JsonSourceGenerator(); + + Compilation newCompilation = CompilationHelper.RunGenerators(compilation, out _, generator); + ImmutableArray generatorDiags = newCompilation.GetDiagnostics(); + + // No diagnostics expected. + CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Info, generatorDiags, Array.Empty<(Location, string)>()); + CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Warning, generatorDiags, Array.Empty<(Location, string)>()); + CompilationHelper.CheckDiagnosticMessages(DiagnosticSeverity.Error, generatorDiags, Array.Empty<(Location, string)>()); + } } } -- 2.7.4