Reject an accessorless property or unknown operator kind before writing anything [patch]

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<InvalidOperationException>(() => 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<InvalidOperationException>(() => unknown.WriteTo(codeBlocker));
+		Assert.AreEqual("int a;\n", codeBlocker.ToString());
+	}
+
 	[TestMethod]
 	public void AConversionIsNamedByItsResultType()
 	{

CodeBlocker/Templates/OperatorTemplate.cs

@@ -59,31 +59,27 @@ public class OperatorTemplate : MemberTemplate
 	/// </summary>
 	/// <param name="codeBlocker">The <see cref="CodeBlocker"/> to write to.</param>
 	/// <exception cref="ArgumentNullException"><paramref name="codeBlocker"/> is <see langword="null"/>.</exception>
+	/// <exception cref="InvalidOperationException">
+	/// <see cref="Kind"/> is not an <see cref="OperatorKind"/> value. Nothing is written.
+	/// </exception>
 	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);

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 <c>{ get; set; }</c> on one line, with any
 	/// <see cref="TemplateBase.DefaultValue"/> 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
+	/// <paramref name="codeBlocker"/>.
 	/// </remarks>
 	/// <exception cref="ArgumentNullException"><paramref name="codeBlocker"/> is <see langword="null"/>.</exception>
+	/// <exception cref="InvalidOperationException">
+	/// <see cref="Getter"/>, <see cref="Setter"/> and <see cref="ExpressionBodyFactory"/> are all
+	/// <see langword="null"/>.
+	/// </exception>
 	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);