Skip to content

[api] Add SourceFile line mapping methods#4420

Open
andrewbranch wants to merge 3 commits into
microsoft:mainfrom
andrewbranch:api-line-map
Open

[api] Add SourceFile line mapping methods#4420
andrewbranch wants to merge 3 commits into
microsoft:mainfrom
andrewbranch:api-line-map

Conversation

@andrewbranch

Copy link
Copy Markdown
Member

Closes #4215

Copilot AI review requested due to automatic review settings June 23, 2026 20:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds line/character mapping helpers to the native-preview SourceFile API surface to close #4215, enabling consumers to convert between UTF-16 positions and { line, character } pairs.

Changes:

  • Introduces LineAndCharacter and adds getLineStarts, getLineAndCharacterOfPosition, and getPositionOfLineAndCharacter to the SourceFile interface.
  • Implements these methods on RemoteSourceFile, including a cached line-starts array and a binary search for position-to-line mapping.
  • Adds encoder round-trip tests covering line starts, UTF-16 semantics, and line/character ↔ position conversion.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
_packages/native-preview/test/encoder.test.ts Adds tests validating line start computation, UTF-16 line/character mapping, caching, and round-tripping.
_packages/native-preview/src/ast/ast.ts Extends the public AST types with LineAndCharacter and new SourceFile mapping APIs.
_packages/native-preview/src/api/node/node.ts Implements the new mapping APIs on RemoteSourceFile using computeLineStarts + binary search.

Comment on lines +212 to +216
getLineAndCharacterOfPosition(position: number): LineAndCharacter {
const lineStarts = this.getLineStarts();
const line = computeLineOfPosition(lineStarts, position);
return { line, character: position - lineStarts[line] };
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The Strada implementation doesn't guard, so I think it's fine as-is

Comment on lines +218 to +224
getPositionOfLineAndCharacter(line: number, character: number): number {
const lineStarts = this.getLineStarts();
if (line < 0 || line >= lineStarts.length) {
throw new Error(`Bad line number. Line: ${line}, lineStarts.length: ${lineStarts.length}`);
}
return lineStarts[line] + character;
}
@andrewbranch andrewbranch enabled auto-merge June 23, 2026 21:14
@andrewbranch andrewbranch added this pull request to the merge queue Jun 23, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Jun 23, 2026
@andrewbranch andrewbranch enabled auto-merge June 23, 2026 22:12
@mrazauskas

Copy link
Copy Markdown

@andrewbranch Thanks for working on this. I bumped into another limitation and was thinking about a solution.

The only place my app is using these is diagnostic formatting logic. This is handy when a diagnostic spans multiple lines. It works well with Strada API, because Diagnostic has a file: SourceFile property. In the current API, Diagnostic has fileName: string and as we agreed, it is the user’s responsibility to retrieve the SourceFile. (For example, using program.getSourceFile(diagnostic.fileName) as suggested in #4316.)

The problem is that errors of TSConfig files are also reported as Diagnostic[], but program.getSourceFile("path/to/tsconfig.json") will not work (and that makes sense). Hence, formatting of program.getConfigFileParsingDiagnostics() is a trouble because of missing line mapping methods.

Perhaps the line mapping could be implemented as a separate utility class rather than adding them directly to SourceFile? One could use it with any file.

class TextFile {
  constructor(text: string) {
    // ...
  }

  getLineStarts(): readonly number[] {
    // ...
  }

  getLineAndCharacterOfPosition(position: number): LineAndCharacter {
    // ...
  }

  // ...
}

@andrewbranch

Copy link
Copy Markdown
Member Author

Thanks for making me realize this PR never merged 😅 Actually, tsconfig files do have SourceFiles (Strada has APIs for parsing them but not fetching them), so I wonder if it would be enough to expose a way of getting at those?

@mrazauskas

Copy link
Copy Markdown

Yes, some way of getting SourceFile of given tsconfig would be enough for diagnostic formatting purpose. It can be I was overthinking.

In a way it is clear that .getConfigFileParsingDiagnostics() always returns Diagnostic that will point to some tsconfig file. Not sure about .getProgramDiagnostics() or .getGlobalDiagnostics(). Would it better to open a separate issue regarding SourceFile of tsconfig files, perhaps?

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.

SourceFile is missing .getLineStarts() and .getLineAndCharacterOfPosition()

4 participants