fix js stacktraces, unify all file,line,col formatting into a single function (#14230)

* fix https://github.com/timotheecour/Nim/issues/135 ; unify all file,line,col formatting into a single function

compiler/jsgen.nim

@@ -2243,7 +2243,7 @@ proc genProcBody(p: PProc, prc: PSym): Rope =
   if hasFrameInfo(p):
     result = frameCreate(p,
               makeJSString(prc.owner.name.s & '.' & prc.name.s),
-              makeJSString(toFilename(p.config, prc.info)))
+              makeJSString(toFilenameOption(p.config, prc.info.fileIndex, foStacktrace)))
   else:
     result = nil
   if p.beforeRetNeeded:
@@ -2583,7 +2583,7 @@ proc genModule(p: PProc, n: PNode) =
   if optStackTrace in p.options:
     p.body.add(frameCreate(p,
         makeJSString("module " & p.module.module.name.s),
-        makeJSString(toFilename(p.config, p.module.module.info))))
+        makeJSString(toFilenameOption(p.config, p.module.module.info.fileIndex, foStacktrace))))
   var transformedN = transformStmt(p.module.graph, p.module.module, n)
   if sfInjectDestructors in p.module.module.flags:
     transformedN = injectDestructorCalls(p.module.graph, p.module.module, transformedN)

compiler/msgs.nim

@@ -10,6 +10,7 @@
 import
   options, strutils, os, tables, ropes, terminal, macros,
   lineinfos, pathutils
+import std/private/miscdollars
 
 proc toCChar*(c: char; result: var string) =
   case c
@@ -132,7 +133,6 @@ proc suggestQuit*() =
 # this format is understood by many text editors: it is the same that
 # Borland and Freepascal use
 const
-  PosFormat    = "$1($2, $3) "
   KindFormat   = " [$1]"
   KindColor    = fgCyan
   ErrorTitle   = "Error: "
@@ -213,16 +213,38 @@ template toFullPath*(conf: ConfigRef; info: TLineInfo): string =
 template toFullPathConsiderDirty*(conf: ConfigRef; info: TLineInfo): string =
   string toFullPathConsiderDirty(conf, info.fileIndex)
 
+type FilenameOption* = enum
+  foAbs # absolute path, eg: /pathto/bar/foo.nim
+  foRelProject # relative to project path, eg: ../foo.nim
+  foMagicSauce # magic sauce, shortest of (foAbs, foRelProject)
+  foName # lastPathPart, eg: foo.nim
+  foShort # foName without extension, eg: foo
+  foStacktrace # if optExcessiveStackTrace: foAbs else: foName
+
+proc toFilenameOption*(conf: ConfigRef, fileIdx: FileIndex, opt: FilenameOption): string =
+  case opt
+  of foAbs: result = toFullPath(conf, fileIdx)
+  of foRelProject: result = toProjPath(conf, fileIdx)
+  of foMagicSauce:
+    let
+      absPath = toFullPath(conf, fileIdx)
+      relPath = toProjPath(conf, fileIdx)
+    result = if (optListFullPaths in conf.globalOptions) or
+                (relPath.len > absPath.len) or
+                (relPath.count("..") > 2):
+               absPath
+             else:
+               relPath
+  of foName: result = toProjPath(conf, fileIdx).lastPathPart
+  of foShort: result = toFilename(conf, fileIdx)
+  of foStacktrace:
+    if optExcessiveStackTrace in conf.globalOptions:
+      result = toFilenameOption(conf, fileIdx, foAbs)
+    else:
+      result = toFilenameOption(conf, fileIdx, foName)
+
 proc toMsgFilename*(conf: ConfigRef; info: FileIndex): string =
