Merge pull request #4616 from som-snytt/issue/9383

SI-9383 Improved unused import warning

5 files changed, 111 insertions, 37 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
index 4c840818da..fa266d43ed 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
@@ -24,7 +24,8 @@ trait Contexts { self: Analyzer =>
 
   object NoContext
     extends Context(EmptyTree, NoSymbol, EmptyScope, NoCompilationUnit,
-                    null) { // We can't pass the uninitialized `this`. Instead, we treat null specially in `Context#outer`
+      // We can't pass the uninitialized `this`. Instead, we treat null specially in `Context#outer`
+                    null) {
     enclClass  = this
     enclMethod = this
 
@@ -48,12 +49,11 @@ trait Contexts { self: Analyzer =>
   def ambiguousDefnAndImport(owner: Symbol, imp: ImportInfo) =
     LookupAmbiguous(s"it is both defined in $owner and imported subsequently by \n$imp")
 
-  private lazy val startContext = {
-    NoContext.make(
+  private lazy val startContext = NoContext.make(
     Template(List(), noSelfType, List()) setSymbol global.NoSymbol setType global.NoType,
     rootMirror.RootClass,
-    rootMirror.RootClass.info.decls)
-  }
+    rootMirror.RootClass.info.decls
+  )
 
   private lazy val allUsedSelectors =
     mutable.Map[ImportInfo, Set[ImportSelector]]() withDefaultValue Set()
@@ -802,6 +802,14 @@ trait Contexts { self: Analyzer =>
         (e ne null) && (e.owner == scope) && (!settings.isScala212 || e.sym.exists)
       })
 
+    /** Do something with the symbols with name `name` imported via the import in `imp`,
+     *  if any such symbol is accessible from this context and is a qualifying implicit.
+     */
+    private def withQualifyingImplicitAlternatives(imp: ImportInfo, name: Name, pre: Type)(f: Symbol => Unit) = for {
+      sym <- importedAccessibleSymbol(imp, name, requireExplicit = false, record = false).alternatives
+      if isQualifyingImplicit(name, sym, pre, imported = true)
+    } f(sym)
+
     private def collectImplicits(syms: Scope, pre: Type, imported: Boolean = false): List[ImplicitInfo] =
       for (sym <- syms.toList if isQualifyingImplicit(sym.name, sym, pre, imported)) yield
         new ImplicitInfo(sym.name, pre, sym)
@@ -819,9 +827,9 @@ trait Contexts { self: Analyzer =>
         case ImportSelector(from, _, to, _) :: sels1 =>
           var impls = collect(sels1) filter (info => info.name != from)
           if (to != nme.WILDCARD) {
-            for (sym <- importedAccessibleSymbol(imp, to).alternatives)
-              if (isQualifyingImplicit(to, sym, pre, imported = true))
-                impls = new ImplicitInfo(to, pre, sym) :: impls
+            withQualifyingImplicitAlternatives(imp, to, pre) { sym =>
+              impls = new ImplicitInfo(to, pre, sym) :: impls
+            }
           }
           impls
       }
@@ -946,11 +954,8 @@ trait Contexts { self: Analyzer =>
     /** The symbol with name `name` imported via the import in `imp`,
      *  if any such symbol is accessible from this context.
      */
-    def importedAccessibleSymbol(imp: ImportInfo, name: Name): Symbol =
-      importedAccessibleSymbol(imp, name, requireExplicit = false)
-
-    private def importedAccessibleSymbol(imp: ImportInfo, name: Name, requireExplicit: Boolean): Symbol =
-      imp.importedSymbol(name, requireExplicit) filter (s => isAccessible(s, imp.qual.tpe, superAccess = false))
+    private def importedAccessibleSymbol(imp: ImportInfo, name: Name, requireExplicit: Boolean, record: Boolean): Symbol =
+      imp.importedSymbol(name, requireExplicit, record) filter (s => isAccessible(s, imp.qual.tpe, superAccess = false))
 
     private def requiresQualifier(s: Symbol) = (
          s.owner.isClass
@@ -1057,7 +1062,7 @@ trait Contexts { self: Analyzer =>
       def imp2Explicit   = imp2 isExplicitImport name
 
       def lookupImport(imp: ImportInfo, requireExplicit: Boolean) =
-        importedAccessibleSymbol(imp, name, requireExplicit) filter qualifies
+        importedAccessibleSymbol(imp, name, requireExplicit, record = true) filter qualifies
 
       // Java: A single-type-import declaration d in a compilation unit c of package p
       // that imports a type named n shadows, throughout c, the declarations of:
@@ -1353,7 +1358,6 @@ trait Contexts { self: Analyzer =>
     protected def handleError(pos: Position, msg: String): Unit = onTreeCheckerError(pos, msg)
   }
 
-
   class ImportInfo(val tree: Import, val depth: Int) {
     def pos = tree.pos
     def posOf(sel: ImportSelector) = tree.pos withPoint sel.namePos
@@ -1369,19 +1373,20 @@ trait Contexts { self: Analyzer =>
     def isExplicitImport(name: Name): Boolean =
       tree.selectors exists (_.rename == name.toTermName)
 
-    /** The symbol with name `name` imported from import clause `tree`.
-     */
-    def importedSymbol(name: Name): Symbol = importedSymbol(name, requireExplicit = false)
+    /** The symbol with name `name` imported from import clause `tree`. */
+    def importedSymbol(name: Name): Symbol = importedSymbol(name, requireExplicit = false, record = true)
 
-    private def recordUsage(sel: ImportSelector, result: Symbol) {
-      def posstr = pos.source.file.name + ":" + posOf(sel).line
-      def resstr = if (tree.symbol.hasCompleteInfo) s"(qual=$qual, $result)" else s"(expr=${tree.expr}, ${result.fullLocationString})"
-      debuglog(s"In $this at $posstr, selector '${selectorString(sel)}' resolved to $resstr")
+    private def recordUsage(sel: ImportSelector, result: Symbol): Unit = {
+      debuglog(s"In $this at ${ pos.source.file.name }:${ posOf(sel).line }, selector '${ selectorString(sel)
+        }' resolved to ${
+          if (tree.symbol.hasCompleteInfo) s"(qual=$qual, $result)"
+          else s"(expr=${tree.expr}, ${result.fullLocationString})"
+        }")
       allUsedSelectors(this) += sel
     }
 
     /** If requireExplicit is true, wildcard imports are not considered. */
-    def importedSymbol(name: Name, requireExplicit: Boolean): Symbol = {
+    def importedSymbol(name: Name, requireExplicit: Boolean, record: Boolean): Symbol = {
       var result: Symbol = NoSymbol
       var renamed = false
       var selectors = tree.selectors
@@ -1398,7 +1403,7 @@ trait Contexts { self: Analyzer =>
         if (result == NoSymbol)
           selectors = selectors.tail
       }
-      if (settings.warnUnusedImport && selectors.nonEmpty && result != NoSymbol && pos != NoPosition)
+      if (record && settings.warnUnusedImport && selectors.nonEmpty && result != NoSymbol && pos != NoPosition)
         recordUsage(current, result)
 
       // Harden against the fallout from bugs like SI-6745
diff --git a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
index 7ce945dd6c..beaf3432b4 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
@@ -1408,7 +1408,7 @@ trait Implicits {
       }
 
       if (result.isFailure && settings.debug) // debuglog is not inlined for some reason
-        log("no implicits found for "+pt+" "+pt.typeSymbol.info.baseClasses+" "+implicitsOfExpectedType)
+        log(s"no implicits found for ${pt} ${pt.typeSymbol.info.baseClasses} ${implicitsOfExpectedType}")
 
       result
     }
diff --git a/test/files/neg/warn-unused-imports.check b/test/files/neg/warn-unused-imports.check
index 36c6dd03c3..0a53d7a9cd 100644
--- a/test/files/neg/warn-unused-imports.check
+++ b/test/files/neg/warn-unused-imports.check
@@ -1,33 +1,55 @@
-warn-unused-imports.scala:57: warning: Unused import
+warn-unused-imports_2.scala:133: error: type mismatch;
+ found   : Int(42)
+ required: Sample.X
+    f(42)                       // error
+      ^
+warn-unused-imports_2.scala:57: warning: Unused import
     import p1.A // warn
               ^
-warn-unused-imports.scala:62: warning: Unused import
+warn-unused-imports_2.scala:62: warning: Unused import
     import p1.{ A, B } // warn on A
                 ^
-warn-unused-imports.scala:67: warning: Unused import
+warn-unused-imports_2.scala:67: warning: Unused import
     import p1.{ A, B } // warn on both
                 ^
-warn-unused-imports.scala:67: warning: Unused import
+warn-unused-imports_2.scala:67: warning: Unused import
     import p1.{ A, B } // warn on both
                    ^
-warn-unused-imports.scala:73: warning: Unused import
+warn-unused-imports_2.scala:73: warning: Unused import
     import c._  // warn
              ^
-warn-unused-imports.scala:78: warning: Unused import
+warn-unused-imports_2.scala:78: warning: Unused import
     import p1._ // warn
               ^
-warn-unused-imports.scala:85: warning: Unused import
+warn-unused-imports_2.scala:85: warning: Unused import
     import c._  // warn
              ^
-warn-unused-imports.scala:91: warning: Unused import
+warn-unused-imports_2.scala:91: warning: Unused import
     import p1.c._  // warn
                 ^
-warn-unused-imports.scala:98: warning: Unused import
+warn-unused-imports_2.scala:98: warning: Unused import
     import p1._   // warn
               ^
-warn-unused-imports.scala:118: warning: Unused import
+warn-unused-imports_2.scala:118: warning: Unused import
     import p1.A   // warn
               ^
-error: No warnings can be incurred under -Xfatal-warnings.
-10 warnings found
+warn-unused-imports_2.scala:132: warning: Unused import
+    import Sample.Implicits._   // warn
+                            ^
+warn-unused-imports_2.scala:143: warning: Unused import
+    import Sample.Implicits.useless     // warn
+                            ^
+warn-unused-imports_2.scala:147: warning: Unused import
+    import java.io.File                 // warn
+                   ^
+warn-unused-imports_2.scala:148: warning: Unused import
+    import scala.concurrent.Future      // warn
+                            ^
+warn-unused-imports_2.scala:149: warning: Unused import
+    import scala.concurrent.ExecutionContext.Implicits.global // warn
+                                                       ^
+warn-unused-imports_2.scala:150: warning: Unused import
+    import p1.A                         // warn
+              ^
+16 warnings found
 one error found
diff --git a/test/files/neg/warn-unused-imports/sample_1.scala b/test/files/neg/warn-unused-imports/sample_1.scala
new file mode 100644
index 0000000000..d2f86239db
--- /dev/null
+++ b/test/files/neg/warn-unused-imports/sample_1.scala
@@ -0,0 +1,17 @@
+
+import language._
+
+object Sample {
+  trait X
+  trait Y
+
+  // import of the non-implicit should be unused
+  object Implicits {
+    def `int to X`(i: Int): X = null
+    implicit def `int to Y`(i: Int): Y = null
+    implicit def useless(i: Int): String = null
+  }
+
+  def f(x: X) = ???
+  def g(y: Y) = ???
+}
diff --git a/test/files/neg/warn-unused-imports.scala b/test/files/neg/warn-unused-imports/warn-unused-imports_2.scala
index b7a2f1c414..ded1186209 100644
--- a/test/files/neg/warn-unused-imports.scala
+++ b/test/files/neg/warn-unused-imports/warn-unused-imports_2.scala
@@ -123,3 +123,33 @@ trait Nested {
     println(new Warn { })
   }
 }
+
+// test unusage of imports from other compilation units after implicit search
+trait Outsiders {
+  {
+    //implicit search should not disable warning
+    import Sample._
+    import Sample.Implicits._   // warn
+    f(42)                       // error
+  }
+  {
+    import Sample._
+    import Sample.Implicits._   // nowarn
+    g(42)                       // ok
+  }
+  {
+    import Sample._
+    import Sample.Implicits.`int to Y`  // nowarn
+    import Sample.Implicits.useless     // warn
+    g(42)                       // ok
+  }
+  {
+    import java.io.File                 // warn
+    import scala.concurrent.Future      // warn
+    import scala.concurrent.ExecutionContext.Implicits.global // warn
+    import p1.A                         // warn
+    import p1.B                         // no warn
+    println("abc".bippy)
+    //Future("abc".bippy)
+  }
+}