From 6e34e83933aaa1120b7d0049f458608fdd6fa27b Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Sun, 1 Feb 2026 16:22:52 -0800 Subject: [PATCH] do not emit null optional fields --- codegen/src/serialization/json.zig | 25 +++++++++++++++++++------ src/aws.zig | 30 ++++++++++++++++++++++++++---- src/aws_test.zig | 27 ++++++++++++++++++++------- 3 files changed, 65 insertions(+), 17 deletions(-) diff --git a/codegen/src/serialization/json.zig b/codegen/src/serialization/json.zig index 02d5b1c..499e850 100644 --- a/codegen/src/serialization/json.zig +++ b/codegen/src/serialization/json.zig @@ -34,9 +34,9 @@ pub fn generateToJsonFunction(shape_id: []const u8, writer: *std.Io.Writer, stat const member_value = try getMemberValueJson(allocator, "self", member); defer allocator.free(member_value); - try writer.print("try jw.objectField(\"{s}\");\n", .{member.json_key}); try writeMemberJson( .{ + .object_field_name = member.json_key, .shape_id = member.target, .field_name = member.field_name, .field_value = member_value, @@ -146,6 +146,8 @@ fn writeMemberValue( } const WriteMemberJsonParams = struct { + object_field_name: []const u8, + quote_object_field_name: bool = true, shape_id: []const u8, field_name: []const u8, field_value: []const u8, @@ -196,9 +198,9 @@ fn writeStructureJson(params: WriteMemberJsonParams, writer: *std.Io.Writer) !vo const member_value = try getMemberValueJson(allocator, object_value, member); defer allocator.free(member_value); - try writer.print("try jw.objectField(\"{s}\");\n", .{member.json_key}); try writeMemberJson( .{ + .object_field_name = member.json_key, .shape_id = member.target, .field_name = member.field_name, .field_value = member_value, @@ -214,7 +216,7 @@ fn writeStructureJson(params: WriteMemberJsonParams, writer: *std.Io.Writer) !vo if (is_optional) { try writer.writeAll("} else {\n"); - try writer.writeAll("try jw.write(null);\n"); + try writer.writeAll("//try jw.write(null);\n"); try writer.writeAll("}\n"); } } @@ -268,7 +270,7 @@ fn writeListJson(list: smithy_tools.ListShape, params: WriteMemberJsonParams, wr if (list_is_optional) { try writer.writeAll("} else {\n"); - try writer.writeAll("try jw.write(null);\n"); + try writer.writeAll("//try jw.write(null);\n"); try writer.writeAll("}\n"); } } @@ -327,9 +329,10 @@ fn writeMapJson(map: smithy_tools.MapShape, params: WriteMemberJsonParams, write // start loop try writer.print("for ({s}) |{s}|", .{ map_value, map_value_capture }); try writer.writeAll("{\n"); - try writer.print("try jw.objectField({s});\n", .{map_capture_key}); try writeMemberJson(.{ + .object_field_name = map_capture_key, + .quote_object_field_name = false, .shape_id = map.value, .field_name = "value", .field_value = map_capture_value, @@ -345,7 +348,7 @@ fn writeMapJson(map: smithy_tools.MapShape, params: WriteMemberJsonParams, write if (map_is_optional) { try writer.writeAll("} else {\n"); - try writer.writeAll("try jw.write(null);\n"); + try writer.writeAll("//try jw.write(null);\n"); try writer.writeAll("}\n"); } } @@ -361,7 +364,16 @@ fn writeMemberJson(params: WriteMemberJsonParams, writer: *std.Io.Writer) anyerr const shape_info = try smithy_tools.getShapeInfo(shape_id, state.file_state.shapes); const shape = shape_info.shape; + const quote = if (params.quote_object_field_name) "\"" else ""; + const is_optional = smithy_tools.shapeIsOptional(params.member.traits); + if (is_optional) { + try writer.print("if ({s}) |_|\n", .{params.field_value}); + try writer.writeAll("{\n"); + } + try writer.print("try jw.objectField({s}{s}{s});\n", .{ quote, params.object_field_name, quote }); + if (state.getTypeRecurrenceCount(shape_id) > 2) { + if (is_optional) try writer.writeAll("\n}\n"); return; } @@ -389,4 +401,5 @@ fn writeMemberJson(params: WriteMemberJsonParams, writer: *std.Io.Writer) anyerr .short => try writeScalarJson("short", params, writer), .service, .resource, .operation, .member, .set => std.debug.panic("Shape type not supported: {}", .{shape}), } + if (is_optional) try writer.writeAll("\n}\n"); } diff --git a/src/aws.zig b/src/aws.zig index f764cce..cf318e2 100644 --- a/src/aws.zig +++ b/src/aws.zig @@ -231,8 +231,18 @@ pub fn Request(comptime request_action: anytype) type { var buffer = std.Io.Writer.Allocating.init(options.client.allocator); defer buffer.deinit(); if (Self.service_meta.aws_protocol == .rest_json_1) { - if (std.mem.eql(u8, "PUT", aws_request.method) or std.mem.eql(u8, "POST", aws_request.method)) - try buffer.writer.print("{f}", .{std.json.fmt(request, .{ .whitespace = .indent_4 })}); + if (std.mem.eql(u8, "PUT", aws_request.method) or std.mem.eql(u8, "POST", aws_request.method)) { + // Buried in the tests are our answer here: + // https://github.com/smithy-lang/smithy/blob/main/smithy-aws-protocol-tests/model/restJson1/json-structs.smithy#L71C24-L71C78 + // documentation: "Rest Json should not serialize null structure values", + try buffer.writer.print( + "{f}", + .{std.json.fmt(request, .{ + .whitespace = .indent_4, + .emit_null_optional_fields = false, + })}, + ); + } } aws_request.body = buffer.written(); var rest_xml_body: ?[]const u8 = null; @@ -320,11 +330,20 @@ pub fn Request(comptime request_action: anytype) type { // smithy spec, "A null value MAY be provided or omitted // for a boxed member with no observable difference." But we're // seeing a lot of differences here between spec and reality + // + // This is deliciously unclear: + // https://github.com/smithy-lang/smithy/blob/main/smithy-aws-protocol-tests/model/awsJson1_1/null.smithy#L36 + // + // It looks like struct nulls are meant to be dropped, but sparse + // lists/maps included. We'll err here on the side of eliminating them const body = try std.fmt.allocPrint( options.client.allocator, "{f}", - .{std.json.fmt(request, .{ .whitespace = .indent_4 })}, + .{std.json.fmt(request, .{ + .whitespace = .indent_4, + .emit_null_optional_fields = false, + })}, ); defer options.client.allocator.free(body); @@ -1193,7 +1212,10 @@ fn buildPath( "{f}", .{std.json.fmt( @field(request, field.name), - .{ .whitespace = .indent_4 }, + .{ + .whitespace = .indent_4, + .emit_null_optional_fields = false, + }, )}, ); const trimmed_replacement_val = std.mem.trim(u8, replacement_buffer.written(), "\""); diff --git a/src/aws_test.zig b/src/aws_test.zig index 3e92483..a05967e 100644 --- a/src/aws_test.zig +++ b/src/aws_test.zig @@ -129,7 +129,7 @@ test "proper serialization for kms" { const parsed_body = try std.json.parseFromSlice(struct { KeyId: []const u8, Plaintext: []const u8, - EncryptionContext: ?struct {}, + EncryptionContext: ?struct {} = null, GrantTokens: [][]const u8, EncryptionAlgorithm: []const u8, DryRun: bool, @@ -166,7 +166,6 @@ test "basic json request serialization" { try buffer.writer.print("{f}", .{std.json.fmt(request, .{ .whitespace = .indent_4 })}); try std.testing.expectEqualStrings( \\{ - \\ "ExclusiveStartTableName": null, \\ "Limit": 1 \\} , buffer.written()); @@ -632,7 +631,7 @@ test "json_1_0_query_with_input: dynamodb listTables runtime" { try req_actuals.expectHeader("X-Amz-Target", "DynamoDB_20120810.ListTables"); const parsed_body = try std.json.parseFromSlice(struct { - ExclusiveStartTableName: ?[]const u8, + ExclusiveStartTableName: ?[]const u8 = null, Limit: u8, }, std.testing.allocator, req_actuals.body.?, .{}); defer parsed_body.deinit(); @@ -701,7 +700,7 @@ test "json_1_1_query_with_input: ecs listClusters runtime" { try req_actuals.expectHeader("X-Amz-Target", "AmazonEC2ContainerServiceV20141113.ListClusters"); const parsed_body = try std.json.parseFromSlice(struct { - nextToken: ?[]const u8, + nextToken: ?[]const u8 = null, maxResults: u8, }, std.testing.allocator, req_actuals.body.?, .{}); defer parsed_body.deinit(); @@ -741,8 +740,8 @@ test "json_1_1_query_no_input: ecs listClusters runtime" { try req_actuals.expectHeader("X-Amz-Target", "AmazonEC2ContainerServiceV20141113.ListClusters"); const parsed_body = try std.json.parseFromSlice(struct { - nextToken: ?[]const u8, - maxResults: ?u8, + nextToken: ?[]const u8 = null, + maxResults: ?u8 = null, }, std.testing.allocator, req_actuals.body.?, .{}); defer parsed_body.deinit(); @@ -1250,6 +1249,20 @@ test "jsonStringify" { try std.testing.expectEqualStrings("1234", json_parsed.value.arn); try std.testing.expectEqualStrings("bar", json_parsed.value.tags.foo); } +test "jsonStringify does not emit null values on serialization" { + { + const lambda = (Services(.{.lambda}){}).lambda; + const request = lambda.CreateFunctionRequest{ + .function_name = "foo", + .role = "bar", + .code = .{}, + }; + + const request_json = try std.fmt.allocPrint(std.testing.allocator, "{f}", .{std.json.fmt(request, .{})}); + defer std.testing.allocator.free(request_json); + try std.testing.expect(std.mem.indexOf(u8, request_json, "null") == null); + } +} test "jsonStringify nullable object" { // structure is not null @@ -1272,7 +1285,7 @@ test "jsonStringify nullable object" { FunctionVersion: []const u8, Name: []const u8, RoutingConfig: struct { - AdditionalVersionWeights: ?struct {}, + AdditionalVersionWeights: ?struct {} = null, }, }, std.testing.allocator, request_json, .{ .ignore_unknown_fields = true }); defer json_parsed.deinit();