Merge pull request #735 from Shopify/jm/hover_opt_param

[LiquidDoc] Hover: Wrap optional parameter names in `[]`

Author: jamesmengo

.changeset/shiny-toys-appear.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme-language-server-common': minor
+---
+
+Render `[]` around name of optional liquidDoc parameters when hovering over `{% render snip█pet %}` tag

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

@@ -16,57 +16,42 @@ 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,
         },
         {
           name: 'no-type',
           description: 'This parameter has no type',
           type: null,
+          required: true,
         },
         {
           name: 'no-description',
           description: null,
           type: 'string',
+          required: true,
         },
         {
           name: 'no-type-or-description',
           description: null,
           type: null,
+          required: true,
         },
       ],
     },
   };
 
-  const createProvider = (getSnippetDefinition: GetSnippetDefinitionForURI) => {
-    return new HoverProvider(
-      new DocumentManager(),
-      {
-        filters: async () => [],
-        objects: async () => [],
-        tags: async () => [],
-        systemTranslations: async () => ({}),
-      },
-      async (_rootUri: string) => ({} as MetafieldDefinitionMap),
-      async () => ({}),
-      async () => [],
-      getSnippetDefinition,
-    );
-  };
-
-  beforeEach(async () => {
-    getSnippetDefinition = async () => mockSnippetDefinition;
-    provider = createProvider(getSnippetDefinition);
-  });
-
   describe('hover', () => {
     it('should return snippet definition with all parameters', async () => {
+      provider = createProvider(async () => mockSnippetDefinition);
       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\n- `no-type` - This parameter has no type\n- `no-description`: string\n- `no-type-or-description`',
+        '### product-card\n\n**Parameters:**\n- `title`: string - The title of the product\n- `border-radius` (Optional): number - The border radius in px\n- `no-type` - This parameter has no type\n- `no-description`: string\n- `no-type-or-description`',
       );
     });
 
@@ -86,5 +71,47 @@ describe('Module: RenderSnippetHoverProvider', async () => {
       await expect(provider).to.hover(`{% assign asdf = 'snip█pet' %}`, null);
       await expect(provider).to.hover(`{{ 'snip█pet' }}`, null);
     });
+
+    it('should wrap optional parameters in []', async () => {
+      provider = createProvider(async () => ({
+        name: 'product-card',
+        liquidDoc: {
+          parameters: [
+            {
+              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,
+            },
+          ],
+        },
+      }));
+      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` (Optional): number - The border radius in px',
+      );
+    });
   });
 });
+
+const createProvider = (getSnippetDefinition: GetSnippetDefinitionForURI) => {
+  return new HoverProvider(
+    new DocumentManager(),
+    {
+      filters: async () => [],
+      objects: async () => [],
+      tags: async () => [],
+      systemTranslations: async () => ({}),
+    },
+    async (_rootUri: string) => ({} as MetafieldDefinitionMap),
+    async () => ({}),
+    async () => [],
+    getSnippetDefinition,
+  );
+};

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}\` (Optional)`;
+        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
           @param {Number} paramWithNoDescription
@@ -46,26 +47,37 @@ 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: null,
             type: null,
+            required: true,
           },
           {
             name: 'paramWithNoDescription',
             description: null,
             type: 'Number',
+            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,
         };
       },
     },