From 17526095ac9c06b557d56e89e9277410b773c645 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 5 Jan 2020 16:15:39 +0100 Subject: [PATCH 1/3] Refactor "did you mean" hinting - Handle all members instead of just decls. - Simplify the logic and implementation. To tell the truth, the previous implementation was too convoluted for me to be able to say what it does exactly. But the new one seems to fit the intuitive spec. --- .../dotc/reporting/diagnostic/messages.scala | 55 +++++++------------ 1 file changed, 21 insertions(+), 34 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 785c47769c05..be37496f15af 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -314,13 +314,19 @@ object messages { val msg: String = { import core.Flags._ - val maxDist = 3 - val decls = site.decls.toList - .filter(_.isType == name.isTypeName) - .flatMap { sym => - if (sym.flagsUNSAFE.isOneOf(Synthetic | PrivateLocal) || sym.isConstructor) Nil - else List((sym.name.show, sym)) - } + val maxDist = 3 // maximal number of differences to be considered for a hint + val missing = name.show + + // The names of all non-synthetic, non-private members of `site` + // that are of the same type/term kind as the missing member. + def candidates: Set[String] = + for + bc <- site.baseClasses.toSet + sym <- bc.info.decls.filter(sym => + sym.isType == name.isTypeName + && !sym.isConstructor + && !sym.flagsUNSAFE.isOneOf(Synthetic | Private)) + yield sym.name.show // Calculate Levenshtein distance def distance(n1: Iterable[?], n2: Iterable[?]) = @@ -333,37 +339,18 @@ object messages { } }.last - // Count number of wrong characters - def incorrectChars(x: (String, Int, Symbol)): (String, Symbol, Int) = { - val (currName, _, sym) = x - val matching = name.show.zip(currName).foldLeft(0) { - case (acc, (x,y)) => if (x != y) acc + 1 else acc - } - (currName, sym, matching) - } - - // Get closest match in `site` - def closest: List[String] = - decls - .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) - .map(_._1) - // [Martin] Note: I have no idea what this does. This shows the - // pitfalls of not naming things, functional or not. + // A list of possible candidate strings with their Levenstein distances + // to the name of the missing member + def closest: List[(Int, String)] = candidates + .toList + .map(n => (distance(n.show, missing), n)) + .filter((d, n) => d <= maxDist && d < missing.length & d < n.length) + .sorted // sort by distance first, alphabetically second val finalAddendum = if addendum.nonEmpty then addendum else closest match { - case n :: _ => + case (d, n) :: _ => val siteName = site match case site: NamedType => site.name.show case site => i"$site" From 2c97437b2fa9c2e120245507bf6d606284173ab9 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 5 Jan 2020 18:30:30 +0100 Subject: [PATCH 2/3] Fix Levenshtein distance calculation The previous algorithm was simply wrong, which went undiscovered because there were no tests to its correctness. Morale: Just because some code is purely functional does not mean it is clear or correct. --- .../dotc/reporting/diagnostic/messages.scala | 23 +++--- tests/neg/name-hints.check | 72 +++++++++++++++++++ tests/neg/name-hints.scala | 26 +++++++ 3 files changed, 111 insertions(+), 10 deletions(-) create mode 100644 tests/neg/name-hints.check create mode 100644 tests/neg/name-hints.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index be37496f15af..f7cb1270cf20 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -321,7 +321,7 @@ object messages { // that are of the same type/term kind as the missing member. def candidates: Set[String] = for - bc <- site.baseClasses.toSet + bc <- site.widen.baseClasses.toSet sym <- bc.info.decls.filter(sym => sym.isType == name.isTypeName && !sym.isConstructor @@ -329,15 +329,18 @@ object messages { yield sym.name.show // Calculate Levenshtein distance - def distance(n1: Iterable[?], n2: Iterable[?]) = - n1.foldLeft(List.range(0, n2.size)) { (prev, x) => - (prev zip prev.tail zip n2).scanLeft(prev.head + 1) { - case (h, ((d, v), y)) => math.min( - math.min(h + 1, v + 1), - if (x == y) d else d + 1 - ) - } - }.last + 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 strings with their Levenstein distances // to the name of the missing member diff --git a/tests/neg/name-hints.check b/tests/neg/name-hints.check new file mode 100644 index 000000000000..8fb70a5526f8 --- /dev/null +++ b/tests/neg/name-hints.check @@ -0,0 +1,72 @@ +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:6:15 ------------------------------------------------------ +6 | val x1 = Int.maxvalue // error + | ^^^^^^^^^^^^ + | value maxvalue is not a member of object Int - did you mean Int.MaxValue? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:7:15 ------------------------------------------------------ +7 | val x2 = Int.MxValue // error + | ^^^^^^^^^^^ + | value MxValue is not a member of object Int - did you mean Int.MaxValue? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:8:15 ------------------------------------------------------ +8 | val x3 = Int.MaxxValue // error + | ^^^^^^^^^^^^^ + | value MaxxValue is not a member of object Int - did you mean Int.MaxValue? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:10:13 ----------------------------------------------------- +10 | val d1 = O.abcd // error + | ^^^^^^ + | value abcd is not a member of object O - did you mean O.abcde? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:11:13 ----------------------------------------------------- +11 | val d2 = O.abc // error + | ^^^^^ + | value abc is not a member of object O - did you mean O.abcde? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:12:13 ----------------------------------------------------- +12 | val d3 = O.ab // error, no hint since distance = 3 > 2 = length + | ^^^^ + | value ab is not a member of object O +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:13:13 ----------------------------------------------------- +13 | val s1 = O.Abcde // error + | ^^^^^^^ + | value Abcde is not a member of object O - did you mean O.abcde? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:14:13 ----------------------------------------------------- +14 | val s3 = O.AbCde // error + | ^^^^^^^ + | value AbCde is not a member of object O - did you mean O.abcde? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:15:13 ----------------------------------------------------- +15 | val s3 = O.AbCdE // error + | ^^^^^^^ + | value AbCdE is not a member of object O - did you mean O.abcde? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:16:13 ----------------------------------------------------- +16 | val s3 = O.AbCDE // error, no hint + | ^^^^^^^ + | value AbCDE is not a member of object O +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:17:13 ----------------------------------------------------- +17 | val a1 = O.abcde0 // error + | ^^^^^^^^ + | value abcde0 is not a member of object O - did you mean O.abcde? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:18:13 ----------------------------------------------------- +18 | val a2 = O.abcde00 // error + | ^^^^^^^^^ + | value abcde00 is not a member of object O - did you mean O.abcde? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:19:13 ----------------------------------------------------- +19 | val a3 = O.abcde000 // error + | ^^^^^^^^^^ + | value abcde000 is not a member of object O - did you mean O.abcde? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:20:13 ----------------------------------------------------- +20 | val a4 = O.abcde0000 // error, no hint + | ^^^^^^^^^^^ + | value abcde0000 is not a member of object O +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:22:13 ----------------------------------------------------- +22 | val y1 = O.x // error, no hint + | ^^^ + | value x is not a member of object O +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:23:13 ----------------------------------------------------- +23 | val y2 = O.xY // error + | ^^^^ + | value xY is not a member of object O - did you mean O.xy? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:24:13 ----------------------------------------------------- +24 | val y3 = O.xyz // error + | ^^^^^ + | value xyz is not a member of object O - did you mean O.xy? +-- [E008] Member Not Found Error: tests/neg/name-hints.scala:25:13 ----------------------------------------------------- +25 | val y2 = O.XY // error, no hint + | ^^^^ + | value XY is not a member of object O diff --git a/tests/neg/name-hints.scala b/tests/neg/name-hints.scala new file mode 100644 index 000000000000..aca20e067e0f --- /dev/null +++ b/tests/neg/name-hints.scala @@ -0,0 +1,26 @@ +object O with + val abcde: Int = 0 + val xy: Int = 1 + +object Test with + val x1 = Int.maxvalue // error + val x2 = Int.MxValue // error + val x3 = Int.MaxxValue // error + + val d1 = O.abcd // error + val d2 = O.abc // error + val d3 = O.ab // error, no hint since distance = 3 > 2 = length + val s1 = O.Abcde // error + val s3 = O.AbCde // error + val s3 = O.AbCdE // error + val s3 = O.AbCDE // error, no hint + val a1 = O.abcde0 // error + val a2 = O.abcde00 // error + val a3 = O.abcde000 // error + val a4 = O.abcde0000 // error, no hint + + val y1 = O.x // error, no hint + val y2 = O.xY // error + val y3 = O.xyz // error + val y2 = O.XY // error, no hint + From 63f87eea8c7c6c933180dca179e203a6f649f5e1 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 6 Jan 2020 17:32:42 +0100 Subject: [PATCH 3/3] Fix typo --- .../src/dotty/tools/dotc/reporting/diagnostic/messages.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index f7cb1270cf20..f1088b68b27b 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -347,7 +347,7 @@ object messages { def closest: List[(Int, String)] = candidates .toList .map(n => (distance(n.show, missing), n)) - .filter((d, n) => d <= maxDist && d < missing.length & d < n.length) + .filter((d, n) => d <= maxDist && d < missing.length && d < n.length) .sorted // sort by distance first, alphabetically second val finalAddendum =