Skip to content

Commit 6fc4590

Browse files
authored
Merge pull request #1103 from EdwardCooke/ec-aotfix
Add a parse method wrapper and caching to fix AoT compilation +semver:breaking
2 parents ba6dd7d + b2a588a commit 6fc4590

13 files changed

Lines changed: 190 additions & 72 deletions

File tree

YamlDotNet.Analyzers.StaticGenerator/StaticTypeInspectorFile.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,45 @@ public override void Write(SerializableSyntaxReceiver syntaxReceiver)
155155
Write("return value.ToString();");
156156
UnIndent(); Write("}");
157157

158+
Write("public bool HasParseMethod(Type type)");
159+
Write("{"); Indent();
160+
foreach (var o in syntaxReceiver.Classes)
161+
{
162+
var classObject = o.Value;
163+
if (classObject.ModuleSymbol.GetMembers("Parse").OfType<IMethodSymbol>().Any(m => m.IsStatic && m.Parameters.Length == 1 && m.Parameters[0].Type.SpecialType == SpecialType.System_String))
164+
{
165+
Write($"if (type == typeof({classObject.ModuleSymbol.GetFullName().Replace("?", string.Empty)}))");
166+
Write("{"); Indent();
167+
Write("return true;");
168+
UnIndent(); Write("}");
169+
}
170+
}
171+
Write("return false;");
172+
UnIndent(); Write("}");
173+
174+
Write("public object Parse(string value, Type expectedType)");
175+
Write("{"); Indent();
176+
foreach (var o in syntaxReceiver.Classes)
177+
{
178+
var classObject = o.Value;
179+
if (classObject.ModuleSymbol
180+
.GetMembers("Parse")
181+
.OfType<IMethodSymbol>()
182+
.Any(m => m.DeclaredAccessibility == Accessibility.Public &&
183+
m.IsStatic &&
184+
m.Parameters.Length == 1 &&
185+
m.Parameters[0].Type.SpecialType == SpecialType.System_String &&
186+
SymbolEqualityComparer.Default.Equals(m.ReturnType, classObject.ModuleSymbol)))
187+
{
188+
Write($"if (expectedType == typeof({classObject.ModuleSymbol.GetFullName().Replace("?", string.Empty)}))");
189+
Write("{"); Indent();
190+
Write($"return {classObject.ModuleSymbol.GetFullName().Replace("?", string.Empty)}.Parse(value);");
191+
UnIndent(); Write("}");
192+
}
193+
}
194+
Write("throw new InvalidOperationException($\"Type '{expectedType.FullName}' does not have a static Parse method.\");");
195+
UnIndent(); Write("}");
196+
158197
UnIndent(); Write("}");
159198
}
160199

YamlDotNet.Core7AoTCompileTest/Program.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
using YamlDotNet.Core7AoTCompileTest.Model;
3333
using YamlDotNet.Serialization;
3434
using YamlDotNet.Serialization.Callbacks;
35+
using YamlDotNet.Serialization.NamingConventions;
3536

3637
string yaml = string.Create(CultureInfo.InvariantCulture, $@"MyBool: true
3738
hi: 1
@@ -106,6 +107,7 @@
106107

107108
var aotContext = new YamlDotNet.Core7AoTCompileTest.StaticContext();
108109
var deserializer = new StaticDeserializerBuilder(aotContext)
110+
.WithNamingConvention(UnderscoredNamingConvention.Instance)
109111
.Build();
110112

111113
var x = deserializer.Deserialize<PrimitiveTypes>(input);
@@ -189,16 +191,17 @@
189191
Console.WriteLine("Serialized:");
190192

191193
var serializer = new StaticSerializerBuilder(aotContext)
194+
.WithNamingConvention(UnderscoredNamingConvention.Instance)
192195
.Build();
193196

194197
var output = serializer.Serialize(x);
195198
Console.WriteLine(output);
196199
Console.WriteLine("============== Done with the primary object");
197200

198-
yaml = @"- myArray:
201+
yaml = @"- my_array:
199202
- 1
200203
- 2
201-
- myArray:
204+
- my_array:
202205
- 3
203206
- 4
204207
";

YamlDotNet.Test/Serialization/MergingParserTests.cs

Lines changed: 75 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -162,53 +162,60 @@ public async Task MergingParserWithMergeKeyBomb_ShouldThrowExceptionWhenTooManyE
162162
// Timebox this test to avoid infinite loops in case of bugs.
163163
// 30 seconds should be more than enough for this test to run even on a slow machine, and if it takes longer than that,
164164
// it's likely that the merging parser is not correctly counting events and enforcing the limit.
165-
var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(30));
165+
var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(60));
166166
cancellationTokenSource.Token.Register(() =>
167167
{
168168
throw new TimeoutException("The test took too long, likely due to an infinite loop in the merging parser.");
169169
});
170170

