treat resolved symbols on RHS of module qualification as identifiers (#24180)

fixes #19866 given #23997

When searching for a module-qualified symbol, `qualifiedLookUp` tries to
obtain the raw identifier from the RHS of the dot field. However it only
does this when the RHS is either an `nkIdent` or an `nkAccQuoted` node,
not when the node is a resolved symbol or a symchoice, such as in
templates and generics when the module symbol can't be resolved yet.
Since the LHS is a module symbol when the compiler checks for this, any
resolved symbol information doesn't matter, since it has to be a member
of the module. So we now obtain the identifier from these nodes as well
as the unresolved identifier nodes.

The test is a bit niche and possibly not officially supported, but this
is likely a more general problem and I just couldn't think of another
test that would be more "proper". It's better than the error message
`'a' has no type` at least.

4 files changed, 34 insertions, 3 deletions

diff --git a/compiler/lookups.nim b/compiler/lookups.nim
index 2a48f6349..d8fcf73e0 100644
--- a/compiler/lookups.nim
+++ b/compiler/lookups.nim
@@ -685,10 +685,12 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
     var m = qualifiedLookUp(c, n[0], (flags * {checkUndeclared}) + {checkModule})
     if m != nil and m.kind == skModule:
       var ident: PIdent = nil
-      if n[1].kind == nkIdent:
-        ident = n[1].ident
-      elif n[1].kind == nkAccQuoted:
+      if n[1].kind == nkAccQuoted:
         ident = considerQuotedIdent(c, n[1])
+      else:
+        # this includes sym and symchoice nodes, but since we are looking in
+        # a module, it shouldn't matter what was captured
+        ident = n[1].getPIdent
       if ident != nil:
         if m == c.module:
           var ti: TIdentIter = default(TIdentIter)
diff --git a/tests/template/mqualifiedtype1.nim b/tests/template/mqualifiedtype1.nim
new file mode 100644
index 000000000..46569107f
--- /dev/null
+++ b/tests/template/mqualifiedtype1.nim
@@ -0,0 +1,2 @@
+type A* = object
+  x*: int
diff --git a/tests/template/mqualifiedtype2.nim b/tests/template/mqualifiedtype2.nim
new file mode 100644
index 000000000..6a61c14bd
--- /dev/null
+++ b/tests/template/mqualifiedtype2.nim
@@ -0,0 +1,2 @@
+type A* = object
+  x*: array[1000, byte]
diff --git a/tests/template/tqualifiedtype.nim b/tests/template/tqualifiedtype.nim
new file mode 100644
index 000000000..6497af6ee
--- /dev/null
+++ b/tests/template/tqualifiedtype.nim
@@ -0,0 +1,25 @@
+# issue #19866
+
+# Switch module import order to switch which of last two
+# doAsserts fails
+import mqualifiedtype1
+import mqualifiedtype2
+
+# this isn't officially supported but needed to point out the issue:
+template f(moduleName: untyped): int = sizeof(`moduleName`.A)
+template g(someType:   untyped): int = sizeof(someType)
+
+# These are legitimately true.
+doAssert sizeof(mqualifiedtype1.A) != sizeof(mqualifiedtype2.A)
+doAssert g(mqualifiedtype1.A) != g(mqualifiedtype2.A)
+
+# Which means that this should not be true, but is in Nim 1.6
+doAssert f(`mqualifiedtype1`) != f(`mqualifiedtype2`)
+doAssert f(mqualifiedtype1) != f(mqualifiedtype2)
+
+# These should be true, but depending on import order, exactly one
+# fails in Nim 1.2, 1.6 and devel.
+doAssert f(`mqualifiedtype1`) == g(mqualifiedtype1.A)
+doAssert f(`mqualifiedtype2`) == g(mqualifiedtype2.A)
+doAssert f(mqualifiedtype1) == g(mqualifiedtype1.A)
+doAssert f(mqualifiedtype2) == g(mqualifiedtype2.A)