warn about inconsistent spacing around binary operators; fixes #7582

changelog.md

@@ -17,6 +17,9 @@
 - The ``not nil`` type annotation now has to be enabled explicitly
   via ``{.experimental: "notnil"}`` as we are still not pleased with how this
   feature works with Nim's containers.
+- The parser now warns about inconsistent spacing around binary operators as
+  these can easily be confused with unary operators. This warning will likely
+  become an error in the future.
 
 
 #### Breaking changes in the standard library
@@ -89,7 +92,7 @@
 - Accessing the binary zero terminator in Nim's native strings
   is now invalid. Internally a Nim string still has the trailing zero for
   zero-copy interoperability with ``cstring``. Compile your code with the
-  next switch ``--laxStrings:on`` if you need a transition period.
+  new switch ``--laxStrings:on`` if you need a transition period.
 
 
 ### Tool changes

compiler/msgs.nim

@@ -131,7 +131,7 @@ type
     warnEachIdentIsTuple, warnShadowIdent,
     warnProveInit, warnProveField, warnProveIndex, warnGcUnsafe, warnGcUnsafe2,
     warnUninit, warnGcMem, warnDestructor, warnLockLevel, warnResultShadowed,
-    warnUser,
+    warnInconsistentSpacing, warnUser,
     hintSuccess, hintSuccessX,
     hintLineTooLong, hintXDeclaredButNotUsed, hintConvToBaseNotNeeded,
     hintConvFromXtoItselfNotNeeded, hintExprAlwaysX, hintQuitCalled,
@@ -416,6 +416,7 @@ const
     warnDestructor: "usage of a type with a destructor in a non destructible context. This will become a compile time error in the future.",
     warnLockLevel: "$1",
     warnResultShadowed: "Special variable 'result' is shadowed.",
+    warnInconsistentSpacing: "Number of spaces around '$#' is not consistent",
     warnUser: "$1",
     hintSuccess: "operation successful",
     hintSuccessX: "operation successful ($# lines compiled; $# sec total; $#; $#)",
@@ -454,7 +455,8 @@ const
     "TypelessParam", "UseBase", "WriteToForeignHeap",
     "UnsafeCode", "EachIdentIsTuple", "ShadowIdent",
     "ProveInit", "ProveField", "ProveIndex", "GcUnsafe", "GcUnsafe2", "Uninit",
