Strformat symbol binding (#10927)

6 files changed, 186 insertions, 138 deletions

diff --git a/changelog.md b/changelog.md
index 975c71ca2..85834fb3e 100644
--- a/changelog.md
+++ b/changelog.md
@@ -95,7 +95,8 @@
 - two poorly documented and not used modules (`subexes`, `scgi`) were moved to
   graveyard (they are available as Nimble packages)
 
-
+- Custom types that should be supported by `strformat` (&) now need an
+  explicit overload of `formatValue`.
 
 #### Breaking changes in the compiler
 
diff --git a/lib/pure/strformat.nim b/lib/pure/strformat.nim
index 8623a43e0..c561f5085 100644
--- a/lib/pure/strformat.nim
+++ b/lib/pure/strformat.nim
@@ -88,48 +88,21 @@ An expression like ``&"{key} is {value:arg} {{z}}"`` is transformed into:
 
 .. code-block:: nim
   var temp = newStringOfCap(educatedCapGuess)
-  format(key, temp)
-  format(" is ", temp)
-  format(value, arg, temp)
-  format(" {z}", temp)
+  temp.formatValue key, ""
+  temp.add " is "
+  temp.formatValue value, arg
+  temp.add " {z}"
   temp
 
 Parts of the string that are enclosed in the curly braces are interpreted
 as Nim code, to escape an ``{`` or ``}`` double it.
 
 ``&`` delegates most of the work to an open overloaded set
-of ``format`` procs. The required signature for a type ``T`` that supports
-formatting is usually ``proc format(x: T; result: var string)`` for efficiency
-but can also be ``proc format(x: T): string``. ``add`` and ``$`` procs are
-used as the fallback implementation.
-
-This is the concrete lookup algorithm that ``&`` uses:
-
-.. code-block:: nim
-
-  when compiles(format(arg, res)):
-    format(arg, res)
-  elif compiles(format(arg)):
-    res.add format(arg)
-  elif compiles(add(res, arg)):
-    res.add(arg)
-  else:
-    res.add($arg)
-
+of ``formatValue`` procs. The required signature for a type ``T`` that supports
+formatting is usually ``proc formatValue(result: var string; x: T; specifier: string)``.
 
 The subexpression after the colon
-(``arg`` in ``&"{key} is {value:arg} {{z}}"``) is an optional argument
-passed to ``format``.
-
-If an optional argument is present the following lookup algorithm is used:
-
-.. code-block:: nim
-
-  when compiles(format(arg, option, res)):
-    format(arg, option, res)
-  else:
-    res.add format(arg, option)
-
+(``arg`` in ``&"{key} is {value:arg} {{z}}"``) is optional. It will be passed as the last argument to ``formatValue``. When the colon with the subexpression it is left out, an empty string will be taken instead.
 
 For strings and numeric types the optional argument is a so-called
 "standard format specifier".
@@ -242,7 +215,7 @@ Future directions
 =================
 
 A curly expression with commas in it like ``{x, argA, argB}`` could be
-transformed to ``format(x, argA, argB, res)`` in order to support
+transformed to ``formatValue(result, x, argA, argB)`` in order to support
 formatters that do not need to parse a custom language within a custom
 language but instead prefer to use Nim's existing syntax. This also
 helps in readability since there is only so much you can cram into
@@ -251,96 +224,7 @@ single letter DSLs.
 ]##
 
 import macros, parseutils, unicode
