Skip to content

Commit 6bf4515

Browse files
committed
fix: key URI caches by NormalizedUri (text cache, settings cache)
TextCache and the per-document settings cache were keyed by raw URI strings, so a differently-encoded URI for the same file (Windows `%3A` vs `:`) could leak a duplicate entry or miss the cache - the same class as the diagnostic-store slip. Both now normalize at the boundary, matching how the reference and symbol indexes already key. Also tighten `lookupUris` and `isVisibleInContext` to `NormalizedUri` (values were already normalized; the type just dropped the brand). Docs: correct a stale architecture note calling MSG/TRA purely highlight-only - they are now parsed server-side for diagnostics.
1 parent 1f2d71c commit 6bf4515

5 files changed

Lines changed: 27 additions & 13 deletions

File tree

docs/architecture.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,8 @@ Snapshot contract:
516516

517517
### Tree-Sitter Grammars
518518

519-
Six tree-sitter grammars compiled to WASM (4 LSP + 2 highlight-only for external editors).
519+
Six tree-sitter grammars compiled to WASM: 4 back full LSP providers, and 2 (MSG, TRA) are
520+
parsed by the server for parse-error diagnostics and provide highlighting for external editors.
520521
See [grammars/README.md](../grammars/README.md) for the full list, build commands,
521522
WASM rationale, and type generation details.
522523

server/src/core/symbol-index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ export class Symbols {
574574
/**
575575
* Check if a symbol is visible in the given context.
576576
*/
577-
private isVisibleInContext(symbol: IndexedSymbol, uri: string, context?: QueryContext): boolean {
577+
private isVisibleInContext(symbol: IndexedSymbol, uri: NormalizedUri, context?: QueryContext): boolean {
578578
// Global and workspace symbols are always visible
579579
if (symbol.scope.level === ScopeLevel.Global || symbol.scope.level === ScopeLevel.Workspace) {
580580
return true;

server/src/handlers/document-lifecycle.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Connection } from "vscode-languageserver/node";
22
import { isHeaderFile } from "../core/location-utils";
3-
import { normalizeUri } from "../core/normalized-uri";
3+
import { type NormalizedUri, normalizeUri } from "../core/normalized-uri";
44
import { registry } from "../provider-registry";
55
import { getServerContext, tryGetServerContext } from "../server-context";
66
import { compile } from "../compile";
@@ -16,7 +16,9 @@ import {
1616
} from "../settings";
1717
import type { HandlerContext } from "./context";
1818

19-
const documentSettings: Map<string, Thenable<MLSsettings>> = new Map();
19+
// Keyed by NormalizedUri so a differently-encoded URI for the same file does not
20+
// leak a duplicate cache entry (or miss the cached settings on close).
21+
const documentSettings: Map<NormalizedUri, Thenable<MLSsettings>> = new Map();
2022

2123
/**
2224
* Build the `getDocumentSettings` function used by compile.ts and the
@@ -30,15 +32,18 @@ export function makeGetDocumentSettings(connection: Connection): (resource: stri
3032
if (!serverCtx?.capabilities.configuration) {
3133
return Promise.resolve(serverCtx?.settings ?? defaultSettings);
3234
}
33-
let result = documentSettings.get(resource);
35+
// Normalize only the cache key; the `scopeUri` request keeps the
36+
// caller's original resource string.
37+
const key = normalizeUri(resource);
38+
let result = documentSettings.get(key);
3439
if (!result) {
3540
result = connection.workspace
3641
.getConfiguration({
3742
scopeUri: resource,
3843
section: "bgforge",
3944
})
4045
.then(normalizeSettings);
41-
documentSettings.set(resource, result);
46+
documentSettings.set(key, result);
4247
}
4348
return result;
4449
};
@@ -51,7 +56,7 @@ export function clearDocumentSettings(): void {
5156

5257
export function register(ctx: HandlerContext): void {
5358
ctx.documents.onDidClose((e) => {
54-
documentSettings.delete(e.document.uri);
59+
documentSettings.delete(normalizeUri(e.document.uri));
5560
registry.handleDocumentClosed(e.document.languageId, e.document.uri);
5661
});
5762

server/src/shared/references-index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ export class ReferencesIndex {
4040
* Look up URIs of all files that reference a symbol name.
4141
* More efficient than lookup() when only file membership is needed (e.g., rename).
4242
*/
43-
lookupUris(symbolName: string): ReadonlySet<string> {
44-
const uris = new Set<string>();
43+
lookupUris(symbolName: string): ReadonlySet<NormalizedUri> {
44+
const uris = new Set<NormalizedUri>();
4545
for (const [uri, fileRefs] of this.files) {
4646
if (fileRefs.has(symbolName)) {
4747
uris.add(uri);

server/src/shared/text-cache.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
* the implementation a plain insertion-ordered Map with no reordering on read.
1414
*/
1515

16+
import { type NormalizedUri, normalizeUri } from "../core/normalized-uri";
17+
1618
/** Cache entry with version and parsed data */
1719
interface CacheEntry<T> {
1820
version: number;
@@ -29,7 +31,9 @@ const DEFAULT_MAX_SIZE = 50;
2931
* @typeParam T - The type of parsed data to cache
3032
*/
3133
export class TextCache<T> {
32-
private readonly cache = new Map<string, CacheEntry<T>>();
34+
// Keyed by NormalizedUri so a differently-encoded URI for the same file
35+
// (Windows `%3A` vs `:`) hits the same entry instead of leaking a duplicate.
36+
private readonly cache = new Map<NormalizedUri, CacheEntry<T>>();
3337
private readonly maxSize: number;
3438

3539
constructor(maxSize: number = DEFAULT_MAX_SIZE) {
@@ -59,8 +63,12 @@ export class TextCache<T> {
5963
return parse(text, uri);
6064
}
6165

66+
// Normalize only the cache key; `parse` still receives the caller's
67+
// original URI so any locations it embeds keep the caller's encoding.
68+
const key = normalizeUri(uri);
69+
6270
// Check cache
63-
const cached = this.cache.get(uri);
71+
const cached = this.cache.get(key);
6472
if (cached && cached.version === version) {
6573
return cached.data;
6674
}
@@ -79,13 +87,13 @@ export class TextCache<T> {
7987
}
8088
}
8189

82-
this.cache.set(uri, { version, data });
90+
this.cache.set(key, { version, data });
8391
return data;
8492
}
8593

8694
/** Clear cache for a specific URI. */
8795
clear(uri: string): void {
88-
this.cache.delete(uri);
96+
this.cache.delete(normalizeUri(uri));
8997
}
9098

9199
/** Clear entire cache. */

0 commit comments

Comments
 (0)