Unchecked assign case object discriminator

.github/workflows/ci.yml

@@ -10,6 +10,7 @@ jobs:
   build:
     uses: status-im/nimbus-common-workflow/.github/workflows/common.yml@main
     with:
+      nimble-version: b920dad9ed76c6619be3ec0cfbf0dde6f9e39092
       test-command: |
         nimble install -y toml_serialization json_serialization unittest2
         rm -f nimble.lock

.gitignore

@@ -3,4 +3,6 @@ nimcache
 nimble.develop
 nimble.paths
 build/
-vendor/
+vendor/
+tests/test_all
+tests/test_nested_cmd

confutils.nim

@@ -562,8 +562,14 @@ func parseCmdArg*(T: type bool, p: string): T {.raises: [ValueError].} =
   except CatchableError:
     raise newException(ValueError, "'" & p & "' is not a valid boolean value. Supported values are on/off, yes/no, true/false or 1/0")
 
+func parseEnumNormalized[T: enum](s: string): T {.raises: [ValueError].} =
+  # Note: In Nim 1.6 `parseEnum` normalizes the string except for the first
+  # character. Nim 1.2 would normalize for all characters. In config options
+  # the latter behaviour is required so this custom function is needed.
+  genEnumCaseStmt(T, s, default = nil, ord(low(T)), ord(high(T)), normalize)
+
 func parseCmdArg*(T: type enum, s: string): T {.raises: [ValueError].} =
-  parseEnum[T](s)
+  parseEnumNormalized[T](s)
 
 proc parseCmdArgAux(T: type, s: string): T {.raises: [ValueError].} =
   # The parseCmdArg procs are allowed to raise only `ValueError`.
@@ -661,12 +667,6 @@ template debugMacroResult(macroName: string) {.dirty.} =
     echo "\n-------- ", macroName, " ----------------------"
     echo result.repr
 
-func parseEnumNormalized[T: enum](s: string): T {.raises: [ValueError].} =
-  # Note: In Nim 1.6 `parseEnum` normalizes the string except for the first
-  # character. Nim 1.2 would normalize for all characters. In config options
-  # the latter behaviour is required so this custom function is needed.
-  genEnumCaseStmt(T, s, default = nil, ord(low(T)), ord(high(T)), normalize)
-
 proc generateFieldSetters(RecordType: NimNode): NimNode =
   var recordDef = getImpl(RecordType)
   let makeDefaultValue = bindSym"makeDefaultValue"
@@ -685,6 +685,7 @@ proc generateFieldSetters(RecordType: NimNode): NimNode =
       configField = newTree(nnkDotExpr, configVar, fieldName)
       defaultValue = field.readPragma"defaultValue"
       completerName = ident($field.name & "Complete")
+      isFieldDiscriminator = newLit field.isDiscriminator
 
     if defaultValue == nil:
       defaultValue = newCall(makeDefaultValue, newTree(nnkTypeOfExpr, configField))
@@ -717,13 +718,13 @@ proc generateFieldSetters(RecordType: NimNode): NimNode =
         sideEffect
         raises: [ValueError]
       .} =
-        when `configField` is enum:
-          # TODO: For some reason, the normal `setField` rejects enum fields
-          # when they are used as case discriminators. File this as a bug.
-          if isSome(val):
-            `configField` = parseEnumNormalized[type(`configField`)](val.get)
-          else:
-            `configField` = `defaultValue`
+        # This works as long as the object is fresh (i.e: `default(theObj)`)
+        # and the fields are processed in order.
+        # See https://github.com/status-im/nim-confutils/pull/117
+        # for a general solution.
+        when `isFieldDiscriminator`:
+          {.cast(uncheckedAssign).}:
+            setField(`configField`, val, `defaultValue`)
         else:
           setField(`configField`, val, `defaultValue`)
 

confutils.nimble

@@ -29,7 +29,8 @@ let verbose = getEnv("V", "") notin ["", "0"]
 let cfg =
   " --styleCheck:usages --styleCheck:error" &
   (if verbose: "" else: " --verbosity:0 --hints:off") &
-  " --skipParentCfg --skipUserCfg --outdir:build --nimcache:build/nimcache -f"
+  " --skipParentCfg --skipUserCfg --outdir:build --nimcache:build/nimcache -f" &
+  (if NimMajor >= 2: "" else: " -d:nimOldCaseObjects")
 
 proc build(args, path: string) =
   exec nimc & " " & lang & " " & cfg & " " & flags & " " & args & " " & path

tests/test_all.nim

@@ -13,7 +13,8 @@ import
   test_envvar,
   test_parsecmdarg,
   test_pragma,
-  test_qualified_ident
+  test_qualified_ident,
+  test_nested_cmd
 
 when defined(windows):
   import test_winreg

tests/test_nested_cmd.nim

