Skip to content

Replace IncrementalParseReusedNodeDelegate with ReusedNodeCallback #1900

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.

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

Merged
merged 1 commit into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 10 additions & 34 deletions Sources/SwiftParser/IncrementalParseTransition.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,61 +35,39 @@ extension Parser {
}
}

/// Accepts the re-used ``Syntax`` nodes that `IncrementalParseTransition`
/// Accepts a re-used ``Syntax`` node that `IncrementalParseTransition`
/// determined they should be re-used for a parse invocation.
///
/// The client can use this information to potentially avoid unnecessary work
/// for regions of the source that have not changed.
///
/// This is also used for testing purposes to ensure incremental reparsing
/// worked as expected.
public protocol IncrementalParseReusedNodeDelegate {
/// Accepts ``Syntax`` node of skipped source region.
///
/// - Parameters:
/// - previousNode: The node from the previous tree that is associated with
/// the skipped source region.
func parserReusedNode(previousNode: Syntax)
}

/// An implementation of `IncrementalParseReusedNodeDelegate` that just collects
/// the range and re-used node into an array.
public final class IncrementalParseReusedNodeCollector:
IncrementalParseReusedNodeDelegate
{
public var nodes: [Syntax] = []

public init() {}

public func parserReusedNode(previousNode: Syntax) {
nodes.append(previousNode)
}
}
public typealias ReusedNodeCallback = (_ node: Syntax) -> ()

/// Keeps track of a previously parsed syntax tree and the source edits that
/// occurred since it was created.
public final class IncrementalParseTransition {
fileprivate let previousTree: SourceFileSyntax
fileprivate let edits: ConcurrentEdits
fileprivate let lookaheadRanges: LookaheadRanges
fileprivate let reusedDelegate: IncrementalParseReusedNodeDelegate?
fileprivate let reusedNodeCallback: ReusedNodeCallback?

/// - Parameters:
/// - previousTree: The previous tree to do lookups on.
/// - edits: The edits that have occurred since the last parse that resulted
/// in the new source that is about to be parsed.
/// - reusedNodeDelegate: Optional delegate to accept information about the
/// reused regions and nodes.
/// - reusedNodeCallback: Optional closure to accept information about the re-used node. For each node that gets re-used `reusedNodeCallback` is called.
public init(
previousTree: SourceFileSyntax,
edits: ConcurrentEdits,
lookaheadRanges: LookaheadRanges,
reusedNodeDelegate: IncrementalParseReusedNodeDelegate? = nil
reusedNodeCallback: ReusedNodeCallback? = nil
) {
self.previousTree = previousTree
self.edits = edits
self.lookaheadRanges = lookaheadRanges
self.reusedDelegate = reusedNodeDelegate
self.reusedNodeCallback = reusedNodeCallback
}
}

Expand All @@ -110,8 +88,8 @@ struct IncrementalParseLookup {
return transition.edits
}

fileprivate var reusedDelegate: IncrementalParseReusedNodeDelegate? {
return transition.reusedDelegate
fileprivate var reusedCallback: ReusedNodeCallback? {
return transition.reusedNodeCallback
}

/// Does a lookup to see if the current source `offset` should be associated
Expand All @@ -132,10 +110,8 @@ struct IncrementalParseLookup {
}
let prevPosition = AbsolutePosition(utf8Offset: prevOffset)
let node = cursorLookup(prevPosition: prevPosition, kind: kind)
if let delegate = reusedDelegate, let node {
delegate.parserReusedNode(
previousNode: node
)
if let node {
reusedCallback?(node)
}
return node
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ public func assertIncrementalParse(

let (originalTree, lookaheadRanges) = Parser.parseIncrementally(source: originalString, parseTransition: nil)

let reusedNodesCollector = IncrementalParseReusedNodeCollector()
var reusedNodes: [Syntax] = []
let transition = IncrementalParseTransition(
previousTree: originalTree,
edits: concurrentEdits,
lookaheadRanges: lookaheadRanges,
reusedNodeDelegate: reusedNodesCollector
reusedNodeCallback: { reusedNodes.append($0) }
)

let newTree = Parser.parse(source: editedString)
Expand Down Expand Up @@ -66,11 +66,11 @@ public func assertIncrementalParse(
}

// Re-used nodes
if reusedNodesCollector.nodes.count != expectedReusedNodes.count {
if reusedNodes.count != expectedReusedNodes.count {
XCTFail(
"""
Expected \(expectedReusedNodes.count) re-used nodes but received \(reusedNodesCollector.nodes.count):
\(reusedNodesCollector.nodes.map {$0.description}.joined(separator: "\n"))
Expected \(expectedReusedNodes.count) re-used nodes but received \(reusedNodes.count):
\(reusedNodes.map {$0.description}.joined(separator: "\n"))
""",
file: file,
line: line
Expand All @@ -84,11 +84,11 @@ public func assertIncrementalParse(
continue
}

guard let reusedNode = reusedNodesCollector.nodes.first(where: { $0.byteRangeAfterTrimmingTrivia == range }) else {
guard let reusedNode = reusedNodes.first(where: { $0.byteRangeAfterTrimmingTrivia == range }) else {
XCTFail(
"""
Fail to match the range of \(expectedReusedNode.source) in:
\(reusedNodesCollector.nodes.map({"\($0.byteRangeAfterTrimmingTrivia): \($0.description)"}).joined(separator: "\n"))
\(reusedNodes.map({"\($0.byteRangeAfterTrimmingTrivia): \($0.description)"}).joined(separator: "\n"))
""",
file: expectedReusedNode.file,
line: expectedReusedNode.line
Expand Down
1 change: 0 additions & 1 deletion Tests/SwiftParserTest/IncrementalParsingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ public class IncrementalParsingTests: XCTestCase {
}

public func testMultiEditMapping() throws {
try XCTSkipIf(true, "Swift parser does not handle node reuse yet")
assertIncrementalParse(
"""
let one: Int;let two: Int; let three: Int; ⏩️⏸️ ⏪️⏩️⏸️ ⏪️let found: Int;let five: Int;
Expand Down