config: more general purpose backwards compatibility handlers

Fixes #7706

We previously had a very specific backwards compatibility handler for
handling renamed fields. We always knew that wouldn't scale but I wanted
to wait for a real case. Well, #7706 is a real case, so here we are.

This commit makes our backwards compatibility handler more general
purpose, and makes a special-case handler for renamed fields built on
top of this same general purpose system. The new system lets us do a lot
more with regards to backwards compatibility.

To start, this addresses #7706 by allowing us to handle a removed single
enum value of a still-existing field.

1 files changed, 139 insertions, 51 deletions

diff --git a/src/cli/args.zig b/src/cli/args.zig
index 3c34e17fe..65e72636e 100644
--- a/src/cli/args.zig
+++ b/src/cli/args.zig
@@ -40,11 +40,14 @@ pub const Error = error{
 /// "DiagnosticList" and any diagnostic messages will be added to that list.
 /// When diagnostics are present, only allocation errors will be returned.
 ///
-/// If the destination type has a decl "renamed", it must be of type
-/// std.StaticStringMap([]const u8) and contains a mapping from the old
-/// field name to the new field name. This is used to allow renaming fields
-/// while still supporting the old name. If a renamed field is set, parsing
-/// will automatically set the new field name.
+/// If the destination type has a decl "compatibility", it must be of type
+/// std.StaticStringMap(CompatibilityHandler(T)), and it will be used to
+/// handle backwards compatibility for fields with the given name. The
+/// field name doesn't need to exist (so you can setup compatibility for
+/// removed fields). The value is a function that will be called when
+/// all other parsing fails for that field. If a field changes such that
+/// the old values would NOT error, then the caller should handle that
+/// downstream after parsing is done, not through this method.
 ///
 /// Note: If the arena is already non-null, then it will be used. In this
 /// case, in the case of an error some memory might be leaked into the arena.
@@ -57,24 +60,6 @@ pub fn parse(
     const info = @typeInfo(T);
     assert(info == .@"struct");
 
-    comptime {
-        // Verify all renamed fields are valid (source does not exist,
-        // destination does exist).
-        if (@hasDecl(T, "renamed")) {
-            for (T.renamed.keys(), T.renamed.values()) |key, value| {
-                if (@hasField(T, key)) {
-                    @compileLog(key);
-                    @compileError("renamed field source exists");
-                }
-
-                if (!@hasField(T, value)) {
-                    @compileLog(value);
-                    @compileError("renamed field destination does not exist");
-                }
-            }
-        }
-    }
-
     // Make an arena for all our allocations if we support it. Otherwise,
     // use an allocator that always fails. If the arena is already set on
     // the config, then we reuse that. See memory note in parse docs.
@@ -148,6 +133,16 @@ pub fn parse(
         };
 
         parseIntoField(T, arena_alloc, dst, key, value) catch |err| {
+            // If we get an error parsing a field, then we try to fall
+            // back to compatibility handlers if able.
+            if (@hasDecl(T, "compatibility")) {
+                // If we have a compatibility handler for this key, then
+                // we call it and see if it handles the error.
+                if (T.compatibility.get(key)) |handler| {
+                    if (handler(dst, arena_alloc, key, value)) return;
+                }
+            }
+
             if (comptime !canTrackDiags(T)) return err;
 
             // The error set is dependent on comptime T, so we always add
@@ -177,6 +172,58 @@ pub fn parse(
     }
 }
 
+/// The function type for a compatibility handler. The compatibility
+/// handler is documented in the `parse` function documentation.
+///
+/// The function type should return bool if the compatibility was
+/// handled, and false otherwise. If false is returned then the
+/// naturally occurring error will continue to be processed as if
+/// this compatibility handler was not present.
+///
+/// Compatibility handlers aren't allowed to return errors because
+/// they're generally only called in error cases, so we already have
+/// an error message to show users. If there is an error in handling
+/// the compatibility, then the handler should return false.
+pub fn CompatibilityHandler(comptime T: type) type {
+    return *const fn (
+        dst: *T,
+        alloc: Allocator,
+        key: []const u8,
+        value: ?[]const u8,
+    ) bool;
+}
+
+/// Convenience function to create a compatibility handler that
+/// renames a field from `from` to `to`.
+pub fn compatibilityRenamed(
+    comptime T: type,
+    comptime to: []const u8,
+) CompatibilityHandler(T) {
+    comptime assert(@hasField(T, to));
+
+    return (struct {
+        fn compat(
+            dst: *T,
+            alloc: Allocator,
+            key: []const u8,
+            value: ?[]const u8,
+        ) bool {
+            _ = key;
+
+            parseIntoField(T, alloc, dst, to, value) catch |err| {
+                log.warn("error parsing renamed field {s}: {}", .{
+                    to,
+                    err,
+                });
+
+                return false;
+            };
+
+            return true;
+        }
+    }).compat;
+}
+
 fn formatValueRequired(
     comptime T: type,
     arena_alloc: std.mem.Allocator,
@@ -401,16 +448,6 @@ pub fn parseIntoField(
         }
     }
 
-    // Unknown field, is the field renamed?
-    if (@hasDecl(T, "renamed")) {
-        for (T.renamed.keys(), T.renamed.values()) |old, new| {
-            if (mem.eql(u8, old, key)) {
-                try parseIntoField(T, alloc, dst, new, value);
-                return;
-            }
-        }
-    }
-
     return error.InvalidField;
 }
 
@@ -752,6 +789,75 @@ test "parse: diagnostic location" {
     }
 }
 
