Undeprecate isvalidfilename (#19643)

* Remove deprecated isvalidfilename
* https://github.com/nim-lang/Nim/pull/19643#issuecomment-1235102314
* https://github.com/nim-lang/Nim/pull/19643#issuecomment-1235102314
* https://github.com/nim-lang/Nim/pull/19643#issuecomment-1235102314
* Add unittests
* Add more

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>

changelog.md

@@ -99,6 +99,7 @@
 - `strutils.find` now uses and defaults to `last = -1` for whole string searches,
   making limiting it to just the first char (`last = 0`) valid.
 - `random.rand` now works with `Ordinal`s.
+- Undeprecated `os.isvalidfilename`.
 - `std/oids` now uses `int64` to store time internally (before it was int32), the length of
   the string form of `Oid` changes from 24 to 32.
 

lib/pure/os.nim

@@ -3500,14 +3500,21 @@ proc setLastModificationTime*(file: string, t: times.Time) {.noWeirdTarget.} =
     discard h.closeHandle
     if res == 0'i32: raiseOSError(osLastError(), file)
 
-func isValidFilename*(filename: string, maxLen = 259.Positive): bool {.since: (1, 1), deprecated: "Deprecated since v1.5.1".} =
+
-  ## Returns true if ``filename`` is valid for crossplatform use.
+func isValidFilename*(filename: string, maxLen = 259.Positive): bool {.since: (1, 1).} =
+  ## Returns `true` if `filename` is valid for crossplatform use.
   ##
   ## This is useful if you want to copy or save files across Windows, Linux, Mac, etc.
-  ## You can pass full paths as argument too, but func only checks filenames.
-  ##
   ## It uses `invalidFilenameChars`, `invalidFilenames` and `maxLen` to verify the specified `filename`.
   ##
+  ## See also:
+  ##
+  ## * https://docs.microsoft.com/en-us/dotnet/api/system.io.pathtoolongexception
+  ## * https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
+  ## * https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx
+  ##
+  ## .. warning:: This only checks filenames, not whole paths
+  ##    (because basically you can mount anything as a path on Linux).
   runnableExamples:
     assert not isValidFilename(" foo")     # Leading white space
     assert not isValidFilename("foo ")     # Trailing white space
@@ -3518,9 +3525,6 @@ func isValidFilename*(filename: string, maxLen = 259.Positive): bool {.since: (1
     assert not isValidFilename("")         # Empty string
     assert not isValidFilename("foo/")     # Filename is empty
 
-  # https://docs.microsoft.com/en-us/dotnet/api/system.io.pathtoolongexception
-  # https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
-  # https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx
   result = true
   let f = filename.splitFile()
   if unlikely(f.name.len + f.ext.len > maxLen or f.name.len == 0 or
@@ -3529,6 +3533,7 @@ func isValidFilename*(filename: string, maxLen = 259.Positive): bool {.since: (1
   for invalid in invalidFilenames:
     if cmpIgnoreCase(f.name, invalid) == 0: return false
 
+
 # deprecated declarations
 when not defined(nimscript):
   when not defined(js): # `noNimJs` doesn't work with templates, this should improve.

tests/stdlib/tos.nim

@@ -672,32 +672,24 @@ block: # normalizePathEnd
     doAssert r"E:/".normalizePathEnd(trailingSep = true) == r"E:\"
     doAssert "/".normalizePathEnd == r"\"
 
-block: # isValidFilename
+
-  # Negative Tests.
+import sugar
-  doAssert not isValidFilename("abcd", maxLen = 2)
+
-  doAssert not isValidFilename("0123456789", maxLen = 8)
+block: # normalizeExe
-  doAssert not isValidFilename("con")
+  doAssert "".dup(normalizeExe) == ""
-  doAssert not isValidFilename("aux")
+  when defined(posix):
-  doAssert not isValidFilename("prn")
+    doAssert "foo".dup(normalizeExe) == "./foo"
-  doAssert not isValidFilename("OwO|UwU")
+    doAssert "foo/../bar".dup(normalizeExe) == "foo/../bar"
-  doAssert not isValidFilename(" foo")
+  when defined(windows):
-  doAssert not isValidFilename("foo ")
+    doAssert "foo".dup(normalizeExe) == "foo"
-  doAssert not isValidFilename("foo.")
+
-  doAssert not isValidFilename("con.txt")
+block: # isAdmin
-  doAssert not isValidFilename("aux.bat")
+  let isAzure = existsEnv("TF_BUILD") # xxx factor with testament.specs.isAzure
-  doAssert not isValidFilename("prn.exe")
+  # In Azure on Windows tests run as an admin user
-  doAssert not isValidFilename("nim>.nim")
+  if isAzure and defined(windows): doAssert isAdmin()
-  doAssert not isValidFilename(" foo.log")
+  # In Azure on POSIX tests run as a normal user
-  # Positive Tests.
+  if isAzure and defined(posix): doAssert not isAdmin()
-  doAssert isValidFilename("abcd", maxLen = 42.Positive)
+
-  doAssert isValidFilename("c0n")
-  doAssert isValidFilename("foo.aux")
-  doAssert isValidFilename("bar.prn")
-  doAssert isValidFilename("OwO_UwU")
-  doAssert isValidFilename("cron")
-  doAssert isValidFilename("ux.bat")
-  doAssert isValidFilename("nim.nim")
-  doAssert isValidFilename("foo.log")
 
 import sugar
 
@@ -793,3 +785,32 @@ else:
     doAssert parentDirs("/home/user", fromRoot=false).toSeq == @["/home/user", "/home", "/"]
     doAssert parentDirs("home/user", fromRoot=true).toSeq == @["home/", "home/user"]
     doAssert parentDirs("home/user", fromRoot=false).toSeq == @["home/user", "home"]
+
+
+# https://github.com/nim-lang/Nim/pull/19643#issuecomment-1235102314
+block:  # isValidFilename
+  # Negative Tests.
+  doAssert not isValidFilename("abcd", maxLen = 2)
+  doAssert not isValidFilename("0123456789", maxLen = 8)
+  doAssert not isValidFilename("con")
+  doAssert not isValidFilename("aux")
+  doAssert not isValidFilename("prn")
+  doAssert not isValidFilename("OwO|UwU")
+  doAssert not isValidFilename(" foo")
+  doAssert not isValidFilename("foo ")
+  doAssert not isValidFilename("foo.")
+  doAssert not isValidFilename("con.txt")
+  doAssert not isValidFilename("aux.bat")
+  doAssert not isValidFilename("prn.exe")
+  doAssert not isValidFilename("nim>.nim")
+  doAssert not isValidFilename(" foo.log")
+  # Positive Tests.
+  doAssert isValidFilename("abcd", maxLen = 42.Positive)
+  doAssert isValidFilename("c0n")
+  doAssert isValidFilename("foo.aux")
+  doAssert isValidFilename("bar.prn")
+  doAssert isValidFilename("OwO_UwU")
+  doAssert isValidFilename("cron")
+  doAssert isValidFilename("ux.bat")
+  doAssert isValidFilename("nim.nim")
+  doAssert isValidFilename("foo.log")