-import strutils
-
-template callFormat(res, arg) {.dirty.} =
-  when arg is string:
-    # workaround in order to circumvent 'strutils.format' which matches
-    # too but doesn't adhere to our protocol.
-    res.add arg
-  elif compiles(format(arg, res)) and
-      # Check if format returns void
-      not (compiles do: discard format(arg, res)):
-    format(arg, res)
-  elif compiles(format(arg)):
-    res.add format(arg)
-  elif compiles(add(res, arg)):
-    res.add(arg)
-  else:
-    res.add($arg)
-
-template callFormatOption(res, arg, option) {.dirty.} =
-  when compiles(format(arg, option, res)):
-    format(arg, option, res)
-  elif compiles(format(arg, option)):
-    res.add format(arg, option)
-  else:
-    format($arg, option, res)
-
-macro `&`*(pattern: string): untyped =
-  ## For a specification of the ``&`` macro, see the module level documentation.
-  if pattern.kind notin {nnkStrLit..nnkTripleStrLit}:
-    error "string formatting (fmt(), &) only works with string literals", pattern
-  let f = pattern.strVal
-  var i = 0
-  let res = genSym(nskVar, "fmtRes")
-  result = newNimNode(nnkStmtListExpr, lineInfoFrom=pattern)
-  # XXX: https://github.com/nim-lang/Nim/issues/8405
-  # When compiling with -d:useNimRtl, certain procs such as `count` from the strutils
-  # module are not accessible at compile-time:
-  let expectedGrowth = when defined(useNimRtl): 0 else: count(f, '{') * 10
-  result.add newVarStmt(res, newCall(bindSym"newStringOfCap", newLit(f.len + expectedGrowth)))
-  var strlit = ""
-  while i < f.len:
-    if f[i] == '{':
-      inc i
-      if f[i] == '{':
-        inc i
-        strlit.add '{'
-      else:
-        if strlit.len > 0:
-          result.add newCall(bindSym"add", res, newLit(strlit))
-          strlit = ""
-
-        var subexpr = ""
-        while i < f.len and f[i] != '}' and f[i] != ':':
-          subexpr.add f[i]
-          inc i
-        let x = parseExpr(subexpr)
-
-        if f[i] == ':':
-          inc i
-          var options = ""
-          while i < f.len and f[i] != '}':
-            options.add f[i]
-            inc i
-          result.add getAst(callFormatOption(res, x, newLit(options)))
-        else:
-          result.add getAst(callFormat(res, x))
-        if f[i] == '}':
-          inc i
-        else:
-          doAssert false, "invalid format string: missing '}'"
-    elif f[i] == '}':
-      if f[i+1] == '}':
-        strlit.add '}'
-        inc i, 2
-      else:
-        doAssert false, "invalid format string: '}' instead of '}}'"
-        inc i
-    else:
-      strlit.add f[i]
-      inc i
-  if strlit.len > 0:
-    result.add newCall(bindSym"add", res, newLit(strlit))
-  result.add res
-  when defined(debugFmtDsl):
-    echo repr result
-
-template fmt*(pattern: string): untyped =
-  ## An alias for ``&``.
-  bind `&`
-  &pattern
+import strutils except format
 
 proc mkDigit(v: int, typ: char): string {.inline.} =
   assert(v < 26)