-  let
-    absPath = toFullPath(conf, info)
-    relPath = toProjPath(conf, info)
-  result = if (optListFullPaths in conf.globalOptions) or
-              (relPath.len > absPath.len) or
-              (relPath.count("..") > 2):
-             absPath
-           else:
-             relPath
+  toFilenameOption(conf, info, foMagicSauce)
 
 template toMsgFilename*(conf: ConfigRef; info: TLineInfo): string =
   toMsgFilename(conf, info.fileIndex)
@@ -234,9 +256,7 @@ proc toColumn*(info: TLineInfo): int {.inline.} =
   result = info.col
 
 proc toFileLineCol*(conf: ConfigRef; info: TLineInfo): string {.inline.} =
-  # consider calling `helpers.lineInfoToString` instead
-  result = toMsgFilename(conf, info) & "(" & $info.line & ", " &
-    $(info.col + ColOffset) & ")"
+  result.toLocation(toMsgFilename(conf, info), info.line.int, info.col.int + ColOffset)
 
 proc `$`*(conf: ConfigRef; info: TLineInfo): string = toFileLineCol(conf, info)
 
@@ -322,10 +342,6 @@ template styledMsgWriteln*(args: varargs[typed]) =
       when defined(windows):
         flushFile(stderr)
 
-proc coordToStr(coord: int): string =
-  if coord == -1: result = "???"
-  else: result = $coord
-
 proc msgKindToString*(kind: TMsgKind): string =
   # later versions may provide translated error messages
   result = MsgKindToStr[kind]
@@ -389,12 +405,7 @@ proc writeContext(conf: ConfigRef; lastinfo: TLineInfo) =
           instantiationFrom
         else:
           instantiationOfFrom.format(context.detail)
-        styledMsgWriteln(styleBright,
-                         PosFormat % [toMsgFilename(conf, context.info),
-                                      coordToStr(context.info.line.int),
-                                      coordToStr(context.info.col+ColOffset)],
-                         resetStyle,
-                         message)
+        styledMsgWriteln(styleBright, conf.toFileLineCol(context.info), " ", resetStyle, message)
     info = context.info
 
 proc ignoreMsgBecauseOfIdeTools(conf: ConfigRef; msg: TMsgKind): bool =
@@ -473,10 +484,7 @@ proc formatMsg*(conf: ConfigRef; info: TLineInfo, msg: TMsgKind, arg: string): s
               of warnMin..warnMax: WarningTitle
               of hintMin..hintMax: HintTitle
               else: ErrorTitle
-  result = PosFormat % [toMsgFilename(conf, info), coordToStr(info.line.int),
-                        coordToStr(info.col+ColOffset)] &
-           title &
-           getMessageStr(msg, arg)
+  conf.toFileLineCol(info) & " " & title & getMessageStr(msg, arg)
 
 proc liMessage(conf: ConfigRef; info: TLineInfo, msg: TMsgKind, arg: string,
                eh: TErrorHandling) =
