[TODO] Nim now allows modules with same name in a package

compiler/modules.nim

@@ -14,31 +14,43 @@ import
   idents, os, lexer, idgen, passes, syntaxes, llstream, modulegraphs, rod,
   lineinfos
 
+from strutils import rsplit
+
 proc resetSystemArtifacts*(g: ModuleGraph) =
   magicsys.resetSysTypes(g)
 
+proc getPackageAux(graph: ModuleGraph, pkg: string, info: TLineInfo): PSym =
+  let pkgIdent = getIdent(graph.cache, pkg)
+  result = graph.packageSyms.strTableGet(pkgIdent)
+  if result==nil:
+    result = newSym(skPackage, pkgIdent, nil, info)
+    initStrTable(result.tab)
+    graph.packageSyms.strTableAdd(result)
+    let pkgs = rsplit(pkg, {'.'}, maxsplit=1)
+    if pkgs.len > 1:
+      let parent = getPackageAux(graph, pkgs[0], info)
+      result.owner = parent
+
+proc getPackage(graph: ModuleGraph, filename: string, info: TLineInfo): PSym =
+  let
+    pck = getPackageName(graph.config, filename)
+    pck2 = if pck.len > 0: pck else: "unknown" #CHECKME
+  result = getPackageAux(graph, pck2, info)
+
 proc newModule(graph: ModuleGraph; fileIdx: FileIndex): PSym =
   # We cannot call ``newSym`` here, because we have to circumvent the ID
   # mechanism, which we do in order to assign each module a persistent ID.
   new(result)
   result.id = -1             # for better error checking
   result.kind = skModule
   let filename = toFullPath(graph.config, fileIdx)
+  # TODO: should modules kept track of their fully qualified name (fqname)?
   result.name = getIdent(graph.cache, splitFile(filename).name)
   if not isNimIdentifier(result.name.s):
     rawMessage(graph.config, errGenerated, "invalid module name: " & result.name.s)
 
   result.info = newLineInfo(fileIdx, 1, 1)
-  let
-    pck = getPackageName(graph.config, filename)
-    pck2 = if pck.len > 0: pck else: "unknown"
-    pack = getIdent(graph.cache, pck2)
-  var packSym = graph.packageSyms.strTableGet(pack)
-  if packSym == nil:
-    packSym = newSym(skPackage, getIdent(graph.cache, pck2), nil, result.info)
-    initStrTable(packSym.tab)
-    graph.packageSyms.strTableAdd(packSym)
-
+  let packSym = getPackage(graph, filename, result.info)
   result.owner = packSym
   result.position = int fileIdx
 
@@ -52,8 +64,10 @@ proc newModule(graph: ModuleGraph; fileIdx: FileIndex): PSym =
   strTableAdd(result.tab, result) # a module knows itself
   let existing = strTableGet(packSym.tab, result.name)
   if existing != nil and existing.info.fileIndex != result.info.fileIndex:
+    # this should never happen except in adverserial case with
+    # `mypkg/foo/bar/baz.nim` clashing with `mypkg/foo.bar/baz.nim`
     localError(graph.config, result.info,
-      "module names need to be unique per Nimble package; module clashes with " &
+      "fully qualified module names need to be unique per Nimble package; module clashes with " &
         toFullPath(graph.config, existing.info.fileIndex))
   # strTableIncl() for error corrections:
   discard strTableIncl(packSym.tab, result)

compiler/packagehandling.nim

@@ -12,38 +12,53 @@ iterator myParentDirs(p: string): string =
   var current = p
   while true:
     current = current.parentDir
-    if current.len == 0: break
+    # IMPROVE:parentDir is buggy, "foo.nim".parentDir should be ".", not ""
+    if current.len == 0:
+      current = "."
+      yield current
+      break
     yield current
 
-proc resetPackageCache*(conf: ConfigRef) =
-  conf.packageCache = newPackageCache()
+const packageSep = "."
 
 proc getPackageName*(conf: ConfigRef; path: string): string =
-  var parents = 0
+  var dirs:seq[string] = @[]
+  var pkg = ""
+  var path_root = ""
   block packageSearch:
     for d in myParentDirs(path):
+      path_root = d
       if conf.packageCache.hasKey(d):
-        #echo "from cache ", d, " |", packageCache[d], "|", path.splitFile.name
-        return conf.packageCache[d]
-      inc parents
+        pkg = conf.packageCache[d]
+        break packageSearch
       for file in walkFiles(d / "*.nimble"):
-        result = file.splitFile.name
+        pkg = file.splitFile.name
         break packageSearch
       for file in walkFiles(d / "*.babel"):
