Check a type once per write, so a nested expression body runs twice, not depth + 3 times [patch]

CodeBlocker.Test/TemplateTests.cs

@@ -2181,6 +2181,86 @@ public void EveryTemplateRejectsANullCodeBlocker()
 
 	#region Verbatim string literals in bodies
 
+	[TestMethod]
+	public void ANestedPropertysExpressionBodyRunsOnceToCheckAndOnceToWrite()
+	{
+		// ktsu-dev/CodeBlocker#202: every nesting level and AddClasses validated the subtree again.
+		int calls = 0;
+		PropertyTemplate property = new()
+		{
+			Type = "int",
+			Name = "Id",
+			Keywords = { "public" },
+			ExpressionBodyFactory = codeBlocker =>
+			{
+				calls++;
+				codeBlocker.Write("1");
+			},
+		};
+		ClassTemplate inner = new() { Name = "Inner", Members = { property } };
+		ClassTemplate mid = new() { Name = "Mid", NestedClasses = { inner } };
+		ClassTemplate outer = new() { Name = "Outer", NestedClasses = { mid } };
+		SourceFileTemplate file = new() { Classes = { outer } };
+
+		_ = Render(codeBlocker => codeBlocker.AddSourceFile(file));
+		Assert.AreEqual(2, calls, "AddSourceFile");
+
+		calls = 0;
+		_ = Render(outer);
+		Assert.AreEqual(2, calls, "ClassTemplate.WriteTo");
+
+		calls = 0;
+		_ = Render(codeBlocker => codeBlocker.AddClasses([outer]));
+		Assert.AreEqual(2, calls, "AddClasses");
+	}
+
+	[TestMethod]
+	public void AStatefulExpressionBodyWritesTheSameCodeWhereverItsClassIsNested()
+	{
+		int nextId = 0;
+		PropertyTemplate IdProperty(string name) => new()
+		{
+			Type = "int",
+			Name = name,
+			Keywords = { "public" },
+			ExpressionBodyFactory = codeBlocker => codeBlocker.Write($"{nextId++}"),
+		};
+
+		ClassTemplate standalone = new() { Name = "Ids", Members = { IdProperty("First"), IdProperty("Second") } };
+		string alone = Render(standalone);
+
+		nextId = 0;
+		ClassTemplate nested = new() { Name = "Ids", Members = { IdProperty("First"), IdProperty("Second") } };
+		SourceFileTemplate file = new() { Classes = { new ClassTemplate { Name = "Outer", NestedClasses = { nested } } } };
+		string inFile = Render(codeBlocker => codeBlocker.AddSourceFile(file));
+
+		Assert.Contains("public int First => 2;", alone);
+		Assert.Contains("public int First => 2;", inFile);
+		Assert.Contains("public int Second => 3;", inFile);
+	}
+
+	[TestMethod]
+	public void AClassWrittenFromInsideABodyFactoryIsStillChecked()
+	{
+		// Skipping a repeated check must not skip checking a different type a callback writes.
+		ClassTemplate invalid = new() { Name = "Bad", Members = { InvalidProperty() } };
+		ClassTemplate outer = new()
+		{
+			Name = "Outer",
+			Members =
+			{
+				new MethodTemplate
+				{
+					Name = "M",
+					Type = "void",
+					BodyFactory = codeBlocker => codeBlocker.AddClass(invalid),
+				},
+			},
+		};
+
+		_ = Assert.ThrowsExactly<InvalidOperationException>(() => Render(outer));
+	}
+
 	private static string RenderInClass(MemberTemplate member)
 	{
 		ClassTemplate type = new() { Name = "C", Members = { member } };

CodeBlocker/CodeBlocker.cs

@@ -41,8 +41,79 @@ public class CodeBlocker : IDisposable
 	/// </remarks>
 	private bool isMidLine;
 
+	/// <summary>
+	/// The types already checked by the template write under way, or <see langword="null"/> when
+	/// none is under way.
+	/// </summary>
+	/// <remarks>
+	/// A type is checked, members and nested types included, before anything of it is written. The
+	/// entry points that write several types check all of them first, and each nested type is
+	/// written through the same path. Without this, a subtree would be checked again at every level,
+	/// running an expression body's callback once per level (ktsu-dev/CodeBlocker#202). Types are
+	/// matched by reference, so a different type that a callback writes is still checked.
+	/// </remarks>
+	private HashSet<object>? checkedTemplates;
+
+	private int templateWriteDepth;
+
 	private IndentedTextWriter IndentedTextWriter { get; }
 
+	/// <summary>
+	/// Starts a template write, inside which a type checked once is not checked again. Dispose the
+	/// result when the write ends, whether or not it succeeds.
+	/// </summary>
+	/// <returns>The scope to dispose when the write ends.</returns>
+	internal TemplateWriteScope BeginTemplateWrite()
+	{
+		checkedTemplates ??= new HashSet<object>(ReferenceComparer.Instance);
+		templateWriteDepth++;
+		return new TemplateWriteScope(this);
+	}
+
+	/// <summary>
+	/// Records that <paramref name="template"/> has been checked in the template write under way.
+	/// Does nothing outside one.
+	/// </summary>
+	/// <param name="template">The checked template.</param>
+	internal void MarkChecked(object template) => checkedTemplates?.Add(template);
+
+	/// <summary>
+	/// Gets whether <paramref name="template"/> has already been checked in the template write
+	/// under way.
+	/// </summary>
+	/// <param name="template">The template.</param>
+	/// <returns><see langword="true"/> when it has been checked and need not be again.</returns>
+	internal bool IsChecked(object template) => checkedTemplates?.Contains(template) == true;
+
+	private void EndTemplateWrite()
+	{
+		if (--templateWriteDepth == 0)
+		{
+			checkedTemplates = null;
+		}
+	}
+
+	/// <summary>
+	/// Compares by reference, so a template type that overrides <see cref="object.Equals(object)"/>
+	/// is still told apart from an equal copy. (<c>ReferenceEqualityComparer</c> is not available on
+	/// .NET Standard.)
+	/// </summary>
+	private sealed class ReferenceComparer : IEqualityComparer<object>
+	{
+		internal static ReferenceComparer Instance { get; } = new();
+
+		public new bool Equals(object? x, object? y) => ReferenceEquals(x, y);
+
+		public int GetHashCode(object obj) => System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(obj);
+	}
+
+	/// <summary>Ends a template write started by <see cref="BeginTemplateWrite"/>.</summary>
+	internal readonly struct TemplateWriteScope(CodeBlocker owner) : IDisposable
+	{
+		/// <inheritdoc/>
+		public void Dispose() => owner.EndTemplateWrite();
+	}
+
 	/// <summary>
 	/// Get the current indent string being used.
 	/// </summary>

CodeBlocker/Templates/ClassTemplate.cs

@@ -72,10 +72,18 @@ public override void WriteTo(CodeBlocker codeBlocker)
 	{
 		Ensure.NotNull(codeBlocker);
 
+		using CodeBlocker.TemplateWriteScope scope = codeBlocker.BeginTemplateWrite();
+
 		// Everything is checked before the doc comment, attributes and modifiers are written, so a
 		// failing check cannot leave an unfinished declaration behind, nor a closed body that is
-		// silently missing the failing member and everything after it.
-		Validate(codeBlocker);
+		// silently missing the failing member and everything after it. A type already checked by an
+		// enclosing type or by AddClasses/AddSourceFile is not checked again, so an expression body's
+		// callback runs once to check it and once to write it, however deeply its type is nested.
+		if (!codeBlocker.IsChecked(this))
+		{
+			Validate(codeBlocker);
+		}
+
 		string keyword = TypeKindKeywords.For(Kind);
 		base.WriteTo(codeBlocker);
 
@@ -122,10 +130,12 @@ internal override void Validate(CodeBlocker codeBlocker)
 			member.Validate(codeBlocker);
 		}
 
-		foreach (ClassTemplate nestedClass in NestedClasses)
+		foreach (ClassTemplate nestedClass in NestedClasses.Where(nested => !codeBlocker.IsChecked(nested)))
 		{
 			nestedClass.Validate(codeBlocker);
 		}
+
+		codeBlocker.MarkChecked(this);
 	}
 
 	/// <summary>