-    "GcMem", "Destructor", "LockLevel", "ResultShadowed", "User"]
+    "GcMem", "Destructor", "LockLevel", "ResultShadowed",
+    "Spacing", "User"]
 
   HintsToStr* = ["Success", "SuccessX", "LineTooLong",
     "XDeclaredButNotUsed", "ConvToBaseNotNeeded", "ConvFromXtoItselfNotNeeded",

compiler/parser.nim

@@ -268,13 +268,9 @@ proc isUnary(p: TParser): bool =
 proc checkBinary(p: TParser) {.inline.} =
   ## Check if the current parser token is a binary operator.
   # we don't check '..' here as that's too annoying
-  if p.strongSpaces and p.tok.tokType == tkOpr:
+  if p.tok.tokType == tkOpr:
     if p.tok.strongSpaceB > 0 and p.tok.strongSpaceA != p.tok.strongSpaceB:
-      parMessage(p, errGenerated,
+      parMessage(p, warnInconsistentSpacing, prettyTok(p.tok))
-                 "Number of spaces around '$#' not consistent" %
-                 prettyTok(p.tok))
-    elif p.tok.strongSpaceA notin {0,1,2,4,8}:
-      parMessage(p, errGenerated, "Number of spaces must be 0,1,2,4 or 8")
 
 #| module = stmt ^* (';' / IND{=})
 #|

compiler/plugins/locals/locals.nim

@@ -9,7 +9,7 @@
 
 ## The builtin 'system.locals' implemented as a plugin.
 
-import "../../"  / [pluginsupport, ast, astalgo,
+import "../../" / [pluginsupport, ast, astalgo,
   magicsys, lookups, semdata, lowerings]
 
 proc semLocals(c: PContext, n: PNode): PNode =

compiler/renderer.nim

@@ -342,7 +342,7 @@ proc atom(g: TSrcGen; n: PNode): string =
   of nkIdent: result = n.ident.s
   of nkSym: result = n.sym.name.s
   of nkStrLit: result = ""; result.addQuoted(n.strVal)
-  of nkRStrLit: result = "r\"" & replace(n.strVal, "\"", "\"\"")  & '\"'
+  of nkRStrLit: result = "r\"" & replace(n.strVal, "\"", "\"\"") & '\"'
   of nkTripleStrLit: result = "\"\"\"" & n.strVal & "\"\"\""
   of nkCharLit:
     result = "\'"

compiler/semstmts.nim

@@ -846,7 +846,7 @@ proc checkCovariantParamsUsages(genericType: PType) =
         if subType != nil:
           subresult traverseSubTypes(subType)
       if result:
-        error("non-invariant type param used in a proc type: " &  $t)
+        error("non-invariant type param used in a proc type: " & $t)
 
     of tySequence:
       return traverseSubTypes(t[0])

compiler/vmgen.nim

@@ -790,7 +790,7 @@ proc genIntCast(c: PCtx; n: PNode; dest: var TDest) =
     elif src.kind in signedIntegers and dst.kind in unsignedIntegers:
       # cast signed to unsigned integer of same size
       # unsignedVal = (offset +% signedVal +% 1) and offset
-      let offset = (1 shl (src_size * 8))  - 1
+      let offset = (1 shl (src_size * 8)) - 1
       c.gABx(n, opcLdConst, tmp2, mkIntLit(offset))
       c.gABx(n, opcLdConst, dest, mkIntLit(offset+1))
       c.gABC(n, opcAddu, tmp3, tmp, dest)

lib/pure/algorithm.nim

@@ -395,7 +395,7 @@ proc rotateInternal[T](arg: var openarray[T]; first, middle, last: int): int =
 
   swap(arg[mFirst], arg[next])
   mFirst += 1
-  next  += 1
+  next += 1
   if mFirst == mMiddle:
     mMiddle = next
 

lib/system/assign.nim

@@ -74,7 +74,7 @@ proc genericAssignAux(dest, src: pointer, mt: PNimType, shallow: bool) =
       var dst = cast[ByteAddress](cast[PPointer](dest)[])
       for i in 0..seq.len-1:
         genericAssignAux(
-          cast[pointer](dst +% i*% mt.base.size +% GenericSeqSize),
+          cast[pointer](dst +% i *% mt.base.size +% GenericSeqSize),
           cast[pointer](cast[ByteAddress](s2) +% i *% mt.base.size +%
                       GenericSeqSize),
           mt.base, shallow)
@@ -100,8 +100,8 @@ proc genericAssignAux(dest, src: pointer, mt: PNimType, shallow: bool) =
     genericAssignAux(dest, src, mt.node, shallow)
   of tyArray, tyArrayConstr:
     for i in 0..(mt.size div mt.base.size)-1:
-      genericAssignAux(cast[pointer](d +% i*% mt.base.size),
+      genericAssignAux(cast[pointer](d +% i *% mt.base.size),
-                       cast[pointer](s +% i*% mt.base.size), mt.base, shallow)
+                       cast[pointer](s +% i *% mt.base.size), mt.base, shallow)
   of tyRef:
     unsureAsgnRef(cast[PPointer](dest), cast[PPointer](s)[])
   of tyOptAsRef:
@@ -166,8 +166,8 @@ proc genericAssignOpenArray(dest, src: pointer, len: int,
     d = cast[ByteAddress](dest)
     s = cast[ByteAddress](src)
   for i in 0..len-1:
-    genericAssign(cast[pointer](d +% i*% mt.base.size),
+    genericAssign(cast[pointer](d +% i *% mt.base.size),
-                  cast[pointer](s +% i*% mt.base.size), mt.base)
+                  cast[pointer](s +% i *% mt.base.size), mt.base)
 
 proc objectInit(dest: pointer, typ: PNimType) {.compilerProc, benign.}
 proc objectInitAux(dest: pointer, n: ptr TNimNode) {.benign.} =
@@ -235,7 +235,7 @@ proc genericReset(dest: pointer, mt: PNimType) =
     pint[] = nil
   of tyArray, tyArrayConstr:
     for i in 0..(mt.size div mt.base.size)-1:
-      genericReset(cast[pointer](d +% i*% mt.base.size), mt.base)
+      genericReset(cast[pointer](d +% i *% mt.base.size), mt.base)
   else:
     zeroMem(dest, mt.size) # set raw bits to zero
 

lib/system/deepcopy.nim

@@ -105,7 +105,7 @@ proc genericDeepCopyAux(dest, src: pointer, mt: PNimType; tab: var PtrTable) =
     var dst = cast[ByteAddress](cast[PPointer](dest)[])
     for i in 0..seq.len-1:
       genericDeepCopyAux(
-        cast[pointer](dst +% i*% mt.base.size +% GenericSeqSize),
+        cast[pointer](dst +% i *% mt.base.size +% GenericSeqSize),
         cast[pointer](cast[ByteAddress](s2) +% i *% mt.base.size +%
                      GenericSeqSize),
         mt.base, tab)
@@ -122,8 +122,8 @@ proc genericDeepCopyAux(dest, src: pointer, mt: PNimType; tab: var PtrTable) =
     genericDeepCopyAux(dest, src, mt.node, tab)
   of tyArray, tyArrayConstr:
     for i in 0..(mt.size div mt.base.size)-1:
-      genericDeepCopyAux(cast[pointer](d +% i*% mt.base.size),
+      genericDeepCopyAux(cast[pointer](d +% i *% mt.base.size),
-                         cast[pointer](s +% i*% mt.base.size), mt.base, tab)
+                         cast[pointer](s +% i *% mt.base.size), mt.base, tab)
   of tyRef, tyOptAsRef:
     let s2 = cast[PPointer](src)[]
     if s2 == nil:
@@ -183,5 +183,5 @@ proc genericDeepCopyOpenArray(dest, src: pointer, len: int,
     d = cast[ByteAddress](dest)
     s = cast[ByteAddress](src)
   for i in 0..len-1:
-    genericDeepCopy(cast[pointer](d +% i*% mt.base.size),
+    genericDeepCopy(cast[pointer](d +% i *% mt.base.size),
-                    cast[pointer](s +% i*% mt.base.size), mt.base)
+                    cast[pointer](s +% i *% mt.base.size), mt.base)

lib/system/gc.nim

@@ -535,7 +535,7 @@ proc growObj(old: pointer, newsize: int, gch: var GcHeap): pointer =
 
   var oldsize = cast[PGenericSeq](old).len*elemSize + GenericSeqSize
   copyMem(res, ol, oldsize + sizeof(Cell))
-  zeroMem(cast[pointer](cast[ByteAddress](res)+% oldsize +% sizeof(Cell)),
+  zeroMem(cast[pointer](cast[ByteAddress](res) +% oldsize +% sizeof(Cell)),
           newsize-oldsize)
   sysAssert((cast[ByteAddress](res) and (MemAlign-1)) == 0, "growObj: 3")
   # This can be wrong for intermediate temps that are nevertheless on the

lib/system/osalloc.nim

@@ -58,7 +58,7 @@ when defined(emscripten):
 
     # Convert pointer to PageSize correct one.
     var new_pos = cast[ByteAddress](pos) +% (PageSize - (pos %% PageSize))
-    if (new_pos-pos)< sizeof(EmscriptenMMapBlock):
+    if (new_pos-pos) < sizeof(EmscriptenMMapBlock):
       new_pos = new_pos +% PageSize
     result = cast[pointer](new_pos)