Skip to content

Commit 5abd244

Browse files
committed
fix(vscode): resolve correctness & robustness bugs from extension bug hunt
Adversarially-verified bug hunt across the VS Code extension. Fixes: - resxDiagnostics: use path.basename so language-code extraction works on Windows (lastIndexOf('/') returned -1, breaking all diagnostics there). - jsonDocumentParser: duplicate simple key names in different scopes now resolve to their own location (scoped offset search); multi-line values carry a real endLine; getKeyRange returns null for missing keys instead of a bogus 0,0 range. - extension: ensureBackendReady() guard so commands don't crash with 'Cannot read properties of undefined' when backend init failed; the Restart Backend recovery path stays usable. File-watcher refresh is debounced + serialized to avoid concurrent invalidate/refetch races. - dashboard: coerce stats numbers so a malformed response never renders NaN. - resourceTreeView: null-safe key.values to avoid crashing the tree render. - definitionProvider: fallback paths return a Range (highlights) not a bare Position. - completionProvider: invalidateCache() also drops the config cache so lrm.json changes take effect immediately. - statusBar: tooltip shows 0 instead of undefined total keys. - resourceEditor: guard missing default language in Auto-Translate. - codeLens/cacheService: data-annotation keys now resolve details and coverage against the group named by their ResourceType (already on the wire as resourceTypeClassName); getMissingLanguages is group-aware. Adds regression tests for the JSON parser and CodeLens fixes.
1 parent 9945b97 commit 5abd244

15 files changed

Lines changed: 299 additions & 39 deletions

vscode-extension/src/backend/apiClient.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ export interface KeyReference {
4040
pattern: string;
4141
confidence: string;
4242
warning?: string;
43+
/**
44+
* For keys referenced via a Data Annotation (e.g.
45+
* `[Display(..., ResourceType = typeof(GlassResources))]`), the simple class name
46+
* of the bound resource type ("GlassResources"). Maps to a resx group's base name,
47+
* so a typed reference can be resolved against the correct group in multi-group
48+
* projects. Undefined for ordinary, group-agnostic references.
49+
*/
50+
resourceTypeClassName?: string;
4351
}
4452

