From 940a46d41f8cc6a92f2f8f93b108d556410e22f2 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <m@mitchellh.com>
Date: Thu, 17 Oct 2024 21:39:34 -0700
Subject: [PATCH] cli: positional arguments are invalid when parsing
 configuration

---
 src/cli/args.zig        | 109 ++++++++++++++++++++++++++--------------
 src/cli/diagnostics.zig |  15 ++++++
 2 files changed, 87 insertions(+), 37 deletions(-)

diff --git a/src/cli/args.zig b/src/cli/args.zig
index ab66ba4fb..3dcc08dac 100644
--- a/src/cli/args.zig
+++ b/src/cli/args.zig
@@ -93,46 +93,60 @@ pub fn parse(
             }
         }
 
-        if (mem.startsWith(u8, arg, "--")) {
-            var key: []const u8 = arg[2..];
-            const value: ?[]const u8 = value: {
-                // If the arg has "=" then the value is after the "=".
-                if (mem.indexOf(u8, key, "=")) |idx| {
-                    defer key = key[0..idx];
-                    break :value key[idx + 1 ..];
-                }
+        // If this doesn't start with "--" then it isn't a config
+        // flag. We don't support positional arguments or configuration
+        // values set with spaces so this is an error.
+        if (!mem.startsWith(u8, arg, "--")) {
+            if (comptime !canTrackDiags(T)) return Error.InvalidField;
 
-                break :value null;
-            };
+            // Add our diagnostic
+            try dst._diagnostics.append(arena_alloc, .{
+                .key = try arena_alloc.dupeZ(u8, arg),
+                .message = "invalid field",
+                .location = diags.Location.fromIter(iter),
+            });
 
-            parseIntoField(T, arena_alloc, dst, key, value) catch |err| {
-                if (comptime !canTrackDiags(T)) return err;
-
-                // The error set is dependent on comptime T, so we always add
-                // an extra error so we can have the "else" below.
-                const ErrSet = @TypeOf(err) || error{Unknown};
-                const message: [:0]const u8 = switch (@as(ErrSet, @errorCast(err))) {
-                    // OOM is not recoverable since we need to allocate to
-                    // track more error messages.
-                    error.OutOfMemory => return err,
-                    error.InvalidField => "unknown field",
-                    error.ValueRequired => "value required",
-                    error.InvalidValue => "invalid value",
-                    else => try std.fmt.allocPrintZ(
-                        arena_alloc,
-                        "unknown error {}",
-                        .{err},
-                    ),
-                };
-
-                // Add our diagnostic
-                try dst._diagnostics.append(arena_alloc, .{
-                    .key = try arena_alloc.dupeZ(u8, key),
-                    .message = message,
-                    .location = diags.Location.fromIter(iter),
-                });
-            };
+            continue;
         }
+
+        var key: []const u8 = arg[2..];
+        const value: ?[]const u8 = value: {
+            // If the arg has "=" then the value is after the "=".
+            if (mem.indexOf(u8, key, "=")) |idx| {
+                defer key = key[0..idx];
+                break :value key[idx + 1 ..];
+            }
+
+            break :value null;
+        };
+
+        parseIntoField(T, arena_alloc, dst, key, value) catch |err| {
+            if (comptime !canTrackDiags(T)) return err;
+
+            // The error set is dependent on comptime T, so we always add
+            // an extra error so we can have the "else" below.
+            const ErrSet = @TypeOf(err) || error{Unknown};
+            const message: [:0]const u8 = switch (@as(ErrSet, @errorCast(err))) {
+                // OOM is not recoverable since we need to allocate to
+                // track more error messages.
+                error.OutOfMemory => return err,
+                error.InvalidField => "unknown field",
+                error.ValueRequired => "value required",
+                error.InvalidValue => "invalid value",
+                else => try std.fmt.allocPrintZ(
+                    arena_alloc,
+                    "unknown error {}",
+                    .{err},
+                ),
+            };
+
+            // Add our diagnostic
+            try dst._diagnostics.append(arena_alloc, .{
+                .key = try arena_alloc.dupeZ(u8, key),
+                .message = message,
+                .location = diags.Location.fromIter(iter),
+            });
+        };
     }
 }
 
@@ -452,6 +466,27 @@ test "parse: empty value resets to default" {
     try testing.expect(!data.b);
 }
 
+test "parse: positional arguments are invalid" {
+    const testing = std.testing;
+
+    var data: struct {
+        a: u8 = 42,
+        _arena: ?ArenaAllocator = null,
+    } = .{};
+    defer if (data._arena) |arena| arena.deinit();
+
+    var iter = try std.process.ArgIteratorGeneral(.{}).init(
+        testing.allocator,
+        "--a=84 what",
+    );
+    defer iter.deinit();
+    try testing.expectError(
+        error.InvalidField,
+        parse(@TypeOf(data), testing.allocator, &data, &iter),
+    );
+    try testing.expectEqual(@as(u8, 84), data.a);
+}
+
 test "parse: diagnostic tracking" {
     const testing = std.testing;
 
diff --git a/src/cli/diagnostics.zig b/src/cli/diagnostics.zig
index 1e56379c8..e4d390c03 100644
--- a/src/cli/diagnostics.zig
+++ b/src/cli/diagnostics.zig
@@ -46,6 +46,8 @@ pub const Location = union(enum) {
         line: usize,
     },
 
+    pub const Key = @typeInfo(Location).Union.tag_type.?;
+
     pub fn fromIter(iter: anytype) Location {
         const Iter = t: {
             const T = @TypeOf(iter);
@@ -121,4 +123,17 @@ pub const DiagnosticList = struct {
     pub fn items(self: *const DiagnosticList) []const Diagnostic {
         return self.list.items;
     }
+
+    /// Returns true if there are any diagnostics for the given
+    /// location type.
+    pub fn containsLocation(
+        self: *const DiagnosticList,
+        location: Location.Key,
+    ) bool {
+        for (self.list.items) |diag| {
+            if (diag.location == location) return true;
+        }
+
+        return false;
+    }
 };