From 8553bfcc2dc17073b49b3fbed82fa1b7079abcc7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Fri, 3 May 2024 17:40:12 +0200
Subject: [PATCH] Move the isAccessible test up to registerSym, instead of
 isInImport.

That test does not rely on any information dependent on the import
selectors. It only relies on information at the use site. Eagerly
checking it means we do not put as many symbols into the
`usedInScope` set, which is good because it is one of the
complexity factors of the unused-imports analysis.
---
 .../tools/dotc/transform/CheckUnused.scala    | 23 ++++++++++---------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
index 84bf705905b1..bd4ef73d6eea 100644
--- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
+++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -367,7 +367,7 @@ object CheckUnused:
      *
      * See the `isAccessibleAsIdent` extension method below in the file
      */
-    private val usedInScope = MutStack(MutSet[(Symbol,Boolean, Option[Name], Boolean)]())
+    private val usedInScope = MutStack(MutSet[(Symbol, Option[Name], Boolean)]())
     private val usedInPosition = MutMap.empty[Name, MutSet[Symbol]]
     /* unused import collected during traversal */
     private val unusedImport = MutList.empty[ImportSelectorData]
@@ -415,12 +415,16 @@ object CheckUnused:
         if sym.isConstructor then
           registerUsed(sym.owner, None, includeForImport) // constructor are "implicitly" imported with the class
         else
-          val accessibleAsIdent = sym.isAccessibleAsIdent
+          // If the symbol is accessible in this scope without an import, do not register it for unused import analysis
+          val includeForImport1 =
+            includeForImport
+              && (name.exists(_.toTermName != sym.name.toTermName) || !sym.isAccessibleAsIdent)
+
           def addIfExists(sym: Symbol): Unit =
             if sym.exists then
               usedDef += sym
-              if includeForImport then
-                usedInScope.top += ((sym, accessibleAsIdent, name, isDerived))
+              if includeForImport1 then
+                usedInScope.top += ((sym, name, isDerived))
           addIfExists(sym)
           addIfExists(sym.companionModule)
           addIfExists(sym.companionClass)
@@ -504,9 +508,9 @@ object CheckUnused:
       val selDatas = impInScope.pop()
 
       for usedInfo <- usedInfos do
-        val (sym, isAccessible, optName, isDerived) = usedInfo
+        val (sym, optName, isDerived) = usedInfo
         val usedData = selDatas.find { selData =>
-          sym.isInImport(selData, isAccessible, optName, isDerived)
+          sym.isInImport(selData, optName, isDerived)
         }
         usedData match
           case Some(data) =>
@@ -700,15 +704,12 @@ object CheckUnused:
         }
 
       /** Given an import and accessibility, return selector that matches import<->symbol */
-      private def isInImport(selData: ImportSelectorData, isAccessible: Boolean, altName: Option[Name], isDerived: Boolean)(using Context): Boolean =
+      private def isInImport(selData: ImportSelectorData, altName: Option[Name], isDerived: Boolean)(using Context): Boolean =
         assert(sym.exists)
 
         val selector = selData.selector
 
-        if isAccessible && !altName.exists(_.toTermName != sym.name.toTermName) then
-          // Even if this import matches, it is pointless because the symbol would be accessible anyway
-          false
-        else if !selector.isWildcard then
+        if !selector.isWildcard then
           if altName.exists(explicitName => selector.rename != explicitName.toTermName) then
             // if there is an explicit name, it must match
             false