171-
await Task.Run(() =>
171+
try
172172
{
173-
var sb = new StringBuilder();
174-
175-
// Base anchor
176-
sb.AppendLine("a0: &a0");
177-
sb.AppendLine(" x: 1");
178-
sb.AppendLine();
179-
180-
// Each level merges the previous anchor TWICE (fanout=2), doubling event count
181-
for (int i = 1; i <= 25; i++)
173+
await Task.Run(() =>
182174
{
183-
sb.AppendLine($"a{i}: &a{i}");
184-
sb.AppendLine($" <<: *a{i - 1}"); // first merge
185-
sb.AppendLine($" <<: *a{i - 1}"); // second merge
175+
var sb = new StringBuilder();
176+
177+
// Base anchor
178+
sb.AppendLine("a0: &a0");
179+
sb.AppendLine(" x: 1");
186180
sb.AppendLine();
187-
}
188181

189-
sb.AppendLine("final:");
190-
sb.AppendLine(" <<: *a25");
182+
// Each level merges the previous anchor TWICE (fanout=2), doubling event count
183+
for (int i = 1; i <= 25; i++)
184+
{
185+
sb.AppendLine($"a{i}: &a{i}");
186+
sb.AppendLine($" <<: *a{i - 1}"); // first merge
187+
sb.AppendLine($" <<: *a{i - 1}"); // second merge
188+
sb.AppendLine();
189+
}
191190

192-
var yaml = sb.ToString();
193-
var parser = new Parser(new StringReader(yaml));
194-
var mergingParser = new MergingParser(parser, 1000);
195-
try
196-
{
197-
while (mergingParser.MoveNext())
191+
sb.AppendLine("final:");
192+
sb.AppendLine(" <<: *a25");
193+
194+
var yaml = sb.ToString();
195+
var parser = new Parser(new StringReader(yaml));
196+
var mergingParser = new MergingParser(parser, 1000);
197+
try
198198
{
199-
//move through everything, we're in a timebox so if this takes too long, the cancellation token will trigger and fail the test
199+
while (mergingParser.MoveNext())
200+
{
201+
//move through everything, we're in a timebox so if this takes too long, the cancellation token will trigger and fail the test
202+
}
200203
}
201-
}
202-
catch (YamlException ex) when (ex.Message.Contains("Too many parsing events"))
203-
{
204-
// Expected exception, test passes
205-
return;
206-
}
207-
catch (Exception ex)
208-
{
209-
throw new Exception($"Unexpected exception", ex);
210-
}
211-
}, cancellationTokenSource.Token);
204+
catch (YamlException ex) when (ex.Message.Contains("Too many parsing events"))
205+
{
206+
// Expected exception, test passes
207+
return;
208+
}
209+
catch (Exception ex)
210+
{
211+
throw new Exception($"Unexpected exception", ex);
212+
}
213+
}, cancellationTokenSource.Token);
214+
}
215+
catch (TimeoutException ex)
216+
{
217+
Assert.Fail($"Test failed due to timeout: {ex.Message}");
218+
}
212219
}
213220

214221
[Fact]
@@ -217,42 +224,51 @@ public async Task MergingParserWithManySmallMerges_ShouldThrowExceptionWhenCumul
217224
// Timebox this test to avoid infinite loops in case of bugs.
218225
// 30 seconds should be more than enough for this test to run even on a slow machine, and if it takes longer than that,
219226
// it's likely that the merging parser is not correctly counting events and enforcing the limit.
220-
var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(30));
227+
var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(60));
221228
cancellationTokenSource.Token.Register(() =>
222229
{
223230
throw new TimeoutException("The test took too long, likely due to an infinite loop in the merging parser.");
224231
});
225232

