ambiguous identifier resolution (#23123)

fixes #23002, fixes #22841, refs comments in #23097

When an identifier is ambiguous in scope (i.e. multiple imports contain
symbols with the same name), attempt resolving it through type inference
(by creating a symchoice). To do this efficiently, `qualifiedLookUp` had
to be broken up so that `semExpr` can access the ambiguous candidates
directly (now obtained directly via `lookUpCandidates`).

This fixes the linked issues, but an example like:

```nim
let on = 123

{.warning[ProveInit]: on.}
```

will still fail, since `on` is unambiguously the local `let` symbol here
(this is also true for `proc on` but `proc` symbols generate symchoices
anyway).

Type symbols are not considered to not confuse the type inference. This
includes the change in sigmatch, up to this point symchoices with
nonoverloadable symbols could be created, they just wouldn't be
considered during disambiguation. Now every proper symbol except types
are considered in disambiguation, so the correct symbols must be picked
during the creation of the symchoice node. I remember there being a
violating case of this in the compiler, but this was very likely fixed
by excluding type symbols as CI seems to have found no issues.

The pure enum ambiguity test was disabled because ambiguous pure enums
now behave like overloadable enums with this behavior, so we get a
longer error message for `echo amb` like `type mismatch: got <MyEnum |
OtherEnum> but expected T`

13 files changed, 146 insertions, 71 deletions

diff --git a/compiler/lookups.nim b/compiler/lookups.nim
index cfefe764b..52296644d 100644
--- a/compiler/lookups.nim
+++ b/compiler/lookups.nim
@@ -303,8 +303,16 @@ proc isAmbiguous*(c: PContext, s: PIdent, filter: TSymKinds, sym: var PSym): boo
     # imports had a candidate but wasn't ambiguous
     return false
 
-proc errorSym*(c: PContext, n: PNode): PSym =
+proc errorSym*(c: PContext, ident: PIdent, info: TLineInfo): PSym =
   ## creates an error symbol to avoid cascading errors (for IDE support)
+  result = newSym(skError, ident, c.idgen, getCurrOwner(c), info, {})
+  result.typ = errorType(c)
+  incl(result.flags, sfDiscardable)
+  # pretend it's from the top level scope to prevent cascading errors:
+  if c.config.cmd != cmdInteractive and c.compilesContextId == 0:
+    c.moduleScope.addSym(result)
+
+proc errorSym*(c: PContext, n: PNode): PSym =
   var m = n
   # ensure that 'considerQuotedIdent' can't fail:
   if m.kind == nkDotExpr: m = m[1]
@@ -312,12 +320,7 @@ proc errorSym*(c: PContext, n: PNode): PSym =
       considerQuotedIdent(c, m)
     else:
       getIdent(c.cache, "err:" & renderTree(m))
-  result = newSym(skError, ident, c.idgen, getCurrOwner(c), n.info, {})
-  result.typ = errorType(c)
-  incl(result.flags, sfDiscardable)
-  # pretend it's from the top level scope to prevent cascading errors:
-  if c.config.cmd != cmdInteractive and c.compilesContextId == 0:
-    c.moduleScope.addSym(result)
+  result = errorSym(c, ident, n.info)
 
 type
   TOverloadIterMode* = enum
@@ -499,7 +502,7 @@ proc mustFixSpelling(c: PContext): bool {.inline.} =
   result = c.config.spellSuggestMax != 0 and c.compilesContextId == 0
     # don't slowdown inside compiles()
 
-proc fixSpelling(c: PContext, n: PNode, ident: PIdent, result: var string) =
+proc fixSpelling(c: PContext, ident: PIdent, result: var string) =
   ## when we cannot find the identifier, suggest nearby spellings
   var list = initHeapQueue[SpellCandidate]()
   let name0 = ident.s.nimIdentNormalize
@@ -558,7 +561,7 @@ proc errorUseQualifier*(c: PContext; info: TLineInfo; s: PSym) =
   var amb: bool
   discard errorUseQualifier(c, info, s, amb)
 
-proc errorUseQualifier(c: PContext; info: TLineInfo; candidates: seq[PSym]; prefix = "use one of") =
+proc errorUseQualifier*(c: PContext; info: TLineInfo; candidates: seq[PSym]; prefix = "use one of") =
   var err = "ambiguous identifier: '" & candidates[0].name.s & "'"
   var i = 0
   for candidate in candidates:
@@ -589,11 +592,11 @@ proc errorUndeclaredIdentifier*(c: PContext; info: TLineInfo; name: string, extr
       c.recursiveDep = ""
   localError(c.config, info, errGenerated, err)
 
-proc errorUndeclaredIdentifierHint*(c: PContext; n: PNode, ident: PIdent): PSym =
+proc errorUndeclaredIdentifierHint*(c: PContext; ident: PIdent; info: TLineInfo): PSym =
   var extra = ""
-  if c.mustFixSpelling: fixSpelling(c, n, ident, extra)
-  errorUndeclaredIdentifier(c, n.info, ident.s, extra)
-  result = errorSym(c, n)
+  if c.mustFixSpelling: fixSpelling(c, ident, extra)
+  errorUndeclaredIdentifier(c, info, ident.s, extra)
+  result = errorSym(c, ident, info)
 
 proc lookUp*(c: PContext, n: PNode): PSym =
   # Looks up a symbol. Generates an error in case of nil.
@@ -601,13 +604,13 @@ proc lookUp*(c: PContext, n: PNode): PSym =
   case n.kind
   of nkIdent:
     result = searchInScopes(c, n.ident, amb)
-    if result == nil: result = errorUndeclaredIdentifierHint(c, n, n.ident)
+    if result == nil: result = errorUndeclaredIdentifierHint(c, n.ident, n.info)
   of nkSym:
     result = n.sym
   of nkAccQuoted:
     var ident = considerQuotedIdent(c, n)
     result = searchInScopes(c, ident, amb)
-    if result == nil: result = errorUndeclaredIdentifierHint(c, n, ident)
+    if result == nil: result = errorUndeclaredIdentifierHint(c, ident, n.info)
   else:
     internalError(c.config, n.info, "lookUp")
     return nil
@@ -621,16 +624,29 @@ type
   TLookupFlag* = enum
     checkAmbiguity, checkUndeclared, checkModule, checkPureEnumFields
 
+const allExceptModule = {low(TSymKind)..high(TSymKind)} - {skModule, skPackage}
+
+proc lookUpCandidates*(c: PContext, ident: PIdent, filter: set[TSymKind]): seq[PSym] =
+  result = searchInScopesFilterBy(c, ident, filter)
+  if result.len == 0:
+    result.add allPureEnumFields(c, ident)
+
 proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
-  const allExceptModule = {low(TSymKind)..high(TSymKind)} - {skModule, skPackage}
   case n.kind
   of nkIdent, nkAccQuoted:
     var amb = false
     var ident = considerQuotedIdent(c, n)
     if checkModule in flags:
       result = searchInScopes(c, ident, amb)
+      if result == nil:
+        let candidates = allPureEnumFields(c, ident)
+        if candidates.len > 0:
+          result = candidates[0]
+          amb = candidates.len > 1
+          if amb and checkAmbiguity in flags:
+            errorUseQualifier(c, n.info, candidates)
     else:
-      let candidates = searchInScopesFilterBy(c, ident, allExceptModule)
+      let candidates = lookUpCandidates(c, ident, allExceptModule)
       if candidates.len > 0:
         result = candidates[0]
         amb = candidates.len > 1
@@ -638,16 +654,8 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
           errorUseQualifier(c, n.info, candidates)
       else:
         result = nil
-    if result == nil:
-      let candidates = allPureEnumFields(c, ident)
-      if candidates.len > 0:
-        result = candidates[0]
-        amb = candidates.len > 1
-        if amb and checkAmbiguity in flags:
-          errorUseQualifier(c, n.info, candidates)
-
     if result == nil and checkUndeclared in flags:
-      result = errorUndeclaredIdentifierHint(c, n, ident)
+      result = errorUndeclaredIdentifierHint(c, ident, n.info)
     elif checkAmbiguity in flags and result != nil and amb:
       result = errorUseQualifier(c, n.info, result, amb)
     c.isAmbiguous = amb
@@ -672,12 +680,12 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
           else:
             result = someSym(c.graph, m, ident)
         if result == nil and checkUndeclared in flags:
-          result = errorUndeclaredIdentifierHint(c, n[1], ident)
+          result = errorUndeclaredIdentifierHint(c, ident, n[1].info)
       elif n[1].kind == nkSym:
         result = n[1].sym
         if result.owner != nil and result.owner != m and checkUndeclared in flags:
           # dotExpr in templates can end up here
-          result = errorUndeclaredIdentifierHint(c, n[1], considerQuotedIdent(c, n[1]))
+          result = errorUndeclaredIdentifierHint(c, result.name, n[1].info)
       elif checkUndeclared in flags and
            n[1].kind notin {nkOpenSymChoice, nkClosedSymChoice}:
         localError(c.config, n[1].info, "identifier expected, but got: " &
diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index b46965875..67eee3a19 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -131,11 +131,13 @@ proc semSym(c: PContext, n: PNode, sym: PSym, flags: TExprFlags): PNode
 proc isSymChoice(n: PNode): bool {.inline.} =
   result = n.kind in nkSymChoices
 
-proc semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode =
-  result = n
+proc resolveSymChoice(c: PContext, n: var PNode, flags: TExprFlags = {}, expectedType: PType = nil) =
+  ## Attempts to resolve a symchoice `n`, `n` remains a symchoice if
+  ## it cannot be resolved (this is the case even when `n.len == 1`).
   if expectedType != nil:
-    result = fitNode(c, expectedType, result, n.info)
-  if isSymChoice(result) and efAllowSymChoice notin flags:
+    # resolve from type inference, see paramTypesMatch
+    n = fitNode(c, expectedType, n, n.info)
+  if isSymChoice(n) and efAllowSymChoice notin flags:
     # some contexts might want sym choices preserved for later disambiguation
     # in general though they are ambiguous
     let first = n[0].sym
@@ -145,17 +147,24 @@ proc semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: P
         foundSym == first:
       # choose the first resolved enum field, i.e. the latest in scope
       # to mirror behavior before overloadable enums
-      result = n[0]
-    else:
-      var err = "ambiguous identifier '" & first.name.s &
-        "' -- use one of the following:\n"
-      for child in n:
-        let candidate = child.sym
-        err.add "  " & candidate.owner.name.s & "." & candidate.name.s
-        err.add ": " & typeToString(candidate.typ) & "\n"
-      localError(c.config, n.info, err)
-      n.typ = errorType(c)
-      result = n
+      n = n[0]
+
+proc semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode =
+  result = n
+  resolveSymChoice(c, result, flags, expectedType)
+  if isSymChoice(result) and result.len == 1:
+    # resolveSymChoice can leave 1 sym
+    result = result[0]
+  if isSymChoice(result) and efAllowSymChoice notin flags:
+    var err = "ambiguous identifier: '" & result[0].sym.name.s &
+      "' -- use one of the following:\n"
+    for child in n:
+      let candidate = child.sym
+      err.add "  " & candidate.owner.name.s & "." & candidate.name.s
+      err.add ": " & typeToString(candidate.typ) & "\n"
+    localError(c.config, n.info, err)
+    n.typ = errorType(c)
+    result = n
   if result.kind == nkSym:
     result = semSym(c, result, result.sym, flags)
 
@@ -2989,6 +2998,55 @@ proc semPragmaStmt(c: PContext; n: PNode) =
   else:
     pragma(c, c.p.owner, n, stmtPragmas, true)
 
+proc resolveIdentToSym(c: PContext, n: PNode, resultNode: var PNode,
+                       flags: TExprFlags, expectedType: PType): PSym =
+  # result is nil on error or if a node that can't produce a sym is resolved
+  let ident = considerQuotedIdent(c, n)
+  if expectedType != nil and (
+      let expected = expectedType.skipTypes(abstractRange-{tyDistinct});
+      expected.kind == tyEnum):
+    let nameId = ident.id
+    for f in expected.n:
+      if f.kind == nkSym and f.sym.name.id == nameId:
+        return f.sym
+  var filter = {low(TSymKind)..high(TSymKind)}
+  if efNoEvaluateGeneric in flags:
+    # `a[...]` where `a` is a module or package is not possible
+    filter.excl {skModule, skPackage}
+  let candidates = lookUpCandidates(c, ident, filter)
+  if candidates.len == 0:
+    result = errorUndeclaredIdentifierHint(c, ident, n.info)
+  elif candidates.len == 1 or {efNoEvaluateGeneric, efInCall} * flags != {}:
+    # unambiguous, or we don't care about ambiguity
+    result = candidates[0]
+  else:
+    # ambiguous symbols have 1 last chance as a symchoice,
+    # but type symbols cannot participate in symchoices
+    var choice = newNodeIT(nkClosedSymChoice, n.info, newTypeS(tyNone, c))
+    for c in candidates:
+      if c.kind notin {skType, skModule, skPackage}:
+        choice.add newSymNode(c, n.info)
+    if choice.len == 0:
+      # we know candidates.len > 1, we just couldn't put any in a symchoice
+      errorUseQualifier(c, n.info, candidates)
+      return nil
+    resolveSymChoice(c, choice, flags, expectedType)
+    # choice.len == 1 can be true here but as long as it's a symchoice
+    # it's still not resolved
+    if isSymChoice(choice):
+      result = nil
+      if efAllowSymChoice in flags:
+        resultNode = choice
+      else:
+        errorUseQualifier(c, n.info, candidates)
+    else:
+      if choice.kind == nkSym:
+        result = choice.sym
+      else:
+        # resolution could have generated nkHiddenStdConv etc
+        resultNode = semExpr(c, choice, flags, expectedType)
+        result = nil
+
 proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode =
   when defined(nimCompilerStacktraceHints):
     setFrameMsg c.config$n.info & " " & $n.kind
@@ -3026,25 +3084,10 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
   if nfSem in n.flags: return
   case n.kind
   of nkIdent, nkAccQuoted:
-    var s: PSym = nil
-    if expectedType != nil and (
-        let expected = expectedType.skipTypes(abstractRange-{tyDistinct});
-        expected.kind == tyEnum):
-      let nameId = considerQuotedIdent(c, n).id
-      for f in expected.n:
-        if f.kind == nkSym and f.sym.name.id == nameId:
-          s = f.sym
-          break
+    let s = resolveIdentToSym(c, n, result, flags, expectedType)
     if s == nil:
-      let checks = if efNoEvaluateGeneric in flags:
-          {checkUndeclared, checkPureEnumFields}
-        elif efInCall in flags:
-          {checkUndeclared, checkModule, checkPureEnumFields}
-        else:
-          {checkUndeclared, checkModule, checkAmbiguity, checkPureEnumFields}
-      s = qualifiedLookUp(c, n, checks)
-      if s == nil:
-        return
+      # resolveIdentToSym either errored or gave a result node
+      return
     if c.matchedConcept == nil: semCaptureSym(s, c.p.owner)
     case s.kind
     of skProc, skFunc, skMethod, skConverter, skIterator:
diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim
index df70ff3b4..2dfab4d17 100644
--- a/compiler/sigmatch.nim
+++ b/compiler/sigmatch.nim
@@ -2338,8 +2338,8 @@ proc paramTypesMatch*(m: var TCandidate, f, a: PType,
   if arg == nil or arg.kind notin nkSymChoices:
     result = paramTypesMatchAux(m, f, a, arg, argOrig)
   else:
-    let matchSet = {skProc, skFunc, skMethod, skConverter,skIterator, skMacro,
-                    skTemplate, skEnumField}
+    # symbol kinds that don't participate in symchoice type disambiguation:
+    let matchSet = {low(TSymKind)..high(TSymKind)} - {skModule, skPackage, skType}
     
     var best = -1
     result = arg
diff --git a/tests/enum/tambiguousoverloads.nim b/tests/enum/tambiguousoverloads.nim
index aa75eaa91..12c78c848 100644
--- a/tests/enum/tambiguousoverloads.nim
+++ b/tests/enum/tambiguousoverloads.nim
@@ -9,7 +9,7 @@ block: # bug #21887
     EnumC = enum C
 
   doAssert typeof(EnumC(A)) is EnumC #[tt.Error
-                        ^ ambiguous identifier 'A' -- use one of the following:
+                        ^ ambiguous identifier: 'A' -- use one of the following:
   EnumA.A: EnumA
   EnumB.A: EnumB]#
 
@@ -21,6 +21,6 @@ block: # issue #22598
       red
 
   let a = red #[tt.Error
-          ^ ambiguous identifier 'red' -- use one of the following:
+          ^ ambiguous identifier: 'red' -- use one of the following:
   A.red: A
   B.red: B]#
diff --git a/tests/enum/tpure_enums_conflict.nim b/tests/enum/tpure_enums_conflict.nim
index 3c7528a72..4411fd2a6 100644
--- a/tests/enum/tpure_enums_conflict.nim
+++ b/tests/enum/tpure_enums_conflict.nim
@@ -1,4 +1,5 @@
 discard """
+  disabled: true # pure enums behave like overloaded enums on ambiguity now which gives a different error message
   errormsg: "ambiguous identifier: 'amb'"
   line: 19
 """
diff --git a/tests/errmsgs/t8064.nim b/tests/errmsgs/t8064.nim
index 6be83fd1a..c35a3abcc 100644
--- a/tests/errmsgs/t8064.nim
+++ b/tests/errmsgs/t8064.nim
@@ -5,5 +5,5 @@ values
 
 discard """
   # either this or "expression has no type":
-  errormsg: "ambiguous identifier 'values' -- use one of the following:"
+  errormsg: "ambiguous identifier: 'values' -- use one of the following:"
 """
diff --git a/tests/lookups/tambiguousemit.nim b/tests/lookups/tambiguousemit.nim
index 0ebd0a255..4f4bacce4 100644
--- a/tests/lookups/tambiguousemit.nim
+++ b/tests/lookups/tambiguousemit.nim
@@ -7,6 +7,6 @@ proc foo(x: int) = discard
 proc foo(x: float) = discard
 
 {.emit: ["// ", foo].} #[tt.Error
-                ^ ambiguous identifier 'foo' -- use one of the following:
+                ^ ambiguous identifier: 'foo' -- use one of the following:
   tambiguousemit.foo: proc (x: int){.noSideEffect, gcsafe.}
   tambiguousemit.foo: proc (x: float){.noSideEffect, gcsafe.}]#
diff --git a/tests/lookups/tambprocvar.nim b/tests/lookups/tambprocvar.nim
index 33323fbb2..f5c3fde4f 100644
--- a/tests/lookups/tambprocvar.nim
+++ b/tests/lookups/tambprocvar.nim
@@ -2,7 +2,7 @@ discard """
   action: reject
   cmd: "nim check $file"
   nimout: '''
-tambprocvar.nim(15, 11) Error: ambiguous identifier 'foo' -- use one of the following:
+tambprocvar.nim(15, 11) Error: ambiguous identifier: 'foo' -- use one of the following:
   tambprocvar.foo: proc (x: int){.noSideEffect, gcsafe.}
   tambprocvar.foo: proc (x: float){.noSideEffect, gcsafe.}
 '''
@@ -16,4 +16,4 @@ block:
 
 block:
   let x = `+` #[tt.Error
-           ^ ambiguous identifier '+' -- use one of the following:]#
+          ^ ambiguous identifier: '+' -- use one of the following:]#
diff --git a/tests/lookups/tambsym3.nim b/tests/lookups/tambsym3.nim
index 6e7589cd8..6bbebca10 100644
--- a/tests/lookups/tambsym3.nim
+++ b/tests/lookups/tambsym3.nim
@@ -1,5 +1,5 @@
 discard """
-  errormsg: "ambiguous identifier 'mDec' -- use one of the following:"
+  errormsg: "ambiguous identifier: 'mDec' -- use one of the following:"
   file: "tambsym3.nim"
   line: 11
 """
diff --git a/tests/macros/t23032_2.nim b/tests/macros/t23032_2.nim
index cb8e772e7..8dde29e10 100644
--- a/tests/macros/t23032_2.nim
+++ b/tests/macros/t23032_2.nim
@@ -1,6 +1,6 @@
 discard """
   action: "reject"
-  errormsg: "ambiguous identifier '%*'"
+  errormsg: "ambiguous identifier: '%*'"
 """
 import std/macros
 
diff --git a/tests/pragmas/monoff1.nim b/tests/pragmas/monoff1.nim
new file mode 100644
index 000000000..85d6c57b3
--- /dev/null
+++ b/tests/pragmas/monoff1.nim
@@ -0,0 +1 @@
+proc on*() = discard
diff --git a/tests/pragmas/tonoff1.nim b/tests/pragmas/tonoff1.nim
new file mode 100644
index 000000000..20ba7def2
--- /dev/null
+++ b/tests/pragmas/tonoff1.nim
@@ -0,0 +1,8 @@
+# issue #23002
+
+import monoff1
+
+proc test() =
+  {.warning[ProveInit]: on.}
+
+test()
diff --git a/tests/pragmas/tonoff2.nim b/tests/pragmas/tonoff2.nim
new file mode 100644
index 000000000..9dff5ef11
--- /dev/null
+++ b/tests/pragmas/tonoff2.nim
@@ -0,0 +1,14 @@
+discard """
+    action: compile
+"""
+
+# issue #22841
+
+import unittest
+
+proc on() =
+    discard
+
+suite "some suite":
+    test "some test":
+        discard