@@ -491,8 +375,7 @@ proc parseStandardFormatSpecifier*(s: string; start = 0;
     raise newException(ValueError,
       "invalid format string, cannot parse: " & s[i..^1])
 
-
-proc format*(value: SomeInteger; specifier: string; res: var string) =
+proc formatValue*(result: var string; value: SomeInteger; specifier: string) =
   ## Standard format implementation for ``SomeInteger``. It makes little
   ## sense to call this directly, but it is required to exist
   ## by the ``&`` macro.
@@ -507,9 +390,9 @@ proc format*(value: SomeInteger; specifier: string; res: var string) =
     raise newException(ValueError,
       "invalid type in format string for number, expected one " &
       " of 'x', 'X', 'b', 'd', 'o' but got: " & spec.typ)
-  res.add formatInt(value, radix, spec)
+  result.add formatInt(value, radix, spec)
 
-proc format*(value: SomeFloat; specifier: string; res: var string) =
+proc formatValue*(result: var string; value: SomeFloat; specifier: string): void =
   ## Standard format implementation for ``SomeFloat``. It makes little
   ## sense to call this directly, but it is required to exist
   ## by the ``&`` macro.
@@ -557,14 +440,13 @@ proc format*(value: SomeFloat; specifier: string; res: var string) =
 
   # the default for numbers is right-alignment:
   let align = if spec.align == '\0': '>' else: spec.align
-  let result = alignString(f, spec.minimumWidth,
-                           align, spec.fill)
+  let res = alignString(f, spec.minimumWidth, align, spec.fill)
   if spec.typ in {'A'..'Z'}:
-    res.add toUpperAscii(result)
+    result.add toUpperAscii(res)
   else:
-    res.add result
+    result.add res
 
-proc format*(value: string; specifier: string; res: var string) =
+proc formatValue*(result: var string; value: string; specifier: string) =
   ## Standard format implementation for ``string``. It makes little
   ## sense to call this directly, but it is required to exist
   ## by the ``&`` macro.
@@ -579,7 +461,92 @@ proc format*(value: string; specifier: string; res: var string) =
   if spec.precision != -1:
     if spec.precision < runelen(value):
       setLen(value, runeOffset(value, spec.precision))
-  res.add alignString(value, spec.minimumWidth, spec.align, spec.fill)
+  result.add alignString(value, spec.minimumWidth, spec.align, spec.fill)
+
+template formatValue[T: enum](result: var string; value: T; specifier: string) =
+  result.add $value
+
+template formatValue(result: var string; value: char; specifier: string) =
+  result.add value
+
+template formatValue(result: var string; value: cstring; specifier: string) =
+  result.add value
+
+proc formatValue[T](result: var string; value: openarray[T]; specifier: string) =
+  result.add "["
+  for i, it in value:
+    if i != 0:
+      result.add ", "
+    result.formatValue(it, specifier)
+  result.add "]"
+
+macro `&`*(pattern: string): untyped =
+  ## For a specification of the ``&`` macro, see the module level documentation.
+  if pattern.kind notin {nnkStrLit..nnkTripleStrLit}:
+    error "string formatting (fmt(), &) only works with string literals", pattern
+  let f = pattern.strVal
+  var i = 0
+  let res = genSym(nskVar, "fmtRes")
+  result = newNimNode(nnkStmtListExpr, lineInfoFrom=pattern)
+  # XXX: https://github.com/nim-lang/Nim/issues/8405
+  # When compiling with -d:useNimRtl, certain procs such as `count` from the strutils
+  # module are not accessible at compile-time:
+  let expectedGrowth = when defined(useNimRtl): 0 else: count(f, '{') * 10
+  result.add newVarStmt(res, newCall(bindSym"newStringOfCap", newLit(f.len + expectedGrowth)))
+  var strlit = ""
+  while i < f.len:
+    if f[i] == '{':
+      inc i
+      if f[i] == '{':
+        inc i
+        strlit.add '{'
+      else:
+        if strlit.len > 0:
+          result.add newCall(bindSym"add", res, newLit(strlit))
+          strlit = ""
+
+        var subexpr = ""
+        while i < f.len and f[i] != '}' and f[i] != ':':
+          subexpr.add f[i]
+          inc i
+        var x: NimNode
+        try:
+          x = parseExpr(subexpr)
+        except ValueError:
+          let msg = getCurrentExceptionMsg()
+          error("could not parse ``" & subexpr & "``.\n" & msg, pattern)
+        let formatSym = bindSym("formatValue", brOpen)
+        var options = ""
+        if f[i] == ':':
+          inc i
+          while i < f.len and f[i] != '}':
+            options.add f[i]
+            inc i
+        if f[i] == '}':
+          inc i
+        else:
+          doAssert false, "invalid format string: missing '}'"
+        result.add newCall(formatSym, res, x, newLit(options))
+    elif f[i] == '}':
+      if f[i+1] == '}':
+        strlit.add '}'
+        inc i, 2
+      else:
+        doAssert false, "invalid format string: '}' instead of '}}'"
+        inc i
+    else:
+      strlit.add f[i]
+      inc i
+  if strlit.len > 0:
+    result.add newCall(bindSym"add", res, newLit(strlit))
+  result.add res
+  when defined(debugFmtDsl):
+    echo repr result
+
+template fmt*(pattern: string): untyped =
+  ## An alias for ``&``.
+  bind `&`
+  &pattern
 
 when isMainModule:
   template check(actual, expected: string) =
diff --git a/lib/pure/times.nim b/lib/pure/times.nim
index 5fd57c843..8074c09e8 100644
--- a/lib/pure/times.nim
+++ b/lib/pure/times.nim
@@ -2330,6 +2330,10 @@ proc format*(dt: DateTime, f: static[string]): string {.raises: [].} =
   const f2 = initTimeFormat(f)
   result = dt.format(f2)
 
+template formatValue*(result: var string; value: DateTime, specifier: string) =
+  ## adapter for strformat. Not intended to be called directly.
+  result.add format(value, specifier)
+
 proc format*(time: Time, f: string, zone: Timezone = local()): string
     {.raises: [TimeFormatParseError].} =
   ## Shorthand for constructing a ``TimeFormat`` and using it to format
@@ -2349,6 +2353,10 @@ proc format*(time: Time, f: static[string], zone: Timezone = local()): string
   const f2 = initTimeFormat(f)
   result = time.inZone(zone).format(f2)
 
+template formatValue*(result: var string; value: Time, specifier: string) =
+  ## adapter for strformat. Not intended to be called directly.
+  result.add format(value, specifier)
+
 proc parse*(input: string, f: TimeFormat, zone: Timezone = local()): DateTime
     {.raises: [TimeParseError, Defect].} =
   ## Parses ``input`` as a ``DateTime`` using the format specified by ``f``.
diff --git a/tests/stdlib/genericstrformat.nim b/tests/stdlib/genericstrformat.nim
new file mode 100644
index 000000000..0446f3269
--- /dev/null
+++ b/tests/stdlib/genericstrformat.nim
@@ -0,0 +1,16 @@
+# from issue #7632
+# imported and used in tstrformat
+
+import strformat
+
+proc fails*(a: static[int]): string =
+  &"formatted {a:2}"
+
+proc fails2*[N: static[int]](a: int): string =
+  &"formatted {a:2}"
+
+proc works*(a: int): string =
+  &"formatted {a:2}"
+
+proc fails0*(a: int or uint): string =
+  &"formatted {a:2}"
diff --git a/tests/stdlib/tstrformat.nim b/tests/stdlib/tstrformat.nim
index db76899d4..d99aeb2f1 100644
--- a/tests/stdlib/tstrformat.nim
+++ b/tests/stdlib/tstrformat.nim
@@ -1,5 +1,5 @@
 discard """
-    action: "run"
+action: "run"
 """
 
 import strformat
@@ -8,6 +8,10 @@ type Obj = object
 
 proc `$`(o: Obj): string = "foobar"
 
+# for custom types, formatValue needs to be overloaded.
+template formatValue(result: var string; value: Obj; specifier: string) =
+  result.formatValue($value, specifier)
+
 var o: Obj
 doAssert fmt"{o}" == "foobar"
 doAssert fmt"{o:10}" == "foobar    "
@@ -42,7 +46,7 @@ doAssert fmt"^7.9 :: {str:^7.9}" == "^7.9 :: äöüe\u0309\u0319o\u0307\u0359"
 doAssert fmt"{15:08}" == "00000015" # int, works
 doAssert fmt"{1.5:08}" == "000001.5" # float, works
 doAssert fmt"{1.5:0>8}" == "000001.5" # workaround using fill char works for positive floats
-doAssert fmt"{-1.5:0>8}" == "0000-1.5" # even that does not work for negative floats  
+doAssert fmt"{-1.5:0>8}" == "0000-1.5" # even that does not work for negative floats
 doAssert fmt"{-1.5:08}" == "-00001.5" # works
 doAssert fmt"{1.5:+08}" == "+00001.5" # works
 doAssert fmt"{1.5: 08}" == " 00001.5" # works
@@ -54,3 +58,39 @@ doassert fmt"{-0.0: g}" == "-0"
 doAssert fmt"{0.0:g}" == "0"
 doAssert fmt"{0.0:+g}" == "+0"
 doAssert fmt"{0.0: g}" == " 0"
+
+# seq format
+
+let data1 = [1'i64, 10000'i64, 10000000'i64]
+let data2 = [10000000'i64, 100'i64, 1'i64]
+
+doAssert fmt"data1: {data1:8} ∨" == "data1: [       1,    10000, 10000000] ∨"
+doAssert fmt"data2: {data2:8} ∧" == "data2: [10000000,      100,        1] ∧"
+
+# custom format Value
+
+type
+  Vec2[T] = object
+    x,y: T
+  Vec2f = Vec2[float32]
+  Vec2i = Vec2[int32]
+
+proc formatValue[T](result: var string; value: Vec2[T]; specifier: string) =
+  result.add '['
+  result.formatValue value.x, specifier
+  result.add ", "
+  result.formatValue value.y, specifier
+  result.add "]"
+
+let v1 = Vec2f(x:1.0, y: 2.0)
+let v2 = Vec2i(x:1, y: 1337)
+doAssert fmt"v1: {v1:+08}  v2: {v2:>4}" == "v1: [+0000001, +0000002]  v2: [   1, 1337]"
+
+# issue #7632
+
+import genericstrformat
+
+doAssert works(5) == "formatted  5"
+doAssert fails0(6) == "formatted  6"
+doAssert fails(7) == "formatted  7"
+doAssert fails2[0](8) == "formatted  8"
diff --git a/tests/stdlib/tstrformatMissingFormatValue.nim b/tests/stdlib/tstrformatMissingFormatValue.nim
new file mode 100644
index 000000000..66c1c8772
--- /dev/null
+++ b/tests/stdlib/tstrformatMissingFormatValue.nim
@@ -0,0 +1,16 @@
+discard """
+errormsg: '''type mismatch: got <string, Obj, string>'''
+nimout: '''proc formatValue'''
+"""
+
+# This test is here to make sure that there is a clean error that
+# that indicates ``formatValue`` needs to be overloaded with the custom type.
+
+import strformat
+
+type Obj = object
+
+proc `$`(o: Obj): string = "foobar"
+
+var o: Obj
+doAssert fmt"{o}" == "foobar"