macros.treeRepr + friends: collapse SymChoice (#18072)

* macros.treeRepr + friends: collapse SymChoice

* make repr+friends work with invalid symchoice nodes

* address comment

4 files changed, 158 insertions, 50 deletions

diff --git a/changelog.md b/changelog.md
index 06251abfc..1050e4916 100644
--- a/changelog.md
+++ b/changelog.md
@@ -81,6 +81,9 @@
 
 - Deprecated `proc reversed*[T](a: openArray[T], first: Natural, last: int): seq[T]` in `std/algorithm`.
 
+-  In `std/macros`, `treeRepr,lispRepr,astGenRepr` now represent SymChoice nodes in a collapsed way,
+   use `-d:nimLegacyMacrosCollapseSymChoice` to get previous behavior.
+
 - The configuration subsystem now allows for `-d:release` and `-d:danger` to work as expected.
   The downside is that these defines now have custom logic that doesn't apply for
   other defines.
diff --git a/lib/core/macros.nim b/lib/core/macros.nim
index 89f90a23b..37f855d94 100644
--- a/lib/core/macros.nim
+++ b/lib/core/macros.nim
@@ -856,6 +856,29 @@ proc nestList*(op: NimNode; pack: NimNode; init: NimNode): NimNode =
   for i in countdown(pack.len - 1, 0):
     result = newCall(op, pack[i], result)
 
+proc eqIdent*(a: string; b: string): bool {.magic: "EqIdent", noSideEffect.}
+  ## Style insensitive comparison.
+
+proc eqIdent*(a: NimNode; b: string): bool {.magic: "EqIdent", noSideEffect.}
+  ## Style insensitive comparison.  `a` can be an identifier or a
+  ## symbol. `a` may be wrapped in an export marker
+  ## (`nnkPostfix`) or quoted with backticks (`nnkAccQuoted`),
+  ## these nodes will be unwrapped.
+
+proc eqIdent*(a: string; b: NimNode): bool {.magic: "EqIdent", noSideEffect.}
+  ## Style insensitive comparison.  `b` can be an identifier or a
+  ## symbol. `b` may be wrapped in an export marker
+  ## (`nnkPostfix`) or quoted with backticks (`nnkAccQuoted`),
+  ## these nodes will be unwrapped.
+
+proc eqIdent*(a: NimNode; b: NimNode): bool {.magic: "EqIdent", noSideEffect.}
+  ## Style insensitive comparison.  `a` and `b` can be an
+  ## identifier or a symbol. Both may be wrapped in an export marker
+  ## (`nnkPostfix`) or quoted with backticks (`nnkAccQuoted`),
+  ## these nodes will be unwrapped.
+
+const collapseSymChoice = not defined(nimLegacyMacrosCollapseSymChoice)
+
 proc treeTraverse(n: NimNode; res: var string; level = 0; isLisp = false, indented = false) {.benign.} =
   if level > 0:
     if indented:
@@ -883,8 +906,21 @@ proc treeTraverse(n: NimNode; res: var string; level = 0; isLisp = false, indent
     res.add(" " & $n.strVal.newLit.repr)
   of nnkNone:
     assert false
+  elif n.kind in {nnkOpenSymChoice, nnkClosedSymChoice} and collapseSymChoice:
+    res.add(" " & $n.len)
+    if n.len > 0:
+      var allSameSymName = true
+      for i in 0..<n.len:
+        if n[i].kind != nnkSym or not eqIdent(n[i], n[0]):
+          allSameSymName = false
+          break
+      if allSameSymName:
+        res.add(" " & $n[0].strVal.newLit.repr)
+      else:
+        for j in 0 ..< n.len:
+          n[j].treeTraverse(res, level+1, isLisp, indented)
   else:
-    for j in 0 .. n.len-1:
+    for j in 0 ..< n.len:
       n[j].treeTraverse(res, level+1, isLisp, indented)
 
   if isLisp:
@@ -932,6 +968,10 @@ proc astGenRepr*(n: NimNode): string {.benign.} =
     of nnkStrLit..nnkTripleStrLit, nnkCommentStmt, nnkIdent, nnkSym:
       res.add(n.strVal.newLit.repr)
     of nnkNone: assert false
+    elif n.kind in {nnkOpenSymChoice, nnkClosedSymChoice} and collapseSymChoice:
+      res.add(", # unrepresentable symbols: " & $n.len)
+      if n.len > 0:
+        res.add(" " & n[0].strVal.newLit.repr)
     else:
       res.add(".newTree(")
       for j in 0..<n.len:
@@ -1394,27 +1434,6 @@ proc copy*(node: NimNode): NimNode =
   ## An alias for `copyNimTree<#copyNimTree,NimNode>`_.
   return node.copyNimTree()
 
-proc eqIdent*(a: string; b: string): bool {.magic: "EqIdent", noSideEffect.}
-  ## Style insensitive comparison.
-
-proc eqIdent*(a: NimNode; b: string): bool {.magic: "EqIdent", noSideEffect.}
-  ## Style insensitive comparison.  `a` can be an identifier or a
-  ## symbol. `a` may be wrapped in an export marker
-  ## (`nnkPostfix`) or quoted with backticks (`nnkAccQuoted`),
-  ## these nodes will be unwrapped.
-
-proc eqIdent*(a: string; b: NimNode): bool {.magic: "EqIdent", noSideEffect.}
-  ## Style insensitive comparison.  `b` can be an identifier or a
-  ## symbol. `b` may be wrapped in an export marker
-  ## (`nnkPostfix`) or quoted with backticks (`nnkAccQuoted`),
-  ## these nodes will be unwrapped.
-
-proc eqIdent*(a: NimNode; b: NimNode): bool {.magic: "EqIdent", noSideEffect.}
-  ## Style insensitive comparison.  `a` and `b` can be an
-  ## identifier or a symbol. Both may be wrapped in an export marker
-  ## (`nnkPostfix`) or quoted with backticks (`nnkAccQuoted`),
-  ## these nodes will be unwrapped.
-
 proc expectIdent*(n: NimNode, name: string) {.since: (1,1).} =
   ## Check that `eqIdent(n,name)` holds true. If this is not the
   ## case, compilation aborts with an error message. This is useful
diff --git a/tests/macros/tdumpast.nim b/tests/macros/tdumpast.nim
index b6bcbe8f0..484b3c2f3 100644
--- a/tests/macros/tdumpast.nim
+++ b/tests/macros/tdumpast.nim
@@ -2,34 +2,69 @@
 
 import macros
 
-template plus(a, b: untyped): untyped {.dirty} =
-  a + b
-
-macro call(e: untyped): untyped =
-  result = newCall("foo", newStrLitNode("bar"))
-
-macro dumpAST(n: untyped): untyped =
-  # dump AST as a side-effect and return the inner node
-  let n = callsite()
-  echo n.lispRepr
-  echo n.treeRepr
-
-  var plusAst = getAst(plus(1, 2))
-  echo plusAst.lispRepr
-
-  var callAst = getAst(call(4))
-  echo callAst.lispRepr
-
-  var e = parseExpr("foo(bar + baz)")
-  echo e.lispRepr
-
-  result = n[1]
-
-dumpAST:
-  proc add(x, y: int): int =
-    return x + y
-
-  proc sub(x, y: int): int = return x - y
+block:
+  template plus(a, b: untyped): untyped {.dirty} =
+    a + b
+
+  macro call(e: untyped): untyped =
+    result = newCall("foo", newStrLitNode("bar"))
+
+  macro dumpAST(n: untyped): string =
+    var msg = ""
+    msg.add "lispRepr:\n" & n.lispRepr & "\n"
+    msg.add "treeRepr:\n" & n.treeRepr & "\n"
+
+    var plusAst = getAst(plus(1, 2))
+    msg.add "lispRepr:\n" & n.lispRepr & "\n"
+
+    var callAst = getAst(call(4))
+    msg.add "callAst.lispRepr:\n" & callAst.lispRepr & "\n"
+
+    var e = parseExpr("foo(bar + baz)")
+    msg.add "e.lispRepr:\n" & e.lispRepr & "\n"
+    result = msg.newLit
+
+  let a = dumpAST:
+    proc add(x, y: int): int =
+      return x + y
+    const foo = 3
+
+  doAssert a == """
+lispRepr:
+(StmtList (ProcDef (Ident "add") (Empty) (Empty) (FormalParams (Ident "int") (IdentDefs (Ident "x") (Ident "y") (Ident "int") (Empty))) (Empty) (Empty) (StmtList (ReturnStmt (Infix (Ident "+") (Ident "x") (Ident "y"))))) (ConstSection (ConstDef (Ident "foo") (Empty) (IntLit 3))))
+treeRepr:
+StmtList
+  ProcDef
+    Ident "add"
+    Empty
+    Empty
+    FormalParams
+      Ident "int"
+      IdentDefs
+        Ident "x"
+        Ident "y"
+        Ident "int"
+        Empty
+    Empty
+    Empty
+    StmtList
+      ReturnStmt
+        Infix
+          Ident "+"
+          Ident "x"
+          Ident "y"
+  ConstSection
+    ConstDef
+      Ident "foo"
+      Empty
+      IntLit 3
+lispRepr:
+(StmtList (ProcDef (Ident "add") (Empty) (Empty) (FormalParams (Ident "int") (IdentDefs (Ident "x") (Ident "y") (Ident "int") (Empty))) (Empty) (Empty) (StmtList (ReturnStmt (Infix (Ident "+") (Ident "x") (Ident "y"))))) (ConstSection (ConstDef (Ident "foo") (Empty) (IntLit 3))))
+callAst.lispRepr:
+(Call (Ident "foo") (StrLit "bar"))
+e.lispRepr:
+(Call (Ident "foo") (Infix (Ident "+") (Ident "bar") (Ident "baz")))
+"""
 
 macro fun() =
   let n = quote do:
@@ -79,3 +114,48 @@ block: # repr
   doAssert repr2((1, 2, 3.0)) == "(1, 2, 3.0)"
   doAssert repr2((1)) == "(1)"
   doAssert repr2((1+2)) == "(1 + 2)"
+
+block: # treeRepr
+  macro treeRepr2(a: untyped): string = newLit a.treeRepr
+  macro treeRepr3(a: typed): string = newLit a.treeRepr
+
+  doAssert treeRepr2(1+1 == 2) == """
+Infix
+  Ident "=="
+  Infix
+    Ident "+"
+    IntLit 1
+    IntLit 1
+  IntLit 2"""
+
+  proc baz() = discard
+  proc baz(a: int) = discard
+  proc baz(a: float) = discard
+
+  doAssert treeRepr3(baz()) == """
+Call
+  Sym "baz""""
+
+  let a = treeRepr3(block:
+    proc bar(a: auto) = baz())
+  doAssert a == """
+BlockStmt
+  Empty
+  ProcDef
+    Sym "bar"
+    Empty
+    GenericParams
+      Sym "a:type"
+    FormalParams
+      Empty
+      IdentDefs
+        Sym "a"
+        Sym "auto"
+        Empty
+    Empty
+    Bracket
+      Empty
+      Empty
+    StmtList
+      Call
+        OpenSymChoice 3 "baz""""
diff --git a/tests/stdlib/tmacros.nim b/tests/stdlib/tmacros.nim
index 52ea1e898..18cfdc75c 100644
--- a/tests/stdlib/tmacros.nim
+++ b/tests/stdlib/tmacros.nim
@@ -1,3 +1,9 @@
+#[
+xxx macros tests need to be reorganized to makes sure each API is tested once
+See also:
+  tests/macros/tdumpast.nim for treeRepr + friends
+]#
+
 import std/macros
 
 block: # hasArgOfName