Skip to content

Fix #8763: Limit number of import suggestions #8773

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 24, 2020
Merged

Fix #8763: Limit number of import suggestions #8773

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4745d99
Fix #8763: Limit number of import suggestions
odersky Apr 22, 2020
24856df
Avoid recomputing the same context
smarter Apr 24, 2020
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
57 changes: 48 additions & 9 deletions compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ import collection.mutable
trait ImportSuggestions:
this: Typer =>

/** The maximal number of suggested imports to make */
inline val MaxSuggestions = 10

import tpd._

/** Timeout to test a single implicit value as a suggestion, in ms */
Expand Down Expand Up @@ -249,6 +252,46 @@ trait ImportSuggestions:
finally timer.cancel()
end importSuggestions

/** The `ref` parts of this list of pairs, discarding subsequent elements that
* have the same String part. Elements are sorted by their String parts.
*/
def (refs: List[(TermRef, String)]).distinctRefs(using Context): List[TermRef] = refs match
case (ref, str) :: refs1 =>
ref :: refs1.dropWhile(_._2 == str).distinctRefs
case Nil =>
Nil

/** The best `n` references in `refs`, according to `compare`
* `compare` is a partial order. If there's a tie, we take elements
* in the order thy appear in the list.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small typo: thy should probably be they.

*/
def (refs: List[TermRef]).best(n: Int)(using Context): List[TermRef] =
val top = new Array[TermRef](n)
var filled = 0
val rest = new mutable.ListBuffer[TermRef]
val noImplicitsCtx = ctx.retractMode(Mode.ImplicitsEnabled)
for ref <- refs do
var i = 0
var diff = 0
while i < filled && diff == 0 do
diff = compare(ref, top(i))(using noImplicitsCtx)
if diff > 0 then
rest += top(i)
top(i) = ref
i += 1
end while
if diff == 0 && filled < n then
top(filled) = ref
filled += 1
else if diff <= 0 then
rest += ref
end for
val remaining =
if filled < n && rest.nonEmpty then rest.toList.best(n - filled)
else Nil
top.take(filled).toList ++ remaining
end best

/** An addendum to an error message where the error might be fixed
* by some implicit value of type `pt` that is however not found.
* The addendum suggests given imports that might fix the problem.
Expand All @@ -265,15 +308,11 @@ trait ImportSuggestions:
s" import ${ctx.printer.toTextRef(ref).show}"
val suggestions = suggestedRefs
.zip(suggestedRefs.map(importString))
.filter((ref, str) => str.contains('.'))
.sortWith { (x, y) =>
// sort by specificity first, alphabetically second
val ((ref1, str1), (ref2, str2)) = (x, y)
val diff = compare(ref1, ref2)
diff > 0 || diff == 0 && str1 < str2
}
.map((ref, str) => str)
.distinct // TermRefs might be different but generate the same strings
.filter((ref, str) => str.contains('.')) // must be a real import with `.`
.sortBy(_._2) // sort first alphabetically for stability
.distinctRefs // TermRefs might be different but generate the same strings
.best(MaxSuggestions) // take MaxSuggestions best references according to specificity
.map(importString)
if suggestions.isEmpty then ""
else
val fix =
Expand Down