add -d:nimStrictMode in CI to keep code from regressing; fixes ConvFromXtoItselfNotNeeded, UnusedImport notes (#16764)

20 files changed, 43 insertions, 23 deletions

diff --git a/changelog.md b/changelog.md
index 80b9919d0..059f8d986 100644
--- a/changelog.md
+++ b/changelog.md
@@ -201,6 +201,8 @@ provided by the operating system.
   in both rst2html (as before) as well as common tools rendering rst directly (e.g. github), by
   adding: `default-role:: code` directive inside the rst file, which is now handled by rst2html.
 
+- Added `-d:nimStrictMode` in CI in several places to ensure code doesn't have certain hints/warnings
+
 ## Tool changes
 
 - The rst parser now supports markdown table syntax.
diff --git a/compiler/condsyms.nim b/compiler/condsyms.nim
index 45cc55e94..336bfe11c 100644
--- a/compiler/condsyms.nim
+++ b/compiler/condsyms.nim
@@ -126,4 +126,5 @@ proc initDefines*(symbols: StringTableRef) =
   defineSymbol("nimHasCastPragmaBlocks")
   defineSymbol("nimHasDeclaredLocs")
   defineSymbol("nimHasJsBigIntBackend")
+  defineSymbol("nimHasWarningAsError")
   defineSymbol("nimHasHintAsError")
diff --git a/compiler/dfa.nim b/compiler/dfa.nim
index cc2875370..517d13831 100644
--- a/compiler/dfa.nim
+++ b/compiler/dfa.nim
@@ -32,8 +32,6 @@
 import ast, types, intsets, lineinfos, renderer
 import std/private/asciitables
 
-from patterns import sameTrees
-
 type
   InstrKind* = enum
     goto, fork, def, use
diff --git a/config/config.nims b/config/config.nims
index f17f2e30b..2ae86012c 100644
--- a/config/config.nims
+++ b/config/config.nims
@@ -2,3 +2,15 @@
 
 cppDefine "errno"
 cppDefine "unix"
+
+when defined(nimStrictMode):
+  # xxx add more flags here, and use `-d:nimStrictMode` in more contexts in CI.
+
+  # pending bug #14246, enable this:
+  # when defined(nimHasWarningAsError):
+  #   switch("warningAsError", "UnusedImport")
+
+  when defined(nimHasHintAsError):
+    # switch("hint", "ConvFromXtoItselfNotNeeded")
+    switch("hintAsError", "ConvFromXtoItselfNotNeeded")
+    # future work: XDeclaredButNotUsed
diff --git a/koch.nim b/koch.nim
index b5247dd5d..0cfe318cd 100644
--- a/koch.nim
+++ b/koch.nim
@@ -526,7 +526,7 @@ proc runCI(cmd: string) =
   echo "runCI: ", cmd
   echo hostInfo()
   # boot without -d:nimHasLibFFI to make sure this still works
-  kochExecFold("Boot in release mode", "boot -d:release")
+  kochExecFold("Boot in release mode", "boot -d:release -d:nimStrictMode")
 
   ## build nimble early on to enable remainder to depend on it if needed
   kochExecFold("Build Nimble", "nimble")
@@ -549,7 +549,7 @@ proc runCI(cmd: string) =
     #[
     BUG: with initOptParser, `--batch:'' all` interprets `all` as the argument of --batch
     ]#
-    execFold("Run tester", "nim c -r -d:nimCoroutines --putenv:NIM_TESTAMENT_REMOTE_NETWORKING:1 testament/testament --batch:$1 all -d:nimCoroutines" % ["NIM_TESTAMENT_BATCH".getEnv("_")])
+    execFold("Run tester", "nim c -r -d:nimCoroutines --putenv:NIM_TESTAMENT_REMOTE_NETWORKING:1 -d:nimStrictMode testament/testament --batch:$1 all -d:nimCoroutines" % ["NIM_TESTAMENT_BATCH".getEnv("_")])
 
     block CT_FFI:
       when defined(posix): # windows can be handled in future PR's
diff --git a/lib/pure/asyncdispatch.nim b/lib/pure/asyncdispatch.nim
index 1b3548268..cf276fc7b 100644
--- a/lib/pure/asyncdispatch.nim
+++ b/lib/pure/asyncdispatch.nim
@@ -1279,7 +1279,7 @@ else:
     var newList = newSeqOfCap[Callback](newLength)
 
     var cb = curList[0]
-    if not cb(fd.AsyncFD):
+    if not cb(fd):
       newList.add(cb)
 
     withData(p.selector, fd.int, adata) do:
