diff --git a/CodeBlocker.Test/TemplateTests.cs b/CodeBlocker.Test/TemplateTests.cs index e9dfce8..3fd8237 100644 --- a/CodeBlocker.Test/TemplateTests.cs +++ b/CodeBlocker.Test/TemplateTests.cs @@ -490,17 +490,44 @@ public void AnExpressionBodyStartingWithABlankLineKeepsItsFirstLineOnTheDeclarat } [TestMethod] - public void AnEmptyExpressionBodyStillTerminatesTheDeclaration() + public void AnEmptyExpressionBodyIsRejectedBeforeAnythingIsWritten() { + // It used to render "public int X => ;", which does not compile, and raise nothing. PropertyTemplate property = new() { Type = "int", Name = "X", Keywords = { "public" }, + Documentation = new DocComment { Summary = "s" }, ExpressionBodyFactory = _ => { }, }; - Assert.AreEqual("public int X => ;\n", Render(property)); + 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()); + } + + [TestMethod] + public void AnExpressionBodyOfOnlyBlankLinesIsRejectedBeforeAnythingIsWritten() + { + PropertyTemplate property = new() + { + Type = "int", + Name = "X", + ExpressionBodyFactory = codeBlocker => + { + codeBlocker.NewLine(); + codeBlocker.WriteLine(" "); + }, + }; + + 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()); } private static string RenderExpressionBody(Action expressionFactory) => @@ -733,6 +760,33 @@ public void AnExpressionAccessorWithNoBodyIsRejectedBeforeAnythingIsWritten() Assert.AreEqual("int a;\n", codeBlocker.ToString()); } + [TestMethod] + public void AnExpressionAccessorThatWritesNoExpressionIsRejectedBeforeAnythingIsWritten() + { + PropertyTemplate property = new() + { + Type = "int", + Name = "P", + Getter = AccessorTemplate.Expression(_ => { }), + }; + + 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()); + } + + [TestMethod] + public void AStandaloneExpressionAccessorThatWritesNoExpressionWritesNothing() + { + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + codeBlocker.WriteLine("int a;"); + + Assert.ThrowsExactly(() => AccessorTemplate.Expression(_ => { }).WriteTo(codeBlocker, "get")); + Assert.AreEqual("int a;\n", codeBlocker.ToString()); + } + #endregion #region MethodTemplate diff --git a/CodeBlocker/Templates/AccessorTemplate.cs b/CodeBlocker/Templates/AccessorTemplate.cs index eb1cdf5..553308b 100644 --- a/CodeBlocker/Templates/AccessorTemplate.cs +++ b/CodeBlocker/Templates/AccessorTemplate.cs @@ -79,13 +79,37 @@ public static AccessorTemplate Block(Action body) => /// is . /// /// is not a known kind, or is with no - /// . Nothing is written. + /// or with one that writes no expression. Nothing is written. /// public void WriteTo(CodeBlocker codeBlocker, string keyword) { Ensure.NotNull(codeBlocker); Validate(); + WriteTo(codeBlocker, keyword, RenderExpression(codeBlocker, keyword)); + } + + /// + /// Renders the expression of an accessor, so an empty one + /// is rejected before the property it belongs to writes anything. + /// + /// The the accessor will be written to. + /// The accessor keyword, for the exception message. + /// The expression's lines, or for any other kind. + /// The wrote no expression. + internal string[]? RenderExpression(CodeBlocker codeBlocker, string keyword) => + Kind == AccessorKind.Expression + ? TemplateRendering.RenderExpression(codeBlocker, BodyFactory, $"The '{keyword}' accessor") + : null; + + /// + /// Writes the accessor, using an expression already rendered by . + /// + /// The to write to. + /// The accessor keyword: get, set or init. + /// The rendered expression, for an accessor. + internal void WriteTo(CodeBlocker codeBlocker, string keyword, string[]? expression) + { string prefix = string.IsNullOrEmpty(Modifier) ? keyword : $"{Modifier} {keyword}"; switch (Kind) @@ -96,7 +120,7 @@ public void WriteTo(CodeBlocker codeBlocker, string keyword) case AccessorKind.Expression: codeBlocker.Write(prefix); - TemplateRendering.WriteExpressionBody(codeBlocker, BodyFactory); + TemplateRendering.WriteExpressionBody(codeBlocker, expression ?? RenderExpression(codeBlocker, keyword)!); break; case AccessorKind.Block: @@ -120,6 +144,10 @@ public void WriteTo(CodeBlocker codeBlocker, string keyword) /// is not a known kind, or is with no /// , which would render as get => ;. /// + /// + /// This never runs . A factory that runs and writes no expression is + /// caught by , which the writers call before writing anything. + /// internal void Validate() { if (Kind is not (AccessorKind.Auto or AccessorKind.Expression or AccessorKind.Block)) diff --git a/CodeBlocker/Templates/PropertyTemplate.cs b/CodeBlocker/Templates/PropertyTemplate.cs index c817d28..471ec0a 100644 --- a/CodeBlocker/Templates/PropertyTemplate.cs +++ b/CodeBlocker/Templates/PropertyTemplate.cs @@ -28,7 +28,8 @@ public class PropertyTemplate : MemberTemplate /// /// /// An expression body replaces the accessor list, so and - /// are ignored when this is set. + /// are ignored when this is set. A callback that writes no expression, or + /// only blank lines, is rejected: => ; does not compile. /// public Action? ExpressionBodyFactory { get; set; } @@ -48,28 +49,39 @@ public class PropertyTemplate : MemberTemplate /// /// , and are all /// ; or, with no expression body, an accessor's kind is unknown, or is - /// with no . + /// with no ; or + /// the expression body, or an expression accessor's body, writes no expression. Nothing is + /// written in any of these cases. /// public override void WriteTo(CodeBlocker codeBlocker) { Ensure.NotNull(codeBlocker); Validate(); - base.WriteTo(codeBlocker); if (ExpressionBodyFactory is not null) { - TemplateRendering.WriteExpressionBody(codeBlocker, ExpressionBodyFactory); + // Rendered before the declaration is written: an empty expression is only found by + // running the callback, and "Type Name =>" would otherwise be left behind. + string[] expression = TemplateRendering.RenderExpression(codeBlocker, ExpressionBodyFactory, $"Property '{Name}'"); + base.WriteTo(codeBlocker); + TemplateRendering.WriteExpressionBody(codeBlocker, expression); return; } + // Likewise rendered up front, for an expression accessor's body. + string[]? getterExpression = Getter?.RenderExpression(codeBlocker, "get"); + string[]? setterExpression = Setter?.RenderExpression(codeBlocker, SetterKeyword); + + base.WriteTo(codeBlocker); + if (CanUseShorthand) { WriteShorthand(codeBlocker); return; } - WriteAccessorList(codeBlocker); + WriteAccessorList(codeBlocker, getterExpression, setterExpression); } /// @@ -128,14 +140,14 @@ private void WriteShorthand(CodeBlocker codeBlocker) codeBlocker.WriteLine(HasDefaultValue ? ";" : string.Empty); } - private void WriteAccessorList(CodeBlocker codeBlocker) + private void WriteAccessorList(CodeBlocker codeBlocker, string[]? getterExpression, string[]? setterExpression) { codeBlocker.WriteLine(); codeBlocker.WriteLine("{"); codeBlocker.Indent(); - Getter?.WriteTo(codeBlocker, "get"); - Setter?.WriteTo(codeBlocker, SetterKeyword); + Getter?.WriteTo(codeBlocker, "get", getterExpression); + Setter?.WriteTo(codeBlocker, SetterKeyword, setterExpression); codeBlocker.Outdent(); diff --git a/CodeBlocker/Templates/TemplateRendering.cs b/CodeBlocker/Templates/TemplateRendering.cs index 0cdfd39..f993ece 100644 --- a/CodeBlocker/Templates/TemplateRendering.cs +++ b/CodeBlocker/Templates/TemplateRendering.cs @@ -322,13 +322,38 @@ internal static void WriteBody(CodeBlocker codeBlocker, Action? bod } /// - /// Writes an expression body — the arrow, the expression and its terminating semicolon — and - /// ends the line. + /// Renders an expression body's expression into its lines, so it can be checked before anything + /// of the declaration it belongs to is written. /// - /// The to write to. + /// The the expression will be written to. /// /// The callback that writes the expression, without => or a semicolon. /// + /// Names the declaration in the exception message, for example Property 'P'. + /// The expression's lines, for . + /// + /// The callback wrote nothing but blank lines. An expression body has no valid empty form: it + /// would render as => ;, which does not compile. + /// + internal static string[] RenderExpression(CodeBlocker codeBlocker, Action? expressionFactory, string owner) + { + string[] lines = SplitLines(codeBlocker, RenderFragment(codeBlocker, expressionFactory)); + if (lines.All(string.IsNullOrWhiteSpace)) + { + throw new InvalidOperationException( + $"{owner} has an expression body whose callback wrote no expression, which would render as " + + "\"=> ;\". Write an expression, or use a block body or an automatic accessor instead."); + } + + return lines; + } + + /// + /// Writes an expression body — the arrow, the expression and its terminating semicolon — and + /// ends the line. + /// + /// The to write to. + /// The expression, as rendered it. /// /// The first line of the expression stays on the declaration line, after the arrow, and every /// following line is spliced at the current indent, so a multi-line switch expression, collection @@ -337,10 +362,8 @@ internal static void WriteBody(CodeBlocker codeBlocker, Action? bod /// of its own. A line comment there keeps the semicolon in front of it, and a line holding only a /// comment is not code, so the semicolon never ends up inside a comment. /// - internal static void WriteExpressionBody(CodeBlocker codeBlocker, Action? expressionFactory) + internal static void WriteExpressionBody(CodeBlocker codeBlocker, string[] lines) { - string[] lines = SplitLines(codeBlocker, RenderFragment(codeBlocker, expressionFactory)); - int last = lines.Length - 1; while (last >= 0 && lines[last].Length == 0) { @@ -355,12 +378,6 @@ internal static void WriteExpressionBody(CodeBlocker codeBlocker, Action ;"); - return; - } - // A directive must be the first thing on its line (CS1040), so one that opens the // expression goes on the line after the arrow rather than on the declaration line. if (IsDirective(lines[first]))