226-
await Task.Run(() =>
233+
try
227234
{
228-
var sb = new StringBuilder();
229-
sb.AppendLine("base: &base");
230-
for (var i = 0; i < 25; i++)
235+
await Task.Run(() =>
231236
{
232-
sb.AppendLine($" k{i}: v{i}");
233-
}
237+
var sb = new StringBuilder();
238+
sb.AppendLine("base: &base");
239+
for (var i = 0; i < 25; i++)
240+
{
241+
sb.AppendLine($" k{i}: v{i}");
242+
}
234243

235-
sb.AppendLine();
236-
for (var i = 0; i < 35; i++)
237-
{
238-
sb.AppendLine($"entry{i}:");
239-
sb.AppendLine(" <<: *base");
240244
sb.AppendLine();
241-
}
242-
243-
var parser = new Parser(new StringReader(sb.ToString()));
244-
var mergingParser = new MergingParser(parser, 1000);
245-
246-
Action parse = () =>
247-
{
248-
while (mergingParser.MoveNext())
245+
for (var i = 0; i < 35; i++)
249246
{
247+
sb.AppendLine($"entry{i}:");
248+
sb.AppendLine(" <<: *base");
249+
sb.AppendLine();
250250
}
251-
};
252251

253-
parse.Should().Throw<YamlException>()
254-
.Where(ex => ex.Message.Contains("Too many parsing events"));
255-
}, cancellationTokenSource.Token);
252+
var parser = new Parser(new StringReader(sb.ToString()));
253+
var mergingParser = new MergingParser(parser, 1000);
254+
var totalParsedEvents = 0;
255+
Action parse = () =>
256+
{
257+
while (mergingParser.MoveNext())
258+
{
259+
totalParsedEvents++;
260+
}
261+
};
262+
263+
parse.Should().Throw<YamlException>()
264+
.Where(ex => ex.Message.Contains("Too many parsing events"));
265+
Console.WriteLine($"Total parsed events before exception: {totalParsedEvents}");
266+
}, cancellationTokenSource.Token);
267+
}
268+
catch (TimeoutException ex)
269+
{
270+
Assert.Fail($"Test failed due to timeout: {ex.Message}");
271+
}
256272
}
257273

258274
[Fact]

YamlDotNet/Serialization/ITypeInspector.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,19 @@ public interface ITypeInspector
6666
/// <param name="enumValue"></param>
6767
/// <returns></returns>
6868
string GetEnumValue(object enumValue);
69+
70+
/// <summary>
71+
/// Indicates whether the specified type has a static Parse method that can be used to convert a string to an instance of that type.
72+
/// </summary>
73+
/// <param name="type">The type to check for a static Parse method.</param>
74+
/// <returns>True if the type has a static Parse method; otherwise, false.</returns>
75+
bool HasParseMethod(Type type);
76+
77+
/// <summary> Converts a string to an instance of the specified type using a static Parse method.
78+
/// </summary>
79+
/// <param name="value">The string value to convert.</param>
80+
/// <param name="expectedType">The type to convert the string to.</param>
81+
/// <returns>An instance of the specified type.</returns>
82+
object? Parse(string value, Type expectedType);
6983
}
7084
}

