Skip to content

Make suggestions of missing implicits imports on type errors #7862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 27 commits into from
Jan 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
7cea875
Make "did you mean hints" less chatty
odersky Dec 28, 2019
9a3c33a
Make suggestions of missing implicits imports on type errors
odersky Dec 28, 2019
53a6098
Sort suggestions alphabetically
odersky Dec 29, 2019
b848c57
Don't treat package object's <init> methods as package members
odersky Dec 30, 2019
13c1839
Refine search and reporting criteria
odersky Dec 30, 2019
e1360eb
Don't suggest imports for conversions from Null or Nothing
odersky Dec 30, 2019
5e3f11c
Implement cancellation
odersky Dec 31, 2019
540f69a
Subject implicit suggestion searches to timeouts
odersky Dec 31, 2019
2ccec1f
Streamline suggestion printing
odersky Jan 1, 2020
51f19ec
Use java.util.Timer
odersky Jan 1, 2020
55b1a31
Drop Scheduler
odersky Jan 2, 2020
9c67ee4
Revise search logic
odersky Jan 2, 2020
b3a4966
Some refinements
odersky Jan 2, 2020
717cb7d
Refine implicit search criterion and prioritization
odersky Jan 4, 2020
ad5055b
Further refinement of "did you mean" hints
odersky Jan 4, 2020
848dd0a
Print package symbols always under their full names
odersky Jan 4, 2020
72df855
Fix repl test
odersky Jan 4, 2020
0501bfa
Refactorings and comments for better clarity
odersky Jan 4, 2020
1124b35
Fix two more repl tests
odersky Jan 4, 2020
c601a72
Issue a "did you mean" hint only if there's nothing else to say
odersky Jan 5, 2020
02d7594
Remove dead code and data in "did you mean" hints
odersky Jan 5, 2020
9e72e6c
Better explanation for missing members that have extension methods
odersky Jan 5, 2020
aabcd81
Also suggest imports of extension methods
odersky Jan 5, 2020
427da51
Factor out suggestions logic into its own trait
odersky Jan 5, 2020
5ac8b80
Also suggest partial matches for missing implicit arguments
odersky Jan 5, 2020
8289d51
Don't mention evidence$n parameters in error message
odersky Jan 5, 2020
6f94a83
Fix unrelated test
odersky Jan 5, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/TypeErrors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion compiler/src/dotty/tools/dotc/printing/Printer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/reporting/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
42 changes: 22 additions & 20 deletions compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
Expand Down
10 changes: 9 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
128 changes: 70 additions & 58 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 =
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
*/
Expand All @@ -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)
Expand Down
Loading