Merge pull request #10645 from som-snytt/issue/12690-import-commit

Track import selector of implicit info in search

src/compiler/scala/tools/nsc/backend/jvm/PostProcessor.scala

@@ -16,7 +16,7 @@ package backend.jvm
 import java.util.concurrent.ConcurrentHashMap
 
 import scala.collection.mutable
-import scala.reflect.internal.util.{NoPosition, Position, StringContextStripMarginOps}
+import scala.reflect.internal.util.{NoPosition, Position}
 import scala.reflect.io.AbstractFile
 import scala.tools.asm.ClassWriter
 import scala.tools.asm.tree.ClassNode

src/compiler/scala/tools/nsc/plugins/PluginDescription.scala

@@ -13,8 +13,6 @@
 package scala.tools.nsc
 package plugins
 
-import scala.reflect.internal.util.StringContextStripMarginOps
-
 /** A description of a compiler plugin, suitable for serialization
  *  to XML for inclusion in the plugin's .jar file.
  *

src/compiler/scala/tools/nsc/settings/ScalaSettings.scala

@@ -22,7 +22,7 @@ import java.util.zip.Deflater
 import scala.annotation.{elidable, nowarn}
 import scala.collection.mutable
 import scala.language.existentials
-import scala.reflect.internal.util.{ StatisticsStatics, StringContextStripMarginOps }
+import scala.reflect.internal.util.StatisticsStatics
 import scala.tools.nsc.util.DefaultJarFactory
 import scala.tools.util.PathResolver.Defaults
 import scala.util.chaining._

src/compiler/scala/tools/nsc/typechecker/Contexts.scala

@@ -728,7 +728,7 @@ trait Contexts { self: Analyzer =>
     def makeImportContext(tree: Import): Context =
       make(tree).tap { ctx =>
         if (settings.warnUnusedImport && openMacros.isEmpty && !ctx.isRootImport)
-          allImportInfos(ctx.unit) ::= ((ctx.importOrNull, ctx.owner))
+          allImportInfos(ctx.unit) ::= ctx.importOrNull -> ctx.owner
       }
 
     /** Use reporter (possibly buffered) for errors/warnings and enable implicit conversion **/
@@ -770,6 +770,7 @@ trait Contexts { self: Analyzer =>
     def makeImplicit(reportAmbiguousErrors: Boolean) = {
       val c = makeSilent(reportAmbiguousErrors)
       c(ImplicitsEnabled | EnrichmentEnabled) = false
+      c(InImplicitSearch) = true
       c
     }
 
@@ -1095,9 +1096,9 @@ trait Contexts { self: Analyzer =>
         f(imported)
     }
 
-    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)
+    private def collectImplicits(syms: Scope, pre: Type): List[ImplicitInfo] =
+      for (sym <- syms.toList if isQualifyingImplicit(sym.name, sym, pre, imported = false))
+      yield new ImplicitInfo(sym.name, pre, sym)
 
     private def collectImplicitImports(imp: ImportInfo): List[ImplicitInfo] = if (isExcludedRootImport(imp)) List() else {
       val qual = imp.qual
@@ -1112,12 +1113,13 @@ trait Contexts { self: Analyzer =>
           // I haven't been able to boil that down the an automated test yet.
           // Looking up implicit members in the package, rather than package object, here is at least
           // consistent with what is done just below for named imports.
-          collectImplicits(qual.tpe.implicitMembers, pre, imported = true)
+          for (sym <- qual.tpe.implicitMembers.toList if isQualifyingImplicit(sym.name, sym, pre, imported = true))
+          yield new ImplicitInfo(sym.name, pre, sym, imp, sel)
         case (sel @ ImportSelector(from, _, to, _)) :: sels1 =>
           var impls = collect(sels1).filter(_.name != from)
           if (!sel.isMask)
             withQualifyingImplicitAlternatives(imp, to, pre) { sym =>
-              impls = new ImplicitInfo(to, pre, sym) :: impls
+              impls = new ImplicitInfo(to, pre, sym, imp, sel) :: impls
             }
           impls
       }
@@ -1353,7 +1355,6 @@ trait Contexts { self: Analyzer =>
       }
     }
 
