Improve YAML parsing error message for unresolve alias (Shopify#297)

Fixes Shopify#276

.changeset/silver-terms-happen.md

@@ -0,0 +1,23 @@
+---
+'@shopify/theme-check-node': patch
+---
+
+Improve YAML parsing error messages caused by YAML aliases
+
+```yaml
+ignore:
+  # this causes an error because * at the start of a statement in YAML
+  # is used for aliases to anchors such as `&.code-workspaces`
+  - *.code-workspaces
+
+  # what you want is this
+  - '*.code-worksapces'
+```
+
+It’s a pretty obscure error for the non-initiated, so now we’ll
+instead throw an error like this:
+
+```
+YAML parsing error: Unresolved alias *.code-workspaces
+Did you forget to wrap it in quotes? '*.code-workspaces'
+```

.vscode/settings.json

@@ -9,7 +9,5 @@
         "dist": true // set this to false to include "dist" folder in search results
     },
     // Turn off tsc task auto detection since we have the necessary tasks as npm scripts
-    "typescript.tsc.autoDetect": "off",
-    "vitest.commandLine": "yarn test --",
-    "vitest.showFailMessages": true
+    "typescript.tsc.autoDetect": "off"
 }

package.json

@@ -43,7 +43,7 @@
     "ts-node": "^10.9.1",
     "tsconfig-paths-webpack-plugin": "^4.1.0",
     "typescript": "^4.9.3",
-    "vitest": "^0.34.1",
+    "vitest": "^1.3.0",
     "vscode-languageserver-protocol": "^3.17.3",
     "vscode-languageserver-textdocument": "^1.0.8",
     "webpack": "^5.76.0",

packages/prettier-plugin-liquid/vitest.config.ts

@@ -5,7 +5,13 @@ import { configDefaults } from 'vitest/config';
 export default defineConfig({
   test: {
     exclude: [...configDefaults.exclude],
-    singleThread: true,
+    pool: 'threads',
+    poolOptions: {
+      threads: {
+        singleThread: true,
+        isolate: true,
+      },
+    },
     globalSetup: ['./src/test/test-setup.js'],
     setupFiles: ['../liquid-html-parser/build/shims.js'],
   },

packages/theme-check-node/src/config/resolve/read-yaml.spec.ts

@@ -236,11 +236,21 @@ describe('Unit: readYamlConfigDescription', () => {
 
   it('throws an error when a check setting value is not a plain object', async () => {
     const filePath = await createMockYamlFile('SomeCheck: not_an_object');
-    await expect(readYamlConfigDescription(filePath)).rejects.toThrow(
+    await expect(readYamlConfigDescription(filePath)).rejects.toThrowError(
       'Expected a plain object value for SomeCheck but got string',
     );
   });
 
+  it('throws a meaningful error message when the YAML alias error is thrown', async () => {
+    const filePath = await createMockYamlFile(`
+ignore:
+  - *.code-workspace
+`);
+    expect(readYamlConfigDescription(filePath)).rejects.toThrowError(
+      /YAML parsing error: Unresolved alias \*\.code-workspace/,
+    );
+  });
+
   it('does not complain over legacy settings', async () => {
     const filePath = await createMockYamlFile(`
 include_categories: []

packages/theme-check-node/src/config/resolve/read-yaml.ts

@@ -11,6 +11,41 @@ import {
   ConfigFragment,
 } from '../types';
 
+class UnresolvedAliasError extends Error {
+  constructor(message: string, alias: string) {
+    super(message);
+    this.name = 'UnresolvedAliasError';
+    this.message = `YAML parsing error: Unresolved alias *${alias}.
+Did you forget to wrap your ignore statement in quotes? '*${alias}'
+${message}`;
+  }
+}
+
+function parseYamlFile(absolutePath: string, contents: string): { [k in string]: any } {
+  try {
+    const result = contents.trim() === '' ? {} : parse(contents);
+    if (!isPlainObject(result)) {
+      throw new Error(
+        `Expecting parsed contents of config file at path '${absolutePath}' to be a plain object`,
+      );
+    }
+
+    return result;
+  } catch (error) {
+    if (
+      error instanceof Error &&
+      error.name === 'ReferenceError' &&
+      error.message.includes('Unresolved alias') &&
+      /: .*$/m.test(error.message)
+    ) {
+      const alias = /: .*$/m.exec(error.message)![0].slice(2);
+      throw new UnresolvedAliasError(error.message, alias);
+    } else {
+      throw error;
+    }
+  }
+}
+
 /**
  * Takes an absolute path, parses the yaml at that path and turns it into a
  * ConfigFragment object.
@@ -23,13 +58,7 @@ export async function readYamlConfigDescription(
 ): Promise<ConfigFragment> {
   const root = path.dirname(absolutePath);
   const contents = await fs.readFile(absolutePath, 'utf8');
-  const yamlFile = contents.trim() === '' ? {} : parse(contents);
-
-  if (!isPlainObject(yamlFile)) {
-    throw new Error(
-      `Expecting parsed contents of config file at path '${absolutePath}' to be a plain object`,
-    );
-  }
+  const yamlFile = parseYamlFile(absolutePath, contents);
 
   const config: ConfigFragment = {
     checkSettings: {},

vitest.config.ts

@@ -8,7 +8,13 @@ const prettierPluginExclude = ['./packages/prettier-plugin-liquid'];
 export default defineConfig({
   test: {
     exclude: CI ? [...configDefaults.exclude, ...prettierPluginExclude] : configDefaults.exclude,
-    singleThread: true,
+    pool: 'threads',
+    poolOptions: {
+      threads: {
+        singleThread: true,
+        isolate: true,
+      },
+    },
     setupFiles: [
       './packages/theme-check-common/src/test/test-setup.ts',
       './packages/theme-language-server-common/src/test/test-setup.ts',