diff --git a/lib/pure/asyncfutures.nim b/lib/pure/asyncfutures.nim
index 1a3b901df..d13d58bd6 100644
--- a/lib/pure/asyncfutures.nim
+++ b/lib/pure/asyncfutures.nim
@@ -364,7 +364,10 @@ proc read*[T](future: Future[T] | FutureVar[T]): T =
   ##
   ## If the result of the future is an error then that error will be raised.
   {.push hint[ConvFromXtoItselfNotNeeded]: off.}
-  let fut = Future[T](future)
+  when future is Future[T]:
+    let fut = future
+  else:
+    let fut = Future[T](future)
   {.pop.}
   if fut.finished:
     if fut.error != nil:
diff --git a/lib/pure/browsers.nim b/lib/pure/browsers.nim
index ee53e557e..aff95e09b 100644
--- a/lib/pure/browsers.nim
+++ b/lib/pure/browsers.nim
@@ -20,7 +20,9 @@ when defined(windows):
   import winlean
   from os import absolutePath
 else:
-  import os, osproc
+  import os
+  when not defined(osx):
+    import osproc
 
 const osOpenCmd* =
   when defined(macos) or defined(macosx) or defined(windows): "open" else: "xdg-open" ## \
diff --git a/lib/pure/concurrency/cpuinfo.nim b/lib/pure/concurrency/cpuinfo.nim
index 57d13fde3..dab5a2447 100644
--- a/lib/pure/concurrency/cpuinfo.nim
+++ b/lib/pure/concurrency/cpuinfo.nim
@@ -15,7 +15,7 @@ runnableExamples:
 
 include "system/inclrtl"
 
-when not defined(windows):
+when defined(linux):
   import posix
 
 when defined(freebsd) or defined(macosx):
diff --git a/lib/pure/net.nim b/lib/pure/net.nim
index b353227a3..decf97777 100644
--- a/lib/pure/net.nim
+++ b/lib/pure/net.nim
@@ -88,7 +88,6 @@
 import std/private/since
 
 import nativesockets, os, strutils, times, sets, options, std/monotimes
-from ssl_certs import scanSSLCertificates
 import ssl_config
 export nativesockets.Port, nativesockets.`$`, nativesockets.`==`
 export Domain, SockType, Protocol
@@ -101,6 +100,8 @@ when useWinVersion:
 
 when defineSsl:
   import openssl
+  when not defined(nimDisableCertificateValidation):
+    from ssl_certs import scanSSLCertificates
 
 # Note: The enumerations are mapped to Window's constants.
 
@@ -670,7 +671,7 @@ when defineSsl:
     # That means we can assume that the next internal index is the length of
     # extra data indexes.
     for i in ctx.referencedData:
-      GC_unref(getExtraData(ctx, i).RootRef)
+      GC_unref(getExtraData(ctx, i))
     ctx.context.SSL_CTX_free()
 
   proc `pskIdentityHint=`*(ctx: SslContext, hint: string) =
diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim
index a35ff3a9a..00dc907e0 100644
--- a/lib/pure/osproc.nim
+++ b/lib/pure/osproc.nim
@@ -1234,7 +1234,7 @@ elif not defined(useNimRtl):
 
   when defined(macosx) or defined(freebsd) or defined(netbsd) or
        defined(openbsd) or defined(dragonfly):
-    import kqueue, times
+    import kqueue
 
     proc waitForExit(p: Process, timeout: int = -1): int =
       if p.exitFlag:
diff --git a/lib/pure/terminal.nim b/lib/pure/terminal.nim
index 14a2613fe..046fa42df 100644
--- a/lib/pure/terminal.nim
+++ b/lib/pure/terminal.nim
@@ -769,8 +769,6 @@ proc getch*(): char =
     discard fd.tcSetAttr(TCSADRAIN, addr oldMode)
 
 when defined(windows):
-  from unicode import toUTF8, Rune, runeLenAt
-
   proc readPasswordFromStdin*(prompt: string, password: var string):
                               bool {.tags: [ReadIOEffect, WriteIOEffect].} =
     ## Reads a `password` from stdin without printing it. `password` must not
diff --git a/lib/std/monotimes.nim b/lib/std/monotimes.nim
index c56bf77b3..f84670ecc 100644
--- a/lib/std/monotimes.nim
+++ b/lib/std/monotimes.nim
@@ -74,7 +74,7 @@ when defined(js):
     system.`+`(a, b)
   {.pop.}
 
-elif defined(posix):
+elif defined(posix) and not defined(osx):
   import posix
 
 elif defined(windows):