-        result = file.splitFile.name
+        pkg = file.splitFile.name
         break packageSearch
-  # we also store if we didn't find anything:
-  if result.isNil: result = ""
-  for d in myParentDirs(path):
-    #echo "set cache ", d, " |", result, "|", parents
-    conf.packageCache[d] = result
-    dec parents
-    if parents <= 0: break
+      dirs.add d.splitPath.tail
+
+  # at this point, path_root maps to pkg
+  for index in 0 .. dirs.len:
+    if index > 0:
+      let dir = dirs[^index]
+      path_root = path_root & DirSep & dir
+      pkg = pkg & packageSep & dir
+    if conf.packageCache.hasKey(path_root):
+      doAssert conf.packageCache[path_root] == pkg
+    else:
+      conf.packageCache[path_root] = pkg
+  result = pkg
+
+proc fullyQualifiedName*(conf: ConfigRef; path: string): string =
+  let pkg = getPackageName(conf, path)
+  doAssert pkg.len > 0
+  let (p, file, ext) = path.splitFile
+  result = pkg & packageSep & file
 
 proc withPackageName*(conf: ConfigRef; path: string): string =
-  let x = getPackageName(conf, path)
-  if x.len == 0:
-    result = path
-  else:
-    let (p, file, ext) = path.splitFile
-    result = (p / (x & '_' & file)) & ext
+  let fqname = fullyQualifiedName(conf, path)
+  let (p, file, ext) = path.splitFile
+  # TODO: is `p/` part needed?
+  result = p / (fqname & ext)

compiler/vmgen.nim

@@ -1770,12 +1770,18 @@ proc genTupleConstr(c: PCtx, n: PNode, dest: var TDest) =
 
 proc genProc*(c: PCtx; s: PSym): int
 
+proc moduleBaseName(fqname: string): string =
+  ## returns last component of a fully qualified name, eg:
+  ## stdlib.pure.times => times
+  rsplit(fqname, {'.'}, maxsplit=1)[^1]
+
 proc matches(s: PSym; x: string): bool =
   let y = x.split('.')
   var s = s
   var L = y.len-1
   while L >= 0:
-    if s == nil or (y[L].cmpIgnoreStyle(s.name.s) != 0 and y[L] != "*"):
+    if s == nil: return false
+    if (y[L].cmpIgnoreStyle(s.name.s.moduleBaseName) != 0 and y[L] != "*"):
       return false
     s = s.owner
     dec L
@@ -1785,7 +1791,8 @@ proc matches(s: PSym; y: varargs[string]): bool =
   var s = s
   var L = y.len-1
   while L >= 0:
-    if s == nil or (y[L].cmpIgnoreStyle(s.name.s) != 0 and y[L] != "*"):
+    if s == nil: return false
+    if (y[L].cmpIgnoreStyle(s.name.s.moduleBaseName) != 0 and y[L] != "*"):
       return false
     s = if sfFromGeneric in s.flags: s.owner.owner else: s.owner
     dec L

compiler/vmops.nim

@@ -16,19 +16,19 @@ from math import sqrt, ln, log10, log2, exp, round, arccos, arcsin,
 from os import getEnv, existsEnv, dirExists, fileExists, putEnv, walkDir
 
 template mathop(op) {.dirty.} =
-  registerCallback(c, "stdlib.math." & astToStr(op), `op Wrapper`)
+  registerCallback(c, "stdlib.pure.math." & astToStr(op), `op Wrapper`)
 
 template osop(op) {.dirty.} =
-  registerCallback(c, "stdlib.os." & astToStr(op), `op Wrapper`)
+  registerCallback(c, "stdlib.pure.os." & astToStr(op), `op Wrapper`)
 
 template ospathsop(op) {.dirty.} =
-  registerCallback(c, "stdlib.ospaths." & astToStr(op), `op Wrapper`)
+  registerCallback(c, "stdlib.pure.ospaths." & astToStr(op), `op Wrapper`)
 
 template systemop(op) {.dirty.} =
   registerCallback(c, "stdlib.system." & astToStr(op), `op Wrapper`)
 
 template macrosop(op) {.dirty.} =
-  registerCallback(c, "stdlib.macros." & astToStr(op), `op Wrapper`)
+  registerCallback(c, "stdlib.core.macros." & astToStr(op), `op Wrapper`)
 
 template wrap1f_math(op) {.dirty.} =
   proc `op Wrapper`(a: VmArgs) {.nimcall.} =
@@ -116,6 +116,7 @@ proc registerAdditionalOps*(c: PCtx) =
     wrap2svoid(writeFile, systemop)
     wrap1s(readFile, systemop)
     systemop getCurrentExceptionMsg
