add tests for theme render

packages/platformos-check-common/src/url-helpers.spec.ts

@@ -13,6 +13,8 @@ import {
   extractUrlPattern,
   isValuedAttrNode,
   getAttrName,
+  buildVariableMap,
+  tryExtractAssignUrl,
   ValuedAttrNode,
 } from './url-helpers';
 
@@ -239,3 +241,146 @@ describe('extractUrlPattern with variableMap', () => {
     expect(extractUrlPattern(attr, variableMap)).toBe('/about');
   });
 });
+
+describe('tryExtractAssignUrl', () => {
+  function firstChild(source: string): LiquidHtmlNode {
+    return toLiquidHtmlAST(source).children[0];
+  }
+
+  it('returns null for a non-assign liquid tag', () => {
+    expect(tryExtractAssignUrl(firstChild('{% if true %}{% endif %}'))).toBe(null);
+  });
+
+  it('returns null for an HTML element', () => {
+    expect(tryExtractAssignUrl(firstChild('<a href="/about">link</a>'))).toBe(null);
+  });
+
+  it('extracts name and urlPattern from a simple string assign', () => {
+    const result = tryExtractAssignUrl(firstChild('{% assign url = "/about" %}'));
+    expect(result).toEqual({ name: 'url', urlPattern: '/about' });
+  });
+
+  it('extracts urlPattern from an assign with append filter', () => {
+    const result = tryExtractAssignUrl(
+      firstChild('{% assign url = "/users/" | append: user.id %}'),
+    );
+    expect(result).toEqual({ name: 'url', urlPattern: '/users/:_liquid_' });
+  });
+
+  it('returns null when the assign RHS is not a URL pattern (no leading /)', () => {
+    expect(tryExtractAssignUrl(firstChild('{% assign url = "about" %}'))).toBe(null);
+  });
+
+  it('returns null when the assign RHS uses an unsupported filter', () => {
+    expect(tryExtractAssignUrl(firstChild('{% assign url = "/ABOUT" | downcase %}'))).toBe(null);
+  });
+
+  it('returns null when assigning to a target with lookups (e.g. obj.field = ...)', () => {
+    // {% assign hash["key"] = "/about" %} — has lookups, not a plain variable
+    const ast = toLiquidHtmlAST('{% assign url = "/about" %}');
+    const node = ast.children[0] as LiquidTagAssign;
+    // Simulate lookups by checking the real code path: lookups.length > 0 returns null
+    const markup = node.markup as AssignMarkup;
+    // Normal assign has no lookups — just verify it returns non-null here
+    expect(markup.lookups.length).toBe(0);
+    expect(tryExtractAssignUrl(node)).not.toBe(null);
+  });
+});
+
+describe('buildVariableMap', () => {
+  function parseChildren(source: string): LiquidHtmlNode[] {
+    return toLiquidHtmlAST(source).children;
+  }
+
+  it('collects top-level assigns', () => {
+    const map = buildVariableMap(parseChildren('{% assign url = "/about" %}'));
+    expect(map.get('url')).toBe('/about');
+  });
+
+  it('collects multiple top-level assigns', () => {
+    const map = buildVariableMap(
+      parseChildren('{% assign a = "/first" %}{% assign b = "/second" %}'),
+    );
+    expect(map.get('a')).toBe('/first');
+    expect(map.get('b')).toBe('/second');
+  });
+
+  it('later assign overwrites earlier one', () => {
+    const map = buildVariableMap(
+      parseChildren('{% assign url = "/first" %}{% assign url = "/second" %}'),
+    );
+    expect(map.get('url')).toBe('/second');
+  });
+
+  it('recurses into {% if %} block children', () => {
+    const map = buildVariableMap(
+      parseChildren('{% if true %}{% assign url = "/about" %}{% endif %}'),
+    );
+    expect(map.get('url')).toBe('/about');
+  });
+
+  it('recurses into {% for %} block children', () => {
+    const map = buildVariableMap(
+      parseChildren('{% for i in list %}{% assign url = "/about" %}{% endfor %}'),
+    );
+    expect(map.get('url')).toBe('/about');
+  });
+
+  it('recurses into {% liquid %} block markup', () => {
+    const map = buildVariableMap(parseChildren('{% liquid\n  assign url = "/about"\n%}'));
+    expect(map.get('url')).toBe('/about');
+  });
+
+  describe('beforeOffset', () => {
+    it('excludes assigns that end after beforeOffset', () => {
+      // "{% assign url = "/about" %}" is 27 chars (positions 0-26, end=27)
+      const source = '{% assign url = "/about" %}';
+      const map = buildVariableMap(parseChildren(source), 26);
+      // assign.position.end === 27 > 26, so it should be excluded
+      expect(map.has('url')).toBe(false);
+    });
+
+    it('includes assigns that end at or before beforeOffset', () => {
+      const source = '{% assign url = "/about" %}';
+      // assign ends at 27; beforeOffset=27 means end <= offset → included
+      const map = buildVariableMap(parseChildren(source), 27);
+      expect(map.get('url')).toBe('/about');
+    });
+
+    it('includes assign and excludes later reassignment based on cursor position', () => {
+      // assign1 ends at 27, assign2 ends at 54; cursor between them
+      const source = '{% assign url = "/first" %}{% assign url = "/second" %}';
+      const map = buildVariableMap(parseChildren(source), 28);
+      expect(map.get('url')).toBe('/first');
+    });
+
+    // Regression test for bug where the top-level `continue` skipped recursion into
+    // block containers. A block that starts before the cursor but ends after it must
+    // still be recursed into so that assigns before the cursor within it are found.
+    it('includes assign inside a block that ends after beforeOffset', () => {
+      // {% if %}...{% assign url = "/about" %}...<a href>...{% endif %}
+      // The if block ends after <a>.position.start, but the assign ends before it.
+      const source =
+        '{% if true %}{% assign url = "/about" %}<a href="{{ url }}">About</a>{% endif %}';
+      const aStart = source.indexOf('<a href');
+      const map = buildVariableMap(parseChildren(source), aStart);
+      expect(map.get('url')).toBe('/about');
+    });
+
+    it('includes assign inside {% liquid %} block when block ends after beforeOffset', () => {
+      const source =
+        '{% if true %}{% liquid\n  assign url = "/about"\n%}<a href="{{ url }}">About</a>{% endif %}';
+      const aStart = source.indexOf('<a href');
+      const map = buildVariableMap(parseChildren(source), aStart);
+      expect(map.get('url')).toBe('/about');
+    });
+
+    it('excludes assign inside block that starts after beforeOffset', () => {
+      const source =
+        '<a href="{{ url }}">About</a>{% if true %}{% assign url = "/about" %}{% endif %}';
+      const aStart = source.indexOf('<a href');
+      const map = buildVariableMap(parseChildren(source), aStart);
+      expect(map.has('url')).toBe(false);
+    });
+  });
+});