@@ -0,0 +1,105 @@
+# confutils
+# Copyright (c) 2018-2025 Status Research & Development GmbH
+# Licensed under either of
+#  * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE))
+#  * MIT license ([LICENSE-MIT](LICENSE-MIT))
+# at your option.
+# This file may not be copied, modified, or distributed except according to
+# those terms.
+
+import unittest2, ../confutils
+
+type
+  OuterCmd = enum
+    noCommand
+    outerCmd1
+
+  InnerCmd = enum
+    innerCmd1 = "Inner cmd 1"
+    innerCmd2 = "Inner cmd 2"
+
+  TestConf = object
+    case cmd {.
+      command
+      defaultValue: OuterCmd.noCommand }: OuterCmd
+    of OuterCmd.noCommand:
+      outerArg {.
+        defaultValue: "outerArg default"
+        desc: "outerArg desc"
+        name: "outer-arg" }: string
+    of OuterCmd.outerCmd1:
+      outerArg1 {.
+        defaultValue: "outerArg1 default"
+        desc: "outerArg1 desc"
+        name: "outer-arg1" }: string
+      case innerCmd {.command.}: InnerCmd
+      of InnerCmd.innerCmd1:
+        innerArg1 {.
+          defaultValue: "innerArg1 default"
+          desc: "innerArg1 desc"
+          name: "inner-arg1" }: string
+      of InnerCmd.innerCmd2:
+        innerArg2 {.
+          defaultValue: "innerArg2 default"
+          desc: "innerArg2 desc"
+          name: "inner-arg2" }: string
+
+suite "test nested cmd":
+  test "no command":
+    let conf = TestConf.load(cmdLine = @[
+       "--outer-arg=foobar"
+    ])
+    check:
+      conf.cmd == OuterCmd.noCommand
+      conf.outerArg == "foobar"
+
+  test "subcommand outerCmd1 innerCmd1":
+    let conf = TestConf.load(cmdLine = @[
+      "outerCmd1",
+      "innerCmd1",
+      "--inner-arg1=foobar"
+    ])
+    check:
+      conf.cmd == OuterCmd.outerCmd1
+      conf.innerCmd == InnerCmd.innerCmd1
+      conf.innerArg1 == "foobar"
+
+  test "subcommand outerCmd1 innerCmd2":
+    let conf = TestConf.load(cmdLine = @[
+      "outerCmd1",
+      "innerCmd2",
+      "--inner-arg2=foobar"
+    ])
+    check:
+      conf.cmd == OuterCmd.outerCmd1
+      conf.innerCmd == InnerCmd.innerCmd2
+      conf.innerArg2 == "foobar"
+
+suite "test nested cmd default args":
+  test "no command default":
+    let conf = TestConf.load(cmdLine = newSeq[string]())
+    check:
+      conf.cmd == OuterCmd.noCommand
+      conf.outerArg == "outerArg default"
+
+  test "subcommand outerCmd1 innerCmd1":
+    let conf = TestConf.load(cmdLine = @[
+      "outerCmd1",
+      "innerCmd1"
+    ])
+    check:
+      conf.cmd == OuterCmd.outerCmd1
+      conf.innerCmd == InnerCmd.innerCmd1
+      conf.outerArg1 == "outerArg1 default"
+      conf.innerArg1 == "innerArg1 default"
+
+  test "subcommand outerCmd1 innerCmd2":
+    let conf = TestConf.load(cmdLine = @[
+      "outerCmd1",
+      "innerCmd2"
+    ])
+    check:
+      conf.cmd == OuterCmd.outerCmd1
+      conf.innerCmd == InnerCmd.innerCmd2
+      conf.outerArg1 == "outerArg1 default"
+      conf.innerArg2 == "innerArg2 default"

tests/test_parsecmdarg.nim

@@ -9,7 +9,7 @@ import
   std/[sequtils],
   unittest2,
   ../confutils
-  
+
 func testValidValues[T](lo: T = low(T), hi: T = high(T)): bool =
   allIt(lo .. hi, T.parseCmdArg($it) == it)
 
@@ -105,3 +105,21 @@ suite "parseCmdArg":
           false
         except ValueError:
           true
+
+  test "enum":
+    type TestEnumArg {.pure.} = enum
+      arg1 = "Arg1"
+      arg2
+      Arg3
+    check:
+      parseCmdArg(TestEnumArg, "Arg1") == TestEnumArg.arg1
+      parseCmdArg(TestEnumArg, "arg1") == TestEnumArg.arg1
+      parseCmdArg(TestEnumArg, "aRG1") == TestEnumArg.arg1
+      parseCmdArg(TestEnumArg, "arg2") == TestEnumArg.arg2
+      parseCmdArg(TestEnumArg, "Arg2") == TestEnumArg.arg2
+      parseCmdArg(TestEnumArg, "aRG2") == TestEnumArg.arg2
+      parseCmdArg(TestEnumArg, "arg3") == TestEnumArg.Arg3
+      parseCmdArg(TestEnumArg, "Arg3") == TestEnumArg.Arg3
+      parseCmdArg(TestEnumArg, "aRG3") == TestEnumArg.Arg3
+    expect ValueError:
+      discard parseCmdArg(TestEnumArg, "Arg123")