diff --git a/lib/std/private/globs.nim b/lib/std/private/globs.nim
index e697ca91c..b98a7808b 100644
--- a/lib/std/private/globs.nim
+++ b/lib/std/private/globs.nim
@@ -4,7 +4,9 @@ this can eventually be moved to std/os and `walkDirRec` can be implemented in te
 to avoid duplication
 ]##
 
-import std/[os,strutils]
+import std/[os]
+when defined(windows):
+  from strutils import replace
 
 type
   PathEntry* = object
diff --git a/lib/system/dollars.nim b/lib/system/dollars.nim
index 16569ea12..4c72c2901 100644
--- a/lib/system/dollars.nim
+++ b/lib/system/dollars.nim
@@ -5,7 +5,6 @@ proc `$`*(x: int): string {.magic: "IntToStr", noSideEffect.}
 
 when defined(js):
   import std/private/since
-
   since (1, 3):
     proc `$`*(x: uint): string =
       ## Caveat: currently implemented as $(cast[int](x)), tied to current
diff --git a/lib/system/excpt.nim b/lib/system/excpt.nim
index 95dbfe3af..dbb39f536 100644
--- a/lib/system/excpt.nim
+++ b/lib/system/excpt.nim
@@ -378,7 +378,8 @@ proc reportUnhandledErrorAux(e: ref Exception) {.nodestroy.} =
     # ugly, but avoids heap allocations :-)
     template xadd(buf, s, slen) =
       if L + slen < high(buf):
-        copyMem(addr(buf[L]), cstring(s), slen)
+
+        copyMem(addr(buf[L]), (when s is cstring: s else: cstring(s)), slen)
         inc L, slen
     template add(buf, s) =
       xadd(buf, s, s.len)
diff --git a/testament/backend.nim b/testament/backend.nim
index c7d13be7e..1770c6657 100644
--- a/testament/backend.nim
+++ b/testament/backend.nim
@@ -23,7 +23,7 @@ proc getMachine*(): MachineId =
   var name = execProcess("hostname").strip
   if name.len == 0:
     name = when defined(posix): getEnv("HOSTNAME")
-           else: getEnv("COMPUTERNAME").string
+           else: getEnv("COMPUTERNAME")
   if name.len == 0:
     quit "cannot determine the machine name"
 
diff --git a/testament/caasdriver.nim b/testament/caasdriver.nim
index 4f17651b5..01e402e07 100644
--- a/testament/caasdriver.nim
+++ b/testament/caasdriver.nim
@@ -175,7 +175,7 @@ when isMainModule:
     verbose = false
 
   for i in 0..paramCount() - 1:
-    let param = string(paramStr(i + 1))
+    let param = paramStr(i + 1)
     case param
     of "verbose": verbose = true
     else: filter = param
diff --git a/testament/testament.nim b/testament/testament.nim
index 2ee1539ff..8637f9464 100644
--- a/testament/testament.nim
+++ b/testament/testament.nim
@@ -683,7 +683,7 @@ proc main() =
     case p.key.normalize
     of "print", "verbose": optPrintResults = true
     of "failing": optFailing = true
-    of "pedantic": discard # deadcode
+    of "pedantic": discard # deadcode refs https://github.com/nim-lang/Nim/issues/16731
     of "targets":
       targetsStr = p.val
       gTargets = parseTargets(targetsStr)
@@ -739,7 +739,7 @@ proc main() =
   var r = initResults()
   case action
   of "all":
-    #processCategory(r, Category"megatest", p.cmdLineRest.string, testsDir, runJoinableTests = false)
+    #processCategory(r, Category"megatest", p.cmdLineRest, testsDir, runJoinableTests = false)
 
     var myself = quoteShell(getAppFilename())
     if targetsStr.len > 0:
@@ -798,8 +798,7 @@ proc main() =
     p.next
     processPattern(r, pattern, p.cmdLineRest, simulate)
   of "r", "run":
-    var subPath = p.key
-    let (cat, path) = splitTestFile(subPath)
+    let (cat, path) = splitTestFile(p.key)
     processSingleTest(r, cat.Category, p.cmdLineRest, path, gTargets, targetsSet)
   of "html":
     generateHtml(resultsFile, optFailing)
diff --git a/tests/config.nims b/tests/config.nims
index b7d828183..ac90d37e8 100644
--- a/tests/config.nims
+++ b/tests/config.nims
@@ -21,3 +21,5 @@ hint("Processing", off)
 # uncomment to enable all flaky tests disabled by this flag
 # (works through process calls, e.g. tests that invoke nim).
 # switch("define", "nimTestsEnableFlaky")
+
+# switch("hint", "ConvFromXtoItselfNotNeeded")