Wrap optional parameters name in [] in hover tooltip

-----
Behaviour diverges slightly from JSDoc here. JSDoc does not indicate that it's optional, rather it implicitly adds `| undefined` to the types

Author: jamesmengo

packages/theme-language-server-common/src/hover/providers/RenderSnippetHoverProvider.spec.ts

@@ -15,11 +15,13 @@ describe('Module: RenderSnippetHoverProvider', async () => {
           name: 'title',
           description: 'The title of the product',
           type: 'string',
+          required: true,
         },
         {
           name: 'border-radius',
           description: 'The border radius in px',
           type: 'number',
+          required: false,
         },
       ],
     },
@@ -47,13 +49,6 @@ describe('Module: RenderSnippetHoverProvider', async () => {
   });
 
   describe('hover', () => {
-    it('should return snippet definition with all parameters', async () => {
-      await expect(provider).to.hover(
-        `{% render 'product-car█d' %}`,
-        '### product-card\n\n**Parameters:**\n- `title`: string - The title of the product\n- `border-radius`: number - The border radius in px',
-      );
-    });
-
     it('should return null if no LiquidDocDefinition found', async () => {
       getSnippetDefinition = async () => ({ name: 'unknown-snippet', liquidDoc: undefined });
       provider = createProvider(getSnippetDefinition);
@@ -70,5 +65,12 @@ describe('Module: RenderSnippetHoverProvider', async () => {
       await expect(provider).to.hover(`{% assign asdf = 'snip█pet' %}`, null);
       await expect(provider).to.hover(`{{ 'snip█pet' }}`, null);
     });
+
+    it('should return snippet definition with all parameters', async () => {
+      await expect(provider).to.hover(
+        `{% render 'product-car█d' %}`,
+        '### product-card\n\n**Parameters:**\n- `title`: string - The title of the product\n- `[border-radius]`: number - The border radius in px',
+      );
+    });
   });
 });

packages/theme-language-server-common/src/hover/providers/RenderSnippetHoverProvider.ts

@@ -50,13 +50,7 @@ export class RenderSnippetHoverProvider implements BaseHoverProvider {
     const parts = [`### ${snippetDefinition.name}`];
 
     if (liquidDoc.parameters?.length) {
-      const parameters = liquidDoc.parameters
-        ?.map(
-          ({ name, type, description }: LiquidDocParameter) =>
-            `- \`${name}\`${type ? `: ${type}` : ''} ${description ? `- ${description}` : ''}`,
-        )
-        .join('\n');
-
+      const parameters = this.buildParameters(liquidDoc.parameters);
       parts.push('', '**Parameters:**', parameters);
     }
 
@@ -67,4 +61,15 @@ export class RenderSnippetHoverProvider implements BaseHoverProvider {
       },
     };
   }
+
+  private buildParameters(parameters: LiquidDocParameter[]) {
+    return parameters
+      .map(({ name, type, description, required }: LiquidDocParameter) => {
+        const nameStr = required ? `\`${name}\`` : `\`[${name}]\``;
+        const typeStr = type ? `: ${type}` : '';
+        const descStr = description ? ` - ${description}` : '';
+        return `- ${nameStr}${typeStr}${descStr}`;
+      })
+      .join('\n');
+  }
 }

packages/theme-language-server-common/src/liquidDoc.spec.ts

@@ -4,7 +4,7 @@ import { toSourceCode } from '@shopify/theme-check-common';
 import { describe } from 'vitest';
 import { getSnippetDefinition } from './liquidDoc';
 
-describe('Unit: makeGetLiquidDocDefinitions', () => {
+describe('Unit: getSnippetDefinition', () => {
   function toAST(code: string) {
     return toSourceCode('/tmp/foo.liquid', code).ast as LiquidHtmlNode;
   }
@@ -31,6 +31,7 @@ describe('Unit: makeGetLiquidDocDefinitions', () => {
         {% doc %}
           @param {String} firstParam - The first param
           @param {Number} secondParam - The second param
+          @param {String} [optionalParam] - The optional param
           @param paramWithNoType - param with no type
           @param paramWithOnlyName
         {% enddoc %}
@@ -45,21 +46,31 @@ describe('Unit: makeGetLiquidDocDefinitions', () => {
             name: 'firstParam',
             description: 'The first param',
             type: 'String',
+            required: true,
           },
           {
             name: 'secondParam',
             description: 'The second param',
             type: 'Number',
+            required: true,
+          },
+          {
+            name: 'optionalParam',
+            description: 'The optional param',
+            type: 'String',
+            required: false,
           },
           {
             name: 'paramWithNoType',
             description: 'param with no type',
             type: null,
+            required: true,
           },
           {
             name: 'paramWithOnlyName',
             description: '',
             type: null,
+            required: true,
           },
         ],
       },

packages/theme-language-server-common/src/liquidDoc.ts

@@ -22,6 +22,7 @@ export type LiquidDocParameter = {
   name: string;
   description: string | null;
   type: string | null;
+  required: boolean;
 };
 
 export function getSnippetDefinition(
@@ -36,6 +37,7 @@ export function getSnippetDefinition(
           name: node.paramName.value,
           description: node.paramDescription?.value ?? null,
           type: node.paramType?.value ?? null,
+          required: node.required,
         };
       },
     },