fix: filter symbols for conflict check are in scope before inline

scalameta · Dec 4, 2024 · 632c510 · 632c510
1 parent 9194e21
commit 632c510
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 35 deletions.
diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/PcInlineValueProviderImpl.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/PcInlineValueProviderImpl.scala
@@ -65,37 +65,29 @@ final class PcInlineValueProviderImpl(
     }
   }
 
-  private def symbolsUsedInDef(
-      symbol: Symbol,
-      rhs: Tree
-  ): List[Symbol] = {
-    val classParents =
-      if (symbol != null) symbol.ownersIterator.filter(_.isType).toSet
-      else Set.empty
-
+  private def symbolsUsedInDef(rhs: Tree): List[Symbol] = {
     def validateSymbol(symbol: Symbol) =
-      !symbol.isSynthetic && !symbol.isImplicit &&
-        (!symbol.safeOwner.isType || symbol.safeOwner.isModuleClass || classParents
-          .exists(_ == symbol.safeOwner))
+      !symbol.isSynthetic && !symbol.isImplicit
 
     @tailrec
     def collectNames(
-        symbols: List[Tree],
+        symbols: List[Symbol],
         toTraverse: List[Tree]
-    ): List[Tree] =
+    ): List[Symbol] =
       toTraverse match {
         case tree :: toTraverse => {
           val nextSymbols =
             tree match {
-              case id: Ident if validateSymbol(id.symbol) => id :: symbols
-              case s: Select if validateSymbol(s.symbol) => s :: symbols
+              case id: Ident if validateSymbol(id.symbol) =>
+                id.symbol :: symbols
+              case s: Select if validateSymbol(s.symbol) => s.symbol :: symbols
               case _ => symbols
             }
           collectNames(nextSymbols, toTraverse ++ tree.children)
         }
         case Nil => symbols
       }
-    collectNames(List(), List(rhs)).map(_.symbol)
+    collectNames(List(), List(rhs))
   }
 
   private def referenceEdits(
@@ -106,7 +98,10 @@ final class PcInlineValueProviderImpl(
       .drop(1)
       .exists(e => e.isTerm)
     val allreferences = allOccurences.filterNot(_.isDefn)
-    val symbols = symbolsUsedInDef(definition.tree.symbol, definition.tree.rhs)
+    val importContext = doLocateContext(definition.tree.rhs.pos)
+    val symbols = symbolsUsedInDef(definition.tree.rhs).distinct.filter { sym =>
+      importContext.symbolIsInScope(sym)
+    }
     def inlineAll() =
       makeRefs(allreferences, symbols).right
         .map((true, _))

diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/PcInlineValueProviderImpl.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/PcInlineValueProviderImpl.scala
@@ -1,6 +1,7 @@
 package scala.meta.internal.pc
 
 import scala.meta.internal.mtags.MtagsEnrichments.*
+import scala.meta.internal.pc.IndexedContext.Result
 import scala.meta.internal.pc.InlineValueProvider.Errors
 import scala.meta.pc.OffsetParams
 
@@ -43,7 +44,9 @@ final class PcInlineValueProviderImpl(
           DefinitionTree(defn, pos)
         }
         .toRight(Errors.didNotFindDefinition)
-      symbols = symbolsUsedInDefn(definition.tree.symbol, definition.tree.rhs)
+      path = Interactive.pathTo(unit.tpdTree, definition.tree.rhs.span)(using newctx)
+      indexedContext = IndexedContext(Interactive.contextOfPath(path)(using newctx))
+      symbols = symbolsUsedInDefn(definition.tree.rhs).filter(indexedContext.lookupSym(_) == Result.InScope)
       references <- getReferencesToInline(definition, allOccurences, symbols)
     yield
       val (deleteDefinition, refsEdits) = references
@@ -109,33 +112,25 @@ final class PcInlineValueProviderImpl(
   /** 
    * Return all scope symbols used in this
    */
-  private def symbolsUsedInDefn(
-      symbol: Symbol,
-      rhs: Tree
-  ): List[Symbol] =
-    val classParents =
-      if (symbol != null) symbol.ownersIterator.filter(_.isType).toSet
-      else Set.empty
-
+  private def symbolsUsedInDefn(rhs: Tree): Set[Symbol] =
     def collectNames(
-        symbols: List[Symbol],
+        symbols: Set[Symbol],
         tree: Tree,
-    ): List[Symbol] =
+    ): Set[Symbol] =
       tree match
         case id: (Ident | Select)
-            if !id.symbol.is(Synthetic) && !id.symbol.is(Implicit) &&
-            (!id.symbol.owner.isType || id.symbol.owner.is(ModuleClass) || classParents.contains(id.symbol.owner)) =>
-          tree.symbol :: symbols
+            if !id.symbol.is(Synthetic) && !id.symbol.is(Implicit) =>
+          symbols + tree.symbol
         case _ => symbols
 
-    val traverser = new DeepFolder[List[Symbol]](collectNames)
-    traverser(List(), rhs)
+    val traverser = new DeepFolder[Set[Symbol]](collectNames)
+    traverser(Set(), rhs)
   end symbolsUsedInDefn
 
   private def getReferencesToInline(
       definition: DefinitionTree,
       allOccurences: List[Occurence],
-      symbols: List[Symbol],
+      symbols: Set[Symbol],
   ): Either[String, (Boolean, List[Reference])] =
     val defIsLocal = definition.tree.symbol.ownersIterator
       .drop(1)
@@ -160,7 +155,7 @@ final class PcInlineValueProviderImpl(
 
   private def makeRefsEdits(
       refs: List[Occurence],
-      symbols: List[Symbol],
+      symbols: Set[Symbol],
   ): Either[String, List[Reference]] =
     val newctx = driver.currentCtx.fresh.setCompilationUnit(unit)
     def buildRef(occurence: Occurence): Either[String, Reference] =

diff --git a/tests/cross/src/test/scala/tests/pc/InlineValueSuite.scala b/tests/cross/src/test/scala/tests/pc/InlineValueSuite.scala
@@ -317,6 +317,29 @@ class InlineValueSuite extends BaseCodeActionSuite with CommonMtagsEnrichments {
        |""".stripMargin
   )
 
+  checkEdit(
+    "i6924-2",
+    """|object O {
+       |  def ==(o: O) = false
+       |}
+       |object P {
+       |  def test() = {
+       |    val isOne = O == O
+       |    <<i>>sOne
+       |  }
+       |}
+       |""".stripMargin,
+    """|object O {
+       |  def ==(o: O) = false
+       |}
+       |object P {
+       |  def test() = {
+       |    O == O
+       |  }
+       |}
+       |""".stripMargin
+  )
+
   checkError(
     "scoping-packages",
     """|package a
@@ -365,6 +388,26 @@ class InlineValueSuite extends BaseCodeActionSuite with CommonMtagsEnrichments {
     InlineErrors.variablesAreShadowed("A.k")
   )
 
+  checkError(
+    "bad-scoping-3",
+    """|class T {
+       |    val a = 1
+       |}
+       |
+       |class O {
+       |  val t = new T()
+       |  import t._
+       |  val bb = a + a
+       |
+       |  class Inner {
+       |    val a = 123
+       |    val cc = <<b>>b
+       |  }
+       |}
+       |""".stripMargin,
+    InlineErrors.variablesAreShadowed("T.a")
+  )
+
   def checkEdit(
       name: TestOptions,
       original: String,