Skip to content

Commit fc0625d

Browse files
charlespwdclaude
andcommitted
Address PR review feedback for rename boundary fix
Use indexOf for test positions instead of hardcoded numbers, clarify comment about @param keyword/type annotation exclusion, and expand negative tests to cover @param keyword and space before param name. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent cd0bec1 commit fc0625d

2 files changed

Lines changed: 34 additions & 11 deletions

File tree

packages/theme-language-server-common/src/rename/providers/LiquidVariableRenameProvider.spec.ts

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ describe('LiquidVariableRenameProvider', () => {
159159
it('returns new name when cursor is at first character of liquid doc param name', async () => {
160160
const params = {
161161
textDocument,
162-
position: Position.create(0, 25),
162+
position: Position.create(0, documentSource.indexOf('food')),
163163
newName: 'meal',
164164
};
165165

@@ -181,6 +181,17 @@ describe('LiquidVariableRenameProvider', () => {
181181
);
182182
});
183183

184+
it('does not trigger rename when cursor is on type annotation', async () => {
185+
const params = {
186+
textDocument,
187+
position: Position.create(0, documentSource.indexOf('{string}') + 1),
188+
newName: 'meal',
189+
};
190+
191+
const result = await provider.rename(params);
192+
expect(result).to.be.null;
193+
});
194+
184195
it('returns new name after liquid doc param is renamed on variable usage', async () => {
185196
const params = {
186197
textDocument,
@@ -354,7 +365,7 @@ describe('LiquidVariableRenameProvider', () => {
354365
it('returns new name when cursor is at first character of untyped param name', async () => {
355366
const params = {
356367
textDocument,
357-
position: Position.create(0, 16),
368+
position: Position.create(0, documentSource.indexOf('prod')),
358369
newName: 'ppp',
359370
};
360371
const result = await provider.rename(params);
@@ -372,15 +383,27 @@ describe('LiquidVariableRenameProvider', () => {
372383
);
373384
});
374385

375-
it('does not trigger rename when cursor is at @ symbol position', async () => {
376-
const params = {
377-
textDocument,
378-
position: Position.create(0, 9),
379-
newName: 'ppp',
380-
};
386+
[
387+
{ desc: 'at @ symbol', position: Position.create(0, documentSource.indexOf('@param')) },
388+
{
389+
desc: 'at @param keyword',
390+
position: Position.create(0, documentSource.indexOf('@param') + 1),
391+
},
392+
{
393+
desc: 'at space before param name',
394+
position: Position.create(0, documentSource.indexOf('@param') + '@param'.length),
395+
},
396+
].forEach(({ desc, position }) => {
397+
it(`does not trigger rename when cursor is ${desc}`, async () => {
398+
const params = {
399+
textDocument,
400+
position,
401+
newName: 'ppp',
402+
};
381403

382-
const result = await provider.rename(params);
383-
expect(result).to.be.null;
404+
const result = await provider.rename(params);
405+
expect(result).to.be.null;
406+
});
384407
});
385408

386409
it('returns new name after variable is renamed inside loop, but is scoped outside it', async () => {

packages/theme-language-server-common/src/rename/providers/LiquidVariableRenameProvider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export class LiquidVariableRenameProvider implements BaseRenameProvider {
8080
if (document.ast instanceof Error) return null;
8181
if (!supportedTags(node, ancestors)) return null;
8282

83-
// When node is a LiquidDocParamNode, ensure cursor is on the param name, not the @ prefix
83+
// When node is a LiquidDocParamNode, ensure cursor is on the param name, not the @param keyword or type annotation
8484
if (node.type === NodeTypes.LiquidDocParamNode) {
8585
const cursorOffset = textDocument.offsetAt(params.position);
8686
const nameEnd = node.paramName.position.start + node.paramName.value.length;

0 commit comments

Comments
 (0)