doAssert, assert now print full path of failing line on error (#8555)

6 files changed, 108 insertions, 31 deletions

diff --git a/lib/core/macros.nim b/lib/core/macros.nim
index 90fea440e..bac9ce7a2 100644
--- a/lib/core/macros.nim
+++ b/lib/core/macros.nim
@@ -8,6 +8,7 @@
 #
 
 include "system/inclrtl"
+include "system/helpers"
 
 ## This module contains the interface to the compiler's abstract syntax
 ## tree (`AST`:idx:). Macros operate on this tree.
@@ -422,7 +423,8 @@ type
     line*,column*: int
 
 proc `$`*(arg: Lineinfo): string =
-  result = arg.filename & "(" & $arg.line & ", " & $arg.column & ")"
+  # BUG: without `result = `, gives compile error
+  result = lineInfoToString(arg.filename, arg.line, arg.column)
 
 #proc lineinfo*(n: NimNode): LineInfo {.magic: "NLineInfo", noSideEffect.}
   ## returns the position the node appears in the original source file
diff --git a/lib/system.nim b/lib/system.nim
index dcfb04c5a..8b98706ab 100644
--- a/lib/system.nim
+++ b/lib/system.nim
@@ -3758,6 +3758,16 @@ proc failedAssertImpl*(msg: string) {.raises: [], tags: [].} =
                                     tags: [].}
   Hide(raiseAssert)(msg)
 
+include "system/helpers" # for `lineInfoToString`
+
+template assertImpl(cond: bool, msg = "", enabled: static[bool]) =
+  const loc = $instantiationInfo(-1, true)
+  bind instantiationInfo
+  mixin failedAssertImpl
+  when enabled:
+    if not cond:
+      failedAssertImpl(loc & " `" & astToStr(cond) & "` " & msg)
+
 template assert*(cond: bool, msg = "") =
   ## Raises ``AssertionError`` with `msg` if `cond` is false. Note
   ## that ``AssertionError`` is hidden from the effect system, so it doesn't
@@ -3767,23 +3777,11 @@ template assert*(cond: bool, msg = "") =
   ## The compiler may not generate any code at all for ``assert`` if it is
   ## advised to do so through the ``-d:release`` or ``--assertions:off``
   ## `command line switches <nimc.html#command-line-switches>`_.
-  # NOTE: `true` is correct here; --excessiveStackTrace:on will control whether
-  # or not to output full paths.
-  bind instantiationInfo
-  mixin failedAssertImpl
-  {.line: instantiationInfo(fullPaths = true).}:
-    when compileOption("assertions"):
-      if not cond: failedAssertImpl(astToStr(cond) & ' ' & msg)
+  assertImpl(cond, msg, compileOption("assertions"))
 
 template doAssert*(cond: bool, msg = "") =
-  ## same as `assert` but is always turned on and not affected by the
-  ## ``--assertions`` command line switch.
-  # NOTE: `true` is correct here; --excessiveStackTrace:on will control whether
-  # or not to output full paths.
-  bind instantiationInfo
-  mixin failedAssertImpl
-  {.line: instantiationInfo(fullPaths = true).}:
-    if not cond: failedAssertImpl(astToStr(cond) & ' ' & msg)
+  ## same as ``assert`` but is always turned on regardless of ``--assertions``
+  assertImpl(cond, msg, true)
 
 iterator items*[T](a: seq[T]): T {.inline.} =
   ## iterates over each item of `a`.
diff --git a/lib/system/helpers.nim b/lib/system/helpers.nim
new file mode 100644
index 000000000..fb1218684
--- /dev/null
+++ b/lib/system/helpers.nim
@@ -0,0 +1,11 @@
+## helpers used system.nim and other modules, avoids code duplication while
+## also minimizing symbols exposed in system.nim
+#
+# TODO: move other things here that should not be exposed in system.nim
+
+proc lineInfoToString(file: string, line, column: int): string =
+  file & "(" & $line & ", " & $column & ")"
+
+proc `$`(info: type(instantiationInfo(0))): string =
+  # The +1 is needed here
+  lineInfoToString(info.fileName, info.line, info.column+1)
diff --git a/tests/assert/testhelper.nim b/tests/assert/testhelper.nim
new file mode 100644
index 000000000..754d562ec
--- /dev/null
+++ b/tests/assert/testhelper.nim
@@ -0,0 +1,12 @@
+from strutils import endsWith, split
+from ospaths import isAbsolute
+
+proc checkMsg*(msg, expectedEnd, name: string)=
+  let filePrefix = msg.split(' ', maxSplit = 1)[0]
+  if not filePrefix.isAbsolute:
+    echo name, ":not absolute: `", msg & "`"
+  elif not msg.endsWith expectedEnd:
+    echo name, ":expected suffix:\n`" & expectedEnd & "`\ngot:\n`" & msg & "`"
+  else:
+    echo name, ":ok"
+
diff --git a/tests/assert/tfailedassert.nim b/tests/assert/tfailedassert.nim
index 8b260a3ab..c3231bb8d 100644
--- a/tests/assert/tfailedassert.nim
+++ b/tests/assert/tfailedassert.nim
@@ -1,20 +1,65 @@
 discard """
   output: '''
-WARNING: false first assertion from bar
-ERROR: false second assertion from bar
+test1:ok
+test2:ok
+test3:ok
+test4:ok
+test5:ok
+test6:ok
+test7:ok
 -1
-tfailedassert.nim:27 false assertion from foo
+tfailedassert.nim
+test7:ok
 '''
 """
 
