Skip to content
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

Let ChangesetBuilder.invalidated search even container elements #6548

Merged
merged 6 commits into from
May 5, 2023
Merged

Let ChangesetBuilder.invalidated search even container elements #6548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
34c504b
Test to tease ChangesetBuilder.invalidated with complex line expression
JaroslavTulach May 4, 2023
97be52f
Fill the gaps between leaf IR elements with their parent IR containers
JaroslavTulach May 4, 2023
f867313
Prefer old behavior when it delivers its results
JaroslavTulach May 4, 2023
491bad7
Line 9 column 72
JaroslavTulach May 5, 2023
8a2671b
Removing the IDE metadata line
JaroslavTulach May 5, 2023
07fc59c
More descriptive identifiers
JaroslavTulach May 5, 2023
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
88 changes: 70 additions & 18 deletions engine/runtime/src/main/scala/org/enso/compiler/context/ChangesetBuilder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,12 @@ final class ChangesetBuilder[A: TextEditor: IndexedSource](
else {
val edit = edits.dequeue()
val locationEdit = ChangesetBuilder.toLocationEdit(edit, source)
val invalidatedSet =
ChangesetBuilder.invalidated(tree, locationEdit.location)
var invalidatedSet =
ChangesetBuilder.invalidated(tree, locationEdit.location, true)
if (invalidatedSet.isEmpty) {
invalidatedSet =
ChangesetBuilder.invalidated(tree, locationEdit.location, false)
}
val newTree = ChangesetBuilder.updateLocations(tree, locationEdit)
val newSource = TextEditor[A].edit(source, edit)
go(newTree, newSource, edits, ids ++= invalidatedSet.map(_.id))
Expand Down Expand Up @@ -278,7 +282,7 @@ object ChangesetBuilder {
* @param id the node id
* @param location the node location
*/
private case class Node(id: NodeId, location: Location) {
private case class Node(id: NodeId, location: Location, leaf: Boolean) {

/** Shift the node location.
*
Expand All @@ -302,7 +306,7 @@ object ChangesetBuilder {
* @return the node if `ir` contains a location
*/
def fromIr(ir: IR): Option[Node] =
ir.location.map(loc => Node(NodeId(ir), loc.location))
ir.location.map(loc => Node(NodeId(ir), loc.location, true))

/** Create an artificial node with fixed [[NodeId]]. It is used to select
* nodes by location in the tree.
Expand All @@ -311,7 +315,11 @@ object ChangesetBuilder {
* @return a select node
*/
def select(location: Location): Node =
new Node(NodeId(UUID.nameUUIDFromBytes(Array()), None, None), location)
new Node(
NodeId(UUID.nameUUIDFromBytes(Array()), None, None),
location,
false
)

implicit val ordering: Ordering[Node] = (x: Node, y: Node) => {
val compareStart =
Expand All @@ -336,17 +344,55 @@ object ChangesetBuilder {
* @return the tree representation of the IR
*/
private def buildTree(ir: IR): Tree = {
@scala.annotation.tailrec
def go(input: mutable.Queue[IR], acc: Tree): Tree =
if (input.isEmpty) acc
else {
val ir = input.dequeue()
if (ir.children.isEmpty) {
Node.fromIr(ir).foreach(acc.add)
def depthFirstSearch(currentIr: IR, acc: Tree): Unit = {
if (currentIr.children.isEmpty) {
Node.fromIr(currentIr).foreach(acc.add)
} else {
val hasImportantId = currentIr.getExternalId.nonEmpty
if (hasImportantId) {
val collectChildrenIntervals = new Tree()
currentIr.children.map(depthFirstSearch(_, collectChildrenIntervals))

def fillGapsInChildrenNodesWithNonLeafNodes(
previousPosition: Int,
nextNode: Node
): Int = {
if (previousPosition < nextNode.location.start) {
val nodeBetweenPreviousPositionAndNextNode =
Node(
NodeId(currentIr),
Location(previousPosition, nextNode.location.start),
false
)
acc += nodeBetweenPreviousPositionAndNextNode
}
acc += nextNode
nextNode.location.end
}
val beginOfNonLeafIr = currentIr.location.get.location.start
val endOfNonLeafIr = currentIr.location.get.location.end
val lastCoveredPosition =
collectChildrenIntervals.foldLeft(beginOfNonLeafIr)(
fillGapsInChildrenNodesWithNonLeafNodes
)
val hasRemainingTextAfterLastChild =
lastCoveredPosition < endOfNonLeafIr
if (hasRemainingTextAfterLastChild) {
val nodeAfterLastChild = Node(
NodeId(currentIr),
Location(lastCoveredPosition, endOfNonLeafIr),
false
)
acc += nodeAfterLastChild
}
} else {
currentIr.children.map(depthFirstSearch(_, acc))
}
go(input ++= ir.children, acc)
}
go(mutable.Queue(ir), mutable.TreeSet())
}
val collectNodes = new Tree()
depthFirstSearch(ir, collectNodes)
collectNodes
}

/** Update the tree locations after applying the edit.
Expand All @@ -371,12 +417,18 @@ object ChangesetBuilder {
* @param edit the location of the edit
* @return the invalidated nodes of the tree
*/
private def invalidated(tree: Tree, edit: Location): Tree = {
private def invalidated(
tree: Tree,
edit: Location,
onlyLeafs: Boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

and why do we need this parameter now?

Copy link
Member Author

@JaroslavTulach JaroslavTulach May 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Without f867313 the "should module with undefined literal" test fails:

$ git show f867313 | patch -p1 -R
$ sbt runtime/testOnly *ChangesetBuilderTest
[info] - should module with undefined literal *** FAILED ***
[info]   Set(b1c393b2-67be-488b-b46d-2adba21bca6d, c05a82f3-9dea-49fb-81ce-8dfa5703e5ac)
 did not contain the same elements as
 List(c05a82f3-9dea-49fb-81ce-8dfa5703e5ac) (ChangesetBuilderTest.scala:368)

The additional b1c393b2-67be-488b-b46d-2adba21bca6d id represents 1 + undefined text. The fillInHoles function associates the space between + and undefined to b1c393b2-67be-488b-b46d-2adba21bca6d - as the edit replaces undefined with 42 it touches the space on the boundary and as such we get two change sets. To keep the original behavior we first work on "leafs" only as before and only when they find nothing, we try harder with non-leaf elements.

--- engine/runtime/src/test/scala/org/enso/compiler/test/context/ChangesetBuilderTest.scala
+++ engine/runtime/src/test/scala/org/enso/compiler/test/context/ChangesetBuilderTest.scala
@@ -366,6 +366,7 @@ class ChangesetBuilderTest extends CompilerTest {
         .originalName
 
       invalidated(ir, code, edit) should contain theSameElementsAs Seq(
+        UUID.fromString("b1c393b2-67be-488b-b46d-2adba21bca6d"),
         undefinedName.getId
       )
       invalidatedAll(ir, code, edit) should contain theSameElementsAs Seq(

Alternative is to modify the test and ignore or accept the additional result.

): Tree = {
val invalidated = mutable.TreeSet[ChangesetBuilder.Node]()
tree.iterator.foreach { node =>
if (intersect(edit, node)) {
invalidated += node
tree -= node
if (!onlyLeafs || node.leaf) {
if (intersect(edit, node)) {
invalidated += node
tree -= node
}
}
}
invalidated
Expand Down
69 changes: 68 additions & 1 deletion engine/runtime/src/test/scala/org/enso/compiler/test/context/ChangesetBuilderTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,77 @@ class ChangesetBuilderTest extends CompilerTest {
)
}

"toggle defaulted boolean parameter" in {
implicit val moduleContext: ModuleContext = freshModuleContext

val code =
"""
|from Standard.Base import all
|from Standard.Base.Data.Boolean import Boolean
|import Standard.Visualization
|
|main =
| text2 = " B B B "
| text1 = "B"
| text3 = "@"
| operator2 = text2.replace text1 text3 (Case_Sensitivity.Insensitive)
|
|
|
|
|#### METADATA ####
|[[{"index":{"value":5},"size":{"value":8}},"6d97d061-2c1d-4540-8f7d-3d8aa765323d"],[{"index":{"value":13},"size":{"value":1}},"a6a1b572-4867-4317-8a6e-b496b286ea2d"],[{"index":{"value":14},"size":{"value":4}},"b82e52c6-e2f0-4e16-b835-59ab8838cb2c"],[{"index":{"value":5},"size":{"value":13}},"4dd74fbb-c6df-4109-8821-f4643d483355"],[{"index":{"value":0},"size":{"value":29}},"82014253-a4f5-431b-971b-2348f5a2ca1f"],[{"index":{"value":35},"size":{"value":8}},"b22aad40-a237-4d47-a140-930d731ca80e"],[{"index":{"value":43},"size":{"value":1}},"9d7f45f1-f537-480d-b65f-37a90c77455a"],[{"index":{"value":44},"size":{"value":4}},"c6ad377f-1fce-431d-bbd7-cfe88624c50d"],[{"index":{"value":35},"size":{"value":13}},"202d7fa4-c782-4bb8-900f-7bf5f9ebad99"],[{"index":{"value":48},"size":{"value":1}},"6847cb14-aa49-455d-be9d-e82a39460005"],[{"index":{"value":49},"size":{"value":4}},"36dcdde8-1667-495a-ab87-7154c8d3be67"],[{"index":{"value":35},"size":{"value":18}},"d67c4ce2-711a-4dd9-a1f8-b48f2730b141"],[{"index":{"value":53},"size":{"value":1}},"295fa6b1-260d-4f23-8bd6-64a1d60f01e5"],[{"index":{"value":54},"size":{"value":7}},"923a2769-ddc0-4339-b919-941155a4d1b7"],[{"index":{"value":35},"size":{"value":26}},"64ef6c8a-810e-42f4-8334-8b12c4ab14bc"],[{"index":{"value":69},"size":{"value":7}},"e8cf68da-2b41-4c70-b163-567a223459fd"],[{"index":{"value":30},"size":{"value":46}},"0b934140-6464-4a05-ae6c-416670e2c7e2"],[{"index":{"value":84},"size":{"value":8}},"eaa41303-7d43-420f-8d64-da8d376fab65"],[{"index":{"value":92},"size":{"value":1}},"d582d163-0d4b-4406-96a3-e4591766406d"],[{"index":{"value":93},"size":{"value":13}},"2649f663-b1b8-44a1-9158-f2cf23038471"],[{"index":{"value":84},"size":{"value":22}},"5b4e8628-e0b7-408e-8070-e5636514c7ac"],[{"index":{"value":77},"size":{"value":29}},"78558eb9-cfb0-41d1-8f9a-0f0c17db3b7c"],[{"index":{"value":108},"size":{"value":4}},"58c9224e-644d-40c1-be25-068b6b6dc947"],[{"index":{"value":113},"size":{"value":1}},"d01b8ec7-e780-40a3-b401-97e89e000484"],[{"index":{"value":120},"size":{"value":5}},"aa43682d-01ee-453d-9bcb-34c905d8d314"],[{"index":{"value":126},"size":{"value":1}},"2b5d6c3f-e030-4ee4-9102-b573f066efa0"],[{"index":{"value":128},"size":{"value":16}},"54493640-4981-4437-ad6e-fc80ab1b581f"],[{"index":{"value":120},"size":{"value":24}},"c04888d6-bc60-4534-aa7e-df3dd5d52410"],[{"index":{"value":149},"size":{"value":5}},"85ae1943-cfcf-4e53-97c7-f73fdae42529"],[{"index":{"value":155},"size":{"value":1}},"113ae47b-3a1d-497e-8a2a-b7775be6c66e"],[{"index":{"value":157},"size":{"value":3}},"c143e355-9d64-4780-b387-dd364eefa2f9"],[{"index":{"value":149},"size":{"value":11}},"73113bd8-d3cb-4b06-a4d7-425a88bca849"],[{"index":{"value":165},"size":{"value":5}},"a286047b-54a5-45bb-995e-60f207e2af65"],[{"index":{"value":171},"size":{"value":1}},"abace020-1a84-4fa4-a5ee-2f98f73dfcb1"],[{"index":{"value":173},"size":{"value":3}},"ed3e40fb-d19e-4fb6-bebd-f74ee179ae08"],[{"index":{"value":165},"size":{"value":11}},"8ac6594b-c472-41d5-9b00-f13e6f30d5f2"],[{"index":{"value":181},"size":{"value":9}},"25eaa42e-026d-4afd-a433-dde5804b7a5c"],[{"index":{"value":191},"size":{"value":1}},"799a83db-5893-40f7-9f1e-e07c7a3071cd"],[{"index":{"value":193},"size":{"value":5}},"49332623-fe9a-4fe5-9657-7c0ea2b23942"],[{"index":{"value":198},"size":{"value":1}},"7690e4c2-31e7-4a12-8f5c-78c4484b9553"],[{"index":{"value":199},"size":{"value":7}},"3606a45e-50be-49c5-9dda-415263145a15"],[{"index":{"value":193},"size":{"value":13}},"91640cfc-f985-49da-95c8-87181d725299"],[{"index":{"value":207},"size":{"value":5}},"afb24ea2-2f35-4c7e-8394-3ba34626484e"],[{"index":{"value":193},"size":{"value":19}},"86eb77c6-14cc-487a-8993-bf9998244321"],[{"index":{"value":213},"size":{"value":5}},"52a3e9b8-f8d0-43c9-9f6d-9785359c7ba7"],[{"index":{"value":193},"size":{"value":25}},"1cddcb6a-95c8-445b-9a6f-267093b2609c"],[{"index":{"value":220},"size":{"value":16}},"df114ed3-b64d-4e74-ad60-b43a7147ec19"],[{"index":{"value":236},"size":{"value":1}},"b6a14995-bb54-4956-9abd-9b8cff898ad3"],[{"index":{"value":237},"size":{"value":11}},"fdf942d8-8064-4067-ae38-131b80fa6e0c"],[{"index":{"value":220},"size":{"value":28}},"b8df7e14-9004-4f61-8076-09354f8dbd36"],[{"index":{"value":193},"size":{"value":56}},"0d4b3071-3f7d-472a-bfa7-af118222894e"],[{"index":{"value":181},"size":{"value":68}},"d9ee4c00-74b3-4859-a252-6c9105085076"],[{"index":{"value":115},"size":{"value":135}},"3eba123a-ea77-4db5-89d8-800d35e102e7"],[{"index":{"value":108},"size":{"value":142}},"11d6a77c-3644-4a27-8e15-16808af51c19"]]
|""".stripMargin.linesIterator.mkString("\n")
val edit =
TextEdit(Range(Position(9, 72), Position(9, 72)), " Boolean.True")

val ir = code.preprocessModule
val atId = "0d4b3071-3f7d-472a-bfa7-af118222894e"
val at = findIR(ir, atId)
val atCode = findCode(code, at)
atCode shouldBe "text2.replace text1 text3 (Case_Sensitivity.Insensitive)"

val all = new ChangesetBuilder(Rope(code), ir).invalidated(Seq(edit))

all
.map(n => n.externalId.getOrElse(n.internalId))
.map(findCode(code, ir, _)) should contain theSameElementsAs Seq(
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved
atCode
)
}
}

def findIR(ir: IR, uuid: String): IR = {
val list = ir.preorder.filter(
_.location
.map(_.id.map(_.toString() == uuid).getOrElse(false))
.getOrElse(false)
)
if (list.isEmpty) {
null
} else {
list.head
}
}

def findCode(code: String, at: IR): String = {
val loc = at.location.get.location
code.substring(loc.start, loc.end)
}

def findCode(code: String, ir: IR, uuid: UUID): String = {
val at = findIR(ir, uuid.toString())
if (at == null) {
uuid.toString
} else {
findCode(code, at)
}
}

def invalidated(ir: IR, code: String, edits: TextEdit*): Set[IR.Identifier] =
new ChangesetBuilder(Rope(code), ir).invalidated(edits).map(_.internalId)
new ChangesetBuilder(Rope(code), ir)
.invalidated(edits)
.map(n => n.externalId.getOrElse(n.internalId))

def invalidatedAll(
ir: IR,
Expand Down