Merge pull request #4899 from som-snytt/issue/9616

[backport] SI-9616 False positive in unused import warning

4 files changed, 96 insertions, 24 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
index c2ff17452d..5ec16e84bb 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Contexts.scala
@@ -806,6 +806,12 @@ trait Contexts { self: Analyzer =>
         (e ne null) && (e.owner == scope) && (!settings.isScala212 || e.sym.exists)
       })
 
+    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)
@@ -832,9 +838,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
       }
@@ -958,11 +964,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))
 
     /** Is `sym` defined in package object of package `pkg`?
      *  Since sym may be defined in some parent of the package object,
@@ -1108,7 +1111,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:
@@ -1422,7 +1425,7 @@ trait Contexts { self: Analyzer =>
 
     /** The symbol with name `name` imported from import clause `tree`.
      */
-    def importedSymbol(name: Name): Symbol = importedSymbol(name, requireExplicit = false)
+    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
@@ -1432,7 +1435,7 @@ trait Contexts { self: Analyzer =>
     }
 
     /** 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
@@ -1449,7 +1452,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/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)
+  }
+}