Skip to content

Commit 351bfe2

Browse files
Address code review feedback: Add documentation and null validation
- Add XML documentation comments for Union struct fields - Add null validation for property names - Replace null-forgiving operators with defensive null handling - Use `?? string.Empty` for jsonrpc property - All 1035 tests still pass Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
1 parent bc53b01 commit 351bfe2

1 file changed

Lines changed: 17 additions & 2 deletions

File tree

src/ModelContextProtocol.Core/Protocol/JsonRpcMessage.cs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,25 @@ public sealed class Converter : JsonConverter<JsonRpcMessage>
7676
/// </summary>
7777
private struct Union
7878
{
79+
/// <summary>The JSON-RPC protocol version (must be "2.0").</summary>
7980
public string JsonRpc;
81+
/// <summary>The message identifier for requests and responses.</summary>
8082
public RequestId Id;
83+
/// <summary>The method name for requests and notifications.</summary>
8184
public string? Method;
85+
/// <summary>The parameters for requests and notifications.</summary>
8286
public JsonNode? Params;
87+
/// <summary>The error details for error responses.</summary>
8388
public JsonRpcErrorDetail? Error;
89+
/// <summary>The result for successful responses.</summary>
8490
public JsonNode? Result;
91+
/// <summary>Indicates whether an 'id' property was present.</summary>
8592
public bool HasId;
93+
/// <summary>Indicates whether a 'method' property was present.</summary>
8694
public bool HasMethod;
95+
/// <summary>Indicates whether an 'error' property was present.</summary>
8796
public bool HasError;
97+
/// <summary>Indicates whether a 'result' property was present.</summary>
8898
public bool HasResult;
8999

90100
/// <summary>
@@ -111,13 +121,18 @@ public static Union Parse(ref Utf8JsonReader reader, JsonSerializerOptions optio
111121
throw new JsonException("Expected PropertyName token");
112122
}
113123

114-
string propertyName = reader.GetString()!;
124+
string? propertyName = reader.GetString();
125+
if (propertyName is null)
126+
{
127+
throw new JsonException("Property name cannot be null");
128+
}
129+
115130
reader.Read();
116131

117132
switch (propertyName)
118133
{
119134
case "jsonrpc":
120-
union.JsonRpc = reader.GetString()!;
135+
union.JsonRpc = reader.GetString() ?? string.Empty;
121136
break;
122137

123138
case "id":

0 commit comments

Comments
 (0)