@@ -511,8 +519,7 @@ proc liMessage(conf: ConfigRef; info: TLineInfo, msg: TMsgKind, arg: string,
     color = HintColor
     if msg != hintUserRaw: kind = HintsToStr[ord(msg) - ord(hintMin)]
     inc(conf.hintCounter)
-  let x = PosFormat % [toMsgFilename(conf, info), coordToStr(info.line.int),
-                       coordToStr(info.col+ColOffset)]
+  let x = conf.toFileLineCol(info) & " "
   let s = getMessageStr(msg, arg)
 
   if not ignoreMsg:

lib/std/private/miscdollars.nim

@@ -0,0 +1,15 @@
+template toLocation*(result: var string, file: string | cstring, line: int, col: int) =
+  ## avoids spurious allocations
+  # Hopefully this can be re-used everywhere so that if a user needs to customize,
+  # it can be done in a single place.
+  result.add file
+  if line > 0:
+    result.add "("
+    # simplify this after moving moving `include strmantle` above import assertions`
+    when declared(addInt): result.addInt line
+    else: result.add $line
+    if col > 0:
+      result.add ", "
+      when declared(addInt): result.addInt col
+      else: result.add $col
+    result.add ")"

lib/system/assertions.nim

@@ -1,17 +1,17 @@
 when not declared(sysFatal):
   include "system/fatal"
 
+import std/private/miscdollars
 # ---------------------------------------------------------------------------
 # helpers
 
 type InstantiationInfo = tuple[filename: string, line: int, column: int]
 
 proc `$`(x: int): string {.magic: "IntToStr", noSideEffect.}
-
 proc `$`(info: InstantiationInfo): string =
   # The +1 is needed here
   # instead of overriding `$` (and changing its meaning), consider explicit name.
-  info.filename & "(" & $info.line & ", " & $(info.column+1) & ")"
+  result.toLocation(info.filename, info.line, info.column+1)
 
 # ---------------------------------------------------------------------------
 

lib/system/excpt.nim

@@ -10,6 +10,8 @@
 # Exception handling code. Carefully coded so that tiny programs which do not
 # use the heap (and nor exceptions) do not include the GC or memory allocator.
 
+import std/private/miscdollars
+
 var
   errorMessageWriter*: (proc(msg: string) {.tags: [WriteIOEffect], benign,
                                             nimcall.})
@@ -240,11 +242,7 @@ proc auxWriteStackTrace(f: PFrame; s: var seq[StackTraceEntry]) =
 
 template addFrameEntry(s: var string, f: StackTraceEntry|PFrame) =
   var oldLen = s.len
-  add(s, f.filename)
-  if f.line > 0:
-    add(s, '(')
-    add(s, $f.line)
-    add(s, ')')
+  s.toLocation(f.filename, f.line, 0)
   for k in 1..max(1, 25-(s.len-oldLen)): add(s, ' ')
   add(s, f.procname)
   when NimStackTraceMsgs:

lib/system/jssys.nim

@@ -8,6 +8,7 @@
 #
 
 include system/indexerrors
+import std/private/miscdollars
 
 proc log*(s: cstring) {.importc: "console.log", varargs, nodecl.}
 
@@ -70,7 +71,7 @@ proc getCurrentExceptionMsg*(): string =
 
 proc auxWriteStackTrace(f: PCallFrame): string =
   type
-    TempFrame = tuple[procname: cstring, line: int]
+    TempFrame = tuple[procname: cstring, line: int, filename: cstring]
   var
     it = f
     i = 0
@@ -79,6 +80,7 @@ proc auxWriteStackTrace(f: PCallFrame): string =
   while it != nil and i <= high(tempFrames):
     tempFrames[i].procname = it.procname
     tempFrames[i].line = it.line
+    tempFrames[i].filename = it.filename
     inc(i)
     inc(total)
     it = it.prev
@@ -92,10 +94,9 @@ proc auxWriteStackTrace(f: PCallFrame): string =
     add(result, $(total-i))
     add(result, " calls omitted) ...\n")
   for j in countdown(i-1, 0):
+    result.toLocation($tempFrames[j].filename, tempFrames[j].line, 0)
+    add(result, " at ")
     add(result, tempFrames[j].procname)
-    if tempFrames[j].line > 0:
-      add(result, ", line: ")
-      add(result, $tempFrames[j].line)
     add(result, "\n")
 
 proc rawWriteStackTrace(): string =

tests/js/t7224.nim

@@ -1,10 +1,11 @@
 discard """
   cmd: "nim $target $options --stackTrace:on --lineTrace:on $file"
   outputsub: '''
-t7224.aaa, line: 21
-t7224.bbb, line: 18
-t7224.ccc, line: 15
-t7224.ddd, line: 12
+t7224.nim(25) at module t7224
+t7224.nim(22) at t7224.aaa
+t7224.nim(19) at t7224.bbb
+t7224.nim(16) at t7224.ccc
+t7224.nim(13) at t7224.ddd
 '''
 """