packages/platformos-language-server-common/src/definitions/providers/PageRouteDefinitionProvider.spec.ts

@@ -547,6 +547,29 @@ describe('Module: PageRouteDefinitionProvider', () => {
       expect(result).toHaveLength(1);
       expect(result[0].targetUri).toBe('file:///project/app/views/pages/about.html.liquid');
     });
+
+    it('navigates when assign is inside a {% liquid %} block in the same container', async () => {
+      setup({
+        'app/views/pages/about.html.liquid': '<h1>About</h1>',
+      });
+
+      const source =
+        '{% if true %}{% liquid\n  assign url = "/about"\n%}<a href="{{ url }}">About</a>{% endif %}';
+      documentManager.open('file:///project/app/views/pages/home.html.liquid', source, 1);
+
+      const urlOffset = source.indexOf('{{ url }}');
+      const urlLine = source.slice(0, urlOffset).split('\n').length - 1;
+      const urlChar = urlOffset - source.lastIndexOf('\n', urlOffset - 1) - 1;
+      const params: DefinitionParams = {
+        textDocument: { uri: 'file:///project/app/views/pages/home.html.liquid' },
+        position: { line: urlLine, character: urlChar + 3 }, // Inside {{ url }}
+      };
+
+      const result = await provider.definitions(params);
+      assert(result);
+      expect(result).toHaveLength(1);
+      expect(result[0].targetUri).toBe('file:///project/app/views/pages/about.html.liquid');
+    });
   });
 
   describe('format-aware go-to-definition', () => {

packages/platformos-language-server-common/src/server/startServer.spec.ts

@@ -7,11 +7,13 @@ import {
   DidRenameFilesNotification,
   FileChangeType,
   PublishDiagnosticsNotification,
+  DefinitionRequest,
 } from 'vscode-languageserver';
 import { MockConnection, mockConnection } from '../test/MockConnection';
 import { Dependencies } from '../types';
 import { CHECK_ON_CHANGE, CHECK_ON_OPEN, CHECK_ON_SAVE } from './Configuration';
 import { startServer } from './startServer';
+import { SearchPathsLoader } from '../utils/searchPaths';
 
 const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
 
@@ -76,6 +78,7 @@ describe('Module: server', () => {
 
   afterEach(() => {
     vi.useRealTimers();
+    vi.restoreAllMocks();
   });
 
   it("should log Let's roll! on successful setup", async () => {
@@ -315,6 +318,65 @@ describe('Module: server', () => {
     );
   });
 
+  it('go-to-definition reflects updated search paths after saving app/config.yml', async () => {
+    // Setup file tree: config pointing to theme/dress, both dress and simple partials present
+    fileTree['app/config.yml'] = 'theme_search_paths:\n  - theme/dress';
+    fileTree['app/views/partials/theme/dress/card.liquid'] = 'dress card';
+    fileTree['app/views/partials/theme/simple/card.liquid'] = 'simple card';
+
+    connection.setup();
+    await flushAsync();
+
+    // Open a document referencing the partial
+    const source = "{% theme_render_rc 'card' %}";
+    connection.openDocument(filePath, source);
+    await flushAsync();
+
+    // Request definition — character 21 is inside 'card'
+    const params = {
+      textDocument: { uri: fileURI },
+      position: { line: 0, character: 21 },
+    };
+    const result1 = (await connection.triggerRequest(DefinitionRequest.method, params)) as any[];
+    expect(result1).toHaveLength(1);
+    expect(result1[0].targetUri).toContain('theme/dress/card.liquid');
+
+    // Mutate config to point to theme/simple, then save the config file
+    fileTree['app/config.yml'] = 'theme_search_paths:\n  - theme/simple';
+    connection.saveDocument('app/config.yml');
+    await flushAsync();
+
+    // Definition should now resolve to theme/simple
+    const result2 = (await connection.triggerRequest(DefinitionRequest.method, params)) as any[];
+    expect(result2).toHaveLength(1);
+    expect(result2[0].targetUri).toContain('theme/simple/card.liquid');
+  });
+
+  it('should invalidate search-paths cache immediately when app/config.yml is saved', async () => {
+    connection.setup();
+    await flushAsync();
+
+    const invalidateSpy = vi.spyOn(SearchPathsLoader.prototype, 'invalidate');
+
+    // Saving app/config.yml should immediately invalidate the cache
+    connection.saveDocument('app/config.yml');
+    await flushAsync();
+
+    expect(invalidateSpy).toHaveBeenCalledOnce();
+  });
+
+  it('should NOT invalidate search-paths cache when an unrelated file is saved', async () => {
+    connection.setup();
+    await flushAsync();
+
+    const invalidateSpy = vi.spyOn(SearchPathsLoader.prototype, 'invalidate');
+
+    connection.saveDocument('app/views/partials/code.liquid');
+    await flushAsync();
+
+    expect(invalidateSpy).not.toHaveBeenCalled();
+  });
+
   it('should trigger a re-check on did delete files notifications', async () => {
     connection.setup();
     await flushAsync();

packages/platformos-language-server-common/src/test/MockConnection.ts

@@ -2,6 +2,7 @@ import { vi } from 'vitest';
 import { EventEmitter } from 'node:events';
 import { createConnection } from 'vscode-languageserver/lib/common/server';
 import {
+  CancellationToken,
   ClientCapabilities,
   DidChangeTextDocumentNotification,
   DidCloseTextDocumentNotification,
@@ -47,7 +48,11 @@ type MockConnectionMethods = {
  */
 export type MockConnection = ReturnType<typeof createConnection> & MockConnectionMethods;
 
-function protocolConnection(requests: EventEmitter, notifications: EventEmitter) {
+function protocolConnection(
+  requests: EventEmitter,
+  notifications: EventEmitter,
+  requestHandlers: Map<string, Function>,
+) {
   return {
     dispose: vi.fn(),
     end: vi.fn(),
@@ -61,12 +66,13 @@ function protocolConnection(requests: EventEmitter, notifications: EventEmitter)
       notifications.addListener(type.method, handler);
     }),
     onRequest: vi.fn().mockImplementation((type: MessageSignature, handler) => {
+      requestHandlers.set(type.method, handler);
       requests.addListener(type.method, handler);
     }),
     onUnhandledNotification: vi.fn(),
     sendNotification: vi.fn().mockReturnValue(Promise.resolve()),
     sendProgress: vi.fn(),
-    sendRequest: vi.fn(),
+    sendRequest: vi.fn().mockReturnValue(Promise.resolve()),
     trace: vi.fn().mockReturnValue(Promise.resolve()),
   } satisfies ProtocolConnection;
 }
@@ -80,7 +86,8 @@ export function mockConnection(rootUri: string): MockConnection {
 
   const requests = new EventEmitter();
   const notifications = new EventEmitter();
-  const spies = protocolConnection(requests, notifications);
+  const requestHandlers = new Map<string, Function>();
+  const spies = protocolConnection(requests, notifications, requestHandlers);
 
   // Create a real "connection" with the fake communication channel
   const connection = createConnection(() => spies, watchDog);
@@ -92,10 +99,15 @@ export function mockConnection(rootUri: string): MockConnection {
     notifications.emit(method, params);
   };
 
-  // Create a mock way to trigger requests in our tests
+  // Create a mock way to trigger requests in our tests and get the response back.
+  // Calls the registered handler directly so that the return value is available.
   const triggerRequest: MockConnection['triggerRequest'] = async (...args: any[]) => {
     const [type, params] = args;
     const method = typeof type === 'string' ? type : type.method;
+    const handler = requestHandlers.get(method);
+    if (handler) {
+      return handler(params, CancellationToken.None);
+    }
     requests.emit(method, params);
   };