YamlDotNet/Serialization/NodeDeserializers/ScalarNodeDeserializer.cs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -147,17 +147,9 @@ public bool Deserialize(IParser parser, Type expectedType, Func<IParser, Type, o
147147
// TimeSpan, DateTimeOffset, Guid, IPAddress, and others.
148148
// This is especially important for the static/AOT deserialization
149149
// path where the typeConverter is a NullTypeConverter.
150-
var parseMethod = underlyingType.GetPublicStaticMethod("Parse", typeof(string), typeof(IFormatProvider));
151-
if (parseMethod != null)
150+
if (typeInspector.HasParseMethod(underlyingType))
152151
{
153-
try
154-
{
155-
value = parseMethod.Invoke(null, new object[] { scalar.Value, CultureInfo.InvariantCulture });
156-
}
157-
catch (System.Reflection.TargetInvocationException ex) when (ex.InnerException != null)
158-
{
159-
throw ex.InnerException;
160-
}
152+
value = typeInspector.Parse(scalar.Value, underlyingType);
161153
}
162154
else
163155
{

YamlDotNet/Serialization/TypeInspectors/CachedTypeInspector.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public class CachedTypeInspector : TypeInspectorSkeleton
3636
private readonly ConcurrentDictionary<Type, List<IPropertyDescriptor>> cache = new ConcurrentDictionary<Type, List<IPropertyDescriptor>>();
3737
private readonly ConcurrentDictionary<Type, ConcurrentDictionary<string, string>> enumNameCache = new ConcurrentDictionary<Type, ConcurrentDictionary<string, string>>();
3838
private readonly ConcurrentDictionary<object, string> enumValueCache = new ConcurrentDictionary<object, string>();
39+
private readonly ConcurrentDictionary<Type, bool> hasParseMethodCache = new ConcurrentDictionary<Type, bool>();
3940

4041
public CachedTypeInspector(ITypeInspector innerTypeDescriptor)
4142
{
@@ -74,5 +75,13 @@ public override IEnumerable<IPropertyDescriptor> GetProperties(Type type, object
7475
},
7576
(container, innerTypeDescriptor));
7677
}
78+
79+
public override bool HasParseMethod(Type type)
80+
{
81+
return hasParseMethodCache.GetOrAdd(type, static (t, typeDescriptor) =>
82+
typeDescriptor.HasParseMethod(t), innerTypeDescriptor);
83+
}
84+
85+
public override object? Parse(string value, Type expectedType) => innerTypeDescriptor.Parse(value, expectedType);
7786
}
7887
}

YamlDotNet/Serialization/TypeInspectors/CompositeTypeInspector.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,5 +86,22 @@ public override IEnumerable<IPropertyDescriptor> GetProperties(Type type, object
8686
return typeInspectors
8787
.SelectMany(i => i.GetProperties(type, container));
8888
}
89+
90+
public override bool HasParseMethod(Type type)
91+
{
92+
return typeInspectors.Any(i => i.HasParseMethod(type));
93+
}
94+
95+
public override object? Parse(string value, Type expectedType)
96+
{
97+
var parsableInspector = typeInspectors.FirstOrDefault(i => i.HasParseMethod(expectedType));
98+
99+
if (parsableInspector == null)
100+
{
101+
throw new ArgumentOutOfRangeException(nameof(expectedType), $"No inspector with a Parse method for type {expectedType.FullName} was found.");
102+
}
103+
104+
return parsableInspector.Parse(value, expectedType);
105+
}
89106
}
90107
}

YamlDotNet/Serialization/TypeInspectors/NamingConventionTypeInspector.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,5 +57,9 @@ public override IEnumerable<IPropertyDescriptor> GetProperties(Type type, object
5757
return new PropertyDescriptor(p) { Name = namingConvention.Apply(p.Name) };
5858
});
5959
}
60+
61+
public override bool HasParseMethod(Type type) => this.innerTypeDescriptor.HasParseMethod(type);
62+
63+
public override object? Parse(string value, Type expectedType) => this.innerTypeDescriptor.Parse(value, expectedType);
6064
}
6165
}

YamlDotNet/Serialization/TypeInspectors/ReadableAndWritablePropertiesTypeInspector.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,9 @@ public override IEnumerable<IPropertyDescriptor> GetProperties(Type type, object
4646
return innerTypeDescriptor.GetProperties(type, container)
4747
.Where(p => p.CanWrite);
4848
}
49+
50+
public override bool HasParseMethod(Type type) => this.innerTypeDescriptor.HasParseMethod(type);
51+
52+
public override object? Parse(string value, Type expectedType) => this.innerTypeDescriptor.Parse(value, expectedType);
4953
}
5054
}

YamlDotNet/Serialization/TypeInspectors/ReflectionTypeInspector.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,5 +71,18 @@ public override string GetEnumValue(object enumValue)
7171
return result!;
7272
}
7373

74+
public override bool HasParseMethod(Type type) => type.GetMethod("Parse", BindingFlags.Public | BindingFlags.Static, null, [typeof(string)], null) != null;
75+
76+
public override object? Parse(string value, Type expectedType)
77+
{
78+
var method = expectedType.GetMethod("Parse", [typeof(string)]);
79+
80+
if (method == null)
81+
{
82+
throw new InvalidOperationException($"Type '{expectedType.FullName}' does not have a static Parse method.");
83+
}
84+
85+
return method.Invoke(null, new object[] { value });
86+
}
7487
}
7588
}

0 commit comments

Comments
 (0)