ser: Make sure skip and skip_ser_if don't decrement length twice

getty-zig · Feb 19, 2024 · e3f8894 · e3f8894
1 parent fc27249
commit e3f8894
Showing 1 changed file with 89 additions and 35 deletions.
diff --git a/src/ser/blocks/struct.zig b/src/ser/blocks/struct.zig
@@ -29,48 +29,44 @@ pub fn serialize(
     const attributes = comptime getAttributes(T, @TypeOf(serializer));
 
     // The number of fields that will be serialized.
-    //
-    // length is initially set to the number of fields in the struct, but is
-    // decremented for any field that has the "skip" attribute set.
-    var length: usize = comptime blk: {
-        var len: usize = fields.len;
-
-        if (attributes) |attrs| {
-            for (std.meta.fields(@TypeOf(attrs))) |field| {
-                const attr = @field(attrs, field.name);
-
-                const skipped = @hasField(@TypeOf(attr), "skip") and attr.skip;
-                if (skipped) {
-                    len -= 1;
-                    continue;
+    const length = length: {
+        var len: usize = 0;
+
+        inline for (fields) |field| {
+            const attrs = comptime blk: {
+                if (attributes) |attrs| {
+                    if (@hasField(@TypeOf(attrs), field.name)) {
+                        const a = @field(attrs, field.name);
+                        const A = @TypeOf(a);
+
+                        break :blk @as(?A, a);
+                    }
                 }
-            }
-        }
-
-        break :blk len;
-    };
 
-    inline for (fields) |field| {
-        const attrs = comptime blk: {
-            if (attributes) |attrs| {
-                if (@hasField(@TypeOf(attrs), field.name)) {
-                    const a = @field(attrs, field.name);
-                    const A = @TypeOf(a);
+                break :blk null;
+            };
 
-                    break :blk @as(?A, a);
+            skip: {
+                if (attrs) |a| {
+                    const skipped = @hasField(@TypeOf(a), "skip") and a.skip;
+                    if (skipped) {
+                        break :skip;
+                    }
+
+                    const skip_if = @hasField(@TypeOf(a), "skip_ser_if");
+                    if (skip_if and a.skip_ser_if(@field(value, field.name))) {
+                        break :skip;
+                    }
                 }
-            }
-
-            break :blk null;
-        };
 
-        if (attrs) |a| {
-            const skip_if = @hasField(@TypeOf(a), "skip_ser_if");
-            if (skip_if and a.skip_ser_if(@field(value, field.name))) {
-                length -= 1;
+                len += 1;
             }
         }
-    }
+
+        std.debug.assert(len <= fields.len);
+
+        break :length len;
+    };
 
     var s = try serializer.serializeStruct(@typeName(T), length);
     const st = s.structure();
@@ -248,6 +244,64 @@ test "serialize - struct, attributes (skip_ser_if)" {
     });
 }
 
+test "serialize - struct, attributes (skip, skip_ser_if)" {
+    const T = struct {
+        a: i32,
+        b: i32,
+        c: ?i32,
+        d: ?i32,
+        e: ?i32,
+        f: ?i32,
+
+        pub const @"getty.sb" = struct {
+            pub const attributes = .{
+                .a = .{
+                    .skip = true,
+                    .skip_ser_if = isNull,
+                },
+                .b = .{
+                    .skip = false,
+                    .skip_ser_if = isNull,
+                },
+                .c = .{
+                    .skip = true,
+                    .skip_ser_if = isNull,
+                },
+                .d = .{
+                    .skip = true,
+                    .skip_ser_if = isNull,
+                },
+                .e = .{
+                    .skip = false,
+                    .skip_ser_if = isNull,
+                },
+                .f = .{
+                    .skip = false,
+                    .skip_ser_if = isNull,
+                },
+            };
+        };
+    };
+    const v = T{
+        .a = 1,
+        .b = 2,
+        .c = null,
+        .d = 4,
+        .e = null,
+        .f = 6,
+    };
+
+    try t.run(null, serialize, v, &.{
+        .{ .Struct = .{ .name = @typeName(T), .len = 2 } },
+        .{ .String = "b" },
+        .{ .I32 = 2 },
+        .{ .String = "f" },
+        .{ .Some = {} },
+        .{ .I32 = 6 },
+        .{ .StructEnd = {} },
+    });
+}
+
 fn isNull(v: anytype) bool {
     return switch (@typeInfo(@TypeOf(v))) {
         .Null, .Optional => v == null,