From 8ae069697ea46662a81765488fbb08b4b79e25e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 15:29:21 +0000 Subject: [PATCH] Reject an accessorless property or unknown operator kind before writing anything [patch] PropertyTemplate.WriteTo wrote the doc comment, attributes, modifiers and "Type Name" before checking that the property had an accessor or an expression body, so the throw left an unfinished declaration in the buffer; inside a class the scope then closed it as "public int Foo}". OperatorTemplate did the same for an out-of-range Kind. Both now validate first, so a caller that catches the exception can keep using the CodeBlocker. The PropertyTemplate.WriteTo remarks, which still described the old "terminated with a semicolon" behaviour, now describe the throw. Fixes #128 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014h6btvAuM1Ew7cCFSpuFWc --- CodeBlocker.Test/TemplateTests.cs | 40 +++++++++++++++++++++++ CodeBlocker/Templates/OperatorTemplate.cs | 32 ++++++++---------- CodeBlocker/Templates/PropertyTemplate.cs | 28 +++++++++------- 3 files changed, 71 insertions(+), 29 deletions(-) diff --git a/CodeBlocker.Test/TemplateTests.cs b/CodeBlocker.Test/TemplateTests.cs index 5c40f52..772238d 100644 --- a/CodeBlocker.Test/TemplateTests.cs +++ b/CodeBlocker.Test/TemplateTests.cs @@ -440,6 +440,27 @@ public void APropertyWithNoAccessorsAtAllIsRejected() Assert.Contains("Value", exception.Message); } + [TestMethod] + public void ARejectedPropertyWritesNothing() + { + // The declaration used to be written before the check, so a caller that caught the + // exception and carried on was left with "public int Foo" and no terminator. + PropertyTemplate property = new() + { + Type = "int", + Name = "Foo", + Keywords = { "public" }, + Attributes = { "Obsolete" }, + Documentation = new DocComment { Summary = "Foo." }, + }; + + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + codeBlocker.WriteLine("int a;"); + + Assert.ThrowsExactly(() => property.WriteTo(codeBlocker)); + Assert.AreEqual("int a;\n", codeBlocker.ToString()); + } + #endregion #region MethodTemplate @@ -733,6 +754,25 @@ public void AnOperatorIsNamedByItsSymbol() Render(op)); } + [TestMethod] + public void AnOperatorOfUnknownKindWritesNothing() + { + OperatorTemplate unknown = new() + { + Kind = (OperatorKind)99, + Type = "Money", + Symbol = "+", + Keywords = { "public", "static" }, + Attributes = { "Obsolete" }, + }; + + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + codeBlocker.WriteLine("int a;"); + + Assert.ThrowsExactly(() => unknown.WriteTo(codeBlocker)); + Assert.AreEqual("int a;\n", codeBlocker.ToString()); + } + [TestMethod] public void AConversionIsNamedByItsResultType() { diff --git a/CodeBlocker/Templates/OperatorTemplate.cs b/CodeBlocker/Templates/OperatorTemplate.cs index f7e67e4..bdfcf8c 100644 --- a/CodeBlocker/Templates/OperatorTemplate.cs +++ b/CodeBlocker/Templates/OperatorTemplate.cs @@ -59,31 +59,27 @@ public class OperatorTemplate : MemberTemplate /// /// The to write to. /// is . + /// + /// is not an value. Nothing is written. + /// public override void WriteTo(CodeBlocker codeBlocker) { Ensure.NotNull(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}'."), + }; + // Not through the MemberTemplate implementation: an operator's signature is its symbol and // result type, not a type followed by a name. codeBlocker.AddTemplate(this); - - switch (Kind) - { - case OperatorKind.Normal: - codeBlocker.Write($"{Type} operator {Symbol}"); - break; - - case OperatorKind.Implicit: - codeBlocker.Write($"implicit operator {Type}"); - break; - - case OperatorKind.Explicit: - codeBlocker.Write($"explicit operator {Type}"); - break; - - default: - throw new InvalidOperationException($"Unknown operator kind '{Kind}'."); - } + codeBlocker.Write(signature); TemplateRendering.WriteParameterList(codeBlocker, Parameters); TemplateRendering.WriteBody(codeBlocker, BodyFactory); diff --git a/CodeBlocker/Templates/PropertyTemplate.cs b/CodeBlocker/Templates/PropertyTemplate.cs index 87260ef..03f2eba 100644 --- a/CodeBlocker/Templates/PropertyTemplate.cs +++ b/CodeBlocker/Templates/PropertyTemplate.cs @@ -40,32 +40,38 @@ public class PropertyTemplate : MemberTemplate /// An expression body is written on the declaration line. Otherwise, accessors that are all /// automatic and unqualified collapse to { get; set; } on one line, with any /// as its initialiser; anything else gets a braced - /// accessor list. A property with no accessors at all is declared and terminated with a - /// semicolon, which is what an abstract or interface property looks like. + /// accessor list. A property with no accessors and no expression body is rejected before + /// anything is written, so a caller that catches the exception can keep using + /// . /// /// is . + /// + /// , and are all + /// . + /// public override void WriteTo(CodeBlocker codeBlocker) { Ensure.NotNull(codeBlocker); - base.WriteTo(codeBlocker); - - if (ExpressionBodyFactory is not null) - { - TemplateRendering.WriteExpressionBody(codeBlocker, ExpressionBodyFactory); - return; - } - - if (Getter is null && Setter is null) + 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 // accessorless property used to render as. An abstract or interface property is // spelled with accessors that have no body: Getter = AccessorTemplate.Auto(). + // Checked before the declaration is written, which would otherwise be left unfinished. throw new InvalidOperationException( $"Property '{Name}' declares no accessors and no expression body. Set Getter, " + "Setter, or ExpressionBodyFactory."); } + base.WriteTo(codeBlocker); + + if (ExpressionBodyFactory is not null) + { + TemplateRendering.WriteExpressionBody(codeBlocker, ExpressionBodyFactory); + return; + } + if (CanUseShorthand) { WriteShorthand(codeBlocker);