Skip to content

Commit b0ec9ae

Browse files
committed
fix(filesystem): preserve literal $ in edit_file newText (closes #4157)
The prior code passed `normalizedNew` as String.prototype.replace's replacement argument. JS interprets `$$`, `$&`, `$'`, `$``, and `$n` as replacement patterns in that form, so any actual `$` characters in the user's newText - price tags, regex backreferences in docs, shell examples - get expanded or dropped. The callback form (`replace(..., () => string)`) disables pattern interpretation, so the string is written verbatim. Two regression tests pin the callback form. Closes #4157.
1 parent b1e1eb1 commit b0ec9ae

2 files changed

Lines changed: 35 additions & 1 deletion

File tree

src/filesystem/__tests__/lib.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,40 @@ describe('Lib Functions', () => {
440440
);
441441
});
442442

443+
it("preserves literal $ characters in newText (regression for #4157)", async () => {
444+
// String.prototype.replace interprets $$, $&, $`, $' in the
445+
// replacement argument when it is a string. The callback form
446+
// disables that interpretation. This test pins the callback form
447+
// so the bug can't silently regress.
448+
mockFs.readFile.mockResolvedValue("price: PLACEHOLDER\n");
449+
const edits = [
450+
{ oldText: "PLACEHOLDER", newText: "$$100 USD" }
451+
];
452+
mockFs.rename.mockResolvedValueOnce(undefined);
453+
await applyFileEdits("/test/file.txt", edits, false);
454+
expect(mockFs.writeFile).toHaveBeenCalledWith(
455+
expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/),
456+
"price: $$100 USD\n",
457+
"utf-8"
458+
);
459+
});
460+
461+
it("preserves $&, $` and $' replacement-pattern tokens in newText (#4157)", async () => {
462+
mockFs.readFile.mockResolvedValue("TARGET\n");
463+
const edits = [
464+
{ oldText: "TARGET", newText: "a $& b $` c $' d" }
465+
];
466+
mockFs.rename.mockResolvedValueOnce(undefined);
467+
await applyFileEdits("/test/file.txt", edits, false);
468+
// Without the fix: $& would expand to "TARGET", $` to "" (before-match),
469+
// and $' to "" (after-match), corrupting the output.
470+
expect(mockFs.writeFile).toHaveBeenCalledWith(
471+
expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/),
472+
"a $& b $` c $' d\n",
473+
"utf-8"
474+
);
475+
});
476+
443477
it('handles dry run mode', async () => {
444478
const edits = [
445479
{ oldText: 'line2', newText: 'modified line2' }

src/filesystem/lib.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ export async function applyFileEdits(
207207

208208
// If exact match exists, use it
209209
if (modifiedContent.includes(normalizedOld)) {
210-
modifiedContent = modifiedContent.replace(normalizedOld, normalizedNew);
210+
modifiedContent = modifiedContent.replace(normalizedOld, () => normalizedNew);
211211
continue;
212212
}
213213

0 commit comments

Comments
 (0)