Fix rename cursor boundary for LiquidDoc param names

.changeset/fix-rename-boundary.md

@@ -0,0 +1,5 @@
+---
+"@shopify/theme-language-server-common": patch
+---
+
+Fix rename not triggering when cursor is at the first character of a LiquidDoc param name

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

@@ -156,6 +156,68 @@ describe('LiquidVariableRenameProvider', () => {
       );
     });
 
+    it('does not rename type annotation when type and param name share the same text', async () => {
+      const collisionSource = `{% doc %}@param {food} food - favorite food{% enddoc %}
+{% liquid
+  echo food
+%}`;
+      const collisionDoc = TextDocument.create(textDocumentUri, 'liquid', 1, collisionSource);
+      documentManager.open(collisionDoc.uri, collisionSource, 1);
+
+      const params = {
+        textDocument: collisionDoc,
+        position: Position.create(0, collisionSource.indexOf('} food') + 2),
+        newName: 'meal',
+      };
+
+      const result = await provider.rename(params);
+      assert(result);
+      assert(result.documentChanges);
+      expect((result.documentChanges[0] as TextDocumentEdit).edits).to.applyEdits(
+        collisionDoc,
+        `{% doc %}@param {food} meal - favorite food{% enddoc %}
+{% liquid
+  echo meal
+%}`,
+      );
+    });
+
+    it('returns new name when cursor is at first character of liquid doc param name', async () => {
+      const params = {
+        textDocument,
+        position: Position.create(0, documentSource.indexOf('food')),
+        newName: 'meal',
+      };
+
+      const result = await provider.rename(params);
+      assert(result);
+      assert(result.documentChanges);
+      expect((result.documentChanges[0] as TextDocumentEdit).edits).to.applyEdits(
+        textDocument,
+        `{% doc %}@param {string} meal - favorite food{% enddoc %}
+{% assign animal = 'dog' %}
+{% assign plant = 'cactus' %}
+{% liquid
+  echo plant
+  echo meal
+%}
+{% assign painting = 'mona lisa' %}
+{% assign paintings = 'starry night, sunday afternoon, the scream' %}
+<p>I have a cool animal, a great plant, and my favorite food</p>`,
+      );
+    });
+
+    it('does not trigger rename when cursor is on type annotation', async () => {
+      const params = {
+        textDocument,
+        position: Position.create(0, documentSource.indexOf('{string}') + 1),
+        newName: 'meal',
+      };
+
+      const result = await provider.rename(params);
+      expect(result).to.be.null;
+    });
+
     it('returns new name after liquid doc param is renamed on variable usage', async () => {
       const params = {
         textDocument,
@@ -326,6 +388,50 @@ describe('LiquidVariableRenameProvider', () => {
         );
       });
 
+      it('returns new name when cursor is at first character of untyped param name', async () => {
+        const params = {
+          textDocument,
+          position: Position.create(0, documentSource.indexOf('prod')),
+          newName: 'ppp',
+        };
+        const result = await provider.rename(params);
+        assert(result);
+        assert(result.documentChanges);
+        expect((result.documentChanges[0] as TextDocumentEdit).edits).to.applyEdits(
+          textDocument,
+          `{% doc %}@param ppp - product{% enddoc %}
+{% liquid
+  for prod in products
+    echo prod.title
+  endfor
+  echo ppp
+%}`,
+        );
+      });
+
+      [
+        { desc: 'at @ symbol', position: Position.create(0, documentSource.indexOf('@param')) },
+        {
+          desc: 'at @param keyword',
+          position: Position.create(0, documentSource.indexOf('@param') + 1),
+        },
+        {
+          desc: 'at space before param name',
+          position: Position.create(0, documentSource.indexOf('@param') + '@param'.length),
+        },
+      ].forEach(({ desc, position }) => {
+        it(`does not trigger rename when cursor is ${desc}`, async () => {
+          const params = {
+            textDocument,
+            position,
+            newName: 'ppp',
+          };
+
+          const result = await provider.rename(params);
+          expect(result).to.be.null;
+        });
+      });
+
       it('returns new name after variable is renamed inside loop, but is scoped outside it', async () => {
         const params = {
           textDocument,

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

@@ -1,6 +1,7 @@
 import {
   AssignMarkup,
   ForMarkup,
+  LiquidDocParamNode,
   LiquidHtmlNode,
   LiquidTagFor,
   LiquidTagTablerow,
@@ -48,15 +49,18 @@ export class LiquidVariableRenameProvider implements BaseRenameProvider {
     if (!textDocument || !node || !ancestors) return null;
     if (!supportedTags(node, ancestors)) return null;
 
+    const targetNode = node.type === NodeTypes.LiquidDocParamNode ? node.paramName : node;
     const oldName = variableName(node);
-    const offsetOfVariableNameEnd = node.position.start + oldName.length;
+    const cursorOffset = textDocument.offsetAt(params.position);
+    const offsetOfVariableNameEnd = targetNode.position.start + oldName.length;
 
-    // The cursor could be past the end of the variable name
-    if (textDocument.offsetAt(params.position) > offsetOfVariableNameEnd) return null;
+    // The cursor must be within the variable name range
+    if (cursorOffset < targetNode.position.start || cursorOffset > offsetOfVariableNameEnd)
+      return null;
 
     return {
       range: Range.create(
-        textDocument.positionAt(node.position.start),
+        textDocument.positionAt(targetNode.position.start),
         textDocument.positionAt(offsetOfVariableNameEnd),
       ),
       placeholder: oldName,
@@ -76,6 +80,13 @@ export class LiquidVariableRenameProvider implements BaseRenameProvider {
     if (document.ast instanceof Error) return null;
     if (!supportedTags(node, ancestors)) return null;
 
+    // When node is a LiquidDocParamNode, ensure cursor is on the param name, not the @param keyword or type annotation
+    if (node.type === NodeTypes.LiquidDocParamNode) {
+      const cursorOffset = textDocument.offsetAt(params.position);
+      const nameEnd = node.paramName.position.start + node.paramName.value.length;
+      if (cursorOffset < node.paramName.position.start || cursorOffset > nameEnd) return null;
+    }
+
     const oldName = variableName(node);
     const scope = variableNameBlockScope(oldName, ancestors);
     const replaceRange = textReplaceRange(oldName, textDocument, scope);
@@ -87,7 +98,9 @@ export class LiquidVariableRenameProvider implements BaseRenameProvider {
       AssignMarkup: replaceRange,
       ForMarkup: replaceRange,
       TextNode: (node: LiquidHtmlNode, ancestors: (LiquidHtmlNode | JSONNode)[]) => {
-        if (ancestors.at(-1)?.type !== NodeTypes.LiquidDocParamNode) return;
+        const parentNode = ancestors.at(-1);
+        if (parentNode?.type !== NodeTypes.LiquidDocParamNode || parentNode?.paramName !== node)
+          return;
 
         liquidDocParamUpdated = true;
 
@@ -117,12 +130,13 @@ export class LiquidVariableRenameProvider implements BaseRenameProvider {
 function supportedTags(
   node: LiquidHtmlNode,
   ancestors: LiquidHtmlNode[],
-): node is AssignMarkup | LiquidVariableLookup | ForMarkup | TextNode {
+): node is AssignMarkup | LiquidVariableLookup | ForMarkup | TextNode | LiquidDocParamNode {
   return (
     node.type === NodeTypes.AssignMarkup ||
     node.type === NodeTypes.VariableLookup ||
     node.type === NodeTypes.ForMarkup ||
-    isLiquidDocParamNameNode(node, ancestors)
+    isLiquidDocParamNameNode(node, ancestors) ||
+    isLiquidDocParamNode(node)
   );
 }
 
@@ -140,6 +154,10 @@ function isLiquidDocParamNameNode(
   );
 }
 
+function isLiquidDocParamNode(node: LiquidHtmlNode): node is LiquidDocParamNode {
+  return node.type === NodeTypes.LiquidDocParamNode && node.paramName?.type === NodeTypes.TextNode;
+}
+
 function variableName(node: LiquidHtmlNode): string {
   switch (node.type) {
     case NodeTypes.VariableLookup:
@@ -149,6 +167,8 @@ function variableName(node: LiquidHtmlNode): string {
       return node.variableName ?? '';
     case NodeTypes.TextNode:
       return node.value;
+    case NodeTypes.LiquidDocParamNode:
+      return node.paramName.value;
     default:
       return '';
   }