diff --git a/CodeBlocker.Test/TemplateTests.cs b/CodeBlocker.Test/TemplateTests.cs index 3fd8237..35f284a 100644 --- a/CodeBlocker.Test/TemplateTests.cs +++ b/CodeBlocker.Test/TemplateTests.cs @@ -787,6 +787,62 @@ public void AStandaloneExpressionAccessorThatWritesNoExpressionWritesNothing() Assert.AreEqual("int a;\n", codeBlocker.ToString()); } + private static void WritePlaceholderComment(CodeBlocker codeBlocker) => codeBlocker.WriteLine("// placeholder"); + + private static void WriteEmptyDebugConditional(CodeBlocker codeBlocker) + { + codeBlocker.WriteLine("#if DEBUG"); + codeBlocker.WriteLine("#endif"); + } + + [TestMethod] + public void AnExpressionBodyOfOnlyACommentIsRejectedBeforeAnythingIsWritten() + { + // Used to render "int P => ;// placeholder" (CS1525). + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + codeBlocker.WriteLine("int a;"); + + PropertyTemplate property = new() { Type = "int", Name = "P", ExpressionBodyFactory = WritePlaceholderComment }; + + Assert.ThrowsExactly(() => property.WriteTo(codeBlocker)); + Assert.AreEqual("int a;\n", codeBlocker.ToString()); + } + + [TestMethod] + public void AnExpressionAccessorOfOnlyACommentIsRejectedBeforeAnythingIsWritten() + { + // Used to render "get => ;// placeholder" (CS1525). + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + codeBlocker.WriteLine("int a;"); + + PropertyTemplate property = new() { Type = "int", Name = "P", Getter = AccessorTemplate.Expression(WritePlaceholderComment) }; + + Assert.ThrowsExactly(() => property.WriteTo(codeBlocker)); + Assert.ThrowsExactly(() => AccessorTemplate.Expression(WritePlaceholderComment).WriteTo(codeBlocker, "get")); + Assert.AreEqual("int a;\n", codeBlocker.ToString()); + } + + [TestMethod] + public void AnExpressionBodyOfOnlyDirectivesIsRejectedBeforeAnythingIsWritten() + { + // Used to render "int P =>", then the directives, then a lone ";" (CS1525). + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + + PropertyTemplate property = new() { Type = "int", Name = "P", ExpressionBodyFactory = WriteEmptyDebugConditional }; + + Assert.ThrowsExactly(() => property.WriteTo(codeBlocker)); + Assert.AreEqual(string.Empty, codeBlocker.ToString()); + } + + [TestMethod] + public void AnExpressionBodyWithCodeAroundItsCommentsAndDirectivesIsStillWritten() + { + Assert.AreEqual("public int X => x; // c\n", RenderExpressionBody(codeBlocker => codeBlocker.Write("x // c"))); + Assert.AreEqual( + "int P =>\n#if DEBUG\n1\n#else\n2\n#endif\n;\n", + Render(new PropertyTemplate { Type = "int", Name = "P", ExpressionBodyFactory = WriteDebugConditional })); + } + #endregion #region MethodTemplate @@ -1200,6 +1256,51 @@ public void AClassWithAnInvalidMemberWritesNothing() Assert.AreEqual(string.Empty, codeBlocker.ToString()); } + public static IEnumerable PropertiesWhoseExpressionWritesNothing => + [ + ["empty expression body", new PropertyTemplate { Type = "int", Name = "P", ExpressionBodyFactory = _ => { } }], + ["empty expression accessor", new PropertyTemplate { Type = "int", Name = "P", Getter = AccessorTemplate.Expression(_ => { }) }], + ["comment-only expression body", new PropertyTemplate { Type = "int", Name = "P", ExpressionBodyFactory = WritePlaceholderComment }], + ["comment-only expression accessor", new PropertyTemplate { Type = "int", Name = "P", Getter = AccessorTemplate.Expression(WritePlaceholderComment) }], + ]; + + [TestMethod] + [DynamicData(nameof(PropertiesWhoseExpressionWritesNothing))] + public void AClassWithAPropertyWhoseExpressionWritesNothingWritesNothing(string description, PropertyTemplate property) + { + // The empty expression used to be found only when the property was written, inside the + // class's body, so the class was closed without it and every member after it (#183). + ClassTemplate type = new() + { + Name = "C", + Members = { new FieldTemplate { Type = "int", Name = "f" }, property, new FieldTemplate { Type = "int", Name = "g" } }, + }; + + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + + Assert.ThrowsExactly(() => codeBlocker.AddClass(type), description); + Assert.AreEqual(string.Empty, codeBlocker.ToString(), description); + } + + [TestMethod] + public void ASourceFileWithAnEmptyExpressionBodyWritesNothing() + { + SourceFileTemplate file = new() + { + Namespace = "Contoso", + Classes = + { + new ClassTemplate { Name = "Good" }, + new ClassTemplate { Name = "Bad", Members = { new PropertyTemplate { Type = "int", Name = "P", ExpressionBodyFactory = _ => { } } } }, + }, + }; + + using CodeBlocker codeBlocker = CodeBlocker.Create(CodeBlocker.DefaultIndentString, NewLines.Lf); + + Assert.ThrowsExactly(() => codeBlocker.AddSourceFile(file)); + Assert.AreEqual(string.Empty, codeBlocker.ToString()); + } + [TestMethod] public void AnInvalidMemberOfANestedClassLeavesTheOuterClassUnwritten() { diff --git a/CodeBlocker/Templates/AccessorTemplate.cs b/CodeBlocker/Templates/AccessorTemplate.cs index 553308b..bc9d143 100644 --- a/CodeBlocker/Templates/AccessorTemplate.cs +++ b/CodeBlocker/Templates/AccessorTemplate.cs @@ -146,7 +146,8 @@ internal void WriteTo(CodeBlocker codeBlocker, string keyword, string[]? express /// /// /// This never runs . A factory that runs and writes no expression is - /// caught by , which the writers call before writing anything. + /// caught by , which the writers, and a property's own validation, + /// call before writing anything. /// internal void Validate() { diff --git a/CodeBlocker/Templates/ClassTemplate.cs b/CodeBlocker/Templates/ClassTemplate.cs index 9304811..a8f670a 100644 --- a/CodeBlocker/Templates/ClassTemplate.cs +++ b/CodeBlocker/Templates/ClassTemplate.cs @@ -73,7 +73,7 @@ public override void WriteTo(CodeBlocker codeBlocker) // 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(); + Validate(codeBlocker); string keyword = TypeKindKeywords.For(Kind); base.WriteTo(codeBlocker); @@ -102,18 +102,18 @@ public override void WriteTo(CodeBlocker codeBlocker) } /// - internal override void Validate() + internal override void Validate(CodeBlocker codeBlocker) { _ = TypeKindKeywords.For(Kind); foreach (MemberTemplate member in Members) { - member.Validate(); + member.Validate(codeBlocker); } foreach (ClassTemplate nestedClass in NestedClasses) { - nestedClass.Validate(); + nestedClass.Validate(codeBlocker); } } diff --git a/CodeBlocker/Templates/OperatorTemplate.cs b/CodeBlocker/Templates/OperatorTemplate.cs index 4c237b3..1aeccc2 100644 --- a/CodeBlocker/Templates/OperatorTemplate.cs +++ b/CodeBlocker/Templates/OperatorTemplate.cs @@ -80,7 +80,7 @@ public override void WriteTo(CodeBlocker codeBlocker) } /// - internal override void Validate() => _ = Signature; + internal override void Validate(CodeBlocker codeBlocker) => _ = Signature; private string Signature => Kind switch { diff --git a/CodeBlocker/Templates/PropertyTemplate.cs b/CodeBlocker/Templates/PropertyTemplate.cs index 471ec0a..5bc45c3 100644 --- a/CodeBlocker/Templates/PropertyTemplate.cs +++ b/CodeBlocker/Templates/PropertyTemplate.cs @@ -57,7 +57,7 @@ public override void WriteTo(CodeBlocker codeBlocker) { Ensure.NotNull(codeBlocker); - Validate(); + ValidateDeclaration(); if (ExpressionBodyFactory is not null) { @@ -85,7 +85,31 @@ public override void WriteTo(CodeBlocker codeBlocker) } /// - internal override void Validate() + /// + /// 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. + /// + internal override void Validate(CodeBlocker codeBlocker) + { + ValidateDeclaration(); + + if (ExpressionBodyFactory is not null) + { + _ = TemplateRendering.RenderExpression(codeBlocker, ExpressionBodyFactory, $"Property '{Name}'"); + return; + } + + _ = Getter?.RenderExpression(codeBlocker, "get"); + _ = Setter?.RenderExpression(codeBlocker, SetterKeyword); + } + + /// + /// Throws when the property cannot be written whatever its callbacks write: when it has no + /// accessors and no expression body, or an accessor that cannot be written. + /// + private void ValidateDeclaration() { if (ExpressionBodyFactory is null && Getter is null && Setter is null) { diff --git a/CodeBlocker/Templates/SourceFileTemplate.cs b/CodeBlocker/Templates/SourceFileTemplate.cs index 0ed8015..db6f904 100644 --- a/CodeBlocker/Templates/SourceFileTemplate.cs +++ b/CodeBlocker/Templates/SourceFileTemplate.cs @@ -59,7 +59,7 @@ public static CodeBlocker AddSourceFile(this CodeBlocker codeBlocker, SourceFile // 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); + ValidateAll(codeBlocker, template.Classes); codeBlocker.AddDocumentation(template.Documentation); codeBlocker.AddComments(template.Comments); @@ -101,7 +101,7 @@ public static CodeBlocker AddClasses(this CodeBlocker codeBlocker, IEnumerable classList = [.. classes]; - ValidateAll(classList); + ValidateAll(codeBlocker, classList); foreach (ClassTemplate classTemplate in classList) { @@ -112,12 +112,12 @@ public static CodeBlocker AddClasses(this CodeBlocker codeBlocker, IEnumerable classes) + private static void ValidateAll(CodeBlocker codeBlocker, IEnumerable classes) { foreach (ClassTemplate classTemplate in classes) { Ensure.NotNull(classTemplate); - classTemplate.Validate(); + classTemplate.Validate(codeBlocker); } } } diff --git a/CodeBlocker/Templates/TemplateBase.cs b/CodeBlocker/Templates/TemplateBase.cs index 1abb91b..41915f8 100644 --- a/CodeBlocker/Templates/TemplateBase.cs +++ b/CodeBlocker/Templates/TemplateBase.cs @@ -88,11 +88,16 @@ public virtual void WriteTo(CodeBlocker codeBlocker) /// /// Throws if would refuse this template, without writing anything. /// + /// + /// The the template will be written to. Nothing is written to it: it + /// supplies the indent and line terminator a callback is rendered with, for a check that can + /// only be made by running the callback. + /// /// /// 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() + internal virtual void Validate(CodeBlocker codeBlocker) { } diff --git a/CodeBlocker/Templates/TemplateRendering.cs b/CodeBlocker/Templates/TemplateRendering.cs index f993ece..ee2b7c9 100644 --- a/CodeBlocker/Templates/TemplateRendering.cs +++ b/CodeBlocker/Templates/TemplateRendering.cs @@ -332,17 +332,21 @@ internal static void WriteBody(CodeBlocker codeBlocker, Action? bod /// 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. + /// The callback wrote nothing but blank lines, line comments and preprocessor directives. 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)) + // A comment or directive is no more an expression than a blank line is: a placeholder + // comment would otherwise take the semicolon in front of it and render as "=> ;// ...". + if (!HasCode(lines)) { 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."); + $"{owner} has an expression body whose callback wrote no expression, only blank lines, " + + "comments or directives, which would render as \"=> ;\". Write an expression, or use a " + + "block body or an automatic accessor instead."); } return lines;