From 632c510a7298d3d71407f5d9d5617d7f1ab3a4eb Mon Sep 17 00:00:00 2001 From: kasiaMarek Date: Wed, 4 Dec 2024 16:03:53 +0100 Subject: [PATCH] fix: filter symbols for conflict check are in scope before inline --- .../pc/PcInlineValueProviderImpl.scala | 29 ++++++------- .../pc/PcInlineValueProviderImpl.scala | 31 ++++++------- .../scala/tests/pc/InlineValueSuite.scala | 43 +++++++++++++++++++ 3 files changed, 68 insertions(+), 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 index 99dce8e8649..e551bbaaffa 100644 --- 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 index 1eb12d48264..5d443d45fa0 100644 --- 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 index 79e438cee25..4e74198ce9d 100644 --- 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 + | <>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 + | } + |} + |""".stripMargin, + InlineErrors.variablesAreShadowed("T.a") + ) + def checkEdit( name: TestOptions, original: String,