Skip to content

Commit

Permalink
Backport "Give "did you mean ...?" hints also for simple identifiers"…
Browse files Browse the repository at this point in the history
… to LTS (#20742)

Backports #18747 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
  • Loading branch information
WojciechMazur committed Jun 23, 2024
2 parents 8f7b8a7 + c613076 commit 53fff81
Show file tree
Hide file tree
Showing 14 changed files with 295 additions and 68 deletions.
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/ast/untpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
val rename: TermName = renamed match
case Ident(rename: TermName) => rename
case _ => name

def isUnimport = rename == nme.WILDCARD
}

case class Number(digits: String, kind: NumberKind)(implicit @constructorOnly src: SourceFile) extends TermTree
Expand Down
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
90 changes: 36 additions & 54 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
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 @@ -1540,7 +1540,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 seen.contains(sel.name) then
report.error(ImportRenamedTwice(sel.imported), sel.imported.srcPos)
seen += sel.name
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 @@ -161,7 +161,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 @@ -173,7 +173,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 @@ -202,7 +202,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 @@ -656,7 +656,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 @@ -745,7 +745,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

0 comments on commit 53fff81

Please sign in to comment.