Skip to content

Commit 3f76ac7

Browse files
committed
fix: resolve parser and utility edge case bugs
## Changed - TS parser brace counting now skips string literals and comments, preventing file corruption with `{name}` placeholders - Android XML and PO parsers use `??` instead of `||` to preserve falsy values like `0` and `""` - deepMerge replaces arrays from source instead of concatenating, preventing unbounded growth - Android XML no longer collapses single-item arrays unnecessarily - Checksum calculation detects deleted source keys and marks them for removal from target files - Translation engine filters out deleted keys so stale translations are cleaned up
1 parent 7fa7604 commit 3f76ac7

11 files changed

Lines changed: 196 additions & 24 deletions

File tree

src/__tests__/parsers/android-xml.parser.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -930,6 +930,37 @@ describe('AndroidXmlParser', () => {
930930
});
931931
});
932932

933+
describe('falsy value handling', () => {
934+
it('should parse numeric zero string value correctly', () => {
935+
const content = `<?xml version="1.0" encoding="utf-8"?>
936+
<resources>
937+
<string name="zero">0</string>
938+
<string name="hello">Hello</string>
939+
</resources>`;
940+
const result = parser.parse(content);
941+
942+
expect(result).toEqual({
943+
zero: 0,
944+
hello: 'Hello',
945+
});
946+
});
947+
948+
it('should round-trip serialize data with falsy values', () => {
949+
const originalContent = `<?xml version="1.0" encoding="utf-8"?>
950+
<resources>
951+
<string name="zero">0</string>
952+
<string name="empty">text</string>
953+
</resources>`;
954+
const data = {
955+
zero: 0,
956+
empty: '',
957+
};
958+
const result = parser.serialize(data, { originalContent } as AndroidXmlParserOptionsType);
959+
expect(result).toContain('<string name="zero">0</string>');
960+
expect(result).toContain('<string name="empty"></string>');
961+
});
962+
});
963+
933964
describe('getFallback', () => {
934965
it('should return default XML structure', () => {
935966
const result = parser.getFallback();

src/__tests__/parsers/po.parser.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,36 @@ describe('PoParser', () => {
795795
});
796796
});
797797

798+
describe('falsy value handling', () => {
799+
it('should serialize numeric zero value as "0" not empty string', () => {
800+
const originalContent = `
801+
msgid ""
802+
msgstr ""
803+
"Content-Type: text/plain; charset=UTF-8\\n"
804+
805+
msgid "Hello"
806+
msgstr "Ciao"
807+
`;
808+
809+
parser.parse(originalContent);
810+
811+
const data: Record<string, unknown> = {};
812+
const key = JSON.stringify({
813+
msgid: 'Hello',
814+
msgctxt: undefined,
815+
msgid_plural: undefined,
816+
idx: 0,
817+
order: 0,
818+
});
819+
data[key] = 0;
820+
821+
const result = parser.serialize(data, { targetLocale: 'fr' });
822+
const resultStr = result.toString();
823+
824+
expect(resultStr).toContain('msgstr "0"');
825+
});
826+
});
827+
798828
describe('getFallback', () => {
799829
it('should return default PO template', () => {
800830
const result = parser.getFallback();

src/__tests__/parsers/ts.parser.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,22 @@ describe('TsParser', () => {
448448
});
449449
});
450450

451+
describe('brace counting in string context', () => {
452+
it('should round-trip values containing braces without corruption', () => {
453+
const originalContent =
454+
'const messages = { en: { greeting: "Hello {name}, you have {count} items" } };\n\nexport default messages;';
455+
const parsed = parser.parse(originalContent, { targetLocale: 'en' } as any);
456+
457+
expect(parsed).toEqual({ greeting: 'Hello {name}, you have {count} items' });
458+
459+
const serialized = parser.serialize(parsed, { originalContent, targetLocale: 'en' });
460+
const resultStr = serialized.toString();
461+
const match = resultStr.match(/const\s+messages\s*=\s*({[\s\S]*?});/);
462+
const reparsed = JSON.parse(match?.[1] || '{}');
463+
expect(reparsed.en.greeting).toBe('Hello {name}, you have {count} items');
464+
});
465+
});
466+
451467
describe('getFallback', () => {
452468
it('should return default TypeScript template', () => {
453469
const result = parser.getFallback();

src/__tests__/utils/checksum.test.ts

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
22
import * as fs from 'fs';
33
import * as yaml from 'yaml';
44
import * as crypto from 'crypto';
5-
import { calculateChecksum } from '#utils/checksum.js';
5+
import { calculateChecksum, resetChecksumCache } from '#utils/checksum.js';
66
import { ParserFactory } from '../../parsers/parser.factory.js';
77

88
// Helper function to calculate hash
@@ -26,6 +26,7 @@ describe('checksum utils', () => {
2626

2727
beforeEach(() => {
2828
vi.clearAllMocks();
29+
resetChecksumCache();
2930
vi.spyOn(process, 'cwd').mockReturnValue('/mock/path');
3031
});
3132

@@ -243,7 +244,15 @@ describe('checksum utils', () => {
243244
const result = calculateChecksum(mockFileName, mockParser as unknown as ParserFactory, '');
244245

245246
expect(result).toEqual({});
246-
expect(fs.writeFileSync).not.toHaveBeenCalled();
247+
// writeFileSync may be called once for initial checksum file creation,
248+
// but updateChecksum should not be called (no changes detected)
249+
const writeCalls = vi.mocked(fs.writeFileSync).mock.calls;
250+
// If called, it should only be the initial file creation, not an update
251+
for (const call of writeCalls) {
252+
const content = call[1] as string;
253+
// The initial creation writes an empty files object
254+
expect(content).not.toContain(mockFileName);
255+
}
247256
});
248257

249258
it('should handle file with no existing checksum entry', () => {
@@ -284,6 +293,48 @@ describe('checksum utils', () => {
284293
expect(fs.writeFileSync).toHaveBeenCalled();
285294
});
286295

296+
it('should detect deleted keys with state deleted', () => {
297+
// Use a unique file name to avoid cache conflicts
298+
const uniqueFileName = 'test/deleted-keys.json';
299+
// File now only has key1 and key2, but checksum has key1, key2, key3
300+
const fileContent = {
301+
key1: 'value1',
302+
key2: 'value2',
303+
};
304+
305+
const mockParser = {
306+
parse: vi.fn().mockReturnValue(fileContent),
307+
};
308+
309+
const fileNameHash = getHash(uniqueFileName);
310+
const existingChecksum = {
311+
version: '1.0.0',
312+
files: {
313+
[fileNameHash]: {
314+
key1: getHash('value1'),
315+
key2: getHash('value2'),
316+
key3: getHash('value3'),
317+
},
318+
},
319+
};
320+
321+
vi.mocked(ParserFactory).mockImplementation(() => mockParser as unknown as void);
322+
vi.mocked(fs.existsSync).mockReturnValue(true);
323+
vi.mocked(fs.readFileSync).mockReturnValue('mock yaml content');
324+
vi.mocked(yaml.parse).mockReturnValue(existingChecksum);
325+
vi.mocked(yaml.stringify).mockReturnValue('version: 1.0.0\nfiles: {}');
326+
327+
const result = calculateChecksum(uniqueFileName, mockParser as unknown as ParserFactory, '');
328+
329+
expect(result).toEqual({
330+
key1: { value: 'value1', state: 'unchanged' },
331+
key2: { value: 'value2', state: 'unchanged' },
332+
key3: { value: null, state: 'deleted' },
333+
});
334+
// changed should be true because of the deleted key
335+
expect(fs.writeFileSync).toHaveBeenCalled();
336+
});
337+
287338
it('should handle object values correctly when hashing', () => {
288339
// Use a unique file name to avoid cache conflicts
289340
const uniqueFileName = 'test/object-values.json';

src/__tests__/utils/parser.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ describe('parser utils', () => {
8888
});
8989
});
9090

91-
it('should merge arrays instead of replacing them', () => {
91+
it('should replace arrays from source instead of concatenating', () => {
9292
const target = {
9393
items: [1, 2, 3],
9494
nested: {
@@ -104,9 +104,9 @@ describe('parser utils', () => {
104104
const result = deepMerge(target, source);
105105

106106
expect(result).toEqual({
107-
items: [1, 2, 3, 4, 5],
107+
items: [4, 5],
108108
nested: {
109-
arr: ['a', 'b', 'c'],
109+
arr: ['c'],
110110
},
111111
});
112112
});
@@ -209,7 +209,7 @@ describe('parser utils', () => {
209209
string: 'world',
210210
number: 100,
211211
boolean: false,
212-
array: [1, 2, 3, 4, 5],
212+
array: [3, 4, 5],
213213
},
214214
});
215215
});

src/modules/translation/translation.engine.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ export class TranslationEngine {
174174
await Promise.all(
175175
Object.entries(changelog)
176176
.filter(([key]) => !this.isIgnored(TranslationEngine.toUserKey(key)))
177+
.filter(([, value]) => value.state !== 'deleted')
177178
.map(async ([key, value]) => {
178179
const userKey = TranslationEngine.toUserKey(key);
179180
const state = value.state;

src/parsers/android-xml.parser.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ export class AndroidXmlParser implements Parser<Record<string, unknown>, Android
151151
// Skip non-translatable strings
152152
if (str['@_translatable'] === 'false') continue;
153153

154-
const value = str['#text'] || '';
154+
const value = str['#text'] ?? '';
155155
translations[name] = value;
156156
}
157157
}
@@ -169,7 +169,7 @@ export class AndroidXmlParser implements Parser<Record<string, unknown>, Android
169169
for (const item of items) {
170170
const quantity = item['@_quantity'];
171171
if (!quantity) continue;
172-
const value = item['#text'] || '';
172+
const value = item['#text'] ?? '';
173173
const key = `${name}/${quantity}`;
174174
translations[key] = value;
175175
}
@@ -194,7 +194,7 @@ export class AndroidXmlParser implements Parser<Record<string, unknown>, Android
194194
const item = items[i];
195195
if (item === undefined) continue;
196196
// Handle both object format ({'#text': 'value'}) and string format ('value')
197-
const value = typeof item === 'string' ? item : (item['#text'] || '');
197+
const value = typeof item === 'string' ? item : (item['#text'] ?? '');
198198
const key = `${name}/${i}`;
199199
translations[key] = value;
200200
}
@@ -290,7 +290,7 @@ export class AndroidXmlParser implements Parser<Record<string, unknown>, Android
290290
if (str['@_translatable'] === 'false') continue;
291291

292292
if (data[name] !== undefined) {
293-
str['#text'] = String(data[name] || '');
293+
str['#text'] = String(data[name] ?? '');
294294
}
295295
}
296296
}
@@ -308,7 +308,7 @@ export class AndroidXmlParser implements Parser<Record<string, unknown>, Android
308308
if (!quantity) continue;
309309
const key = `${name}/${quantity}`;
310310
if (data[key] !== undefined) {
311-
item['#text'] = String(data[key] || '');
311+
item['#text'] = String(data[key] ?? '');
312312
}
313313
}
314314
}
@@ -336,15 +336,15 @@ export class AndroidXmlParser implements Parser<Record<string, unknown>, Android
336336
// Handle both object format and string format
337337
if (typeof item === 'string') {
338338
// Convert string to object format
339-
items[i] = { '#text': String(data[key] || '') };
339+
items[i] = { '#text': String(data[key] ?? '') };
340340
} else if (item && typeof item === 'object') {
341-
item['#text'] = String(data[key] || '');
341+
item['#text'] = String(data[key] ?? '');
342342
}
343343
}
344344
}
345345
// Update the items array back to the stringArray
346346
if (items.length > 0) {
347-
stringArray.item = items.length === 1 ? items[0] : items;
347+
stringArray.item = items;
348348
}
349349
}
350350
}
@@ -403,8 +403,8 @@ export class AndroidXmlParser implements Parser<Record<string, unknown>, Android
403403
const str = entry.resource;
404404
const name = str['@_name'];
405405
const translatable = str['@_translatable'];
406-
const value = str['#text'] || '';
407-
406+
const value = str['#text'] ?? '';
407+
408408
const escapedName = this.escapeTextContent(String(name));
409409
const escapedValue = this.escapeTextContent(value);
410410

@@ -429,7 +429,7 @@ export class AndroidXmlParser implements Parser<Record<string, unknown>, Android
429429
const quantity = item['@_quantity'];
430430
if (!quantity) continue;
431431

432-
const value = item['#text'] || '';
432+
const value = item['#text'] ?? '';
433433

434434
const escapedQuantity = this.escapeTextContent(String(quantity));
435435
const escapedValue = this.escapeTextContent(value);
@@ -457,7 +457,7 @@ export class AndroidXmlParser implements Parser<Record<string, unknown>, Android
457457

458458
for (const item of items) {
459459
// Handle both object format ({'#text': 'value'}) and string format ('value')
460-
const value = typeof item === 'string' ? item : (item['#text'] || '');
460+
const value = typeof item === 'string' ? item : (item['#text'] ?? '');
461461
const escapedValue = this.escapeTextContent(value);
462462

463463
lines.push(`${indent}${indent}<item>${escapedValue}</item>`);

src/parsers/po.parser.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ export class PoParser implements Parser<Record<string, unknown>, PoParserOptions
222222
const msgstrValue =
223223
typeof data[serializedKey] === 'string'
224224
? (data[serializedKey] as string)
225-
: String(data[serializedKey] || '');
225+
: String(data[serializedKey] ?? '');
226226

227227
const context = keyObj.msgctxt || '';
228228

src/parsers/ts.parser.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,36 @@ export class TsParser implements Parser<Record<string, unknown>, TsParserOptions
203203

204204
for (let i = startIndex; i < content.length; i++) {
205205
const char = content[i];
206+
207+
// Skip single-line comments
208+
if (char === '/' && content[i + 1] === '/') {
209+
const newlineIdx = content.indexOf('\n', i);
210+
i = newlineIdx === -1 ? content.length - 1 : newlineIdx;
211+
continue;
212+
}
213+
214+
// Skip block comments
215+
if (char === '/' && content[i + 1] === '*') {
216+
const closeIdx = content.indexOf('*/', i + 2);
217+
i = closeIdx === -1 ? content.length - 1 : closeIdx + 1;
218+
continue;
219+
}
220+
221+
// Skip string literals (single quote, double quote, backtick)
222+
if (char === "'" || char === '"' || char === '`') {
223+
const quote = char;
224+
i++;
225+
while (i < content.length) {
226+
if (content[i] === '\\') {
227+
i++; // skip escaped character
228+
} else if (content[i] === quote) {
229+
break;
230+
}
231+
i++;
232+
}
233+
continue;
234+
}
235+
206236
if (char === '{') {
207237
braceCount++;
208238
foundStart = true;

src/utils/checksum.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@ enum ChecksumState {
88
NEW = 'new',
99
UPDATED = 'updated',
1010
UNCHANGED = 'unchanged',
11+
DELETED = 'deleted',
1112
}
1213

1314
type ChecksumChangelog = Record<
1415
string,
1516
{
1617
value: unknown;
17-
state: 'new' | 'updated' | 'unchanged';
18+
state: 'new' | 'updated' | 'unchanged' | 'deleted';
1819
}
1920
>;
2021

@@ -84,6 +85,14 @@ function calculateChecksum(
8485
continue;
8586
}
8687

88+
// Detect deleted keys: keys present in checksum but no longer in file
89+
for (const key in checksum) {
90+
if (!(key in fileContent)) {
91+
changelog[key] = { value: null, state: ChecksumState.DELETED };
92+
changed = true;
93+
}
94+
}
95+
8796
if (changed) {
8897
updateChecksum(fileName, fileContent);
8998
}
@@ -147,4 +156,8 @@ function getHash(s: unknown) {
147156
return crypto.createHash('md5').update(data).digest('hex');
148157
}
149158

150-
export { calculateChecksum };
159+
function resetChecksumCache() {
160+
cachedChecksumFile = null;
161+
}
162+
163+
export { calculateChecksum, resetChecksumCache };

0 commit comments

Comments
 (0)