Merge pull request #7862 from dotty-staging/add-suggest-givens

Make suggestions of missing implicits imports on type errors

Author: odersky

compiler/src/dotty/tools/dotc/Run.scala

@@ -31,6 +31,12 @@ import scala.util.control.NonFatal
 /** A compiler run. Exports various methods to compile source files */
 class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with ConstraintRunInfo {
 
+  /** If this variable is set to `true`, some core typer operations will
+   *  return immediately. Currently these early abort operations are
+   *  `Typer.typed` and `Implicits.typedImplicit`.
+   */
+  @volatile var isCancelled = false
+
   /** Produces the following contexts, from outermost to innermost
    *
    *    bootStrap:   A context with next available runId and a scope consisting of

compiler/src/dotty/tools/dotc/core/Definitions.scala

@@ -210,7 +210,9 @@ class Definitions {
   }
   @tu lazy val ScalaPackageObject: Symbol = ctx.requiredModule("scala.package")
   @tu lazy val JavaPackageVal: TermSymbol = ctx.requiredPackage(nme.java)
+  @tu lazy val JavaPackageClass: ClassSymbol = JavaPackageVal.moduleClass.asClass
   @tu lazy val JavaLangPackageVal: TermSymbol = ctx.requiredPackage(jnme.JavaLang)
+  @tu lazy val JavaLangPackageClass: ClassSymbol = JavaLangPackageVal.moduleClass.asClass
 
   // fundamental modules
   @tu lazy val SysPackage : Symbol = ctx.requiredModule("scala.sys.package")

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -2205,7 +2205,7 @@ object SymDenotations {
 
       if (symbol `eq` defn.ScalaPackageClass) {
         val denots = super.computeNPMembersNamed(name)
-        if (denots.exists) denots
+        if (denots.exists || name == nme.CONSTRUCTOR) denots
         else recur(packageObjs, NoDenotation)
       }
       else recur(packageObjs, NoDenotation)

compiler/src/dotty/tools/dotc/core/TypeErrors.scala

@@ -153,7 +153,7 @@ object CyclicReference {
   def apply(denot: SymDenotation)(implicit ctx: Context): CyclicReference = {
     val ex = new CyclicReference(denot)
     if (!(ctx.mode is Mode.CheckCyclic)) {
-      cyclicErrors.println(ex.getMessage)
+      cyclicErrors.println(s"Cyclic reference involving $denot")
       for (elem <- ex.getStackTrace take 200)
         cyclicErrors.println(elem.toString)
     }

compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala

@@ -278,7 +278,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
   }
 
   /** The string representation of this type used as a prefix */
-  protected def toTextRef(tp: SingletonType): Text = controlled {
+  def toTextRef(tp: SingletonType): Text = controlled {
     tp match {
       case tp: TermRef =>
         toTextPrefix(tp.prefix) ~ selectionString(tp)
@@ -438,6 +438,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
     (kindString(sym) ~~ {
       if (sym.isAnonymousClass) toTextParents(sym.info.parents) ~~ "{...}"
       else if (hasMeaninglessName(sym) && !printDebug) simpleNameString(sym.owner) + idString(sym)
+      else if sym.is(Package) then fullNameString(sym)
       else nameString(sym)
     }).close
 

compiler/src/dotty/tools/dotc/printing/Printer.scala

@@ -4,7 +4,7 @@ package printing
 
 import core._
 import Texts._, ast.Trees._
-import Types.Type, Symbols.Symbol, Scopes.Scope, Constants.Constant,
+import Types.{Type, SingletonType}, Symbols.Symbol, Scopes.Scope, Constants.Constant,
        Names.Name, Denotations._, Annotations.Annotation
 import typer.Implicits.SearchResult
 import util.SourcePosition
@@ -97,6 +97,9 @@ abstract class Printer {
    */
   def toText(sym: Symbol): Text
 
+  /** Textual representation of singeton type reference */
+  def toTextRef(tp: SingletonType): Text
+
   /** Textual representation of symbol's declaration */
   def dclText(sym: Symbol): Text
 

compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala

@@ -900,6 +900,8 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
     def name =
       if (printDebug)
         nameString(sym)
+      else if sym.is(Package) then
+        fullNameString(sym)
       else if (sym.is(ModuleClass) && sym.isPackageObject && sym.name.stripModuleClassSuffix == tpnme.PACKAGE)
         nameString(sym.owner.name)
       else if (sym.is(ModuleClass))

compiler/src/dotty/tools/dotc/reporting/Reporter.scala

@@ -15,6 +15,7 @@ import diagnostic.messages._
 import diagnostic._
 import ast.{tpd, Trees}
 import Message._
+import core.Decorators._
 
 import java.lang.System.currentTimeMillis
 import java.io.{ BufferedReader, PrintWriter }

compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala

@@ -343,33 +343,35 @@ object messages {
       }
 
       // Get closest match in `site`
-      val closest =
+      def closest: List[String] =
         decls
-        .map { case (n, sym) => (n, distance(n, name.show), sym) }
-        .collect { case (n, dist, sym) if dist <= maxDist => (n, dist, sym) }
+        .map { (n, sym) => (n, distance(n, name.show), sym) }
+        .collect {
+          case (n, dist, sym)
+          if dist <= maxDist && dist < (name.toString.length min n.length) =>
+            (n, dist, sym)
+        }
         .groupBy(_._2).toList
         .sortBy(_._1)
         .headOption.map(_._2).getOrElse(Nil)
         .map(incorrectChars).toList
         .sortBy(_._3)
-        .take(1).map { case (n, sym, _) => (n, sym) }
-
-      val siteName = site match {
-        case site: NamedType => site.name.show
-        case site => i"$site"
-      }
-
-      val closeMember = closest match {
-        case (n, sym) :: Nil =>
-          s" - did you mean $siteName.$n?"
-        case Nil => ""
-        case _ => assert(
-          false,
-          "Could not single out one distinct member to match on input with"
-        )
-      }
+        .map(_._1)
+          // [Martin] Note: I have no idea what this does. This shows the
+          // pitfalls of not naming things, functional or not.
+
+      val finalAddendum =
+        if addendum.nonEmpty then addendum
+        else closest match {
+          case n :: _ =>
+            val siteName = site match
+              case site: NamedType => site.name.show
+              case site => i"$site"
+            s" - did you mean $siteName.$n?"
+          case Nil => ""
+        }
 
-      ex"$selected $name is not a member of ${site.widen}$closeMember$addendum"
+      ex"$selected $name is not a member of ${site.widen}$finalAddendum"
     }
 
     val explanation: String = ""

compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala

@@ -150,7 +150,15 @@ object ErrorReporting {
       val expected1 = reported(expected)
       val (found2, expected2) =
         if (found1 frozen_<:< expected1) (found, expected) else (found1, expected1)
-      TypeMismatch(found2, expected2, whyNoMatchStr(found, expected), postScript)
+      val postScript1 =
+        if !postScript.isEmpty
+           || expected.isRef(defn.AnyClass)
+           || expected.isRef(defn.AnyValClass)
+           || expected.isRef(defn.ObjectClass)
+           || defn.isBottomType(found)
+        then postScript
+        else ctx.typer.importSuggestionAddendum(ViewProto(found.widen, expected))
+      TypeMismatch(found2, expected2, whyNoMatchStr(found, expected), postScript1)
     }
 
     /** Format `raw` implicitNotFound or implicitAmbiguous argument, replacing

compiler/src/dotty/tools/dotc/typer/Implicits.scala

@@ -14,7 +14,7 @@ import Flags._
 import TypeErasure.{erasure, hasStableErasure}
 import Mode.ImplicitsEnabled
 import NameOps._
-import NameKinds.LazyImplicitName
+import NameKinds.{LazyImplicitName, EvidenceParamName}
 import Symbols._
 import Denotations._
 import Types._
@@ -67,6 +67,14 @@ object Implicits {
     final val Extension = 4
   }
 
+  /** If `expected` is a selection prototype, does `tp` have an extension
+   *  method with the selecting name? False otherwise.
+   */
+  def hasExtMethod(tp: Type, expected: Type)(given Context) = expected match
+    case SelectionProto(name, _, _, _) =>
+      tp.memberBasedOnFlags(name, required = ExtensionMethod).exists
+    case _ => false
+
   /** A common base class of contextual implicits and of-type implicits which
    *  represents a set of references to implicit definitions.
    */
@@ -147,11 +155,7 @@ object Implicits {
               val isImplicitConversion = tpw.derivesFrom(defn.ConversionClass)
               // An implementation of <:< counts as a view
               val isConforms = tpw.derivesFrom(defn.SubTypeClass)
-              val hasExtensions = resType match {
-                case SelectionProto(name, _, _, _) =>
-                  tpw.memberBasedOnFlags(name, required = ExtensionMethod).exists
-                case _ => false
-              }
+              val hasExtensions = hasExtMethod(tpw, resType)
               val conversionKind =
                 if (isFunctionInS2 || isImplicitConversion || isConforms) Candidate.Conversion
                 else Candidate.None
@@ -1215,32 +1219,37 @@ trait Implicits { self: Typer =>
             pt.typeSymbol.typeParams.map(_.name.unexpandedName.toString),
             pt.widenExpr.argInfos))
 
-        def hiddenImplicitsAddendum: String = arg.tpe match {
-          case fail: SearchFailureType =>
-
-            def hiddenImplicitNote(s: SearchSuccess) =
-              em"\n\nNote: given instance ${s.ref.symbol.showLocated} was not considered because it was not imported with `import given`."
+        def hiddenImplicitsAddendum: String =
+
+          def hiddenImplicitNote(s: SearchSuccess) =
+            em"\n\nNote: given instance ${s.ref.symbol.showLocated} was not considered because it was not imported with `import given`."
+
+          def FindHiddenImplicitsCtx(ctx: Context): Context =
+            if (ctx == NoContext) ctx
+            else ctx.freshOver(FindHiddenImplicitsCtx(ctx.outer)).addMode(Mode.FindHiddenImplicits)
+
+          val normalImports = arg.tpe match
+            case fail: SearchFailureType =>
+              if (fail.expectedType eq pt) || isFullyDefined(fail.expectedType, ForceDegree.none) then
+                inferImplicit(fail.expectedType, fail.argument, arg.span)(
+                  FindHiddenImplicitsCtx(ctx)) match {
+                  case s: SearchSuccess => hiddenImplicitNote(s)
+                  case f: SearchFailure =>
+                    f.reason match {
+                      case ambi: AmbiguousImplicits => hiddenImplicitNote(ambi.alt1)
+                      case r => ""
+                    }
+                }
+              else
+                // It's unsafe to search for parts of the expected type if they are not fully defined,
+                // since these come with nested contexts that are lost at this point. See #7249 for an
+                // example where searching for a nested type causes an infinite loop.
+                ""
 
-            def FindHiddenImplicitsCtx(ctx: Context): Context =
-              if (ctx == NoContext) ctx
-              else ctx.freshOver(FindHiddenImplicitsCtx(ctx.outer)).addMode(Mode.FindHiddenImplicits)
+          def suggestedImports = importSuggestionAddendum(pt)
+          if normalImports.isEmpty then suggestedImports else normalImports
+        end hiddenImplicitsAddendum
 
-            if (fail.expectedType eq pt) || isFullyDefined(fail.expectedType, ForceDegree.none) then
-              inferImplicit(fail.expectedType, fail.argument, arg.span)(
-                FindHiddenImplicitsCtx(ctx)) match {
-                case s: SearchSuccess => hiddenImplicitNote(s)
-                case f: SearchFailure =>
-                  f.reason match {
-                    case ambi: AmbiguousImplicits => hiddenImplicitNote(ambi.alt1)
-                    case r => ""
-                  }
-              }
-            else
-              // It's unsafe to search for parts of the expected type if they are not fully defined,
-              // since these come with nested contexts that are lost at this point. See #7249 for an
-              // example where searching for a nested type causes an infinite loop.
-              ""
-        }
         msg(userDefined.getOrElse(
           em"no implicit argument of type $pt was found${location("for")}"))() ++
         hiddenImplicitsAddendum
@@ -1256,7 +1265,8 @@ trait Implicits { self: Typer =>
         def addendum = if (qt1 eq qt) "" else (i"\nwhich is an alias of: $qt1")
         em"parameter of ${qual.tpe.widen}$addendum"
       case _ =>
-        em"parameter ${paramName} of $methodStr"
+        em"${ if paramName.is(EvidenceParamName) then "an implicit parameter"
+              else s"parameter $paramName" } of $methodStr"
     }
 
   private def strictEquality(implicit ctx: Context): Boolean =
@@ -1352,32 +1362,10 @@ trait Implicits { self: Typer =>
       ctx.searchHistory.emitDictionary(span, result)
     }
 
-  /** An implicit search; parameters as in `inferImplicit` */
-  class ImplicitSearch(protected val pt: Type, protected val argument: Tree, span: Span)(implicit ctx: Context) {
-    assert(argument.isEmpty || argument.tpe.isValueType || argument.tpe.isInstanceOf[ExprType],
-        em"found: $argument: ${argument.tpe}, expected: $pt")
-
-    private def nestedContext() =
-      ctx.fresh.setMode(ctx.mode &~ Mode.ImplicitsEnabled)
-
-    private def implicitProto(resultType: Type, f: Type => Type) =
-      if (argument.isEmpty) f(resultType) else ViewProto(f(argument.tpe.widen), f(resultType))
-        // Not clear whether we need to drop the `.widen` here. All tests pass with it in place, though.
-
-    private def isCoherent = pt.isRef(defn.EqlClass)
-
-    /** The expected type for the searched implicit */
-    @threadUnsafe lazy val fullProto: Type = implicitProto(pt, identity)
-
-    /** The expected type where parameters and uninstantiated typevars are replaced by wildcard types */
-    val wildProto: Type = implicitProto(pt, wildApprox(_))
-
-    val isNot: Boolean = wildProto.classSymbol == defn.NotClass
-
-      //println(i"search implicits $pt / ${eligible.map(_.ref)}")
-
-    /** Try to typecheck an implicit reference */
-    def typedImplicit(cand: Candidate, contextual: Boolean)(implicit ctx: Context): SearchResult = trace(i"typed implicit ${cand.ref}, pt = $pt, implicitsEnabled == ${ctx.mode is ImplicitsEnabled}", implicits, show = true) {
+  /** Try to typecheck an implicit reference */
+  def typedImplicit(cand: Candidate, pt: Type, argument: Tree, span: Span)(implicit ctx: Context): SearchResult =  trace(i"typed implicit ${cand.ref}, pt = $pt, implicitsEnabled == ${ctx.mode is ImplicitsEnabled}", implicits, show = true) {
+    if ctx.run.isCancelled then NoMatchingImplicitsFailure
+    else
       record("typedImplicit")
       val ref = cand.ref
       val generated: Tree = tpd.ref(ref).withSpan(span.startPos)
@@ -1431,6 +1419,30 @@ trait Implicits { self: Typer =>
       }
     }
 
+  /** An implicit search; parameters as in `inferImplicit` */
+  class ImplicitSearch(protected val pt: Type, protected val argument: Tree, span: Span)(implicit ctx: Context) {
+    assert(argument.isEmpty || argument.tpe.isValueType || argument.tpe.isInstanceOf[ExprType],
+        em"found: $argument: ${argument.tpe}, expected: $pt")
+
+    private def nestedContext() =
+      ctx.fresh.setMode(ctx.mode &~ Mode.ImplicitsEnabled)
+
+    private def implicitProto(resultType: Type, f: Type => Type) =
+      if (argument.isEmpty) f(resultType) else ViewProto(f(argument.tpe.widen), f(resultType))
+        // Not clear whether we need to drop the `.widen` here. All tests pass with it in place, though.
+
+    private def isCoherent = pt.isRef(defn.EqlClass)
+
+    /** The expected type for the searched implicit */
+    @threadUnsafe lazy val fullProto: Type = implicitProto(pt, identity)
+
+    /** The expected type where parameters and uninstantiated typevars are replaced by wildcard types */
+    val wildProto: Type = implicitProto(pt, wildApprox(_))
+
+    val isNot: Boolean = wildProto.classSymbol == defn.NotClass
+
+      //println(i"search implicits $pt / ${eligible.map(_.ref)}")
+
     /** Try to type-check implicit reference, after checking that this is not
       * a diverging search
       */
@@ -1440,7 +1452,7 @@ trait Implicits { self: Typer =>
       else {
         val history = ctx.searchHistory.nest(cand, pt)
         val result =
-          typedImplicit(cand, contextual)(nestedContext().setNewTyperState().setFreshGADTBounds.setSearchHistory(history))
+          typedImplicit(cand, pt, argument, span)(nestedContext().setNewTyperState().setFreshGADTBounds.setSearchHistory(history))
         result match {
           case res: SearchSuccess =>
             ctx.searchHistory.defineBynameImplicit(pt.widenExpr, res)