[LSP] fix go to definition

[rgoomar]

Overview

These changes made it so I could go to a proper definition. This approach aligns with the way the TypeScript version of the compiler does it using VFS only and VLQ decoding.

Basically we have a situation where we have an import in a .ts file like

import { getLogger } from '@latticehr/o11y';

in a directory like apps/app-one in an index.ts file and the @latticehr is an alias that should reference packages/o11y and it does go to the packages/o11y/dist/*.d.ts file although not the source file implementation

This gets to the correct file and line position using VLQ decoding like the TS version does. This also enables the differentiation between "Go to Definition" and "Go to Type Definition". They both work now.

TypeScript Source Code References

Our Go implementation replicates several key interfaces and functions from the TypeScript codebase. Here are the specific references:

Core Interfaces and Types

1. DocumentPosition Interface

• Go: internal/sourcemap/mapper.go:11-15
• TypeScript: _submodules/TypeScript/src/compiler/types.ts:9889-9892

export interface DocumentPosition {
    fileName: string;
    pos: number;
}

2. DocumentPositionMapper Interface

• Go: internal/sourcemap/mapper.go:17-21
• TypeScript: _submodules/TypeScript/src/compiler/types.ts:9883-9886

export interface DocumentPositionMapper {
    getSourcePosition(input: DocumentPosition): DocumentPosition;
    getGeneratedPosition(input: DocumentPosition): DocumentPosition;
}

3. DocumentPositionMapperHost Interface

• Go: internal/sourcemap/mapper.go:23-28
• TypeScript: _submodules/TypeScript/src/compiler/types.ts:9872-9876

export interface DocumentPositionMapperHost {
    getSourceFileLike(fileName: string): SourceFileLike | undefined;
    getCanonicalFileName(path: string): string;
    log(text: string): void;
}

4. SourceFileLike Interface

• Go: internal/sourcemap/mapper.go:30-33
• TypeScript: _submodules/TypeScript/src/compiler/types.ts:4274-4280

export interface SourceFileLike {
    readonly text: string;
    /** @internal */
    lineMap?: readonly number[];
    /** @internal */
    getPositionOfLineAndCharacter?(line: number, character: number, allowEdits?: true): number;
}

Core Implementation Functions

5. createDocumentPositionMapper Function

• Go: internal/sourcemap/mapper.go:36-49 (CreateDocumentPositionMapper)
• TypeScript: _submodules/TypeScript/src/compiler/sourcemap.ts:699-713

export function createDocumentPositionMapper(host: DocumentPositionMapperHost, map: RawSourceMap, mapPath: string): DocumentPositionMapper

Service Layer Interfaces

6. SourceMapper Interface

• Go: internal/sourcemap/sourcemapper.go:10-15
• TypeScript: _submodules/TypeScript/src/services/sourcemaps.ts:33-39

export interface SourceMapper {
    toLineColumnOffset(fileName: string, position: number): LineAndCharacter;
    tryGetSourcePosition(info: DocumentPosition): DocumentPosition | undefined;
    tryGetGeneratedPosition(info: DocumentPosition): DocumentPosition | undefined;
    clearCache(): void;
    documentPositionMappers: Map<string, DocumentPositionMapper>;
}

7. SourceMapperHost Interface

• Go: internal/sourcemap/sourcemapper.go:17-24
• TypeScript: _submodules/TypeScript/src/services/sourcemaps.ts:42-51

export interface SourceMapperHost {
    useCaseSensitiveFileNames(): boolean;
    getCurrentDirectory(): string;
    getProgram(): Program | undefined;
    fileExists?(path: string): boolean;
    readFile?(path: string, encoding?: string): string | undefined;
    getSourceFileLike?(fileName: string): SourceFileLike | undefined;
    getDocumentPositionMapper?(generatedFileName: string, sourceFileName?: string): DocumentPositionMapper | undefined;
    log(s: string): void;
}

8. getSourceMapper Function

• Go: internal/sourcemap/sourcemapper.go:40-47 (CreateSourceMapper)
• TypeScript: _submodules/TypeScript/src/services/sourcemaps.ts:54-65

export function getSourceMapper(host: SourceMapperHost): SourceMapper

9. getDocumentPositionMapper Function (within SourceMapper)

• Go: internal/sourcemap/sourcemapper.go:83-98 (getDocumentPositionMapper method)
• TypeScript: _submodules/TypeScript/src/services/sourcemaps.ts:71-91

function getDocumentPositionMapper(generatedFileName: string, sourceFileName?: string)

10. tryGetSourcePosition Function (within SourceMapper)

• Go: internal/sourcemap/sourcemapper.go:55-70 (TryGetSourcePosition method)
• TypeScript: _submodules/TypeScript/src/services/sourcemaps.ts:93-99

function tryGetSourcePosition(info: DocumentPosition): DocumentPosition | undefined

Key Implementation Details

11. VLQ Decoding and Mapping Processing

• Go: internal/sourcemap/mapper.go:135-180 (ensureMappingsDecoded)
• TypeScript: _submodules/TypeScript/src/compiler/sourcemap.ts:737-752

function getDecodedMappings() {
    if (decodedMappings === undefined) {
        const decoder = decodeMappings(map.mappings);
        const mappings = arrayFrom(decoder, processMapping);
        // ...
    }
    return decodedMappings;
}

The Go code maintains the same separation of concerns, with the sourcemap package handling low-level mapping operations and the ls package providing LSP-specific integration, just as TypeScript separates compiler-level and service-level functionality.

[rgoomar]

@microsoft-github-policy-service agree

[alexwork1611]

Keeping an eye on this...

[iisaduan]

This shouldn't be added onto the compilerhost since d.ts.map is only used by ls features. Can you refactor so only the calls to readFile in definitionSourceMap has this fallback for d.ts.map files? It should probably also read into the snapshot's fs, instead of the underlying file system

[rgoomar]

Makes sense, I'll adjust that

[rgoomar]

@iisaduan I made the change with an alternative method here. Turns out that fs.source.FS() is the snapshot FS

[rgoomar]

This is working for multiple engineers internally for us 😄

[jakebailey]

I think this is going in the right direction, and I've now skimmed the old code to see how this is going, but I think the separation of concerns isn't quite right.

The source map decoding stuff needs to move into the sourcemap package and not reference the LS code. There is a lot of code already there that should be reused.

I also don't like the hardcoding of the ^ stuff, which we didn't have to do before as far as I can tell. That to me means something isn't quite working, but I think I'd like to see the code split a bit to help me be less distracted at the actual .map code.

[jakebailey]

I think the core idea is right, but I still don't like how much indirection and layering this introduces. It seems like a bunch of hosts, create functions returning more layers, etc, when it could all just be one thing, one host, etc. The fallback thing I can understand needing, but it's simpler to instead accept an FS that won't fail on reads, and we typically do not subdivide out specific read/write methods, unlike the old codebase with the many many host implementations.

I don't know what specific feedback I can give to make this better, though. It's not really one thing, more of a general design feeling that makes this seem much more indirected than other code we have in the repo.

[jakebailey]

This already exists in the tspath package, and other places you've used HasSuffix for this can be replaced too.

[jakebailey]

This isn't a correct implementation, unfortunately. If we need to compare paths like this, we use tspath.ToPath.

We also don't pass around "canonical filename functions" anymore, instead choosing to pass around Paths and the case sensitivity.

[jakebailey]

This func already exists in util.go.

[jakebailey]

This ClearCache infra is unused.

[jakebailey]

This could really just be an FS() method