From acae54dd965b576e0c053eac206a3fbff9e692e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 03:29:00 +0000 Subject: [PATCH 1/2] Refuse Void anywhere but a return type or a Result's value [minor] Void was refused as a parameter and nowhere else. A wrapper is transparent to everything else in validation - a class named inside an Optional is checked exactly as one named directly is - and that transparency is what let it through: a member typed Void, or an Optional or Span anywhere, validated cleanly and then emitted `void x{};`, `std::optional` or `std::span`, none of which are types. The compiler that refused them was the consumer's, pointing at generated code rather than at the schema that produced it. The editor's type picker offers Void for return types and the same picker serves members, so it is a reachable edit rather than a contrived one. ValidateType now carries a TypePosition down the descent, and ValidateTypeStandsHere asks the question once on the way down rather than at each call site. The two positions an absent value may stand in are a function's return type and the value a Result carries, which is how "can fail, produces nothing" is spelled and is a type in both target languages. None is reported on the same descent, but only below a declaration: a member, a parameter and a return type each already say what is unfinished in their own words, and a vector says what its components have to be, so those keep their one message. Below them nobody was saying anything, so an Optional reached the generator and threw there. AbsentTypeValidationTests covers both halves - the shapes now refused, and the six places that must not gain a second message. CppTypeMappingTests pins why validation has to be the gate for Void in particular: unlike None, the mapper has an answer for it wherever it is asked, so generation cannot be what catches it. Fixes #172 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JhsxzNdL3VieTYFXFpwmov --- CLAUDE.md | 13 + Schema.Cpp.Test/CppTypeMappingTests.cs | 37 +++ Schema.Test/AbsentTypeValidationTests.cs | 298 +++++++++++++++++++++++ Schema/Models/Schema.Validation.cs | 103 +++++++- 4 files changed, 443 insertions(+), 8 deletions(-) create mode 100644 Schema.Test/AbsentTypeValidationTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 0cb9313..425a424 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -330,6 +330,19 @@ grew annotations until it was a worse version of the language it described; keep what stops that. `Schema.Validation.cs` enforces the corollaries - no `Array`, `Result` or `Void` parameters, no `None` anywhere generatable. +Both halves of that last sentence are checked **on the way down**, by `ValidateTypeStandsHere`, +rather than at the top of each declaration. A wrapper is transparent to everything else in +validation - a class named inside an `Optional` is checked exactly as one named directly is - and +that transparency is what used to let the two absent types through: a `Void` was refused as a +parameter and nowhere else, so a member typed `Void`, or an `Optional` or `Span` +anywhere, validated cleanly and then emitted `void x{};`, `std::optional` or +`std::span`, none of which are types. `TypePosition` is what the descent carries, and it +names the only two positions an absent type may stand in: a function's **return type**, and the +**value a `Result` carries**, which is how "can fail, produces nothing" is spelled. A `None` is +reported there too, but only below a declaration - a member, a parameter and a return type each +already say what is unfinished in their own words, and a vector says what its components have to +be, so those keep their own message rather than gaining a second one. + What a failure *says* is global for the same reason: `Schema.ErrorType` names the enum a failed `Result` carries, once, rather than every fallible signature choosing its own. An enum rather than any type, because an error is one of a closed set of reasons. A schema that never returns a `Result` diff --git a/Schema.Cpp.Test/CppTypeMappingTests.cs b/Schema.Cpp.Test/CppTypeMappingTests.cs index 3b7faee..aa5e631 100644 --- a/Schema.Cpp.Test/CppTypeMappingTests.cs +++ b/Schema.Cpp.Test/CppTypeMappingTests.cs @@ -127,6 +127,43 @@ public void AMemberWithNoTypeIsRefused() Assert.Contains("no type chosen", refused.Message, StringComparison.Ordinal); } + /// + /// Void is the one absent type the mapper has an answer for wherever it is asked, so + /// nothing here refuses it and generation cannot be what catches it. + /// + /// + /// Both halves are asserted together on purpose. The first says what the generator writes - + /// three spellings that are not types, since void x{}; is not a declaration and neither + /// std::optional<void> nor std::span<void> is instantiable - and the + /// second says the schema refuses the same three before a generator is ever reached. That is + /// the difference between an author being told which member is wrong and a consumer's compiler + /// pointing at generated code. + /// + [TestMethod] + public void VoidIsRefusedByTheSchemaRatherThanByTheMapper() + { + (BaseType Type, string IllFormed)[] shapes = + [ + (new Void(), "void value{};"), + (new Optional { ElementType = new Void() }, "std::optional"), + (new Span { ElementType = new Void() }, "std::span"), + ]; + + foreach ((BaseType type, string illFormed) in shapes) + { + Assert.Contains(illFormed, Generate(type), StringComparison.Ordinal); + + Models.Schema schema = new(); + schema.AddClass("Holder".As())!.AddMember("Value".As())!.SetType(type); + + Assert.Contains( + i => i.Severity == SchemaValidationSeverity.Error + && i.Message.Contains("Void carries no value", StringComparison.Ordinal), + schema.Validate(), + illFormed); + } + } + /// /// A member's name is spelled the target's way; a type's keeps the schema's, because a type /// name is the same word in both places. diff --git a/Schema.Test/AbsentTypeValidationTests.cs b/Schema.Test/AbsentTypeValidationTests.cs new file mode 100644 index 0000000..6a23dff --- /dev/null +++ b/Schema.Test/AbsentTypeValidationTests.cs @@ -0,0 +1,298 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Schema.Tests; + +using System.Collections.ObjectModel; + +using ktsu.Schema.Models; +using ktsu.Schema.Models.Names; +using ktsu.Schema.Models.Types; +using ktsu.Semantics.Strings; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Covers the two types that describe the absence of a value rather than a value, and the one +/// position each of them is allowed to stand in. +/// +/// +/// is a decision - this function returns nothing - and is an +/// unfinished one. Neither is a type a value can be, so neither can be generated where a value has +/// to be, and a wrapper is not a place one stops being needed: std::optional<void> and +/// std::span<void> are not instantiable and void x{}; is not a declaration. +/// +[TestClass] +public sealed class AbsentTypeValidationTests +{ + /// + /// A member holding nothing is not a member. The editor's type picker offers Void for + /// return types and the same picker serves members, so this is a reachable edit rather than a + /// contrived one. + /// + [TestMethod] + public void AVoidMemberIsRejected() + { + Schema schema = HolderOf(new Void()); + + Assert.Contains( + i => i.Severity == SchemaValidationSeverity.Error + && i.Path == "Holder.Value" + && i.Message.Contains("Void carries no value", StringComparison.Ordinal), + schema.Validate(), + string.Join("; ", schema.Validate())); + } + + /// + /// Every wrapper resolves to whatever it wraps, and wrapping nothing yields nothing. Each of + /// these validated cleanly and then emitted C++ the consumer's compiler refused. + /// + [TestMethod] + [DataRow("Optional")] + [DataRow("Span")] + [DataRow("Handle")] + [DataRow("Array")] + public void AVoidInsideAWrapperIsRejected(string wrapper) + { + Schema schema = HolderOf(Wrap(wrapper, new Void())); + + Assert.Contains( + i => i.Severity == SchemaValidationSeverity.Error + && i.Path == "Holder.Value" + && i.Message.Contains("Void carries no value", StringComparison.Ordinal), + schema.Validate(), + string.Join("; ", schema.Validate())); + } + + /// + /// The check is on the way down rather than at the top, so burying it deeper does not get it + /// past. + /// + [TestMethod] + public void AVoidTwoWrappersDeepIsRejected() + { + Schema schema = HolderOf(new Array + { + ElementType = new Optional { ElementType = new Void() }, + Container = Array.VectorContainer.As(), + }); + + Assert.Contains( + i => i.Message.Contains("Void carries no value", StringComparison.Ordinal), + schema.Validate(), + string.Join("; ", schema.Validate())); + } + + /// + /// A member whose type was never chosen is one unfinished edit, and says so in the words of + /// the thing that is unfinished. Below a member there was nobody saying anything at all, so an + /// Optional<None> validated clean and threw at generation time. + /// + [TestMethod] + [DataRow("Optional")] + [DataRow("Span")] + [DataRow("Handle")] + [DataRow("Array")] + public void AnUnchosenTypeInsideAWrapperIsRejected(string wrapper) + { + Schema schema = HolderOf(Wrap(wrapper, new None())); + + Assert.Contains( + i => i.Severity == SchemaValidationSeverity.Error + && i.Path == "Holder.Value" + && i.Message.Contains("No type was chosen for what this carries", StringComparison.Ordinal), + schema.Validate(), + string.Join("; ", schema.Validate())); + } + + /// + /// A fallible call that produces nothing is the one thing a wrapper over Void may say, + /// and it is a type in both target languages - std::expected<void, E> and the + /// one-argument Result<TError>. + /// + [TestMethod] + public void ResultOfVoidIsAccepted() + { + Schema schema = Fallible(); + SchemaFunction flush = schema.AddInterface("Sink".As())! + .AddFunction("Flush".As())!; + flush.SetReturnType(new Result { ElementType = new Void() }); + + Assert.IsEmpty(schema.Validate(), string.Join("; ", schema.Validate())); + } + + /// + /// The exemption is the Result's own value and does not carry through a wrapper inside + /// it: std::expected<std::optional<void>, E> is as ill-formed as + /// std::optional<void> alone. + /// + [TestMethod] + public void ResultOfAWrapperOverVoidIsRejected() + { + Schema schema = Fallible(); + SchemaFunction flush = schema.AddInterface("Sink".As())! + .AddFunction("Flush".As())!; + flush.SetReturnType(new Result { ElementType = new Optional { ElementType = new Void() } }); + + Assert.Contains( + i => i.Message.Contains("Void carries no value", StringComparison.Ordinal), + schema.Validate(), + string.Join("; ", schema.Validate())); + } + + /// + /// A Result whose value was never chosen is reported wherever it is written. In a return + /// position the caller already says it in better words - "use Result<Void>" - so this is + /// the member position, which said nothing before. + /// + [TestMethod] + public void ResultOfAnUnchosenTypeIsRejected() + { + Schema schema = Fallible(); + SchemaClass holder = schema.AddClass("Holder".As())!; + holder.AddMember("Value".As())!.SetType(new Result { ElementType = new None() }); + + Assert.Contains( + i => i.Message.Contains("No type was chosen for what this carries", StringComparison.Ordinal), + schema.Validate(), + string.Join("; ", schema.Validate())); + } + + /// + /// A function returning nothing is what Void is for, and it is still accepted there. + /// + [TestMethod] + public void AVoidReturnTypeIsAccepted() + { + Schema schema = new(); + schema.AddInterface("Logger".As())! + .AddFunction("Write".As())! + .SetReturnType(new Void()); + + Assert.IsEmpty(schema.Validate(), string.Join("; ", schema.Validate())); + } + + /// + /// A Void parameter keeps the message that names it, which says what to do about it. + /// + [TestMethod] + public void AVoidParameterKeepsItsOwnMessage() + { + Schema schema = new(); + SchemaFunction write = schema.AddInterface("Logger".As())! + .AddFunction("Write".As())!; + write.SetReturnType(new Void()); + write.AddParameter("nothing".As())!.SetType(new Void()); + + Collection issues = schema.Validate(); + + Assert.ContainsSingle(issues, string.Join("; ", issues)); + Assert.Contains("Remove it", issues[0].Message, StringComparison.Ordinal); + } + + /// + /// A parameter is only exempt at the top: the wrapper arm is what let one through. + /// + [TestMethod] + public void AVoidInsideAParameterIsRejected() + { + Schema schema = new(); + SchemaFunction draw = schema.AddInterface("Renderer".As())! + .AddFunction("Draw".As())!; + draw.SetReturnType(new Void()); + draw.AddParameter("vertices".As())!.SetType(new Span { ElementType = new Void() }); + + Assert.Contains( + i => i.Message.Contains("Void carries no value", StringComparison.Ordinal), + schema.Validate(), + string.Join("; ", schema.Validate())); + } + + /// + /// A member with no type is one unfinished edit, and is told about once. Reporting the absence + /// again on the way down would make a schema being typed into noisier the less finished it is. + /// + [TestMethod] + public void AMemberWithoutATypeIsStillOneMessage() + { + Schema schema = new(); + schema.AddClass("Holder".As())!.AddMember("Value".As()); + + Collection issues = schema.Validate(); + + Assert.ContainsSingle(issues, string.Join("; ", issues)); + Assert.AreEqual(SchemaValidationSeverity.Warning, issues[0].Severity); + } + + /// + /// A vector already says what its components have to be, which is more use than being told + /// that Void carries no value. One mistake, one message. + /// + [TestMethod] + public void AVectorOfNothingSaysWhatAComponentHasToBe() + { + foreach (BaseType component in new BaseType[] { new Void(), new None() }) + { + Schema schema = HolderOf(new Vector3 { ElementType = component }); + + Collection issues = schema.Validate(); + + Assert.ContainsSingle(issues, string.Join("; ", issues)); + Assert.Contains("components are numbers", issues[0].Message, StringComparison.Ordinal); + } + } + + /// + /// A colour says the same thing about its channels. + /// + [TestMethod] + public void AColourOfNothingSaysWhatAChannelHasToBe() + { + Schema schema = HolderOf(new ColorRGB { ElementType = new Void() }); + + Collection issues = schema.Validate(); + + Assert.ContainsSingle(issues, string.Join("; ", issues)); + Assert.Contains("channels are floats", issues[0].Message, StringComparison.Ordinal); + } + + /// + /// A schema holding one class whose single member has the given type. + /// + private static Schema HolderOf(BaseType type) + { + Schema schema = new(); + schema.AddClass("Holder".As())! + .AddMember("Value".As())! + .SetType(type); + + return schema; + } + + /// + /// A schema that can say why a call failed, which a Result anywhere in it needs. + /// + private static Schema Fallible() + { + Schema schema = new(); + schema.AddEnum("ErrorCode".As())!.TryAddValue("NotFound".As()); + schema.ErrorType = "ErrorCode".As(); + + return schema; + } + + /// + /// The four carriers that hold one element, by name, so one case covers all of them. + /// + /// + /// Result is not among them: it is the one whose element may legitimately be + /// Void, and it has its own cases above. + /// + private static BaseType Wrap(string wrapper, BaseType element) => wrapper switch + { + "Optional" => new Optional { ElementType = element }, + "Span" => new Span { ElementType = element }, + "Handle" => new Handle { ElementType = element }, + "Array" => new Array { ElementType = element, Container = Array.VectorContainer.As() }, + _ => throw new ArgumentOutOfRangeException(nameof(wrapper), wrapper, "Not a wrapper this test knows."), + }; +} diff --git a/Schema/Models/Schema.Validation.cs b/Schema/Models/Schema.Validation.cs index bc3f2f0..a304b29 100644 --- a/Schema/Models/Schema.Validation.cs +++ b/Schema/Models/Schema.Validation.cs @@ -111,7 +111,7 @@ private void ValidateClasses(Collection issues) }); } - ValidateType(issues, member.Type, memberPath, member); + ValidateType(issues, member.Type, memberPath, member, TypePosition.Declared); ValidateMetadata(issues, member, member.Type, memberPath, member, "member"); ValidateTravelsAsBytes(issues, schemaClass, member, memberPath); } @@ -676,7 +676,7 @@ private void ValidateReturnType(Collection issues, Schema return; default: - ValidateType(issues, function.ReturnType, path, function); + ValidateType(issues, function.ReturnType, path, function, TypePosition.Return); return; } } @@ -707,13 +707,52 @@ private void ValidateParameter(Collection issues, SchemaP return; default: - ValidateType(issues, parameter.Type, path, element); + ValidateType(issues, parameter.Type, path, element, TypePosition.Declared); return; } } - private void ValidateType(Collection issues, BaseType type, string path, ISchemaElement? element) + /// + /// Where a type was written, which is what decides whether may stand + /// there and who reports a type that was never chosen. + /// + private enum TypePosition + { + /// + /// The type of a member or a parameter, as the author wrote it. + /// + /// + /// A here is reported by the caller, in a message that can name what is + /// unfinished - a member, a parameter - rather than describing the hole it left. + /// + Declared, + + /// + /// A function's return type, which is the one place is a decision + /// rather than a gap. + /// + Return, + + /// + /// The value a carries when the call succeeds. + /// + /// + /// Result<Void> is the honest spelling of "can fail, produces nothing", and it + /// is a type wherever it is written: C++ spells it std::expected<void, E> and + /// C# the one-argument Result<TError>. + /// + ResultValue, + + /// + /// Inside a wrapper, an array or a vector - somewhere a value has to actually be. + /// + Nested, + } + + private void ValidateType(Collection issues, BaseType type, string path, ISchemaElement? element, TypePosition position) { + ValidateTypeStandsHere(issues, type, path, element, position); + switch (type) { case Enum enumType: @@ -749,13 +788,13 @@ private void ValidateType(Collection issues, BaseType typ // call can fail, and what a failure says is the schema's to declare. case Result result: ValidateResultHasAnErrorType(issues, path, element); - ValidateType(issues, result.ElementType, path, element); + ValidateType(issues, result.ElementType, path, element, TypePosition.ResultValue); break; // Every wrapper resolves to whatever it wraps, so a class named inside a Span, // Handle, Result or Optional is checked exactly as one named directly is. case WrapperType wrapper: - ValidateType(issues, wrapper.ElementType, path, element); + ValidateType(issues, wrapper.ElementType, path, element, TypePosition.Nested); break; default: @@ -763,6 +802,48 @@ private void ValidateType(Collection issues, BaseType typ } } + /// + /// Two of the types describe the absence of a value rather than a value, and neither can be + /// generated where one has to be. + /// + /// + /// + /// Asked on the way down rather than at each call site, because a wrapper is transparent to + /// everything else here - a class named inside an is checked exactly as + /// one named directly is - and that transparency is what let these two through. A + /// was refused as a parameter and nowhere else, so a member typed + /// Void, or an Optional<Void> or Span<Void> anywhere, validated + /// cleanly and then emitted void x{};, std::optional<void> or + /// std::span<void> - none of which are types. The compiler that refused them was + /// the consumer's, pointing at generated code rather than at the schema that produced it. + /// + /// + /// A is reported here only where the author was not already told: a member, + /// a parameter and a return type each say what is unfinished in their own words, and a vector + /// says what its components have to be. Below that there was nobody saying anything, so an + /// Optional<None> reached the generator and threw. + /// + /// + private static void ValidateTypeStandsHere(Collection issues, BaseType type, string path, ISchemaElement? element, TypePosition position) + { + switch (type) + { + case Types.Void when position is TypePosition.Return or TypePosition.ResultValue: + return; + + case Types.Void: + Report(issues, path, element!, "Void carries no value, so nothing can be declared as one. Only a function's return type, or the value a Result carries, may be Void."); + return; + + case None when position is TypePosition.Nested or TypePosition.ResultValue: + Report(issues, path, element!, "No type was chosen for what this carries, so there is nothing to generate."); + return; + + default: + return; + } + } + /// /// The enum a failure carries has to be one the schema declares. /// @@ -933,7 +1014,13 @@ private static bool SameDimension(DimensionInfo left, DimensionInfo right) /// private void ValidateVectorElement(Collection issues, Vector vectorType, string path, ISchemaElement? element) { - ValidateType(issues, vectorType.ElementType, path, element); + // A component that is not a type at all is left to the checks below, which name what a + // vector's components have to be. Being told instead that Void carries no value is true and + // less use, and saying both is two messages for one mistake. + if (vectorType.ElementType is not (None or Types.Void)) + { + ValidateType(issues, vectorType.ElementType, path, element, TypePosition.Nested); + } // A colour's components are the channels, and every consumer of one - a picker, a // shader, a serialiser - reads them as floats. A colour of anything else is a Vector. @@ -1026,7 +1113,7 @@ private void ValidateClassReference(Collection issues, Na private void ValidateArray(Collection issues, Array arrayType, string path, ISchemaElement? element) { - ValidateType(issues, arrayType.ElementType, path, element); + ValidateType(issues, arrayType.ElementType, path, element, TypePosition.Nested); ValidateArrayContainer(issues, arrayType, path, element); if (string.IsNullOrEmpty(arrayType.Key)) From f1f360058cbf778893e3dfdf1e62147fb327d245 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 03:32:58 +0000 Subject: [PATCH 2/2] test: split the vector case into a row per absent type github-code-quality flagged the loop for mapping its iteration variable straight to another one. A row per case is what the wrapper cases beside it already do, and it says which of the two failed rather than stopping at the first. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JhsxzNdL3VieTYFXFpwmov --- Schema.Test/AbsentTypeValidationTests.cs | 25 ++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/Schema.Test/AbsentTypeValidationTests.cs b/Schema.Test/AbsentTypeValidationTests.cs index 6a23dff..5bb3546 100644 --- a/Schema.Test/AbsentTypeValidationTests.cs +++ b/Schema.Test/AbsentTypeValidationTests.cs @@ -228,17 +228,16 @@ public void AMemberWithoutATypeIsStillOneMessage() /// that Void carries no value. One mistake, one message. /// [TestMethod] - public void AVectorOfNothingSaysWhatAComponentHasToBe() + [DataRow("Void")] + [DataRow("None")] + public void AVectorOfNothingSaysWhatAComponentHasToBe(string component) { - foreach (BaseType component in new BaseType[] { new Void(), new None() }) - { - Schema schema = HolderOf(new Vector3 { ElementType = component }); + Schema schema = HolderOf(new Vector3 { ElementType = Absent(component) }); - Collection issues = schema.Validate(); + Collection issues = schema.Validate(); - Assert.ContainsSingle(issues, string.Join("; ", issues)); - Assert.Contains("components are numbers", issues[0].Message, StringComparison.Ordinal); - } + Assert.ContainsSingle(issues, string.Join("; ", issues)); + Assert.Contains("components are numbers", issues[0].Message, StringComparison.Ordinal); } /// @@ -280,6 +279,16 @@ private static Schema Fallible() return schema; } + /// + /// The two types that describe the absence of a value, by name, so a case can name one. + /// + private static BaseType Absent(string type) => type switch + { + "Void" => new Void(), + "None" => new None(), + _ => throw new ArgumentOutOfRangeException(nameof(type), type, "Not an absent type."), + }; + /// /// The four carriers that hold one element, by name, so one case covers all of them. ///