+import testhelper
+
 type
   TLineInfo = tuple[filename: string, line: int, column: int]
-
   TMyError = object of Exception
     lineinfo: TLineInfo
-
   EMyError = ref TMyError
 
+echo("")
+
+
+# NOTE: when entering newlines, adjust `expectedEnd` ouptuts
+
+try:
+  doAssert(false, "msg1") # doAssert test
+except AssertionError as e:
+  checkMsg(e.msg, "tfailedassert.nim(30, 11) `false` msg1", "test1")
+
+try:
+  assert false, "msg2"  # assert test
+except AssertionError as e:
+  checkMsg(e.msg, "tfailedassert.nim(35, 10) `false` msg2", "test2")
+
+try:
+  assert false # assert test with no msg
+except AssertionError as e:
+  checkMsg(e.msg, "tfailedassert.nim(40, 10) `false` ", "test3")
+
+try:
+  let a = 1
+  doAssert(a+a==1) # assert test with Ast expression
+  # BUG: const folding would make "1+1==1" appear as `false` in
+  # assert message
+except AssertionError as e:
+  checkMsg(e.msg, "`a + a == 1` ", "test4")
+
+try:
+  let a = 1
+  doAssert a+a==1 # ditto with `doAssert` and no parens
+except AssertionError as e:
+  checkMsg(e.msg, "`a + a == 1` ", "test5")
+
+proc fooStatic() =
+  # protect against https://github.com/nim-lang/Nim/issues/8758
+  static: doAssert(true)
+fooStatic()
+
 # module-wide policy to change the failed assert
 # exception type in order to include a lineinfo
 onFailedAssert(msg):
@@ -26,26 +71,26 @@ onFailedAssert(msg):
 proc foo =
   assert(false, "assertion from foo")
 
+
 proc bar: int =
-  # local overrides that are active only
-  # in this proc
-  onFailedAssert(msg): echo "WARNING: " & msg
+  # local overrides that are active only in this proc
+  onFailedAssert(msg):
+    checkMsg(msg, "tfailedassert.nim(80, 9) `false` first assertion from bar", "test6")
 
   assert(false, "first assertion from bar")
 
   onFailedAssert(msg):
-    echo "ERROR: " & msg
+    checkMsg(msg, "tfailedassert.nim(86, 9) `false` second assertion from bar", "test7")
     return -1
 
   assert(false, "second assertion from bar")
   return 10
 
-echo("")
 echo(bar())
 
 try:
   foo()
 except:
   let e = EMyError(getCurrentException())
-  echo e.lineinfo.filename, ":", e.lineinfo.line, " ", e.msg
-
+  echo e.lineinfo.filename
+  checkMsg(e.msg, "tfailedassert.nim(72, 9) `false` assertion from foo", "test7")
diff --git a/tests/assert/tfaileddoassert.nim b/tests/assert/tfaileddoassert.nim
index 3195fb406..e1245f578 100644
--- a/tests/assert/tfaileddoassert.nim
+++ b/tests/assert/tfaileddoassert.nim
@@ -1,11 +1,20 @@
 discard """
   cmd: "nim $target -d:release $options $file"
   output: '''
-assertion occured!!!!!! false
+test1:ok
+test2:ok
 '''
 """
 
+import testhelper
+
+onFailedAssert(msg):
+  checkMsg(msg, "tfaileddoassert.nim(15, 9) `a == 2` foo", "test1")
+
+var a = 1
+doAssert(a == 2, "foo")
+
 onFailedAssert(msg):
-  echo("assertion occured!!!!!! ", msg)
+  checkMsg(msg, "tfaileddoassert.nim(20, 10) `a == 3` ", "test2")
 
-doAssert(1 == 2)
+doAssert a == 3