Improvements to "did you mean ...?" scheme

1. Only show accessible members 2. Show several alternatives if they are at same distance. Unlike Scala 2, we do not show alternatives at larger distance. I fear that would produce more noise than signal. Fixes #17067 [Cherry-picked 5ce1ac9]
scala · Jun 23, 2024 · d9a9024 · d9a9024
1 parent b7e599a
commit d9a9024
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 14 deletions.
diff --git a/compiler/src/dotty/tools/dotc/reporting/DidYouMean.scala b/compiler/src/dotty/tools/dotc/reporting/DidYouMean.scala
@@ -126,21 +126,33 @@ object DidYouMean:
   def didYouMean(candidates: List[(Int, Binding)], proto: Type, prefix: String)(using Context): String =
 
     def qualifies(b: Binding)(using Context): Boolean =
-      proto match
-        case _: SelectionProto => true
-        case _ =>
-          try !b.sym.isNoValue
-          catch case ex: Exception => false
+      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
-          s" - did you mean $prefix${showName(b.name, b.sym)}?"
+          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 _ => ""

diff --git a/tests/neg/i18682.check b/tests/neg/i18682.check
@@ -16,17 +16,35 @@
   |        Not found: hellx - did you mean hello?
   |
   | longer explanation available when compiling with `-explain`
--- [E008] Not Found Error: tests/neg/i18682.scala:13:12 ----------------------------------------------------------------
-13 |val _ = bar.Bap // error, App does shown as hint, too far away
+-- [E008] Not Found Error: tests/neg/i18682.scala:16:12 ----------------------------------------------------------------
+16 |val _ = bar.Bap // error, App does not show as hint, too far away
    |        ^^^^^^^
    |        value Bap is not a member of object Bar
--- [E008] Not Found Error: tests/neg/i18682.scala:14:12 ----------------------------------------------------------------
-14 |val _ = bar.Bap() // error
+-- [E008] Not Found Error: tests/neg/i18682.scala:17:12 ----------------------------------------------------------------
+17 |val _ = bar.Bap() // error
    |        ^^^^^^^
    |        value Bap is not a member of object Bar - did you mean bar.Baz?
--- [E006] Not Found Error: tests/neg/i18682.scala:16:8 -----------------------------------------------------------------
-16 |val _ = error // error, java.lang.Error does not show as hint, since it is not a value
+-- [E006] Not Found Error: tests/neg/i18682.scala:19:8 -----------------------------------------------------------------
+19 |val _ = error // error, java.lang.Error does not show as hint, since it is not a value
    |        ^^^^^
    |        Not found: error
    |
    | longer explanation available when compiling with `-explain`
+-- [E008] Not Found Error: tests/neg/i18682.scala:22:50 ----------------------------------------------------------------
+22 |val _ = "123".view.reverse.padTo(5, '0').iterator.reverse // error, no hint since `reversed` is not accessible
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |        value reverse is not a member of Iterator[Char]
+-- [E006] Not Found Error: tests/neg/i18682.scala:27:8 -----------------------------------------------------------------
+27 |val _ = pool // error
+   |        ^^^^
+   |        Not found: pool - did you mean cool? or perhaps wool?
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E008] Not Found Error: tests/neg/i18682.scala:29:12 ----------------------------------------------------------------
+29 |val _ = bar.poodle // error
+   |        ^^^^^^^^^^
+   |        value poodle is not a member of object Bar - did you mean bar.pool?
+-- [E008] Not Found Error: tests/neg/i18682.scala:31:12 ----------------------------------------------------------------
+31 |val _ = bar.ool // error
+   |        ^^^^^^^
+   |        value ool is not a member of object Bar - did you mean bar.cool? or perhaps bar.pool or bar.wool?
diff --git a/tests/neg/i18682.scala b/tests/neg/i18682.scala
@@ -8,9 +8,24 @@ val _ = hellx // error
 object Bar:
   class Baz()
   object App
+  def cool = 1
+  def wool = 2
+  def pool = 3
 
 val bar = Bar
-val _ = bar.Bap // error, App does shown as hint, too far away
+val _ = bar.Bap // error, App does not show as hint, too far away
 val _ = bar.Bap() // error
 
-val _ = error // error, java.lang.Error does not show as hint, since it is not a value
+val _ = error // error, java.lang.Error does not show as hint, since it is not a value
+
+// #17067
+val _ = "123".view.reverse.padTo(5, '0').iterator.reverse // error, no hint since `reversed` is not accessible
+
+val cool = "cool"
+val wool = "wool"
+
+val _ = pool // error
+
+val _ = bar.poodle // error
+
+val _ = bar.ool // error