System.Text.Json code generator generates invalid code when Json property contains a dash

[Jericho]

I have a class that contains the following property:

[JsonPropertyName("smtp-id")]
public string SmtpId { get; set; }

The code generator generates the following line in MyLibrary.PropertyNames.g.cs:

private static global::System.Text.Json.JsonEncodedText smtp-idPropName = global::System.Text.Json.JsonEncodedText.Encode("smtp-id");

Notice smtp-idPropName. This results in the following errors (and of course prevents my library from being compiled:

CS1026	) expected	MyLibrary (net5.0)	D:\_build\MyLibrary\Source\MyLibrary\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonSerializerContext.PropertyNames.g.cs	264	Active
CS1002	; expected	MyLibrary (net5.0)	D:\_build\MyLibrary\Source\MyLibrary\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonSerializerContext.PropertyNames.g.cs	264	Active

When I scroll down a little further in the "Error List" window, I notice another related error:

CS1519	Invalid token '"smtp-id"' in class, record, struct, or interface member declaration	MyLibrary (net5.0)	D:\_build\MyLibrary\Source\MyLibrary\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonSerializerContext.PropertyNames.g.cs	264	Active

My library parses Json from a 3rd party API therefore I can't simply change the name of this property. I have to match the content of their Json which indeed contains a property with a dash.

❯ dotnet --info
.NET SDK (reflecting any global.json):
Version: 5.0.400

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22000
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\5.0.400\

Host (useful for support):
Version: 6.0.0-preview.7.21377.19

.NET SDKs installed:
2.1.816 [C:\Program Files\dotnet\sdk]
3.1.302 [C:\Program Files\dotnet\sdk]
3.1.400 [C:\Program Files\dotnet\sdk]
3.1.411 [C:\Program Files\dotnet\sdk]
5.0.103 [C:\Program Files\dotnet\sdk]
5.0.104 [C:\Program Files\dotnet\sdk]
5.0.205 [C:\Program Files\dotnet\sdk]
5.0.302 [C:\Program Files\dotnet\sdk]
5.0.400 [C:\Program Files\dotnet\sdk]
6.0.100-preview.7.21379.14 [C:\Program Files\dotnet\sdk]

Microsoft Visual Studio Community 2019
Version 16.11.1
VisualStudio.16.Release/16.11.1+31613.86
Microsoft .NET Framework
Version 4.8.04161

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

Issue Details

---

I have a class that contains the following property:

[JsonPropertyName("smtp-id")]
public string SmtpId { get; set; }

The code generator generates the following line in MyLibrary.PropertyNames.g.cs:

private static global::System.Text.Json.JsonEncodedText smtp-idPropName = global::System.Text.Json.JsonEncodedText.Encode("smtp-id");

Notice smtp-idPropName. This results in the following errors (and of course prevents my library from being compiled:

CS1026	) expected	MyLibrary (net5.0)	D:\_build\MyLibrary\Source\MyLibrary\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonSerializerContext.PropertyNames.g.cs	264	Active
CS1002	; expected	MyLibrary (net5.0)	D:\_build\MyLibrary\Source\MyLibrary\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonSerializerContext.PropertyNames.g.cs	264	Active

When I scroll down a little further in the "Error List" window, I notice another related error:

CS1519	Invalid token '"smtp-id"' in class, record, struct, or interface member declaration	MyLibrary (net5.0)	D:\_build\MyLibrary\Source\MyLibrary\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyJsonSerializerContext.PropertyNames.g.cs	264	Active

My library parses Json from a 3rd party API therefore I can't simply change the name of this property. I have to match the content of their Json which indeed contains a property with a dash.

❯ dotnet --info
.NET SDK (reflecting any global.json):
Version: 5.0.400
Commit: d61950f9bf

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22000
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\5.0.400\

Host (useful for support):
Version: 6.0.0-preview.7.21377.19
Commit: 91ba017

.NET SDKs installed:
2.1.816 [C:\Program Files\dotnet\sdk]
3.1.302 [C:\Program Files\dotnet\sdk]
3.1.400 [C:\Program Files\dotnet\sdk]
3.1.411 [C:\Program Files\dotnet\sdk]
5.0.103 [C:\Program Files\dotnet\sdk]
5.0.104 [C:\Program Files\dotnet\sdk]
5.0.205 [C:\Program Files\dotnet\sdk]
5.0.302 [C:\Program Files\dotnet\sdk]
5.0.400 [C:\Program Files\dotnet\sdk]
6.0.100-preview.7.21379.14 [C:\Program Files\dotnet\sdk]

