Skip to content

Commit d2acee7

Browse files
committed
Fix nullable payload flow after ThrowIfProblem
1 parent 3a21386 commit d2acee7

5 files changed

Lines changed: 151 additions & 6 deletions

File tree

‎Directory.Build.props‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
<RepositoryUrl>https://github.com/managedcode/Communication</RepositoryUrl>
2828
<PackageProjectUrl>https://github.com/managedcode/Communication</PackageProjectUrl>
2929
<Product>Managed Code - Communication</Product>
30-
<Version>10.3.15</Version>
31-
<PackageVersion>10.3.15</PackageVersion>
30+
<Version>10.3.16</Version>
31+
<PackageVersion>10.3.16</PackageVersion>
3232

3333
</PropertyGroup>
3434
<PropertyGroup Condition="'$(GITHUB_ACTIONS)' == 'true'">
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
using System;
2+
using System.IO;
3+
using System.Linq;
4+
using System.Reflection;
5+
using System.Runtime.Loader;
6+
using Microsoft.CodeAnalysis;
7+
using Microsoft.CodeAnalysis.CSharp;
8+
using Shouldly;
9+
10+
namespace ManagedCode.Communication.Tests.Results;
11+
12+
public sealed class ResultThrowIfProblemFlowTests
13+
{
14+
private const string AssemblyName = "CommunicationThrowIfProblemRegression";
15+
private const string TrustedAssembliesKey = "TRUSTED_PLATFORM_ASSEMBLIES";
16+
private const string EntryType = "Scenario";
17+
private const string EntryMethod = "Execute";
18+
private const string Consumer = """
19+
#nullable enable
20+
using ManagedCode.Communication;
21+
using ManagedCode.Communication.CollectionResultT;
22+
23+
public static class Scenario
24+
{
25+
private const string Payload = "payload";
26+
private const string FailureTitle = "The entry could not be read.";
27+
private const string FailureDetail = "The requested entry does not exist.";
28+
29+
private static int Read(Result<string> result)
30+
{
31+
result.ThrowIfProblem();
32+
return result.Value.Length;
33+
}
34+
35+
private static int ReadNullable(Result<string?> result)
36+
{
37+
result.ThrowIfProblem();
38+
return result.Value.Length;
39+
}
40+
41+
private static int ReadCollection(CollectionResult<string> result)
42+
{
43+
result.ThrowIfProblem();
44+
return result.Value.Length + result.Collection.Length;
45+
}
46+
47+
public static bool Execute()
48+
{
49+
if (Read(Result<string>.Succeed(Payload)) != Payload.Length ||
50+
ReadNullable(Result<string?>.Succeed(Payload)) != Payload.Length ||
51+
ReadCollection(CollectionResult<string>.Succeed([Payload])) != 2 ||
52+
ReadCollection(CollectionResult<string>.Succeed([])) != 0)
53+
{
54+
return false;
55+
}
56+
57+
var problem = Problem.Create(FailureTitle, FailureDetail);
58+
var failed = Result<string>.Fail(problem);
59+
try { _ = Read(failed); return false; }
60+
catch (ProblemException exception)
61+
{
62+
if (!ReferenceEquals(exception.Problem, problem)) { return false; }
63+
}
64+
65+
var nullable = Result<string?>.Fail(problem);
66+
try { _ = ReadNullable(nullable); return false; }
67+
catch (ProblemException exception)
68+
{
69+
if (!ReferenceEquals(exception.Problem, problem)) { return false; }
70+
}
71+
72+
var page = CollectionResult<string>.Fail(problem);
73+
try { _ = ReadCollection(page); return false; }
74+
catch (ProblemException exception)
75+
{
76+
if (!ReferenceEquals(exception.Problem, problem)) { return false; }
77+
}
78+
79+
var emptyPage = default(CollectionResult<string>);
80+
if (emptyPage.Value.Length != 0 || emptyPage.Collection.Length != 0) { return false; }
81+
try { _ = Read(default); return false; }
82+
catch (ProblemException exception)
83+
{
84+
if (exception.Problem is null) { return false; }
85+
}
86+
try { _ = ReadNullable(default); return false; }
87+
catch (ProblemException exception)
88+
{
89+
if (exception.Problem is null) { return false; }
90+
}
91+
try { _ = ReadCollection(emptyPage); return false; }
92+
catch (ProblemException exception)
93+
{
94+
return exception.Problem is not null;
95+
}
96+
}
97+
}
98+
""";
99+
100+
[Test]
101+
public void ThrowIfProblem_ConsumerCompilesWithoutNullableWarnings_ExecutesSuccessFailureAndDefault()
102+
{
103+
var references = ((string)AppContext.GetData(TrustedAssembliesKey)!).Split(Path.PathSeparator)
104+
.Append(typeof(Result).Assembly.Location).Distinct().Select(path => MetadataReference.CreateFromFile(path));
105+
var compilation = CSharpCompilation.Create(AssemblyName,
106+
[CSharpSyntaxTree.ParseText(Consumer)], references,
107+
new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary,
108+
nullableContextOptions: NullableContextOptions.Enable, generalDiagnosticOption: ReportDiagnostic.Error));
109+
using var image = new MemoryStream();
110+
var emitted = compilation.Emit(image);
111+
emitted.Success.ShouldBeTrue(string.Join(Environment.NewLine, emitted.Diagnostics));
112+
emitted.Diagnostics.ShouldBeEmpty();
113+
image.Position = 0;
114+
var context = new AssemblyLoadContext(AssemblyName, isCollectible: true);
115+
try
116+
{
117+
var assembly = context.LoadFromStream(image);
118+
assembly.GetType(EntryType)!.GetMethod(EntryMethod, BindingFlags.Public | BindingFlags.Static)!
119+
.Invoke(null, null).ShouldBe(true);
120+
}
121+
finally
122+
{
123+
context.Unload();
124+
}
125+
}
126+
}

