Skip to content
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

Give "did you mean ...?" hints also for simple identifiers #18747

Merged
merged 3 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
162 changes: 162 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/DidYouMean.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
package dotty.tools
package dotc
package reporting

import core._
import Contexts._
import Decorators.*, Symbols.*, Names.*, Types.*, Flags.*
import typer.ProtoTypes.{FunProto, SelectionProto}
import transform.SymUtils.isNoValue

/** A utility object to support "did you mean" hinting */
object DidYouMean:

def kindOK(sym: Symbol, isType: Boolean, isApplied: Boolean)(using Context): Boolean =
if isType then sym.isType
else sym.isTerm || isApplied && sym.isClass && !sym.is(ModuleClass)
// also count classes if followed by `(` since they have constructor proxies,
// but these don't show up separately as members
// Note: One need to be careful here not to complete symbols. For instance,
// we run into trouble if we ask whether a symbol is a legal value.

/** The names of all non-synthetic, non-private members of `site`
* that are of the same type/term kind as the missing member.
*/
def memberCandidates(site: Type, isType: Boolean, isApplied: Boolean)(using Context): collection.Set[Symbol] =
for
bc <- site.widen.baseClasses.toSet
sym <- bc.info.decls.filter(sym =>
kindOK(sym, isType, isApplied)
&& !sym.isConstructor
&& !sym.flagsUNSAFE.isOneOf(Synthetic | Private))
yield sym

case class Binding(name: Name, sym: Symbol, site: Type)

/** The name, symbol, and prefix type of all non-synthetic declarations that are
* defined or imported in some enclosing scope and that are of the same type/term
* kind as the missing member.
*/
def inScopeCandidates(isType: Boolean, isApplied: Boolean, rootImportOK: Boolean)(using Context): collection.Set[Binding] =
val acc = collection.mutable.HashSet[Binding]()
def nextInteresting(ctx: Context): Context =
if ctx.outer.isImportContext
|| ctx.outer.scope != ctx.scope
|| ctx.outer.owner.isClass && ctx.outer.owner != ctx.owner
|| (ctx.outer eq NoContext)
then ctx.outer
else nextInteresting(ctx.outer)

def recur()(using Context): Unit =
if ctx eq NoContext then
() // done
else if ctx.isImportContext then
val imp = ctx.importInfo.nn
if imp.isRootImport && !rootImportOK then
() // done
else imp.importSym.info match
case ImportType(expr) =>
val candidates = memberCandidates(expr.tpe, isType, isApplied)
if imp.isWildcardImport then
for cand <- candidates if !imp.excluded.contains(cand.name.toTermName) do
acc += Binding(cand.name, cand, expr.tpe)
for sel <- imp.selectors do
val selStr = sel.name.show
if sel.name == sel.rename then
for cand <- candidates if cand.name.toTermName.show == selStr do
acc += Binding(cand.name, cand, expr.tpe)
else if !sel.isUnimport then
for cand <- candidates if cand.name.toTermName.show == selStr do
acc += Binding(sel.rename.likeSpaced(cand.name), cand, expr.tpe)
case _ =>
recur()(using nextInteresting(ctx))
else
if ctx.owner.isClass then
for sym <- memberCandidates(ctx.owner.typeRef, isType, isApplied) do
acc += Binding(sym.name, sym, ctx.owner.thisType)
else
ctx.scope.foreach: sym =>
if kindOK(sym, isType, isApplied)
&& !sym.isConstructor
&& !sym.flagsUNSAFE.is(Synthetic)
then acc += Binding(sym.name, sym, NoPrefix)
recur()(using nextInteresting(ctx))
end recur

recur()
acc
end inScopeCandidates

/** The Levenshtein distance between two strings */
def distance(s1: String, s2: String): Int =
val dist = Array.ofDim[Int](s2.length + 1, s1.length + 1)
for
j <- 0 to s2.length
i <- 0 to s1.length
do
dist(j)(i) =
if j == 0 then i
else if i == 0 then j
else if s2(j - 1) == s1(i - 1) then dist(j - 1)(i - 1)
else (dist(j - 1)(i) min dist(j)(i - 1) min dist(j - 1)(i - 1)) + 1
dist(s2.length)(s1.length)

