From d048ea5201a9e5799a7828b6d7ae93e28e6c92a0 Mon Sep 17 00:00:00 2001
From: Kathryn Baldauf <k_baldauf@apple.com>
Date: Tue, 5 Aug 2025 16:33:07 -0700
Subject: [PATCH] Native builder: remove option token in favor of string
 literals (#450)

Remove the option token from the dockerfile tokenizer for the native
builder. This cleans up some of the logic around handling options
depending on if they're instruction options or user provided options to
a command.

Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
---
 .../Docker/DockerfileParser.swift             | 82 ++++++++++++++-----
 .../Docker/DockerfileTokenizer.swift          | 24 ------
 .../ContainerBuildParser/Types.swift          | 13 ---
 .../TokenizerTests.swift                      | 64 ++++++++-------
 4 files changed, 95 insertions(+), 88 deletions(-)

diff --git a/Sources/NativeBuilder/ContainerBuildParser/Docker/DockerfileParser.swift b/Sources/NativeBuilder/ContainerBuildParser/Docker/DockerfileParser.swift
index 96a9de7a..0dcd4b6c 100644
--- a/Sources/NativeBuilder/ContainerBuildParser/Docker/DockerfileParser.swift
+++ b/Sources/NativeBuilder/ContainerBuildParser/Docker/DockerfileParser.swift
@@ -74,6 +74,50 @@ public struct DockerfileParser: BuildParser {
         }
     }
 
