From ad0e911a40ba35c929f5f50846fee8c8de994b90 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 14:27:02 +0000 Subject: [PATCH 1/2] Check a type once per write, so a nested expression body runs twice, not depth + 3 times [patch] ClassTemplate.WriteTo validated its whole subtree, and every nested type's WriteTo validated its own subtree again; AddSourceFile and AddClasses each validated all types once more. A property's ExpressionBodyFactory ran depth + 3 times, so a stateful factory wrote different code depending on how deeply its class sat. A template write now opens a scope on the CodeBlocker that records which types have been checked, by reference. A type already checked in the scope is not checked again, so the callback runs once to check and once to write. A different type that a callback writes is still checked. Fixes ktsu-dev/CodeBlocker#202 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW5YjyP4TWYsDuKLv95 --- CodeBlocker.Test/TemplateTests.cs | 80 +++++++++++++++++++++ CodeBlocker/CodeBlocker.cs | 71 ++++++++++++++++++ CodeBlocker/Templates/ClassTemplate.cs | 19 ++++- CodeBlocker/Templates/PropertyTemplate.cs | 3 +- CodeBlocker/Templates/SourceFileTemplate.cs | 8 ++- 5 files changed, 176 insertions(+), 5 deletions(-) diff --git a/CodeBlocker.Test/TemplateTests.cs b/CodeBlocker.Test/TemplateTests.cs index c703d78..6b8f9be 100644 --- a/CodeBlocker.Test/TemplateTests.cs +++ b/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)); + + StringAssert.Contains(alone, "public int First => 2;"); + StringAssert.Contains(inFile, "public int First => 2;"); + StringAssert.Contains(inFile, "public int Second => 3;"); + } + + [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(() => Render(outer)); + } + private static string RenderInClass(MemberTemplate member) { ClassTemplate type = new() { Name = "C", Members = { member } }; diff --git a/CodeBlocker/CodeBlocker.cs b/CodeBlocker/CodeBlocker.cs index 59006d8..2eda519 100644 --- a/CodeBlocker/CodeBlocker.cs +++ b/CodeBlocker/CodeBlocker.cs @@ -41,8 +41,79 @@ public class CodeBlocker : IDisposable /// private bool isMidLine; + /// + /// The types already checked by the template write under way, or when + /// none is under way. + /// + /// + /// 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. + /// + private HashSet? checkedTemplates; + + private int templateWriteDepth; + private IndentedTextWriter IndentedTextWriter { get; } + /// + /// 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. + /// + /// The scope to dispose when the write ends. + internal TemplateWriteScope BeginTemplateWrite() + { + checkedTemplates ??= new HashSet(ReferenceComparer.Instance); + templateWriteDepth++; + return new TemplateWriteScope(this); + } + + /// + /// Records that has been checked in the template write under way. + /// Does nothing outside one. + /// + /// The checked template. + internal void MarkChecked(object template) => checkedTemplates?.Add(template); + + /// + /// Gets whether has already been checked in the template write + /// under way. + /// + /// The template. + /// when it has been checked and need not be again. + internal bool IsChecked(object template) => checkedTemplates?.Contains(template) == true; + + private void EndTemplateWrite() + { + if (--templateWriteDepth == 0) + { + checkedTemplates = null; + } + } + + /// + /// Compares by reference, so a template type that overrides + /// is still told apart from an equal copy. (ReferenceEqualityComparer is not available on + /// .NET Standard.) + /// + private sealed class ReferenceComparer : IEqualityComparer + { + 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); + } + + /// Ends a template write started by . + internal readonly struct TemplateWriteScope(CodeBlocker owner) : IDisposable + { + /// + public void Dispose() => owner.EndTemplateWrite(); + } + /// /// Get the current indent string being used. /// diff --git a/CodeBlocker/Templates/ClassTemplate.cs b/CodeBlocker/Templates/ClassTemplate.cs index 30289d6..67afd84 100644 --- a/CodeBlocker/Templates/ClassTemplate.cs +++ b/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); @@ -124,8 +132,13 @@ internal override void Validate(CodeBlocker codeBlocker) foreach (ClassTemplate nestedClass in NestedClasses) { - nestedClass.Validate(codeBlocker); + if (!codeBlocker.IsChecked(nestedClass)) + { + nestedClass.Validate(codeBlocker); + } } + + codeBlocker.MarkChecked(this); } /// diff --git a/CodeBlocker/Templates/PropertyTemplate.cs b/CodeBlocker/Templates/PropertyTemplate.cs index 9db18f0..06d3337 100644 --- a/CodeBlocker/Templates/PropertyTemplate.cs +++ b/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. /// internal override void Validate(CodeBlocker codeBlocker) { diff --git a/CodeBlocker/Templates/SourceFileTemplate.cs b/CodeBlocker/Templates/SourceFileTemplate.cs index ddaf1ea..e210b5e 100644 --- a/CodeBlocker/Templates/SourceFileTemplate.cs +++ b/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 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 Date: Wed, 11 Nov 2026 14:39:29 +0000 Subject: [PATCH 2/2] Use Assert.Contains and a Where filter, as SonarCloud asks Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW5YjyP4TWYsDuKLv95 --- CodeBlocker.Test/TemplateTests.cs | 6 +++--- CodeBlocker/Templates/ClassTemplate.cs | 7 ++----- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/CodeBlocker.Test/TemplateTests.cs b/CodeBlocker.Test/TemplateTests.cs index 6b8f9be..78e5f4c 100644 --- a/CodeBlocker.Test/TemplateTests.cs +++ b/CodeBlocker.Test/TemplateTests.cs @@ -2234,9 +2234,9 @@ public void AStatefulExpressionBodyWritesTheSameCodeWhereverItsClassIsNested() SourceFileTemplate file = new() { Classes = { new ClassTemplate { Name = "Outer", NestedClasses = { nested } } } }; string inFile = Render(codeBlocker => codeBlocker.AddSourceFile(file)); - StringAssert.Contains(alone, "public int First => 2;"); - StringAssert.Contains(inFile, "public int First => 2;"); - StringAssert.Contains(inFile, "public int Second => 3;"); + Assert.Contains("public int First => 2;", alone); + Assert.Contains("public int First => 2;", inFile); + Assert.Contains("public int Second => 3;", inFile); } [TestMethod] diff --git a/CodeBlocker/Templates/ClassTemplate.cs b/CodeBlocker/Templates/ClassTemplate.cs index 67afd84..5b890fd 100644 --- a/CodeBlocker/Templates/ClassTemplate.cs +++ b/CodeBlocker/Templates/ClassTemplate.cs @@ -130,12 +130,9 @@ internal override void Validate(CodeBlocker codeBlocker) member.Validate(codeBlocker); } - foreach (ClassTemplate nestedClass in NestedClasses) + foreach (ClassTemplate nestedClass in NestedClasses.Where(nested => !codeBlocker.IsChecked(nested))) { - if (!codeBlocker.IsChecked(nestedClass)) - { - nestedClass.Validate(codeBlocker); - } + nestedClass.Validate(codeBlocker); } codeBlocker.MarkChecked(this);