fix length computation of node with postifx comment (fixes #64)

* don't count pending whitespace twice for overflow detection
* fix redundant do call in stmtlistexpr
* fix a few extra empty lines being added
* fix dedented postfix comment attachment

nph.nimble

@@ -65,6 +65,24 @@ proc formatProject(
       cd ".."
   cd "../.."
 
+proc againProject(
+  name, url, branch: string, dirs: openArray[string]
+) =
+  if not dirExists("playground"):
+    mkdir("playground")
+  cd "playground/"
+  cd name
+  for dir in dirs:
+    if dir.len > 0:
+      cd dir
+    try:
+      exec "git ls-files | grep .nim$ | xargs nph"
+      exec "git diff"
+    except: discard
+    if dir.len > 0:
+      cd ".."
+  cd "../.."
+
 proc commitProject(
   name, url, branch: string, dirs: openArray[string]
 ) =
@@ -91,6 +109,10 @@ task play, "Format several popular projects":
   for p in projects:
     formatProject(p[0], p[1], p[2], p[3])
 
+task again, "Format code formatted by replay (instead of raw code)":
+  for p in projects:
+    againProject(p[0], p[1], p[2], p[3])
+
 task replay, "Commit formatted sources":
   for p in projects:
     commitProject(p[0], p[1], p[2], p[3])

src/phast.nim

@@ -1515,20 +1515,6 @@ proc newProcNode*(
 const AttachedOpToStr*: array[TTypeAttachedOp, string] =
   ["=wasMoved", "=destroy", "=copy", "=dup", "=sink", "=trace", "=deepcopy"]
 
-proc mergeLoc(a: var TLoc, b: TLoc) =
-  if a.k == low(typeof(a.k)):
-    a.k = b.k
-
-  if a.storage == low(typeof(a.storage)):
-    a.storage = b.storage
-
-  a.flags.incl b.flags
-  if a.lode == nil:
-    a.lode = b.lode
-
-  if a.r == "":
-    a.r = b.r
-
 proc newSons*(father: Indexable, length: int) =
   setLen(father.sons, length)
 

src/phparser.nim

@@ -308,6 +308,8 @@ proc isRightAssociative(tok: Token): bool {.inline.} =
   # or (tok.ident.s.len > 1 and tok.ident.s[^1] == '>')
 
 proc wrap(a, b: PNode): PNode =
+  a.info = b.info
+  a.endInfo = b.endInfo
   a.prefix = move(b.prefix)
   a.add(b)
   a
@@ -566,14 +568,14 @@ proc dotExpr(p: var Parser, a: PNode): PNode =
     result = y
 
 proc dotLikeExpr(p: var Parser, a: PNode): PNode =
-  var info = p.parLineInfo
-  result = newNodeI(nkInfix, info)
+  result = newNodeI(nkInfix, a.info)
   optInd(p, result)
   var opNode = newIdentNodeP(p.tok.ident, p)
   getTok(p)
   result.add(opNode)
   result.add(a)
   result.add(parseSymbol(p, smAfterDot))
+  setEndInfo(result)
 
 proc qualifiedIdent(p: var Parser): PNode =
   #| qualifiedIdent = symbol ('.' optInd symbolOrKeyword)?
@@ -1032,7 +1034,6 @@ proc parseOperators(
     a.add(b)
     # Reset the "beginning" of the infix to capture empty lines correctly
     a.info = result.info
-    a.endInfo = b.endInfo
 
     result = a
     opPrec = getPrecedence(p.tok)
@@ -1554,9 +1555,12 @@ proc binaryNot(p: var Parser, a: PNode): PNode =
     optInd(p, notOpr)
     let b = primary(p, pmTypeDesc)
     result = newNodeP(nkInfix, p)
+    result.info = a.info
+    result.endInfo = b.endInfo
     result.add notOpr
     result.add a
     result.add b
+    setEndInfo(result)
   else:
     result = a
 
@@ -1876,10 +1880,10 @@ proc parseReturnOrRaise(p: var Parser, kind: TNodeKind): PNode =
     result.add(p.emptyNode)
   else:
     var e = parseExpr(p)
-    splitLookahead(p, e, clPostfix)
+    splitLookahead(p, e, p.currInd, clPostfix)
     e = postExprBlocks(p, e)
     if e.kind != nkEmpty:
-      splitLookahead(p, result, clPostfix)
+      splitLookahead(p, result, p.currInd, clPostfix)
     result.add(e)
   setEndInfo()
 
@@ -2198,7 +2202,7 @@ proc parseSection(
   splitLookahead(p, result, clMid)
   if realInd(p):
     withInd(p):
-      while sameInd(p) or p.tok.tokType == tkComment and p.tok.indent > p.currInd:
+      while sameInd(p) or p.tok.tokType == tkComment and validInd(p):
         case p.tok.tokType
         of tkSymbol, tkAccent, tkParLe:
           var a = defparser(p)
@@ -2210,11 +2214,13 @@ proc parseSection(
         else:
           parMessage(p, errIdentifierExpected, p.tok)
           break
+      addSkipped(p, result)
     if result.len == 0:
       parMessage(p, errIdentifierExpected, p.tok)
   elif p.tok.tokType in {tkSymbol, tkAccent, tkParLe} and p.tok.indent < 0:
     # tkParLe is allowed for ``var (x, y) = ...`` tuple parsing
     result.add(defparser(p))
+    splitLookahead(p, result[^1], p.currInd, clPostfix)
   else:
     parMessage(p, errIdentifierExpected, p.tok)
   setEndInfo()

src/phrenderer.nim

@@ -79,6 +79,7 @@ type
     config*: ConfigRef
     fid: FileIndex
     nl: bool # When computing line length, does a "forced" newline appear
+    firstLen: int # Length of first line
 
   ListFlag = enum
     lfFirstSticky
@@ -195,7 +196,7 @@ proc initSrcGen(g: var TSrcGen, config: ConfigRef) =
   g.idx = 0
   g.buf = ""
   g.pendingNL = -1
-  g.pendingWhitespace = -1
+  g.pendingWhitespace = 0
   g.config = config
 
 proc containsNL(s: string): bool =
@@ -209,7 +210,10 @@ proc containsNL(s: string): bool =
   result = false
 
 proc addTok(g: var TSrcLen, kind: TokType, s: string) =
-  g.nl = g.nl or containsNL(s)
+  if not g.nl:
+    g.nl = containsNL(s)
+    if not g.nl:
+      g.firstLen += s.len
   g.tokens.add TRenderTok(kind: kind, length: s.len)
 
 proc addTok(g: var TSrcGen, kind: TokType, s: string) =
@@ -237,12 +241,12 @@ proc addPendingNL(g: var TOutput) =
 
     g.lineLen = g.pendingNL
     g.pendingNL = -1
-    g.pendingWhitespace = -1
+    g.pendingWhitespace = 0
   elif g.pendingWhitespace >= 1:
     if g.lineLen > g.pendingWhitespace:
       addTok(g, tkSpaces, spaces(g.pendingWhitespace))
 
-    g.pendingWhitespace = -1
+    g.pendingWhitespace = 0
 
 proc optNL(g: var TOutput, indent: int) =
   g.pendingNL = indent
@@ -385,10 +389,8 @@ proc hasIndent(n: PNode): bool =
     nkObjectTy, nkEnumTy, nkBlockStmt, nkBlockExpr,
   }
 
-const postExprBlocks = {
-  nkStmtList, nkStmtListExpr, nkOfBranch, nkElifBranch, nkElse, nkExceptBranch,
-  nkFinally, nkDo,
-}
+const postExprBlocks =
+  {nkStmtList, nkOfBranch, nkElifBranch, nkElse, nkExceptBranch, nkFinally, nkDo}
 
 proc isStackedCall(n: PNode, inCall: bool): bool =
   # At least two calls to enable "stacking" mode
@@ -565,39 +567,25 @@ proc gstmts(g: var TOutput, n: PNode, flags: SubFlags = {}, doIndent = true)
 
 template withSrcLen(g: TSrcGen, body: untyped): LineLen =
   var sl {.inject.} = TSrcLen.init(g)
-  let pre = sl.lineLen
+  # We don't count pending whitespace towards the length of the body because it
+  # is already accounted for in `lineLen` and iff the body ends up on a new
+  # line, the pending whitespace will not actually be added to the output
+  let discount = g.pendingWhitespace
   body
   let post =
     if sl.nl:
       MaxLineLen + 1
+    elif sl.firstLen > 0:
+      sl.firstLen - discount
     else:
-      sl.lineLen - pre
-  (post, sl.nl)
-
-template withSrcLenNl(g: TSrcGen, nlParam: bool, body: untyped): LineLen =
-  var sl {.inject.} = TSrcLen.init(g)
-  if nlParam:
-    optNL(sl)
-    addPendingNL(sl)
-    sl.nl = false
-
-  let pre = sl.lineLen
-  body
-  let post =
-    if sl.nl:
-      MaxLineLen + 1
-    else:
-      sl.lineLen - pre
+      0
   (post, sl.nl)
 
 template withSrcLen(g: TSrcLen, body: untyped): LineLen =
   (0, false)
 
-template withSrcLenNl(g: TSrcLen, nlParam: bool, body: untyped): LineLen =
-  (0, false)
-
-proc lsub(g: TOutput, n: PNode, flags: SubFlags = {}, extra = 0, nl = false): LineLen =
-  withSrcLenNl(g, nl):
+proc lsub(g: TOutput, n: PNode, flags: SubFlags = {}, extra = 0): LineLen =
+  withSrcLen(g):
     gsub(sl, n, flags, extra)
 
 proc lsons(
@@ -789,7 +777,8 @@ proc gcomma(
   # avoid wasting significant vertical space on lists of numbers and the like)
   let
     onePerLine =
-      if not overflows(
+      count > 1 and
+      overflows(
         g,
         lcomma(
           g,
@@ -801,14 +790,11 @@ proc gcomma(
           flags - {lfFirstSticky, lfFirstAlone},
           subFlags,
         ),
-      ):
-        false
-      else:
-        count > 1 and
-          anyIt(
-            n.sons[sstart + ord(lfFirstComplex in flags) .. n.len + theEnd],
-            not isSimple(it, n.kind == nkIdentDefs),
-          )
+      ) and
+      anyIt(
+        n.sons[sstart + ord(lfFirstComplex in flags) .. n.len + theEnd],
+        not isSimple(it, n.kind == nkIdentDefs),
+      )
     sepAtEnd = lfSepAtEnd in flags
     longSepAtEnd = lfLongSepAtEnd in flags and count > 1
 
@@ -921,7 +907,6 @@ proc gsection(g: var TOutput, n: PNode, kind: TokType, k: string) =
         optNL(g)
       gsub(g, n[i])
 
-    optNL(g)
     dedent(g)
   else:
     gsub(g, n[0])
@@ -1394,9 +1379,9 @@ proc postStatements(
     else:
       optSpace(g)
       put(g, tkDo, "do")
-      put(g, tkColon, ":")
+      putWithSpace(g, tkColon, ":")
   elif not skipColon:
-    put(g, tkColon, ":")
+    putWithSpace(g, tkColon, ":")
 
   gsub(g, n[i])
 
@@ -1407,7 +1392,7 @@ proc postStatements(
     elif n[j].kind in {nkStmtList, nkStmtListExpr}:
       optNL(g)
       put(g, tkDo, "do")
-      put(g, tkColon, ":")
+      putWithSpace(g, tkColon, ":")
 
     gsub(g, n[j])
 

tests/after/comments.nim

@@ -468,3 +468,8 @@ discard
     a and # infix post-operator
     b # infix post
   ) # infix post par
+
+block:
+  block:
+    discard
+  # dedented comment post discard

tests/after/comments.nim.nph.yaml

@@ -1059,8 +1059,8 @@ sons:
           - kind: "nkEmpty"
           - kind: "nkIntLit"
             intVal: 52
-    postfix:
-      - "# let all on one line"
+        postfix:
+          - "# let all on one line"
   - kind: "nkLetSection"
     sons:
       - kind: "nkIdentDefs"
@@ -1160,8 +1160,8 @@ sons:
           - kind: "nkEmpty"
           - kind: "nkIntLit"
             intVal: 5
-    postfix:
-      - "# let section postfix"
+      - kind: "nkCommentStmt"
+        "comment": "# let section postfix"
   - kind: "nkConstSection"
     sons:
       - kind: "nkConstDef"
@@ -1178,8 +1178,8 @@ sons:
           - kind: "nkEmpty"
           - kind: "nkIntLit"
             intVal: 5
-    postfix:
-      - "# const section postfix"
+      - kind: "nkCommentStmt"
+        "comment": "# const section postfix"
   - kind: "nkDiscardStmt"
     sons:
       - kind: "nkIntLit"
@@ -1781,3 +1781,18 @@ sons:
               - "# infix post"
         postfix:
           - "# infix post par"
+  - kind: "nkBlockStmt"
+    sons:
+      - kind: "nkEmpty"
+      - kind: "nkStmtList"
+        sons:
+          - kind: "nkBlockStmt"
+            sons:
+              - kind: "nkEmpty"
+              - kind: "nkStmtList"
+                sons:
+                  - kind: "nkDiscardStmt"
+                    sons:
+                      - kind: "nkEmpty"
+          - kind: "nkCommentStmt"
+            "comment": "# dedented comment post discard"

tests/before/comments.nim

@@ -449,3 +449,8 @@ discard
     a and # infix post-operator
     b # infix post
   ) # infix post par
+
+block:
+  block:
+    discard
+  # dedented comment post discard