fix: reject a null success value in GitResult.FromValue [minor]

GitIntegration.Test/Execution/GitResultTests.cs

@@ -65,6 +65,22 @@ public void FromErrorRejectsNullError()
 		_ = Assert.ThrowsExactly<ArgumentNullException>(() => GitResult<string>.FromError(null!));
 	}
 
+	[TestMethod]
+	public void FromValueRejectsNullValue()
+	{
+		_ = Assert.ThrowsExactly<ArgumentNullException>(() => GitResult<string>.FromValue(null!));
+	}
+
+	[TestMethod]
+	public void FromValueRejectsNullValueForAnyReferenceType()
+	{
+		// GitCommandBuilder<TResult> is public, unsealed, and puts no notnull constraint on TResult,
+		// so a derived builder whose ParseResult returns null on an edge case reaches FromValue with
+		// null for whatever reference type it closed over — not just string.
+		_ = Assert.ThrowsExactly<ArgumentNullException>(
+			() => GitResult<IReadOnlyList<string>>.FromValue(null!));
+	}
+
 	[TestMethod]
 	public void ProcessResultReportsSuccessOnZeroExitCode()
 	{

GitIntegration/Builders/GitCommandBuilder.cs

@@ -13,14 +13,19 @@ namespace ktsu.GitIntegration;
 /// The shared behaviour of every git command builder: global argument injection, execution, and
 /// failure translation.
 /// </summary>
-/// <typeparam name="TResult">The parsed result type.</typeparam>
+/// <typeparam name="TResult">
+/// The parsed result type. Constrained to <c>notnull</c> so that a derived builder whose
+/// <see cref="ParseResult"/> returns null on some edge case cannot yield a successful-looking
+/// <see cref="GitResult{T}"/> with a null value; <see cref="GitResult{T}.FromValue(T)"/> throws instead.
+/// </typeparam>
 /// <param name="runner">Runs the assembled command.</param>
 /// <param name="repositoryPath">
 /// The repository to scope the command to, or <see langword="null"/> for commands that are not
 /// repository-scoped, such as <c>init</c>, <c>clone</c>, and <c>--version</c>.
 /// </param>
 public abstract class GitCommandBuilder<TResult>(IGitProcessRunner runner, AbsoluteDirectoryPath? repositoryPath)
 	: IGitCommandBuilder<TResult>
+	where TResult : notnull
 {
 	/// <summary>
 	/// Gets the runner this builder executes through.

GitIntegration/Builders/IGitCommandBuilder.cs

@@ -16,8 +16,12 @@ namespace ktsu.GitIntegration;
 /// a fresh builder per command. The underlying <see cref="IGitProcessRunner"/> is the opposite: it
 /// is a shared singleton and safe to call concurrently.
 /// </remarks>
-/// <typeparam name="TResult">The parsed result type.</typeparam>
+/// <typeparam name="TResult">
+/// The parsed result type. Constrained to <c>notnull</c> because <see cref="GitResult{T}"/> reports
+/// success purely by the absence of an error, so it cannot represent a null success value.
+/// </typeparam>
 public interface IGitCommandBuilder<TResult>
+	where TResult : notnull
 {
 	/// <summary>
 	/// Gets the exact argument vector this builder will pass to git.

GitIntegration/Execution/GitResult.cs

@@ -24,6 +24,7 @@
 /// </summary>
 /// <typeparam name="T">The parsed result type on success.</typeparam>
 /// <remarks>
+/// <para>
 /// This is a sealed record class, not a struct, so that there is no <c>default</c> instance to
 /// accidentally observe. A struct with independently-set members would let
 /// <c>default(GitResult&lt;T&gt;)</c> — reachable via an uninitialized field, an array, or a failed
@@ -32,8 +33,19 @@
 /// reasonably dereferences the field matching the branch it is on. Restricting construction to
 /// <see cref="FromValue(T)"/> and <see cref="FromError(GitCommandError)"/> makes every instance one
 /// or the other, never both undefined.
+/// </para>
+/// <para>
+/// <typeparamref name="T"/> is constrained to <c>notnull</c>, and <see cref="FromValue(T)"/>
+/// rejects a null value the way <see cref="FromError(GitCommandError)"/> rejects a null error.
+/// Without both, <c>FromValue(null)</c> would report <see cref="Success"/> with nothing for the
+/// success branch to read — the one case the guarantee above does not survive. The constraint stops
+/// short of <c>class</c> because two result types are value types: <c>rev-list</c> counts commits
+/// into an <see cref="int"/>, and <c>Divergence()</c> answers with a
+/// <see cref="GitDivergence"/> struct.
+/// </para>
 /// </remarks>
 public sealed record GitResult<T>
+	where T : notnull
 {
 	private GitResult() { }
 
@@ -49,7 +61,29 @@
 	/// <summary>Creates a successful result.</summary>
 	/// <param name="value">The parsed result.</param>
 	/// <returns>A successful result carrying <paramref name="value"/>.</returns>
-	public static GitResult<T> FromValue(T value) => new() { Value = value };
+	/// <exception cref="ArgumentNullException">
+	/// <paramref name="value"/> is <see langword="null"/>. <see cref="Success"/> is derived from
+	/// <see cref="Error"/> alone, so a null value here would report success while leaving nothing for
+	/// the success branch to read — the one reading this type's remarks promise is safe.
+	/// </exception>
+	public static GitResult<T> FromValue(T value)
+	{
+		// The notnull constraint is a compile-time check a caller can suppress, and the extension
+		// point this guards — a derived GitCommandBuilder whose ParseResult returns null — is
+		// reachable from outside this assembly, so the invariant is enforced at run time too.
+		//
+		// Ensure.NotNull constrains its own argument to a reference type, which T is not: the
+		// constraint above stops at notnull for GitResult<int> and GitResult<GitDivergence>. Casting
+		// to object satisfies that constraint for the reference-type instantiations, where null is
+		// the only place it is reachable. typeof(T).IsValueType is a per-instantiation constant, so
+		// the value-type instantiations fold this check away instead of boxing on every result.
+		if (!typeof(T).IsValueType)
+		{
+			Ensure.NotNull((object?)value, nameof(value));
+		}
+
+		return new() { Value = value };
+	}
 
 	/// <summary>Creates a failed result.</summary>
 	/// <param name="error">The failure detail.</param>