‎ManagedCode.Communication/CollectionResultT/CollectionResult.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,10 @@ public T[] Collection
7979
}
8080

8181
/// <summary>
82-
/// Gets the collection as a Value property, for <see cref="IResult{T}" /> compatibility.
82+
/// Gets the non-null collection as a Value property, for <see cref="IResult{T}" /> compatibility.
8383
/// </summary>
8484
[JsonIgnore]
85-
public T[]? Value => Collection;
85+
public T[] Value => Collection;
8686

8787
/// <summary>
8888
/// 1-based index of this page.

‎ManagedCode.Communication/ResultT/ResultT.Exception.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,13 @@ public partial struct Result<T>
1919
/// <summary>
2020
/// Throws a ProblemException if the result has a problem.
2121
/// </summary>
22-
[MemberNotNullWhen(false, nameof(Value))]
22+
[MemberNotNull(nameof(Value))]
2323
public void ThrowIfProblem()
2424
{
25-
ResultProblemExtensions.ThrowIfProblem(this);
25+
if (IsFailed)
26+
{
27+
throw new ProblemException(Problem);
28+
}
2629
}
2730

2831
/// <summary>

‎README.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,22 @@ These guarantees concern the payload itself. A nullable property inside the payl
172172
inside a collection, still needs its own domain check. Keep the guard and value access on the same result;
173173
reassigning it or checking a different result does not establish the guarantee.
174174

175+
When the caller needs an exception instead of a failed result, use `ThrowIfProblem()`:
176+
177+
```csharp
178+
static int RequireTextLength(Result<string> result)
179+
{
180+
result.ThrowIfProblem();
181+
return result.Value.Length;
182+
}
183+
```
184+
185+
A failed result throws `ProblemException` carrying its original `Problem`. On normal return,
186+
`Result<T>.ThrowIfProblem()` establishes a non-null `Value`, including nullable type arguments;
187+
no additional null guard or null-forgiving operator is needed. Collection results expose their
188+
existing non-null item array through both `Collection` and `Value`, including an empty array
189+
when there are no items.
190+
175191
### Success without data
176192

177193
Use non-generic `Result` for acknowledgements, completed mutations, and deliberately ignored events:

0 commit comments

Comments
 (0)