Improved collection-to-string behavior (#6825)

changelog.md

@@ -123,3 +123,6 @@ This now needs to be written as:
   to [http://www.gii.upv.es/tlsf/](http://www.gii.upv.es/tlsf/) the maximum
   fragmentation measured is lower than 25%. As a nice bonus ``alloc`` and
   ``dealloc`` became O(1) operations.
+- The behavior of ``$`` has been changed for all standard library collections. The
+  collection-to-string implementations now perform proper quoting and escaping of
+  strings and chars.

lib/pure/collections/critbits.nim

@@ -141,8 +141,8 @@ proc excl*[T](c: var CritBitTree[T], key: string) =
 
 proc missingOrExcl*[T](c: var CritBitTree[T], key: string): bool =
   ## Returns true iff `c` does not contain the given `key`. If the key
-  ## does exist, c.excl(key) is performed. 
-  let oldCount = c.count 
+  ## does exist, c.excl(key) is performed.
+  let oldCount = c.count
   var n = exclImpl(c, key)
   result = c.count == oldCount
 
@@ -326,7 +326,7 @@ proc `$`*[T](c: CritBitTree[T]): string =
       result.add($key)
       when T isnot void:
         result.add(": ")
-        result.add($val)
+        result.addQuoted(val)
     result.add("}")
 
 when isMainModule:

lib/pure/collections/deques.nim

@@ -185,7 +185,7 @@ proc `$`*[T](deq: Deque[T]): string =
   result = "["
   for x in deq:
     if result.len > 1: result.add(", ")
-    result.add($x)
+    result.addQuoted(x)
   result.add("]")
 
 when isMainModule:

lib/pure/collections/lists.nim

@@ -135,7 +135,7 @@ proc `$`*[T](L: SomeLinkedCollection[T]): string =
   result = "["
   for x in nodes(L):
     if result.len > 1: result.add(", ")
-    result.add($x.value)
+    result.addQuoted(x.value)
   result.add("]")
 
 proc find*[T](L: SomeLinkedCollection[T], value: T): SomeLinkedNode[T] =

lib/pure/collections/sets.nim

@@ -406,7 +406,7 @@ template dollarImpl() {.dirty.} =
   result = "{"
   for key in items(s):
     if result.len > 1: result.add(", ")
-    result.add($key)
+    result.addQuoted(key)
   result.add("}")
 
 proc `$`*[A](s: HashSet[A]): string =

lib/pure/collections/tables.nim

@@ -338,9 +338,9 @@ template dollarImpl(): untyped {.dirty.} =
     result = "{"
     for key, val in pairs(t):
       if result.len > 1: result.add(", ")
-      result.add($key)
+      result.addQuoted(key)
       result.add(": ")
-      result.add($val)
+      result.addQuoted(val)
     result.add("}")
 
 proc `$`*[A, B](t: Table[A, B]): string =

lib/pure/strutils.nim

@@ -1761,29 +1761,15 @@ proc insertSep*(s: string, sep = '_', digits = 3): string {.noSideEffect,
 
 proc escape*(s: string, prefix = "\"", suffix = "\""): string {.noSideEffect,
   rtl, extern: "nsuEscape".} =
-  ## Escapes a string `s`.
+  ## Escapes a string `s`. See `system.addEscapedChar <system.html#addEscapedChar>`_
+  ## for the escaping scheme.
   ##
-  ## This does these operations (at the same time):
-  ## * replaces any ``\`` by ``\\``
-  ## * replaces any ``'`` by ``\'``
-  ## * replaces any ``"`` by ``\"``
-  ## * replaces any other character in the set ``{'\0'..'\31', '\127'..'\255'}``
-  ##   by ``\xHH`` where ``HH`` is its hexadecimal value.
-  ## The procedure has been designed so that its output is usable for many
-  ## different common syntaxes. The resulting string is prefixed with
-  ## `prefix` and suffixed with `suffix`. Both may be empty strings.
-  ## **Note**: This is not correct for producing Ansi C code!
+  ## The resulting string is prefixed with `prefix` and suffixed with `suffix`.
+  ## Both may be empty strings.
   result = newStringOfCap(s.len + s.len shr 2)
   result.add(prefix)
   for c in items(s):
-    case c
-    of '\0'..'\31', '\127'..'\255':
-      add(result, "\\x")
-      add(result, toHex(ord(c), 2))
-    of '\\': add(result, "\\\\")
-    of '\'': add(result, "\\'")
-    of '\"': add(result, "\\\"")
-    else: add(result, c)
+    result.addEscapedChar(c)
   add(result, suffix)
 
 proc unescape*(s: string, prefix = "\"", suffix = "\""): string {.noSideEffect,

lib/system.nim

@@ -1325,6 +1325,7 @@ proc add*(x: var string, y: string) {.magic: "AppendStrStr", noSideEffect.}
   ##   tmp.add("cd")
   ##   assert(tmp == "abcd")
 
+
 type
   Endianness* = enum ## is a type describing the endianness of a processor.
     littleEndian, bigEndian
@@ -2503,9 +2504,9 @@ proc `$`*[T: tuple|object](x: T): string =
     when compiles($value):
       when compiles(value.isNil):
         if value.isNil: result.add "nil"
-        else: result.add($value)
+        else: result.addQuoted(value)
       else:
-        result.add($value)
+        result.addQuoted(value)
       firstElement = false
     else:
       result.add("...")
@@ -2525,12 +2526,9 @@ proc collectionToString[T](x: T, prefix, separator, suffix: string): string =
       if value.isNil:
         result.add "nil"
       else:
-        result.add($value)
-    # prevent temporary string allocation
-    elif compiles(result.add(value)):
-      result.add(value)
+        result.addQuoted(value)
     else:
-      result.add($value)
+      result.addQuoted(value)
 
   result.add(suffix)
 
@@ -3893,6 +3891,65 @@ proc compiles*(x: untyped): bool {.magic: "Compiles", noSideEffect, compileTime.
 when declared(initDebugger):
   initDebugger()
 
+proc addEscapedChar*(s: var string, c: char) {.noSideEffect, inline.} =
+  ## Adds a char to string `s` and applies the following escaping:
+  ##
+  ## * replaces any ``\`` by ``\\``
+  ## * replaces any ``'`` by ``\'``
+  ## * replaces any ``"`` by ``\"``
+  ## * replaces any other character in the set ``{'\0'..'\31', '\127'..'\255'}``
+  ##   by ``\xHH`` where ``HH`` is its hexadecimal value.
+  ##
+  ## The procedure has been designed so that its output is usable for many
+  ## different common syntaxes.
+  ## **Note**: This is not correct for producing Ansi C code!
+  case c
+  of '\0'..'\31', '\127'..'\255':
+    add(s, "\\x")
+    const HexChars = "0123456789ABCDEF"
+    let n = ord(c)
+    s.add(HexChars[int((n and 0xF0) shr 4)])
+    s.add(HexChars[int(n and 0xF)])
+  of '\\': add(s, "\\\\")
+  of '\'': add(s, "\\'")
+  of '\"': add(s, "\\\"")
+  else: add(s, c)
+
+proc addQuoted*[T](s: var string, x: T) =
+  ## Appends `x` to string `s` in place, applying quoting and escaping
+  ## if `x` is a string or char. See
+  ## `addEscapedChar <system.html#addEscapedChar>`_
+  ## for the escaping scheme.
+  ##
+  ## The Nim standard library uses this function on the elements of
+  ## collections when producing a string representation of a collection.
+  ## It is recommended to use this function as well for user-side collections.
+  ## Users may overload `addQuoted` for custom (string-like) types if
+  ## they want to implement a customized element representation.
+  ##
+  ## .. code-block:: Nim
+  ##   var tmp = ""
+  ##   tmp.addQuoted(1)
+  ##   tmp.add(", ")
+  ##   tmp.addQuoted("string")
+  ##   tmp.add(", ")
+  ##   tmp.addQuoted('c')
+  ##   assert(tmp == """1, "string", 'c'""")
+  when T is string:
+    s.add("\"")
+    for c in x:
+      s.addEscapedChar(c)
+    s.add("\"")
+  elif T is char:
+    s.add("'")
+    s.addEscapedChar(x)
+    s.add("'")
+  # prevent temporary string allocation
+  elif compiles(s.add(x)):
+    s.add(x)
+  else:
+    s.add($x)
+
 when hasAlloc:
   # XXX: make these the default (or implement the NilObject optimization)
   proc safeAdd*[T](x: var seq[T], y: T) {.noSideEffect.} =
@@ -4034,4 +4091,4 @@ template doAssertRaises*(exception, code: untyped): typed =
   except Exception as exc:
     raiseAssert(astToStr(exception) &
                 " wasn't raised, another error was raised instead by:\n"&
-                astToStr(code))
+                astToStr(code))

tests/array/troof1.nim

@@ -4,7 +4,7 @@ discard """
 3
 @[(Field0: 1, Field1: 2), (Field0: 3, Field1: 5)]
 2
-@[a, new one, c]
+@["a", "new one", "c"]
 @[1, 2, 3]'''
 """
 

tests/ccgbugs/t6279.nim

@@ -1,6 +1,6 @@
 discard """
 cmd: "nim c -r -d:fulldebug -d:smokeCycles --gc:refc $file"
-output: '''@[a]'''
+output: '''@["a"]'''
 """
 
 # bug #6279

tests/ccgbugs/tbug1081.nim

@@ -3,7 +3,7 @@ discard """
 0
 0
 0
-x = [a, b, c, 0, 1, 2, 3, 4, 5, 6] and y = [a, b, c, 0, 1, 2, 3, 4, 5, 6]'''
+x = ['a', 'b', 'c', '0', '1', '2', '3', '4', '5', '6'] and y = ['a', 'b', 'c', '0', '1', '2', '3', '4', '5', '6']'''
 """
 
 proc `1/1`() = echo(1 div 1)

tests/ccgbugs/tconstobj.nim

@@ -1,5 +1,5 @@
 discard """
-  output: '''(FirstName: James, LastName: Franco)'''
+  output: '''(FirstName: "James", LastName: "Franco")'''
 """
 
 # bug #1547

tests/ccgbugs/tobjconstr_regression.nim

@@ -1,5 +1,5 @@
 discard """
-  output: "@[(username: user, role: admin, description: desc, email_addr: email), (username: user, role: admin, description: desc, email_addr: email)]"
+  output: '''@[(username: "user", role: "admin", description: "desc", email_addr: "email"), (username: "user", role: "admin", description: "desc", email_addr: "email")]'''
 """
 
 type

tests/collections/tcollections_to_string.nim

@@ -0,0 +1,106 @@
+discard """
+  exitcode: 0
+  output: ""
+"""
+import sets
+import tables
+import deques
+import lists
+import critbits
+
+# Tests for tuples
+doAssert $(1, 2, 3) == "(Field0: 1, Field1: 2, Field2: 3)"
+doAssert $("1", "2", "3") == """(Field0: "1", Field1: "2", Field2: "3")"""
+doAssert $('1', '2', '3') == """(Field0: '1', Field1: '2', Field2: '3')"""
+
+# Tests for seqs
+doAssert $(@[1, 2, 3]) == "@[1, 2, 3]"
+doAssert $(@["1", "2", "3"]) == """@["1", "2", "3"]"""
+doAssert $(@['1', '2', '3']) == """@['1', '2', '3']"""
+
+# Tests for sets
+doAssert $(toSet([1])) == "{1}"
+doAssert $(toSet(["1"])) == """{"1"}"""
+doAssert $(toSet(['1'])) == """{'1'}"""
+doAssert $(toOrderedSet([1, 2, 3])) == "{1, 2, 3}"
+doAssert $(toOrderedSet(["1", "2", "3"])) == """{"1", "2", "3"}"""
+doAssert $(toOrderedSet(['1', '2', '3'])) == """{'1', '2', '3'}"""
+
+# Tests for tables
+doAssert $({1: "1", 2: "2"}.toTable) == """{1: "1", 2: "2"}"""
+doAssert $({"1": 1, "2": 2}.toTable) == """{"1": 1, "2": 2}"""
+
+# Tests for deques
+block:
+  var d = initDeque[int]()
+  d.addLast(1)
+  doAssert $d == "[1]"
+block:
+  var d = initDeque[string]()
+  d.addLast("1")
+  doAssert $d == """["1"]"""
+block:
+  var d = initDeque[char]()
+  d.addLast('1')
+  doAssert $d == "['1']"
+
+# Tests for lists
+block:
+  var l = initDoublyLinkedList[int]()
+  l.append(1)
+  l.append(2)
+  l.append(3)
+  doAssert $l == "[1, 2, 3]"
+block:
+  var l = initDoublyLinkedList[string]()
+  l.append("1")
+  l.append("2")
+  l.append("3")
+  doAssert $l == """["1", "2", "3"]"""
+block:
+  var l = initDoublyLinkedList[char]()
+  l.append('1')
+  l.append('2')
+  l.append('3')
+  doAssert $l == """['1', '2', '3']"""
+
+# Tests for critbits
+block:
+  var t: CritBitTree[int]
+  t["a"] = 1
+  doAssert $t == "{a: 1}"
+block:
+  var t: CritBitTree[string]
+  t["a"] = "1"
+  doAssert $t == """{a: "1"}"""
+block:
+  var t: CritBitTree[char]
+  t["a"] = '1'
+  doAssert $t == "{a: '1'}"
+
+
+# Test escaping behavior
+block:
+  var s = ""
+  s.addQuoted('\0')
+  s.addQuoted('\31')
+  s.addQuoted('\127')
+  s.addQuoted('\255')
+  doAssert s == "'\\x00''\\x1F''\\x7F''\\xFF'"
+block:
+  var s = ""
+  s.addQuoted('\\')
+  s.addQuoted('\'')
+  s.addQuoted('\"')
+  doAssert s == """'\\''\'''\"'"""
+
+# Test customized element representation
+type CustomString = object
+
+proc addQuoted(s: var string, x: CustomString) =
+  s.add("<CustomString>")
+
+block:
+  let s = @[CustomString()]
+  doAssert $s == "@[<CustomString>]"
+

tests/collections/ttables.nim

@@ -48,7 +48,7 @@ block tableTest1:
     for y in 0..1:
       assert t[(x,y)] == $x & $y
   assert($t ==
-    "{(x: 0, y: 1): 01, (x: 0, y: 0): 00, (x: 1, y: 0): 10, (x: 1, y: 1): 11}")
+    "{(x: 0, y: 1): \"01\", (x: 0, y: 0): \"00\", (x: 1, y: 0): \"10\", (x: 1, y: 1): \"11\"}")
 
 block tableTest2:
   var t = initTable[string, float]()

tests/collections/ttablesref.nim

@@ -47,7 +47,7 @@ block tableTest1:
     for y in 0..1:
       assert t[(x,y)] == $x & $y
   assert($t ==
-    "{(x: 0, y: 1): 01, (x: 0, y: 0): 00, (x: 1, y: 0): 10, (x: 1, y: 1): 11}")
+    "{(x: 0, y: 1): \"01\", (x: 0, y: 0): \"00\", (x: 1, y: 0): \"10\", (x: 1, y: 1): \"11\"}")
 
 block tableTest2:
   var t = newTable[string, float]()
@@ -139,7 +139,7 @@ proc orderedTableSortTest() =
 block anonZipTest:
   let keys = @['a','b','c']
   let values = @[1, 2, 3]
-  doAssert "{a: 1, b: 2, c: 3}" == $ toTable zip(keys, values)
+  doAssert "{'a': 1, 'b': 2, 'c': 3}" == $ toTable zip(keys, values)
 
 block clearTableTest:
   var t = newTable[string, float]()

tests/concepts/t3330.nim

@@ -6,10 +6,10 @@ but expected one of:
 proc test(foo: Foo[int])
 t3330.nim(25, 8) Hint: Non-matching candidates for add(k, string, T)
 proc add(x: var string; y: string)
+proc add(result: var string; x: float)
 proc add(x: var string; y: char)
 proc add(result: var string; x: int64)
 proc add(x: var string; y: cstring)
-proc add(result: var string; x: float)
 proc add[T](x: var seq[T]; y: openArray[T])
 proc add[T](x: var seq[T]; y: T)
 

tests/exprs/tstmtexprs.nim

@@ -1,6 +1,6 @@
 discard """
   output: '''24
-(bar: bar)
+(bar: "bar")
 1244
 6
 abcdefghijklmnopqrstuvwxyz

tests/fields/timplicitfieldswithpartial.nim

@@ -1,5 +1,5 @@
 discard """
-  output: '''(foo: 38, other: string here)
+  output: '''(foo: 38, other: "string here")
 43
 100
 90'''

tests/generics/treentranttypes.nim

@@ -1,6 +1,6 @@
 discard """
 output: '''
-(Field0: 10, Field1: (Field0: test, Field1: 1.2))
+(Field0: 10, Field1: (Field0: "test", Field1: 1.2))
 3x3 Matrix [[0.0, 2.0, 3.0], [2.0, 0.0, 5.0], [2.0, 0.0, 5.0]]
 
 2x3 Matrix [[0.0, 2.0, 3.0], [2.0, 0.0, 5.0]]
@@ -43,7 +43,7 @@ type
 
   Matrix*[M: static[int]; N: static[int]; T] =
     Vector[M, Vector[N, T]]
-  
+
 proc arrayTest =
   # every kind of square matrix works just fine
   let mat_good: Matrix[3, 3, float] = [[0.0, 2.0, 3.0],

tests/js/trefbyvar.nim

@@ -66,4 +66,4 @@ proc initTypeA1(a: int; b: string; c: pointer = nil): TypeA1 =
   result.c_impl = c
 
 let x = initTypeA1(1, "a")
-doAssert($x == "(a_impl: 1, b_impl: a, c_impl: ...)")
+doAssert($x == "(a_impl: 1, b_impl: \"a\", c_impl: ...)")

tests/metatype/ttypedesc2.nim

@@ -1,5 +1,5 @@
 discard """
-  output: "(x: a)"
+  output: '''(x: 'a')'''
 """
 
 type

tests/misc/tlocals.nim

@@ -1,5 +1,5 @@
 discard """
-  output: "(x: string here, a: 1)"
+  output: '''(x: "string here", a: 1)'''
 """
 
 proc simple[T](a: T) =

tests/objects/tobjconstr.nim

@@ -1,14 +1,14 @@
 discard """
-  output: '''(k: kindA, a: (x: abc, z: [1, 1, 3]), method: ())
-(k: kindA, a: (x: abc, z: [1, 2, 3]), method: ())
-(k: kindA, a: (x: abc, z: [1, 3, 3]), method: ())
-(k: kindA, a: (x: abc, z: [1, 4, 3]), method: ())
-(k: kindA, a: (x: abc, z: [1, 5, 3]), method: ())
-(k: kindA, a: (x: abc, z: [1, 6, 3]), method: ())
-(k: kindA, a: (x: abc, z: [1, 7, 3]), method: ())
-(k: kindA, a: (x: abc, z: [1, 8, 3]), method: ())
-(k: kindA, a: (x: abc, z: [1, 9, 3]), method: ())
-(k: kindA, a: (x: abc, z: [1, 10, 3]), method: ())
+  output: '''(k: kindA, a: (x: "abc", z: [1, 1, 3]), method: ())
+(k: kindA, a: (x: "abc", z: [1, 2, 3]), method: ())
+(k: kindA, a: (x: "abc", z: [1, 3, 3]), method: ())
+(k: kindA, a: (x: "abc", z: [1, 4, 3]), method: ())
+(k: kindA, a: (x: "abc", z: [1, 5, 3]), method: ())
+(k: kindA, a: (x: "abc", z: [1, 6, 3]), method: ())
+(k: kindA, a: (x: "abc", z: [1, 7, 3]), method: ())
+(k: kindA, a: (x: "abc", z: [1, 8, 3]), method: ())
+(k: kindA, a: (x: "abc", z: [1, 9, 3]), method: ())
+(k: kindA, a: (x: "abc", z: [1, 10, 3]), method: ())
 (x: 123)
 (x: 123)
 (z: 89, y: 0, x: 128)

tests/showoff/thello2.nim

@@ -1,5 +1,5 @@
 discard """
-  output: '''(a: 3, b: 4, s: abc)'''
+  output: '''(a: 3, b: 4, s: "abc")'''
 """
 
 type

tests/stdlib/tlists.nim

@@ -17,7 +17,7 @@ block SinglyLinkedListTest1:
 block SinglyLinkedListTest2:
   var L: TSinglyLinkedList[string]
   for d in items(data): L.prepend($d)
-  assert($L == "[6, 5, 4, 3, 2, 1]")
+  assert($L == """["6", "5", "4", "3", "2", "1"]""")
 
   assert("4" in L)
 

tests/stdlib/treloop.nim

@@ -1,5 +1,5 @@
 discard """
-  output: "@[(, +,  1,  2, )]"
+  output: '''@["(", "+", " 1", " 2", ")"]'''
 """
 
 import re

tests/system/toString.nim

@@ -4,12 +4,12 @@ discard """
 
 doAssert "@[23, 45]" == $(@[23, 45])
 doAssert "[32, 45]" == $([32, 45])
-doAssert "@[, foo, bar]" == $(@["", "foo", "bar"])
-doAssert "[, foo, bar]" ==  $(["", "foo", "bar"])
+doAssert """@["", "foo", "bar"]""" == $(@["", "foo", "bar"])
+doAssert """["", "foo", "bar"]""" ==  $(["", "foo", "bar"])
 
 # bug #2395
 let alphaSet: set[char] = {'a'..'c'}
-doAssert "{a, b, c}" == $alphaSet
+doAssert "{'a', 'b', 'c'}" == $alphaSet
 doAssert "2.3242" == $(2.3242)
 doAssert "2.982" == $(2.982)
 doAssert "123912.1" == $(123912.1)
@@ -49,5 +49,5 @@ import strutils
 # array test
 
 let arr = ['H','e','l','l','o',' ','W','o','r','l','d','!','\0']
-doAssert $arr == "[H, e, l, l, o,  , W, o, r, l, d, !, \0]"
+doAssert $arr == "['H', 'e', 'l', 'l', 'o', ' ', 'W', 'o', 'r', 'l', 'd', '!', '\\x00']"
 doAssert $cstring(unsafeAddr arr) == "Hello World!"

tests/typerel/typeof_in_template.nim

@@ -1,5 +1,5 @@
 discard """
-  output: '''@[a, c]'''
+  output: '''@["a", "c"]'''
 """
 
 # bug #3230

tests/types/tinheritpartialgeneric.nim

@@ -1,6 +1,6 @@
 discard """
-  output: '''(c: hello, a: 10, b: 12.0)
-(a: 15.5, b: hello)
+  output: '''(c: "hello", a: 10, b: 12.0)
+(a: 15.5, b: "hello")
 (a: 11.75, b: 123)'''
 """
 

tests/types/tparameterizedparent2.nim

@@ -2,7 +2,7 @@ discard """
   output: '''(width: 11, color: 13)
 (width: 15, weight: 13, taste: 11, color: 14)
 (width: 17, color: 16)
-(width: 12.0, taste: yummy, color: 13)
+(width: 12.0, taste: "yummy", color: 13)
 (width: 0, tast_e: 0.0, kind: Smooth, skin: 1.5, color: 12)'''
 """
 # bug #5264

tests/vm/tableinstatic.nim

@@ -2,7 +2,7 @@ discard """
   nimout: '''0
 0
 0
-{hallo: 123, welt: 456}'''
+{"hallo": "123", "welt": "456"}'''
 """
 
 import tables

tests/vm/tconstobj.nim

@@ -1,5 +1,5 @@
 discard """
-  output: '''(name: hello)
+  output: '''(name: "hello")
 (-1, 0)'''
 """