Handle nil end position in getMappedLocation

[jakebailey]

Fixes #1867

Analogous to "this shouldn't happen" 🙃

/** @internal */
export function getMappedDocumentSpan(documentSpan: DocumentSpan, sourceMapper: SourceMapper, fileExists?: (path: string) => boolean): DocumentSpan | undefined {
    const { fileName, textSpan } = documentSpan;
    const newPosition = getMappedLocation({ fileName, pos: textSpan.start }, sourceMapper, fileExists);
    if (!newPosition) return undefined;
    const newEndPosition = getMappedLocation({ fileName, pos: textSpan.start + textSpan.length }, sourceMapper, fileExists);
    const newLength = newEndPosition
        ? newEndPosition.pos - newPosition.pos
        : textSpan.length; // This shouldn't happen

[Copilot]

Pull Request Overview

This PR fixes a crash in the language service when handling source map positions where the end position is nil. The fix adds a nil check for the end position in getMappedLocation and provides a fallback calculation based on the start position and range length.

Key changes:

• Added nil handling for end position in source map location mapping
• Re-enabled a previously failing test that was affected by this bug
• Updated baseline outputs for multiple fourslash tests

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
internal/ls/source_map.go	Added nil check for endPos with fallback calculation using start position and range length
internal/fourslash/tests/gen/findAllRefsForDefaultExport03_test.go	Removed t.Skip() call to re-enable test
internal/fourslash/_scripts/failingTests.txt	Removed TestFindAllRefsForDefaultExport03 from failing tests list
testdata/baselines/reference/fourslash/findAllReferences/*.baseline.jsonc	Generated baseline outputs for newly passing tests
testdata/baselines/reference/fourslash/documentHighlights/*.baseline.jsonc	Generated baseline outputs for newly passing tests

[andrewbranch]

Looks right as a port, but since this is happening, do we need to ensure this position is inside the length of the file?

[jakebailey]

I'm not sure; my understanding is that the conversion to an LSP range will clamp it via the line map, but maybe not... I can grab the script and min the text length.

[andrewbranch]

It looks like PositionToLineAndCharacter will crash with a slice range out of bounds when rescanning the line for the UTF-16 character offset. Maybe it’s worth adding a guard there and not worrying about it here.

[jakebailey]

Hm, I can. I'm questioning whether or not it's a good idea, though, given we don't do the same sort of bounds checks when going from an LSP position to a byte position... Could go either way. Either way it's sort of papering over something.