CodeBlocker/Templates/PropertyTemplate.cs

@@ -90,7 +90,8 @@ public override void WriteTo(CodeBlocker codeBlocker)
 	/// The expression body, or an expression accessor's body, is rendered to check that it writes an
 	/// expression, so a class holding this property refuses it before writing its own header rather
 	/// than after the members ahead of it. That runs the callback once more when the property is
-	/// then written.
+	/// then written, and no more than that: a type is checked once per write, however deeply it is
+	/// nested.
 	/// </remarks>
 	internal override void Validate(CodeBlocker codeBlocker)
 	{

CodeBlocker/Templates/SourceFileTemplate.cs

@@ -57,6 +57,8 @@ public static CodeBlocker AddSourceFile(this CodeBlocker codeBlocker, SourceFile
 		Ensure.NotNull(codeBlocker);
 		Ensure.NotNull(template);
 
+		using CodeBlocker.TemplateWriteScope scope = codeBlocker.BeginTemplateWrite();
+
 		// Checked before the header is written, so a type that would be refused leaves nothing
 		// behind rather than a file holding only the types before it.
 		ValidateAll(codeBlocker, template.Classes);
@@ -102,6 +104,7 @@ public static CodeBlocker AddClasses(this CodeBlocker codeBlocker, IEnumerable<C
 		Ensure.NotNull(classes);
 
 		List<ClassTemplate> classList = [.. classes];
+		using CodeBlocker.TemplateWriteScope scope = codeBlocker.BeginTemplateWrite();
 		ValidateAll(codeBlocker, classList);
 
 		foreach (ClassTemplate classTemplate in classList)
@@ -118,7 +121,10 @@ private static void ValidateAll(CodeBlocker codeBlocker, IEnumerable<ClassTempla
 		foreach (ClassTemplate classTemplate in classes)
 		{
 			Ensure.NotNull(classTemplate);
-			classTemplate.Validate(codeBlocker);
+			if (!codeBlocker.IsChecked(classTemplate))
+			{
+				classTemplate.Validate(codeBlocker);
+			}
 		}
 	}
 }