From 69e0cdb6c0f1efafc558b36af77f3a07964d3c7e Mon Sep 17 00:00:00 2001
From: Jake Leahy <jake@leahy.dev>
Date: Thu, 12 Dec 2024 06:55:55 +1100
Subject: [PATCH] Fix line info for import  (#24523)

Refs #24158


Fixes the line info of the module symbol (cases like `import as` and
grouped imports had wrong line info). Since that symbol's line info is
now used for the warnings, there isn't a separate line info stored for
`unusedImports`

Examples of fixed cases
```nim
import strutils as test #[
                ^ before
                   ^ after ]#

# This case was fixed by #24158, but only for unused imports
import std/[strutils, strutils] #[
        ^ before
                      ^ after ]#

from strutils import split #[
^ before
     ^ after ]#
```
---
 compiler/importer.nim               | 11 ++++++-----
 compiler/lookups.nim                |  2 +-
 compiler/sem.nim                    |  6 +++---
 tests/import/tduplicate_imports.nim | 17 +++++++++++++++++
 tests/msgs/tused2.nim               |  2 +-
 tests/tools/tunused_imports.nim     | 14 ++++++++------
 6 files changed, 36 insertions(+), 16 deletions(-)
 create mode 100644 tests/import/tduplicate_imports.nim

diff --git a/compiler/importer.nim b/compiler/importer.nim
index ffb7e03056..7296f8f070 100644
--- a/compiler/importer.nim
+++ b/compiler/importer.nim
@@ -246,8 +246,9 @@ proc importModuleAs(c: PContext; n: PNode, realModule: PSym, importHidden: bool)
     result = createModuleAliasImpl(realModule.name)
   if importHidden:
     result.options.incl optImportHidden
-  let moduleIdent = if n.kind == nkInfix: n[^1] else: n
-  c.unusedImports.add((result, moduleIdent.info))
+  let moduleIdent = if n.kind in {nkInfix, nkImportAs}: n[^1] else: n
+  result.info = moduleIdent.info
+  c.unusedImports.add((result, result.info))
   c.importModuleMap[result.id] = realModule.id
   c.importModuleLookup.mgetOrPut(result.name.id, @[]).addUnique realModule.id
 
@@ -335,7 +336,7 @@ proc impMod(c: PContext; it: PNode; importStmtResult: PNode) =
   let m = myImportModule(c, it, importStmtResult)
   if m != nil:
     # ``addDecl`` needs to be done before ``importAllSymbols``!
-    addDecl(c, m, it.info) # add symbol to symbol table of module
+    addDecl(c, m) # add symbol to symbol table of module
     importAllSymbols(c, m)
     #importForwarded(c, m.ast, emptySet, m)
     afterImport(c, m)
@@ -372,7 +373,7 @@ proc evalFrom*(c: PContext, n: PNode): PNode =
   var m = myImportModule(c, n[0], result)
   if m != nil:
     n[0] = newSymNode(m)
-    addDecl(c, m, n.info)               # add symbol to symbol table of module
+    addDecl(c, m)               # add symbol to symbol table of module
 
     var im = ImportedModule(m: m, mode: importSet, imported: initIntSet())
     for i in 1..<n.len:
@@ -387,7 +388,7 @@ proc evalImportExcept*(c: PContext, n: PNode): PNode =
   var m = myImportModule(c, n[0], result)
   if m != nil:
     n[0] = newSymNode(m)
-    addDecl(c, m, n.info)               # add symbol to symbol table of module
+    addDecl(c, m)               # add symbol to symbol table of module
     importAllSymbolsExcept(c, m, readExceptSet(c, n))
     #importForwarded(c, m.ast, exceptSet, m)
     afterImport(c, m)
diff --git a/compiler/lookups.nim b/compiler/lookups.nim
index 745915cdf9..e452da959d 100644
--- a/compiler/lookups.nim
+++ b/compiler/lookups.nim
@@ -726,7 +726,7 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
 proc initOverloadIter*(o: var TOverloadIter, c: PContext, n: PNode): PSym =
   if n.kind == nkOpenSym:
     # maybe the logic in semexprs should be mirrored here instead
-    # for now it only seems this is called for `pickSym` in `getTypeIdent` 
+    # for now it only seems this is called for `pickSym` in `getTypeIdent`
     return initOverloadIter(o, c, n[0])
   o.importIdx = -1
   o.marked = initIntSet()
diff --git a/compiler/sem.nim b/compiler/sem.nim
index a42719a753..1b18dc633f 100644
--- a/compiler/sem.nim
+++ b/compiler/sem.nim
@@ -855,9 +855,9 @@ proc semWithPContext*(c: PContext, n: PNode): PNode =
 
 proc reportUnusedModules(c: PContext) =
   if c.config.cmd == cmdM: return
-  for i in 0..high(c.unusedImports):
-    if sfUsed notin c.unusedImports[i][0].flags:
-      message(c.config, c.unusedImports[i][1], warnUnusedImportX, c.unusedImports[i][0].name.s)
+  for (s, info) in c.unusedImports:
+    if sfUsed notin s.flags:
+      message(c.config, info, warnUnusedImportX, s.name.s)
 
 proc closePContext*(graph: ModuleGraph; c: PContext, n: PNode): PNode =
   if c.config.cmd == cmdIdeTools and not c.suggestionsMade:
diff --git a/tests/import/tduplicate_imports.nim b/tests/import/tduplicate_imports.nim
new file mode 100644
index 0000000000..cd6b319c10
--- /dev/null
+++ b/tests/import/tduplicate_imports.nim
@@ -0,0 +1,17 @@
+discard """
+  cmd: '''nim c --hint:Processing:off $file'''
+  action: compile
+  nimout: '''
+tduplicate_imports.nim(12, 23) Hint: duplicate import of 'strutils'; previous import here: tduplicate_imports.nim(12, 13) [DuplicateModuleImport]
+tduplicate_imports.nim(15, 20) Hint: duplicate import of 'foobar'; previous import here: tduplicate_imports.nim(14, 20) [DuplicateModuleImport]
+tduplicate_imports.nim(16, 10) Hint: duplicate import of 'strutils'; previous import here: tduplicate_imports.nim(12, 23) [DuplicateModuleImport]
+tduplicate_imports.nim(17, 8) Hint: duplicate import of 'strutils'; previous import here: tduplicate_imports.nim(16, 10) [DuplicateModuleImport]
+'''
+"""
+
+import std/[strutils, strutils]
+
+import strutils as foobar
+import strutils as foobar
+from std/strutils import split
+import strutils except split
diff --git a/tests/msgs/tused2.nim b/tests/msgs/tused2.nim
index 5ccda77379..4aff9fa951 100644
--- a/tests/msgs/tused2.nim
+++ b/tests/msgs/tused2.nim
@@ -9,7 +9,7 @@ mused2a.nim(20, 7) Hint: 'fn7' is declared but not used [XDeclaredButNotUsed]
 mused2a.nim(23, 6) Hint: 'T1' is declared but not used [XDeclaredButNotUsed]
 mused2a.nim(1, 12) Warning: imported and not used: 'strutils' [UnusedImport]
 mused2a.nim(3, 10) Warning: imported and not used: 'os' [UnusedImport]
-mused2a.nim(5, 23) Warning: imported and not used: 'typetraits2' [UnusedImport]
+mused2a.nim(5, 26) Warning: imported and not used: 'typetraits2' [UnusedImport]
 mused2a.nim(6, 10) Warning: imported and not used: 'setutils' [UnusedImport]
 tused2.nim(42, 8) Warning: imported and not used: 'mused2a' [UnusedImport]
 tused2.nim(45, 12) Warning: imported and not used: 'strutils' [UnusedImport]
diff --git a/tests/tools/tunused_imports.nim b/tests/tools/tunused_imports.nim
index 539608ad63..35e3720b1f 100644
--- a/tests/tools/tunused_imports.nim
+++ b/tests/tools/tunused_imports.nim
@@ -1,12 +1,13 @@
 discard """
   cmd: '''nim c --hint:Processing:off $file'''
   nimout: '''
-tunused_imports.nim(14, 10) Warning: BEGIN [User]
-tunused_imports.nim(41, 10) Warning: END [User]
-tunused_imports.nim(37, 8) Warning: imported and not used: 'strutils' [UnusedImport]
-tunused_imports.nim(38, 13) Warning: imported and not used: 'strtabs' [UnusedImport]
-tunused_imports.nim(38, 22) Warning: imported and not used: 'cstrutils' [UnusedImport]
-tunused_imports.nim(39, 12) Warning: imported and not used: 'macrocache' [UnusedImport]
+tunused_imports.nim(15, 10) Warning: BEGIN [User]
+tunused_imports.nim(43, 10) Warning: END [User]
+tunused_imports.nim(38, 8) Warning: imported and not used: 'strutils' [UnusedImport]
+tunused_imports.nim(39, 13) Warning: imported and not used: 'strtabs' [UnusedImport]
+tunused_imports.nim(39, 22) Warning: imported and not used: 'cstrutils' [UnusedImport]
+tunused_imports.nim(40, 12) Warning: imported and not used: 'macrocache' [UnusedImport]
+tunused_imports.nim(41, 25) Warning: imported and not used: 'basics' [UnusedImport]
 '''
   action: "compile"
 """
@@ -37,5 +38,6 @@ bar()
 import strutils
 import std/[strtabs, cstrutils]
 import std/macrocache
+import std/strbasics as basics
 
 {.warning: "END".}