/** List of possible candidate names with their Levenstein distances
* to the name `from` of the missing member.
* @param maxDist Maximal number of differences to be considered for a hint
* A distance qualifies if it is at most `maxDist`, shorter than
* the lengths of both the candidate name and the missing member name
* and not greater than half the average of those lengths.
*/
extension [S <: Symbol | Binding](candidates: collection.Set[S])
def closestTo(str: String, maxDist: Int = 3)(using Context): List[(Int, S)] =
def nameStr(cand: S): String = cand match
case sym: Symbol => sym.name.show
case bdg: Binding => bdg.name.show
candidates
.toList
.map(cand => (distance(nameStr(cand), str), cand))
.filter((d, cand) =>
d <= maxDist
&& d * 4 <= str.length + nameStr(cand).length
&& d < str.length
&& d < nameStr(cand).length)
.sortBy((d, cand) => (d, nameStr(cand))) // sort by distance first, alphabetically second

def didYouMean(candidates: List[(Int, Binding)], proto: Type, prefix: String)(using Context): String =

def qualifies(b: Binding)(using Context): Boolean =
try
val valueOK = proto match
case _: SelectionProto => true
case _ => !b.sym.isNoValue
val accessOK = b.sym.isAccessibleFrom(b.site)
valueOK && accessOK
catch case ex: Exception => false
// exceptions might arise when completing (e.g. malformed class file, or cyclic reference)

def showName(name: Name, sym: Symbol)(using Context): String =
if sym.is(ModuleClass) then s"${name.show}.type"
else name.show

def alternatives(distance: Int, candidates: List[(Int, Binding)]): List[Binding] = candidates match
case (d, b) :: rest if d == distance =>
if qualifies(b) then b :: alternatives(distance, rest) else alternatives(distance, rest)
case _ =>
Nil

def recur(candidates: List[(Int, Binding)]): String = candidates match
case (d, b) :: rest
if d != 0 || b.sym.is(ModuleClass) => // Avoid repeating the same name in "did you mean"
if qualifies(b) then
def hint(b: Binding) = prefix ++ showName(b.name, b.sym)
val alts = alternatives(d, rest).map(hint).take(3)
val suffix = if alts.isEmpty then "" else alts.mkString(" or perhaps ", " or ", "?")
s" - did you mean ${hint(b)}?$suffix"
else
recur(rest)
case _ => ""

recur(candidates)
end didYouMean
end DidYouMean
92 changes: 37 additions & 55 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import ErrorMessageID._
import ast.Trees
import config.{Feature, ScalaVersion}
import typer.ErrorReporting.{err, matchReductionAddendum, substitutableTypeSymbolsInScope}
import typer.ProtoTypes.ViewProto
import typer.ProtoTypes.{ViewProto, SelectionProto, FunProto}
import typer.Implicits.*
import typer.Inferencing
import scala.util.control.NonFatal
Expand All @@ -34,6 +34,7 @@ import dotty.tools.dotc.util.Spans.Span
import dotty.tools.dotc.util.SourcePosition
import scala.jdk.CollectionConverters.*
import dotty.tools.dotc.util.SourceFile
import DidYouMean.*

