Remove the null-forgiving operators

.buildvana/hooks/release/post-release.cs

@@ -3,6 +3,7 @@
 
 #:package Buildvana.Runtime
 
+using System.Diagnostics;
 using System.IO;
 using System.Text.RegularExpressions;
 using Buildvana.Runtime;
@@ -28,7 +29,9 @@
 // next file sits beside the one it replaces, and is checked in, so a missing one is a broken tree:
 // File.Copy throws, the hook exits non-zero, and the release fails naming the path. Skipping instead
 // would revert the configuration file at the release after.
-var nextConfigFile = Path.Combine(Path.GetDirectoryName(configFile)!, "buildvana.next.jsonc");
+var configDirectory = Path.GetDirectoryName(configFile);
+Debug.Assert(configDirectory is not null, "GetDirectoryName returns null only for a root, and a file path is not one.");
+var nextConfigFile = Path.Combine(configDirectory, "buildvana.next.jsonc");
 File.Copy(nextConfigFile, configFile, overwrite: true);
 
 // Same expression as SelfVersionService.SchemaUrlRegex in src/Buildvana.Tool, which `bv self-update` applies to

.claude/rules/common/csharp-style-guide.md

@@ -51,6 +51,40 @@ Semantic differences to keep in mind when converting:
 - List, property, and recursive patterns imply a null test. `propertyPath is [PropertyName, PackageId]` is `false` when `propertyPath` is null, where `propertyPath.Count == 2` would throw. That is usually an improvement, but it is a behavior change, not a pure rewrite.
 - Constant patterns never use the type's `==` operator. A non-null constant compares with `Equals`. `null` is a plain reference test, or a `HasValue` check for a nullable value type. With `float` and `double` the pattern is what you want: `x is double.NaN` matches NaN, while `x == double.NaN` is always `false`. Exception: when a type overloads `==` so that `x == null` and `x is null` disagree, keep the operator form.
 
+## The null-forgiving operator
+
+Do not write the null-forgiving operator, `!`. The operator tells the compiler that a value is not null, and the compiler stops checking it. The fact behind the operator then lives in no type, and a reader has to rebuild it from code elsewhere. `Nullable.Extended.Analyzer` reports every operator as NX0001 to NX0004, and every warning is an error here.
+
+Remove the cause of the operator instead. Try these fixes in order:
+
+1. Let a type state the fact. Make the member non-nullable, or return a type that carries the non-null value next to the rest. A method that filters items on a nullable member returns each item with its non-null value.
+2. Keep a value the code built. Do not read it back through a nullable API.
+3. Write a check that flow analysis follows: a pattern such as `is { } value` or `is JsonObject obj`, or a call to a `[NotNullWhen]` method inside the condition. Flow analysis does not follow a `bool` local that records the outcome of a check.
+4. When no type and no check removes the operator, state the fact with `Debug.Assert(value is not null, "<reason>")`. The message says why the value cannot be null. Store the value in a local first, when it has none.
+
+Do not suppress the analyzer with its justification comment, a `// !` line above the statement. The comment states the fact where no tool checks it. `Debug.Assert` checks it in every Debug build, tests included.
+
+### In tests
+
+A test checks the null with the test framework, so that a null fails the test instead of throwing:
+
+- `await Assert.That(value).IsNotNull()` returns the value, non-null.
+- A comparison through `?.` fails on a null: `await Assert.That(slash["type"]?.GetValue<string>()).IsEqualTo("string")`.
+- A test that asserts a value reached through `?.` is null first asserts the parent non-null. Otherwise a missing parent passes the test.
+- Chain `WithMessage` or `WithMessageContaining` onto `Throws<T>()`, instead of reading `Message` from the exception it returns.
+- To read another member of the exception, use `Assert.Throws<T>(Action)` for a synchronous act, which returns the exception non-null. After an awaited `Throws<T>()`, write `Debug.Assert(exception is not null, "Throws returns null only inside Assert.Multiple.")`.
+
+A synchronous helper cannot await an assertion. When the null comes from the code under test, the helper throws `InvalidOperationException`. When the fact does not depend on the code under test, such as `Path.GetDirectoryName` of a file path, the helper uses `Debug.Assert`. Keep `Debug.Assert` for such facts, because a failed `Debug.Assert` ends the test process through `Environment.FailFast`.
+
+One case keeps the operator. When a test passes `null!` or `default!` on purpose, as the input under test, put this comment on the line above the statement:
+
+```csharp
+// ! Null on purpose: testing null detection
+static GitignorePattern? Act() => GitignorePattern.TryParse(null!);
+```
+
+Use this text and no other, so that a search finds every such input.
+
 ## Partial classes
 
 When a class exceeds 500 lines of code, or has nested types, split it into partial class files.

