Fixes #64159 - JsonSerializerContext source generation fails with keyword identifiers...
authorSteve Dunn <steve@dunnhq.com>
Mon, 20 Jun 2022 07:38:23 +0000 (08:38 +0100)
committerGitHub <noreply@github.com>
Mon, 20 Jun 2022 07:38:23 +0000 (09:38 +0200)
* 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>
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs
src/libraries/System.Text.Json/gen/Reflection/FieldInfoWrapper.cs
src/libraries/System.Text.Json/gen/Reflection/PropertyInfoWrapper.cs
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.cs
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Unit.Tests/JsonSourceGeneratorTests.cs

index f18b452..a68bc3b 100644 (file)
@@ -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);
index 5880347..b539258 100644 (file)
@@ -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,
index c9c4cbd..62d6ff7 100644 (file)
@@ -9,6 +9,13 @@ namespace System.Text.Json.SourceGeneration
     [DebuggerDisplay("Name={Name}, Type={TypeMetadata}")]
     internal sealed class PropertyGenerationSpec
     {
+        /// <summary>
+        /// The exact name specified in the source code. This might be different
+        /// from the <see cref="ClrName"/> because source code might be decorated
+        /// with '@' for reserved keywords, e.g. public string @event { get; set; }
+        /// </summary>
+        public string NameSpecifiedInSourceCode { get; init; }
+
         public string ClrName { get; init; }
 
         /// <summary>
index b2fd062..92f0530 100644 (file)
@@ -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)
index 120daa5..086c7df 100644 (file)
@@ -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)
index ea17141..4b3c128 100644 (file)
@@ -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<GreetingCard>(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<GreetingCardWithFields>(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()
index f1d5b19..245c1a4 100644 (file)
@@ -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<Diagnostic> 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<Diagnostic> 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)>());
+        }
     }
 }