/** Messages
* ========
Expand Down Expand Up @@ -243,14 +244,29 @@ extends NamingMsg(DuplicateBindID) {
}
}

class MissingIdent(tree: untpd.Ident, treeKind: String, val name: Name)(using Context)
class MissingIdent(tree: untpd.Ident, treeKind: String, val name: Name, proto: Type)(using Context)
extends NotFoundMsg(MissingIdentID) {
def msg(using Context) = i"Not found: $treeKind$name"
def msg(using Context) =
val missing = name.show
val addendum =
didYouMean(
inScopeCandidates(name.isTypeName, isApplied = proto.isInstanceOf[FunProto], rootImportOK = true)
.closestTo(missing),
proto, "")

i"Not found: $treeKind$name$addendum"
def explain(using Context) = {
i"""|The identifier for `$treeKind$name` is not bound, that is,
|no declaration for this identifier can be found.
|That can happen, for example, if `$name` or its declaration has either been
|misspelt or if an import is missing."""
i"""|Each identifier in Scala needs a matching declaration. There are two kinds of
|identifiers: type identifiers and value identifiers. Value identifiers are introduced
|by `val`, `def`, or `object` declarations. Type identifiers are introduced by `type`,
|`class`, `enum`, or `trait` declarations.
|
|Identifiers refer to matching declarations in their environment, or they can be
|imported from elsewhere.
|
|Possible reasons why no matching declaration was found:
| - The declaration or the use is mis-spelt.
| - An import is missing."""
}
}

Expand Down Expand Up @@ -309,48 +325,13 @@ class TypeMismatch(found: Type, expected: Type, inTree: Option[untpd.Tree], adde

end TypeMismatch

class NotAMember(site: Type, val name: Name, selected: String, addendum: => String = "")(using Context)
class NotAMember(site: Type, val name: Name, selected: String, proto: Type, addendum: => String = "")(using Context)
extends NotFoundMsg(NotAMemberID), ShowMatchTrace(site) {
//println(i"site = $site, decls = ${site.decls}, source = ${site.typeSymbol.sourceFile}") //DEBUG

def msg(using Context) = {
import core.Flags._
val maxDist = 3 // maximal number of differences to be considered for a hint
val missing = name.show

// The symbols of all non-synthetic, non-private members of `site`
// that are of the same type/term kind as the missing member.
def candidates: Set[Symbol] =
for
bc <- site.widen.baseClasses.toSet
sym <- bc.info.decls.filter(sym =>
sym.isType == name.isTypeName
&& !sym.isConstructor
&& !sym.flagsUNSAFE.isOneOf(Synthetic | Private))
yield sym

// Calculate Levenshtein distance
def distance(s1: String, s2: String): Int =
val dist = Array.ofDim[Int](s2.length + 1, s1.length + 1)
for
j <- 0 to s2.length
i <- 0 to s1.length
do
dist(j)(i) =
if j == 0 then i
else if i == 0 then j
else if s2(j - 1) == s1(i - 1) then dist(j - 1)(i - 1)
else (dist(j - 1)(i) min dist(j)(i - 1) min dist(j - 1)(i - 1)) + 1
dist(s2.length)(s1.length)

// A list of possible candidate symbols with their Levenstein distances
// to the name of the missing member
def closest: List[(Int, Symbol)] = candidates
.toList
.map(sym => (distance(sym.name.show, missing), sym))
.filter((d, sym) => d <= maxDist && d < missing.length && d < sym.name.show.length)
.sortBy((d, sym) => (d, sym.name.show)) // sort by distance first, alphabetically second

val enumClause =
if ((name eq nme.values) || (name eq nme.valueOf)) && site.classSymbol.companionClass.isEnumClass then
val kind = if name eq nme.values then i"${nme.values} array" else i"${nme.valueOf} lookup method"
Expand All @@ -367,17 +348,18 @@ extends NotFoundMsg(NotAMemberID), ShowMatchTrace(site) {

val finalAddendum =
if addendum.nonEmpty then prefixEnumClause(addendum)
else closest match
case (d, sym) :: _ =>
val siteName = site match
case site: NamedType => site.name.show
case site => i"$site"
val showName =
// Add .type to the name if it is a module
if sym.is(ModuleClass) then s"${sym.name.show}.type"
else sym.name.show
s" - did you mean $siteName.$showName?$enumClause"
case Nil => prefixEnumClause("")
else
val hint = didYouMean(
memberCandidates(site, name.isTypeName, isApplied = proto.isInstanceOf[FunProto])
.closestTo(missing)
.map((d, sym) => (d, Binding(sym.name, sym, site))),
proto,
prefix = site match
case site: NamedType => i"${site.name}."
case site => i"$site."
)
if hint.isEmpty then prefixEnumClause("")
else hint ++ enumClause

i"$selected $name is not a member of ${site.widen}$finalAddendum"
}
Expand Down Expand Up @@ -876,7 +858,7 @@ extends Message(PatternMatchExhaustivityID) {

val pathes = List(
ActionPatch(
srcPos = endPos,
srcPos = endPos,
replacement = uncoveredCases.map(c => indent(s"case $c => ???", startColumn))
.mkString("\n", "\n", "")
),
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1529,7 +1529,7 @@ trait Checking {
&& !qualType.member(sel.name).exists
&& !qualType.member(sel.name.toTypeName).exists
then
report.error(NotAMember(qualType, sel.name, "value"), sel.imported.srcPos)
report.error(NotAMember(qualType, sel.name, "value", WildcardType), sel.imported.srcPos)
if sel.isUnimport then
if originals.contains(sel.name) then
report.error(UnimportedAndImported(sel.name, targets.contains(sel.name)), sel.imported.srcPos)
Expand Down
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ trait TypeAssigner {

def importSuggestionAddendum(pt: Type)(using Context): String = ""

def notAMemberErrorType(tree: untpd.Select, qual: Tree)(using Context): ErrorType =
def notAMemberErrorType(tree: untpd.Select, qual: Tree, proto: Type)(using Context): ErrorType =
val qualType = qual.tpe.widenIfUnstable
def kind = if tree.isType then "type" else "value"
val foundWithoutNull = qualType match
Expand All @@ -177,7 +177,7 @@ trait TypeAssigner {
def addendum = err.selectErrorAddendum(tree, qual, qualType, importSuggestionAddendum, foundWithoutNull)
val msg: Message =
if tree.name == nme.CONSTRUCTOR then em"$qualType does not have a constructor"
else NotAMember(qualType, tree.name, kind, addendum)
else NotAMember(qualType, tree.name, kind, proto, addendum)
errorType(msg, tree.srcPos)

def inaccessibleErrorType(tpe: NamedType, superAccess: Boolean, pos: SrcPos)(using Context): Type =
Expand Down Expand Up @@ -206,7 +206,7 @@ trait TypeAssigner {
def assignType(tree: untpd.Select, qual: Tree)(using Context): Select =
val rawType = selectionType(tree, qual)
val checkedType = ensureAccessible(rawType, qual.isInstanceOf[Super], tree.srcPos)
val ownType = checkedType.orElse(notAMemberErrorType(tree, qual))
val ownType = checkedType.orElse(notAMemberErrorType(tree, qual, WildcardType))
assignType(tree, ownType)

/** Normalize type T appearing in a new T by following eta expansions to
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
then // we are in the arguments of a this(...) constructor call
errorTree(tree, em"$tree is not accessible from constructor arguments")
else
errorTree(tree, MissingIdent(tree, kind, name))
errorTree(tree, MissingIdent(tree, kind, name, pt))
end typedIdent

/** (1) If this reference is neither applied nor selected, check that it does
Expand Down Expand Up @@ -754,7 +754,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
case rawType: NamedType =>
inaccessibleErrorType(rawType, superAccess, tree.srcPos)
case _ =>
notAMemberErrorType(tree, qual))
notAMemberErrorType(tree, qual, pt))
end typedSelect

def typedSelect(tree: untpd.Select, pt: Type)(using Context): Tree = {
Expand Down
2 changes: 1 addition & 1 deletion tests/neg-macros/i15009a.check
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@
-- [E006] Not Found Error: tests/neg-macros/i15009a.scala:12:2 ---------------------------------------------------------
12 | $int // error: Not found: $int
| ^^^^
| Not found: $int
| Not found: $int - did you mean int?
|
| longer explanation available when compiling with `-explain`
2 changes: 1 addition & 1 deletion tests/neg/i13320.check
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@
-- [E008] Not Found Error: tests/neg/i13320.scala:4:22 -----------------------------------------------------------------
4 |var x: Foo.Booo = Foo.Booo // error // error
| ^^^^^^^^
| value Booo is not a member of object Foo - did you mean Foo.Boo?
| value Booo is not a member of object Foo - did you mean Foo.Boo?
2 changes: 1 addition & 1 deletion tests/neg/i16653.check
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
-- [E006] Not Found Error: tests/neg/i16653.scala:1:7 ------------------------------------------------------------------
1 |import demo.implicits._ // error
| ^^^^
| Not found: demo
| Not found: demo - did you mean Demo?
|
| longer explanation available when compiling with `-explain`
Loading
Loading