-
     private def isReplImportWrapperImport(tree: Tree): Boolean = {
       tree match {
         case Import(expr, selector :: Nil) =>
@@ -1697,10 +1698,15 @@ trait Contexts { self: Analyzer =>
         }
 
         // the choice has been made
-        imp1.recordUsage(impSel, impSym)
+        if (lookupError == null) {
+          // implicit searcher decides when import was used
+          if (thisContext.contextMode.inNone(InImplicitSearch))
+            imp1.recordUsage(impSel, impSym)
 
-        // optimization: don't write out package prefixes
-        finish(duplicateAndResetPos.transform(imp1.qual), impSym)
+          // optimization: don't write out package prefixes
+          finish(duplicateAndResetPos.transform(imp1.qual), impSym)
+        }
+        else finish(EmptyTree, NoSymbol)
       }
       else finish(EmptyTree, NoSymbol)
     }
@@ -1911,7 +1917,7 @@ trait Contexts { self: Analyzer =>
     def qual: Tree = tree.symbol.info match {
       case ImportType(expr) => expr
       case ErrorType        => tree setType NoType // fix for #2870
-      case _                => throw new FatalError("symbol " + tree.symbol + " has bad type: " + tree.symbol.info) //debug
+      case bad              => throw new FatalError(s"symbol ${tree.symbol} has bad type: ${bad}")
     }
 
     /** Is name imported explicitly, not via wildcard? */
@@ -1978,7 +1984,7 @@ trait Contexts { self: Analyzer =>
           if (tree.symbol.hasCompleteInfo) s"(qual=$qual, $result)"
           else s"(expr=${tree.expr}, ${result.fullLocationString})"
         }")
-      if (settings.warnUnusedImport && result != NoSymbol && pos != NoPosition)
+      if (settings.warnUnusedImport && !isRootImport && result != NoSymbol && pos != NoPosition)
         allUsedSelectors(this) += sel
     }
 