+test "parse: compatibility handler" {
+    const testing = std.testing;
+
+    var data: struct {
+        a: bool = false,
+        _arena: ?ArenaAllocator = null,
+
+        pub const compatibility: std.StaticStringMap(
+            CompatibilityHandler(@This()),
+        ) = .initComptime(&.{
+            .{ "a", compat },
+        });
+
+        fn compat(
+            self: *@This(),
+            alloc: Allocator,
+            key: []const u8,
+            value: ?[]const u8,
+        ) bool {
+            _ = alloc;
+            if (std.mem.eql(u8, key, "a")) {
+                if (value) |v| {
+                    if (mem.eql(u8, v, "yuh")) {
+                        self.a = true;
+                        return true;
+                    }
+                }
+            }
+
+            return false;
+        }
+    } = .{};
+    defer if (data._arena) |arena| arena.deinit();
+
+    var iter = try std.process.ArgIteratorGeneral(.{}).init(
+        testing.allocator,
+        "--a=yuh",
+    );
+    defer iter.deinit();
+    try parse(@TypeOf(data), testing.allocator, &data, &iter);
+    try testing.expect(data._arena != null);
+    try testing.expect(data.a);
+}
+
+test "parse: compatibility renamed" {
+    const testing = std.testing;
+
+    var data: struct {
+        a: bool = false,
+        _arena: ?ArenaAllocator = null,
+
+        pub const compatibility: std.StaticStringMap(
+            CompatibilityHandler(@This()),
+        ) = .initComptime(&.{
+            .{ "old", compatibilityRenamed(@This(), "a") },
+        });
+    } = .{};
+    defer if (data._arena) |arena| arena.deinit();
+
+    var iter = try std.process.ArgIteratorGeneral(.{}).init(
+        testing.allocator,
+        "--old=true",
+    );
+    defer iter.deinit();
+    try parse(@TypeOf(data), testing.allocator, &data, &iter);
+    try testing.expect(data._arena != null);
+    try testing.expect(data.a);
+}
+
 test "parseIntoField: ignore underscore-prefixed fields" {
     const testing = std.testing;
     var arena = ArenaAllocator.init(testing.allocator);
@@ -1176,24 +1282,6 @@ test "parseIntoField: tagged union missing tag" {
     );
 }
 
-test "parseIntoField: renamed field" {
-    const testing = std.testing;
-    var arena = ArenaAllocator.init(testing.allocator);
-    defer arena.deinit();
-    const alloc = arena.allocator();
-
-    var data: struct {
-        a: []const u8,
-
-        const renamed = std.StaticStringMap([]const u8).initComptime(&.{
-            .{ "old", "a" },
-        });
-    } = undefined;
-
-    try parseIntoField(@TypeOf(data), alloc, &data, "old", "42");
-    try testing.expectEqualStrings("42", data.a);
-}
-
 /// An iterator that considers its location to be CLI args. It
 /// iterates through an underlying iterator and increments a counter
 /// to track the current CLI arg index.