CHANGELOG.md

@@ -152,6 +152,7 @@ The patch number restarted from 1, below the 2.0.x versions already published.
 - `bv release` fails when the remote rejects the [push of its commits](docs/tool-commands/release.md#the-release-tag), where it used to go on as if the push had succeeded.
 - `bv release` reports a failed fetch or [push](docs/tool-commands/release.md#the-release-tag) as one error line naming the release remote, not as an unhandled exception.
 - When a `PackageVersion` item versions a package Buildvana SDK injects, the reference [is an ordinary `PackageReference`](docs/sdk-modules/standard-analyzers.md#package-references), where NuGet rejected the item with NU1009.
+- `bv` no longer fails with `ArgumentException` when two [version tags](docs/tool-commands/version.md#bv-version-show) name the same commit.
 
 ### Known problems introduced by this release
 

Common.props

@@ -39,9 +39,6 @@
     <IsPackable>false</IsPackable>
     <GenerateDocumentationFile>false</GenerateDocumentationFile> <!--Do not generate XML documentation -->
     <UsePublicApiAnalyzers>false</UsePublicApiAnalyzers>
-
-    <!-- The null-forgiving operators of this repository still lack a "// !" justification comment. -->
-    <UseNullableExtendedAnalyzer>false</UseNullableExtendedAnalyzer>
   </PropertyGroup>
 
   <!-- Target frameworks -->

docs/tool-commands/version.md

@@ -41,6 +41,7 @@ Current branch:        main
 - The latest version is the version of the first tag, walking the history from `HEAD`, whose name is a semantic version.
 - The latest stable version is the version of the first such tag with no prerelease part.
   Both lines read `(none)` when there is no such tag.
+- When one commit carries several such tags, the highest version among them counts.
 - The public release line says whether the current branch [produces public releases](../sdk-modules/versioning.md#public-releases).
 - The prerelease line says whether `VERSION` marks a prerelease line.
 - The current branch is the short name of the branch, or `(detached HEAD)`.

src/Buildvana.Core.Configuration/BuildvanaJsonConfigExample.cs

@@ -166,6 +166,7 @@ internal static IReadOnlyList<string> WrapDescription(string description)
     /// <param name="root">The schema document the pointer is relative to.</param>
     /// <param name="path">The setting the reference was found at, for the error message.</param>
     /// <returns>The subschema the pointer names.</returns>
+    /// <exception cref="ArgumentException"><paramref name="schema"/> carries no <c>$ref</c> keyword.</exception>
     /// <exception cref="InvalidOperationException">The pointer does not name a subschema.</exception>
     /// <remarks>
     /// <para>A document-local pointer is the only kind the exporter emits, for a member type that occurs more
@@ -177,7 +178,8 @@ internal static JsonObject ResolveReference(JsonObject schema, JsonObject root,
         ArgumentNullException.ThrowIfNull(schema);
         ArgumentNullException.ThrowIfNull(root);
 
-        var pointer = schema["$ref"]!.GetValue<string>();
+        var pointer = schema["$ref"]?.GetValue<string>()
+            ?? throw new ArgumentException("The schema carries no \"$ref\" keyword.", nameof(schema));
         JsonNode node = root;
         foreach (var token in pointer.Split('/').Skip(1))
         {
@@ -310,7 +312,12 @@ private static void WriteValue(StringBuilder text, JsonObject schema, JsonObject
     private static List<(string Name, JsonObject Schema)> CollectMembers(JsonObject schema)
     {
         List<(string Name, JsonObject Schema)> members = [];
-        foreach (var (name, node) in (JsonObject)schema["properties"]!)
+        if (schema["properties"] is not JsonObject properties)
+        {
+            return members;
+        }
+
+        foreach (var (name, node) in properties)
         {
             if (node is JsonObject member)
             {

src/Buildvana.Core.Configuration/BuildvanaJsonConfigProvider.cs

@@ -105,11 +105,11 @@ public static BuildvanaJsonConfig LoadFile(string? path)
 
         var node = Parse(json, path);
 
-        // Parse throws unless the document is well-formed JSON, and well-formed JSON always makes a map.
-        Validate(node, sourceMap!, path);
+        Debug.Assert(sourceMap is not null, "Parse throws unless the document is well-formed JSON, which always makes a map.");
+        Validate(node, sourceMap, path);
 
         // Validation guarantees a non-null object at the root, so deserialization cannot return null here.
-        return node!.Deserialize(BuildvanaJsonConfigContext.Default.BuildvanaJsonConfig) ?? new BuildvanaJsonConfig();
+        return node.Deserialize(BuildvanaJsonConfigContext.Default.BuildvanaJsonConfig) ?? new BuildvanaJsonConfig();
     }
 
     // The one probe for the configuration file: nothing outside this class asks which file a home directory

src/Buildvana.Core.IO/Gitignore/GitignorePattern-matching.cs

@@ -134,10 +134,10 @@ private static bool MatchTokens(
         return tokenIndex == tokens.Length;
     }
 
-    private static bool MatchesChar(GitignoreToken token, char c, bool ignoreCase) => token.Kind switch {
-        GitignoreTokenKind.Literal => CharsEqual(token.Value, c, ignoreCase),
-        GitignoreTokenKind.AnyChar => true,
-        GitignoreTokenKind.CharClass => token.CharClass!.Matches(c, ignoreCase),
+    private static bool MatchesChar(GitignoreToken token, char c, bool ignoreCase) => token switch {
+        { Kind: GitignoreTokenKind.Literal } => CharsEqual(token.Value, c, ignoreCase),
+        { Kind: GitignoreTokenKind.AnyChar } => true,
+        { Kind: GitignoreTokenKind.CharClass, CharClass: { } charClass } => charClass.Matches(c, ignoreCase),
         _ => false, // AnyRun is consumed by MatchTokens and never reaches here.
     };
 

src/Buildvana.Core.Json/JsonHelper-InsertProperty.cs

@@ -111,11 +111,12 @@ private static bool TryFindParentObject(ref Utf8JsonReader reader, IReadOnlyList
                     break;
 
                 case JsonTokenType.StartObject or JsonTokenType.StartArray:
-                    var pushed = pendingProperty is not null;
-                    if (pushed)
+                    var pushed = false;
+                    if (pendingProperty is { } property)
                     {
-                        pathSegments.Add(pendingProperty!);
+                        pathSegments.Add(property);
                         pendingProperty = null;
+                        pushed = true;
                     }
 
                     containerPushedSegment.Push(pushed);

src/Buildvana.Core.Json/JsonHelper.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.IO;
 using System.Text.Encodings.Web;
 using System.Text.Json;
@@ -281,7 +282,8 @@ private static void TryRecordStringEdit(
         string propertyName,
         List<JsonValueEdit> edits)
     {
-        var currentValue = reader.GetString()!;
+        var currentValue = reader.GetString();
+        Debug.Assert(currentValue is not null, "GetString returns null only for a null token, and the reader is on a string.");
         pathSegments.Add(propertyName);
         var newValue = rewriter(pathSegments, currentValue);
         pathSegments.RemoveAt(pathSegments.Count - 1);

src/Buildvana.Core.Json/JsonKeyedObjectConverter.ListConverter`1.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Buffers;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Text.Json;
 using System.Text.Json.Serialization;
@@ -51,7 +52,8 @@ public override IReadOnlyList<T> Read(
                 var entries = new List<(string Key, JsonElement Value)>();
                 while (reader.Read() && reader.TokenType != JsonTokenType.EndObject)
                 {
-                    var key = reader.GetString()!;
+                    var key = reader.GetString();
+                    Debug.Assert(key is not null, "GetString returns null only for a null token, and the reader is on a property name.");
                     _ = reader.Read();
                     var document = JsonDocument.ParseValue(ref reader);
                     documents.Add(document);
@@ -88,7 +90,9 @@ public override IReadOnlyList<T> Read(
                     buffer.ResetWrittenCount();
                     elementWriter.Reset(buffer);
                     WriteElementJson(elementWriter, key, value);
-                    result.Add(JsonSerializer.Deserialize(buffer.WrittenSpan, typeInfo)!);
+                    var element = JsonSerializer.Deserialize(buffer.WrittenSpan, typeInfo);
+                    Debug.Assert(element is not null, "WriteElementJson writes an object, and an object never deserializes as null.");
+                    result.Add(element);
                 }
 
                 return result;
@@ -123,11 +127,13 @@ public override void Write(Utf8JsonWriter writer, IReadOnlyList<T> value, JsonSe
 
         private void WriteElementJson(Utf8JsonWriter writer, string key, JsonElement value)
         {
+            Debug.Assert(_keyJsonName is not null, "GetElementTypeInfo sets the key name.");
             writer.WriteStartObject();
-            writer.WriteString(_keyJsonName!, key);
+            writer.WriteString(_keyJsonName, key);
             if (valuePropertyName is not null)
             {
-                writer.WritePropertyName(_valueJsonName!);
+                Debug.Assert(_valueJsonName is not null, "GetElementTypeInfo sets the value name for a value property.");
+                writer.WritePropertyName(_valueJsonName);
                 value.WriteTo(writer);
             }
             else
@@ -157,14 +163,17 @@ private void WriteElementJson(Utf8JsonWriter writer, string key, JsonElement val
 
         private void WriteElement(Utf8JsonWriter writer, JsonElement element, HashSet<string> writtenKeys)
         {
-            if (!element.TryGetProperty(_keyJsonName!, out var keyElement) || keyElement.ValueKind != JsonValueKind.String)
+            Debug.Assert(_keyJsonName is not null, "GetElementTypeInfo sets the key name.");
+            var key = element.TryGetProperty(_keyJsonName, out var keyElement) && keyElement.ValueKind == JsonValueKind.String
+                ? keyElement.GetString()
+                : null;
+            if (key is null)
             {
                 throw new JsonException($"The key property '{_keyJsonName}' must serialize as a non-null string.");
             }
 
             // Utf8JsonWriter validates structure only, so without this check two elements sharing a key would
             // produce a document this converter's own Read refuses or silently collapses.
-            var key = keyElement.GetString()!;
             if (!writtenKeys.Add(key))
             {
                 throw new JsonException($"Duplicate key '{key}'.");
@@ -192,7 +201,8 @@ private void WriteElement(Utf8JsonWriter writer, JsonElement element, HashSet<st
 
         private void WriteValueProperty(Utf8JsonWriter writer, JsonElement element)
         {
-            if (element.TryGetProperty(_valueJsonName!, out var valueElement))
+            Debug.Assert(_valueJsonName is not null, "GetElementTypeInfo sets the value name for a value property.");
+            if (element.TryGetProperty(_valueJsonName, out var valueElement))
             {
                 valueElement.WriteTo(writer);
             }

src/Buildvana.Core.Json/JsonKeyedObjectConverter.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Reflection;
 using System.Text.Json;
 using System.Text.Json.Serialization;
@@ -32,23 +33,27 @@ public override bool CanConvert(Type typeToConvert)
     /// <inheritdoc/>
     public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
     {
-        var elementType = TryGetElementType(typeToConvert)!;
-        var attribute = elementType.GetCustomAttribute<JsonKeyedObjectAttribute>(inherit: false)!;
+        var keyedElementType = TryGetKeyedElementType(typeToConvert);
+        Debug.Assert(keyedElementType is not null, "The serializer calls CreateConverter only after CanConvert returns true.");
+        var (elementType, attribute) = keyedElementType.Value;
 
         // The attribute's CLR property names are resolved to JSON names by the converter itself, at first use,
         // from the element's JsonTypeInfo: the serializer's own metadata is right by construction under any
         // naming policy, TypeInfoResolver, or contract customization. Recomputing the names here from
         // reflection would silently disagree the moment a contract modifier renames a property.
         var converterType = typeof(ListConverter<>).MakeGenericType(elementType);
-        return (JsonConverter)Activator.CreateInstance(converterType, attribute.KeyPropertyName, attribute.ValuePropertyName)!;
+        var converter = (JsonConverter?)Activator.CreateInstance(converterType, attribute.KeyPropertyName, attribute.ValuePropertyName);
+        Debug.Assert(converter is not null, "Activator.CreateInstance returns null only for a nullable value type.");
+        return converter;
     }
 
     // Also serves JsonSchemaGenerator, which renders a keyed list from the same detection this factory
     // converts it by, so the parser and the schema cannot drift apart.
-    internal static Type? TryGetKeyedElementType(Type type)
-        => TryGetElementType(type) is { } elementType && elementType.IsDefined(typeof(JsonKeyedObjectAttribute), inherit: false)
-            ? elementType
-            : null;
+    internal static (Type ElementType, JsonKeyedObjectAttribute Attribute)? TryGetKeyedElementType(Type type)
+        => TryGetElementType(type) is { } elementType
+            && elementType.GetCustomAttribute<JsonKeyedObjectAttribute>(inherit: false) is { } attribute
+                ? (elementType, attribute)
+                : null;
 
     // Resolves the attribute's CLR property names to the JSON names the serializer will actually use, and
     // refuses the models the converter cannot read: a non-string key property, and key and value resolving to

src/Buildvana.Core.Json/Schema/JsonSchemaGenerator-KeyedObjects.cs

@@ -18,6 +18,7 @@ partial class JsonSchemaGenerator
     // is synthesized here from the element type's own schema.
     private static JsonObject CreateKeyedObjectSchema(
         Type elementType,
+        JsonKeyedObjectAttribute attribute,
         JsonSerializerOptions options,
         NullabilityInfoContext nullabilityContext,
         HashSet<Type> keyedTypesInProgress)
@@ -33,7 +34,6 @@ private static JsonObject CreateKeyedObjectSchema(
 
         try
         {
-            var attribute = elementType.GetCustomAttribute<JsonKeyedObjectAttribute>(inherit: false)!;
             var (keyJsonName, valueJsonName) = JsonKeyedObjectConverter.ResolveKeyedNames(
                 options.GetTypeInfo(elementType),
                 attribute.KeyPropertyName,

src/Buildvana.Core.Json/Schema/JsonSchemaGenerator.cs

@@ -157,10 +157,11 @@ private static JsonNode TransformSchemaNode(
         var keyedElementType = context.TypeInfo.Kind is JsonTypeInfoKind.None
             ? JsonKeyedObjectConverter.TryGetKeyedElementType(context.TypeInfo.Type)
             : null;
-        if (keyedElementType is not null)
+        if (keyedElementType is var (elementType, attribute))
         {
             var keyedSchema = CreateKeyedObjectSchema(
-                keyedElementType,
+                elementType,
+                attribute,
                 context.TypeInfo.Options,
                 nullabilityContext,
                 keyedTypesInProgress);
@@ -477,9 +478,9 @@ private static void RemoveNullFromType(JsonObject schema)
             }
         }
 
-        if (typeArray.Count == 1)
+        if (typeArray is [{ } single])
         {
-            schema["type"] = typeArray[0]!.GetValue<string>();
+            schema["type"] = single.GetValue<string>();
         }
     }