Skip to content

Update contains method to include start position (#34)#37

Open
tothambrus11 wants to merge 2 commits into
ChimeHQ:mainfrom
tothambrus11:patch-1
Open

Update contains method to include start position (#34)#37
tothambrus11 wants to merge 2 commits into
ChimeHQ:mainfrom
tothambrus11:patch-1

Conversation

@tothambrus11
Copy link
Copy Markdown

Fixes #34

@tothambrus11 tothambrus11 mentioned this pull request May 7, 2026
@mattmassicotte
Copy link
Copy Markdown
Contributor

Ahh yes, this is a tricky one. Ranges are always subtle. I was planning on adding some testing around this to make sure I understood what "correct" was. What do you think about that?

@tothambrus11
Copy link
Copy Markdown
Author

tothambrus11 commented May 7, 2026

Ranges are half-open, as specified by the LSP. When implementing language servers to resolve a node under a cursor, we would typically not use this, but something that matches on both ends inclusively though. E.g. if we have an identifier x, the cursor should match on both sides of it.

import FrontEnd
import Logging

extension Program {

  /// - Requires: The source file of `position` is present in `self`.
  public func innermostTree(
    containing position: SourcePosition, reportingLogsTo logger: Logger, in f: SourceFile.ID
  ) -> AnySyntaxIdentity? {
    var v = NodeFinder(position)
    visit(topLevelDeclarations(in: f), calling: &v)
    return v.deepestMatch
  }

}

/// Requires that the visiting happens in a depth-first order.
private struct NodeFinder: SyntaxVisitor {

  // todo use binary search for efficiency if we can assume AST entries are sorted by position (probably they aren't though)

  let targetPosition: SourcePosition
  private(set) var deepestMatch: AnySyntaxIdentity?
  private var deepestMatchDepth: Int = -1
  private var currentDepth: Int = 0

  public init(_ targetPosition: SourcePosition) {
    self.targetPosition = targetPosition
  }

  mutating func willEnter(_ n: AnySyntaxIdentity, in program: Program) -> Bool {
    if program[n].site.region.containsInclusive(targetPosition.index) {
      if currentDepth > deepestMatchDepth {
        deepestMatchDepth = currentDepth
        deepestMatch = n
      }
    } else {
      if program.isScope(n) {
        // If it's a scope, we know its children's sites are strictly subsumed, so we can skip them.
        return false
      }
    }

    currentDepth += 1

    return true  // continue visiting children
  }

  public mutating func willExit(_ node: AnySyntaxIdentity, in program: Program) {
    currentDepth -= 1
  }

}

extension Range {

  func containsInclusive(_ i: Bound) -> Bool {
    return self.lowerBound <= i && i <= self.upperBound
  }

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LSPRange contains method is not inclusive at the start

2 participants