Fix RemoteAsset false positives for hash URLs and variable lookups

- Add isHashUrl() to skip linting hash-only URLs (#anchor, #section)
- Add startsWithVariableLookup() to skip linting when attribute value
  starts with a VariableLookup (can't statically analyze variables)
- Add test cases for both patterns

Author: charlespwd

packages/theme-check-common/src/checks/remote-asset/index.spec.ts

@@ -183,19 +183,39 @@ describe('Module: RemoteAsset', () => {
     );
   });
 
-  it('should report an offense for image drops without img_url filter', async () => {
+  it('should not report an offense for variable-based URLs', async () => {
     const sourceCode = `
       <img src="{{ image }}">
       <img src="{{ image.src }}">
+      <source src="{{ source.url }}" type="{{ source.mime_type }}">
+      <source src="{{ video_source.url }}">
     `;
 
     const offenses = await runLiquidCheck(RemoteAsset, sourceCode);
-    expect(offenses).to.have.length(2);
-    offenses.forEach((offense) => {
-      expect(offense.message).to.equal(
-        'Use one of the asset_url filters to serve assets for better performance.',
-      );
-    });
+    expect(offenses).to.be.empty;
+  });
+
+  it('should not report an offense for hash URLs', async () => {
+    const sourceCode = `
+      <link rel="expect" href="#MainContent" blocking="render">
+      <link href="#section-header" rel="preload">
+      <img src="#placeholder">
+    `;
+
+    const offenses = await runLiquidCheck(RemoteAsset, sourceCode);
+    expect(offenses).to.be.empty;
+  });
+
+  it('should still report an offense for string literals without asset_url filter', async () => {
+    const sourceCode = `
+      <img src="{{ 'image.png' }}" />
+    `;
+
+    const offenses = await runLiquidCheck(RemoteAsset, sourceCode);
+    expect(offenses).to.have.length(1);
+    expect(offenses[0].message).to.equal(
+      'Use one of the asset_url filters to serve assets for better performance.',
+    );
   });
 
   it('should not report an offence if url is a shopify CDN', async () => {

packages/theme-check-common/src/checks/remote-asset/index.ts

@@ -43,6 +43,29 @@ function isLiquidVariable(node: LiquidHtmlNode | string): node is LiquidVariable
   return typeof node !== 'string' && node.type === NodeTypes.LiquidVariable;
 }
 
+function isHashUrl(url: string): boolean {
+  return url.startsWith('#');
+}
+
+/**
+ * Checks if the attribute value starts with a variable lookup.
+ * When a value starts with a VariableLookup (e.g., {{ source.url }}, {{ image }}),
+ * we can't statically analyze what it contains, so we skip linting.
+ * This prevents false positives for cases like video_source.url which already
+ * returns CDN-hosted URLs.
+ */
+function startsWithVariableLookup(attr: ValuedHtmlAttribute): boolean {
+  const firstNode = attr.value[0];
+  if (!firstNode) return false;
+
+  if (!isLiquidVariableOutput(firstNode)) return false;
+
+  const variable = firstNode.markup;
+  if (!isLiquidVariable(variable)) return false;
+
+  return variable.expression.type === NodeTypes.VariableLookup;
+}
+
 function isUrlHostedbyShopify(url: string, allowedDomains: string[] = []): boolean {
   if (/^\/cdn\//.test(url)) {
     return true;
@@ -157,6 +180,13 @@ export const RemoteAsset: LiquidCheckDefinition<typeof schema> = {
 
       if (!urlAttribute) return;
 
+      const firstTextNode = urlAttribute.value.find(
+        (node): node is TextNode => node.type === NodeTypes.TextNode,
+      );
+      if (firstTextNode && isHashUrl(firstTextNode.value)) return;
+
+      if (startsWithVariableLookup(urlAttribute)) return;
+
       const isShopifyUrl = urlAttribute.value
         .filter((node): node is TextNode => node.type === NodeTypes.TextNode)
         .some((textNode) => isUrlHostedbyShopify(textNode.value, allowedDomains));