Usability: compiler suggests possible names in NotAMemberError #6711

eed3si9n · 2018-06-03T05:28:29Z

With this change, NotAMemberError checks for possible members under target with edit distance of 2 or 1. To avoid false positives, candidates with size <2 (eq, ne, _1 etc) are filtered out.

scala> import scala.io.StdIn.{readline, readInt}
              ^
       error: value readline is not a member of object scala.io.StdIn
       did you mean readLine?

scala> import scala.io.stdin.{readLine => line}
                       ^
       error: object stdin is not a member of package io
       did you mean StdIn?

scala> import scala.sth
              ^
       error: object sth is not a member of package scala
       did you mean sys or math?

note

See also -Ysuggest-idents (3e9e4ec), which got axed apparently for possible "performance impact". Unlike that, this is a bit more constrained because I am just looking up the members of target, I think.

olofwalker · 2018-06-03T08:23:17Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

+              case _ =>
+                val threshold = 2
+                val x = name.toString
+                val names = target.nonPrivateMembers.toList.map(_.name.toString).distinct


.toVector instead ? I do not know the size of nonPrivateMembers but it sounds significant and a Vector would perform better.

target.nonPrivateMembers returns Scope containing method names and type names available under some package or object so the size would be around 20 most of the times? Scope implements only .toList, so if we were to use Vector I'd need to call .toList.toVector.

Different codebase, still scopes.. 😀

som-snytt · 2018-06-03T14:13:35Z

This is great. I feel like I'm in one of those dystopian novels where the heroes go back in time to fix things, and now there is peace, harmony, and income equality.

I looked at t3118 just for fun. Suggesting x looked suspicious because x used to leak from Predef implicits. It's suggesting member x instead of case class C in x.C() or new x.C(), where the suggestion makes no possible sense.

Ideally, we'd advise only solutions that type check, but maybe there are some heuristics to apply, such as does it have an apply method if the wrong expr is an apply. Similarly for a new thing.

Even so, better selection is a win. "We have a fine selection of selections that may fulfill your needs. May I suggest x, or perhaps y or z?" Under -Yobsequious-messages or -Xinveigle.

eed3si9n · 2018-06-03T19:08:23Z

This is great. I feel like I'm in one of those dystopian novels where the heroes go back in time to fix things, and now there is peace, harmony, and income equality.

Thank you :)

I looked at t3118 just for fun. Suggesting x looked suspicious because x used to leak from Predef implicits. It's suggesting member x instead of case class C in x.C() or new x.C(), where the suggestion makes no possible sense.

In general, all 1 letter suggestions are shots in the dark, since they can be reached from any other 1 letter input with 2 edit distance. I probably should ignore them for now.

Ideally, we'd advise only solutions that type check, but maybe there are some heuristics to apply, such as does it have an apply method if the wrong expr is an apply.

That would be amazing, but I don't have enough charges left on my portal gun.

som-snytt · 2018-06-03T20:37:20Z

I was going to joke that 1-letter alternates should be within a certain distance on the keyboard, but then you'd have to know which keyboard. -Ykb:dvorak. I agree on saving ammo; wait until someone notices.

lrytz · 2018-06-04T15:33:44Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

