From 9de2f64c4806e8a10c44079ef536c3a7bc383494 Mon Sep 17 00:00:00 2001
From: Som Snytt <som.snytt@gmail.com>
Date: Tue, 21 Jan 2025 20:01:06 -0800
Subject: [PATCH] Restrict import suggestions or mention accessibility

---
 .../tools/dotc/typer/ImportSuggestions.scala  | 34 ++++++++++++++-----
 tests/neg/i22429.check                        | 18 ++++++++++
 tests/neg/i22429.scala                        | 11 ++++++
 3 files changed, 55 insertions(+), 8 deletions(-)
 create mode 100644 tests/neg/i22429.check
 create mode 100644 tests/neg/i22429.scala

diff --git a/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala b/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala
index 3ae533d58b2e..02939bf014f5 100644
--- a/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala
+++ b/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala
@@ -12,7 +12,7 @@ import config.Printers.{implicits, implicitsDetailed}
 import ast.{untpd, tpd}
 import Implicits.{hasExtMethod, Candidate}
 import java.util.{Timer, TimerTask}
-import collection.mutable
+import collection.mutable, mutable.ListBuffer
 import scala.util.control.NonFatal
 import cc.isCaptureChecking
 
@@ -57,7 +57,7 @@ trait ImportSuggestions:
    *     skipped as an optimization, since they won't contain implicits anyway.
    */
   private def suggestionRoots(using Context) =
-    val seen = mutable.Set[TermRef]()
+    val seen = mutable.Set.empty[TermRef]
 
     def lookInside(root: Symbol)(using Context): Boolean =
       explore {
@@ -70,7 +70,7 @@ trait ImportSuggestions:
       }
 
     def nestedRoots(site: Type)(using Context): List[Symbol] =
-      val seenNames = mutable.Set[Name]()
+      val seenNames = mutable.Set.empty[Name]
       site.baseClasses.flatMap { bc =>
         bc.info.decls.filter { dcl =>
           lookInside(dcl)
@@ -275,7 +275,7 @@ trait ImportSuggestions:
    *  have the same String part. Elements are sorted by their String parts.
    */
   extension (refs: List[(TermRef, String)]) def distinctRefs(using Context): List[TermRef] =
-    val buf = new mutable.ListBuffer[TermRef]
+    val buf = ListBuffer.empty[TermRef]
     var last = ""
     for (ref, str) <- refs do
       if last != str then
@@ -290,7 +290,7 @@ trait ImportSuggestions:
   extension (refs: List[TermRef]) def best(n: Int)(using Context): List[TermRef] =
     val top = new Array[TermRef](n)
     var filled = 0
-    val rest = new mutable.ListBuffer[TermRef]
+    val rest = ListBuffer.empty[TermRef]
     val noImplicitsCtx = ctx.retractMode(Mode.ImplicitsEnabled)
     for ref <- refs do
       var i = 0
@@ -335,15 +335,33 @@ trait ImportSuggestions:
         else
           ctx.printer.toTextRef(ref).show
       s"  import $imported"
-    val suggestions = suggestedRefs
+    def indubitably(ref: TermRef): Boolean =
+      ref.symbol.isAccessibleFrom(ctx.owner.info)
+    val (suggested, dubious) = suggestedRefs
       .zip(suggestedRefs.map(importString))
       .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 ""
+      .partition(indubitably)
+    if suggested.isEmpty then
+      if dubious.isEmpty then ""
+      else
+        def isImportable(ref: TermRef): Boolean =
+          ctx.outersIterator.exists(outer => outer.isImportContext && !outer.importInfo.nn.isRootImport
+          && outer.importInfo.nn.site =:= ref.prefix
+          && outer.importInfo.nn.selectors.exists(sel => sel.isWildcard || sel.name == ref.name))
+        def dubiousAdvice(ref: TermRef): String =
+          s"${importString(ref)}${if isImportable(ref) then " // existing imported member is not accessible" else ""}"
+        i"""
+           |
+           |Consider making one of the following imports accessible to $help:
+           |
+           |${dubious.map(dubiousAdvice)}%\n%
+           |
+           |"""
     else
+      val suggestions = suggested.map(importString)
       val fix =
         if suggestions.tail.isEmpty then "The following import"
         else "One of the following imports"
diff --git a/tests/neg/i22429.check b/tests/neg/i22429.check
new file mode 100644
index 000000000000..d4951457ee0e
--- /dev/null
+++ b/tests/neg/i22429.check
@@ -0,0 +1,18 @@
+-- [E008] Not Found Error: tests/neg/i22429.scala:9:16 -----------------------------------------------------------------
+9 |  val a = "abc".testExt // error
+  |          ^^^^^^^^^^^^^
+  |          value testExt is not a member of String, but could be made available as an extension method.
+  |
+  |          Consider making one of the following imports accessible to fix:
+  |
+  |            import Extensions.testExt // existing imported member is not accessible
+  |
+-- [E008] Not Found Error: tests/neg/i22429.scala:10:13 ----------------------------------------------------------------
+10 |  val b = 42.xs // error
+   |          ^^^^^
+   |          value xs is not a member of Int, but could be made available as an extension method.
+   |
+   |          Consider making one of the following imports accessible to fix:
+   |
+   |            import Extensions.xs
+   |
diff --git a/tests/neg/i22429.scala b/tests/neg/i22429.scala
new file mode 100644
index 000000000000..acfc0ce805d0
--- /dev/null
+++ b/tests/neg/i22429.scala
@@ -0,0 +1,11 @@
+
+import Extensions.testExt
+
+object Extensions:
+  extension (s: String) private def testExt = s.reverse
+  extension (n: Int) private def xs = "x" * n
+
+@main def test() = println:
+  val a = "abc".testExt // error
+  val b = 42.xs // error
+  a + b