+    #TODO: should that be: stdlib.pure.*.staticWalkDir ?
     registerCallback c, "stdlib.*.staticWalkDir", proc (a: VmArgs) {.nimcall.} =
       setResult(a, staticWalkDirImpl(getString(a, 0), getBool(a, 1)))
     systemop gorgeEx

lib/system/nimscript.nim

@@ -92,7 +92,7 @@ proc patchFile*(package, filename, replacement: string) =
   ##
   ## .. code-block:: nim
   ##
-  ##   patchFile("stdlib", "asyncdispatch", "patches/replacement")
+  ##   patchFile("stdlib.pure", "asyncdispatch", "patches/replacement")
   discard
 
 proc getCommand*(): string =

tests/modules/import_styles.nim

@@ -0,0 +1,25 @@
+discard """
+  output: "ok"
+"""
+import strutils as strutils1
+import strutils as strutils2
+import std/strutils as strutils3
+import std/strutils
+
+import std/sha1
+import std/sha1 as sha1_2
+# TODO: Error: cannot open file: stdlib/std/sha1; inconsistent with the fact we
+# generate `stdlib.std.sha1`
+# import stdlib/std/sha1 as sha1_3
+
+import ./mnotuniquename
+import ./tnotuniquename/mnotuniquename as mnotuniquename1
+
+import pure/ospaths as ospaths1
+import std/ospaths as ospaths2
+import ospaths as ospaths3
+
+# Error: cannot open file: std/pure/ospaths
+# import std/pure/ospaths as ospaths4
+
+echo "ok"

tests/modules/tnotuniquename1.nim

@@ -0,0 +1,8 @@
+discard """
+  output: "ok"
+"""
+
+# module names need not be unique (but must be renamed to avoid clashes)
+import mnotuniquename
+import tnotuniquename/mnotuniquename as mnotuniquename1
+echo "ok"

tests/modules/tnotuniquename2.nim

@@ -1,7 +1,7 @@
 discard """
-  file: "tnotuniquename/mnotuniquename.nim"
-  errormsg: "module names need to be unique per Nimble package"
+  errormsg: "redefinition of 'mnotuniquename'"
 """
 
+# avoid name clashes
 import mnotuniquename
 import tnotuniquename/mnotuniquename

tests/newconfig/tfoo.nims

@@ -12,7 +12,7 @@ import ospaths
 warning("uninit", off)
 hint("processing", off)
 #--verbosity:2
-patchFile("stdlib", "math", "mymath")
+patchFile("stdlib.pure", "math", "mymath")
 
 task listDirs, "lists every subdirectory":
   for x in listDirs("."):

tests/testament/tester.nim

@@ -229,12 +229,20 @@ proc cmpMsgs(r: var TResults, expected, given: TSpec, test: TTest, target: TTarg
     r.addResult(test, target, expected.msg, given.msg, reSuccess)
     inc(r.passed)
 
+import compiler/options
+var conf = newConfigRef()
+
 proc generatedFile(test: TTest, target: TTarget): string =
   let (_, name, _) = test.name.splitFile
   let ext = targetToExt[target]
-  result = nimcacheDir(test.name, test.options, target) /
-    (if target == targetJS: "" else: "compiler_") &
-    name.changeFileExt(ext)
+
+  let fqname = fullyQualifiedName(conf, test.name)
+  # echo ("generatedFile", test.name, fqname, ext)
+  result = nimcacheDir(test.name, test.options, target)
+  if target == targetJS: #TODO:why?
+    result = result / name.changeFileExt(ext)
+  else:
+    result = result / fqname & "." & ext
 
 proc needsCodegenCheck(spec: TSpec): bool =
   result = spec.maxCodeSize > 0 or spec.ccodeCheck.len > 0
@@ -262,6 +270,7 @@ proc codegenCheck(test: TTest, target: TTarget, spec: TSpec, expectedMsg: var st
     echo getCurrentExceptionMsg()
   except IOError:
     given.err = reCodeNotFound
+    echo getCurrentExceptionMsg()
 
 proc nimoutCheck(test: TTest; expectedNimout: string; given: var TSpec) =
   let exp = expectedNimout.strip.replace("\C\L", "\L")
@@ -509,7 +518,8 @@ proc main() =
   backend.close()
   var failed = r.total - r.passed - r.skipped
   if failed != 0:
-    echo "FAILURE! total: ", r.total, " passed: ", r.passed, " skipped: ", r.skipped
+    echo "FAILURE! total: ", r.total, " passed: ", r.passed, " skipped: ",
+      r.skipped, " failed: ", failed
     quit(QuitFailure)
 
 if paramCount() == 0: