From 7f03a34585f10227586a9f51abc35f693a6a4aa0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 06:28:58 +0000 Subject: [PATCH 1/2] Check a class's members before writing any of it [patch] ClassTemplate wrote its members one at a time inside a Scope, so a member that failed its check threw mid-body, and the Scope still wrote the closing brace on the way out. The buffer was left with a class that parses cleanly but silently lacks the failing member, everything sorted after it, and every nested class. AddSourceFile and AddClasses did the same one level up. Each template's up-front checks now live in an internal Validate(). ClassTemplate validates its kind, its members and its nested classes, recursively, before writing anything, and AddSourceFile and AddClasses validate every class before writing the header or the first type. The exception still names the offending member. Fixes #164 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW3PT52Zo3M7fFP8Z6J --- CodeBlocker.Test/TemplateTests.cs | 107 ++++++++++++++++++++ CodeBlocker/Templates/ClassTemplate.cs | 27 ++++- CodeBlocker/Templates/OperatorTemplate.cs | 19 ++-- CodeBlocker/Templates/PropertyTemplate.cs | 37 ++++--- CodeBlocker/Templates/SourceFileTemplate.cs | 27 ++++- CodeBlocker/Templates/TemplateBase.cs | 11 ++ 6 files changed, 200 insertions(+), 28 deletions(-) diff --git a/CodeBlocker.Test/TemplateTests.cs b/CodeBlocker.Test/TemplateTests.cs index cbe4b4a..48bbf26 100644 --- a/CodeBlocker.Test/TemplateTests.cs +++ b/CodeBlocker.Test/TemplateTests.cs @@ -916,6 +916,82 @@ public void AClassOfUnknownKindWritesNothing() Assert.AreEqual("int a;\n", codeBlocker.ToString()); } + private static PropertyTemplate InvalidProperty() => new() { Name = "P", Type = "int" }; + + [TestMethod] + public void AClassWithAnInvalidMemberWritesNothing() + { + ClassTemplate type = new() { Name = "C" }; + type.Members.Add(new FieldTemplate { Name = "a", Type = "int" }); + type.Members.Add(InvalidProperty()); + type.Members.Add(new FieldTemplate { Name = "b", Type = "int", Keywords = { "private" } }); + + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + + InvalidOperationException exception = + Assert.ThrowsExactly(() => codeBlocker.AddClass(type)); + StringAssert.Contains(exception.Message, "'P'"); + Assert.AreEqual(string.Empty, codeBlocker.ToString()); + } + + [TestMethod] + public void AnInvalidMemberOfANestedClassLeavesTheOuterClassUnwritten() + { + ClassTemplate inner = new() { Name = "Inner", Members = { InvalidProperty() } }; + ClassTemplate outer = new() + { + Name = "Outer", + Members = { new FieldTemplate { Name = "a", Type = "int" } }, + NestedClasses = { inner }, + }; + + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + codeBlocker.WriteLine("int x;"); + + Assert.ThrowsExactly(() => codeBlocker.AddClass(outer)); + Assert.AreEqual("int x;\n", codeBlocker.ToString()); + } + + [TestMethod] + public void ANestedClassOfUnknownKindLeavesTheOuterClassUnwritten() + { + ClassTemplate outer = new() { Name = "Outer", NestedClasses = { new ClassTemplate { Name = "Inner", Kind = (TypeKind)99 } } }; + + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + + Assert.ThrowsExactly(() => codeBlocker.AddClass(outer)); + Assert.AreEqual(string.Empty, codeBlocker.ToString()); + } + + [TestMethod] + public void AnOperatorOfUnknownKindLeavesItsClassUnwritten() + { + ClassTemplate type = new() + { + Name = "C", + Members = { new OperatorTemplate { Kind = (OperatorKind)99, Type = "C", Symbol = "+" } }, + }; + + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + + Assert.ThrowsExactly(() => codeBlocker.AddClass(type)); + Assert.AreEqual(string.Empty, codeBlocker.ToString()); + } + + [TestMethod] + public void AClassWithValidMembersIsStillWritten() => + Assert.AreEqual( + "class C\n{\n\tint a;\n\n\tint P { get; }\n}\n", + Render(new ClassTemplate + { + Name = "C", + Members = + { + new FieldTemplate { Name = "a", Type = "int" }, + new PropertyTemplate { Name = "P", Type = "int", Getter = AccessorTemplate.Auto() }, + }, + })); + [TestMethod] public void AConversionIsNamedByItsResultType() { @@ -1194,6 +1270,37 @@ private class Inner public void AnEmptySourceFileWritesNothing() => Assert.AreEqual(string.Empty, Render(codeBlocker => codeBlocker.AddSourceFile(new SourceFileTemplate()))); + [TestMethod] + public void ASourceFileWithAnInvalidClassWritesNothing() + { + SourceFileTemplate file = new() + { + Namespace = "Contoso", + Usings = { "System" }, + Comments = { "// " }, + Classes = + { + new ClassTemplate { Name = "Good" }, + new ClassTemplate { Name = "Bad", Members = { InvalidProperty() } }, + }, + }; + + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + + Assert.ThrowsExactly(() => codeBlocker.AddSourceFile(file)); + Assert.AreEqual(string.Empty, codeBlocker.ToString()); + } + + [TestMethod] + public void AddClassesWithAnInvalidClassWritesNothing() + { + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + + Assert.ThrowsExactly(() => codeBlocker.AddClasses( + [new ClassTemplate { Name = "Good" }, new ClassTemplate { Name = "Bad", Members = { InvalidProperty() } }])); + Assert.AreEqual(string.Empty, codeBlocker.ToString()); + } + [TestMethod] public void ASourceFileWritesItsPreambleThenItsTypes() { diff --git a/CodeBlocker/Templates/ClassTemplate.cs b/CodeBlocker/Templates/ClassTemplate.cs index 9b7cc2d..9304811 100644 --- a/CodeBlocker/Templates/ClassTemplate.cs +++ b/CodeBlocker/Templates/ClassTemplate.cs @@ -61,14 +61,19 @@ public class ClassTemplate : TemplateBase /// The to write to. /// is . /// - /// is not a known kind. Nothing is written. + /// , or the kind of a nested type, is not a known kind. Nothing is written. + /// + /// + /// A member, or a member of a nested type, would be refused when written. Nothing is written. /// public override void WriteTo(CodeBlocker codeBlocker) { Ensure.NotNull(codeBlocker); - // Resolved before the doc comment, attributes and modifiers are written, so an unknown kind - // does not leave them behind as an unfinished declaration. + // 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(); string keyword = TypeKindKeywords.For(Kind); base.WriteTo(codeBlocker); @@ -96,6 +101,22 @@ public override void WriteTo(CodeBlocker codeBlocker) WriteBodyTo(codeBlocker); } + /// + internal override void Validate() + { + _ = TypeKindKeywords.For(Kind); + + foreach (MemberTemplate member in Members) + { + member.Validate(); + } + + foreach (ClassTemplate nestedClass in NestedClasses) + { + nestedClass.Validate(); + } + } + /// /// Gets a value indicating whether this is a record whose positional parameters are its whole /// definition, which is declared with a semicolon instead of an empty body. diff --git a/CodeBlocker/Templates/OperatorTemplate.cs b/CodeBlocker/Templates/OperatorTemplate.cs index bdfcf8c..4c237b3 100644 --- a/CodeBlocker/Templates/OperatorTemplate.cs +++ b/CodeBlocker/Templates/OperatorTemplate.cs @@ -68,13 +68,7 @@ public override void WriteTo(CodeBlocker codeBlocker) // Worked out before anything is written, so an unknown kind cannot leave a half-written // declaration behind. - string signature = Kind switch - { - OperatorKind.Normal => $"{Type} operator {Symbol}", - OperatorKind.Implicit => $"implicit operator {Type}", - OperatorKind.Explicit => $"explicit operator {Type}", - _ => throw new InvalidOperationException($"Unknown operator kind '{Kind}'."), - }; + string signature = Signature; // Not through the MemberTemplate implementation: an operator's signature is its symbol and // result type, not a type followed by a name. @@ -84,4 +78,15 @@ public override void WriteTo(CodeBlocker codeBlocker) TemplateRendering.WriteParameterList(codeBlocker, Parameters); TemplateRendering.WriteBody(codeBlocker, BodyFactory); } + + /// + internal override void Validate() => _ = Signature; + + private string Signature => Kind switch + { + OperatorKind.Normal => $"{Type} operator {Symbol}", + OperatorKind.Implicit => $"implicit operator {Type}", + OperatorKind.Explicit => $"explicit operator {Type}", + _ => throw new InvalidOperationException($"Unknown operator kind '{Kind}'."), + }; } diff --git a/CodeBlocker/Templates/PropertyTemplate.cs b/CodeBlocker/Templates/PropertyTemplate.cs index d6bc947..dfe82a7 100644 --- a/CodeBlocker/Templates/PropertyTemplate.cs +++ b/CodeBlocker/Templates/PropertyTemplate.cs @@ -54,6 +54,27 @@ public override void WriteTo(CodeBlocker codeBlocker) { Ensure.NotNull(codeBlocker); + Validate(); + base.WriteTo(codeBlocker); + + if (ExpressionBodyFactory is not null) + { + TemplateRendering.WriteExpressionBody(codeBlocker, ExpressionBodyFactory); + return; + } + + if (CanUseShorthand) + { + WriteShorthand(codeBlocker); + return; + } + + WriteAccessorList(codeBlocker); + } + + /// + internal override void Validate() + { if (ExpressionBodyFactory is null && Getter is null && Setter is null) { // Writing "Type Name;" here would emit a field, not a property — which is what an @@ -71,22 +92,6 @@ public override void WriteTo(CodeBlocker codeBlocker) Getter?.Validate(); Setter?.Validate(); } - - base.WriteTo(codeBlocker); - - if (ExpressionBodyFactory is not null) - { - TemplateRendering.WriteExpressionBody(codeBlocker, ExpressionBodyFactory); - return; - } - - if (CanUseShorthand) - { - WriteShorthand(codeBlocker); - return; - } - - WriteAccessorList(codeBlocker); } /// The keyword the setter is written with. diff --git a/CodeBlocker/Templates/SourceFileTemplate.cs b/CodeBlocker/Templates/SourceFileTemplate.cs index 9b644f9..0ed8015 100644 --- a/CodeBlocker/Templates/SourceFileTemplate.cs +++ b/CodeBlocker/Templates/SourceFileTemplate.cs @@ -49,11 +49,18 @@ public static class SourceFileTemplateExtensions /// /// or is . /// + /// + /// A type in the file would be refused when written. Nothing is written. + /// public static CodeBlocker AddSourceFile(this CodeBlocker codeBlocker, SourceFileTemplate template) { Ensure.NotNull(codeBlocker); Ensure.NotNull(template); + // 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(template.Classes); + codeBlocker.AddDocumentation(template.Documentation); codeBlocker.AddComments(template.Comments); if (template.Attributes.Count > 0) @@ -82,14 +89,21 @@ public static CodeBlocker AddSourceFile(this CodeBlocker codeBlocker, SourceFile /// The types to write. /// The same , for chaining. /// - /// or is . + /// or , or one of the types, is + /// . Nothing is written. + /// + /// + /// One of the types would be refused when written. Nothing is written. /// public static CodeBlocker AddClasses(this CodeBlocker codeBlocker, IEnumerable classes) { Ensure.NotNull(codeBlocker); Ensure.NotNull(classes); - foreach (ClassTemplate classTemplate in classes) + List classList = [.. classes]; + ValidateAll(classList); + + foreach (ClassTemplate classTemplate in classList) { codeBlocker.AddClass(classTemplate); codeBlocker.NewLine(); @@ -97,4 +111,13 @@ public static CodeBlocker AddClasses(this CodeBlocker codeBlocker, IEnumerable classes) + { + foreach (ClassTemplate classTemplate in classes) + { + Ensure.NotNull(classTemplate); + classTemplate.Validate(); + } + } } diff --git a/CodeBlocker/Templates/TemplateBase.cs b/CodeBlocker/Templates/TemplateBase.cs index ee31222..d8a224a 100644 --- a/CodeBlocker/Templates/TemplateBase.cs +++ b/CodeBlocker/Templates/TemplateBase.cs @@ -78,6 +78,17 @@ public virtual void WriteTo(CodeBlocker codeBlocker) codeBlocker.AddKeywords(Keywords); } + /// + /// Throws if would refuse this template, without writing anything. + /// + /// + /// A container calls this on everything it holds before writing any of it, so a template that + /// fails its check cannot leave a container written without it, and the ones after it. + /// + internal virtual void Validate() + { + } + /// /// Writes = value when is set, as an escaped string literal /// when is set. From 11bfd086efe5eb3c62e9a00bcdbe88ee3ab8c420 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 06:38:08 +0000 Subject: [PATCH 2/2] Use Assert.Contains in the invalid-member test (MSTEST0046) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW3PT52Zo3M7fFP8Z6J --- CodeBlocker.Test/TemplateTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CodeBlocker.Test/TemplateTests.cs b/CodeBlocker.Test/TemplateTests.cs index 48bbf26..b59680d 100644 --- a/CodeBlocker.Test/TemplateTests.cs +++ b/CodeBlocker.Test/TemplateTests.cs @@ -930,7 +930,7 @@ public void AClassWithAnInvalidMemberWritesNothing() InvalidOperationException exception = Assert.ThrowsExactly(() => codeBlocker.AddClass(type)); - StringAssert.Contains(exception.Message, "'P'"); + Assert.Contains("'P'", exception.Message); Assert.AreEqual(string.Empty, codeBlocker.ToString()); }