+              val x = name.toString
+              if (x.size < 2) Vector()
+              else {
+                val names = target.nonPrivateMembers.toList.toVector.map(_.name.toString).distinct


nonPrivateMembers.iterator.filterNot(m => m.isConstructor).map(_.name.decode).filter(_.length > 2).distinct.toList

This does it in a single traversal. Using the decoded name to avoid the issue below:

scala> object C { def <= = 0 } defined object C warning: previously defined class C is not a companion to object C. Companions must be defined together; you may wish to use :paste mode for this. scala> C.lesseq ^ error: value lesseq is not a member of object C did you mean $less$eq?

I think filtering short names also here makes sense. Doing it after decoding removes operators like <=.

I think it still includes too much, for example

scala> case class C(x: Int) scala> typeOf[C].nonPrivateMembers.iterator.filterNot(m => m.isConstructor).map(_.name.decode).filter(_.length > 2).distinct.toVector res32: scala.collection.immutable.Vector[String] = Vector(equals, toString, hashCode, canEqual, productIterator, productElement, productArity, productPrefix, copy$default$1, copy, $asInstanceOf, $isInstanceOf, synchronized, finalize, wait, notifyAll, notify, clone, getClass, asInstanceOf, isInstanceOf)

We should exclude copy$default$1, $asInstanceOf, $isInstanceOf. But filtering all of m.isSynthetic is too much

scala> typeOf[C].nonPrivateMembers.iterator.filterNot(m => m.isSynthetic || m.isConstructor).map(_.name.decode).filter(_.length > 2).distinct.toVector res33: scala.collection.immutable.Vector[String] = Vector(synchronized, finalize, wait, notifyAll, notify, clone, getClass, asInstanceOf, isInstanceOf)

We lose copy, hashCode, ...

Maybe we can do it by character (.contains("$"))? Since we're using decoded names, $ should only show up in names that are not supposed to be used by the user.

You could also delay the toList until after the filters below

lrytz

a few more comments

lrytz · 2018-06-04T19:41:11Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

          val owner            = qual.tpe.typeSymbol
          val target           = qual.tpe.widen
          def targetKindString = if (owner.isTypeParameterOrSkolem) "type parameter " else ""
          def nameString       = decodeWithKind(name, owner)
          /* Illuminating some common situations and errors a bit further. */
          def addendum         = {
-            val companion = {
+            val companionSymbol: Option[Symbol] = {


We usually use NoSymbol instead of None: Option[Symbol]. Then we can also remove the pattern match below.

lrytz · 2018-06-04T19:43:30Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

+            // find out all the names available under target within 2 edit distances
+            lazy val alternatives: Vector[String] = {
+              val editThreshold = 2
+              val x = name.toString


should be name.decode

lrytz · 2018-06-04T19:48:59Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

+              val x = name.toString
+              if (x.size < 2) Vector()
+              else {
+                val names = target.nonPrivateMembers.toList.toVector.map(_.name.toString).distinct


You could also delay the toList until after the filters below

lrytz · 2018-06-04T19:58:30Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

+              else {
+                val names = target.nonPrivateMembers.toList.toVector.map(_.name.toString).distinct
+                names filter { n =>
+                  EditDistance.levenshtein(n, x) <= editThreshold


We should do some pre-filtering, only compute the distance if math.abs(n.length - x.length) <= 2.

lrytz · 2018-06-04T19:59:10Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

+                names filter { n =>
+                  EditDistance.levenshtein(n, x) <= editThreshold
+                } filterNot { s =>
+                  (s == x) || EditDistance.excludeMethods(s)


This is cheaper than computing the distance, so should be done first

retronym · 2018-06-05T08:49:36Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

          val owner            = qual.tpe.typeSymbol
          val target           = qual.tpe.widen
          def targetKindString = if (owner.isTypeParameterOrSkolem) "type parameter " else ""
          def nameString       = decodeWithKind(name, owner)
          /* Illuminating some common situations and errors a bit further. */
          def addendum         = {
+            val companionSymbol: Symbol = {


Is this code ever/often called in type errors generated during failed branches of implicit search? If so, we should disable the (expensive) addendum creation. We recently did something similar

I am not sure if it's called in the implicit search.
Inside typedSelect (

scala/src/compiler/scala/tools/nsc/typechecker/Typers.scala

Line 4969 in 2b49d71

def typedSelect(tree: Tree, qual: Tree, name: Name): Tree = {

), val qual1 = adaptToMemberWithArgs(tree, qual, name, mode) happens at line 4988, which I am guessing is where implicit search is done.
At line 5036, typedSelect calls lookupInQualifier, which can call NotAMemberError. Could that mean that we're ok?

An experiment I did was to add sys.error(..) in alternatives below, and then added

object Suggest { implicit class RichInt(val i: Int) { def flippity: Unit = () } 1.flippity }

to pos/implicits-new.scala test, and it ran without blowing up.

Could run all of partest --pos and -partest --run with that patch? If that goes well, I think we don't need to add any guards.

@lrytz Good call. I am seeing a bunch of errors, but they do not look like implicit searches. Most of the failures look like spec-arrays-pos.log:

error: Int $plus$eq List()

likely caused by something like i += 1.

Only other kind that I noticed that doesn't involve $eq is t8013-pos.log etc:

error: t8013.Perverse.Impervolator pImpl List()

where pImpl shows up as a macro implementation:

def p(args: Any*): String = macro pImpl

These occur seldom enough, so I think they are ok, but I'm going to put a guard for x.endsWith("=").

lrytz · 2018-06-06T11:34:38Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

+            // find out all the names available under target within 2 edit distances
+            lazy val alternatives: List[String] = {
+              val editThreshold = 2
+              val x = name.toString


name.decode

retronym · 2018-06-07T00:28:56Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

+              else
+                alternatives match {
+                  case Nil => ""
+                  case xs  => "\ndid you mean " + StringUtil.oxford(xs, "or") + "?"


We should abbreviate this list if xs.length is unreasonably large.

retronym · 2018-06-07T00:29:56Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

+                    (n != x) &&
+                    !n.contains("$") &&
+                    EditDistance.levenshtein(n, x) <= editThreshold)
+                  .distinct.toList


We should probably only suggest term/type names depending on whether name is a term/type name.

retronym · 2018-06-07T00:36:24Z

This reminds me of a solution I submitted to Ruby Quiz way back in 2006

In deference to the style of the solutions I've read since discovering
Ruby Quiz recently my solution includes obligatory (if gratuitous) use
of (method|const)_missing :)
...
ptus gtt_xeet.cllocet {|t| TexMtunegr::mgnue_txet(t)}.jion("\n" * 2)

👴

retronym · 2018-06-07T01:02:54Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

+              val x = name.toString
+              if (x.size < 2) Nil
+              else {
+                target.nonPrivateMembers.iterator


It would be nice to look at all members and filter by those that are accessible at the callsite's Context. We should them be careful to exclude fields:

.filterNot(m => nme.isLocalName(m.name)))

Added 1e8ec90 to address the accessibility.

lrytz · 2018-06-08T08:56:27Z

Some test .check files need updating https://scala-ci.typesafe.com/job/scala-2.13.x-validate-main/2374/#showFailuresLink

eed3si9n · 2018-06-08T21:38:32Z

@lrytz

Some test .check files need updating https://scala-ci.typesafe.com/job/scala-2.13.x-validate-main/2374/#showFailuresLink

Yes. I've been only doing partest --update-check --neg, but it seems like we've picked up a few suggestions from run.

scala> 1 -> 2
         ^
       error: value -> is not a member of Int
       did you mean >>>?

lrytz

This looks good to me otherwise!

som-snytt · 2018-06-12T21:52:04Z

Is it deemed not useful to provide at least as many suggestions as completions?

scala> import scala.io.StdIn.{readline, readInt}
              ^
       error: value readline is not a member of object scala.io.StdIn
       did you mean readLine?

scala> 42.isShole
          ^
       error: value isShole is not a member of Int

scala> 42.isWhole
res1: Boolean = true

scala> 42.is
isInfinite     isNaN           isValidByte   isValidLong       
isInfinity     isNegInfinity   isValidChar   isValidShort      
isInstanceOf   isPosInfinity   isValidInt    isWhole

Folks working in Java are likely to do this:

scala> "".toShortOptional
          ^
       error: value toShortOptional is not a member of String

scala> class C { def toIntOption = Option(42) }
defined class C

scala> val c = new C
c: C = C@ddffa6c

scala> c.toIntOptional
         ^
       error: value toIntOptional is not a member of C
       did you mean toIntOption?

Yes! That's exactly what I meant!

scala> List(1).unzip2
               ^
       error: value unzip2 is not a member of List[Int]
       did you mean unzip or unzip3?

scala> List(1,2,3) reverse_:: List(4,5,6)
                   ^
       error: value reverse_:: is not a member of List[Int]
       did you mean reverse_:::?

scala> List(1).toSea
               ^
       error: value toSea is not a member of List[Int]
       did you mean toSeq or toSet?

List to sea means your ship is in trouble.

This is the one I wanted to test:

scala> new C().asshole
               ^
       error: value asshole is not a member of C
       did you mean isWhole?

eed3si9n · 2018-07-03T01:19:57Z

Is it deemed not useful to provide at least as many suggestions as completions?

Wouldn't that require presentation compiler etc? Maybe we could work on that in another round of PR if that's feasible. I'd like to see this PR land on 2.13 if possible.

adriaanm · 2018-08-06T16:14:37Z

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

+            lazy val alternatives: List[String] = {
+              val editThreshold = 2
+              val x = name.decode
+              if ((x.size < 2) || x.endsWith("=")) Nil


@retronym, I assume you meant adding context.openImplicits.nonEmpty to the if condition, so that we don't suggest alternatives when an implicit search is underway? With that, does this LGTY?

Added 96e590d.

lrytz · 2018-11-02T09:11:25Z

Rebased on 2.13.x to make sure there are no semantic merge conflicts. I'll merge this once it goes green.

Fixes scala/bug#10181 With this change, NotAMemberError checks for possible members under target with edit distance of 2 or 1. ```scala scala> import scala.io.StdIn.{readline, readInt} ^ error: value readline is not a member of object scala.io.StdIn did you mean readLine? scala> import scala.io.stdin.{readLine => line} ^ error: object stdin is not a member of package io did you mean StdIn? scala> import scala.sth ^ error: object sth is not a member of package scala did you mean sys or math? ```

lrytz · 2018-11-02T09:35:06Z

Added file headers to the new files, and squashed.

lrytz · 2018-11-02T10:15:25Z

🎉 thank you, @eed3si9n!

SethTisue · 2018-11-02T10:18:55Z

this is very, very cool

eed3si9n · 2018-11-02T16:26:57Z

I am happy that this landed in time for 2.13.

smarter · 2018-11-20T22:05:04Z

FWIW, dotty has had an implementation of something like this for a couple of years: https://github.com/lampepfl/dotty/blob/614265d779cd601de10edd23eea93598cd824c21//compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala#L310-L364 It'd be good to synchronize the two implementations.

scala-jenkins added this to the 2.13.0-M5 milestone Jun 3, 2018

eed3si9n mentioned this pull request Jun 3, 2018

Am I missing something? More error help. scala/bug#10181

Closed

olofwalker reviewed Jun 3, 2018

View reviewed changes

lrytz reviewed Jun 4, 2018

View reviewed changes

retronym requested changes Jun 5, 2018

View reviewed changes

lrytz reviewed Jun 6, 2018

View reviewed changes

retronym reviewed Jun 7, 2018

View reviewed changes

eed3si9n force-pushed the wip/editdistance branch from 48b64f6 to ddf17e5 Compare June 9, 2018 01:07

lrytz reviewed Jun 11, 2018

View reviewed changes

eed3si9n force-pushed the wip/editdistance branch from 5ab5944 to 08b27c3 Compare June 12, 2018 07:28

adriaanm reviewed Aug 6, 2018

View reviewed changes

SethTisue modified the milestones: 2.13.0-M5, 2.13.0-RC1 Aug 7, 2018

adriaanm modified the milestones: 2.13.0-RC1, 2.13.x Aug 7, 2018

lrytz force-pushed the wip/editdistance branch from 96e590d to 0f303bb Compare November 2, 2018 09:10

lrytz force-pushed the wip/editdistance branch from 0f303bb to 1db387a Compare November 2, 2018 09:34

lrytz merged commit 3b28fb7 into scala:2.13.x Nov 2, 2018

SethTisue modified the milestones: 2.13.1, 2.13.0-RC1 Nov 2, 2018

SethTisue added the release-notes worth highlighting in next release notes label Nov 2, 2018

eed3si9n deleted the wip/editdistance branch November 2, 2018 16:26

smarter mentioned this pull request Nov 20, 2018

Fix #5460: Improve completion of import nodes scala/scala3#5476

Merged

Duhemm mentioned this pull request Nov 23, 2018

IDE: Consider fuzzy-search in completion scala/scala3#5507

Closed

SethTisue changed the title ~~Suggest possible names in NotAMemberError~~ Usability: compiler suggests possible names in NotAMemberError Apr 4, 2019

eed3si9n mentioned this pull request Aug 21, 2019

Help with typos in qualifiers scala/bug#11699

Open

Usability: compiler suggests possible names in NotAMemberError #6711

Usability: compiler suggests possible names in NotAMemberError #6711

Conversation

eed3si9n commented Jun 3, 2018 • edited

note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

som-snytt commented Jun 3, 2018

eed3si9n commented Jun 3, 2018

som-snytt commented Jun 3, 2018

lrytz Jun 4, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrytz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

retronym commented Jun 7, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrytz commented Jun 8, 2018

eed3si9n commented Jun 8, 2018

lrytz left a comment

Choose a reason for hiding this comment

som-snytt commented Jun 12, 2018

eed3si9n commented Jul 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrytz commented Nov 2, 2018

lrytz commented Nov 2, 2018

lrytz commented Nov 2, 2018

SethTisue commented Nov 2, 2018

eed3si9n commented Nov 2, 2018

smarter commented Nov 20, 2018

eed3si9n commented Jun 3, 2018 •

edited

lrytz Jun 4, 2018 •

edited

retronym commented Jun 7, 2018 •

edited