@@ -2101,12 +2107,17 @@ object ContextMode {
   final val DiagUsedDefaults: ContextMode         = 1 << 18
 
   /** Are we currently typing the core or args of an annotation?
-    * When set, Java annotations may be instantiated directly.
-    */
+   *  When set, Java annotations may be instantiated directly.
+   */
   final val TypingAnnotation: ContextMode         = 1 << 19
 
   final val InPackageClauseName: ContextMode      = 1 << 20
 
+  /** Context created with makeImplicit, for use in implicit search.
+   *  Controls whether import elements are marked used on lookup.
+   */
+  final val InImplicitSearch: ContextMode         = 1 << 21
+
   /** TODO: The "sticky modes" are EXPRmode, PATTERNmode, TYPEmode.
    *  To mimic the sticky mode behavior, when captain stickyfingers
    *  comes around we need to propagate those modes but forget the other

src/compiler/scala/tools/nsc/typechecker/Implicits.scala

@@ -127,12 +127,10 @@ trait Implicits extends splain.SplainData {
     if (settings.areStatisticsEnabled) statistics.stopCounter(subtypeImpl, subtypeStart)
 
     if (result.isSuccess && settings.lintImplicitRecursion && result.tree.symbol != null) {
-      val s =
-        if (result.tree.symbol.isAccessor) result.tree.symbol.accessed
-        else if (result.tree.symbol.isModule) result.tree.symbol.moduleClass
-        else result.tree.symbol
+      val rts = result.tree.symbol
+      val s = if (rts.isAccessor) rts.accessed else if (rts.isModule) rts.moduleClass else rts
       if (s != NoSymbol && context.owner.hasTransOwner(s))
-        context.warning(result.tree.pos, s"Implicit resolves to enclosing ${result.tree.symbol}", WarningCategory.WFlagSelfImplicit)
+        context.warning(result.tree.pos, s"Implicit resolves to enclosing $rts", WarningCategory.WFlagSelfImplicit)
     }
     implicitSearchContext.emitImplicitDictionary(result)
   }
@@ -196,9 +194,8 @@ trait Implicits extends splain.SplainData {
    *                  that were instantiated by the winning implicit.
    *  @param undetparams undetermined type parameters
    */
-  class SearchResult(val tree: Tree, val subst: TreeTypeSubstituter, val undetparams: List[Symbol]) {
-    override def toString = "SearchResult(%s, %s)".format(tree,
-      if (subst.isEmpty) "" else subst)
+  class SearchResult(val tree: Tree, val subst: TreeTypeSubstituter, val undetparams: List[Symbol], val implicitInfo: ImplicitInfo = null) {
+    override def toString = s"SearchResult($tree, ${if (subst.isEmpty) "" else subst})"
 
     def isFailure          = false
     def isAmbiguousFailure = false
@@ -225,7 +222,7 @@ trait Implicits extends splain.SplainData {
    *  @param   pre    The prefix type of the implicit
    *  @param   sym    The symbol of the implicit
    */
-  class ImplicitInfo(val name: Name, val pre: Type, val sym: Symbol) {
+  class ImplicitInfo (val name: Name, val pre: Type, val sym: Symbol, val importInfo: ImportInfo = null, val importSelector: ImportSelector = null) {
     private[this] var tpeCache: Type = null
     private[this] var depolyCache: Type = null
     private[this] var isErroneousCache: TriState = TriState.Unknown
@@ -1207,8 +1204,12 @@ trait Implicits extends splain.SplainData {
           val savedInfos = undetParams.map(_.info)
           val typedFirstPending = {
             try {
-              if(isView || wildPtNotInstantiable || matchesPtInst(firstPending))
-                typedImplicit(firstPending, ptChecked = true, isLocalToCallsite)
+              if (isView || wildPtNotInstantiable || matchesPtInst(firstPending)) {
+                val res = typedImplicit(firstPending, ptChecked = true, isLocalToCallsite)
+                if (res.isFailure) res
+                else
+                  new SearchResult(res.tree, res.subst, res.undetparams, firstPending)
+              }
               else SearchFailure
             } finally {
               foreach2(undetParams, savedInfos){ (up, si) => up.setInfo(si) }
@@ -1268,6 +1269,8 @@ trait Implicits extends splain.SplainData {
               s"\n Note: implicit ${invalidImplicits.head} is not applicable here because it comes after the application point and it lacks an explicit result type.${if (invalidImplicits.head.isModule) " An object can be written as a lazy val with an explicit type." else ""}"
             )
         }
+        else if (best.implicitInfo != null && best.implicitInfo.importInfo != null)
+          best.implicitInfo.importInfo.recordUsage(best.implicitInfo.importSelector, best.tree.symbol)
 
         best
       }

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -18,7 +18,7 @@ import scala.annotation.{tailrec, unused}
 import scala.collection.mutable
 import mutable.ListBuffer
 import scala.reflect.internal.{Chars, TypesStats}
-import scala.reflect.internal.util.{CodeAction, FreshNameCreator, ListOfNil, Statistics, StringContextStripMarginOps}
+import scala.reflect.internal.util.{CodeAction, FreshNameCreator, ListOfNil, Statistics}
 import scala.tools.nsc.Reporting.{MessageFilter, Suppression, WConf, WarningCategory}, WarningCategory.Scala3Migration
 import scala.util.chaining._
 import symtab.Flags._

test/files/neg/t12690.check

@@ -0,0 +1,6 @@
+t12690.scala:10: warning: Unused import
+  import A._ // missing unused warning (order of imports is significant)
+           ^
+error: No warnings can be incurred under -Werror.
+1 warning
+1 error

test/files/neg/t12690.scala

@@ -0,0 +1,12 @@
+
+//> using options -Werror -Wunused:imports
+
+class X
+class Y extends X
+object A { implicit val x: X = new X }
+object B { implicit val y: Y = new Y }
+class C {
+  import B._
+  import A._ // missing unused warning (order of imports is significant)
+  def t = implicitly[X]
+}

test/files/neg/t12690b.check

@@ -0,0 +1,14 @@
+t12690b.scala:15: error: reference to v is ambiguous;
+it is imported twice in the same scope by
+import Y.v
+and import X.v
+    v
+    ^
+t12690b.scala:11: warning: Unused import
+  import X.v
+           ^
+t12690b.scala:12: warning: Unused import
+  import Y.v
+           ^
+2 warnings
+1 error

test/files/neg/t12690b.scala

@@ -0,0 +1,17 @@
+
+// scalac: -Wunused:imports -Werror
+
+object X {
+  val v = 27
+}
+object Y {
+  val v = 42
+}
+object Main {
+  import X.v
+  import Y.v
+  def main(args: Array[String]) = println {
+    //"hello, world"
+    v
+  }
+}

test/files/neg/warn-unused-imports/sample_1.scala

@@ -1,3 +1,4 @@
+// scalac: -Werror -Wunused:imports
 
 import language._