+    private struct InstructionOpt {
+        let key: String
+        let value: String
+    }
+
+    private func parseInstructionOpts(start: Int, tokens: [Token]) throws -> (Int, [InstructionOpt]) {
+        var done = false
+        var opts: [InstructionOpt] = []
+        var index = start
+
+        while index < tokens.endIndex, !done {
+            guard case .stringLiteral(let raw) = tokens[index] else {
+                done = true
+                break
+            }
+
+            guard raw.hasPrefix("--") else {
+                done = true
+                break
+            }
+
+            let components = raw.split(separator: "=", maxSplits: 1)
+            if components.count == 2 {
+                opts.append(InstructionOpt(key: String(components[0]), value: String(components[1])))
+            } else {
+                // instruction options should ALWAYS have a value, so if we're at
+                // the end or the next value is a string list, that's invalid input
+                index += 1
+                guard index < tokens.endIndex else {
+                    throw ParseError.invalidOption(raw)
+                }
+
+                guard case .stringLiteral(let value) = tokens[index] else {
+                    throw ParseError.invalidOption(raw)
+                }
+
+                opts.append(InstructionOpt(key: raw, value: value))
+            }
+            index += 1
+        }
+
+        return (index, opts)
+    }
+
     internal func tokensToFromInstruction(tokens: [Token]) throws -> FromInstruction {
         var index = tokens.startIndex
         index += 1  // skip the instruction
@@ -82,16 +126,18 @@ public struct DockerfileParser: BuildParser {
         var platform: String?
         var imageName: String?
 
-        // Step 1: parse options
-        while index < tokens.endIndex {
-            guard case .option(let option) = tokens[index] else {
-                break
-            }
-            guard FromOptions(rawValue: option.key) == .platform else {
+        // Step 1: Parse instruction options
+        let (newIndex, instructionOpts) = try parseInstructionOpts(start: index, tokens: tokens)
+        index = newIndex
+
+        for option in instructionOpts {
+            guard FromOptions(rawValue: String(option.key)) == .platform else {
                 throw ParseError.unexpectedValue
             }
+            if platform != nil {
+                throw ParseError.duplicateOptionSet(FromOptions.platform.rawValue)
+            }
             platform = option.value
-            index += 1
         }
 
         // Step 2: Parse image name
@@ -137,12 +183,11 @@ public struct DockerfileParser: BuildParser {
         var rawMounts = [String]()
         var network: String? = nil
 
-        // Step 1: parse options
-        while index < tokens.endIndex {
-            guard case .option(let option) = tokens[index] else {
-                break
-            }
+        // Step 1: Parse instruction options
+        let (lastOption, instructionOpts) = try parseInstructionOpts(start: index, tokens: tokens)
+        index = lastOption
 
+        for option in instructionOpts {
             guard let runOpt = RunOptions(rawValue: option.key) else {
                 throw ParseError.unexpectedValue
             }
@@ -155,7 +200,6 @@ public struct DockerfileParser: BuildParser {
             default:
                 throw ParseError.unexpectedValue
             }
-            index += 1
         }
 
         // Step 2: parse run command
@@ -185,8 +229,6 @@ public struct DockerfileParser: BuildParser {
                 break
             } else if case .stringLiteral(let value) = tokens[index] {
                 command.append(value)
-            } else if case .option(let option) = tokens[index] {
-                command.append(option.raw)
             } else {
                 break
             }
@@ -206,12 +248,11 @@ public struct DockerfileParser: BuildParser {
         var chown: String? = nil
         var link: String? = nil
 
-        // Step 1: parse options
-        while index < tokens.endIndex {
-            guard case .option(let option) = tokens[index] else {
-                break
-            }
+        // Step 1: Parse instruction options
+        let (newIndex, instructionOpts) = try parseInstructionOpts(start: index, tokens: tokens)
+        index = newIndex
 
+        for option in instructionOpts {
             guard let copyOpt = CopyOptions(rawValue: option.key) else {
                 throw ParseError.unexpectedValue
             }
@@ -240,7 +281,6 @@ public struct DockerfileParser: BuildParser {
             default:
                 throw ParseError.unexpectedValue
             }
-            index += 1
         }
 
         // Step 2: Get all source paths and destination path
diff --git a/Sources/NativeBuilder/ContainerBuildParser/Docker/DockerfileTokenizer.swift b/Sources/NativeBuilder/ContainerBuildParser/Docker/DockerfileTokenizer.swift
index a3c13947..a50e161f 100644
--- a/Sources/NativeBuilder/ContainerBuildParser/Docker/DockerfileTokenizer.swift
+++ b/Sources/NativeBuilder/ContainerBuildParser/Docker/DockerfileTokenizer.swift
@@ -54,8 +54,6 @@ struct DockerfileTokenizer {
                 results.append(listToken)
             } else if char == "#" {
                 parseComment()
-            } else if char == "-" {
-                results.append(parseOption())
             } else {
                 let start = position
                 parseWord()
@@ -120,26 +118,4 @@ struct DockerfileTokenizer {
             position = input.index(after: position)
         }
     }
-
-    mutating private func parseOption() -> Token {
-        let wordStart = position
-        parseWord()
-
-        let rawWord = input[wordStart..<position]
-        guard rawWord.contains("=") else {
-            // skip whitespace
-            while position < endPosition && input[position].isWhitespace {
-                position = input.index(after: position)
-                continue
-            }
-            let valueStart = position
-            parseWord()
-            let rawValue = input[valueStart..<position]
-            let raw = input[wordStart..<position]
-            return .option(Option(key: String(rawWord), value: String(rawValue), raw: String(raw)))
-        }
-        // split by equal
-        let optionComponents = rawWord.split(separator: "=", maxSplits: 1)
-        return .option(Option(key: String(optionComponents[0]), value: String(optionComponents[1]), raw: String(rawWord)))
-    }
 }
diff --git a/Sources/NativeBuilder/ContainerBuildParser/Types.swift b/Sources/NativeBuilder/ContainerBuildParser/Types.swift
index d2290f45..0ee1a445 100644
--- a/Sources/NativeBuilder/ContainerBuildParser/Types.swift
+++ b/Sources/NativeBuilder/ContainerBuildParser/Types.swift
@@ -41,17 +41,4 @@ public enum ParseError: Error, Equatable {
 public enum Token: Sendable, Equatable {
     case stringLiteral(String)
     case stringList([String])
-    case option(Option)
-}
-
-public struct Option: Sendable, Equatable {
-    let key: String
-    let value: String
-    let raw: String
-
-    init(key: String, value: String, raw: String) {
-        self.key = key
-        self.value = value
-        self.raw = raw
-    }
 }
diff --git a/Tests/NativeBuilderTests/ContainerBuildParserTests/TokenizerTests.swift b/Tests/NativeBuilderTests/ContainerBuildParserTests/TokenizerTests.swift
index 386a823c..ff73df23 100644
--- a/Tests/NativeBuilderTests/ContainerBuildParserTests/TokenizerTests.swift
+++ b/Tests/NativeBuilderTests/ContainerBuildParserTests/TokenizerTests.swift
@@ -46,7 +46,7 @@ import Testing
             input: "RUN --mount=type=cache /app",
             expectedTokens: [
                 .stringLiteral("RUN"),
-                .option(Option(key: "--mount", value: "type=cache", raw: "--mount=type=cache")),
+                .stringLiteral("--mount=type=cache"),
                 .stringLiteral("/app"),
             ]
         ),
@@ -54,7 +54,7 @@ import Testing
             input: "RUN --network=default /app",
             expectedTokens: [
                 .stringLiteral("RUN"),
-                .option(Option(key: "--network", value: "default", raw: "--network=default")),
+                .stringLiteral("--network=default"),
                 .stringLiteral("/app"),
             ]
         ),
@@ -62,8 +62,8 @@ import Testing
             input: "RUN --mount=type=bind,target=/target --network=host build.sh",
             expectedTokens: [
                 .stringLiteral("RUN"),
-                .option(Option(key: "--mount", value: "type=bind,target=/target", raw: "--mount=type=bind,target=/target")),
-                .option(Option(key: "--network", value: "host", raw: "--network=host")),
+                .stringLiteral("--mount=type=bind,target=/target"),
+                .stringLiteral("--network=host"),
                 .stringLiteral("build.sh"),
             ]
         ),
@@ -71,7 +71,8 @@ import Testing
             input: "RUN --mount type=cache /app",
             expectedTokens: [
                 .stringLiteral("RUN"),
-                .option(Option(key: "--mount", value: "type=cache", raw: "--mount type=cache")),
+                .stringLiteral("--mount"),
+                .stringLiteral("type=cache"),
                 .stringLiteral("/app"),
             ]
         ),
@@ -82,9 +83,10 @@ import Testing
                 """,
             expectedTokens: [
                 .stringLiteral("RUN"),
-                .option(Option(key: "--mount", value: "type=cache", raw: "--mount=type=cache")),
+                .stringLiteral("--mount=type=cache"),
                 .stringLiteral("build.sh"),
-                .option(Option(key: "--input", value: "hello", raw: "--input hello")),
+                .stringLiteral("--input"),
+                .stringLiteral("hello"),
             ]
         ),
         tokenizerTestInput(
@@ -94,7 +96,7 @@ import Testing
                 """#,
             expectedTokens: [
                 .stringLiteral("RUN"),
-                .option(Option(key: "--mount", value: "type=cache", raw: "--mount=type=cache")),
+                .stringLiteral("--mount=type=cache"),
                 .stringList(["build.sh", "--input", "hello"]),
             ]
         ),
@@ -105,7 +107,7 @@ import Testing
                 """#,
             expectedTokens: [
                 .stringLiteral("RUN"),
-                .option(Option(key: "--mount", value: "type=cache", raw: "--mount=type=cache")),
+                .stringLiteral("--mount=type=cache"),
                 .stringList(["build.sh", "--input", "hello"]),
             ]
         ),
@@ -116,7 +118,7 @@ import Testing
                 """#,
             expectedTokens: [
                 .stringLiteral("RUN"),
-                .option(Option(key: "--mount", value: "type=cache", raw: "--mount=type=cache")),
+                .stringLiteral("--mount=type=cache"),
                 .stringLiteral("build.sh --input hello"),
             ]
         ),
@@ -128,7 +130,7 @@ import Testing
                 """#,
             expectedTokens: [
                 .stringLiteral("RUN"),
-                .option(Option(key: "--mount", value: "type=cache", raw: "--mount=type=cache")),
+                .stringLiteral("--mount=type=cache"),
                 .stringLiteral("build.sh --input hello"),
             ]
         ),
@@ -139,7 +141,7 @@ import Testing
                 """#,
             expectedTokens: [
                 .stringLiteral("RUN"),
-                .option(Option(key: "--mount", value: "type=cache", raw: "--mount=type=cache")),
+                .stringLiteral("--mount=type=cache"),
                 .stringLiteral("build.sh --input hello"),
             ]
         ),
@@ -147,7 +149,7 @@ import Testing
             input: "COPY --from=alpine src /dest",
             expectedTokens: [
                 .stringLiteral("COPY"),
-                .option(Option(key: "--from", value: "alpine", raw: "--from=alpine")),
+                .stringLiteral("--from=alpine"),
                 .stringLiteral("src"),
                 .stringLiteral("/dest"),
             ]
@@ -157,7 +159,7 @@ import Testing
             input: "COPY --from=alpine src src1 src2 src3 /dest",
             expectedTokens: [
                 .stringLiteral("COPY"),
-                .option(Option(key: "--from", value: "alpine", raw: "--from=alpine")),
+                .stringLiteral("--from=alpine"),
                 .stringLiteral("src"),
                 .stringLiteral("src1"),
                 .stringLiteral("src2"),
@@ -169,7 +171,7 @@ import Testing
             input: "COPY --chown=10:11 src /dest",
             expectedTokens: [
                 .stringLiteral("COPY"),
-                .option(Option(key: "--chown", value: "10:11", raw: "--chown=10:11")),
+                .stringLiteral("--chown=10:11"),
                 .stringLiteral("src"),
                 .stringLiteral("/dest"),
             ]
@@ -178,7 +180,7 @@ import Testing
             input: "COPY --chown=bin stuff.txt /stuffdest/",
             expectedTokens: [
                 .stringLiteral("COPY"),
-                .option(Option(key: "--chown", value: "bin", raw: "--chown=bin")),
+                .stringLiteral("--chown=bin"),
                 .stringLiteral("stuff.txt"),
                 .stringLiteral("/stuffdest/"),
             ]
@@ -187,7 +189,7 @@ import Testing
             input: "COPY --chown=1 source /destination",
             expectedTokens: [
                 .stringLiteral("COPY"),
-                .option(Option(key: "--chown", value: "1", raw: "--chown=1")),
+                .stringLiteral("--chown=1"),
                 .stringLiteral("source"),
                 .stringLiteral("/destination"),
             ]
@@ -196,7 +198,7 @@ import Testing
             input: "COPY --chmod=440 src /dest/",
             expectedTokens: [
                 .stringLiteral("COPY"),
-                .option(Option(key: "--chmod", value: "440", raw: "--chmod=440")),
+                .stringLiteral("--chmod=440"),
                 .stringLiteral("src"),
                 .stringLiteral("/dest/"),
             ]
@@ -205,7 +207,7 @@ import Testing
             input: "COPY --link=false src /dest/",
             expectedTokens: [
                 .stringLiteral("COPY"),
-                .option(Option(key: "--link", value: "false", raw: "--link=false")),
+                .stringLiteral("--link=false"),
                 .stringLiteral("src"),
                 .stringLiteral("/dest/"),
             ]
@@ -302,7 +304,7 @@ import Testing
             TokenTest(
                 tokens: [
                     .stringLiteral("FROM"),
-                    .option(Option(key: "--platform", value: "linux/arm64", raw: "--platform=linux/arm64")),
+                    .stringLiteral("--platform=linux/arm64"),
                     .stringLiteral("alpine"),
                 ],
                 expectedInstruction: try FromInstruction(image: "alpine", platform: "linux/arm64")
@@ -323,7 +325,7 @@ import Testing
     @Test func testTokensToRunWithShellCommand() throws {
         let tokens: [Token] = [
             .stringLiteral("RUN"),
-            .option(Option(key: "--mount", value: "type=cache,target=/cache", raw: "--mount=type=cache,target=/cache")),
+            .stringLiteral("--mount=type=cache,target=/cache"),
             .stringLiteral("build.sh --input hello"),
         ]
 
@@ -337,7 +339,7 @@ import Testing
         let command = ["build.sh", "--input", "hello"]
         let tokens: [Token] = [
             .stringLiteral("RUN"),
-            .option(Option(key: "--mount", value: "type=cache,target=/mytarget", raw: "--mount=type=cache,target=/mytarget")),
+            .stringLiteral("--mount=type=cache,target=/mytarget"),
             .stringList(command),
         ]
 
@@ -350,13 +352,13 @@ import Testing
     static let extraTokensTests: [[Token]] = [
         [
             .stringLiteral("RUN"),
-            .option(Option(key: "--mount", value: "type=tmpfs,size=1000", raw: "--mount=type=tmpfs,size=1000")),
+            .stringLiteral("--mount=type=tmpfs,size=1000"),
             .stringList(["build.sh", "--input", "hello"]),
             .stringLiteral("extra"),
         ],
         [
             .stringLiteral("RUN"),
-            .option(Option(key: "--mount", value: "type=bind,target=/target", raw: "--mount=type=bind,target=/target")),
+            .stringLiteral("--mount=type=bind,target=/target"),
             .stringLiteral("build.sh"),
             .stringLiteral("--input"),
             .stringLiteral("hello"),
@@ -377,7 +379,7 @@ import Testing
             TokenTest(
                 tokens: [
                     .stringLiteral("COPY"),
-                    .option(Option(key: "--link", value: "false", raw: "--link=false")),
+                    .stringLiteral("--link=false"),
                     .stringLiteral("src"),
                     .stringLiteral("/dest/"),
                 ],
@@ -389,7 +391,7 @@ import Testing
             TokenTest(
                 tokens: [
                     .stringLiteral("COPY"),
-                    .option(Option(key: "--chmod", value: "440", raw: "--chmod=440")),
+                    .stringLiteral("--chmod=440"),
                     .stringLiteral("src"),
                     .stringLiteral("/dest"),
                 ],
@@ -402,7 +404,8 @@ import Testing
             TokenTest(
                 tokens: [
                     .stringLiteral("COPY"),
-                    .option(Option(key: "--chown", value: "11:mygroup", raw: "--chown 11:mygroup")),
+                    .stringLiteral("--chown"),
+                    .stringLiteral("11:mygroup"),
                     .stringLiteral("source"),
                     .stringLiteral("destination"),
                 ],
@@ -415,7 +418,7 @@ import Testing
             TokenTest(
                 tokens: [
                     .stringLiteral("COPY"),
-                    .option(Option(key: "--from", value: "alpine", raw: "--from=alpine")),
+                    .stringLiteral("--from=alpine"),
                     .stringLiteral("src"),
                     .stringLiteral("src1"),
                     .stringLiteral("src2"),
@@ -431,7 +434,8 @@ import Testing
             TokenTest(
                 tokens: [
                     .stringLiteral("COPY"),
-                    .option(Option(key: "--from", value: "base", raw: "--from base")),
+                    .stringLiteral("--from"),
+                    .stringLiteral("base"),
                     .stringLiteral("Source"),
                     .stringLiteral("Dest"),
                 ],
@@ -463,7 +467,7 @@ import Testing
         [
             // no sources
             .stringLiteral("COPY"),
-            .option(Option(key: "--from", value: "alpine", raw: "--from=alpine")),
+            .stringLiteral("--from=alpine"),
         ],
     ]