Microsoft Visual Studio Community 2019
Version 16.11.1
VisualStudio.16.Release/16.11.1+31613.86
Microsoft .NET Framework
Version 4.8.04161

Author:	Jericho
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[steveharter]

The likely implementation will be to detect special characters and encode in a C# compatible manner.

[Tornhoof]

There are a lot of cases where this does not work properly, a few more examples

• [JsonPropertyName("12345")]
• emoticons (like 😀😁)
• "@@",
• Unicode Escaped characters (like "\ud83d\ude00\ud83d\ude01").

The assumption that you can simply take that value and append PropName to it only works for simply cases, it might really be simpler to generate the propertynames via some other mechanism than value+propName, i.e. "PropName"+counter or something similar, e.g PropName1, PropName2 and not rely at all on the value from the attribute.

[Marv51]

[JsonPropertyName("$schema")] has the same problem for me.

[steveharter]

Thanks for the samples. I'll create a PR soon. Currently the change is to detect whether the value in [JsonPropertyName(value)] is a valid C# identifier. If it is, is does essentially what it does today. If not, it encodes the string via byte[] = Encoding.UTF8.GetBytes(string) and then generates hex values using the returned byte[]. Note this only needs to be done for variable that contains the JsonEncodedText of the property name.

[eiriktsarpalis]

Currently the change is to detect whether the value in [JsonPropertyName(value)] is a valid C# identifier. If it is, is does essentially what it does today. If not, it encodes the string via byte[] = Encoding.UTF8.GetBytes(string) and then generates hex values using the returned byte[]

Currently, the generated code looks like this:

    internal partial class MyJsonContext
    {
        
        private static global::System.Text.Json.JsonEncodedText $schemaPropName = global::System.Text.Json.JsonEncodedText.Encode("$schema");
    }

So compilation is failing because $schemaPropName is not a valid C# identifier. I think we should simply avoid applying the value of JsonPropertyNameAttribute when generating identifiers, and always use the name of the C# property name:

    internal partial class MyJsonContext
    {
        
        private static global::System.Text.Json.JsonEncodedText SmtpIdPropName = global::System.Text.Json.JsonEncodedText.Encode("$schema");
    }

[steveharter]

I think we should simply avoid applying the value of JsonPropertyNameAttribute when generating identifiers, and always use the name of the C# property name:

Note today the JsonEncodedText properties are shared across all serializable types for performance.

Using the C# property name has issues:

• Naming conflicts may occur since the same C# property name (e.g. "SmtpId") on different C# types may have different JsonPropertyNames values (e.g. JsonPropertyName["smtp-id"], JsonPropertyName["SMTP-ID"]). To fix this, the full class name would need to be added to the variable.
• Minor perf issue since there can be several C# property names (e.g. "SmtpID", "SmtpId") that have the same JsonPropertyName["smtp-id"]. This means extra JsonEncodedText properties are unnecessary created.

The current approach introduces the least amount of risk as it makes minor changes to the code. However, usability of the such JsonEncodedText properties while stepping through code is not ideal since the variable name would be like: "EncodedPropName_536D74702D4964". However, since that variable maps 1:1 with the declaration:

private static global::System.Text.Json.JsonEncodedText EncodedPropName_536D74702D4964 = global::System.Text.Json.JsonEncodedText.Encode("Smtp-Id");

with "Smtp-Id" in clear text, that's not so bad.

Some thoughts for making changes v.next (non would be breaking; just re-gen):

• Perform a two-pass collision detection and include the full class name for duplicates. This also suffers a bit from performance, since extra JsonEncodedText properties will be created\instantiated.
• Append the first CLR property name like so: "EncodedPropName_SmtpId_536D74702D4964". May be confusing, however, in the rare case there are completely different CLR property names.
• Add XML doc to the JsonEncodedText property that lists the various (or first) C# property names used.
• ...

[Tornhoof]

What I noticed, is there a reason those generated values are not readonly?