From 91ccd4bd418c25294270be60968afb8c543641a7 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Wed, 25 Oct 2023 13:12:10 +0300 Subject: [PATCH] Improve property name escaping * don't allow "init", "fromJS" or "toJSON" for TypeScript member name * improve performance by not doing string.Replace unless necessary * update xUnit --- .../NJsonSchema.Benchmark.csproj | 6 +- .../InheritanceTests.cs | 2 +- ...nSchema.CodeGeneration.CSharp.Tests.csproj | 6 +- .../CSharpPropertyNameGenerator.cs | 38 ++++++++---- .../InheritanceSerializationTests.cs | 4 +- .../NJsonSchema.CodeGeneration.Tests.csproj | 9 ++- .../Samples/SampleTests.cs | 4 +- .../InheritanceTests.cs | 2 +- ...ema.CodeGeneration.TypeScript.Tests.csproj | 6 +- .../PropertyNameTests.cs | 32 ++++++++++ ...d_properties_they_are_escaped.verified.txt | 58 +++++++++++++++++++ .../TypeScriptPropertyNameGenerator.cs | 37 +++++++----- .../NJsonSchema.NewtonsoftJson.Tests.csproj | 6 +- .../NJsonSchema.Tests.csproj | 6 +- .../NJsonSchema.Yaml.Tests.csproj | 6 +- .../Generation/SampleJsonDataGenerator.cs | 12 ++-- 16 files changed, 176 insertions(+), 58 deletions(-) create mode 100644 src/NJsonSchema.CodeGeneration.TypeScript.Tests/PropertyNameTests.cs create mode 100644 src/NJsonSchema.CodeGeneration.TypeScript.Tests/Snapshots/PropertyNameTests.When_class_has_restricted_properties_they_are_escaped.verified.txt diff --git a/src/NJsonSchema.Benchmark/NJsonSchema.Benchmark.csproj b/src/NJsonSchema.Benchmark/NJsonSchema.Benchmark.csproj index dcd071859..100e99e4e 100644 --- a/src/NJsonSchema.Benchmark/NJsonSchema.Benchmark.csproj +++ b/src/NJsonSchema.Benchmark/NJsonSchema.Benchmark.csproj @@ -14,11 +14,11 @@ - + - - + + diff --git a/src/NJsonSchema.CodeGeneration.CSharp.Tests/InheritanceTests.cs b/src/NJsonSchema.CodeGeneration.CSharp.Tests/InheritanceTests.cs index 6d3118ca1..ac9c4e67e 100644 --- a/src/NJsonSchema.CodeGeneration.CSharp.Tests/InheritanceTests.cs +++ b/src/NJsonSchema.CodeGeneration.CSharp.Tests/InheritanceTests.cs @@ -41,7 +41,7 @@ public async Task When_empty_class_inherits_from_dictionary_then_allOf_inheritan //// Assert var dictionarySchema = schema.Definitions["EmptyClassInheritingDictionary"]; - Assert.Equal(0, dictionarySchema.AllOf.Count); + Assert.Empty(dictionarySchema.AllOf); Assert.True(dictionarySchema.IsDictionary); Assert.Contains("Foobar.", data); Assert.Contains("Foobar.", code); diff --git a/src/NJsonSchema.CodeGeneration.CSharp.Tests/NJsonSchema.CodeGeneration.CSharp.Tests.csproj b/src/NJsonSchema.CodeGeneration.CSharp.Tests/NJsonSchema.CodeGeneration.CSharp.Tests.csproj index 91e7723c3..6226a3045 100644 --- a/src/NJsonSchema.CodeGeneration.CSharp.Tests/NJsonSchema.CodeGeneration.CSharp.Tests.csproj +++ b/src/NJsonSchema.CodeGeneration.CSharp.Tests/NJsonSchema.CodeGeneration.CSharp.Tests.csproj @@ -13,9 +13,9 @@ - - - + + + diff --git a/src/NJsonSchema.CodeGeneration.CSharp/CSharpPropertyNameGenerator.cs b/src/NJsonSchema.CodeGeneration.CSharp/CSharpPropertyNameGenerator.cs index cc9cbad8b..2c61c752f 100644 --- a/src/NJsonSchema.CodeGeneration.CSharp/CSharpPropertyNameGenerator.cs +++ b/src/NJsonSchema.CodeGeneration.CSharp/CSharpPropertyNameGenerator.cs @@ -9,15 +9,21 @@ namespace NJsonSchema.CodeGeneration.CSharp { /// Generates the property name for a given CSharp . - public class CSharpPropertyNameGenerator : IPropertyNameGenerator + public sealed class CSharpPropertyNameGenerator : IPropertyNameGenerator { + private static readonly char[] _reservedFirstPassChars = { '"', '\'', '@', '?', '!', '$', '[', ']', '(', ')', '.', '=', '+' }; + private static readonly char[] _reservedSecondPassChars = { '*', ':', '-', '#', '&' }; + /// Generates the property name. /// The property. /// The new name. - public virtual string Generate(JsonSchemaProperty property) + public string Generate(JsonSchemaProperty property) { - return ConversionUtilities.ConvertToUpperCamelCase(property.Name - .Replace("\"", string.Empty) + var name = property.Name; + + if (name.IndexOfAny(_reservedFirstPassChars) != -1) + { + name = name.Replace("\"", string.Empty) .Replace("'", string.Empty) .Replace("@", string.Empty) .Replace("?", string.Empty) @@ -29,12 +35,22 @@ public virtual string Generate(JsonSchemaProperty property) .Replace(")", string.Empty) .Replace(".", "-") .Replace("=", "-") - .Replace("+", "plus"), true) - .Replace("*", "Star") - .Replace(":", "_") - .Replace("-", "_") - .Replace("#", "_") - .Replace("&", "And"); + .Replace("+", "plus"); + } + + name = ConversionUtilities.ConvertToUpperCamelCase(name, true); + + if (name.IndexOfAny(_reservedSecondPassChars) != -1) + { + name = name + .Replace("*", "Star") + .Replace(":", "_") + .Replace("-", "_") + .Replace("#", "_") + .Replace("&", "And"); + } + + return name; } } -} +} \ No newline at end of file diff --git a/src/NJsonSchema.CodeGeneration.Tests/InheritanceSerializationTests.cs b/src/NJsonSchema.CodeGeneration.Tests/InheritanceSerializationTests.cs index 13e7e0c32..f93750e43 100644 --- a/src/NJsonSchema.CodeGeneration.Tests/InheritanceSerializationTests.cs +++ b/src/NJsonSchema.CodeGeneration.Tests/InheritanceSerializationTests.cs @@ -183,7 +183,7 @@ public async Task When_dates_are_converted_then_JsonInheritanceConverter_should_ } [Fact] - public void JsonInheritanceConverter_is_thread_safe() + public async Task JsonInheritanceConverter_is_thread_safe() { //// Arrange var tasks = new List(); @@ -196,7 +196,7 @@ public void JsonInheritanceConverter_is_thread_safe() } //// Act - Task.WaitAll(tasks.ToArray()); + await Task.WhenAll(tasks.ToArray()); //// Assert // No exceptions diff --git a/src/NJsonSchema.CodeGeneration.Tests/NJsonSchema.CodeGeneration.Tests.csproj b/src/NJsonSchema.CodeGeneration.Tests/NJsonSchema.CodeGeneration.Tests.csproj index a688110fd..d6499e3c6 100644 --- a/src/NJsonSchema.CodeGeneration.Tests/NJsonSchema.CodeGeneration.Tests.csproj +++ b/src/NJsonSchema.CodeGeneration.Tests/NJsonSchema.CodeGeneration.Tests.csproj @@ -10,9 +10,12 @@ - - - + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + diff --git a/src/NJsonSchema.CodeGeneration.Tests/Samples/SampleTests.cs b/src/NJsonSchema.CodeGeneration.Tests/Samples/SampleTests.cs index 24cb2810e..bca904162 100644 --- a/src/NJsonSchema.CodeGeneration.Tests/Samples/SampleTests.cs +++ b/src/NJsonSchema.CodeGeneration.Tests/Samples/SampleTests.cs @@ -101,7 +101,7 @@ public async Task When_JSON_contains_DateTime_is_available_then_string_validator var errors = schema.Validate(dataJson); //// Assert - Assert.Equal(0, errors.Count); + Assert.Empty(errors); } [Fact] @@ -135,7 +135,7 @@ public async Task When_JSON_contains_DateTime_is_available_then_JObject_validato var errors = schema.Validate(data); //// Assert - Assert.Equal(0, errors.Count); + Assert.Empty(errors); } } } diff --git a/src/NJsonSchema.CodeGeneration.TypeScript.Tests/InheritanceTests.cs b/src/NJsonSchema.CodeGeneration.TypeScript.Tests/InheritanceTests.cs index 531ffde21..63cc0ef12 100644 --- a/src/NJsonSchema.CodeGeneration.TypeScript.Tests/InheritanceTests.cs +++ b/src/NJsonSchema.CodeGeneration.TypeScript.Tests/InheritanceTests.cs @@ -51,7 +51,7 @@ public async Task When_empty_class_inherits_from_dictionary_then_allOf_inheritan //// Assert var dschema = schema.Definitions["EmptyClassInheritingDictionary"]; - Assert.Equal(0, dschema.AllOf.Count); + Assert.Empty(dschema.AllOf); Assert.True(dschema.IsDictionary); if (inlineNamedDictionaries) diff --git a/src/NJsonSchema.CodeGeneration.TypeScript.Tests/NJsonSchema.CodeGeneration.TypeScript.Tests.csproj b/src/NJsonSchema.CodeGeneration.TypeScript.Tests/NJsonSchema.CodeGeneration.TypeScript.Tests.csproj index 7deab23d1..68c0d8a7f 100644 --- a/src/NJsonSchema.CodeGeneration.TypeScript.Tests/NJsonSchema.CodeGeneration.TypeScript.Tests.csproj +++ b/src/NJsonSchema.CodeGeneration.TypeScript.Tests/NJsonSchema.CodeGeneration.TypeScript.Tests.csproj @@ -11,10 +11,10 @@ - + - - + + diff --git a/src/NJsonSchema.CodeGeneration.TypeScript.Tests/PropertyNameTests.cs b/src/NJsonSchema.CodeGeneration.TypeScript.Tests/PropertyNameTests.cs new file mode 100644 index 000000000..5a3199fdf --- /dev/null +++ b/src/NJsonSchema.CodeGeneration.TypeScript.Tests/PropertyNameTests.cs @@ -0,0 +1,32 @@ +using System.Threading.Tasks; +using NJsonSchema.Annotations; +using NJsonSchema.NewtonsoftJson.Generation; +using VerifyXunit; +using Xunit; + +using static NJsonSchema.CodeGeneration.TypeScript.Tests.VerifyHelper; + +namespace NJsonSchema.CodeGeneration.TypeScript.Tests; + +[UsesVerify] +public class PropertyNameTests +{ + private class TypeWithRestrictedProperties + { + public string Constructor { get; set; } + public string Init { get; set; } + public string FromJS { get; set; } + public string ToJSON { get; set; } + } + + [Fact] + public async Task When_class_has_restricted_properties_they_are_escaped() + { + var schema = NewtonsoftJsonSchemaGenerator.FromType(); + + var generator = new TypeScriptGenerator(schema, new TypeScriptGeneratorSettings { TypeScriptVersion = 4.3m }); + var output = generator.GenerateFile(nameof(TypeWithRestrictedProperties)); + + await Verify(output); + } +} \ No newline at end of file diff --git a/src/NJsonSchema.CodeGeneration.TypeScript.Tests/Snapshots/PropertyNameTests.When_class_has_restricted_properties_they_are_escaped.verified.txt b/src/NJsonSchema.CodeGeneration.TypeScript.Tests/Snapshots/PropertyNameTests.When_class_has_restricted_properties_they_are_escaped.verified.txt new file mode 100644 index 000000000..9a716687c --- /dev/null +++ b/src/NJsonSchema.CodeGeneration.TypeScript.Tests/Snapshots/PropertyNameTests.When_class_has_restricted_properties_they_are_escaped.verified.txt @@ -0,0 +1,58 @@ +//---------------------- +// +// +//---------------------- + + + + + + + +export class TypeWithRestrictedProperties implements ITypeWithRestrictedProperties { + constructor_!: string | undefined; + init_!: string | undefined; + fromJS_!: string | undefined; + toJSON_!: string | undefined; + + constructor(data?: ITypeWithRestrictedProperties) { + if (data) { + for (var property in data) { + if (data.hasOwnProperty(property)) + (this)[property] = (data)[property]; + } + } + } + + init(_data?: any) { + if (_data) { + this.constructor_ = _data["Constructor"]; + this.init_ = _data["Init"]; + this.fromJS_ = _data["FromJS"]; + this.toJSON_ = _data["ToJSON"]; + } + } + + static fromJS(data: any): TypeWithRestrictedProperties { + data = typeof data === 'object' ? data : {}; + let result = new TypeWithRestrictedProperties(); + result.init(data); + return result; + } + + toJSON(data?: any) { + data = typeof data === 'object' ? data : {}; + data["Constructor"] = this.constructor_; + data["Init"] = this.init_; + data["FromJS"] = this.fromJS_; + data["ToJSON"] = this.toJSON_; + return data; + } +} + +export interface ITypeWithRestrictedProperties { + constructor_: string | undefined; + init_: string | undefined; + fromJS_: string | undefined; + toJSON_: string | undefined; +} \ No newline at end of file diff --git a/src/NJsonSchema.CodeGeneration.TypeScript/TypeScriptPropertyNameGenerator.cs b/src/NJsonSchema.CodeGeneration.TypeScript/TypeScriptPropertyNameGenerator.cs index cf3203144..6f92c8e84 100644 --- a/src/NJsonSchema.CodeGeneration.TypeScript/TypeScriptPropertyNameGenerator.cs +++ b/src/NJsonSchema.CodeGeneration.TypeScript/TypeScriptPropertyNameGenerator.cs @@ -6,32 +6,43 @@ // Rico Suter, mail@rsuter.com //----------------------------------------------------------------------- +using System; using System.Collections.Generic; -using System.Linq; namespace NJsonSchema.CodeGeneration.TypeScript { /// Generates the property name for a given TypeScript . - public class TypeScriptPropertyNameGenerator : IPropertyNameGenerator + public sealed class TypeScriptPropertyNameGenerator : IPropertyNameGenerator { + private static readonly char[] _reservedFirstPassChars = { '"', '@', '?', '.', '=', '+' }; + private static readonly char[] _reservedSecondPassChars = { '*', ':', '-' }; + /// Gets or sets the reserved names. - public IEnumerable ReservedPropertyNames { get; set; } = new List { "constructor" }; + public HashSet ReservedPropertyNames { get; set; } = new(StringComparer.Ordinal) { "constructor", "init", "fromJS", "toJSON" }; - /// Generates the property name. - /// The property. - /// The new name. - public virtual string Generate(JsonSchemaProperty property) + /// + public string Generate(JsonSchemaProperty property) { - var name = ConversionUtilities.ConvertToLowerCamelCase(property.Name - .Replace("\"", string.Empty) + var name = property.Name; + + if (name.IndexOfAny(_reservedFirstPassChars) != -1) + { + name = name.Replace("\"", string.Empty) .Replace("@", string.Empty) .Replace("?", string.Empty) .Replace(".", "-") .Replace("=", "-") - .Replace("+", "plus"), true) - .Replace("*", "Star") - .Replace(":", "_") - .Replace("-", "_"); + .Replace("+", "plus"); + } + + name = ConversionUtilities.ConvertToLowerCamelCase(name, true); + + if (name.IndexOfAny(_reservedSecondPassChars) != -1) + { + name = name.Replace("*", "Star") + .Replace(":", "_") + .Replace("-", "_"); + } if (ReservedPropertyNames.Contains(name)) { diff --git a/src/NJsonSchema.NewtonsoftJson.Tests/NJsonSchema.NewtonsoftJson.Tests.csproj b/src/NJsonSchema.NewtonsoftJson.Tests/NJsonSchema.NewtonsoftJson.Tests.csproj index 822b53a07..a144bf63a 100644 --- a/src/NJsonSchema.NewtonsoftJson.Tests/NJsonSchema.NewtonsoftJson.Tests.csproj +++ b/src/NJsonSchema.NewtonsoftJson.Tests/NJsonSchema.NewtonsoftJson.Tests.csproj @@ -17,9 +17,9 @@ - - - + + + diff --git a/src/NJsonSchema.Tests/NJsonSchema.Tests.csproj b/src/NJsonSchema.Tests/NJsonSchema.Tests.csproj index 68985ba59..1f2253ed4 100644 --- a/src/NJsonSchema.Tests/NJsonSchema.Tests.csproj +++ b/src/NJsonSchema.Tests/NJsonSchema.Tests.csproj @@ -17,9 +17,9 @@ - - - + + + diff --git a/src/NJsonSchema.Yaml.Tests/NJsonSchema.Yaml.Tests.csproj b/src/NJsonSchema.Yaml.Tests/NJsonSchema.Yaml.Tests.csproj index 24f1b54ff..3a00579d0 100644 --- a/src/NJsonSchema.Yaml.Tests/NJsonSchema.Yaml.Tests.csproj +++ b/src/NJsonSchema.Yaml.Tests/NJsonSchema.Yaml.Tests.csproj @@ -8,10 +8,10 @@ - + - - + + diff --git a/src/NJsonSchema/Generation/SampleJsonDataGenerator.cs b/src/NJsonSchema/Generation/SampleJsonDataGenerator.cs index 48a2ec6cb..5b519e131 100644 --- a/src/NJsonSchema/Generation/SampleJsonDataGenerator.cs +++ b/src/NJsonSchema/Generation/SampleJsonDataGenerator.cs @@ -7,10 +7,8 @@ //----------------------------------------------------------------------- using Newtonsoft.Json.Linq; -using NJsonSchema; using System; using System.Collections.Generic; -using System.Globalization; using System.Linq; namespace NJsonSchema.Generation @@ -133,19 +131,19 @@ private JToken Generate(JsonSchema schema, Stack schemaStack) } } - private JToken HandleNumberType(JsonSchema schema) + private static JToken HandleNumberType(JsonSchema schema) { if (schema.ExclusiveMinimumRaw?.Equals(true) == true && schema.Minimum != null) { - return JToken.FromObject(decimal.Parse(schema.Minimum.Value.ToString(CultureInfo.InvariantCulture)) + 0.1m); + return JToken.FromObject(schema.Minimum.Value + 0.1m); } else if (schema.ExclusiveMinimum != null) { - return JToken.FromObject(decimal.Parse(schema.ExclusiveMinimum.Value.ToString(CultureInfo.InvariantCulture))); + return JToken.FromObject(schema.ExclusiveMinimum.Value); } else if (schema.Minimum.HasValue) { - return decimal.Parse(schema.Minimum.ToString()!); + return schema.Minimum.Value; } return JToken.FromObject(0.0); } @@ -167,7 +165,7 @@ private JToken HandleIntegerType(JsonSchema schema) return JToken.FromObject(0); } - private JToken HandleStringType(JsonSchema schema, JsonSchemaProperty? property) + private static JToken HandleStringType(JsonSchema schema, JsonSchemaProperty? property) { if (schema.Format == JsonFormatStrings.Date) {