4553
export interface KeyUsage {
472 Bytes
Binary file not shown.

vscode-extension/src/extension.ts

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,8 @@ function setupEventListeners(context: vscode.ExtensionContext, enableRealtimeSca
341341
const iosWatcher = vscode.workspace.createFileSystemWatcher('**/*.lproj/*.strings');
342342
const lrmConfigWatcher = vscode.workspace.createFileSystemWatcher('**/lrm.json');
343343

344-
// Helper to handle resource file changes
345-
const handleResourceChange = async () => {
344+
// The actual refresh. Kept private; callers go through the debounced wrapper below.
345+
const runResourceRefresh = async () => {
346346
// Invalidate shared cache first
347347
if (cacheService) {
348348
cacheService.invalidate();
@@ -357,6 +357,48 @@ function setupEventListeners(context: vscode.ExtensionContext, enableRealtimeSca
357357
}
358358
};
359359

360+
// Debounce + serialize resource-change handling. Without this, saving several
361+
// resource files at once fires many concurrent handlers that each invalidate the
362+
// cache and refetch, interleaving into an inconsistent tree/diagnostics view and
363+
// doing redundant work. We coalesce a burst into a single trailing refresh, and
364+
// never run two refreshes concurrently (a change arriving mid-refresh schedules
365+
// one more run afterwards).
366+
let refreshTimer: NodeJS.Timeout | undefined;
367+
let refreshInFlight: Promise<void> | undefined;
368+
let rerunRequested = false;
369+
const RESOURCE_REFRESH_DEBOUNCE_MS = 250;
370+
371+
const handleResourceChange = async (): Promise<void> => {
372+
if (refreshTimer) {
373+
clearTimeout(refreshTimer);
374+
}
375+
await new Promise<void>(resolve => {
376+
refreshTimer = setTimeout(() => {
377+
refreshTimer = undefined;
378+
resolve();
379+
}, RESOURCE_REFRESH_DEBOUNCE_MS);
380+
});
381+
382+
// If a refresh is already running, ask it to run once more when it finishes
383+
// rather than starting an overlapping one.
384+
if (refreshInFlight) {
385+
rerunRequested = true;
386+
return refreshInFlight;
387+
}
388+
389+
const drain = async (): Promise<void> => {
390+
do {
391+
rerunRequested = false;
392+
await runResourceRefresh();
393+
} while (rerunRequested);
394+
};
395+
396+
refreshInFlight = drain().finally(() => {
397+
refreshInFlight = undefined;
398+
});
399+
return refreshInFlight;
400+
};
401+
360402
// Helper to filter JSON events to only resource files (exclude config files)
361403
const isResourceJson = (uri: vscode.Uri): boolean => {
362404
const fileName = uri.fsPath.toLowerCase();
@@ -439,10 +481,29 @@ function setupEventListeners(context: vscode.ExtensionContext, enableRealtimeSca
439481
});
440482
}
441483

484+
/**
485+
* True once the backend and its dependent globals (apiClient, cacheService, …) were
486+
* initialized successfully during activation. When the backend fails to start, those
487+
* globals stay undefined but registerCommands() still runs, so command handlers must
488+
* check this before dereferencing them — otherwise any command throws a confusing
489+
* "Cannot read properties of undefined". lrmService itself is created before the
490+
* try block, so the Restart Backend command remains usable to recover.
491+
*/
492+
function ensureBackendReady(): boolean {
493+
if (!apiClient || !cacheService) {
494+
vscode.window.showErrorMessage(
495+
'LRM backend is not running. Use "LRM: Restart Backend" to try again, or check the LRM output log.'
496+
);
497+
return false;
498+
}
499+
return true;
500+
}
501+
442502
function registerCommands(context: vscode.ExtensionContext) {
443503
// Scan Code
444504
context.subscriptions.push(
445505
vscode.commands.registerCommand('lrm.scanCode', async () => {
506+
if (!ensureBackendReady()) return;
446507
try {
447508
const result = await vscode.window.withProgress({
448509
location: vscode.ProgressLocation.Notification,
@@ -501,6 +562,7 @@ function registerCommands(context: vscode.ExtensionContext) {
501562
// Validate Resources
502563
context.subscriptions.push(
503564
vscode.commands.registerCommand('lrm.validateResources', async () => {
565+
if (!ensureBackendReady()) return;
504566
try {
505567
const result = await vscode.window.withProgress({
506568
location: vscode.ProgressLocation.Notification,
@@ -530,6 +592,7 @@ function registerCommands(context: vscode.ExtensionContext) {
530592
// Open Resource Editor
531593
context.subscriptions.push(
532594
vscode.commands.registerCommand('lrm.openResourceEditor', async () => {
595+
if (!ensureBackendReady()) return;
533596
ResourceEditorPanel.createOrShow(context.extensionUri, apiClient, cacheService);
534597
})
535598
);
@@ -981,13 +1044,15 @@ function registerCommands(context: vscode.ExtensionContext) {
9811044
// Open Dashboard
9821045
context.subscriptions.push(
9831046
vscode.commands.registerCommand('lrm.openDashboard', () => {
1047+
if (!ensureBackendReady()) return;
9841048
DashboardPanel.createOrShow(apiClient);
9851049
})
9861050
);
9871051

9881052
// Open Settings
9891053
context.subscriptions.push(
9901054
vscode.commands.registerCommand('lrm.openSettings', () => {
1055+
if (!ensureBackendReady()) return;
9911056
SettingsPanel.createOrShow(apiClient);
9921057
})
9931058
);

vscode-extension/src/parsers/jsonDocumentParser.ts

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,11 @@ export class JsonDocumentParser implements IResourceDocumentParser {
4242

4343
// Find the key whose range contains the position
4444
for (const key of keys) {
45+
if (key.lineNumber < 0) {
46+
continue;
47+
}
4548
const keyOffset = document.offsetAt(new vscode.Position(key.lineNumber, key.columnStart));
46-
const endOffset = document.offsetAt(new vscode.Position(key.lineNumber, key.columnEnd));
49+
const endOffset = document.offsetAt(new vscode.Position(key.endLine ?? key.lineNumber, key.columnEnd));
4750

4851
// Expand range to include the full key-value pair
4952
// Look backwards for the key start
@@ -64,8 +67,13 @@ export class JsonDocumentParser implements IResourceDocumentParser {
6467

6568
for (const key of keys) {
6669
if (key.key === keyName) {
70+
// lineNumber -1 means the key was parsed but its text location could
71+
// not be resolved; return null rather than a bogus range at 0,0.
72+
if (key.lineNumber < 0) {
73+
return null;
74+
}
6775
const startPos = new vscode.Position(key.lineNumber, key.columnStart);
68-
const endPos = new vscode.Position(key.lineNumber, key.columnEnd);
76+
const endPos = new vscode.Position(key.endLine ?? key.lineNumber, key.columnEnd);
6977
return new vscode.Range(startPos, endPos);
7078
}
7179
}
@@ -81,12 +89,18 @@ export class JsonDocumentParser implements IResourceDocumentParser {
8189
prefix: string,
8290
keys: ResourceKey[],
8391
text: string,
84-
document: vscode.TextDocument
92+
document: vscode.TextDocument,
93+
searchFrom = 0
8594
): void {
8695
if (typeof obj !== 'object' || obj === null) {
8796
return;
8897
}
8998

99+
// Local cursor advanced past each key as it is consumed, so sibling keys —
100+
// and identical simple names nested under different parents — resolve to
101+
// distinct, in-order locations rather than all matching the first occurrence.
102+
let cursor = searchFrom;
103+
90104
for (const [key, value] of Object.entries(obj)) {
91105
// Skip metadata properties (except when they contain the value)
92106
if (key.startsWith('_') && key !== '_value') {
@@ -97,7 +111,8 @@ export class JsonDocumentParser implements IResourceDocumentParser {
97111

98112
if (typeof value === 'string') {
99113
// Simple string value
100-
const location = this.findKeyLocation(text, document, prefix ? key : fullKey);
114+
const { endOffset, ...location } = this.findKeyLocation(text, document, key, cursor);
115+
if (location.lineNumber !== -1) cursor = endOffset;
101116
keys.push({
102117
key: fullKey,
103118
value,
@@ -107,7 +122,8 @@ export class JsonDocumentParser implements IResourceDocumentParser {
107122
if (this.isPluralObject(value)) {
108123
// Plural object
109124
const pluralForms = this.extractPluralForms(value);
110-
const location = this.findKeyLocation(text, document, key);
125+
const { endOffset, ...location } = this.findKeyLocation(text, document, key, cursor);
126+
if (location.lineNumber !== -1) cursor = endOffset;
111127
keys.push({
112128
key: fullKey,
113129
value: pluralForms['other'] || pluralForms['one'] || Object.values(pluralForms)[0] || '',
@@ -120,16 +136,24 @@ export class JsonDocumentParser implements IResourceDocumentParser {
120136
const valueObj = value as Record<string, unknown>;
121137
const val = valueObj._value as string;
122138
const comment = valueObj._comment as string | undefined;
123-
const location = this.findKeyLocation(text, document, key);
139+
const { endOffset, ...location } = this.findKeyLocation(text, document, key, cursor);
140+
if (location.lineNumber !== -1) cursor = endOffset;
124141
keys.push({
125142
key: fullKey,
126143
value: val,
127144
comment,
128145
...location
129146
});
130147
} else {
131-
// Recurse into nested object
132-
this.extractKeys(value, fullKey, keys, text, document);
148+
// Recurse into nested object. Locate the parent key first so the
149+
// child scope is searched from inside the parent's braces — this
150+
// is what disambiguates duplicate simple names across scopes.
151+
const parent = this.findKeyLocation(text, document, key, cursor);
152+
if (parent.lineNumber !== -1) cursor = parent.endOffset;
153+
const childStart = parent.lineNumber !== -1
154+
? document.offsetAt(new vscode.Position(parent.lineNumber, parent.columnStart))
155+
: cursor;
156+
this.extractKeys(value, fullKey, keys, text, document, childStart);
133157
}
134158
}
135159
}
@@ -186,7 +210,8 @@ export class JsonDocumentParser implements IResourceDocumentParser {
186210
keys[existingIndex].pluralForms = pluralForms;
187211
} else {
188212
// Add as new plural key
189-
const location = this.findKeyLocation(text, document, `${baseKey}_one`);
213+
const { endOffset, ...location } = this.findKeyLocation(text, document, `${baseKey}_one`);
214+
void endOffset;
190215
keys.push({
191216
key: fullKey,
192217
value: pluralForms['other'] || pluralForms['one'] || Object.values(pluralForms)[0],
@@ -255,12 +280,17 @@ export class JsonDocumentParser implements IResourceDocumentParser {
255280
private findKeyLocation(
256281
text: string,
257282
document: vscode.TextDocument,
258-
key: string
259-
): { lineNumber: number; columnStart: number; columnEnd: number; comment?: string } {
260-
// Search for the key in the text
283+
key: string,
284+
searchFrom = 0
285+
): { lineNumber: number; columnStart: number; endLine: number; columnEnd: number; comment?: string; endOffset: number } {
286+
// Search for the key in the text starting at searchFrom. The caller advances
287+
// searchFrom past each key it has already consumed so that the same simple
288+
// name appearing in different nesting scopes (e.g. users.id vs posts.id)
289+
// resolves to its OWN occurrence rather than always the first global match.
261290
// Pattern: "key": "value" or "key": { ... }
262291
const escapedKey = this.escapeRegexChars(key);
263-
const keyPattern = new RegExp(`"${escapedKey}"\\s*:`);
292+
const keyPattern = new RegExp(`"${escapedKey}"\\s*:`, 'g');
293+
keyPattern.lastIndex = Math.max(0, searchFrom);
264294
const match = keyPattern.exec(text);
265295

266296
if (match) {
@@ -307,15 +337,23 @@ export class JsonDocumentParser implements IResourceDocumentParser {
307337
return {
308338
lineNumber: startPos.line,
309339
columnStart: startPos.character,
310-
columnEnd: endPos.character
340+
// Carry the real end line so multi-line values (multiline strings,
341+
// object values) produce a correct range instead of collapsing the
342+
// end onto the start line.
343+
endLine: endPos.line,
344+
columnEnd: endPos.character,
345+
endOffset
311346
};
312347
}
313348

314-
// Key not found - return defaults
349+
// Key not found. Use lineNumber -1 as a sentinel so callers (getKeyRange)
350+
// can return null instead of pointing at a bogus 0,0 location.
315351
return {
316-
lineNumber: 0,
352+
lineNumber: -1,
317353
columnStart: 0,
318-
columnEnd: 0
354+
endLine: -1,
355+
columnEnd: 0,
356+
endOffset: Math.max(0, searchFrom)
319357
};
320358
}
321359

vscode-extension/src/parsers/resourceDocumentParser.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ export interface ResourceKey {
1010
value: string;
1111
/** Optional comment/description */
1212
comment?: string;
13-
/** Line number in the document (0-based) */
13+
/** Line number in the document (0-based). -1 means the key could not be located. */
1414
lineNumber: number;
1515
/** Column start position */
1616
columnStart: number;
17+
/** End line number (0-based). Defaults to lineNumber when the value is single-line. */
18+
endLine?: number;
1719
/** Column end position */
1820
columnEnd: number;
1921
/** Whether this is a plural key */

vscode-extension/src/providers/codeLens.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,16 @@ export class LrmCodeLensProvider implements vscode.CodeLensProvider {
255255

256256
const keyName = usage.key;
257257

258+
// If any reference binds this key to a specific resource group via a Data
259+
// Annotation's ResourceType (e.g. typeof(GlassResources)), resolve details
260+
// against THAT group. Otherwise the key is group-agnostic and we fall back
261+
// to whichever group the backend resolved it against (its union default).
262+
// Without this, getKeyDetails(keyName) returns the first-cached group's
263+
// details, showing the wrong value/coverage in multi-group projects.
264+
const boundGroup = (usage.references || [])
265+
.map(r => r.resourceTypeClassName)
266+
.find(g => g && g.trim() !== '') || undefined;
267+
258268
for (const reference of usage.references || []) {
259269
// Backend reports 1-based line numbers; CodeLens positions are 0-based.
260270
const lineNumber = Math.max(0, (reference.line ?? 1) - 1);
@@ -281,7 +291,7 @@ export class LrmCodeLensProvider implements vscode.CodeLensProvider {
281291
}
282292

283293
try {
284-
const details = await this.cacheService.getKeyDetails(keyName);
294+
const details = await this.cacheService.getKeyDetails(keyName, false, boundGroup);
285295

286296
// Show value lens
287297
if (config.get<boolean>('codeLens.showValue', true)) {
@@ -305,7 +315,7 @@ export class LrmCodeLensProvider implements vscode.CodeLensProvider {
305315
}
306316

307317
// Show missing languages lens
308-
const missingLanguages = this.cacheService.getMissingLanguages(keyName);
318+
const missingLanguages = this.cacheService.getMissingLanguages(keyName, boundGroup);
309319
if (missingLanguages && missingLanguages.length > 0) {
310320
// Filter out 'default' from missing list
311321
const displayMissing = missingLanguages.filter(l => l !== 'default' && l !== '');

vscode-extension/src/providers/completionProvider.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,5 +239,9 @@ export class LocalizationCompletionProvider implements vscode.CompletionItemProv
239239
*/
240240
public invalidateCache(): void {
241241
this.keysCache = null;
242+
// Also drop the cached configuration: changes to lrm.json (resource class
243+
// names, localizationMethods) must take effect on the next completion rather
244+
// than waiting out the config TTL.
245+
this.configCache = null;
242246
}
243247
}

vscode-extension/src/providers/definitionProvider.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,11 @@ export class LrmDefinitionProvider implements vscode.DefinitionProvider {
228228
const match = pattern.exec(text);
229229

230230
if (match) {
231-
const pos = document.positionAt(match.index);
232-
return new vscode.Location(fileUri, pos);
231+
// Return a Range (not a bare Position) so Go-to-Definition
232+
// highlights the match, matching the parser path's behavior.
233+
const start = document.positionAt(match.index);
234+
const end = document.positionAt(match.index + match[0].length);
235+
return new vscode.Location(fileUri, new vscode.Range(start, end));
233236
}
234237
} else if (fileName.endsWith('.json')) {
235238
// JSON pattern: "KeyName":
@@ -240,8 +243,9 @@ export class LrmDefinitionProvider implements vscode.DefinitionProvider {
240243
const match = pattern.exec(text);
241244

242245
if (match) {
243-
const pos = document.positionAt(match.index);
244-
return new vscode.Location(fileUri, pos);
246+
const start = document.positionAt(match.index);
247+
const end = document.positionAt(match.index + match[0].length);
248+
return new vscode.Location(fileUri, new vscode.Range(start, end));
245249
}
246250
}
247251
} catch (error) {

vscode-extension/src/providers/resxDiagnostics.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,10 @@ export class ResxDiagnosticProvider {
126126

127127
try {
128128
const document = await vscode.workspace.openTextDocument(fileUri);
129-
const fileName = fileUri.fsPath.substring(fileUri.fsPath.lastIndexOf('/') + 1);
129+
// Use path.basename so this works on Windows too: fsPath uses backslashes
130+
// there, and lastIndexOf('/') would return -1 and yield the whole path,
131+
// breaking language-code extraction (no diagnostics on Windows).
132+
const fileName = path.basename(fileUri.fsPath);
130133

131134
// Determine language code from filename
132135
// e.g., Resources.el.resx -> el, Resources.resx -> default

0 commit comments

Comments
 (0)